Skip to content

Commit

Permalink
Resolves mojohaus#1042: Optimisation - removed the n^2 visits of the …
Browse files Browse the repository at this point in the history 
…tree.
  • Loading branch information
@jarmoniuk
jarmoniuk committed Feb 17, 2024
1 parent 509d6ab commit b3c643d
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 61 deletions.
108 changes: 47 additions & 61 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,6 +25,8 @@
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;
Expand All @@ -37,9 +39,10 @@
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 @@ -93,16 +96,6 @@ public class SetMojo extends AbstractVersionsUpdaterMojo {
@Parameter(property = "processAllModules", defaultValue = "false")
private boolean processAllModules;

/**
* <p>If set to {@code true}, will look up throughout the whole reactor to match modules
* being updated. By default, the goal only descends into modules' subtrees.</p>
* <p><em>Note.</em> This property was split off {@code processAllModules}.</p>
*
* @since 2.16.3
*/
@Parameter(property = "fullTreeMatch", defaultValue = "false")
private boolean fullTreeMatch;

/**
* <p>The <b>non-interpolated</b> groupId of the dependency/module to be selected for update.</p>
* <p>If not set, will be equal to the non-interpolated groupId of the project file.</p>
Expand Down Expand Up @@ -345,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 @@ -390,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 @@ -418,6 +412,25 @@ public void execute() throws MojoExecutionException, MojoFailureException {
}
}

static Map<Pair<String, String>, Set<Model>> computeChildren(Map<File, Model> reactor) {
Map<Pair<String, String>, Set<Model>> result = new HashMap<>();
for (Map.Entry<File, Model> entry : reactor.entrySet()) {
Optional.ofNullable(entry.getValue().getParent())
.map(p -> new ImmutablePair<>(p.getGroupId(), p.getArtifactId()))
.ifPresent(parent -> result.compute(parent, (f1, s) -> Optional.ofNullable(s)
.map(children -> {
children.add(entry.getValue());
return children;
})
.orElse(new HashSet<Model>() {
{
add(entry.getValue());
}
})));
}
return result;
}

/**
* 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 @@ -447,13 +460,9 @@ 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,
Expand Down Expand Up @@ -496,50 +505,27 @@ private void applyChange(

getLog().debug("Looking for modules which use "
+ ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent");

for (Map.Entry<File, Model> stringModelEntry : fullTreeMatch
? 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) + ":" + newVersion);
} else {
getLog().debug(" parent is " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId)
+ ":" + (parent == null ? "" : parent.getVersion()));
getLog().debug(" will become " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId)
+ ":" + newVersion);
}
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))
+ ":" + newVersion);
addChange(
PomHelper.getGroupId(targetModel),
PomHelper.getArtifactId(targetModel),
PomHelper.getVersion(targetModel),
newVersion);
targetModel.setVersion(newVersion);
} 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
57 changes: 57 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,10 +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 @@ -228,4 +236,53 @@ 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("parent2"), new Model() {
{
setArtifactId("parent2");
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()));
}
}

0 comments on commit b3c643d

Please sign in to comment.