From cb442031941e25a59acf8706d6d057bc54a01eac Mon Sep 17 00:00:00 2001 From: Brian Stansberry Date: Sat, 14 Dec 2024 21:25:48 -0500 Subject: [PATCH] [WFCORE-7095] Deprecate ModuleDependency.getIdentifier() for removal; move partly away from MI in ServiceModuleLoader --- .../as/controller/ModuleIdentifierUtil.java | 22 ++++++- .../DeploymentListModulesHandler.java | 9 ++- .../annotation/CompositeIndexProcessor.java | 6 +- .../module/DeploymentVisibilityProcessor.java | 12 ++-- .../deployment/module/ModuleAliasChecker.java | 2 +- .../deployment/module/ModuleDependency.java | 40 ++++++++----- .../module/ModuleSpecProcessor.java | 12 +++- .../module/ModuleSpecification.java | 12 ++-- .../SubDeploymentDependencyProcessor.java | 6 +- .../DeploymentStructureDescriptorParser.java | 2 +- .../jboss/as/server/logging/ServerLogger.java | 8 +-- .../moduleservice/ModuleLoadService.java | 10 ++-- .../ModuleResolvePhaseService.java | 33 +++++------ .../moduleservice/ServiceModuleLoader.java | 57 +++++++++++++++---- .../module/ModuleDependencyUnitTestCase.java | 4 +- .../module/ModuleSpecificationTestCase.java | 14 ++--- 16 files changed, 159 insertions(+), 90 deletions(-) diff --git a/controller/src/main/java/org/jboss/as/controller/ModuleIdentifierUtil.java b/controller/src/main/java/org/jboss/as/controller/ModuleIdentifierUtil.java index 9ae22bcd0ce..bee454a1012 100644 --- a/controller/src/main/java/org/jboss/as/controller/ModuleIdentifierUtil.java +++ b/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 parseModuleIdentifier(String moduleSpec, BiFunction function) { // Note: this is taken from org.jboss.modules.ModuleIdentifier.fromString and lightly adapted. if (moduleSpec == null) { @@ -59,7 +77,7 @@ 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); @@ -67,7 +85,7 @@ public static String canonicalModuleIdentifier(String moduleSpec) { i = moduleSpec.offsetByCodePoints(i, 1); } while(i < moduleSpec.length()); - return canonicalModuleIdentifier(name, b.toString()); + return function.apply(name, b.toString()); } } diff --git a/server/src/main/java/org/jboss/as/server/deployment/DeploymentListModulesHandler.java b/server/src/main/java/org/jboss/as/server/deployment/DeploymentListModulesHandler.java index b6e23b98048..435d2b9012d 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/DeploymentListModulesHandler.java +++ b/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 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 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()); diff --git a/server/src/main/java/org/jboss/as/server/deployment/annotation/CompositeIndexProcessor.java b/server/src/main/java/org/jboss/as/server/deployment/annotation/CompositeIndexProcessor.java index ed2f43cdb73..276ffaa5b8b 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/annotation/CompositeIndexProcessor.java +++ b/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 buildSubdeploymentDependencyMap(DeploymentUnit deploymentUnit) { - Set depModuleIdentifiers = new HashSet<>(); + Set 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 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); } } diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/DeploymentVisibilityProcessor.java b/server/src/main/java/org/jboss/as/server/deployment/module/DeploymentVisibilityProcessor.java index a12a4d986d8..e1ee6623512 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/module/DeploymentVisibilityProcessor.java +++ b/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 deployments = new HashMap(); + final Map 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 modules) { + private void buildModuleMap(final DeploymentUnit deploymentUnit, final Map modules) { if (deploymentUnit.getParent() == null) { final List 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 dependencies, MessageContext context, String deploymentName) { for (ModuleDependency dependency : dependencies) { - String identifier = dependency.getIdentifier().toString(); + String identifier = dependency.getDependencyModule(); checkModuleAlias(context, deploymentName, identifier, false); } } diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/ModuleDependency.java b/server/src/main/java/org/jboss/as/server/deployment/module/ModuleDependency.java index 832c2e9aab9..13f96aa4fb2 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/module/ModuleDependency.java +++ b/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 importFilters = new ArrayList(); - private final List exportFilters = new ArrayList(); + private final List importFilters = new ArrayList<>(); + private final List 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; } diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecProcessor.java b/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecProcessor.java index 7621165f3bb..3025aeb2a25 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecProcessor.java +++ b/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()); } diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java b/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java index 6be02f3c8e7..01f07545246 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java +++ b/server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java @@ -129,12 +129,12 @@ public class ModuleSpecification extends SimpleAttachable { private final List 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 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,7 +227,7 @@ public void addModuleExclusion(final String exclusion) { Iterator 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(); } @@ -235,7 +235,7 @@ public void addModuleExclusion(final String exclusion) { 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(); } diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/SubDeploymentDependencyProcessor.java b/server/src/main/java/org/jboss/as/server/deployment/module/SubDeploymentDependencyProcessor.java index 4110185dca7..57fdc8c2df6 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/module/SubDeploymentDependencyProcessor.java +++ b/server/src/main/java/org/jboss/as/server/deployment/module/SubDeploymentDependencyProcessor.java @@ -33,7 +33,7 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro final ModuleSpecification moduleSpec = deploymentUnit.getAttachment(Attachments.MODULE_SPECIFICATION); final ModuleLoader moduleLoader = deploymentUnit.getAttachment(Attachments.SERVICE_MODULE_LOADER); - final ModuleIdentifier moduleIdentifier = deploymentUnit.getAttachment(Attachments.MODULE_IDENTIFIER); + final String moduleIdentifier = deploymentUnit.getAttachment(Attachments.MODULE_IDENTIFIER).toString(); if (deploymentUnit.getParent() != null) { final ModuleIdentifier parentModule = parent.getAttachment(Attachments.MODULE_IDENTIFIER); @@ -55,14 +55,14 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro for (DeploymentUnit subDeployment : subDeployments) { final ModuleSpecification subModule = subDeployment.getAttachment(Attachments.MODULE_SPECIFICATION); if (!subModule.isPrivateModule() && (!parentModuleSpec.isSubDeploymentModulesIsolated() || subModule.isPublicModule())) { - ModuleIdentifier identifier = subDeployment.getAttachment(Attachments.MODULE_IDENTIFIER); + String identifier = subDeployment.getAttachment(Attachments.MODULE_IDENTIFIER).toString(); ModuleDependency dependency = new ModuleDependency(moduleLoader, identifier, false, false, true, false); dependency.addImportFilter(PathFilters.acceptAll(), true); accessibleModules.add(dependency); } } for (ModuleDependency dependency : accessibleModules) { - if (!dependency.getIdentifier().equals(moduleIdentifier)) { + if (!dependency.getDependencyModule().equals(moduleIdentifier)) { moduleSpec.addLocalDependency(dependency); } } diff --git a/server/src/main/java/org/jboss/as/server/deployment/module/descriptor/DeploymentStructureDescriptorParser.java b/server/src/main/java/org/jboss/as/server/deployment/module/descriptor/DeploymentStructureDescriptorParser.java index 5db77288c35..ad05904f613 100644 --- a/server/src/main/java/org/jboss/as/server/deployment/module/descriptor/DeploymentStructureDescriptorParser.java +++ b/server/src/main/java/org/jboss/as/server/deployment/module/descriptor/DeploymentStructureDescriptorParser.java @@ -244,7 +244,7 @@ private void handleDeployment(final DeploymentPhaseContext phaseContext, final D } // No more nested loop for (ModuleDependency dependency : moduleDependencies) { - String identifier = dependency.getIdentifier().toString(); + String identifier = dependency.getDependencyModule(); if (index.containsKey(identifier)) { aliasDependencies.add(new ModuleDependency(dependency.getModuleLoader(), index.get(identifier).getModuleIdentifier(), dependency.isOptional(), dependency.isExport(), dependency.isImportServices(), dependency.isUserSpecified())); } diff --git a/server/src/main/java/org/jboss/as/server/logging/ServerLogger.java b/server/src/main/java/org/jboss/as/server/logging/ServerLogger.java index ff742703128..f740400be32 100644 --- a/server/src/main/java/org/jboss/as/server/logging/ServerLogger.java +++ b/server/src/main/java/org/jboss/as/server/logging/ServerLogger.java @@ -193,11 +193,11 @@ public interface ServerLogger extends BasicLogger { @LogMessage(level = WARN) @Message(id = 18, value = "Deployment \"%s\" is using a private module (\"%s\") which may be changed or removed in future versions without notice.") - void privateApiUsed(String deployment, ModuleIdentifier dependency); + void privateApiUsed(String deployment, String dependency); @LogMessage(level = WARN) @Message(id = 19, value = "Deployment \"%s\" is using an unsupported module (\"%s\") which may be changed or removed in future versions without notice.") - void unsupportedApiUsed(String deployment, ModuleIdentifier dependency); + void unsupportedApiUsed(String deployment, String dependency); @LogMessage(level = WARN) @Message(id = 20, value = "Exception occurred removing deployment content %s") @@ -733,7 +733,7 @@ public interface ServerLogger extends BasicLogger { IllegalStateException serviceModuleLoaderAlreadyStopped(); @Message(id = 99, value = "'%s' cannot be loaded from a ServiceModuleLoader as its name does not start with '%s'") - IllegalArgumentException missingModulePrefix(ModuleIdentifier identifier, String prefix); + IllegalArgumentException missingModulePrefix(String identifier, String prefix); @Message(id = 100, value = "Failed to read '%s'") DeploymentUnitProcessingException failedToReadVirtualFile(VirtualFile file, @Cause IOException cause); @@ -1153,7 +1153,7 @@ default void suspendingServer(long timeout, TimeUnit unit) { @LogMessage(level = WARN) @Message(id = 221, value = "Deployment \"%s\" is using a deprecated module (\"%s\") which may be removed in future versions without notice.") - void deprecatedApiUsed(String name, ModuleIdentifier id); + void deprecatedApiUsed(String name, String id); @Message(id = 222, value="Illegal permission name '%s'") IllegalArgumentException illegalPermissionName(String name); diff --git a/server/src/main/java/org/jboss/as/server/moduleservice/ModuleLoadService.java b/server/src/main/java/org/jboss/as/server/moduleservice/ModuleLoadService.java index 75ec2772b63..a543db2c594 100644 --- a/server/src/main/java/org/jboss/as/server/moduleservice/ModuleLoadService.java +++ b/server/src/main/java/org/jboss/as/server/moduleservice/ModuleLoadService.java @@ -78,7 +78,7 @@ public synchronized void start(StartContext context) throws StartException { moduleLoader.relinkModule(module); for (ModuleDependency dependency : allDependencies) { if (dependency.isUserSpecified()) { - final ModuleIdentifier id = dependency.getIdentifier(); + final String id = dependency.getDependencyModule(); try { String val = moduleLoader.loadModule(id).getProperty("jboss.api"); if (val != null) { @@ -112,7 +112,7 @@ public Module getValue() throws IllegalStateException, IllegalArgumentException return module; } - private static ServiceName install(final ServiceTarget target, final ModuleIdentifier identifier, ModuleLoadService service) { + private static ServiceName install(final ServiceTarget target, final String identifier, ModuleLoadService service) { final ServiceName serviceName = ServiceModuleLoader.moduleServiceName(identifier); final ServiceBuilder builder = target.addService(serviceName, service); @@ -127,12 +127,12 @@ private static ServiceName install(final ServiceTarget target, final ModuleIdent public static ServiceName install(final ServiceTarget target, final ModuleIdentifier identifier){ final ModuleLoadService service = new ModuleLoadService(); - return install(target, identifier, service); + return install(target, identifier.toString(), service); } public static ServiceName install(final ServiceTarget target, final ModuleIdentifier identifier, final Collection systemDependencies, final Collection localDependencies, final Collection userDependencies) { final ModuleLoadService service = new ModuleLoadService(systemDependencies, localDependencies, userDependencies); - return install(target, identifier, service); + return install(target, identifier.toString(), service); } public static ServiceName installAliases(final ServiceTarget target, final ModuleIdentifier identifier, final List aliases) { @@ -141,7 +141,7 @@ public static ServiceName installAliases(final ServiceTarget target, final Modul dependencies.add(new ModuleDependency(null, i, false, false, false, false)); } final ModuleLoadService service = new ModuleLoadService(dependencies); - return install(target, identifier, service); + return install(target, identifier.toString(), service); } public InjectedValue getServiceModuleLoader() { diff --git a/server/src/main/java/org/jboss/as/server/moduleservice/ModuleResolvePhaseService.java b/server/src/main/java/org/jboss/as/server/moduleservice/ModuleResolvePhaseService.java index cc40c63da90..b24059f6da5 100644 --- a/server/src/main/java/org/jboss/as/server/moduleservice/ModuleResolvePhaseService.java +++ b/server/src/main/java/org/jboss/as/server/moduleservice/ModuleResolvePhaseService.java @@ -9,6 +9,7 @@ import java.util.HashSet; import java.util.Set; +import org.jboss.as.controller.ModuleIdentifierUtil; import org.jboss.as.server.logging.ServerLogger; import org.jboss.as.server.deployment.module.ModuleDependency; import org.jboss.modules.ModuleIdentifier; @@ -36,7 +37,7 @@ public class ModuleResolvePhaseService implements Service alreadyResolvedModules; + private final Set alreadyResolvedModules; private final int phaseNumber; @@ -45,29 +46,23 @@ public class ModuleResolvePhaseService implements Service moduleSpecs = Collections.synchronizedSet(new HashSet()); - public ModuleResolvePhaseService(final ModuleIdentifier moduleIdentifier, final Set alreadyResolvedModules, final int phaseNumber) { + private ModuleResolvePhaseService(final ModuleIdentifier moduleIdentifier, final Set alreadyResolvedModules, final int phaseNumber) { this.moduleIdentifier = moduleIdentifier; this.alreadyResolvedModules = alreadyResolvedModules; this.phaseNumber = phaseNumber; } - public ModuleResolvePhaseService(final ModuleIdentifier moduleIdentifier) { - this.moduleIdentifier = moduleIdentifier; - this.alreadyResolvedModules = Collections.emptySet(); - this.phaseNumber = 0; - } - @Override public void start(final StartContext startContext) throws StartException { Set nextPhaseIdentifiers = new HashSet<>(); - final Set nextAlreadySeen = new HashSet<>(alreadyResolvedModules); + final Set nextAlreadySeen = new HashSet<>(alreadyResolvedModules); for (final ModuleDefinition spec : moduleSpecs) { if (spec != null) { //this can happen for optional dependencies for (ModuleDependency dep : spec.getDependencies()) { if (dep.isOptional()) continue; // we don't care about optional dependencies - if (ServiceModuleLoader.isDynamicModule(dep.getIdentifier())) { - if (!alreadyResolvedModules.contains(dep.getIdentifier())) { - nextAlreadySeen.add(dep.getIdentifier()); + if (ServiceModuleLoader.isDynamicModule(dep.getDependencyModule())) { + if (!alreadyResolvedModules.contains(dep.getDependencyModule())) { + nextAlreadySeen.add(dep.getDependencyModule()); nextPhaseIdentifiers.add(dep); } } @@ -82,17 +77,17 @@ public void start(final StartContext startContext) throws StartException { } public static void installService(final ServiceTarget serviceTarget, final ModuleDefinition moduleDefinition) { - final ModuleResolvePhaseService nextPhaseService = new ModuleResolvePhaseService(moduleDefinition.getModuleIdentifier(), Collections.singleton(moduleDefinition.getModuleIdentifier()), 0); + final ModuleResolvePhaseService nextPhaseService = new ModuleResolvePhaseService(moduleDefinition.getModuleIdentifier(), Collections.singleton(moduleDefinition.getModuleIdentifier().toString()), 0); nextPhaseService.getModuleSpecs().add(moduleDefinition); - ServiceBuilder builder = serviceTarget.addService(moduleSpecServiceName(moduleDefinition.getModuleIdentifier(), 0), nextPhaseService); + ServiceBuilder builder = serviceTarget.addService(moduleResolvePhaseServiceName(moduleDefinition.getModuleIdentifier().toString(), 0), nextPhaseService); builder.install(); } - private static void installService(final ServiceTarget serviceTarget, final ModuleIdentifier moduleIdentifier, int phaseNumber, final Set nextPhaseIdentifiers, final Set nextAlreadySeen) { + private static void installService(final ServiceTarget serviceTarget, final ModuleIdentifier moduleIdentifier, int phaseNumber, final Set nextPhaseIdentifiers, final Set nextAlreadySeen) { final ModuleResolvePhaseService nextPhaseService = new ModuleResolvePhaseService(moduleIdentifier, nextAlreadySeen, phaseNumber); - ServiceBuilder builder = serviceTarget.addService(moduleSpecServiceName(moduleIdentifier, phaseNumber), nextPhaseService); + ServiceBuilder builder = serviceTarget.addService(moduleResolvePhaseServiceName(moduleIdentifier.toString(), phaseNumber), nextPhaseService); for (ModuleDependency module : nextPhaseIdentifiers) { - builder.addDependency(ServiceModuleLoader.moduleSpecServiceName(module.getIdentifier()), ModuleDefinition.class, new Injector() { + builder.addDependency(ServiceModuleLoader.moduleSpecServiceName(ModuleIdentifier.fromString(module.getDependencyModule())), ModuleDefinition.class, new Injector<>() { ModuleDefinition definition; @@ -126,10 +121,10 @@ public Set getModuleSpecs() { return moduleSpecs; } - public static ServiceName moduleSpecServiceName(ModuleIdentifier identifier, int phase) { + public static ServiceName moduleResolvePhaseServiceName(String identifier, int phase) { if (!ServiceModuleLoader.isDynamicModule(identifier)) { throw ServerLogger.ROOT_LOGGER.missingModulePrefix(identifier, ServiceModuleLoader.MODULE_PREFIX); } - return SERVICE_NAME.append(identifier.getName()).append(identifier.getSlot()).append("" + phase); + return ModuleIdentifierUtil.nameSlotServiceName(identifier, SERVICE_NAME).append("" + phase); } } diff --git a/server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java b/server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java index b5ba25ad026..6eccb7144ac 100644 --- a/server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java +++ b/server/src/main/java/org/jboss/as/server/moduleservice/ServiceModuleLoader.java @@ -6,6 +6,7 @@ import java.util.function.Consumer; +import org.jboss.as.controller.ModuleIdentifierUtil; import org.jboss.as.controller.UninterruptibleCountDownLatch; import org.jboss.as.server.Bootstrap; import org.jboss.as.server.logging.ServerLogger; @@ -131,7 +132,7 @@ protected Module preloadModule(final String name) throws ModuleLoadException { @SuppressWarnings("unchecked") @Override public ModuleSpec findModule(ModuleIdentifier identifier) throws ModuleLoadException { - ServiceController controller = (ServiceController) serviceContainer.getService(moduleSpecServiceName(identifier)); + ServiceController controller = (ServiceController) serviceContainer.getService(moduleSpecServiceName(identifier.toString())); if (controller == null) { ServerLogger.MODULE_SERVICE_LOGGER.debugf("Could not load module '%s' as corresponding module spec service '%s' was not found", identifier, identifier); return null; @@ -193,19 +194,32 @@ public static void addService(final ServiceTarget serviceTarget, final Bootstrap * * @param identifier The module identifier * @return The service name of the ModuleSpec service + * + * @deprecated use {@link #moduleSpecServiceName(String)} */ + @Deprecated(forRemoval = true) public static ServiceName moduleSpecServiceName(ModuleIdentifier identifier) { + return moduleSpecServiceName(identifier.toString()); + } + + /** + * Returns the corresponding ModuleSpec service name for the given module. + * + * @param identifier The module identifier + * @return The service name of the ModuleSpec service + */ + public static ServiceName moduleSpecServiceName(String identifier) { if (!isDynamicModule(identifier)) { throw ServerLogger.ROOT_LOGGER.missingModulePrefix(identifier, MODULE_PREFIX); } - return MODULE_SPEC_SERVICE_PREFIX.append(identifier.getName()).append(identifier.getSlot()); + return ModuleIdentifierUtil.nameSlotServiceName(identifier, MODULE_SPEC_SERVICE_PREFIX); } public static void installModuleResolvedService(ServiceTarget serviceTarget, ModuleIdentifier identifier) { - final ServiceName sn = ServiceModuleLoader.moduleResolvedServiceName(identifier); + final ServiceName sn = ServiceModuleLoader.moduleResolvedServiceName(identifier.toString()); final ServiceBuilder sb = serviceTarget.addService(sn); final Consumer moduleIdConsumer = sb.provides(sn); - sb.requires(moduleSpecServiceName(identifier)); + sb.requires(moduleSpecServiceName(identifier.toString())); final org.jboss.msc.Service resolvedService = org.jboss.msc.Service.newInstance(moduleIdConsumer, identifier); sb.setInstance(resolvedService); sb.install(); @@ -213,7 +227,7 @@ public static void installModuleResolvedService(ServiceTarget serviceTarget, Mod /** * Returns the corresponding module resolved service name for the given module. - * + *

* The module resolved service is basically a latch that prevents the module from being loaded * until all the transitive dependencies that it depends upon have have their module spec services * come up. @@ -221,18 +235,28 @@ public static void installModuleResolvedService(ServiceTarget serviceTarget, Mod * @param identifier The module identifier * @return The service name of the ModuleSpec service */ - public static ServiceName moduleResolvedServiceName(ModuleIdentifier identifier) { + public static ServiceName moduleResolvedServiceName(String identifier) { if (!isDynamicModule(identifier)) { throw ServerLogger.ROOT_LOGGER.missingModulePrefix(identifier, MODULE_PREFIX); } - return MODULE_RESOLVED_SERVICE_PREFIX.append(identifier.getName()).append(identifier.getSlot()); + return ModuleIdentifierUtil.nameSlotServiceName(identifier, MODULE_RESOLVED_SERVICE_PREFIX); } /** * Returns true if the module identifier is a dynamic module that will be loaded by this module loader + * + * @deprecated use {@link #isDynamicModule(String)} */ + @Deprecated(forRemoval = true) public static boolean isDynamicModule(ModuleIdentifier identifier) { - return identifier.getName().startsWith(MODULE_PREFIX); + return isDynamicModule(identifier.toString()); + } + + /** + * Returns true if the module identifier is a dynamic module that will be loaded by this module loader + */ + public static boolean isDynamicModule(String moduleIdentifier) { + return moduleIdentifier.startsWith(MODULE_PREFIX); } /** @@ -240,11 +264,24 @@ public static boolean isDynamicModule(ModuleIdentifier identifier) { * * @param identifier The module identifier * @return The service name of the ModuleLoadService service + * + * @deprecated use {@link #moduleServiceName(String)} */ + @Deprecated(forRemoval = true) public static ServiceName moduleServiceName(ModuleIdentifier identifier) { - if (!identifier.getName().startsWith(MODULE_PREFIX)) { + return moduleServiceName(identifier.toString()); + } + + /** + * Returns the corresponding ModuleLoadService service name for the given module. + * + * @param identifier The module identifier + * @return The service name of the ModuleLoadService service + */ + public static ServiceName moduleServiceName(String identifier) { + if (!isDynamicModule(identifier)) { throw ServerLogger.ROOT_LOGGER.missingModulePrefix(identifier, MODULE_PREFIX); } - return MODULE_SERVICE_PREFIX.append(identifier.getName()).append(identifier.getSlot()); + return ModuleIdentifierUtil.nameSlotServiceName(identifier, MODULE_SERVICE_PREFIX); } } diff --git a/server/src/test/java/org/jboss/as/server/deployment/module/ModuleDependencyUnitTestCase.java b/server/src/test/java/org/jboss/as/server/deployment/module/ModuleDependencyUnitTestCase.java index ab2d9c61d53..6914551905c 100644 --- a/server/src/test/java/org/jboss/as/server/deployment/module/ModuleDependencyUnitTestCase.java +++ b/server/src/test/java/org/jboss/as/server/deployment/module/ModuleDependencyUnitTestCase.java @@ -22,7 +22,7 @@ public class ModuleDependencyUnitTestCase { public void testBasicBuilder() { ModuleDependency dep = ModuleDependency.Builder.of(TEST_LOADER, MODULE_NAME).build(); assertEquals(TEST_LOADER, dep.getModuleLoader()); - assertEquals(MODULE_NAME, dep.getIdentifier().getName()); + assertEquals(MODULE_NAME, dep.getDependencyModule()); assertFalse(dep.isExport()); assertFalse(dep.isImportServices()); assertFalse(dep.isOptional()); @@ -43,7 +43,7 @@ public void testSpecifiedBuilder() { .build(); assertEquals(TEST_LOADER, dep.getModuleLoader()); - assertEquals(MODULE_NAME, dep.getIdentifier().getName()); + assertEquals(MODULE_NAME, dep.getDependencyModule()); assertTrue(dep.isExport()); assertTrue(dep.isImportServices()); assertTrue(dep.isOptional()); diff --git a/server/src/test/java/org/jboss/as/server/deployment/module/ModuleSpecificationTestCase.java b/server/src/test/java/org/jboss/as/server/deployment/module/ModuleSpecificationTestCase.java index a8356cf2a99..640622c856c 100644 --- a/server/src/test/java/org/jboss/as/server/deployment/module/ModuleSpecificationTestCase.java +++ b/server/src/test/java/org/jboss/as/server/deployment/module/ModuleSpecificationTestCase.java @@ -61,11 +61,11 @@ public void testUserDependencyConsistency() { ); // Removal consistency - ms.removeUserDependencies(md -> md.getIdentifier().toString().equals("a")); + ms.removeUserDependencies(md -> md.getDependencyModule().equals("a")); Set userDepSet = ms.getUserDependenciesSet(); assertEquals(INITIAL_DEPS.size() /* the initials, minus 'a', plus 'd'*/, userDepSet.size()); for (ModuleDependency md : ALL_DEPS) { - boolean shouldFind = !md.getIdentifier().toString().equals("a"); + boolean shouldFind = !md.getDependencyModule().equals("a"); assertEquals(shouldFind, userDepSet.contains(md)); } assertTrue(userDepSet.contains(DEP_D)); @@ -95,12 +95,12 @@ public void testLocalDependencyConsistency() { public void testModuleAliases() { ModuleSpecification ms = new ModuleSpecification(); for (ModuleDependency dependency : ALL_DEPS) { - ms.addModuleAlias(dependency.getIdentifier().toString()); + ms.addModuleAlias(dependency.getDependencyModule()); } Set aliases = ms.getModuleAliases(); assertEquals(ALL_DEPS.size() - 1, aliases.size()); for (ModuleDependency dep : INITIAL_DEPS) { - assertTrue(dep + " missing", aliases.contains(dep.getIdentifier().toString())); + assertTrue(dep + " missing", aliases.contains(dep.getDependencyModule())); } } @@ -174,7 +174,7 @@ private ModuleSpecification dependencySetConsistencyTest(BiConsumer fictitious = ms.getFictitiousExcludedDependencies(); - Set expected = exclusionsSupported ? Set.of(DEP_E.getIdentifier().toString()) : Set.of(DEP_D.getIdentifier().toString(), DEP_E.getIdentifier().toString()); + Set expected = exclusionsSupported ? Set.of(DEP_E.getDependencyModule()) : Set.of(DEP_D.getDependencyModule(), DEP_E.getDependencyModule()); assertEquals(expected, fictitious); return ms;