From 79e72a49034c2699edf378c88434d1cbe1129760 Mon Sep 17 00:00:00 2001 From: Martin Desruisseaux Date: Sat, 13 Sep 2025 11:44:11 +0200 Subject: [PATCH] [MNG-8696] Hide the cache from DefaultDependencyResolverResult constructor (#2347) Make package-private the `DefaultDependencyResolverResult` constructor having a `PathModularizationCache` argument, and add a public constructor without the cache argument for code in other packages that need to instantiate. The public constructor may be temporary, until we decide in the future where to store session-wide cache. --- .../internal/impl/DefaultProjectBuilder.java | 3 +- .../maven/impl/DefaultDependencyResolver.java | 39 +++++++++++++------ .../impl/DefaultDependencyResolverResult.java | 34 ++++++++++++---- 3 files changed, 55 insertions(+), 21 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProjectBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProjectBuilder.java index b51241642aae..62ae3b9db97d 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProjectBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProjectBuilder.java @@ -189,8 +189,7 @@ public Severity getSeverity() { public Optional 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) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolver.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolver.java index ac6ba73d5f7e..814e71bace23 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolver.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolver.java @@ -68,6 +68,22 @@ @Singleton public class DefaultDependencyResolver implements DependencyResolver { + /** + * Cache of information about the modules contained in a path element. + * + *

TODO: 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.

+ */ + 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) @@ -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); } @@ -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 repositories = request.getRepositories() != null ? request.getRepositories() @@ -191,18 +211,13 @@ public DependencyResolverResult resolve(DependencyResolverRequest request) .map(Artifact::toCoordinates) .collect(Collectors.toList()); Predicate 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) { @@ -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) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolverResult.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolverResult.java index fa7ace6e0d05..4b67c9ee32c6 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolverResult.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultDependencyResolverResult.java @@ -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; @@ -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. */ @@ -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)}. + * + *

WARNING: this constructor may be removed in a future Maven release. + * 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.

+ * + * @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 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)}. @@ -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 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); @@ -350,7 +370,7 @@ public DependencyResolverRequest getRequest() { @Override public List getExceptions() { - return exceptions; + return Collections.unmodifiableList(exceptions); } @Override @@ -360,22 +380,22 @@ public Node getRoot() { @Override public List getNodes() { - return nodes; + return Collections.unmodifiableList(nodes); } @Override public List getPaths() { - return paths; + return Collections.unmodifiableList(paths); } @Override public Map> getDispatchedPaths() { - return dispatchedPaths; + return Collections.unmodifiableMap(dispatchedPaths); } @Override public Map getDependencies() { - return dependencies; + return Collections.unmodifiableMap(dependencies); } @Override