Fix consumer pom builder (#1491)

api/maven-api-core/src/main/java/org/apache/maven/api/Session.java

@@ -37,6 +37,10 @@
 /**
  * The session to install / deploy / resolve artifacts and dependencies.
  *
+ * TODO: move the remote repositories in the requests (plugins will need to access this list somehow)
+ * TODO: add request trace so that requests can be linked together and through the resolver
+ * TODO: add a Request interface holding session + parent request
+ *
  * @since 4.0.0
  */
 @Experimental

api/maven-api-core/src/main/java/org/apache/maven/api/services/LookupException.java

@@ -27,6 +27,11 @@
  */
 @Experimental
 public class LookupException extends MavenException {
+
+    public LookupException(String message) {
+        super(message);
+    }
+
     /**
      * @param message the message to give
      * @param e the {@link Exception}

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelBuilderRequest.java

@@ -37,7 +37,8 @@
  * Request used to build a {@link org.apache.maven.api.Project} using
  * the {@link ProjectBuilder} service.
  *
- * TODO: add validationLevel, activeProfileIds, inactiveProfileIds, resolveDependencies
+ * TODO: replace ModelRepositoryHolder with just the enum for the strategy
+ * TODO: replace validation level with an enum (though, we usually need just a boolean)
  *
  * @since 4.0.0
  */
@@ -138,8 +139,15 @@ public interface ModelBuilderRequest {
     @Nonnull
     Map<String, String> getUserProperties();
 
+    @Nonnull
     ModelResolver getModelResolver();
 
+    @Nonnull
+    ModelRepositoryHolder getModelRepositoryHolder();
+
+    @Nullable
+    ModelCache getModelCache();
+
     @Nullable
     Object getListener();
 
@@ -197,6 +205,8 @@ class ModelBuilderRequestBuilder {
         Map<String, String> systemProperties;
         Map<String, String> userProperties;
         ModelResolver modelResolver;
+        ModelRepositoryHolder modelRepositoryHolder;
+        ModelCache modelCache;
         Object listener;
         ModelBuilderResult interimResult;
         ModelTransformerContextBuilder transformerContextBuilder;
@@ -217,6 +227,8 @@ class ModelBuilderRequestBuilder {
             this.systemProperties = request.getSystemProperties();
             this.userProperties = request.getUserProperties();
             this.modelResolver = request.getModelResolver();
+            this.modelRepositoryHolder = request.getModelRepositoryHolder();
+            this.modelCache = request.getModelCache();
             this.listener = request.getListener();
             this.interimResult = request.getInterimResult();
             this.transformerContextBuilder = request.getTransformerContextBuilder();
@@ -287,6 +299,16 @@ public ModelBuilderRequestBuilder modelResolver(ModelResolver modelResolver) {
             return this;
         }
 
+        public ModelBuilderRequestBuilder modelRepositoryHolder(ModelRepositoryHolder modelRepositoryHolder) {
+            this.modelRepositoryHolder = modelRepositoryHolder;
+            return this;
+        }
+
+        public ModelBuilderRequestBuilder modelCache(ModelCache modelCache) {
+            this.modelCache = modelCache;
+            return this;
+        }
+
         public ModelBuilderRequestBuilder listener(Object listener) {
             this.listener = listener;
             return this;
@@ -318,6 +340,8 @@ public ModelBuilderRequest build() {
                     systemProperties,
                     userProperties,
                     modelResolver,
+                    modelRepositoryHolder,
+                    modelCache,
                     listener,
                     interimResult,
                     transformerContextBuilder);
@@ -336,6 +360,8 @@ private static class DefaultModelBuilderRequest extends BaseRequest implements M
             private final Map<String, String> systemProperties;
             private final Map<String, String> userProperties;
             private final ModelResolver modelResolver;
+            private final ModelRepositoryHolder modelRepositoryHolder;
+            private final ModelCache modelCache;
             private final Object listener;
             private final ModelBuilderResult interimResult;
             private final ModelTransformerContextBuilder transformerContextBuilder;
@@ -355,6 +381,8 @@ private static class DefaultModelBuilderRequest extends BaseRequest implements M
                     Map<String, String> systemProperties,
                     Map<String, String> userProperties,
                     ModelResolver modelResolver,
+                    ModelRepositoryHolder modelRepositoryHolder,
+                    ModelCache modelCache,
                     Object listener,
                     ModelBuilderResult interimResult,
                     ModelTransformerContextBuilder transformerContextBuilder) {
@@ -372,6 +400,8 @@ private static class DefaultModelBuilderRequest extends BaseRequest implements M
                         systemProperties != null ? Map.copyOf(systemProperties) : session.getSystemProperties();
                 this.userProperties = userProperties != null ? Map.copyOf(userProperties) : session.getUserProperties();
                 this.modelResolver = modelResolver;
+                this.modelRepositoryHolder = modelRepositoryHolder;
+                this.modelCache = modelCache;
                 this.listener = listener;
                 this.interimResult = interimResult;
                 this.transformerContextBuilder = transformerContextBuilder;
@@ -437,6 +467,16 @@ public ModelResolver getModelResolver() {
                 return modelResolver;
             }
 
+            @Override
+            public ModelRepositoryHolder getModelRepositoryHolder() {
+                return modelRepositoryHolder;
+            }
+
+            @Override
+            public ModelCache getModelCache() {
+                return modelCache;
+            }
+
             public Object getListener() {
                 return listener;
             }

maven-api-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java → api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelCache.java

@@ -16,12 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.maven.api.services.model;
+package org.apache.maven.api.services;
 
 import java.util.function.Supplier;
 
-import org.apache.maven.api.services.Source;
-
 /**
  * Caches auxiliary data used during model building like already processed raw/effective models. The data in the cache
  * is meant for exclusive consumption by the model builder and is opaque to the cache implementation. The cache key is

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelRepositoryHolder.java

@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.api.services;
+
+import java.util.List;
+
+import org.apache.maven.api.RemoteRepository;
+import org.apache.maven.api.model.Repository;
+
+public interface ModelRepositoryHolder {
+
+    void merge(List<Repository> repos, boolean replace);
+
+    List<RemoteRepository> getRepositories();
+
+    ModelRepositoryHolder copy();
+}

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelResolver.java

@@ -19,107 +19,67 @@
 package org.apache.maven.api.services;
 
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 
 import org.apache.maven.api.Service;
 import org.apache.maven.api.Session;
 import org.apache.maven.api.annotations.Nonnull;
 import org.apache.maven.api.model.Dependency;
 import org.apache.maven.api.model.Parent;
-import org.apache.maven.api.model.Repository;
 
 /**
- * Resolves a POM from its coordinates. During the build process, the
- * {@link org.apache.maven.api.services.ModelBuilder} will add any relevant repositories to the model resolver. In
- * other words, the model resolver is stateful and should not be reused across multiple model building requests.
- *
+ * Resolves a POM from its coordinates.
  */
 public interface ModelResolver extends Service {
 
-    /**
-     * Tries to resolve the POM for the specified coordinates.
-     *
-     * @param groupId The group identifier of the POM, must not be {@code null}.
-     * @param artifactId The artifact identifier of the POM, must not be {@code null}.
-     * @param version The version of the POM, must not be {@code null}.
-     * @return The source of the requested POM, never {@code null}.
-     * @throws ModelResolverException If the POM could not be resolved from any configured repository.
-     */
-    @Nonnull
-    ModelSource resolveModel(
-            @Nonnull Session session, @Nonnull String groupId, @Nonnull String artifactId, @Nonnull String version)
-            throws ModelResolverException;
-
     /**
      * Tries to resolve the POM for the specified parent coordinates possibly updating {@code parent}.
-     * <p>
-     * Unlike the {@link #resolveModel(Session, String, String, String)} method, this method
-     * supports version ranges and updates the given {@code parent} instance to match the returned {@code ModelSource}.
-     * If {@code parent} declares a version range, the version corresponding to the returned {@code ModelSource} will
-     * be set on the given {@code parent}.
-     * </p>
      *
+     * @param session The session to use to resolve the model, must not be {@code null}.
      * @param parent The parent coordinates to resolve, must not be {@code null}.
-     *
+     * @param modified a holder for the updated parent, must not be {@code null}.
      * @return The source of the requested POM, never {@code null}.
-     *
      * @throws ModelResolverException If the POM could not be resolved from any configured repository.
      */
     @Nonnull
-    ModelSource resolveModel(
+    default ModelSource resolveModel(
             @Nonnull Session session, @Nonnull Parent parent, @Nonnull AtomicReference<Parent> modified)
-            throws ModelResolverException;
+            throws ModelResolverException {
+        return resolveModel(
+                session,
+                parent.getGroupId(),
+                parent.getArtifactId(),
+                parent.getVersion(),
+                version -> modified.set(parent.withVersion(version)));
+    }
 
     /**
      * Tries to resolve the POM for the specified dependency coordinates possibly updating {@code dependency}.
-     * <p>
-     * Unlike the {@link #resolveModel(Session, String, String, String)} method, this method
-     * supports version ranges and updates the given {@code dependency} instance to match the returned
-     * {@code ModelSource}. If {@code dependency} declares a version range, the version corresponding to the returned
-     * {@code ModelSource} will be set on the given {@code dependency}.
-     * </p>
      *
+     * @param session The session to use to resolve the model, must not be {@code null}.
      * @param dependency The dependency coordinates to resolve, must not be {@code null}.
-     *
+     * @param modified a holder for the updated dependency, must not be {@code null}.
      * @return The source of the requested POM, never {@code null}.
-     *
      * @throws ModelResolverException If the POM could not be resolved from any configured repository.
-     *
-     * @see Dependency#clone()
      */
     @Nonnull
-    ModelSource resolveModel(
+    default ModelSource resolveModel(
             @Nonnull Session session, @Nonnull Dependency dependency, @Nonnull AtomicReference<Dependency> modified)
-            throws ModelResolverException;
+            throws ModelResolverException {
+        return resolveModel(
+                session,
+                dependency.getGroupId(),
+                dependency.getArtifactId(),
+                dependency.getVersion(),
+                version -> modified.set(dependency.withVersion(version)));
+    }
 
-    /**
-     * Adds a repository to use for subsequent resolution requests. The order in which repositories are added matters,
-     * repositories that were added first should also be searched first. When multiple repositories with the same
-     * identifier are added, only the first repository being added will be used.
-     *
-     * @param repository The repository to add to the internal search chain, must not be {@code null}.
-     * @throws ModelResolverException If the repository could not be added (e.g. due to invalid URL or layout).
-     */
-    void addRepository(@Nonnull Session session, Repository repository) throws ModelResolverException;
-
-    /**
-     * Adds a repository to use for subsequent resolution requests. The order in which repositories are added matters,
-     * repositories that were added first should also be searched first. When multiple repositories with the same
-     * identifier are added, then the value of the replace argument determines the behaviour.
-     *
-     * If replace is false then any existing repository with the same Id will remain in use. If replace
-     * is true the new repository replaces the original.
-     *
-     * @param repository The repository to add to the internal search chain, must not be {@code null}.
-     * @throws ModelResolverException If the repository could not be added (e.g. due to invalid URL or layout).
-     */
-    void addRepository(@Nonnull Session session, Repository repository, boolean replace) throws ModelResolverException;
-
-    /**
-     * Clones this resolver for usage in a forked resolution process. In general, implementors need not provide a deep
-     * clone. The only requirement is that invocations of {@link #addRepository(Session, Repository)} on the clone do not affect
-     * the state of the original resolver and vice versa.
-     *
-     * @return The cloned resolver, never {@code null}.
-     */
-    ModelResolver newCopy();
+    @Nonnull
+    ModelSource resolveModel(
+            @Nonnull Session session,
+            @Nonnull String groupId,
+            @Nonnull String artifactId,
+            @Nonnull String version,
+            @Nonnull Consumer<String> resolvedVersion)
+            throws ModelResolverException;
 }

api/maven-api-core/src/main/java/org/apache/maven/api/services/PathSource.java

@@ -79,4 +79,9 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hash(path);
     }
+
+    @Override
+    public String toString() {
+        return "PathSource[" + "location='" + location + '\'' + ", " + "path=" + path + ']';
+    }
 }

api/maven-api-core/src/main/java/org/apache/maven/api/services/RepositoryFactory.java

@@ -19,10 +19,12 @@
 package org.apache.maven.api.services;
 
 import java.nio.file.Path;
+import java.util.List;
 
 import org.apache.maven.api.LocalRepository;
 import org.apache.maven.api.RemoteRepository;
 import org.apache.maven.api.Service;
+import org.apache.maven.api.Session;
 import org.apache.maven.api.annotations.Experimental;
 import org.apache.maven.api.annotations.Nonnull;
 import org.apache.maven.api.model.Repository;
@@ -43,4 +45,11 @@ public interface RepositoryFactory extends Service {
 
     @Nonnull
     RemoteRepository createRemote(@Nonnull Repository repository);
+
+    @Nonnull
+    List<RemoteRepository> aggregate(
+            @Nonnull Session session,
+            @Nonnull List<RemoteRepository> dominant,
+            @Nonnull List<RemoteRepository> recessive,
+            boolean processRecessive);
 }

maven-api-impl/pom.xml

@@ -118,6 +118,11 @@ under the License.
       <artifactId>mockito-junit-jupiter</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.maven</groupId>
       <artifactId>maven-di</artifactId>

maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultArtifactResolver.java

@@ -76,7 +76,7 @@ public ArtifactResolverResult resolve(ArtifactResolverRequest request)
             }
             return () -> paths;
         } catch (ArtifactResolutionException e) {
-            throw new ArtifactResolverException("Unable to resolve artifact", e);
+            throw new ArtifactResolverException("Unable to resolve artifact: " + e.getMessage(), e);
         }
     }
 }

maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultBuilderProblem.java

@@ -76,7 +76,7 @@ public Severity getSeverity() {
     @Override
     public String getLocation() {
         StringBuilder buffer = new StringBuilder(256);
-        if (!getSource().isEmpty()) {
+        if (getSource() != null && !getSource().isEmpty()) {
             buffer.append(getSource());
         }
         if (getLineNumber() > 0) {