Issue #925: Protect against an NPE if a dependency version is defined…

… in dependencyManagement and dependency processing is enabled
mojohaus · Mar 10, 2023 · 841d24f · 841d24f
1 parent 12d67e0
commit 841d24f
Show file tree

Hide file tree

Showing 3 changed files with 161 additions and 30 deletions.
diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/UseDepVersionMojo.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/UseDepVersionMojo.java
@@ -355,38 +355,44 @@ private void useDepVersion(
 
             Artifact artifact = toArtifact(dep);
             if (isIncluded(artifact)) {
-                if (!forceVersion) {
-                    if (!getHelper().lookupArtifactVersions(artifact, false).containsVersion(depVersion)) {
-                        throw new MojoExecutionException(String.format(
-                                "Version %s is not available for artifact %s:%s",
-                                depVersion, artifact.getGroupId(), artifact.getArtifactId()));
-                    }
-                }
-                if (!propertyName.isPresent()) {
-                    updateDependencyVersion(node.getModifiedPomXMLEventReader(), dep, depVersion, changeKind);
+                if (dep.getVersion() == null) {
+                    getLog().warn(String.format(
+                            "Not updating %s:%s in dependencies: version defined " + "in dependencyManagement",
+                            dep.getGroupId(), dep.getArtifactId()));
                 } else {
-                    // propertyName is present
-                    ofNullable(propertyConflicts.get(propertyName.get()))
-                            .map(conflict -> {
-                                getLog().warn("Cannot update property ${" + propertyName.get() + "}: "
-                                        + "controls more than one dependency: "
-                                        + conflict.stream()
-                                                .map(Dependency::getArtifactId)
-                                                .collect(Collectors.joining(", ")));
-                                return false;
-                            })
-                            .orElseGet(() -> {
-                                if (!updatePropertyValue(node, propertyName.get())) {
-                                    propertyBacklog.add(propertyName.get());
-                                } else {
-                                    if (getLog().isDebugEnabled()) {
-                                        getLog().debug(String.format(
-                                                "Updated the %s property value to %s.",
-                                                propertyName.get(), depVersion));
+                    if (!forceVersion) {
+                        if (!getHelper().lookupArtifactVersions(artifact, false).containsVersion(depVersion)) {
+                            throw new MojoExecutionException(String.format(
+                                    "Version %s is not available for artifact %s:%s",
+                                    depVersion, artifact.getGroupId(), artifact.getArtifactId()));
+                        }
+                    }
+                    if (!propertyName.isPresent()) {
+                        updateDependencyVersion(node.getModifiedPomXMLEventReader(), dep, depVersion, changeKind);
+                    } else {
+                        // propertyName is present
+                        ofNullable(propertyConflicts.get(propertyName.get()))
+                                .map(conflict -> {
+                                    getLog().warn("Cannot update property ${" + propertyName.get() + "}: "
+                                            + "controls more than one dependency: "
+                                            + conflict.stream()
+                                                    .map(Dependency::getArtifactId)
+                                                    .collect(Collectors.joining(", ")));
+                                    return false;
+                                })
+                                .orElseGet(() -> {
+                                    if (!updatePropertyValue(node, propertyName.get())) {
+                                        propertyBacklog.add(propertyName.get());
+                                    } else {
+                                        if (getLog().isDebugEnabled()) {
+                                            getLog().debug(String.format(
+                                                    "Updated the %s property value to %s.",
+                                                    propertyName.get(), depVersion));
+                                        }
                                     }
-                                }
-                                return true;
-                            });
+                                    return true;
+                                });
+                    }
                 }
             }
         }

diff --git a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/UseDepVersionMojoTest.java b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/UseDepVersionMojoTest.java
@@ -24,6 +24,7 @@
 import org.apache.maven.plugin.logging.SystemStreamLog;
 import org.apache.maven.plugin.testing.AbstractMojoTestCase;
 import org.apache.maven.plugin.testing.MojoRule;
+import org.codehaus.mojo.versions.change.DefaultDependencyVersionChange;
 import org.codehaus.mojo.versions.change.DefaultPropertyVersionChange;
 import org.codehaus.mojo.versions.utils.TestChangeRecorder;
 import org.codehaus.mojo.versions.utils.TestUtils;
@@ -348,4 +349,85 @@ public void testPropertyFromParent() throws Exception {
         assertThat(changeRecorder.getChanges(), empty());
         assertThat(log.toString(), containsString("[WARN] Not updating property ${revision}: defined in parent"));
     }
+
+    @Test
+    public void testVersionlessDependency() throws Exception {
+        Log logger = mock(Log.class);
+        StringBuilder log = new StringBuilder();
+        doAnswer(i -> log.append("[WARN] ").append(i.getArgument(0).toString()))
+                .when(logger)
+                .warn(anyString());
+        TestUtils.copyDir(Paths.get("src/test/resources/org/codehaus/mojo/use-dep-version/issue-925"), tempDir);
+        UseDepVersionMojo mojo = (UseDepVersionMojo) mojoRule.lookupConfiguredMojo(tempDir.toFile(), "use-dep-version");
+        setVariableValueToObject(mojo, "reactorProjects", Collections.singletonList(mojo.getProject()));
+        setVariableValueToObject(mojo, "repositorySystem", mockRepositorySystem());
+        setVariableValueToObject(mojo, "aetherRepositorySystem", mockAetherRepositorySystem());
+        setVariableValueToObject(mojo, "changeRecorders", changeRecorder.asTestMap());
+        setVariableValueToObject(mojo, "log", logger);
+        mojo.depVersion = "2.0.0";
+        mojo.forceVersion = true;
+        setVariableValueToObject(mojo, "processDependencies", true);
+        setVariableValueToObject(mojo, "processDependencyManagement", false);
+        setVariableValueToObject(mojo, "includes", new String[] {"default-group:artifactA"});
+
+        mojo.execute();
+
+        assertThat(changeRecorder.getChanges(), empty());
+        assertThat(
+                log.toString(),
+                containsString(
+                        "[WARN] Not updating default-group:artifactA in dependencies: version defined in dependencyManagement"));
+    }
+
+    @Test
+    public void testDependencyManagemenent() throws Exception {
+        TestUtils.copyDir(Paths.get("src/test/resources/org/codehaus/mojo/use-dep-version/issue-925"), tempDir);
+        UseDepVersionMojo mojo = (UseDepVersionMojo) mojoRule.lookupConfiguredMojo(tempDir.toFile(), "use-dep-version");
+        setVariableValueToObject(mojo, "reactorProjects", Collections.singletonList(mojo.getProject()));
+        setVariableValueToObject(mojo, "repositorySystem", mockRepositorySystem());
+        setVariableValueToObject(mojo, "aetherRepositorySystem", mockAetherRepositorySystem());
+        setVariableValueToObject(mojo, "changeRecorders", changeRecorder.asTestMap());
+        mojo.depVersion = "2.0.0";
+        mojo.forceVersion = true;
+        setVariableValueToObject(mojo, "processDependencies", false);
+        setVariableValueToObject(mojo, "processDependencyManagement", true);
+        setVariableValueToObject(mojo, "includes", new String[] {"default-group:artifactA"});
+
+        mojo.execute();
+
+        assertThat(
+                changeRecorder.getChanges(),
+                hasItem(new DefaultDependencyVersionChange("default-group", "artifactA", "1.0.0", "2.0.0")));
+    }
+
+    @Test
+    public void testVersionDefinedInDependencyManagemenent() throws Exception {
+        Log logger = mock(Log.class);
+        StringBuilder log = new StringBuilder();
+        doAnswer(i -> log.append("[WARN] ").append(i.getArgument(0).toString()))
+                .when(logger)
+                .warn(anyString());
+        TestUtils.copyDir(Paths.get("src/test/resources/org/codehaus/mojo/use-dep-version/issue-925"), tempDir);
+        UseDepVersionMojo mojo = (UseDepVersionMojo) mojoRule.lookupConfiguredMojo(tempDir.toFile(), "use-dep-version");
+        setVariableValueToObject(mojo, "reactorProjects", Collections.singletonList(mojo.getProject()));
+        setVariableValueToObject(mojo, "repositorySystem", mockRepositorySystem());
+        setVariableValueToObject(mojo, "aetherRepositorySystem", mockAetherRepositorySystem());
+        setVariableValueToObject(mojo, "changeRecorders", changeRecorder.asTestMap());
+        setVariableValueToObject(mojo, "log", logger);
+        mojo.depVersion = "2.0.0";
+        mojo.forceVersion = true;
+        setVariableValueToObject(mojo, "processDependencies", true);
+        setVariableValueToObject(mojo, "processDependencyManagement", true);
+        setVariableValueToObject(mojo, "includes", new String[] {"default-group:artifactA"});
+
+        mojo.execute();
+
+        assertThat(
+                log.toString(),
+                containsString(
+                        "[WARN] Not updating default-group:artifactA in dependencies: version defined in dependencyManagement"));
+        assertThat(
+                changeRecorder.getChanges(),
+                hasItem(new DefaultDependencyVersionChange("default-group", "artifactA", "1.0.0", "2.0.0")));
+    }
 }
diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/use-dep-version/issue-925/pom.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/use-dep-version/issue-925/pom.xml
@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="UTF-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>test-group</groupId>
+    <artifactId>test-artifact</artifactId>
+    <version>DEVELOP-SNAPSHOT</version>
+
+    <dependencies>
+        <dependency>
+            <groupId>default-group</groupId>
+            <artifactId>artifactA</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>default-group</groupId>
+            <artifactId>artifactB</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>default-group</groupId>
+            <artifactId>artifactC</artifactId>
+        </dependency>
+    </dependencies>
+
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>default-group</groupId>
+                <artifactId>artifactA</artifactId>
+                <version>1.0.0</version>
+            </dependency>
+            <dependency>
+                <groupId>default-group</groupId>
+                <artifactId>artifactB</artifactId>
+                <version>1.0.0</version>
+            </dependency>
+            <dependency>
+                <groupId>default-group</groupId>
+                <artifactId>artifactC</artifactId>
+                <version>1.0.0</version>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+</project>