Skip to content

Commit df6749f

Browse files
andrzejj0andrzejj0
authored
Resolves #1042: Fixed set logic wrt processAllModules (#1051)
1 parent 8d59ea5 commit df6749f

File tree

7 files changed

+220
-58
lines changed

7 files changed

+220
-58
lines changed

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

Lines changed: 61 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,24 @@
2525
import java.util.ArrayList;
2626
import java.util.Arrays;
2727
import java.util.Date;
28+
import java.util.HashMap;
29+
import java.util.HashSet;
2830
import java.util.LinkedHashSet;
2931
import java.util.LinkedList;
3032
import java.util.List;
3133
import java.util.Map;
3234
import java.util.Objects;
35+
import java.util.Optional;
3336
import java.util.Set;
3437
import java.util.SortedMap;
3538
import java.util.TimeZone;
3639
import java.util.TreeMap;
3740
import java.util.regex.Pattern;
3841

42+
import org.apache.commons.lang3.tuple.ImmutablePair;
43+
import org.apache.commons.lang3.tuple.Pair;
3944
import org.apache.maven.artifact.ArtifactUtils;
4045
import org.apache.maven.model.Model;
41-
import org.apache.maven.model.Parent;
4246
import org.apache.maven.plugin.MojoExecutionException;
4347
import org.apache.maven.plugin.MojoFailureException;
4448
import org.apache.maven.plugins.annotations.Mojo;
@@ -334,6 +338,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
334338
Map<File, Model> reactorModels = PomHelper.getChildModels(project, getLog());
335339
final SortedMap<File, Model> reactor = new TreeMap<>(new ReactorDepthComparator(reactorModels));
336340
reactor.putAll(reactorModels);
341+
final Map<Pair<String, String>, Set<Model>> children = computeChildren(reactorModels);
337342

