From 657f9e3f78c1a6c1f0ad2f9e0c458fb29fb8d032 Mon Sep 17 00:00:00 2001 From: jarmoniuk <1554729+jarmoniuk@users.noreply.github.com> Date: Wed, 21 Feb 2024 20:15:41 +0100 Subject: [PATCH] Resolves #1042: Fixed set logic wrt processAllModules (#1051) --- .../org/codehaus/mojo/versions/SetMojo.java | 119 +++++++++--------- .../codehaus/mojo/versions/SetMojoTest.java | 73 +++++++++++ .../mojo/set/issue-1042/child-webapp/pom.xml | 15 +++ .../main-reactor/package-parent/pom.xml | 15 +++ .../main-reactor/pom-parent/pom.xml | 8 ++ .../mojo/set/issue-1042/main-reactor/pom.xml | 13 ++ .../org/codehaus/mojo/set/issue-1042/pom.xml | 35 ++++++ 7 files changed, 220 insertions(+), 58 deletions(-) create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/child-webapp/pom.xml create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/package-parent/pom.xml create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom-parent/pom.xml create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom.xml create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/pom.xml diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java index 93f56e271..6af235e9f 100644 --- a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java +++ b/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 reactorModels = PomHelper.getChildModels(project, getLog()); final SortedMap reactor = new TreeMap<>(new ReactorDepthComparator(reactorModels)); reactor.putAll(reactorModels); + final Map, Set> children = computeChildren(reactorModels); // set of files to update final Set 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, Set> computeChildren(Map 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() { + { + 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 reactor, + Map, Set> children, Set 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 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 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 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); + } + })); } } diff --git a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java index 4ffcdf081..9bebfd58f 100644 --- a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java +++ b/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("testing")); } + + @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( + ".*\\Qchild-reactor\\E\\s*" + "\\Q1.0\\E.*")); + assertThat( + String.join("", Files.readAllLines(tempDir.resolve("child-webapp/pom.xml"))), + matchesRegex(".*\\Qchild-webapp\\E\\s*" + "\\Q1.0\\E.*")); + } + + @Test + public void testComputeChildren() { + Map, Set> children = SetMojo.computeChildren(new HashMap() { + { + 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())); + } } diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/child-webapp/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/child-webapp/pom.xml new file mode 100644 index 000000000..96f8596ec --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/child-webapp/pom.xml @@ -0,0 +1,15 @@ + + 4.0.0 + + default + package-parent + 1.0 + ../main-reactor/package-parent/pom.xml + + + child-webapp + 1.0-SNAPSHOT + pom + + diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/package-parent/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/package-parent/pom.xml new file mode 100644 index 000000000..7729033df --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/package-parent/pom.xml @@ -0,0 +1,15 @@ + + 4.0.0 + + + default + pom-parent + 1.0 + ../pom-parent/pom.xml + + + package-parent + 1.0 + pom + diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom-parent/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom-parent/pom.xml new file mode 100644 index 000000000..4c2ab8876 --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom-parent/pom.xml @@ -0,0 +1,8 @@ + + 4.0.0 + default + pom-parent + 1.0 + pom + diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom.xml new file mode 100644 index 000000000..9229da66c --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom.xml @@ -0,0 +1,13 @@ + + 4.0.0 + default + main-reactor + 1.0 + pom + + + pom-parent + package-parent + + diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/pom.xml new file mode 100644 index 000000000..21efc2e58 --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/pom.xml @@ -0,0 +1,35 @@ + + 4.0.0 + + default + pom-parent + 1.0 + main-reactor/pom-parent/pom.xml + + + child-reactor + 1.0-SNAPSHOT + pom + + + child-webapp + + + + + + org.codehaus.mojo + versions-maven-plugin + + set + + + 1.0 + false + true + + + + +