Skip to content

Commit

Permalink
Merge pull request #5897 from pferraro/WFCORE-6730
Browse files Browse the repository at this point in the history 
WFCORE-6730 A registered operation should never contain parameters or reply parameters not enabled by stability-level of process
  • Loading branch information
@yersan
yersan committed Mar 20, 2024
2 parents 1e0bc9f + 7911f83 commit 2d71776
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 9 deletions.
7 changes: 7 additions & 0 deletions controller/src/main/java/org/jboss/as/controller/SimpleOperationDefinition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,11 @@ public DescriptionProvider getDescriptionProvider() {

private static final DescriptionProvider PRIVATE_PROVIDER = locale -> new ModelNode();

ResourceDescriptionResolver getResolver() {
return this.resolver;
}

ResourceDescriptionResolver getAttributeResolver() {
return this.attributeResolver;
}
}
55 changes: 49 additions & 6 deletions controller/src/main/java/org/jboss/as/controller/SimpleOperationDefinitionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Set;

import org.jboss.as.controller.access.management.AccessConstraintDefinition;
import org.jboss.as.controller.descriptions.DescriptionProvider;
Expand All @@ -23,17 +24,60 @@
*/
public class SimpleOperationDefinitionBuilder {

private static AttributeDefinition[] NO_ATTRIBUTES = new AttributeDefinition[0];
private static final AttributeDefinition[] NO_ATTRIBUTES = new AttributeDefinition[0];

public static SimpleOperationDefinitionBuilder of(String name, ResourceDescriptionResolver resolver) {
return new SimpleOperationDefinitionBuilder(name, resolver);
}

/**
* Convenience method that delegates to {@link #of(String, SimpleOperationDefinition)}.
* @param name the operation name
* @param basis an operation used to configure a new operation
* @return a new operation definition builder
* @throws IllegalArgumentException if the specified basis is not a {@link SimpleOperationDefinition}.
*/
public static SimpleOperationDefinitionBuilder of(String name, OperationDefinition basis) {
if (!(basis instanceof SimpleOperationDefinition)) {
throw new IllegalArgumentException();
}
return of(name, (SimpleOperationDefinition) basis);
}

/**
* Creates an operation definition builder with the specified name, based on the configuration of the specified operation definition.
* @param name the operation name
* @param basis an operation used to configure a new operation
* @return a new operation definition builder
*/
public static SimpleOperationDefinitionBuilder of(String name, SimpleOperationDefinition basis) {
SimpleOperationDefinitionBuilder builder = new SimpleOperationDefinitionBuilder(name, basis.getResolver())
.setAccessConstraints(basis.getAccessConstraints().toArray(AccessConstraintDefinition[]::new))
.setAttributeResolver(basis.getAttributeResolver())
.setDescriptionProvider(basis.getDescriptionProvider())
.setEntryType(basis.getEntryType())
.setParameters(basis.getParameters())
.setReplyParameters(basis.getReplyParameters())
.setReplyType(basis.getReplyType())
.setReplyValueType(basis.getReplyValueType())
.setStability(basis.getStability())
.withFlags(basis.getFlags())
;
if (basis.isReplyAllowNull()) {
builder.allowReturnNull();
}
DeprecationData deprecation = basis.getDeprecationData();
if (deprecation != null) {
builder.setDeprecated(deprecation.getSince(), deprecation.isNotificationUseful());
}
return builder;
}

ResourceDescriptionResolver resolver;
ResourceDescriptionResolver attributeResolver;
protected String name;
protected OperationEntry.EntryType entryType = OperationEntry.EntryType.PUBLIC;
protected EnumSet<OperationEntry.Flag> flags = EnumSet.noneOf(OperationEntry.Flag.class);
protected Set<OperationEntry.Flag> flags = EnumSet.noneOf(OperationEntry.Flag.class);
protected AttributeDefinition[] parameters = NO_ATTRIBUTES;
protected ModelType replyType;
protected ModelType replyValueType;
Expand All @@ -49,7 +93,6 @@ public SimpleOperationDefinitionBuilder(String name, ResourceDescriptionResolver
this.resolver = resolver;
}


public SimpleOperationDefinition build() {
if (attributeResolver == null) {
attributeResolver = resolver;
Expand Down Expand Up @@ -79,7 +122,7 @@ public SimpleOperationDefinitionBuilder setPrivateEntry() {
return this;
}

public SimpleOperationDefinitionBuilder withFlags(EnumSet<OperationEntry.Flag> flags) {
public SimpleOperationDefinitionBuilder withFlags(Set<OperationEntry.Flag> flags) {
this.flags.addAll(flags);
return this;
}
Expand All @@ -102,12 +145,12 @@ public SimpleOperationDefinitionBuilder setReadOnly() {
return withFlag(OperationEntry.Flag.READ_ONLY);
}

public SimpleOperationDefinitionBuilder setParameters(AttributeDefinition... parameters) {//todo add validation for same param name
public SimpleOperationDefinitionBuilder setParameters(AttributeDefinition... parameters) {
this.parameters = parameters;
return this;
}

public SimpleOperationDefinitionBuilder addParameter(AttributeDefinition parameter) {
public SimpleOperationDefinitionBuilder addParameter(AttributeDefinition parameter) {//todo add validation for same param name
int i = parameters.length;
parameters = Arrays.copyOf(parameters, i + 1);
parameters[i] = parameter;
Expand Down
14 changes: 14 additions & 0 deletions controller/src/main/java/org/jboss/as/controller/registry/ConcreteResourceRegistration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.jboss.as.controller.AttributeDefinition;
import org.jboss.as.controller.CapabilityReferenceRecorder;
Expand All @@ -33,6 +34,7 @@
import org.jboss.as.controller.ProvidedResourceDefinition;
import org.jboss.as.controller.ProxyController;
import org.jboss.as.controller.ResourceDefinition;
import org.jboss.as.controller.SimpleOperationDefinitionBuilder;
import org.jboss.as.controller.access.management.AccessConstraintDefinition;
import org.jboss.as.controller.access.management.AccessConstraintUtilizationRegistry;
import org.jboss.as.controller.capability.RuntimeCapability;
Expand Down Expand Up @@ -230,6 +232,18 @@ public void registerOperationHandler(OperationDefinition definition, OperationSt
if (this.enables(definition)) {
String opName = definition.getName();
OperationEntry entry = new OperationEntry(definition, handler, inherited);
boolean filterParameters = !Stream.of(definition.getParameters()).allMatch(this::enables);
boolean filterReplyParameters = !Stream.of(definition.getReplyParameters()).allMatch(this::enables);
if (filterParameters || filterReplyParameters) {
SimpleOperationDefinitionBuilder builder = SimpleOperationDefinitionBuilder.of(opName, definition);
if (filterParameters) {
builder.setParameters(Stream.of(definition.getParameters()).filter(this::enables).toArray(AttributeDefinition[]::new));
}
if (filterReplyParameters) {
builder.setReplyParameters(Stream.of(definition.getReplyParameters()).filter(this::enables).toArray(AttributeDefinition[]::new));
}
entry = new OperationEntry(builder.build(), handler, inherited);
}
writeLock.lock();
try {
if (operations == null) {
Expand Down
5 changes: 2 additions & 3 deletions controller/src/main/java/org/jboss/as/controller/registry/OperationEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package org.jboss.as.controller.registry;

import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -62,8 +61,8 @@ public enum Flag {
* use of {@link EntryType#PRIVATE} not workable. */
HIDDEN;

private static final Map<EnumSet<Flag>, Set<Flag>> flagSets = new ConcurrentHashMap<>(16);
public static Set<OperationEntry.Flag> immutableSetOf(EnumSet<OperationEntry.Flag> flags) {
private static final Map<Set<Flag>, Set<Flag>> flagSets = new ConcurrentHashMap<>(16);
public static Set<OperationEntry.Flag> immutableSetOf(Set<OperationEntry.Flag> flags) {
if (flags == null || flags.isEmpty()) {
return Collections.emptySet();
}
Expand Down
34 changes: 34 additions & 0 deletions ...java/org/jboss/as/controller/registry/CoreManagementResourceRegistrationUnitTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.util.List;
import java.util.Set;

import org.jboss.as.controller.AttributeDefinition;
import org.jboss.as.controller.OperationContext;
import org.jboss.as.controller.OperationDefinition;
import org.jboss.as.controller.OperationFailedException;
Expand All @@ -23,14 +25,17 @@
import org.jboss.as.controller.ProcessType;
import org.jboss.as.controller.ResourceDefinition;
import org.jboss.as.controller.ResourceRegistration;
import org.jboss.as.controller.SimpleAttributeDefinitionBuilder;
import org.jboss.as.controller.SimpleOperationDefinitionBuilder;
import org.jboss.as.controller.SimpleResourceDefinition;
import org.jboss.as.controller.SimpleResourceDefinition.Parameters;
import org.jboss.as.controller.access.management.AccessConstraintDefinition;
import org.jboss.as.controller.access.management.ApplicationTypeAccessConstraintDefinition;
import org.jboss.as.controller.access.management.SensitiveTargetAccessConstraintDefinition;
import org.jboss.as.controller.descriptions.NonResolvingResourceDescriptionResolver;
import org.jboss.as.version.Stability;
import org.jboss.dmr.ModelNode;
import org.jboss.dmr.ModelType;
import org.junit.Before;
import org.junit.Test;

Expand Down Expand Up @@ -339,6 +344,35 @@ public void testInheritedAccessConstraints() {
assertTrue(acds.contains(ApplicationTypeAccessConstraintDefinition.DEPLOYMENT));
}

@Test
public void testFilteredOperationParameters() {
OperationDefinition unstableOperation = SimpleOperationDefinitionBuilder.of("unstable-operation", NonResolvingResourceDescriptionResolver.INSTANCE).setStability(Stability.PREVIEW).build();
OperationStepHandler handler = (context, model) -> {};
this.rootRegistration.registerOperationHandler(unstableOperation, handler);
assertNull(this.rootRegistration.getOperationEntry(PathAddress.EMPTY_ADDRESS, "unstable-operation"));

AttributeDefinition stableParameter = new SimpleAttributeDefinitionBuilder("stable", ModelType.STRING).build();
AttributeDefinition unstableParameter = new SimpleAttributeDefinitionBuilder("unstable", ModelType.STRING).setStability(Stability.EXPERIMENTAL).build();
AttributeDefinition stableReplyParameter = new SimpleAttributeDefinitionBuilder("stable-reply", ModelType.STRING).build();
AttributeDefinition unstableReplyParameter = new SimpleAttributeDefinitionBuilder("unstable-reply", ModelType.STRING).setStability(Stability.EXPERIMENTAL).build();
OperationDefinition stableOperation = SimpleOperationDefinitionBuilder.of("stable-operation", NonResolvingResourceDescriptionResolver.INSTANCE)
.setParameters(stableParameter, unstableParameter)
.setReplyParameters(stableReplyParameter, unstableReplyParameter)
.build();
this.rootRegistration.registerOperationHandler(stableOperation, handler);

OperationEntry entry = this.rootRegistration.getOperationEntry(PathAddress.EMPTY_ADDRESS, "stable-operation");
assertNotNull(entry);
assertSame(handler, entry.getOperationHandler());

OperationDefinition registeredOperation = entry.getOperationDefinition();
assertNotSame(stableOperation, registeredOperation);
assertEquals(1, registeredOperation.getParameters().length);
assertSame(stableParameter, registeredOperation.getParameters()[0]);
assertEquals(1, registeredOperation.getReplyParameters().length);
assertSame(stableReplyParameter, registeredOperation.getReplyParameters()[0]);
}

private static class TestHandler implements OperationStepHandler {

private static TestHandler INSTANCE = new TestHandler();
Expand Down

0 comments on commit 2d71776

Please sign in to comment.