338343
// set of files to update
339344
final Set<File> files = new LinkedHashSet<>();
@@ -379,8 +384,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
379384
|| oldVersionIdRegex.matcher(mVersion).matches())
380385
&& !newVersion.equals(mVersion)) {
381386
applyChange(
382-
project,
383387
reactor,
388+
children,
384389
files,
385390
mGroupId,
386391
m.getArtifactId(),
@@ -407,6 +412,34 @@ public void execute() throws MojoExecutionException, MojoFailureException {
407412
}
408413
}
409414

415+
static Map<Pair<String, String>, Set<Model>> computeChildren(Map<File, Model> reactor) {
416+
return reactor.values().stream()
417+
.filter(v -> v.getParent() != null)
418+
.reduce(
419+
new HashMap<>(),
420+
(map, child) -> {
421+
map.compute(
422+
new ImmutablePair<>(
423+
child.getParent().getGroupId(),
424+
child.getParent().getArtifactId()),
425+
(pair, set) -> Optional.ofNullable(set)
426+
.map(children -> {
427+
children.add(child);
428+
return children;
429+
})
430+
.orElse(new HashSet<Model>() {
431+
{
432+
add(child);
433+
}
434+
}));
435+
return map;
436+
},
437+
(m1, m2) -> {
438+
m1.putAll(m2);
439+
return m1;
440+
});
441+
}
442+
410443
/**
411444
* Returns the incremented version, with the nextSnapshotIndexToIncrement indicating the 1-based index,
412445
* from the left, or the most major version component, of the version string.
@@ -436,28 +469,21 @@ protected String getIncrementedVersion(String version, Integer nextSnapshotIndex
436469
return StringUtils.join(numbers.toArray(new String[0]), ".") + "-SNAPSHOT";
437470
}
438471

439-
private static String fixNullOrEmpty(String value, String defaultValue) {
440-
return StringUtils.isBlank(value) ? defaultValue : value;
441-
}
442-
443472
private void applyChange(
444-
MavenProject project,
445473
SortedMap<File, Model> reactor,
474+
Map<Pair<String, String>, Set<Model>> children,
446475
Set<File> files,
447476
String groupId,
448477
String artifactId,
449478
String oldVersion) {
450479

451480
getLog().debug("Applying change " + groupId + ":" + artifactId + ":" + oldVersion + " -> " + newVersion);
452-
// this is a triggering change
453481
addChange(groupId, artifactId, oldVersion, newVersion);
454-
// now fake out the triggering change
455482

456-
Map.Entry<File, Model> current = PomHelper.getModelEntry(reactor, groupId, artifactId);
457-
if (current != null) {
458-
current.getValue().setVersion(newVersion);
459-
files.add(current.getValue().getPomFile());
460-
}
483+
Optional.ofNullable(PomHelper.getModelEntry(reactor, groupId, artifactId))
484+
.map(Map.Entry::getValue)
485+
.map(Model::getPomFile)
486+
.ifPresent(files::add);
461487

462488
for (Map.Entry<File, Model> sourceEntry : reactor.entrySet()) {
463489
final File sourcePath = sourceEntry.getKey();
@@ -488,50 +514,27 @@ private void applyChange(
488514

489515
getLog().debug("Looking for modules which use "
490516
+ ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent");
491-
492-
for (Map.Entry<File, Model> stringModelEntry : processAllModules
493-
? reactor.entrySet()
494-
: PomHelper.getChildModels(reactor, sourceGroupId, sourceArtifactId)
495-
.entrySet()) {
496-
final Model targetModel = stringModelEntry.getValue();
497-
final Parent parent = targetModel.getParent();
498-
getLog().debug("Module: " + stringModelEntry.getKey());
499-
if (parent != null && sourceVersion.equals(parent.getVersion())) {
500-
getLog().debug(" parent already is "
501-
+ ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + ":" + sourceVersion);
502-
} else {
503-
getLog().debug(" parent is " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId)
504-
+ ":" + (parent == null ? "" : parent.getVersion()));
505-
getLog().debug(" will become " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId)
506-
+ ":" + sourceVersion);
507-
}
508-
final boolean targetExplicit = PomHelper.isExplicitVersion(targetModel);
509-
if ((updateMatchingVersions || !targetExplicit) //
510-
&& (parent != null
511-
&& StringUtils.equals(parent.getVersion(), PomHelper.getVersion(targetModel)))) {
512-
getLog().debug(" module is "
513-
+ ArtifactUtils.versionlessKey(
514-
PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel))
515-
+ ":"
516-
+ PomHelper.getVersion(targetModel));
517-
getLog().debug(" will become "
518-
+ ArtifactUtils.versionlessKey(
519-
PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel))
520-
+ ":" + sourceVersion);
521-
addChange(
522-
PomHelper.getGroupId(targetModel),
523-
PomHelper.getArtifactId(targetModel),
524-
PomHelper.getVersion(targetModel),
525-
sourceVersion);
526-
targetModel.setVersion(sourceVersion);
527-
} else {
528-
getLog().debug(" module is "
529-
+ ArtifactUtils.versionlessKey(
530-
PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel))
531-
+ ":"
532-
+ PomHelper.getVersion(targetModel));
533-
}
534-
}
517+
Optional.ofNullable(children.get(new ImmutablePair<>(sourceGroupId, sourceArtifactId)))
518+
.ifPresent(set -> set.forEach(model -> {
519+
final boolean hasExplicitVersion = PomHelper.isExplicitVersion(model);
520+
if ((updateMatchingVersions || !hasExplicitVersion)
521+
&& (StringUtils.equals(sourceVersion, PomHelper.getVersion(model)))) {
522+
getLog().debug(" module is "
523+
+ ArtifactUtils.versionlessKey(
524+
PomHelper.getGroupId(model), PomHelper.getArtifactId(model))
525+
+ ":"
526+
+ PomHelper.getVersion(model));
527+
getLog().debug(" will become "
528+
+ ArtifactUtils.versionlessKey(
529+
PomHelper.getGroupId(model), PomHelper.getArtifactId(model))
530+
+ ":" + newVersion);
531+
addChange(
532+
PomHelper.getGroupId(model),
533+
PomHelper.getArtifactId(model),
534+
PomHelper.getVersion(model),
535+
newVersion);
536+
}
537+
}));
535538
}
536539
}
537540

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@
55
import java.nio.file.Files;
66
import java.nio.file.Path;
77
import java.nio.file.Paths;
8+
import java.util.HashMap;
9+
import java.util.Map;
10+
import java.util.Set;
811
import java.util.function.Consumer;
912
import java.util.stream.Stream;
1013

14+
import org.apache.commons.lang3.tuple.ImmutablePair;
15+
import org.apache.commons.lang3.tuple.Pair;
1116
import org.apache.maven.model.Model;
17+
import org.apache.maven.model.Parent;
1218
import org.apache.maven.plugin.MojoExecutionException;
1319
import org.apache.maven.plugin.MojoFailureException;
1420
import org.apache.maven.plugin.testing.AbstractMojoTestCase;
@@ -23,8 +29,12 @@
2329
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
2430
import static org.hamcrest.MatcherAssert.assertThat;
2531
import static org.hamcrest.Matchers.containsString;
32+
import static org.hamcrest.Matchers.hasSize;
2633
import static org.hamcrest.Matchers.is;
34+
import static org.hamcrest.Matchers.matchesPattern;
35+
import static org.hamcrest.Matchers.matchesRegex;
2736
import static org.hamcrest.Matchers.not;
37+
import static org.hamcrest.Matchers.nullValue;
2838

2939
public class SetMojoTest extends AbstractMojoTestCase {
3040
@Rule
@@ -212,4 +222,67 @@ public void testParentWithProperty() throws Exception {
212222
String.join("", Files.readAllLines(tempDir.resolve("child/pom.xml"))),
213223
containsString("<version>testing</version>"));
214224
}
225+
226+
@Test
227+
public void testIssue1042() throws Exception {
228+
TestUtils.copyDir(Paths.get("src/test/resources/org/codehaus/mojo/set/issue-1042"), tempDir);
229+
SetMojo mojo = (SetMojo) mojoRule.lookupConfiguredMojo(tempDir.toFile(), "set");
230+
mojo.execute();
231+
assertThat(
232+
String.join("", Files.readAllLines(tempDir.resolve("pom.xml"))),
233+
matchesPattern(
234+
".*\\Q<artifactId>child-reactor</artifactId>\\E\\s*" + "\\Q<version>1.0</version>\\E.*"));
235+
assertThat(
236+
String.join("", Files.readAllLines(tempDir.resolve("child-webapp/pom.xml"))),
237+
matchesRegex(".*\\Q<artifactId>child-webapp</artifactId>\\E\\s*" + "\\Q<version>1.0</version>\\E.*"));
238+
}
239+
240+
@Test
241+
public void testComputeChildren() {
242+
Map<Pair<String, String>, Set<Model>> children = SetMojo.computeChildren(new HashMap<File, Model>() {
243+
{
244+
put(new File("parent"), new Model() {
245+
{
246+
setArtifactId("parent");
247+
setParent(new Parent() {
248+
{
249+
setGroupId("default");
250+
setArtifactId("superParent");
251+
}
252+
});
253+
}
254+
});
255+
put(new File("child2"), new Model() {
256+
{
257+
setArtifactId("child2");
258+
setParent(new Parent() {
259+
{
260+
setGroupId("default");
261+
setArtifactId("superParent");
262+
}
263+
});
264+
}
265+
});
266+
put(new File("superParent"), new Model() {
267+
{
268+
setArtifactId("superParent");
269+
}
270+
});
271+
put(new File("child"), new Model() {
272+
{
273+
setArtifactId("child");
274+
setParent(new Parent() {
275+
{
276+
setGroupId("default");
277+
setArtifactId("parent");
278+
}
279+
});
280+
}
281+
});
282+
}
283+
});
284+
assertThat(children.get(new ImmutablePair<>("default", "superParent")), hasSize(2));
285+
assertThat(children.get(new ImmutablePair<>("default", "parent")), hasSize(1));
286+
assertThat(children.get(new ImmutablePair<>("default", "child")), is(nullValue()));
287+
}
215288
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
<parent>
5+
<groupId>default</groupId>
6+
<artifactId>package-parent</artifactId>
7+
<version>1.0</version>
8+
<relativePath>../main-reactor/package-parent/pom.xml</relativePath>
9+
</parent>
10+
11+
<artifactId>child-webapp</artifactId>
12+
<version>1.0-SNAPSHOT</version>
13+
<packaging>pom</packaging>
14+
15+
</project>

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
5+
<parent>
6+
<groupId>default</groupId>
7+
<artifactId>pom-parent</artifactId>
8+
<version>1.0</version>
9+
<relativePath>../pom-parent/pom.xml</relativePath>
10+
</parent>
11+
12+
<artifactId>package-parent</artifactId>
13+
<version>1.0</version>
14+
<packaging>pom</packaging>
15+
</project>

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
<groupId>default</groupId>
5+
<artifactId>pom-parent</artifactId>
6+
<version>1.0</version>
7+
<packaging>pom</packaging>
8+
</project>

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
<groupId>default</groupId>
5+
<artifactId>main-reactor</artifactId>
6+
<version>1.0</version>
7+
<packaging>pom</packaging>
8+
9+
<modules>
10+
<module>pom-parent</module>
11+
<module>package-parent</module>
12+
</modules>
13+
</project>

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
2+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
<parent>
5+
<groupId>default</groupId>
6+
<artifactId>pom-parent</artifactId>
7+
<version>1.0</version>
8+
<relativePath>main-reactor/pom-parent/pom.xml</relativePath>
9+
</parent>
10+
11+
<artifactId>child-reactor</artifactId>
12+
<version>1.0-SNAPSHOT</version>
13+
<packaging>pom</packaging>
14+
15+
<modules>
16+
<module>child-webapp</module>
17+
</modules>
18+
19+
<build>
20+
<plugins>
21+
<plugin>
22+
<groupId>org.codehaus.mojo</groupId>
23+
<artifactId>versions-maven-plugin</artifactId>
24+
<goals>
25+
<goal>set</goal>
26+
</goals>
27+
<configuration>
28+
<newVersion>1.0</newVersion>
29+
<generateBackupPoms>false</generateBackupPoms>
30+
<processAllModules>true</processAllModules>
31+
</configuration>
32+
</plugin>
33+
</plugins>
34+
</build>
35+
</project>

0 commit comments

Comments
 (0)