[MNG-8696] Hide the cache from DefaultDependencyResolverResult constructor (apache#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.

Author: desruisseaux

impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProjectBuilder.java

@@ -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) {

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.
+     *
+     * <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)
@@ -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<RemoteRepository> repositories = request.getRepositories() != null
                     ? request.getRepositories()
@@ -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) {
@@ -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) {

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)}.
+     *
+     * <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)}.
@@ -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);
@@ -350,7 +370,7 @@ public DependencyResolverRequest getRequest() {
 
     @Override
     public List<Exception> getExceptions() {
-        return exceptions;
+        return Collections.unmodifiableList(exceptions);
     }
 
     @Override
@@ -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