Skip to content

[MNG-8696] Hide the cache from DefaultDependencyResolverResult constructor #11154

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

Merged
merged 1 commit into from
Sep 22, 2025
Merged

[MNG-8696] Hide the cache from DefaultDependencyResolverResult constructor #11154

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
3 changes: 1 addition & 2 deletions impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProjectBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ public Severity getSeverity() {
public Optional<DependencyResolverResult> getDependencyResolverResult() {
return Optional.ofNullable(res.getDependencyResolutionResult())
.map(r -> new DefaultDependencyResolverResult(
// TODO: this should not be null
null, null, r.getCollectionErrors(), session.getNode(r.getDependencyGraph()), 0));
null, r.getCollectionErrors(), session.getNode(r.getDependencyGraph()), 0));
}
};
} catch (ProjectBuildingException e) {
Expand Down
39 changes: 27 additions & 12 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@
@Singleton
Copy link
Contributor

@gnodet gnodet Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If the bean contains session-specific data, maybe we want to make it @SessionScope rather than @Singleton. That may be one possible answer to "until clarified where to store session-wide caches".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I have misused the "session" word, since I'm not sure which data are in a session. This cache should be valid as long as the JAR files do not change. For JAR files that are not produced by the project, it should be the JVM lifetime. For JAR files produced by the project and used as dependencies by sub-projects, this is an open question...

public class DefaultDependencyResolver implements DependencyResolver {

/**
* Cache of information about the modules contained in a path element.
*
* <p><b>TODO:</b> This field should not be in this class, because the cache should be global to the session.
* This field exists here only temporarily, until clarified where to store session-wide caches.</p>
*/
private final PathModularizationCache moduleCache;

/**
* Creates an initially empty resolver.
*/
public DefaultDependencyResolver() {
// TODO: the cache should not be instantiated here, but should rather be session-wide.
moduleCache = new PathModularizationCache();
}

@Nonnull
@Override
public DependencyResolverResult collect(@Nonnull DependencyResolverRequest request)
Expand Down Expand Up @@ -126,7 +142,11 @@ public DependencyResolverResult collect(@Nonnull DependencyResolverRequest reque
final CollectResult result =
session.getRepositorySystem().collectDependencies(systemSession, collectRequest);
return new DefaultDependencyResolverResult(
null, null, result.getExceptions(), session.getNode(result.getRoot(), request.getVerbose()), 0);
null,
moduleCache,
result.getExceptions(),
session.getNode(result.getRoot(), request.getVerbose()),
0);
} catch (DependencyCollectionException e) {
throw new DependencyResolverException("Unable to collect dependencies", e);
}
Expand Down Expand Up @@ -171,8 +191,8 @@ public DependencyResolverResult resolve(DependencyResolverRequest request)
InternalSession session =
InternalSession.from(requireNonNull(request, "request").getSession());
RequestTraceHelper.ResolverTrace trace = RequestTraceHelper.enter(session, request);
DependencyResolverResult result;
try {
DependencyResolverResult result;
DependencyResolverResult collectorResult = collect(request);
List<RemoteRepository> repositories = request.getRepositories() != null
? request.getRepositories()
Expand All @@ -191,18 +211,13 @@ public DependencyResolverResult resolve(DependencyResolverRequest request)
.map(Artifact::toCoordinates)
.collect(Collectors.toList());
Predicate<PathType> filter = request.getPathTypeFilter();
DefaultDependencyResolverResult resolverResult = new DefaultDependencyResolverResult(
null, moduleCache, collectorResult.getExceptions(), collectorResult.getRoot(), nodes.size());
if (request.getRequestType() == DependencyResolverRequest.RequestType.FLATTEN) {
DefaultDependencyResolverResult flattenResult = new DefaultDependencyResolverResult(
null, null, collectorResult.getExceptions(), collectorResult.getRoot(), nodes.size());
for (Node node : nodes) {
flattenResult.addNode(node);
resolverResult.addNode(node);
}
result = flattenResult;
} else {
PathModularizationCache cache =
new PathModularizationCache(); // TODO: should be project-wide cache.
DefaultDependencyResolverResult resolverResult = new DefaultDependencyResolverResult(
null, cache, collectorResult.getExceptions(), collectorResult.getRoot(), nodes.size());
ArtifactResolverResult artifactResolverResult =
session.getService(ArtifactResolver.class).resolve(session, coordinates, repositories);
for (Node node : nodes) {
Expand All @@ -217,13 +232,13 @@ public DependencyResolverResult resolve(DependencyResolverRequest request)
throw cannotReadModuleInfo(path, e);
}
}
result = resolverResult;
}
result = resolverResult;
}
return result;
} finally {
RequestTraceHelper.exit(trace);
}
return result;
}

private static DependencyResolverException cannotReadModuleInfo(final Path path, final IOException cause) {
Expand Down
34 changes: 27 additions & 7 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolverResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
Expand Down Expand Up @@ -56,6 +57,7 @@ public class DefaultDependencyResolverResult implements DependencyResolverResult
* The corresponding request.
*/
private final DependencyResolverRequest request;

/**
* The exceptions that occurred while building the dependency graph.
*/
Expand Down Expand Up @@ -97,6 +99,24 @@ public class DefaultDependencyResolverResult implements DependencyResolverResult
*/
private final PathModularizationCache cache;

/**
* Creates an initially empty result with a temporary cache.
* Callers should add path elements by calls to {@link #addDependency(Node, Dependency, Predicate, Path)}.
*
* <p><b>WARNING: this constructor may be removed in a future Maven release.</b>
* The reason is because {@code DefaultDependencyResolverResult} needs a cache, which should
* preferably be session-wide. How to manage such caches has not yet been clarified.</p>
*
* @param request the corresponding request
* @param exceptions the exceptions that occurred while building the dependency graph
* @param root the root node of the dependency graph
* @param count estimated number of dependencies
*/
public DefaultDependencyResolverResult(
DependencyResolverRequest request, List<Exception> exceptions, Node root, int count) {
this(request, new PathModularizationCache(), exceptions, root, count);
}

/**
* Creates an initially empty result. Callers should add path elements by calls
* to {@link #addDependency(Node, Dependency, Predicate, Path)}.
Expand All @@ -107,14 +127,14 @@ public class DefaultDependencyResolverResult implements DependencyResolverResult
* @param root the root node of the dependency graph
* @param count estimated number of dependencies
*/
public DefaultDependencyResolverResult(
DefaultDependencyResolverResult(
DependencyResolverRequest request,
PathModularizationCache cache,
List<Exception> exceptions,
Node root,
int count) {
this.request = request;
this.cache = cache;
this.cache = Objects.requireNonNull(cache);
this.exceptions = exceptions;
this.root = root;
nodes = new ArrayList<>(count);
Expand Down Expand Up @@ -350,7 +370,7 @@ public DependencyResolverRequest getRequest() {

@Override
public List<Exception> getExceptions() {
return exceptions;
return Collections.unmodifiableList(exceptions);
}

@Override
Expand All @@ -360,22 +380,22 @@ public Node getRoot() {

@Override
public List<Node> getNodes() {
return nodes;
return Collections.unmodifiableList(nodes);
}

@Override
public List<Path> getPaths() {
return paths;
return Collections.unmodifiableList(paths);
}

@Override
public Map<PathType, List<Path>> getDispatchedPaths() {
return dispatchedPaths;
return Collections.unmodifiableMap(dispatchedPaths);
}

@Override
public Map<Dependency, Path> getDependencies() {
return dependencies;
return Collections.unmodifiableMap(dependencies);
}

@Override
Expand Down
Loading