Revert #1051 - fix #1110, #1103

versions-maven-plugin/src/it/it-set-issue-1103/invoker.properties

@@ -0,0 +1 @@
+invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:set -DnewVersion=2

versions-maven-plugin/src/it/it-set-issue-1103/parent/child/grandchild/pom.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-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/maven-v4_0_0.xsd">
+
+  <!-- ========================================================================
+       Basic project information
+       ======================================================================== -->
+
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>com.company.test</groupId>
+    <artifactId>child</artifactId>
+    <version>100-SNAPSHOT</version>
+  </parent>
+
+  <artifactId>grandchild</artifactId>
+
+</project>

versions-maven-plugin/src/it/it-set-issue-1103/parent/child/pom.xml

@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-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/maven-v4_0_0.xsd">
+
+  <!-- ========================================================================
+       Basic project information
+       ======================================================================== -->
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>com.company.test</groupId>
+    <artifactId>parent</artifactId>
+    <version>100-SNAPSHOT</version>
+  </parent>
+
+  <artifactId>child</artifactId>
+  <packaging>pom</packaging>
+
+  <!-- ========================================================================
+       Multi-Module Build: list of child modules
+       ======================================================================== -->
+  <modules>
+    <module>grandchild</module>
+  </modules>
+
+</project>

versions-maven-plugin/src/it/it-set-issue-1103/parent/pom.xml

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-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/maven-v4_0_0.xsd">
+
+  <!-- ========================================================================
+       Basic project information
+       ======================================================================== -->
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>com.company.test</groupId>
+    <artifactId>grandparent</artifactId>
+    <version>1</version>
+  </parent>
+
+  <artifactId>parent</artifactId>
+  <version>100-SNAPSHOT</version>
+  <packaging>pom</packaging>
+
+  <!-- ========================================================================
+       Multi-Module Build: list of child modules
+       ======================================================================== -->
+  <modules>
+    <module>child</module>
+  </modules>
+
+</project>

versions-maven-plugin/src/it/it-set-issue-1103/pom.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-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/maven-v4_0_0.xsd">
+  <!-- =======================================================================
+    Basic project information
+    ======================================================================== -->
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>com.company.test</groupId>
+  <artifactId>grandparent</artifactId>
+  <version>1</version>
+  <packaging>pom</packaging>
+
+  <url>https://github.com/mojohaus/versions/issues/1103</url>
+
+  <!-- ========================================================================
+    Multi-Module Build: list of child modules
+    ======================================================================== -->
+  <modules>
+    <module>parent</module>
+  </modules>
+
+</project>

versions-maven-plugin/src/it/it-set-issue-1103/verify.groovy

@@ -0,0 +1,14 @@
+import groovy.xml.XmlSlurper
+
+def project = new XmlSlurper().parse(new File(basedir, 'pom.xml'))
+assert project.version == '2'
+
+def parent = new XmlSlurper().parse(new File(basedir, 'parent/pom.xml'))
+assert parent.parent.version == '2'
+assert parent.version == '100-SNAPSHOT'
+
+def child = new XmlSlurper().parse(new File(basedir, 'parent/child/pom.xml'))
+assert child.parent.version == '100-SNAPSHOT'
+
+def grandchild = new XmlSlurper().parse(new File(basedir, 'parent/child/grandchild/pom.xml'))
+assert grandchild.parent.version == '100-SNAPSHOT'

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

@@ -25,26 +25,22 @@
 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.StringUtils;
-import org.apache.commons.lang3.tuple.ImmutablePair;
-import org.apache.commons.lang3.tuple.Pair;
 import org.apache.maven.artifact.ArtifactUtils;
 import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
 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;
@@ -339,7 +335,6 @@ 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<>();
@@ -386,7 +381,6 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                         && !newVersion.equals(mVersion)) {
                     applyChange(
                             reactor,
-                            children,
                             files,
                             mGroupId,
                             m.getArtifactId(),
@@ -413,34 +407,6 @@ 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.
@@ -471,20 +437,20 @@ protected String getIncrementedVersion(String version, Integer nextSnapshotIndex
     }
 
     private void applyChange(
-            SortedMap<File, Model> reactor,
-            Map<Pair<String, String>, Set<Model>> children,
-            Set<File> files,
-            String groupId,
-            String artifactId,
-            String oldVersion) {
+            SortedMap<File, Model> reactor, 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);
+        // TODO there is for in for by reactor - should be refactored
 
         for (Map.Entry<File, Model> sourceEntry : reactor.entrySet()) {
             final File sourcePath = sourceEntry.getKey();
@@ -515,27 +481,56 @@ private void applyChange(
 
             getLog().debug("Looking for modules which use "
                     + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent");
-            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);
-                        }
-                    }));
+
+            for (Map.Entry<File, Model> stringModelEntry : processAllModules
+                    ? reactor.entrySet()
+                    : PomHelper.getChildModels(reactor, sourceGroupId, sourceArtifactId)
+                            .entrySet()) {
+                final Model targetModel = stringModelEntry.getValue();
+
+                if (Objects.equals(PomHelper.getGroupId(targetModel), groupId)
+                        && Objects.equals(PomHelper.getArtifactId(targetModel), artifactId)) {
+                    // skip updating the same pom again
+                    continue;
+                }
+
+                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 && Objects.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));
+                }
+            }
         }
     }
 

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

@@ -5,16 +5,10 @@
 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;
@@ -29,12 +23,10 @@
 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
@@ -239,53 +231,4 @@ public void testIssue1042() throws Exception {
                 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()));
-    }
 }