[WFCORE-7095] Deprecate ModuleDependency.getIdentifier() for removal;…

… move partly away from MI in ServiceModuleLoader

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

@@ -4,8 +4,11 @@
  */
 package org.jboss.as.controller;
 
+import java.util.function.BiFunction;
+
 import org.jboss.dmr.ModelNode;
 import org.jboss.dmr.ModelType;
+import org.jboss.msc.service.ServiceName;
 
 /**
  * Provides utilities related to working with names of JBoss Modules modules.
@@ -21,6 +24,21 @@ public final class ModuleIdentifierUtil {
      * @return the canonical representation. Will not return @{code null}
      */
     public static String canonicalModuleIdentifier(String moduleSpec) {
+        return parseModuleIdentifier(moduleSpec, ModuleIdentifierUtil::canonicalModuleIdentifier);
+    }
+
+    /**
+     * Appends the name and slot of the given {@code moduleSpec} to the give {@code serviceName}.
+     *
+     * @param moduleSpec a module name specification in the form {@code name[:slot]}. Cannot be {@code null}
+     * @param parent the {@link ServiceName} that should be the parent of the result
+     * @return a {@link ServiceName} that appends the specified module's name and slot
+     */
+    public static ServiceName nameSlotServiceName(String moduleSpec, ServiceName parent) {
+        return parseModuleIdentifier(moduleSpec, (name, slot) -> parent.append(name).append(slot == null ? "main" : slot));
+    }
+
+    private static <R> R parseModuleIdentifier(String moduleSpec, BiFunction<String, String, R> function) {
         // Note: this is taken from org.jboss.modules.ModuleIdentifier.fromString and lightly adapted.
 
         if (moduleSpec == null) {
@@ -59,15 +77,15 @@ public static String canonicalModuleIdentifier(String moduleSpec) {
             String name = b.toString();
             b.setLength(0);
             if (i >= moduleSpec.length()) {
-                return canonicalModuleIdentifier(name, null);
+                return function.apply(name, null);
             } else {
                 do {
                     c = moduleSpec.codePointAt(i);
                     b.appendCodePoint(c);
                     i = moduleSpec.offsetByCodePoints(i, 1);
                 } while(i < moduleSpec.length());
 
-                return canonicalModuleIdentifier(name, b.toString());
+                return function.apply(name, b.toString());
             }
         }
 

server/src/main/java/org/jboss/as/server/deployment/DeploymentListModulesHandler.java

@@ -79,15 +79,15 @@ public void execute(OperationContext context, ModelNode operation) {
 
                     final ModelNode result = new ModelNode();
                     List<ModuleDependency> dependencies = moduleLoadService.getSystemDependencies();
-                    Collections.sort(dependencies, Comparator.comparing(p -> p.getIdentifier().toString()));
+                    Collections.sort(dependencies, Comparator.comparing(ModuleDependency::getDependencyModule));
                     result.get("system-dependencies").set(buildDependenciesInfo(dependencies, verbose));
 
                     dependencies = moduleLoadService.getLocalDependencies();
-                    Collections.sort(dependencies, Comparator.comparing(p -> p.getIdentifier().toString()));
+                    Collections.sort(dependencies, Comparator.comparing(ModuleDependency::getDependencyModule));
                     result.get("local-dependencies").set(buildDependenciesInfo(dependencies, verbose));
 
                     dependencies = moduleLoadService.getUserDependencies();
-                    Collections.sort(dependencies, Comparator.comparing(p -> p.getIdentifier().toString()));
+                    Collections.sort(dependencies, Comparator.comparing(ModuleDependency::getDependencyModule));
                     result.get("user-dependencies").set(buildDependenciesInfo(dependencies, verbose));
 
                     context.getResult().set(result);
@@ -100,8 +100,7 @@ private ModelNode buildDependenciesInfo(List<ModuleDependency> dependencies, boo
         ModelNode deps = new ModelNode().setEmptyList();
         for (ModuleDependency dependency : dependencies) {
             ModelNode depData = new ModelNode();
-            ModuleIdentifier identifier = dependency.getIdentifier();
-            depData.get("name").set(identifier.toString());
+            depData.get("name").set(dependency.getDependencyModule());
             if (verbose) {
                 depData.get("optional").set(dependency.isOptional());
                 depData.get("export").set(dependency.isExport());

server/src/main/java/org/jboss/as/server/deployment/annotation/CompositeIndexProcessor.java

@@ -150,9 +150,9 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro
     }
 
     private Map<ModuleIdentifier, DeploymentUnit> buildSubdeploymentDependencyMap(DeploymentUnit deploymentUnit) {
-        Set<ModuleIdentifier> depModuleIdentifiers = new HashSet<>();
+        Set<String> depModuleIdentifiers = new HashSet<>();
         for (ModuleDependency dep: deploymentUnit.getAttachment(Attachments.MODULE_SPECIFICATION).getAllDependencies()) {
-            depModuleIdentifiers.add(dep.getIdentifier());
+            depModuleIdentifiers.add(dep.getDependencyModule());
         }
 
         DeploymentUnit top = deploymentUnit.getParent()==null?deploymentUnit:deploymentUnit.getParent();
@@ -161,7 +161,7 @@ private Map<ModuleIdentifier, DeploymentUnit> buildSubdeploymentDependencyMap(De
         if (subDeployments != null) {
             for (DeploymentUnit subDeployment : subDeployments) {
                 ModuleIdentifier moduleIdentifier = subDeployment.getAttachment(Attachments.MODULE_IDENTIFIER);
-                if (depModuleIdentifiers.contains(moduleIdentifier)) {
+                if (depModuleIdentifiers.contains(moduleIdentifier.toString())) {
                     res.put(moduleIdentifier, subDeployment);
                 }
             }

server/src/main/java/org/jboss/as/server/deployment/module/DeploymentVisibilityProcessor.java

@@ -26,26 +26,26 @@ public class DeploymentVisibilityProcessor implements DeploymentUnitProcessor {
     public void deploy(final DeploymentPhaseContext phaseContext) throws DeploymentUnitProcessingException {
         final DeploymentUnit deploymentUnit = phaseContext.getDeploymentUnit();
         final ModuleSpecification moduleSpec = deploymentUnit.getAttachment(org.jboss.as.server.deployment.Attachments.MODULE_SPECIFICATION);
-        final Map<ModuleIdentifier, DeploymentUnit> deployments = new HashMap<ModuleIdentifier, DeploymentUnit>();
+        final Map<String, DeploymentUnit> deployments = new HashMap<>();
         //local classes are always first
         deploymentUnit.addToAttachmentList(Attachments.ACCESSIBLE_SUB_DEPLOYMENTS, deploymentUnit);
         buildModuleMap(deploymentUnit, deployments);
 
         for (final ModuleDependency dependency : moduleSpec.getAllDependencies()) {
-            final DeploymentUnit sub = deployments.get(dependency.getIdentifier());
+            final DeploymentUnit sub = deployments.get(dependency.getDependencyModule());
             if (sub != null) {
                 deploymentUnit.addToAttachmentList(Attachments.ACCESSIBLE_SUB_DEPLOYMENTS, sub);
             }
         }
     }
 
-    private void buildModuleMap(final DeploymentUnit deploymentUnit, final Map<ModuleIdentifier, DeploymentUnit> modules) {
+    private void buildModuleMap(final DeploymentUnit deploymentUnit, final Map<String, DeploymentUnit> modules) {
         if (deploymentUnit.getParent() == null) {
             final List<DeploymentUnit> subDeployments = deploymentUnit.getAttachmentList(org.jboss.as.server.deployment.Attachments.SUB_DEPLOYMENTS);
             for (final DeploymentUnit sub : subDeployments) {
                 final ModuleIdentifier identifier = sub.getAttachment(org.jboss.as.server.deployment.Attachments.MODULE_IDENTIFIER);
                 if (identifier != null) {
-                    modules.put(identifier, sub);
+                    modules.put(identifier.toString(), sub);
                 }
             }
         } else {
@@ -54,14 +54,14 @@ private void buildModuleMap(final DeploymentUnit deploymentUnit, final Map<Modul
             //add the parent description
             final ModuleIdentifier parentIdentifier = parent.getAttachment(org.jboss.as.server.deployment.Attachments.MODULE_IDENTIFIER);
             if (parentIdentifier != null) {
-                modules.put(parentIdentifier, parent);
+                modules.put(parentIdentifier.toString(), parent);
             }
 
             for (final DeploymentUnit sub : subDeployments) {
                 if (sub != deploymentUnit) {
                     final ModuleIdentifier identifier = sub.getAttachment(org.jboss.as.server.deployment.Attachments.MODULE_IDENTIFIER);
                     if (identifier != null) {
-                        modules.put(identifier, sub);
+                        modules.put(identifier.toString(), sub);
                     }
                 }
             }

server/src/main/java/org/jboss/as/server/deployment/module/ModuleAliasChecker.java

@@ -66,7 +66,7 @@ static String getTargetModule(String identifier) {
      */
     public static void checkModuleAliasesForDependencies(List<ModuleDependency> dependencies, MessageContext context, String deploymentName) {
         for (ModuleDependency dependency : dependencies) {
-            String identifier = dependency.getIdentifier().toString();
+            String identifier = dependency.getDependencyModule();
             checkModuleAlias(context, deploymentName, identifier, false);
         }
     }

server/src/main/java/org/jboss/as/server/deployment/module/ModuleDependency.java

@@ -11,6 +11,7 @@
 import java.util.Objects;
 import java.util.Optional;
 
+import org.jboss.as.controller.ModuleIdentifierUtil;
 import org.jboss.modules.ModuleIdentifier;
 import org.jboss.modules.ModuleLoader;
 import org.jboss.modules.filter.PathFilter;
@@ -23,8 +24,7 @@ public final class ModuleDependency implements Serializable {
 
     public static final class Builder {
         private final ModuleLoader moduleLoader;
-        @SuppressWarnings("deprecation")
-        private final ModuleIdentifier identifier;
+        private final String identifier;
         private boolean export;
         private boolean optional;
         private boolean importServices;
@@ -37,8 +37,7 @@ public static Builder of(ModuleLoader moduleLoader, String moduleName) {
 
         private Builder(ModuleLoader moduleLoader, String moduleName) {
             this.moduleLoader = moduleLoader;
-            //noinspection deprecation
-            this.identifier = ModuleIdentifier.fromString(moduleName);
+            this.identifier = ModuleIdentifierUtil.canonicalModuleIdentifier(moduleName);
         }
 
         /**
@@ -66,7 +65,7 @@ public Builder setImportServices(boolean importServices) {
         /**
          * Sets whether this dependency is optional.
          *
-         * @param optional {@code true} if the dependencys is optional
+         * @param optional {@code true} if the dependency is optional
          * @return this builder
          */
         public Builder setOptional(boolean optional) {
@@ -102,7 +101,7 @@ public Builder setReason(String reason) {
          * @return the {@code ModuleDependency}. Will not return {@code null}
          */
         public ModuleDependency build() {
-            return new ModuleDependency(moduleLoader, identifier, optional, export, importServices, userSpecified, reason);
+            return new ModuleDependency(moduleLoader, identifier, reason, optional, export, importServices, userSpecified);
         }
     }
 
@@ -126,17 +125,17 @@ public String toString() {
     private static final long serialVersionUID = 2749276798703740853L;
 
     private final ModuleLoader moduleLoader;
-    private final ModuleIdentifier identifier;
+    private final String identifier;
     private final boolean export;
     private final boolean optional;
-    private final List<FilterSpecification> importFilters = new ArrayList<FilterSpecification>();
-    private final List<FilterSpecification> exportFilters = new ArrayList<FilterSpecification>();
+    private final List<FilterSpecification> importFilters = new ArrayList<>();
+    private final List<FilterSpecification> exportFilters = new ArrayList<>();
     private final boolean importServices;
     private final boolean userSpecified;
     private final String reason;
 
     // NOTE: this constructor isn't deprecated because it's used in over 100 places, and perhaps 40+ more if
-    // the uses of the equivalent c'tor taking ModuleIdentifier make a simple switch to this. Changing all that
+    // the uses of the equivalent constructor taking ModuleIdentifier make a simple switch to this. Changing all that
     // code to use the builder just to clear a deprecation warning is a simple way to introduce bugs.
     /**
      * Construct a new instance.
@@ -149,7 +148,7 @@ public String toString() {
      * @param userSpecified {@code true} if this dependency was specified by the user, {@code false} if it was automatically added
      */
     public ModuleDependency(final ModuleLoader moduleLoader, final String identifier, final boolean optional, final boolean export, final boolean importServices, final boolean userSpecified) {
-        this(moduleLoader, ModuleIdentifier.fromString(identifier), optional, export, importServices, userSpecified, null);
+        this(moduleLoader, ModuleIdentifierUtil.canonicalModuleIdentifier(identifier), null, optional, export, importServices, userSpecified);
     }
 
     /**
@@ -167,7 +166,7 @@ public ModuleDependency(final ModuleLoader moduleLoader, final String identifier
      */
     @Deprecated(forRemoval = true)
     public ModuleDependency(final ModuleLoader moduleLoader, final String identifier, final boolean optional, final boolean export, final boolean importServices, final boolean userSpecified, String reason) {
-        this(moduleLoader, ModuleIdentifier.fromString(identifier), optional, export, importServices, userSpecified, reason);
+        this(moduleLoader, ModuleIdentifierUtil.canonicalModuleIdentifier(identifier), reason, optional, export, importServices, userSpecified);
     }
 
     /**
@@ -180,7 +179,7 @@ public ModuleDependency(final ModuleLoader moduleLoader, final String identifier
      * @param importServices  {@code true} if the dependent module should be able to load services from the dependency
      * @param userSpecified {@code true} if this dependency was specified by the user, {@code false} if it was automatically added
      *
-     * @deprecated Use a {@link Builder} or @link ModuleDependency(ModuleLoader, String, boolean, boolean, boolean, boolean)}
+     * @deprecated Use a {@link Builder} or {@link ModuleDependency(ModuleLoader, String, boolean, boolean, boolean, boolean)}
      */
     @Deprecated(forRemoval = true)
     public ModuleDependency(final ModuleLoader moduleLoader, final ModuleIdentifier identifier, final boolean optional, final boolean export, final boolean importServices, final boolean userSpecified) {
@@ -202,6 +201,10 @@ public ModuleDependency(final ModuleLoader moduleLoader, final ModuleIdentifier
      */
     @Deprecated(forRemoval = true)
     public ModuleDependency(final ModuleLoader moduleLoader, final ModuleIdentifier identifier, final boolean optional, final boolean export, final boolean importServices, final boolean userSpecified, String reason) {
+        this(moduleLoader, identifier.toString(), reason, optional, export, importServices, userSpecified);
+    }
+
+    private ModuleDependency(final ModuleLoader moduleLoader, final String identifier, String reason, final boolean optional, final boolean export, final boolean importServices, final boolean userSpecified) {
         this.identifier = identifier;
         this.optional = optional;
         this.export = export;
@@ -215,7 +218,18 @@ public ModuleLoader getModuleLoader() {
         return moduleLoader;
     }
 
+    /** @deprecated use {@link #getDependencyModule()} */
+    @Deprecated(forRemoval = true)
     public ModuleIdentifier getIdentifier() {
+        return ModuleIdentifier.fromString(identifier);
+    }
+
+    /**
+     * Gets the name of the module upon which there is a dependency.
+     *
+     * @return the {@link ModuleIdentifierUtil#canonicalModuleIdentifier(String) canonical form} of the name of module
+     */
+    public String getDependencyModule() {
         return identifier;
     }
 

server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecProcessor.java

@@ -30,6 +30,7 @@
 import org.jboss.as.server.moduleservice.ModuleResolvePhaseService;
 import org.jboss.as.server.moduleservice.ServiceModuleLoader;
 import org.jboss.modules.DependencySpec;
+import org.jboss.modules.ModuleDependencySpecBuilder;
 import org.jboss.modules.ModuleIdentifier;
 import org.jboss.modules.ModuleLoader;
 import org.jboss.modules.ModuleSpec;
@@ -151,7 +152,7 @@ private void addAllDependenciesAndPermissions(final ModuleSpecification moduleSp
         module.addSystemDependencies(moduleSpecification.getSystemDependenciesSet());
         module.addLocalDependencies(moduleSpecification.getLocalDependenciesSet());
         for(ModuleDependency dep : moduleSpecification.getUserDependenciesSet()) {
-            if(!dep.getIdentifier().equals(module.getModuleIdentifier())) {
+            if(!dep.getDependencyModule().equals(module.getModuleIdentifier().toString())) {
                 module.addUserDependency(dep);
             }
         }
@@ -336,8 +337,13 @@ private void createDependencies(final ModuleSpec.Builder specBuilder, final Coll
                     }
                     exportFilter = exportBuilder.create();
                 }
-                final DependencySpec depSpec = DependencySpec.createModuleDependencySpec(importFilter, exportFilter, dependency
-                        .getModuleLoader(), dependency.getIdentifier(), dependency.isOptional());
+                final DependencySpec depSpec = new ModuleDependencySpecBuilder()
+                        .setModuleLoader(dependency.getModuleLoader())
+                        .setName(dependency.getDependencyModule())
+                        .setOptional(dependency.isOptional())
+                        .setImportFilter(importFilter)
+                        .setExportFilter(exportFilter)
+                        .build();
                 specBuilder.addDependency(depSpec);
                 logger.debugf("Adding dependency %s to module %s", dependency, specBuilder.getIdentifier());
             }

server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java

@@ -129,12 +129,12 @@ public class ModuleSpecification extends SimpleAttachable {
     private final List<PermissionFactory> permissionFactories = new ArrayList<>();
 
     public void addSystemDependency(final ModuleDependency dependency) {
-        if (!exclusions.contains(dependency.getIdentifier().toString())) {
+        if (!exclusions.contains(dependency.getDependencyModule())) {
             if (systemDependenciesSet.add(dependency)) {
                 resetDependencyLists();
             }
         } else {
-            excludedDependencies.add(dependency.getIdentifier().toString());
+            excludedDependencies.add(dependency.getDependencyModule());
         }
     }
 
@@ -173,12 +173,12 @@ public void removeUserDependencies(final Predicate<ModuleDependency> predicate)
     }
 
     public void addLocalDependency(final ModuleDependency dependency) {
-        if (!exclusions.contains(dependency.getIdentifier().toString())) {
+        if (!exclusions.contains(dependency.getDependencyModule())) {
             if (this.localDependenciesSet.add(dependency)) {
                 resetDependencyLists();
             }
         } else {
-            excludedDependencies.add(dependency.getIdentifier().toString());
+            excludedDependencies.add(dependency.getDependencyModule());
         }
     }
 
@@ -227,15 +227,15 @@ public void addModuleExclusion(final String exclusion) {
         Iterator<ModuleDependency> it = systemDependenciesSet.iterator();
         while (it.hasNext()) {
             final ModuleDependency dep = it.next();
-            if (dep.getIdentifier().toString().equals(exclusion)) {
+            if (dep.getDependencyModule().equals(exclusion)) {
                 it.remove();
                 resetDependencyLists();
             }
         }
         it = localDependenciesSet.iterator();
         while (it.hasNext()) {
             final ModuleDependency dep = it.next();
-            if (dep.getIdentifier().toString().equals(exclusion)) {
+            if (dep.getDependencyModule().equals(exclusion)) {
                 it.remove();
                 resetDependencyLists();
             }