Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WFCORE-7095] Deprecate ModuleDependency.getIdentifier() for removal #6282

Merged
merged 1 commit into from
Dec 23, 2024
Merged

[WFCORE-7095] Deprecate ModuleDependency.getIdentifier() for removal #6282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions server/src/main/java/org/jboss/as/server/deployment/DeploymentListModulesHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand Down
6 changes: 3 additions & 3 deletions server/src/main/java/org/jboss/as/server/deployment/annotation/CompositeIndexProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
}
}
Expand Down
12 changes: 6 additions & 6 deletions ...er/src/main/java/org/jboss/as/server/deployment/module/DeploymentVisibilityProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/jboss/as/server/deployment/module/ModuleAliasChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
40 changes: 27 additions & 13 deletions server/src/main/java/org/jboss/as/server/deployment/module/ModuleDependency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Objects;
import java.util.Optional;

import org.jboss.as.controller.ModuleIdentifierUtil;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: uses of canonicalModuleIdentifier in this class are a kind of second line of defense I plan to remove once I'm confident we've correctly handled canonicalization in the code that directly handles external input of module names.

import org.jboss.modules.ModuleIdentifier;
import org.jboss.modules.ModuleLoader;
import org.jboss.modules.filter.PathFilter;
Expand All @@ -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;
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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.
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
12 changes: 9 additions & 3 deletions server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -337,8 +338,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());
}
Expand Down
12 changes: 6 additions & 6 deletions server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,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());
}
}

Expand Down Expand Up @@ -180,12 +180,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());
}
}

Expand Down Expand Up @@ -234,15 +234,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();
}
Expand Down
6 changes: 3 additions & 3 deletions ...src/main/java/org/jboss/as/server/deployment/module/SubDeploymentDependencyProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
}
Expand Down
Loading
Loading