Merge pull request #6126 from pferraro/WFCORE-6945

WFCORE-6945 Required child resource logic in wildfly-subsystem is not stability-level aware

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

@@ -5,6 +5,8 @@
 
 package org.jboss.as.controller;
 
+import java.util.Objects;
+
 import org.jboss.as.version.Stability;
 
 /**
@@ -32,12 +34,7 @@ static ResourceRegistration root() {
      * @return a resource registration
      */
     static ResourceRegistration of(PathElement path) {
-        return new ResourceRegistration() {
-            @Override
-            public PathElement getPathElement() {
-                return path;
-            }
-        };
+        return (path != null) ? new DefaultResourceRegistration(path) : DefaultResourceRegistration.ROOT;
     }
 
     /**
@@ -47,16 +44,42 @@ public PathElement getPathElement() {
      * @return a resource registration
      */
     static ResourceRegistration of(PathElement path, Stability stability) {
-        return new ResourceRegistration() {
-            @Override
-            public PathElement getPathElement() {
-                return path;
-            }
-
+        return (path != null) || (stability != DefaultResourceRegistration.ROOT.getStability()) ? new DefaultResourceRegistration(path) {
             @Override
             public Stability getStability() {
                 return stability;
             }
-        };
+        } : DefaultResourceRegistration.ROOT;
+    }
+
+    class DefaultResourceRegistration implements ResourceRegistration {
+        static final ResourceRegistration ROOT = new DefaultResourceRegistration(null);
+        private final PathElement path;
+
+        DefaultResourceRegistration(PathElement path) {
+            this.path = path;
+        }
+
+        @Override
+        public PathElement getPathElement() {
+            return this.path;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(this.path);
+        }
+
+        @Override
+        public boolean equals(Object object) {
+            if (!(object instanceof ResourceRegistration)) return false;
+            ResourceRegistration registration = (ResourceRegistration) object;
+            return Objects.equals(this.getPathElement(), registration.getPathElement()) && Objects.equals(this.getStability(), registration.getStability());
+        }
+
+        @Override
+        public String toString() {
+            return Objects.toString(this.path);
+        }
     }
 }

controller/src/test/java/org/jboss/as/controller/ResourceRegistrationTestCase.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright The WildFly Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.jboss.as.controller;
+
+import java.util.EnumSet;
+import java.util.Objects;
+
+import org.jboss.as.controller.descriptions.ModelDescriptionConstants;
+import org.jboss.as.version.Stability;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Unit test for {@link ResourceRegistration} factory methods.
+ */
+public class ResourceRegistrationTestCase {
+
+    @Test
+    public void test() {
+        ResourceRegistration root = ResourceRegistration.root();
+        Assert.assertNull(root.getPathElement());
+        Assert.assertSame(Stability.DEFAULT, root.getStability());
+        Assert.assertSame(root, ResourceRegistration.root());
+        Assert.assertSame(root, ResourceRegistration.of(root.getPathElement()));
+        Assert.assertSame(root, ResourceRegistration.of(root.getPathElement(), Stability.DEFAULT));
+        verify(root);
+
+        ResourceRegistration subsystem = ResourceRegistration.of(PathElement.pathElement(ModelDescriptionConstants.SUBSYSTEM, "foo"));
+        Assert.assertNotEquals(subsystem, root);
+        Assert.assertNotEquals(subsystem, ResourceRegistration.of(PathElement.pathElement(ModelDescriptionConstants.SUBSYSTEM, "bar")));
+        verify(subsystem);
+
+        ResourceRegistration unstableSubsystem = ResourceRegistration.of(PathElement.pathElement(ModelDescriptionConstants.SUBSYSTEM, "bar"), Stability.EXPERIMENTAL);
+        Assert.assertNotEquals(unstableSubsystem, root);
+        Assert.assertNotEquals(unstableSubsystem, subsystem);
+        Assert.assertNotEquals(unstableSubsystem, ResourceRegistration.of(unstableSubsystem.getPathElement()));
+        verify(unstableSubsystem);
+    }
+
+    private static void verify(ResourceRegistration subject) {
+        Assert.assertEquals(Objects.hashCode(subject.getPathElement()), subject.hashCode());
+        Assert.assertEquals(subject, ResourceRegistration.of(subject.getPathElement(), subject.getStability()));
+        for (Stability stability : EnumSet.complementOf(EnumSet.of(subject.getStability()))) {
+            Assert.assertNotEquals(subject, ResourceRegistration.of(subject.getPathElement(), stability));
+        }
+    }
+}

