ProjectManager cleanup (#2073)

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

@@ -25,7 +25,6 @@
 import java.util.Optional;
 import java.util.stream.Stream;
 
-import org.apache.maven.api.Artifact;
 import org.apache.maven.api.Language;
 import org.apache.maven.api.ProducedArtifact;
 import org.apache.maven.api.Project;
@@ -39,53 +38,83 @@
 import org.apache.maven.api.annotations.Nullable;
 
 /**
- * Interface to manage the project during its lifecycle.
+ * Interface to manage the project state and artifacts during the Maven build lifecycle.
+ * This service provides operations to:
+ * <ul>
+ *   <li>Manage project artifacts (main and attached)</li>
+ *   <li>Handle source roots and resources</li>
+ *   <li>Access and modify project properties</li>
+ *   <li>Manage repository configurations</li>
+ *   <li>Handle project forking states</li>
+ * </ul>
+ *
+ * The service maintains the mutable state of projects as they progress through
+ * their build lifecycle, ensuring thread-safety and proper state management.
+ * All implementations must be thread-safe as they may be accessed concurrently
+ * during parallel builds.
  *
  * @since 4.0.0
+ * @see org.apache.maven.api.services.ProjectBuilder
+ * @see Project
  */
 @Experimental
 public interface ProjectManager extends Service {
     /**
      * Returns the path to the built project artifact file, if the project has been built.
+     * This path is only available after the artifact has been produced during the build lifecycle.
      *
-     * @param project the project for which to get the path
-     * @return the path of the built project artifact
+     * @param project the project to get the artifact path for
+     * @return an Optional containing the path to the built artifact if available,
+     *         or empty if the artifact hasn't been built yet
      */
     @Nonnull
-    Optional<Path> getPath(Project project);
+    Optional<Path> getPath(@Nonnull Project project);
 
     /**
-     * {@return an immutable collection of attached artifacts for given project}.
+     * Returns an immutable collection of attached artifacts for the given project.
+     * Attached artifacts are secondary artifacts produced during the build (e.g., sources jar,
+     * javadoc jar, test jars). These artifacts are created and attached during specific
+     * lifecycle phases, so the collection contents depend on the build phase when this method
+     * is called.
      *
-     * @param project the project from which to get the attached artifacts
+     * @param project the project to get attached artifacts for
+     * @return an immutable collection of attached artifacts, may be empty if no artifacts
+     *         have been attached yet
+     * @throws IllegalArgumentException if the project is null
+     * @see #getAllArtifacts(Project)
      */
     @Nonnull
-    Collection<ProducedArtifact> getAttachedArtifacts(Project project);
+    Collection<ProducedArtifact> getAttachedArtifacts(@Nonnull Project project);
 
     /**
-     * {@return project's all artifacts as immutable collection}. The list contains all artifacts, even the attached ones,
-     * if any. Hence, the list returned by this method depends on which lifecycle step of the build was it invoked.
-     * The head of returned list is result of {@link Project#getArtifacts()} method, so same applies here: the list can have
-     * minimum of one element. The maximum number of elements is in turn dependent on build configuration and lifecycle
-     * phase when this method was invoked (i.e. is javadoc jar built and attached, is sources jar built attached, are
-     * all the artifact signed, etc.).
-     * <p>
-     * This method is shorthand for {@link Project#getArtifacts()} and {@link #getAttachedArtifacts(Project)} methods.
-     *
-     * @param project the project from which to get all artifacts
-     *
-     * @see org.apache.maven.api.services.ArtifactManager#getPath(Artifact)
+     * Returns project's all artifacts as an immutable ordered collection. The collection contains:
+     * <ul>
+     *   <li>The project's artifacts ({@link Project#getArtifacts()}):
+     *     <ul>
+     *       <li>The POM artifact (always present)</li>
+     *       <li>The main project artifact (if applicable based on packaging)</li>
+     *     </ul>
+     *   </li>
+     *   <li>All attached artifacts in the order they were attached</li>
+     * </ul>
+     * The contents depend on the current lifecycle phase when this method is called, as artifacts
+     * are typically attached during specific phases (e.g., sources jar during package phase).
+     *
+     * @param project the project to get artifacts for
+     * @return an immutable ordered collection of all project artifacts
+     * @see #getAttachedArtifacts(Project)
      */
-    Collection<ProducedArtifact> getAllArtifacts(Project project);
+    @Nonnull
+    Collection<ProducedArtifact> getAllArtifacts(@Nonnull Project project);
 
     /**
      * Attaches an artifact to the project using the given file path. The artifact type will be
-     * determined from the file extension.
+     * determined from the file extension. This method is thread-safe and ensures proper
+     * synchronization of the project's artifact state.
      *
      * @param session the current build session
      * @param project the project to attach the artifact to
      * @param path the path to the artifact file
-     * @throws IllegalArgumentException if the session, project or path is null
      */
     default void attachArtifact(@Nonnull Session session, @Nonnull Project project, @Nonnull Path path) {
         String name = path.getFileName().toString();
@@ -103,8 +132,6 @@ default void attachArtifact(@Nonnull Session session, @Nonnull Project project,
      * @param project the project to attach the artifact to
      * @param type the type of the artifact (e.g., "jar", "war", "sources")
      * @param path the path to the artifact file
-     * @throws IllegalArgumentException if the session, project, type or path is null
-     *
      * @see org.apache.maven.api.Type
      */
     default void attachArtifact(
@@ -121,7 +148,6 @@ default void attachArtifact(
      * @param project the project to attach the artifact to
      * @param artifact the produced artifact to attach
      * @param path the path to the artifact file
-     * @throws IllegalArgumentException if the project, artifact or path is null
      */
     void attachArtifact(@Nonnull Project project, @Nonnull ProducedArtifact artifact, @Nonnull Path path);
 
@@ -154,7 +180,9 @@ default void attachArtifact(
      * @param scope the scope of the sources to return, or {@code null} for all scopes
      * @param language the language of the sources to return, or {@code null} for all languages
      */
-    Stream<SourceRoot> getEnabledSourceRoots(@Nonnull Project project, ProjectScope scope, Language language);
+    @Nonnull
+    Stream<SourceRoot> getEnabledSourceRoots(
+            @Nonnull Project project, @Nullable ProjectScope scope, @Nullable Language language);
 
     /**
      * Adds the given source to the given project.
@@ -189,17 +217,23 @@ void addSourceRoot(
             @Nonnull Project project, @Nonnull ProjectScope scope, @Nonnull Language language, @Nonnull Path directory);
 
     /**
-     * {@return an immutable list of project remote repositories} (directly specified or inherited).
+     * Returns an immutable list of project remote repositories (directly specified or inherited).
+     * The repositories are ordered by declaration order, with inherited repositories appearing
+     * after directly specified ones.
      *
      * @param project the project
+     * @return ordered list of remote repositories
      */
     @Nonnull
     List<RemoteRepository> getRemoteProjectRepositories(@Nonnull Project project);
 
     /**
-     * {@return an immutable list of project remote plugin repositories} (directly specified or inherited).
+     * Returns an immutable list of project plugin remote repositories (directly specified or inherited).
+     * The repositories are ordered by declaration order, with inherited repositories appearing
+     * after directly specified ones.
      *
      * @param project the project
+     * @return ordered list of remote repositories
      */
     @Nonnull
     List<RemoteRepository> getRemotePluginRepositories(@Nonnull Project project);
@@ -215,14 +249,26 @@ void addSourceRoot(
     Map<String, String> getProperties(@Nonnull Project project);
 
     /**
-     * Set a given project property.
+     * Set a given project property. Properties set through this method are only valid
+     * for the current build session and do not modify the underlying project model.
      *
      * @param project the project to modify
      * @param key they property's key
      * @param value the value or {@code null} to unset the property
      */
     void setProperty(@Nonnull Project project, @Nonnull String key, @Nullable String value);
 
+    /**
+     * Returns the original project being built when the input project is a forked project.
+     * During certain lifecycle phases, particularly for aggregator mojos, Maven may create
+     * a forked project (a copy of the original project) to execute a subset of the lifecycle.
+     * This method allows retrieving the original project that initiated the build.
+     *
+     * @param project the potentially forked project
+     * @return an Optional containing the original project if the input is a forked project,
+     *         or an empty Optional if the input is already the original project
+     * @throws IllegalArgumentException if the project is null
+     */
     @Nonnull
     Optional<Project> getExecutionProject(@Nonnull Project project);
 }

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

@@ -43,6 +43,7 @@
 import org.apache.maven.api.di.SessionScoped;
 import org.apache.maven.api.services.ArtifactManager;
 import org.apache.maven.api.services.ProjectManager;
+import org.apache.maven.impl.InternalSession;
 import org.apache.maven.impl.MappedList;
 import org.apache.maven.impl.PropertiesAsMap;
 import org.apache.maven.project.MavenProject;
@@ -67,34 +68,35 @@ public DefaultProjectManager(InternalMavenSession session, ArtifactManager artif
 
     @Nonnull
     @Override
-    public Optional<Path> getPath(Project project) {
+    public Optional<Path> getPath(@Nonnull Project project) {
+        nonNull(project, "project");
         Optional<ProducedArtifact> mainArtifact = project.getMainArtifact();
-        if (mainArtifact.isPresent()) {
-            return artifactManager.getPath(mainArtifact.get());
-        }
-        return Optional.empty();
+        return mainArtifact.flatMap(artifactManager::getPath);
     }
 
     @Nonnull
     @Override
-    public Collection<ProducedArtifact> getAttachedArtifacts(Project project) {
-        InternalMavenSession session = ((DefaultProject) project).getSession();
-        Collection<ProducedArtifact> attached = map(
-                getMavenProject(project).getAttachedArtifacts(),
-                a -> session.getArtifact(ProducedArtifact.class, RepositoryUtils.toArtifact(a)));
+    public Collection<ProducedArtifact> getAttachedArtifacts(@Nonnull Project project) {
+        nonNull(project, "project");
+        Collection<ProducedArtifact> attached =
+                map(getMavenProject(project).getAttachedArtifacts(), a -> getSession(project)
+                        .getArtifact(ProducedArtifact.class, RepositoryUtils.toArtifact(a)));
         return Collections.unmodifiableCollection(attached);
     }
 
     @Override
-    public Collection<ProducedArtifact> getAllArtifacts(Project project) {
+    @Nonnull
+    public Collection<ProducedArtifact> getAllArtifacts(@Nonnull Project project) {
+        nonNull(project, "project");
         ArrayList<ProducedArtifact> result = new ArrayList<>(2);
         result.addAll(project.getArtifacts());
         result.addAll(getAttachedArtifacts(project));
         return Collections.unmodifiableCollection(result);
     }
 
     @Override
-    public void attachArtifact(Project project, ProducedArtifact artifact, Path path) {
+    @SuppressWarnings("deprecation")
+    public void attachArtifact(@Nonnull Project project, @Nonnull ProducedArtifact artifact, @Nonnull Path path) {
         nonNull(project, "project");
         nonNull(artifact, "artifact");
         nonNull(path, "path");
@@ -122,8 +124,8 @@ public void attachArtifact(Project project, ProducedArtifact artifact, Path path
                             + artifact.getBaseVersion());
         }
         getMavenProject(project)
-                .addAttachedArtifact(RepositoryUtils.toArtifact(
-                        ((DefaultProject) project).getSession().toArtifact(artifact)));
+                .addAttachedArtifact(
+                        RepositoryUtils.toArtifact(getSession(project).toArtifact(artifact)));
         artifactManager.setPath(artifact, path);
     }
 
@@ -158,19 +160,21 @@ public void addSourceRoot(
     }
 
     @Override
-    public List<RemoteRepository> getRemoteProjectRepositories(Project project) {
+    @Nonnull
+    public List<RemoteRepository> getRemoteProjectRepositories(@Nonnull Project project) {
         return Collections.unmodifiableList(new MappedList<>(
-                ((DefaultProject) project).getProject().getRemoteProjectRepositories(), session::getRemoteRepository));
+                getMavenProject(project).getRemoteProjectRepositories(), session::getRemoteRepository));
     }
 
     @Override
-    public List<RemoteRepository> getRemotePluginRepositories(Project project) {
-        return Collections.unmodifiableList(new MappedList<>(
-                ((DefaultProject) project).getProject().getRemotePluginRepositories(), session::getRemoteRepository));
+    @Nonnull
+    public List<RemoteRepository> getRemotePluginRepositories(@Nonnull Project project) {
+        return Collections.unmodifiableList(
+                new MappedList<>(getMavenProject(project).getRemotePluginRepositories(), session::getRemoteRepository));
     }
 
     @Override
-    public void setProperty(Project project, String key, String value) {
+    public void setProperty(@Nonnull Project project, @Nonnull String key, String value) {
         Properties properties = getMavenProject(project).getProperties();
         if (value == null) {
             properties.remove(key);
@@ -180,13 +184,15 @@ public void setProperty(Project project, String key, String value) {
     }
 
     @Override
-    public Map<String, String> getProperties(Project project) {
+    @Nonnull
+    public Map<String, String> getProperties(@Nonnull Project project) {
         return Collections.unmodifiableMap(
-                new PropertiesAsMap(((DefaultProject) project).getProject().getProperties()));
+                new PropertiesAsMap(getMavenProject(project).getProperties()));
     }
 
     @Override
-    public Optional<Project> getExecutionProject(Project project) {
+    @Nonnull
+    public Optional<Project> getExecutionProject(@Nonnull Project project) {
         // Session keep tracks of the Project per project id,
         // so we cannot use session.getProject(p) for forked projects
         // which are temporary clones
@@ -197,4 +203,8 @@ public Optional<Project> getExecutionProject(Project project) {
     private MavenProject getMavenProject(Project project) {
         return ((DefaultProject) project).getProject();
     }
+
+    private static InternalSession getSession(Project project) {
+        return ((DefaultProject) project).getSession();
+    }
 }