WFCORE-6407 Deprecate non-default contructors from AbstractAddStepHandler and AbstractWriteAttributeHandler

controller/src/main/java/org/jboss/as/controller/AbstractAddStepHandler.java

@@ -6,18 +6,19 @@
 package org.jboss.as.controller;
 
 import org.jboss.as.controller.capability.RuntimeCapability;
+import org.jboss.as.controller.descriptions.ModelDescriptionConstants;
+import org.jboss.as.controller.registry.AttributeAccess;
 import org.jboss.as.controller.registry.ImmutableManagementResourceRegistration;
 import org.jboss.as.controller.registry.Resource;
 import org.jboss.dmr.ModelNode;
 
 import java.util.AbstractSet;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.ADD_INDEX;
@@ -26,23 +27,32 @@
  * Base class for {@link OperationStepHandler} implementations that add managed resource.
  *
  * @author John Bailey
+ * @author Paul Ferraro
  */
-public class AbstractAddStepHandler implements OperationStepHandler, OperationDescriptor {
+public abstract class AbstractAddStepHandler implements OperationStepHandler, OperationDescriptor {
 
+    private static final String AUTO_POPULATE_MODEL = "auto-populate-model";
+
+    /**
+     * This is only retained to support {@link #getAttributes()} and for those subclasses that reference this protected attribute directly.
+     */
+    @Deprecated(forRemoval = true) // This is referenceable by subclasses
     protected final Collection<? extends AttributeDefinition> attributes;
 
     /**
      * Constructs an add handler.
      */
-    public AbstractAddStepHandler() { //default constructor to preserve backward compatibility
+    public AbstractAddStepHandler() {
         this.attributes = List.of();
     }
 
     /**
      * Constructs an add handler
      * @param attributes attributes to use in {@link #populateModel(OperationContext, org.jboss.dmr.ModelNode, org.jboss.as.controller.registry.Resource)}
+     * @deprecated Use default constructor instead. Resource model auto-population logic relies on the {@link OperationDefinition#getParameters()} method of the operation definition with which this handler was registered.
      */
     @SuppressWarnings("unchecked")
+    @Deprecated(forRemoval = true)
     public AbstractAddStepHandler(Collection<? extends AttributeDefinition> attributes) {
         // Create defensive copy, if collection was not already immutable
         this.attributes = (attributes instanceof Set) ? Set.copyOf((Set<AttributeDefinition>) attributes) : List.copyOf(attributes);
@@ -52,16 +62,30 @@ public AbstractAddStepHandler(Collection<? extends AttributeDefinition> attribut
      * Constructs an add handler
      *
      * @param attributes attributes to use in {@link #populateModel(OperationContext, org.jboss.dmr.ModelNode, org.jboss.as.controller.registry.Resource)}
+     * @deprecated Use default constructor instead. Resource model auto-population logic relies on the {@link OperationDefinition#getParameters()} method of the operation definition with which this handler was registered.
      */
+    @Deprecated(forRemoval = true)
     public AbstractAddStepHandler(AttributeDefinition... attributes) {
         this(List.of(attributes));
     }
 
+    /**
+     * Constructs an add handler
+     *
+     * @param attributes attributes to use in {@link #populateModel(OperationContext, org.jboss.dmr.ModelNode, org.jboss.as.controller.registry.Resource)}
+     * @deprecated Use default constructor instead. Resource model auto-population logic relies on the {@link OperationDefinition#getParameters()} method of the operation definition with which this handler was registered.
+     */
+    @Deprecated(forRemoval = true)
     public AbstractAddStepHandler(Parameters parameters) {
         this.attributes = parameters.attributes;
     }
 
+    /**
+     * Returns the attributes with which this handler was constructed.
+     * @deprecated Use the {@link OperationDefinition#getParameters()} method of the operation definition with which this handler was registered instead.
+     */
     @Override
+    @Deprecated(forRemoval = true)
     public Collection<? extends AttributeDefinition> getAttributes() {
         return this.attributes;
     }
@@ -140,6 +164,26 @@ protected Resource createResource(final OperationContext context) {
      */
     protected void populateModel(final OperationContext context, final ModelNode operation, final Resource resource) throws  OperationFailedException {
         populateModel(operation, resource);
+
+        // Detect operation header written by populateModel(ModelNode, ModelNode)
+        // If header exists, then subclass expects us to populate the model
+        if (operation.hasDefined(ModelDescriptionConstants.OPERATION_HEADERS, AUTO_POPULATE_MODEL)) {
+            ModelNode model = resource.getModel();
+            ImmutableManagementResourceRegistration registration = context.getResourceRegistration();
+            Map<String, AttributeAccess> attributes = registration.getAttributes(PathAddress.EMPTY_ADDRESS);
+            for (AttributeDefinition parameter : registration.getOperationEntry(PathAddress.EMPTY_ADDRESS, ModelDescriptionConstants.ADD).getOperationDefinition().getParameters()) {
+                AttributeAccess attribute = attributes.get(parameter.getName());
+                if ((attribute != null) && !AttributeAccess.Flag.ALIAS.test(attribute)) {
+                    // Auto-populate add resource operation parameters that correspond to resource attributes, omitting aliases
+                    parameter.validateAndSet(operation, model);
+                } else {
+                    // Otherwise, just validate parameter
+                    parameter.validateOperation(operation);
+                }
+            }
+            // Remove header added via populateModel(ModelNode, ModelNode)
+            operation.get(ModelDescriptionConstants.OPERATION_HEADERS).remove(AUTO_POPULATE_MODEL);
+        }
     }
 
     /**
@@ -152,7 +196,9 @@ protected void populateModel(final OperationContext context, final ModelNode ope
      * @param resource the resource that corresponds to the address of {@code operation}
      *
      * @throws OperationFailedException if {@code operation} is invalid or populating the model otherwise fails
+     * @deprecated Override {@link #populateModel(OperationContext, ModelNode, Resource)} if necessary
      */
+    @Deprecated(forRemoval = true)
     protected void populateModel(final ModelNode operation, final Resource resource) throws  OperationFailedException {
         populateModel(operation, resource.getModel());
     }
@@ -168,11 +214,14 @@ protected void populateModel(final ModelNode operation, final Resource resource)
      * @param model persistent configuration model node that corresponds to the address of {@code operation}
      *
      * @throws OperationFailedException if {@code operation} is invalid or populating the model otherwise fails
+     * @deprecated Override {@link #populateModel(OperationContext, ModelNode, Resource)} if necessary
      */
+    @Deprecated(forRemoval = true)
     protected void populateModel(final ModelNode operation, final ModelNode model) throws OperationFailedException {
-        for (AttributeDefinition attr : attributes) {
-            attr.validateAndSet(operation, model);
-        }
+        // Previously model auto-population happened here based on attributes provided via constructor
+        // If this method was invoked, we know that the subclass expects us to populate the model
+        // If so, indicate this via an operation header to be detected by our parent method
+        operation.get(ModelDescriptionConstants.OPERATION_HEADERS, AUTO_POPULATE_MODEL).set(true);
     }
 
     /**
@@ -196,7 +245,8 @@ protected void populateModel(final ModelNode operation, final ModelNode model) t
      */
     protected void recordCapabilitiesAndRequirements(final OperationContext context, final ModelNode operation, Resource resource) throws OperationFailedException {
 
-        for (RuntimeCapability<?> capability : context.getResourceRegistration().getCapabilities()) {
+        ImmutableManagementResourceRegistration registration = context.getResourceRegistration();
+        for (RuntimeCapability<?> capability : registration.getCapabilities()) {
             if (capability.isDynamicallyNamed()) {
                 context.registerCapability(capability.fromBaseCapability(context.getCurrentAddress()));
             } else {
@@ -205,14 +255,18 @@ protected void recordCapabilitiesAndRequirements(final OperationContext context,
         }
 
         ModelNode model = resource.getModel();
-        for (AttributeDefinition ad : attributes) {
-            if (model.hasDefined(ad.getName()) || ad.hasCapabilityRequirements()) {
-                ad.addCapabilityRequirements(context, resource, model.get(ad.getName()));
+        for (AttributeAccess attribute : registration.getAttributes(PathAddress.EMPTY_ADDRESS).values()) {
+            // Skip runtime attributes and aliases
+            if (AttributeAccess.Storage.RUNTIME.test(attribute) || AttributeAccess.Flag.ALIAS.test(attribute)) continue;
+
+            AttributeDefinition definition = attribute.getAttributeDefinition();
+            String attributeName = definition.getName();
+            if (model.hasDefined(attributeName) || definition.hasCapabilityRequirements()) {
+                definition.addCapabilityRequirements(context, resource, model.get(attributeName));
             }
         }
-        ImmutableManagementResourceRegistration mrr = context.getResourceRegistration();
-        assert mrr.getRequirements() != null;
-        for (CapabilityReferenceRecorder recorder : mrr.getRequirements()) {
+        assert registration.getRequirements() != null;
+        for (CapabilityReferenceRecorder recorder : registration.getRequirements()) {
             recorder.addCapabilityRequirements(context, resource, null);
         }
     }
@@ -326,24 +380,6 @@ public OrderedResourceCreator(boolean indexedAdd, Set<String> orderedChildTypes)
             this.orderedChildTypes = orderedChildTypes == null ? Collections.emptySet() : orderedChildTypes;
         }
 
-        /**
-         * Constructor
-         *
-         * @param indexedAdd if ({@code true} this is the child of a parent with ordered children,
-         * and this child will be added at the {@code add-index} of the {@code add} operation in the
-         * parent's list of children of this type. If {@code false} this is a normal child, i.e. the
-         * insert will always happen at the end of the list as normal.
-         * @param orderedChildTypes if not {@code null} or empty, this indicates that this is a parent
-         * resource with ordered children, and the entries here are the type names of children which
-         * are ordered.
-         */
-        public OrderedResourceCreator(boolean indexedAdd, String... orderedChildTypes) {
-            this.indexedAdd = indexedAdd;
-            Set<String> set = new HashSet<>(orderedChildTypes.length);
-            set.addAll(Arrays.asList(orderedChildTypes));
-            this.orderedChildTypes = set;
-        }
-
         @Override
         public Resource createResource(OperationContext context, ModelNode operation) {
             // Creates a parent with ordered children (if set is not empty)
@@ -363,6 +399,10 @@ public Resource createResource(OperationContext context, ModelNode operation) {
         }
     }
 
+    /**
+     * @deprecated Use default constructor instead.  Operation parameters are determined via the {@link OperationDefinition#getParameters()} of the {@code ModelDescriptionConstants#ADD} operation associated resource.
+     */
+    @Deprecated(forRemoval = true)
     public static class Parameters {
         // Set is not the ideal data structure, but since this is a protected field, we are stuck with it
         protected Set<AttributeDefinition> attributes = Set.of();

controller/src/main/java/org/jboss/as/controller/AbstractBoottimeAddStepHandler.java

@@ -29,26 +29,20 @@
  */
 public abstract class AbstractBoottimeAddStepHandler extends AbstractAddStepHandler {
 
-    /**
-     * {@inheritDoc}
-     */
     protected AbstractBoottimeAddStepHandler() {
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    @Deprecated(forRemoval = true)
     protected AbstractBoottimeAddStepHandler(Collection<? extends AttributeDefinition> attributes) {
         super(attributes);
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    @Deprecated(forRemoval = true)
     protected AbstractBoottimeAddStepHandler(AttributeDefinition... attributes) {
         super(attributes);
     }
 
+    @Deprecated(forRemoval = true)
     public AbstractBoottimeAddStepHandler(Parameters parameters) {
         super(parameters);
     }

controller/src/main/java/org/jboss/as/controller/AbstractRemoveStepHandler.java

@@ -33,6 +33,7 @@ public abstract class AbstractRemoveStepHandler implements OperationStepHandler
     protected AbstractRemoveStepHandler() {
     }
 
+    @Override
     public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
 
         Resource resource = context.readResource(PathAddress.EMPTY_ADDRESS);
@@ -49,6 +50,7 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
 
             if (requiresRuntime(context)) {
                 context.addStep(new OperationStepHandler() {
+                    @Override
                     public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
                         performRuntime(context, operation, model);
 
@@ -153,28 +155,27 @@ protected void performRemove(OperationContext context, final ModelNode operation
      *                 is invoked before that method is. Will not be {@code null}
      */
     protected void recordCapabilitiesAndRequirements(OperationContext context, ModelNode operation, Resource resource) throws OperationFailedException {
-        Set<RuntimeCapability> capabilitySet = context.getResourceRegistration().getCapabilities();
-
-        for (RuntimeCapability capability : capabilitySet) {
+        ImmutableManagementResourceRegistration registration = context.getResourceRegistration();
+        for (RuntimeCapability<?> capability : registration.getCapabilities()) {
             if (capability.isDynamicallyNamed()) {
                 context.deregisterCapability(capability.getDynamicName(context.getCurrentAddress()));
             } else {
                 context.deregisterCapability(capability.getName());
             }
         }
         ModelNode model = resource.getModel();
-        ImmutableManagementResourceRegistration mrr = context.getResourceRegistration();
-        for (String attr : mrr.getAttributeNames(PathAddress.EMPTY_ADDRESS)) {
-            AttributeAccess aa = mrr.getAttributeAccess(PathAddress.EMPTY_ADDRESS, attr);
-            if (aa != null) {
-                AttributeDefinition ad = aa.getAttributeDefinition();
-                if (ad != null && (model.hasDefined(ad.getName()) || ad.hasCapabilityRequirements())) {
-                    ad.removeCapabilityRequirements(context, resource, model.get(ad.getName()));
-                }
+        for (AttributeAccess attribute : registration.getAttributes(PathAddress.EMPTY_ADDRESS).values()) {
+            // Skip runtime attributes and aliases
+            if (AttributeAccess.Storage.RUNTIME.test(attribute) || AttributeAccess.Flag.ALIAS.test(attribute)) continue;
+
+            AttributeDefinition definition = attribute.getAttributeDefinition();
+            String attributeName = definition.getName();
+            if (model.hasDefined(attributeName) || definition.hasCapabilityRequirements()) {
+                definition.removeCapabilityRequirements(context, resource, model.get(attributeName));
             }
         }
-        assert mrr.getRequirements() != null;
-        for (CapabilityReferenceRecorder recorder : mrr.getRequirements()) {
+        assert registration.getRequirements() != null;
+        for (CapabilityReferenceRecorder recorder : registration.getRequirements()) {
             recorder.removeCapabilityRequirements(context, resource, null);
         }
     }
@@ -210,7 +211,7 @@ protected boolean requiresRuntime(OperationContext context) {
         return context.isDefaultRequiresRuntime();
     }
 
-    private static boolean hasResource (OperationContext context) {
+    private static boolean hasResource(OperationContext context) {
         try {
             context.readResource(PathAddress.EMPTY_ADDRESS, false);
             return true;