[MNG-5760] Several fixes for the --resume feature.

BuildResumptionDataRepository is not used in MavenCli

Make setResume() on MavenExecutionRequest a traditional setter

Fix resolution of resume.properties file

Add unit test for DefaultBuildResumptionDataRepository#applyResumptionData

Avoid storing and using an empty excludedProjects field in the resume.properties file.

Avoid star imports

Don't create a unneeded Path when resolving resume.properties

Support the scenario where the first project was failed, but subsequent projects succeeded. (e.g. by fail-at-end or parallel builds)

Maven invocations without project shouldn't fail

maven-core/src/main/java/org/apache/maven/execution/BuildResumptionData.java

@@ -20,6 +20,9 @@
  */
 
 import java.util.List;
+import java.util.Optional;
+
+import static java.util.Collections.emptyList;
 
 /**
  * This class holds the information required to enable resuming a Maven build with {@code --resume}.
@@ -32,7 +35,7 @@ public class BuildResumptionData
     private final String resumeFrom;
 
     /**
-     * List of projects to skip if the build would be resumed from {@link #resumeFrom}.
+     * List of projects to skip.
      */
     private final List<String> projectsToSkip;
 
@@ -42,13 +45,23 @@ public BuildResumptionData ( final String resumeFrom, final List<String> project
         this.projectsToSkip = projectsToSkip;
     }
 
-    public String getResumeFrom()
+    /**
+     * Returns the project where the next build can resume from.
+     * This is usually the first failed project in the order of the reactor.
+     * @return An optional containing the group and artifact id of the project. It does not make sense to resume
+     *   the build when the first project of the reactor has failed, so then it will return an empty optional.
+     */
+    public Optional<String> getResumeFrom()
     {
-        return this.resumeFrom;
+        return Optional.ofNullable( this.resumeFrom );
     }
 
+    /**
+     * A list of projects which can be skipped in the next build.
+     * @return A list of group and artifact ids. Can be empty when no projects can be skipped.
+     */
     public List<String> getProjectsToSkip()
     {
-        return this.projectsToSkip;
+        return ( projectsToSkip != null ) ? projectsToSkip : emptyList();
     }
 }

maven-core/src/main/java/org/apache/maven/execution/DefaultBuildResumptionAnalyzer.java

@@ -22,6 +22,7 @@
 import org.apache.maven.lifecycle.LifecycleExecutionException;
 import org.apache.maven.model.Dependency;
 import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.util.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -56,16 +57,31 @@ public Optional<BuildResumptionData> determineBuildResumptionData( final MavenEx
 
         final MavenProject resumeFromProject = failedProjects.get( 0 );
 
+        final String resumeFromSelector;
+        final List<String> projectsToSkip;
         if ( isFailedProjectFirstInBuild( result, resumeFromProject ) )
         {
-            LOGGER.info( "The first module in the build failed, resuming the build would not make sense." );
-            return Optional.empty();
+            // As the first module in the build failed, there is no need to specify this as the resumeFrom project.
+            resumeFromSelector = null;
+            projectsToSkip = determineProjectsToSkip( result, failedProjects, 0 );
+        }
+        else
+        {
+            resumeFromSelector = resumeFromProject.getGroupId() + ":" + resumeFromProject.getArtifactId();
+            List<MavenProject> allProjects = result.getTopologicallySortedProjects();
+            int resumeFromProjectIndex = allProjects.indexOf( resumeFromProject );
+            projectsToSkip = determineProjectsToSkip( result, failedProjects, resumeFromProjectIndex + 1 );
         }
 
-        final String resumeFromSelector = resumeFromProject.getGroupId() + ":" + resumeFromProject.getArtifactId();
-        final List<String> projectsToSkip = determineProjectsToSkip( result, failedProjects, resumeFromProject );
-
-        return Optional.of( new BuildResumptionData( resumeFromSelector, projectsToSkip ) );
+        boolean canBuildBeResumed = StringUtils.isNotEmpty( resumeFromSelector ) || !projectsToSkip.isEmpty();
+        if ( canBuildBeResumed )
+        {
+            return Optional.of( new BuildResumptionData( resumeFromSelector, projectsToSkip ) );
+        }
+        else
+        {
+            return Optional.empty();
+        }
     }
 
     private boolean isFailedProjectFirstInBuild( final MavenExecutionResult result, final MavenProject failedProject )
@@ -82,6 +98,7 @@ private List<MavenProject> getFailedProjectsInOrder( MavenExecutionResult result
                 .filter( LifecycleExecutionException.class::isInstance )
                 .map( LifecycleExecutionException.class::cast )
                 .map( LifecycleExecutionException::getProject )
+                .filter( Objects::nonNull )
                 .sorted( comparing( sortedProjects::indexOf ) )
                 .collect( Collectors.toList() );
     }
