Repository: mojohaus/versions (java)

Issue:
Null logged in "Processing change of null:<artifactId>:<old version> -> <new version>"
Setting the version (e.g. `mvn versions:set -DnewVersion=1.1.0-SNAPSHOT`) in the root dir of a multi module maven project, where the `groupId` is inherited from a parent pom, the `groupId` is logged as `null` in: `Processing change of null:my-artifact-id:1.0.0-SNAPSHOT -> 1.1.0-SNAPSHOT`. It seems only an issue with the log output, as the version of any submodule is changed as expected. The issue was introduced with version 2.16.0 of the plugin.

maven 3.9.9
versions-maven-plugin: 2.17.1


Pull request:
Resolves #1137: using PomHelper.getGroupId if groupId of the current project root is empty
@slawekjaranowski please review

Patch:
diff --git a/pom.xml b/pom.xml
index b434e9ac..3fd30df6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -131,7 +131,7 @@
     <fluidoVersion>${maven-fluido-skin.version}</fluidoVersion>
     <modelloNamespaceRuleVersion>2.1.0</modelloNamespaceRuleVersion>
     <modelloNamespaceReportVersion>2.0.0</modelloNamespaceReportVersion>
-    <byteBuddyVersion>1.15.4</byteBuddyVersion>
+    <byteBuddyVersion>1.15.5</byteBuddyVersion>
     <!-- use the same version as in Maven core -->
     <mavenResolverVersion>1.4.1</mavenResolverVersion>
 
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 2be7d472..b1b9fef9 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
@@ -345,7 +345,7 @@ public class SetMojo extends AbstractVersionsUpdaterMojo {
             // so that the main project can be selected
             Model rootModel = reactorModels.get(session.getCurrentProject().getFile());
             if (groupId == null) {
-                groupId = rootModel.getGroupId();
+                groupId = PomHelper.getGroupId(rootModel);
             }
             if (artifactId == null) {
                 artifactId = rootModel.getArtifactId();


Test patch:
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 898d895b..73038277 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
@@ -6,14 +6,17 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.commons.lang3.tuple.Triple;
 import org.apache.maven.model.Model;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugin.testing.AbstractMojoTestCase;
 import org.apache.maven.plugin.testing.MojoRule;
 import org.apache.maven.project.MavenProject;
+import org.codehaus.mojo.versions.utils.TestLog;
 import org.codehaus.mojo.versions.utils.TestUtils;
 import org.junit.After;
 import org.junit.Before;
@@ -231,4 +234,18 @@ public class SetMojoTest extends AbstractMojoTestCase {
                 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 testIssue1137() throws Exception {
+        TestUtils.copyDir(Paths.get("src/test/resources/org/codehaus/mojo/set/issue-1137"), tempDir);
+        TestLog testLog = new TestLog();
+        SetMojo mojo =
+                (SetMojo) mojoRule.lookupConfiguredMojo(tempDir.resolve("child").toFile(), "set");
+        setVariableValueToObject(mojo, "newVersion", "1.1");
+        setVariableValueToObject(mojo, "log", testLog);
+        mojo.execute();
+        assertThat(
+                testLog.getLoggedMessages().stream().map(Triple::getMiddle).collect(Collectors.joining("\n")),
+                not(containsString("Processing change of null:child")));
+    }
 }
diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1137/child/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1137/child/pom.xml
new file mode 100644
index 00000000..0eb49500
--- /dev/null
+++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1137/child/pom.xml
@@ -0,0 +1,11 @@
+<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>group</groupId>
+        <artifactId>parent</artifactId>
+        <version>1.0</version>
+    </parent>
+    <artifactId>child</artifactId>
+    <version>1.0</version>
+</project>
diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1137/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1137/pom.xml
new file mode 100644
index 00000000..3f5b9e95
--- /dev/null
+++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-1137/pom.xml
@@ -0,0 +1,11 @@
+<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>group</groupId>
+    <artifactId>parent</artifactId>
+    <version>1.0</version>
+    <packaging>pom</packaging>
+    <modules>
+        <module>child</module>
+    </modules>
+</project>
diff --git a/versions-test/src/main/java/org/codehaus/mojo/versions/utils/TestLog.java b/versions-test/src/main/java/org/codehaus/mojo/versions/utils/TestLog.java
new file mode 100644
index 00000000..daef1f4f
--- /dev/null
+++ b/versions-test/src/main/java/org/codehaus/mojo/versions/utils/TestLog.java
@@ -0,0 +1,102 @@
+package org.codehaus.mojo.versions.utils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.commons.lang3.tuple.Triple;
+import org.apache.maven.plugin.logging.Log;
+
+public class TestLog implements Log {
+    private final List<Triple<LogLevel, CharSequence, Throwable>> loggedMessages = new ArrayList<>();
+
+    public List<Triple<LogLevel, CharSequence, Throwable>> getLoggedMessages() {
+        return loggedMessages;
+    }
+
+    @Override
+    public boolean isDebugEnabled() {
+        return false;
+    }
+
+    @Override
+    public void debug(CharSequence charSequence) {
+        debug(charSequence, null);
+    }
+
+    @Override
+    public void debug(CharSequence charSequence, Throwable throwable) {
+        loggedMessages.add(Triple.of(LogLevel.DEBUG, charSequence, throwable));
+    }
+
+    @Override
+    public void debug(Throwable throwable) {
+        debug(null, throwable);
+    }
+
+    @Override
+    public boolean isInfoEnabled() {
+        return false;
+    }
+
+    @Override
+    public void info(CharSequence charSequence) {
+        info(charSequence, null);
+    }
+
+    @Override
+    public void info(CharSequence charSequence, Throwable throwable) {
+        loggedMessages.add(Triple.of(LogLevel.INFO, charSequence, throwable));
+    }
+
+    @Override
+    public void info(Throwable throwable) {
+        info(null, throwable);
+    }
+
+    @Override
+    public boolean isWarnEnabled() {
+        return false;
+    }
+
+    @Override
+    public void warn(CharSequence charSequence) {
+        warn(charSequence, null);
+    }
+
+    @Override
+    public void warn(CharSequence charSequence, Throwable throwable) {
+        loggedMessages.add(Triple.of(LogLevel.WARN, charSequence, throwable));
+    }
+
+    @Override
+    public void warn(Throwable throwable) {
+        warn(null, throwable);
+    }
+
+    @Override
+    public boolean isErrorEnabled() {
+        return false;
+    }
+
+    @Override
+    public void error(CharSequence charSequence) {
+        error(charSequence, null);
+    }
+
+    @Override
+    public void error(CharSequence charSequence, Throwable throwable) {
+        loggedMessages.add(Triple.of(LogLevel.ERROR, charSequence, throwable));
+    }
+
+    @Override
+    public void error(Throwable throwable) {
+        error(null, throwable);
+    }
+
+    public enum LogLevel {
+        DEBUG,
+        INFO,
+        WARN,
+        ERROR
+    }
+}