Skip to content

Commit

Permalink
Merge pull request #5768 from bstansberry/WFCORE-6608
Browse files Browse the repository at this point in the history 
[WFCORE-6601][WFCORE-6607][WFCORE-6608] ModuleSpecification dependency tracking fixes
  • Loading branch information
@yersan
yersan authored Nov 17, 2023
2 parents cc77d6b + 8bafe46 commit 07a310e
Show file tree
Hide file tree
Showing 4 changed files with 218 additions and 47 deletions.
16 changes: 9 additions & 7 deletions server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
import java.security.Permission;
import java.security.Permissions;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.List;
import java.util.PropertyPermission;
import java.util.Set;
import java.util.function.Consumer;

import org.jboss.as.server.deployment.Attachments;
Expand Down Expand Up @@ -146,9 +148,9 @@ private void deployModuleSpec(final DeploymentPhaseContext phaseContext) throws
* @param module The additional module
*/
private void addAllDependenciesAndPermissions(final ModuleSpecification moduleSpecification, final AdditionalModuleSpecification module) {
module.addSystemDependencies(moduleSpecification.getSystemDependencies());
module.addLocalDependencies(moduleSpecification.getLocalDependencies());
for(ModuleDependency dep : moduleSpecification.getUserDependencies()) {
module.addSystemDependencies(moduleSpecification.getSystemDependenciesSet());
module.addLocalDependencies(moduleSpecification.getLocalDependenciesSet());
for(ModuleDependency dep : moduleSpecification.getUserDependenciesSet()) {
if(!dep.getIdentifier().equals(module.getModuleIdentifier())) {
module.addUserDependency(dep);
}
Expand Down Expand Up @@ -202,9 +204,9 @@ private ServiceName createModuleService(final DeploymentPhaseContext phaseContex
for (final DependencySpec dep : moduleSpecification.getModuleSystemDependencies()) {
specBuilder.addDependency(dep);
}
final List<ModuleDependency> dependencies = moduleSpecification.getSystemDependencies();
final List<ModuleDependency> localDependencies = moduleSpecification.getLocalDependencies();
final List<ModuleDependency> userDependencies = moduleSpecification.getUserDependencies();
final Set<ModuleDependency> dependencies = moduleSpecification.getSystemDependenciesSet();
final Set<ModuleDependency> localDependencies = moduleSpecification.getLocalDependenciesSet();
final Set<ModuleDependency> userDependencies = moduleSpecification.getUserDependenciesSet();

final List<PermissionFactory> permFactories = moduleSpecification.getPermissionFactories();

Expand Down Expand Up @@ -301,7 +303,7 @@ private void installAliases(final ModuleSpecification moduleSpecification, final
}
}

private void createDependencies(final ModuleSpec.Builder specBuilder, final List<ModuleDependency> apiDependencies, final boolean requireTransitive) {
private void createDependencies(final ModuleSpec.Builder specBuilder, final Collection<ModuleDependency> apiDependencies, final boolean requireTransitive) {
if (apiDependencies != null) {
for (final ModuleDependency dependency : apiDependencies) {
final boolean export = requireTransitive ? true : dependency.isExport();
Expand Down
158 changes: 123 additions & 35 deletions server/src/main/java/org/jboss/as/server/deployment/module/ModuleSpecification.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;

import org.jboss.as.server.deployment.SimpleAttachable;
import org.jboss.modules.DependencySpec;
Expand All @@ -24,6 +25,10 @@
/**
* Information used to build a module.
*
* <strong>This class is not thread safe.</strong> It should only be used by the deployment unit processors
* associated with a single deployment unit, with a parent deployment and a subdeployment considered to
* be separate deployments.
*
* @author Stuart Douglas
* @author Marius Bogoevici
*/
Expand All @@ -32,15 +37,19 @@ public class ModuleSpecification extends SimpleAttachable {
/**
* System dependencies are dependencies that are added automatically by the container.
*/
private final List<ModuleDependency> systemDependencies = new ArrayList<>();
private final Set<ModuleDependency> systemDependenciesSet = new LinkedHashSet<>();

/** Unique names of system dependencies, used to ensure only one system ModuleDependency per name is added. */
private final Set<ModuleIdentifier> systemDependenciesSet = new HashSet<>();
/**
* List view of {@link #systemDependenciesSet}.
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated(forRemoval = true)
private final List<ModuleDependency> systemDependencies = new ArrayList<>();

/**
* Local dependencies are dependencies on other parts of the deployment, such as class-path entry
* Local dependencies are dependencies on other parts of the deployment, such as a class-path entry
*/
private final Set<ModuleDependency> localDependencies = new LinkedHashSet<>();
private final Set<ModuleDependency> localDependenciesSet = new LinkedHashSet<>();

/**
* If set to true this indicates that a dependency on this module requires a dependency on all it's local
Expand All @@ -54,11 +63,14 @@ public class ModuleSpecification extends SimpleAttachable {
* <p/>
* User dependencies are not affected by exclusions.
*/
private final List<ModuleDependency> userDependencies = new ArrayList<>();
private final Set<ModuleDependency> userDependenciesSet = new HashSet<>();

/**
* Set view of user dependencies, used to prevent duplicates in the userDependencies list.
* List view of {@link #userDependenciesSet}.
*/
private final Set<ModuleDependency> userDependenciesSet = new HashSet<>();
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated(forRemoval = true)
private final List<ModuleDependency> userDependencies = new ArrayList<>();

private final List<ResourceLoaderSpec> resourceLoaders = new ArrayList<>();

Expand Down Expand Up @@ -122,65 +134,92 @@ public class ModuleSpecification extends SimpleAttachable {
* JBoss modules system dependencies, which allow you to specify dependencies on the app class loader
* to get access to JDK classes.
*/
private final List<DependencySpec> moduleSystemDependencies = new ArrayList<DependencySpec>();
private final List<DependencySpec> moduleSystemDependencies = new ArrayList<>();

/**
* The minimum permission set for this module, wrapped as {@code PermissionFactory} instances.
*/
private final List<PermissionFactory> permissionFactories = new ArrayList<>();

public void addSystemDependency(final ModuleDependency dependency) {
if (!exclusions.contains(dependency.getIdentifier()) && !systemDependenciesSet.contains(dependency.getIdentifier())) {
this.systemDependencies.add(dependency);
this.systemDependenciesSet.add(dependency.getIdentifier());
if (!exclusions.contains(dependency.getIdentifier())) {
if (systemDependenciesSet.add(dependency)) {
resetDependencyLists(this.systemDependencies);
}
} else {
excludedDependencies.add(dependency.getIdentifier());
}
}

public void addSystemDependencies(final Collection<ModuleDependency> dependencies) {
allDependencies = null;
for (final ModuleDependency dependency : dependencies) {
addSystemDependency(dependency);
}
}

public void addUserDependency(final ModuleDependency dependency) {
allDependencies = null;
if (this.userDependenciesSet.add(dependency)) {
this.userDependencies.add(dependency);
resetDependencyLists(this.userDependencies);
}
}

public void addUserDependencies(final Collection<ModuleDependency> dependencies) {
allDependencies = null;
for (final ModuleDependency dependency : dependencies) {
addUserDependency(dependency);
}
}

/**
* Remove user dependencies that match the predicate.
*
* @param predicate test for whether a dependency should be removed. Cannot be {@code null}.
*/
public void removeUserDependencies(final Predicate<ModuleDependency> predicate) {
Iterator<ModuleDependency> iter = userDependenciesSet.iterator();
while (iter.hasNext()) {
ModuleDependency md = iter.next();
if (predicate.test(md)) {
iter.remove();
resetDependencyLists(userDependencies);
}
}
}

public void addLocalDependency(final ModuleDependency dependency) {
allDependencies = null;
if (!exclusions.contains(dependency.getIdentifier())) {
this.localDependencies.add(dependency);
if (this.localDependenciesSet.add(dependency)) {
resetDependencyLists(null);
}
} else {
excludedDependencies.add(dependency.getIdentifier());
}
}

public void addLocalDependencies(final Collection<ModuleDependency> dependencies) {
allDependencies = null;
for (final ModuleDependency dependency : dependencies) {
addLocalDependency(dependency);
}
}

/** @deprecated use {@link #getSystemDependenciesSet()} */
@Deprecated(forRemoval = true)
public List<ModuleDependency> getSystemDependencies() {
if (systemDependencies.isEmpty()) {
systemDependencies.addAll(systemDependenciesSet);
}
return Collections.unmodifiableList(systemDependencies);
}

/**
* System dependencies are dependencies that are added automatically by the container.
*
* @return system dependencies iterable in order of addition. Will not return {@code null}.
*/
public Set<ModuleDependency> getSystemDependenciesSet() {
return Collections.unmodifiableSet(systemDependenciesSet);
}

public void addExclusion(final ModuleIdentifier exclusion) {
allDependencies = null;
final String targetModule = ModuleAliasChecker.getTargetModule(exclusion.toString());
if (targetModule != null) {
final ModuleIdentifier identifier = ModuleIdentifier.create(targetModule);
Expand All @@ -200,18 +239,20 @@ public void addExclusion(final ModuleIdentifier exclusion) {
}
// list of exclusions, aliases or target modules
exclusions.add(exclusion);
Iterator<ModuleDependency> it = systemDependencies.iterator();
Iterator<ModuleDependency> it = systemDependenciesSet.iterator();
while (it.hasNext()) {
final ModuleDependency dep = it.next();
if (dep.getIdentifier().equals(exclusion)) {
it.remove();
resetDependencyLists(this.systemDependencies);
}
}
it = localDependencies.iterator();
it = localDependenciesSet.iterator();
while (it.hasNext()) {
final ModuleDependency dep = it.next();
if (dep.getIdentifier().equals(exclusion)) {
it.remove();
resetDependencyLists(null);
}
}
}
Expand All @@ -223,24 +264,63 @@ public void addExclusions(final Iterable<ModuleIdentifier> exclusions) {
}


/**
* @deprecated use {@link #getLocalDependenciesSet()} ()}
*/
@Deprecated(forRemoval = true)
public List<ModuleDependency> getLocalDependencies() {
return Collections.unmodifiableList(new ArrayList<>(localDependencies));
return Collections.unmodifiableList(new ArrayList<>(localDependenciesSet));
}


/**
* Local dependencies are dependencies on other parts of the deployment, such as a class-path entry
*
* @return local dependencies iterable in order of addition. Will not return {@code null}.
*/
public Set<ModuleDependency> getLocalDependenciesSet() {
return Collections.unmodifiableSet(localDependenciesSet);
}

/**
* @deprecated use {@link #getUserDependenciesSet()}
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated(forRemoval = true)
public List<ModuleDependency> getUserDependencies() {
if (userDependencies.isEmpty()) {
userDependencies.addAll(userDependenciesSet);
}
return Collections.unmodifiableList(userDependencies);
}

/**
* User dependencies are dependencies that the user has specifically added, either via jboss-deployment-structure.xml
* or via the manifest.
* <p/>
* User dependencies are not affected by exclusions.
*
* @return user dependencies iterable in order of addition. Will not return {@code null}.
*/
public Set<ModuleDependency> getUserDependenciesSet() {
return Collections.unmodifiableSet(userDependenciesSet);
}

/**
* Gets a modifiable view of the user dependencies list.
*
* @return The user dependencies
*
* @deprecated use {@link #addUserDependency(ModuleDependency)} and {@link #removeUserDependencies(Predicate)}
*/
@SuppressWarnings("DeprecatedIsStillUsed")
@Deprecated(forRemoval = true)
public Collection<ModuleDependency> getMutableUserDependencies() {
allDependencies = null;
resetDependencyLists(this.userDependencies);
return userDependenciesSet;
}

@SuppressWarnings("unused")
public void addResourceLoader(final ResourceLoaderSpec resourceLoader) {
this.resourceLoaders.add(resourceLoader);
}
Expand Down Expand Up @@ -285,32 +365,32 @@ public boolean isPublicModule() {
return publicModule;
}

@SuppressWarnings("unused")
public void setPublicModule(boolean publicModule) {
this.publicModule = publicModule;
}

/**
* Returns true if the {@link #localDependencies} added for this {@link ModuleSpecification} should be made
* Returns true if the {@link #getLocalDependenciesSet() local dependencies} added for this {@link ModuleSpecification} should be made
* transitive (i.e. if any other module 'B' depends on the module 'A' represented by this {@link ModuleSpecification}, then
* module 'B' will be added with all "local dependencies" that are applicable for module "A"). Else returns false.
*
* @return
* @see {@link #localDependencies}
* @return {@code true} if local dependencies should be made transitive
* @see #getLocalDependenciesSet()
*/
public boolean isLocalDependenciesTransitive() {
return localDependenciesTransitive;
}

/**
* Sets whether the {@link #localDependencies} applicable for this {@link ModuleSpecification} are to be treated as transitive dependencies
* Sets whether the {@link #getLocalDependenciesSet() local dependencies} applicable for this {@link ModuleSpecification} are to be treated as transitive dependencies
* for modules which depend on the module represented by this {@link ModuleSpecification}
*
* @param localDependenciesTransitive True if the {@link #localDependencies} added for this {@link ModuleSpecification} should be made
* @param localDependenciesTransitive {@code true} if the {@link #getLocalDependenciesSet()} added for this {@link ModuleSpecification} should be made
* transitive (i.e. if any other module 'B' depends on the module 'A' represented by
* this {@link ModuleSpecification}, then module 'B' will be added with
* all "local dependencies" that are applicable for module "A"). False otherwise
* @return
* @see {@link #localDependencies}
* @see #getLocalDependenciesSet()
*/
public void setLocalDependenciesTransitive(final boolean localDependenciesTransitive) {
this.localDependenciesTransitive = localDependenciesTransitive;
Expand All @@ -324,6 +404,7 @@ public void setLocalLast(final boolean localLast) {
this.localLast = localLast;
}

@SuppressWarnings("unused")
public void addAlias(final ModuleIdentifier moduleIdentifier) {
aliases.add(moduleIdentifier);
}
Expand All @@ -338,10 +419,10 @@ public List<ModuleIdentifier> getAliases() {

public List<ModuleDependency> getAllDependencies() {
if (allDependencies == null) {
allDependencies = new ArrayList<ModuleDependency>();
allDependencies.addAll(systemDependencies);
allDependencies.addAll(userDependencies);
allDependencies.addAll(localDependencies);
allDependencies = new ArrayList<>();
allDependencies.addAll(systemDependenciesSet);
allDependencies.addAll(userDependenciesSet);
allDependencies.addAll(localDependenciesSet);
}
return allDependencies;
}
Expand Down Expand Up @@ -390,4 +471,11 @@ public Set<ModuleIdentifier> getNonexistentExcludedDependencies() {
return unExcludedModuleExclusion;
}

private void resetDependencyLists(List<ModuleDependency> listView) {
if (listView != null) {
listView.clear();
}
allDependencies = null;
}

}
Loading

0 comments on commit 07a310e

Please sign in to comment.