@@ -92,16 +109,15 @@ private List<MavenProject> getFailedProjectsInOrder( MavenExecutionResult result
      * This is not the case these projects are dependent on one of the failed projects.
      * @param result The result of the Maven build.
      * @param failedProjects The list of failed projects in the build.
-     * @param resumeFromProject The project where the build will be resumed with in the next run.
+     * @param startFromProjectIndex Start looking for projects which can be skipped from a certain index.
      * @return A list of projects which can be skipped in a later build.
      */
     private List<String> determineProjectsToSkip( MavenExecutionResult result,
                                                   List<MavenProject> failedProjects,
-                                                  MavenProject resumeFromProject )
+                                                  int startFromProjectIndex )
     {
         List<MavenProject> allProjects = result.getTopologicallySortedProjects();
-        int resumeFromProjectIndex = allProjects.indexOf( resumeFromProject );
-        List<MavenProject> remainingProjects = allProjects.subList( resumeFromProjectIndex + 1, allProjects.size() );
+        List<MavenProject> remainingProjects = allProjects.subList( startFromProjectIndex, allProjects.size() );
 
         List<GroupArtifactPair> failedProjectsGAList = failedProjects.stream()
                 .map( GroupArtifactPair::new )

maven-core/src/main/java/org/apache/maven/execution/DefaultBuildResumptionDataRepository.java

@@ -74,9 +74,15 @@ public void persistResumptionData( MavenProject rootProject, BuildResumptionData
     private Properties convertToProperties( final BuildResumptionData buildResumptionData )
     {
         Properties properties = new Properties();
-        properties.setProperty( RESUME_FROM_PROPERTY, buildResumptionData.getResumeFrom() );
-        String excludedProjects = String.join( PROPERTY_DELIMITER, buildResumptionData.getProjectsToSkip() );
-        properties.setProperty( EXCLUDED_PROJECTS_PROPERTY, excludedProjects );
+
+        buildResumptionData.getResumeFrom()
+                .ifPresent( resumeFrom -> properties.setProperty( RESUME_FROM_PROPERTY, resumeFrom ) );
+
+        if ( !buildResumptionData.getProjectsToSkip().isEmpty() )
+        {
+            String excludedProjects = String.join( PROPERTY_DELIMITER, buildResumptionData.getProjectsToSkip() );
+            properties.setProperty( EXCLUDED_PROJECTS_PROPERTY, excludedProjects );
+        }
 
         return properties;
     }
@@ -105,7 +111,7 @@ public void removeResumptionData( MavenProject rootProject )
     private Properties loadResumptionFile( Path rootBuildDirectory )
     {
         Properties properties = new Properties();
-        Path path = Paths.get( RESUME_PROPERTIES_FILENAME ).resolve( rootBuildDirectory );
+        Path path = rootBuildDirectory.resolve( RESUME_PROPERTIES_FILENAME );
         if ( !Files.exists( path ) )
         {
             LOGGER.warn( "The {} file does not exist. The --resume / -r feature will not work.", path );
@@ -137,9 +143,12 @@ void applyResumptionProperties( MavenExecutionRequest request, Properties proper
         if ( properties.containsKey( EXCLUDED_PROJECTS_PROPERTY ) )
         {
             String propertyValue = properties.getProperty( EXCLUDED_PROJECTS_PROPERTY );
-            String[] excludedProjects = propertyValue.split( PROPERTY_DELIMITER );
-            request.getExcludedProjects().addAll( Arrays.asList( excludedProjects ) );
-            LOGGER.info( "Additionally excluding projects '{}' due to the --resume / -r feature.", propertyValue );
+            if ( !StringUtils.isEmpty( propertyValue ) )
+            {
+                String[] excludedProjects = propertyValue.split( PROPERTY_DELIMITER );
+                request.getExcludedProjects().addAll( Arrays.asList( excludedProjects ) );
+                LOGGER.info( "Additionally excluding projects '{}' due to the --resume / -r feature.", propertyValue );
+            }
         }
     }
 }

maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java

@@ -607,9 +607,9 @@ public MavenExecutionRequest setExcludedProjects( List<String> excludedProjects
     }
 
     @Override
-    public MavenExecutionRequest setResume()
+    public MavenExecutionRequest setResume( boolean resume )
     {
-        resume = true;
+        this.resume = resume;
 
         return this;
     }

maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java

@@ -173,9 +173,10 @@ public interface MavenExecutionRequest
 
     /**
      * Sets whether the build should be resumed from the data in the resume.properties file.
+     * @param resume Whether or not to resume a previous build.
      * @return This request, never {@code null}.
      */
-    MavenExecutionRequest setResume();
+    MavenExecutionRequest setResume( boolean resume );
 
     /**
      * @return Whether the build should be resumed from the data in the resume.properties file.

maven-core/src/test/java/org/apache/maven/execution/DefaultBuildResumptionAnalyzerTest.java

@@ -55,7 +55,7 @@ public void resumeFromGetsDetermined()
         Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
 
         assertThat( result.isPresent(), is( true ) );
-        assertThat( result.get().getResumeFrom(), is( "test:B" ) );
+        assertThat( result.get().getResumeFrom(), is( Optional.of ( "test:B" ) ) );
     }
 
     @Test
@@ -111,7 +111,7 @@ public void projectsFailingAfterAnotherFailedProjectAreNotExcluded()
         Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
 
         assertThat( result.isPresent(), is( true ) );
-        assertThat( result.get().getResumeFrom(), is( "test:B" ) );
+        assertThat( result.get().getResumeFrom(), is( Optional.of ( "test:B" ) ) );
         assertThat( result.get().getProjectsToSkip(), contains( "test:C" ) );
         assertThat( result.get().getProjectsToSkip(), not( contains( "test:D" ) ) );
     }

maven-core/src/test/java/org/apache/maven/execution/DefaultBuildResumptionDataRepositoryTest.java

@@ -19,6 +19,8 @@
  * under the License.
  */
 
+import org.apache.maven.model.Build;
+import org.apache.maven.project.MavenProject;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.junit.MockitoJUnitRunner;
@@ -29,6 +31,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
 
 @RunWith( MockitoJUnitRunner.class )
@@ -75,4 +78,31 @@ public void excludedProjectsFromPropertyGetsAddedToExistingRequestParameters()
 
         assertThat( request.getExcludedProjects(), contains( ":module-a", ":module-b", ":module-c" ) );
     }
+
+    @Test
+    public void excludedProjectsAreNotAddedWhenPropertyValueIsEmpty()
+    {
+        MavenExecutionRequest request = new DefaultMavenExecutionRequest();
+        Properties properties = new Properties();
+        properties.setProperty( "excludedProjects", "" );
+
+        repository.applyResumptionProperties( request, properties );
+
+        assertThat( request.getExcludedProjects(), is( empty() ) );
+    }
+
+    @Test
+    public void applyResumptionData_shouldLoadData()
+    {
+        MavenExecutionRequest request = new DefaultMavenExecutionRequest();
+        Build build = new Build();
+        build.setDirectory( "src/test/resources/org/apache/maven/execution/" );
+        MavenProject rootProject = new MavenProject();
+        rootProject.setBuild( build );
+
+        repository.applyResumptionData( request,  rootProject );
+
+        assertThat( request.getResumeFrom(), is( "example:module-c" ) );
+        assertThat( request.getExcludedProjects(), contains( "example:module-a", "example:module-b" ) );
+    }
 }

maven-core/src/test/resources/org/apache/maven/execution/resume.properties

@@ -0,0 +1,2 @@
+resumeFrom=example:module-c
+excludedProjects=example:module-a, example:module-b

maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java

@@ -48,7 +48,6 @@
 import org.apache.maven.exception.DefaultExceptionHandler;
 import org.apache.maven.exception.ExceptionHandler;
 import org.apache.maven.exception.ExceptionSummary;
-import org.apache.maven.execution.BuildResumptionDataRepository;
 import org.apache.maven.execution.DefaultMavenExecutionRequest;
 import org.apache.maven.execution.ExecutionListener;
 import org.apache.maven.execution.MavenExecutionRequest;
@@ -169,8 +168,6 @@ public class MavenCli
 
     private Map<String, ConfigurationProcessor> configurationProcessors;
 
-    private BuildResumptionDataRepository buildResumptionDataRepository;
-
     public MavenCli()
     {
         this( null );
@@ -708,8 +705,6 @@ protected void configure()
 
         dispatcher = (DefaultSecDispatcher) container.lookup( SecDispatcher.class, "maven" );
 
-        buildResumptionDataRepository = container.lookup( BuildResumptionDataRepository.class );
-
         return container;
     }
 
@@ -1536,7 +1531,7 @@ else if ( modelProcessor != null )
 
         if ( commandLine.hasOption( CLIManager.RESUME ) )
         {
-            request.setResume();
+            request.setResume( true );
         }
 
         if ( commandLine.hasOption( CLIManager.RESUME_FROM ) )