Skip to content

Commit

Permalink
Resolves #1042: Fixed set logic wrt processAllModules
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrzejj0
andrzejj0 committed Feb 18, 2024
1 parent 71de55d commit dd92032
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 58 deletions.
119 changes: 61 additions & 58 deletions versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<>();
Expand Down Expand Up @@ -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(),
Expand All @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}));
}
}

Expand Down
73 changes: 73 additions & 0 deletions versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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()));
}
}
15 changes: 15 additions & 0 deletions ...ons-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/child-webapp/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -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>
15 changes: 15 additions & 0 deletions ...n/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/package-parent/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -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>
8 changes: 8 additions & 0 deletions ...lugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom-parent/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -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>
13 changes: 13 additions & 0 deletions ...ons-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/main-reactor/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -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>
35 changes: 35 additions & 0 deletions versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1042/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -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>

0 comments on commit dd92032

Please sign in to comment.