[MNG-7095] Fix resume for parallel builds

Resolves #444

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

@@ -20,9 +20,6 @@
  */
 
 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}.
@@ -30,38 +27,22 @@
 public class BuildResumptionData
 {
     /**
-     * The project where the next build could resume from.
+     * The list of projects that remain to be built.
      */
-    private final String resumeFrom;
+    private final List<String> remainingProjects;
 
-    /**
+    public BuildResumptionData ( final List<String> remainingProjects )
-     * List of projects to skip.
-     */
-    private final List<String> projectsToSkip;
-
-    public BuildResumptionData ( final String resumeFrom, final List<String> projectsToSkip )
     {
-        this.resumeFrom = resumeFrom;
+        this.remainingProjects = remainingProjects;
-        this.projectsToSkip = projectsToSkip;
     }
 
     /**
-     * Returns the project where the next build can resume from.
+     * Returns the projects that still need to be built when resuming.
-     * This is usually the first failed project in the order of the reactor.
+     * @return A list containing the group and artifact id of the projects.
-     * @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()
+    public List<String> getRemainingProjects()
     {
-        return Optional.ofNullable( this.resumeFrom );
+        return this.remainingProjects;
     }
 
-    /**
-     * 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 ( projectsToSkip != null ) ? projectsToSkip : emptyList();
-    }
 }

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

@@ -19,22 +19,16 @@
  * under the License.
  */
 
-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;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
-import static java.util.Comparator.comparing;
-
 /**
  * Default implementation of {@link BuildResumptionAnalyzer}.
  */
@@ -47,132 +41,34 @@ public class DefaultBuildResumptionAnalyzer implements BuildResumptionAnalyzer
     @Override
     public Optional<BuildResumptionData> determineBuildResumptionData( final MavenExecutionResult result )
     {
-        final List<MavenProject> failedProjects = getFailedProjectsInOrder( result );
+        if ( !result.hasExceptions() )
-
-        if ( failedProjects.isEmpty() )
-        {
-            LOGGER.info( "No failed projects found, resuming the build would not make sense." );
-            return Optional.empty();
-        }
-
-        final MavenProject resumeFromProject = failedProjects.get( 0 );
-
-        final String resumeFromSelector;
-        final List<String> projectsToSkip;
-        if ( isFailedProjectFirstInBuild( result, resumeFromProject ) )
-        {
-            // 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 );
-        }
-
-        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 )
-    {
-        final List<MavenProject> sortedProjects = result.getTopologicallySortedProjects();
-        return sortedProjects.indexOf( failedProject ) == 0;
-    }
-
-    private List<MavenProject> getFailedProjectsInOrder( MavenExecutionResult result )
-    {
         List<MavenProject> sortedProjects = result.getTopologicallySortedProjects();
 
-        return result.getExceptions().stream()
+        boolean hasNoSuccess = sortedProjects.stream()
-                .filter( LifecycleExecutionException.class::isInstance )
+                .noneMatch( project -> result.getBuildSummary( project ) instanceof BuildSuccess );
-                .map( LifecycleExecutionException.class::cast )
-                .map( LifecycleExecutionException::getProject )
-                .filter( Objects::nonNull )
-                .sorted( comparing( sortedProjects::indexOf ) )
-                .collect( Collectors.toList() );
-    }
 
-    /**
+        if ( hasNoSuccess )
-     * Projects after the first failed project could have succeeded by using -T or --fail-at-end.
+        {
-     * These projects can be skipped from later builds.
+            return Optional.empty();
-     * 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 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,
-                                                  int startFromProjectIndex )
-    {
-        List<MavenProject> allProjects = result.getTopologicallySortedProjects();
-        List<MavenProject> remainingProjects = allProjects.subList( startFromProjectIndex, allProjects.size() );
 
-        List<GroupArtifactPair> failedProjectsGAList = failedProjects.stream()
+        List<String> remainingProjects = sortedProjects.stream()
-                .map( GroupArtifactPair::new )
+                .filter( project -> result.getBuildSummary( project ) == null
-                .collect( Collectors.toList() );
+                        || result.getBuildSummary( project ) instanceof BuildFailure )
-
-        return remainingProjects.stream()
-                .filter( project -> result.getBuildSummary( project ) instanceof BuildSuccess )
-                .filter( project -> hasNoDependencyOnProjects( project, failedProjectsGAList ) )
                 .map( project -> project.getGroupId() + ":" + project.getArtifactId() )
                 .collect( Collectors.toList() );
+
+        if ( remainingProjects.isEmpty() )
+        {
+            LOGGER.info( "No remaining projects found, resuming the build would not make sense." );
+            return Optional.empty();
+        }
+
+        return Optional.of( new BuildResumptionData( remainingProjects ) );
     }
 
-    private boolean hasNoDependencyOnProjects( MavenProject project, List<GroupArtifactPair> projectsGAs )
-    {
-        return project.getDependencies().stream()
-                .map( GroupArtifactPair::new )
-                .noneMatch( projectsGAs::contains );
-    }
-
-    private static class GroupArtifactPair
-    {
-        private final String groupId;
-        private final String artifactId;
-
-        GroupArtifactPair( MavenProject project )
-        {
-            this.groupId = project.getGroupId();
-            this.artifactId = project.getArtifactId();
-        }
-
-        GroupArtifactPair( Dependency dependency )
-        {
-            this.groupId = dependency.getGroupId();
-            this.artifactId = dependency.getArtifactId();
-        }
-
-        @Override
-        public boolean equals( Object o )
-        {
-            if ( this == o )
-            {
-                return true;
-            }
-            if ( o == null || getClass() != o.getClass() )
-            {
-                return false;
-            }
-            GroupArtifactPair that = (GroupArtifactPair) o;
-            return Objects.equals( groupId, that.groupId ) && Objects.equals( artifactId, that.artifactId );
-        }
-
-        @Override
-        public int hashCode()
-        {
-            return Objects.hash( groupId, artifactId );
-        }
-    }
 }

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

@@ -32,8 +32,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Arrays;
 import java.util.Properties;
+import java.util.stream.Stream;
 
 /**
  * This implementation of {@link BuildResumptionDataRepository} persists information in a properties file. The file is
@@ -44,8 +44,7 @@
 public class DefaultBuildResumptionDataRepository implements BuildResumptionDataRepository
 {
     private static final String RESUME_PROPERTIES_FILENAME = "resume.properties";
-    private static final String RESUME_FROM_PROPERTY = "resumeFrom";
+    private static final String REMAINING_PROJECTS = "remainingProjects";
-    private static final String EXCLUDED_PROJECTS_PROPERTY = "excludedProjects";
     private static final String PROPERTY_DELIMITER = ", ";
     private static final Logger LOGGER = LoggerFactory.getLogger( DefaultBuildResumptionDataRepository.class );
 
@@ -75,14 +74,8 @@ private Properties convertToProperties( final BuildResumptionData buildResumptio
     {
         Properties properties = new Properties();
 
-        buildResumptionData.getResumeFrom()
+        String value = String.join( PROPERTY_DELIMITER, buildResumptionData.getRemainingProjects() );
-                .ifPresent( resumeFrom -> properties.setProperty( RESUME_FROM_PROPERTY, resumeFrom ) );
+        properties.setProperty( REMAINING_PROJECTS, value );
-
-        if ( !buildResumptionData.getProjectsToSkip().isEmpty() )
-        {
-            String excludedProjects = String.join( PROPERTY_DELIMITER, buildResumptionData.getProjectsToSkip() );
-            properties.setProperty( EXCLUDED_PROJECTS_PROPERTY, excludedProjects );
-        }
 
         return properties;
     }
@@ -133,22 +126,14 @@ private Properties loadResumptionFile( Path rootBuildDirectory )
     // This method is made package-private for testing purposes
     void applyResumptionProperties( MavenExecutionRequest request, Properties properties )
     {
-        if ( properties.containsKey( RESUME_FROM_PROPERTY ) && StringUtils.isEmpty( request.getResumeFrom() ) )
+        if ( properties.containsKey( REMAINING_PROJECTS )
+                && StringUtils.isEmpty( request.getResumeFrom() ) )
         {
-            String propertyValue = properties.getProperty( RESUME_FROM_PROPERTY );
+            String propertyValue = properties.getProperty( REMAINING_PROJECTS );
-            request.setResumeFrom( propertyValue );
+            Stream.of( propertyValue.split( PROPERTY_DELIMITER ) )
+                    .filter( StringUtils::isNotEmpty )
+                    .forEach( request.getSelectedProjects()::add );
             LOGGER.info( "Resuming from {} due to the --resume / -r feature.", propertyValue );
         }
-
-        if ( properties.containsKey( EXCLUDED_PROJECTS_PROPERTY ) )
-        {
-            String propertyValue = properties.getProperty( EXCLUDED_PROJECTS_PROPERTY );
-            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/test/java/org/apache/maven/execution/DefaultBuildResumptionAnalyzerTest.java

@@ -29,9 +29,7 @@
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
-import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.is;
 
 public class DefaultBuildResumptionAnalyzerTest
@@ -55,7 +53,7 @@ public void resumeFromGetsDetermined()
         Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
 
         assertThat( result.isPresent(), is( true ) );
-        assertThat( result.get().getResumeFrom(), is( Optional.of ( "test:B" ) ) );
+        assertThat( result.get().getRemainingProjects(), is( asList ( "test:B" ) ) );
     }
 
     @Test
@@ -81,7 +79,7 @@ public void projectsSucceedingAfterFailedProjectsAreExcluded()
         Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
 
         assertThat( result.isPresent(), is( true ) );
-        assertThat( result.get().getProjectsToSkip(), contains( "test:C" ) );
+        assertThat( result.get().getRemainingProjects(), is( asList( "test:B" ) ) );
     }
 
     @Test
@@ -89,14 +87,14 @@ public void projectsDependingOnFailedProjectsAreNotExcluded()
     {
         MavenProject projectA = createSucceededMavenProject( "A" );
         MavenProject projectB = createFailedMavenProject( "B" );
-        MavenProject projectC = createSucceededMavenProject( "C" );
+        MavenProject projectC = createSkippedMavenProject( "C" );
         projectC.setDependencies( singletonList( toDependency( projectB ) ) );
         executionResult.setTopologicallySortedProjects( asList( projectA, projectB, projectC ) );
 
         Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
 
         assertThat( result.isPresent(), is( true ) );
-        assertThat( result.get().getProjectsToSkip().isEmpty(), is( true ) );
+        assertThat( result.get().getRemainingProjects(), is( asList( "test:B", "test:C" ) ) );
     }
 
     @Test
@@ -111,9 +109,7 @@ public void projectsFailingAfterAnotherFailedProjectAreNotExcluded()
         Optional<BuildResumptionData> result = analyzer.determineBuildResumptionData( executionResult );
 
         assertThat( result.isPresent(), is( true ) );
-        assertThat( result.get().getResumeFrom(), is( Optional.of ( "test:B" ) ) );
+        assertThat( result.get().getRemainingProjects(), is( asList ( "test:B", "test:D" ) ) );
-        assertThat( result.get().getProjectsToSkip(), contains( "test:C" ) );
-        assertThat( result.get().getProjectsToSkip(), not( contains( "test:D" ) ) );
     }
 
     private MavenProject createMavenProject( String artifactId )
@@ -133,6 +129,11 @@ private Dependency toDependency(MavenProject mavenProject )
         return dependency;
     }
 
+    private MavenProject createSkippedMavenProject( String artifactId )
+    {
+        return createMavenProject( artifactId );
+    }
+
     private MavenProject createSucceededMavenProject( String artifactId )
     {
         MavenProject project = createMavenProject( artifactId );

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

@@ -27,6 +27,7 @@
 import java.util.List;
 import java.util.Properties;
 
+import static java.util.Arrays.asList;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.empty;
@@ -41,11 +42,11 @@ public void resumeFromPropertyGetsApplied()
     {
         MavenExecutionRequest request = new DefaultMavenExecutionRequest();
         Properties properties = new Properties();
-        properties.setProperty( "resumeFrom", ":module-a" );
+        properties.setProperty( "remainingProjects", ":module-a" );
 
         repository.applyResumptionProperties( request, properties );
 
-        assertThat( request.getResumeFrom(), is( ":module-a" ) );
+        assertThat( request.getSelectedProjects(), is( asList( ":module-a" ) ) );
     }
 
     @Test
@@ -54,7 +55,7 @@ public void resumeFromPropertyDoesNotOverrideExistingRequestParameters()
         MavenExecutionRequest request = new DefaultMavenExecutionRequest();
         request.setResumeFrom( ":module-b" );
         Properties properties = new Properties();
-        properties.setProperty( "resumeFrom", ":module-a" );
+        properties.setProperty( "remainingProjects", ":module-a" );
 
         repository.applyResumptionProperties( request, properties );
 
@@ -62,30 +63,30 @@ public void resumeFromPropertyDoesNotOverrideExistingRequestParameters()
     }
 
     @Test
-    public void excludedProjectsFromPropertyGetsAddedToExistingRequestParameters()
+    public void projectsFromPropertyGetsAddedToExistingRequestParameters()
     {
         MavenExecutionRequest request = new DefaultMavenExecutionRequest();
-        List<String> excludedProjects = new ArrayList<>();
+        List<String> selectedProjects = new ArrayList<>();
-        excludedProjects.add( ":module-a" );
+        selectedProjects.add( ":module-a" );
-        request.setExcludedProjects( excludedProjects );
+        request.setSelectedProjects( selectedProjects );
         Properties properties = new Properties();
-        properties.setProperty( "excludedProjects", ":module-b, :module-c" );
+        properties.setProperty( "remainingProjects", ":module-b, :module-c" );
 
         repository.applyResumptionProperties( request, properties );
 
-        assertThat( request.getExcludedProjects(), contains( ":module-a", ":module-b", ":module-c" ) );
+        assertThat( request.getSelectedProjects(), contains( ":module-a", ":module-b", ":module-c" ) );
     }
 
     @Test
-    public void excludedProjectsAreNotAddedWhenPropertyValueIsEmpty()
+    public void selectedProjectsAreNotAddedWhenPropertyValueIsEmpty()
     {
         MavenExecutionRequest request = new DefaultMavenExecutionRequest();
         Properties properties = new Properties();
-        properties.setProperty( "excludedProjects", "" );
+        properties.setProperty( "remainingProjects", "" );
 
         repository.applyResumptionProperties( request, properties );
 
-        assertThat( request.getExcludedProjects(), is( empty() ) );
+        assertThat( request.getSelectedProjects(), is( empty() ) );
     }
 
     @Test
@@ -99,7 +100,6 @@ public void applyResumptionData_shouldLoadData()
 
         repository.applyResumptionData( request,  rootProject );
 
-        assertThat( request.getResumeFrom(), is( "example:module-c" ) );
+        assertThat( request.getSelectedProjects(), contains( "example:module-c" ) );
-        assertThat( request.getExcludedProjects(), contains( "example:module-a", "example:module-b" ) );
     }
 }

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

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