Resolves #1042: Fixed set logic wrt processAllModules

versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java

@@ -25,20 +25,24 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TimeZone;
 import java.util.TreeMap;
 import java.util.regex.Pattern;
 
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.maven.artifact.ArtifactUtils;
 import org.apache.maven.model.Model;
-import org.apache.maven.model.Parent;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugins.annotations.Mojo;
@@ -334,6 +338,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
             Map<File, Model> reactorModels = PomHelper.getChildModels(project, getLog());
             final SortedMap<File, Model> reactor = new TreeMap<>(new ReactorDepthComparator(reactorModels));
             reactor.putAll(reactorModels);
+            final Map<Pair<String, String>, Set<Model>> children = computeChildren(reactorModels);
 
             // set of files to update
             final Set<File> files = new LinkedHashSet<>();
@@ -379,8 +384,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                                 || oldVersionIdRegex.matcher(mVersion).matches())
                         && !newVersion.equals(mVersion)) {
                     applyChange(
-                            project,
                             reactor,
+                            children,
                             files,
                             mGroupId,
                             m.getArtifactId(),
@@ -407,6 +412,34 @@ public void execute() throws MojoExecutionException, MojoFailureException {
         }
     }
 
+    static Map<Pair<String, String>, Set<Model>> computeChildren(Map<File, Model> reactor) {
+        return reactor.values().stream()
+                .filter(v -> v.getParent() != null)
+                .reduce(
+                        new HashMap<>(),
+                        (map, child) -> {
+                            map.compute(
+                                    new ImmutablePair<>(
+                                            child.getParent().getGroupId(),
+                                            child.getParent().getArtifactId()),
+                                    (pair, set) -> Optional.ofNullable(set)
+                                            .map(children -> {
+                                                children.add(child);
+                                                return children;
+                                            })
+                                            .orElse(new HashSet<Model>() {
+                                                {
+                                                    add(child);
+                                                }
+                                            }));
+                            return map;
+                        },
+                        (m1, m2) -> {
+                            m1.putAll(m2);
+                            return m1;
+                        });
+    }
+
     /**
      * Returns the incremented version, with the nextSnapshotIndexToIncrement indicating the 1-based index,
      * from the left, or the most major version component, of the version string.
@@ -436,28 +469,21 @@ protected String getIncrementedVersion(String version, Integer nextSnapshotIndex
         return StringUtils.join(numbers.toArray(new String[0]), ".") + "-SNAPSHOT";
     }
 
-    private static String fixNullOrEmpty(String value, String defaultValue) {
-        return StringUtils.isBlank(value) ? defaultValue : value;
-    }
-
     private void applyChange(
-            MavenProject project,
             SortedMap<File, Model> reactor,
+            Map<Pair<String, String>, Set<Model>> children,
             Set<File> files,
             String groupId,
             String artifactId,
             String oldVersion) {
 
         getLog().debug("Applying change " + groupId + ":" + artifactId + ":" + oldVersion + " -> " + newVersion);
-        // this is a triggering change
         addChange(groupId, artifactId, oldVersion, newVersion);
-        // now fake out the triggering change
 
-        Map.Entry<File, Model> current = PomHelper.getModelEntry(reactor, groupId, artifactId);
-        if (current != null) {
-            current.getValue().setVersion(newVersion);
-            files.add(current.getValue().getPomFile());
-        }
+        Optional.ofNullable(PomHelper.getModelEntry(reactor, groupId, artifactId))
+                .map(Map.Entry::getValue)
+                .map(Model::getPomFile)
+                .ifPresent(files::add);
 
         for (Map.Entry<File, Model> sourceEntry : reactor.entrySet()) {
             final File sourcePath = sourceEntry.getKey();
@@ -488,50 +514,27 @@ private void applyChange(
 
             getLog().debug("Looking for modules which use "
                     + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent");
-
-            for (Map.Entry<File, Model> stringModelEntry : processAllModules
-                    ? reactor.entrySet()
-                    : PomHelper.getChildModels(reactor, sourceGroupId, sourceArtifactId)
-                            .entrySet()) {
-                final Model targetModel = stringModelEntry.getValue();
-                final Parent parent = targetModel.getParent();
-                getLog().debug("Module: " + stringModelEntry.getKey());
-                if (parent != null && sourceVersion.equals(parent.getVersion())) {
-                    getLog().debug("    parent already is "
-                            + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + ":" + sourceVersion);
-                } else {
-                    getLog().debug("    parent is " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId)
-                            + ":" + (parent == null ? "" : parent.getVersion()));
-                    getLog().debug("    will become " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId)
-                            + ":" + sourceVersion);
-                }
-                final boolean targetExplicit = PomHelper.isExplicitVersion(targetModel);
-                if ((updateMatchingVersions || !targetExplicit) //
-                        && (parent != null
-                                && StringUtils.equals(parent.getVersion(), PomHelper.getVersion(targetModel)))) {
-                    getLog().debug("    module is "
-                            + ArtifactUtils.versionlessKey(
-                                    PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel))
-                            + ":"
-                            + PomHelper.getVersion(targetModel));
-                    getLog().debug("    will become "
-                            + ArtifactUtils.versionlessKey(
-                                    PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel))
-                            + ":" + sourceVersion);
-                    addChange(
-                            PomHelper.getGroupId(targetModel),
-                            PomHelper.getArtifactId(targetModel),
-                            PomHelper.getVersion(targetModel),
-                            sourceVersion);
-                    targetModel.setVersion(sourceVersion);
-                } else {
-                    getLog().debug("    module is "
-                            + ArtifactUtils.versionlessKey(
-                                    PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel))
-                            + ":"
-                            + PomHelper.getVersion(targetModel));
-                }
-            }
+            Optional.ofNullable(children.get(new ImmutablePair<>(sourceGroupId, sourceArtifactId)))
+                    .ifPresent(set -> set.forEach(model -> {
+                        final boolean hasExplicitVersion = PomHelper.isExplicitVersion(model);
+                        if ((updateMatchingVersions || !hasExplicitVersion)
+                                && (StringUtils.equals(sourceVersion, PomHelper.getVersion(model)))) {
+                            getLog().debug("    module is "
+                                    + ArtifactUtils.versionlessKey(
+                                            PomHelper.getGroupId(model), PomHelper.getArtifactId(model))
+                                    + ":"
+                                    + PomHelper.getVersion(model));
+                            getLog().debug("    will become "
+                                    + ArtifactUtils.versionlessKey(
+                                            PomHelper.getGroupId(model), PomHelper.getArtifactId(model))
+                                    + ":" + newVersion);
+                            addChange(
+                                    PomHelper.getGroupId(model),
+                                    PomHelper.getArtifactId(model),
+                                    PomHelper.getVersion(model),
+                                    newVersion);
+                        }
+                    }));
         }
     }
 

versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java

@@ -5,10 +5,16 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
 
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.maven.model.Model;
+import org.apache.maven.model.Parent;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugin.testing.AbstractMojoTestCase;
@@ -23,8 +29,12 @@
 import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.matchesPattern;
+import static org.hamcrest.Matchers.matchesRegex;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
 
 public class SetMojoTest extends AbstractMojoTestCase {
     @Rule
@@ -212,4 +222,67 @@ public void testParentWithProperty() throws Exception {
                 String.join("", Files.readAllLines(tempDir.resolve("child/pom.xml"))),
                 containsString("<version>testing</version>"));
     }
+
+    @Test
+    public void testIssue1042() throws Exception {
+        TestUtils.copyDir(Paths.get("src/test/resources/org/codehaus/mojo/set/issue-1042"), tempDir);
+        SetMojo mojo = (SetMojo) mojoRule.lookupConfiguredMojo(tempDir.toFile(), "set");
+        mojo.execute();
+        assertThat(
+                String.join("", Files.readAllLines(tempDir.resolve("pom.xml"))),
+                matchesPattern(
+                        ".*\\Q<artifactId>child-reactor</artifactId>\\E\\s*" + "\\Q<version>1.0</version>\\E.*"));
+        assertThat(
+                String.join("", Files.readAllLines(tempDir.resolve("child-webapp/pom.xml"))),
+                matchesRegex(".*\\Q<artifactId>child-webapp</artifactId>\\E\\s*" + "\\Q<version>1.0</version>\\E.*"));
+    }
+
+    @Test
+    public void testComputeChildren() {
+        Map<Pair<String, String>, Set<Model>> children = SetMojo.computeChildren(new HashMap<File, Model>() {
+            {
+                put(new File("parent"), new Model() {
+                    {
+                        setArtifactId("parent");
+                        setParent(new Parent() {
+                            {
+                                setGroupId("default");
+                                setArtifactId("superParent");
+                            }
+                        });
+                    }
+                });
+                put(new File("child2"), new Model() {
+                    {
+                        setArtifactId("child2");
+                        setParent(new Parent() {
+                            {
+                                setGroupId("default");
+                                setArtifactId("superParent");
+                            }
+                        });
+                    }
+                });
+                put(new File("superParent"), new Model() {
+                    {
+                        setArtifactId("superParent");
+                    }
+                });
+                put(new File("child"), new Model() {
+                    {
+                        setArtifactId("child");
+                        setParent(new Parent() {
+                            {
+                                setGroupId("default");
+                                setArtifactId("parent");
+                            }
+                        });
+                    }
+                });
+            }
+        });
+        assertThat(children.get(new ImmutablePair<>("default", "superParent")), hasSize(2));
+        assertThat(children.get(new ImmutablePair<>("default", "parent")), hasSize(1));
+        assertThat(children.get(new ImmutablePair<>("default", "child")), is(nullValue()));
+    }
 }

versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/child-webapp/pom.xml

@@ -0,0 +1,15 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>default</groupId>
+        <artifactId>package-parent</artifactId>
+        <version>1.0</version>
+        <relativePath>../main-reactor/package-parent/pom.xml</relativePath>
+    </parent>
+
+    <artifactId>child-webapp</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+</project>

versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/package-parent/pom.xml

@@ -0,0 +1,15 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>default</groupId>
+        <artifactId>pom-parent</artifactId>
+        <version>1.0</version>
+        <relativePath>../pom-parent/pom.xml</relativePath>
+    </parent>
+
+    <artifactId>package-parent</artifactId>
+    <version>1.0</version>
+    <packaging>pom</packaging>
+</project>

versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom-parent/pom.xml

@@ -0,0 +1,8 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>default</groupId>
+    <artifactId>pom-parent</artifactId>
+    <version>1.0</version>
+    <packaging>pom</packaging>
+</project>

versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom.xml

@@ -0,0 +1,13 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>default</groupId>
+    <artifactId>main-reactor</artifactId>
+    <version>1.0</version>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>pom-parent</module>
+        <module>package-parent</module>
+    </modules>
+</project>

versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/pom.xml

@@ -0,0 +1,35 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>default</groupId>
+        <artifactId>pom-parent</artifactId>
+        <version>1.0</version>
+        <relativePath>main-reactor/pom-parent/pom.xml</relativePath>
+    </parent>
+
+    <artifactId>child-reactor</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>child-webapp</module>
+    </modules>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>versions-maven-plugin</artifactId>
+                <goals>
+                    <goal>set</goal>
+                </goals>
+                <configuration>
+                    <newVersion>1.0</newVersion>
+                    <generateBackupPoms>false</generateBackupPoms>
+                    <processAllModules>true</processAllModules>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+</project>