subsystem/src/main/java/org/wildfly/subsystem/resource/PathElementProvider.java

@@ -11,7 +11,9 @@
 /**
  * Provides a {@link PathElement}.
  * @author Paul Ferraro
+ * @deprecated To be removed without replacement
  */
+@Deprecated(forRemoval = true, since = "26.0.0")
 public interface PathElementProvider extends Supplier<PathElement> {
 
     /**

subsystem/src/main/java/org/wildfly/subsystem/resource/ResourceDescriptor.java

@@ -14,6 +14,7 @@
 import java.util.Set;
 import java.util.function.BiPredicate;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
@@ -23,6 +24,7 @@
 import org.jboss.as.controller.OperationContext;
 import org.jboss.as.controller.OperationStepHandler;
 import org.jboss.as.controller.PathElement;
+import org.jboss.as.controller.ResourceRegistration;
 import org.jboss.as.controller.capability.RuntimeCapability;
 import org.jboss.as.controller.descriptions.ModelDescriptionConstants;
 import org.jboss.as.controller.descriptions.ResourceDescriptionResolver;
@@ -159,8 +161,8 @@ class DefaultResourceDescriptor implements ResourceDescriptor {
         private final Map<RuntimeCapability<?>, BiPredicate<OperationContext, Resource>> capabilities;
         private final Map<AttributeDefinition, OperationStepHandler> readWriteAttributes = new HashMap<>();
         private final Iterable<? extends AttributeDefinition> readOnlyAttributes;
-        private final Set<PathElement> requiredChildren;
-        private final Set<PathElement> requiredSingletonChildren;
+        private final Map<PathElement, ResourceRegistration> requiredChildren;
+        private final Map<PathElement, ResourceRegistration> requiredSingletonChildren;
         private final Map<AttributeDefinition, AttributeTranslation> attributeTranslations;
         private final Set<ResourceCapabilityReferenceRecorder<?>> resourceCapabilityReferences;
         private final Map<String, UnaryOperator<OperationStepHandler>> operationTransformers;
@@ -254,12 +256,12 @@ public OperationStepHandler getWriteAttributeOperationStepHandler(AttributeDefin
         }
 
         @Override
-        public Set<PathElement> getRequiredChildren() {
+        public Map<PathElement, ResourceRegistration> getRequiredChildResources() {
             return this.requiredChildren;
         }
 
         @Override
-        public Set<PathElement> getRequiredSingletonChildren() {
+        public Map<PathElement, ResourceRegistration> getRequiredSingletonChildResources() {
             return this.requiredSingletonChildren;
         }
 
@@ -395,33 +397,77 @@ default C addCapabilities(Collection<RuntimeCapability<?>> capabilities) {
          * Defines a required child of this resource.  Required children will be automatically added, if no child resource exists with the specified path.
          * @param path the path of the required child resource
          * @return a reference to this configurator
+         * @deprecated Superseded by {@link #requireChildResource(ResourceRegistration)}.
          */
+        @Deprecated(forRemoval = true, since = "26.0.0")
         default C requireChild(PathElement path) {
-            return this.requireChildren(Set.of(path));
+            return this.requireChildResources(Set.of(ResourceRegistration.of(path)));
+        }
+
+        /**
+         * Defines a set of required children of this resource.  Required children will be automatically added, if no child resource exists with the specified path.
+         * @param paths a set of paths of the required child resources
+         * @return a reference to this configurator
+         * @deprecated Superseded by {@link #requireChildResources(Set)}.
+         */
+        @Deprecated(forRemoval = true, since = "26.0.0")
+        default C requireChildren(Set<PathElement> paths) {
+            return this.requireChildResources(paths.stream().map(ResourceRegistration::of).collect(Collectors.toUnmodifiableSet()));
+        }
+
+        /**
+         * Defines a required child of this resource.  Required children will be automatically added, if no child resource exists with the specified path.
+         * @param path the path of the required child resource
+         * @return a reference to this configurator
+         */
+        default C requireChildResource(ResourceRegistration child) {
+            return this.requireChildResources(Set.of(child));
         }
 
         /**
          * Defines a set of required children of this resource.  Required children will be automatically added, if no child resource exists with the specified path.
          * @param paths a set of paths of the required child resources
          * @return a reference to this configurator
          */
-        C requireChildren(Set<PathElement> paths);
+        C requireChildResources(Set<? extends ResourceRegistration> children);
 
         /**
          * Defines a required singleton child of this resource.  Required singleton children will be automatically added, if no child resource exists with the same path key.
          * @param path the path of the required singleton child resource
          * @return a reference to this configurator
+         * @deprecated Superseded by {@link #requireSingletonChildResource(ResourceRegistration)}.
          */
+        @Deprecated(forRemoval = true, since = "26.0.0")
         default C requireSingletonChild(PathElement path) {
-            return this.requireSingletonChildren(Set.of(path));
+            return this.requireSingletonChildResources(Set.of(ResourceRegistration.of(path)));
+        }
+
+        /**
+         * Defines a set of required singleton children of this resource.  Required singleton children will be automatically added, if no child resource exists with the same path key.
+         * @param paths a set of paths of the required singleton child resources
+         * @return a reference to this configurator
+         * @deprecated Superseded by {@link #requireSingletonChildResources(Set)}.
+         */
+        @Deprecated(forRemoval = true, since = "26.0.0")
+        default C requireSingletonChild(Set<PathElement> paths) {
+            return this.requireSingletonChildResources(paths.stream().map(ResourceRegistration::of).collect(Collectors.toUnmodifiableSet()));
+        }
+
+        /**
+         * Defines a required singleton child of this resource.  Required singleton children will be automatically added, if no child resource exists with the same path key.
+         * @param path the path of the required singleton child resource
+         * @return a reference to this configurator
+         */
+        default C requireSingletonChildResource(ResourceRegistration path) {
+            return this.requireSingletonChildResources(Set.of(path));
         }
 
         /**
          * Defines a set of required singleton children of this resource.  Required singleton children will be automatically added, if no child resource exists with the same path key.
          * @param paths a set of paths of the required singleton child resources
          * @return a reference to this configurator
          */
-        C requireSingletonChildren(Set<PathElement> paths);
+        C requireSingletonChildResources(Set<? extends ResourceRegistration> paths);
 
         /**
          * Adds a capability reference that records a requirement for this resource.
@@ -529,15 +575,23 @@ default <P extends Supplier<RuntimeCapability<?>>> C provideCapabilities(Collect
          * Defines a set of required children of this resource.  Required children will be automatically added, if no child resource exists with the specified path.
          * @param providers a set of providers of the required child resource paths
          * @return a reference to this configurator
+         * @deprecated Use {@link #requireChildResources(Set)} instead.
          */
-        <P extends Supplier<PathElement>> C provideRequiredChildren(Collection<P> providers);
+        @Deprecated(forRemoval = true, since = "26.0.0")
+        default <P extends Supplier<PathElement>> C provideRequiredChildren(Collection<P> providers) {
+            return this.requireChildResources(providers.stream().map(Supplier::get).map(ResourceRegistration::of).collect(Collectors.toUnmodifiableSet()));
+        }
 
         /**
          * Defines a set of required singleton children of this resource.  Required singleton children will be automatically added, if no child resource exists with the same path key.
          * @param providers a set of providers of the required singleton child resource paths
          * @return a reference to this configurator
+         * @deprecated Use {@link #requireSingletonChildResources(Set)} instead.
          */
-        <P extends Supplier<PathElement>> C provideRequiredSingletonChildren(Collection<P> providers);
+        @Deprecated(forRemoval = true, since = "26.0.0")
+        default <P extends Supplier<PathElement>> C provideRequiredSingletonChildren(Collection<P> providers) {
+            return this.requireSingletonChildResources(providers.stream().map(Supplier::get).map(ResourceRegistration::of).collect(Collectors.toUnmodifiableSet()));
+        }
     }
 
     /**
@@ -569,8 +623,8 @@ abstract static class AbstractConfigurator<C extends Configurator<C>> implements
         private Collection<AttributeDefinition> modelOnlyAttributes = List.of();
         private Collection<AttributeDefinition> readOnlyAttributes = List.of();
         private Map<AttributeDefinition, OperationStepHandler> customAttributes = Map.of();
-        private Set<PathElement> requiredChildren = Set.of();
-        private Set<PathElement> requiredSingletonChildren = Set.of();
+        private Map<PathElement, ResourceRegistration> requiredChildren = Map.of();
+        private Map<PathElement, ResourceRegistration> requiredSingletonChildren = Map.of();
         private Map<AttributeDefinition, AttributeTranslation> attributeTranslations = Map.of();
         private Set<ResourceCapabilityReferenceRecorder<?>> resourceCapabilityReferences = Set.of();
         private Map<String, UnaryOperator<OperationStepHandler>> operationTransformers = Map.of();
@@ -655,14 +709,14 @@ public C addCapabilities(Collection<RuntimeCapability<?>> capabilities, BiPredic
         }
 
         @Override
-        public C requireChildren(Set<PathElement> paths) {
-            this.requiredChildren = this.requiredChildren.isEmpty() ? Set.copyOf(paths) : concat(this.requiredChildren, paths.stream());
+        public C requireChildResources(Set<? extends ResourceRegistration> children) {
+            this.requiredChildren = this.requiredChildren.isEmpty() ? children.stream().collect(Collectors.toMap(ResourceRegistration::getPathElement, Function.identity())) : concat(this.requiredChildren, children.stream().map(child -> Map.entry(child.getPathElement(), child)));
             return this.self();
         }
 
         @Override
-        public C requireSingletonChildren(Set<PathElement> paths) {
-            this.requiredSingletonChildren = this.requiredSingletonChildren.isEmpty() ? Set.copyOf(paths) : concat(this.requiredSingletonChildren, paths.stream());
+        public C requireSingletonChildResources(Set<? extends ResourceRegistration> children) {
+            this.requiredSingletonChildren = this.requiredSingletonChildren.isEmpty() ? children.stream().collect(Collectors.toMap(ResourceRegistration::getPathElement, Function.identity())) : concat(this.requiredSingletonChildren, children.stream().map(child -> Map.entry(child.getPathElement(), child)));
             return this.self();
         }
 
@@ -727,18 +781,6 @@ public <P extends Supplier<RuntimeCapability<?>>> C provideCapabilities(Collecti
             return this.self();
         }
 
-        @Override
-        public <P extends Supplier<PathElement>> C provideRequiredChildren(Collection<P> providers) {
-            this.requiredChildren = concat(this.requiredChildren, stream(providers));
-            return this.self();
-        }
-
-        @Override
-        public <P extends Supplier<PathElement>>C provideRequiredSingletonChildren(Collection<P> providers) {
-            this.requiredSingletonChildren = concat(this.requiredSingletonChildren, stream(providers));
-            return this.self();
-        }
-
         private static <T> Collection<T> copyOf(Collection<T> collection) {
             // Create defensive copy, if collection was not already immutable
             return (collection instanceof Set) ? Set.copyOf((Set<T>) collection) : List.copyOf(collection);