Resolves #973: NPE if actual version is null for a dependency (#974)

mojohaus · Jun 23, 2023 · 7b566f2 · 7b566f2
1 parent ff26c22
commit 7b566f2
Show file tree

Hide file tree

Showing 12 changed files with 143 additions and 51 deletions.
diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java
@@ -82,11 +82,12 @@ protected ArtifactVersion getHighestLowerBound(ArtifactVersion lowerBoundVersion
      * If the artifact is bound by one or more version ranges, returns the restriction that constitutes
      * the version range containing the selected actual version.
      * If there are no version ranges, returns the provided version.
-     * @param selectedVersion actual version used
+     * @param selectedVersion actual version used, may not be {@code null}
      * @return restriction containing the version range selected by the given version,
      * or {@link Optional#empty()} if there are no ranges
      */
     protected Optional<Restriction> getSelectedRestriction(ArtifactVersion selectedVersion) {
+        assert selectedVersion != null;
         return Optional.ofNullable(getCurrentVersionRange())
                 .map(VersionRange::getRestrictions)
                 .flatMap(r -> r.stream()
@@ -116,7 +117,8 @@ public Restriction restrictionForSelectedSegment(ArtifactVersion lowerBound, Opt
     public Restriction restrictionForUnchangedSegment(
             ArtifactVersion actualVersion, Optional<Segment> unchangedSegment, boolean allowDowngrade)
             throws InvalidSegmentException {
-        Optional<Restriction> selectedRestriction = getSelectedRestriction(actualVersion);
+        Optional<Restriction> selectedRestriction =
+                Optional.ofNullable(actualVersion).flatMap(this::getSelectedRestriction);
         ArtifactVersion selectedRestrictionUpperBound =
                 selectedRestriction.map(Restriction::getUpperBound).orElse(actualVersion);
         ArtifactVersion lowerBound = allowDowngrade

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/DefaultVersionsHelper.java
@@ -519,15 +519,15 @@ public ArtifactVersion createArtifactVersion(String version) {
     }
 
     public Map<Dependency, ArtifactVersions> lookupDependenciesUpdates(
-            Set<Dependency> dependencies,
+            Stream<Dependency> dependencies,
             boolean usePluginRepositories,
             boolean useProjectRepositories,
             boolean allowSnapshots)
             throws VersionRetrievalException {
         ExecutorService executor = Executors.newFixedThreadPool(LOOKUP_PARALLEL_THREADS);
         try {
             Map<Dependency, ArtifactVersions> dependencyUpdates = new TreeMap<>(DependencyComparator.INSTANCE);
-            List<Future<? extends Pair<Dependency, ArtifactVersions>>> futures = dependencies.stream()
+            List<Future<? extends Pair<Dependency, ArtifactVersions>>> futures = dependencies
                     .map(dependency -> executor.submit(() -> new ImmutablePair<>(
                             dependency,
                             lookupDependencyUpdates(
@@ -549,7 +549,7 @@ public Map<Dependency, ArtifactVersions> lookupDependenciesUpdates(
 
     @Override
     public Map<Dependency, ArtifactVersions> lookupDependenciesUpdates(
-            Set<Dependency> dependencies, boolean usePluginRepositories, boolean allowSnapshots)
+            Stream<Dependency> dependencies, boolean usePluginRepositories, boolean allowSnapshots)
             throws VersionRetrievalException {
         return lookupDependenciesUpdates(dependencies, usePluginRepositories, !usePluginRepositories, allowSnapshots);
     }
@@ -570,13 +570,13 @@ public ArtifactVersions lookupDependencyUpdates(
     }
 
     @Override
-    public Map<Plugin, PluginUpdatesDetails> lookupPluginsUpdates(Set<Plugin> plugins, boolean allowSnapshots)
+    public Map<Plugin, PluginUpdatesDetails> lookupPluginsUpdates(Stream<Plugin> plugins, boolean allowSnapshots)
             throws VersionRetrievalException {
         ExecutorService executor = Executors.newFixedThreadPool(LOOKUP_PARALLEL_THREADS);
         try {
             Map<Plugin, PluginUpdatesDetails> pluginUpdates = new TreeMap<>(PluginComparator.INSTANCE);
-            List<Future<? extends Pair<Plugin, PluginUpdatesDetails>>> futures = plugins.stream()
-                    .map(p -> executor.submit(() -> new ImmutablePair<>(p, lookupPluginUpdates(p, allowSnapshots))))
+            List<Future<? extends Pair<Plugin, PluginUpdatesDetails>>> futures = plugins.map(
+                            p -> executor.submit(() -> new ImmutablePair<>(p, lookupPluginUpdates(p, allowSnapshots))))
                     .collect(Collectors.toList());
             for (Future<? extends Pair<Plugin, PluginUpdatesDetails>> details : futures) {
                 Pair<Plugin, PluginUpdatesDetails> pair = details.get();
@@ -602,7 +602,7 @@ public PluginUpdatesDetails lookupPluginUpdates(Plugin plugin, boolean allowSnap
             pluginDependencies.addAll(plugin.getDependencies());
         }
         Map<Dependency, ArtifactVersions> pluginDependencyDetails =
-                lookupDependenciesUpdates(pluginDependencies, false, allowSnapshots);
+                lookupDependenciesUpdates(pluginDependencies.stream(), false, allowSnapshots);
 
         ArtifactVersions allVersions = lookupArtifactVersions(
                 createPluginArtifact(plugin.getGroupId(), plugin.getArtifactId(), version), true);

diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionsHelper.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionsHelper.java
@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Stream;
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.resolver.ArtifactResolutionException;
@@ -198,29 +199,29 @@ ArtifactVersions lookupArtifactVersions(Artifact artifact, VersionRange versionR
      * Returns a map of all possible updates per dependency. The lookup is done in parallel using
      * {@code LOOKUP_PARALLEL_THREADS} threads.
      *
-     * @param dependencies The set of {@link Dependency} instances to look up.
+     * @param dependencyStream a stream of {@link Dependency} instances to look up.
      * @param usePluginRepositories Search the plugin repositories.
      * @param allowSnapshots whether snapshots should be included
      * @return map containing the ArtifactVersions object per dependency
      * @throws VersionRetrievalException thrown if a version cannot be retrieved
      */
     Map<Dependency, ArtifactVersions> lookupDependenciesUpdates(
-            Set<Dependency> dependencies, boolean usePluginRepositories, boolean allowSnapshots)
+            Stream<Dependency> dependencyStream, boolean usePluginRepositories, boolean allowSnapshots)
             throws VersionRetrievalException;
 
     /**
      * Returns a map of all possible updates per dependency. The lookup is done in parallel using
      * {@code LOOKUP_PARALLEL_THREADS} threads.
      *
-     * @param dependencies The set of {@link Dependency} instances to look up.
+     * @param dependencies stream of {@link Dependency} instances to look up.
      * @param usePluginRepositories Search the plugin repositories.
      * @param useProjectRepositories whether to use regular project repositories
      * @param allowSnapshots whether snapshots should be included
      * @return map containing the ArtifactVersions object per dependency
      * @throws VersionRetrievalException thrown if a version cannot be retrieved
      */
     Map<Dependency, ArtifactVersions> lookupDependenciesUpdates(
-            Set<Dependency> dependencies,
+            Stream<Dependency> dependencies,
             boolean usePluginRepositories,
             boolean useProjectRepositories,
             boolean allowSnapshots)
@@ -247,13 +248,13 @@ ArtifactVersions lookupDependencyUpdates(
     /**
      * Looks up the updates for a set of plugins.
      *
-     * @param plugins The set of {@link Plugin} instances to look up.
+     * @param plugins A stream of {@link Plugin} instances to look up.
      * @param allowSnapshots Include snapshots in the list of updates.
      * @return A map, keyed by plugin, with values of type {@link org.codehaus.mojo.versions.api.PluginUpdatesDetails}.
      * @throws VersionRetrievalException thrown if version resolution fails
      * @since 1.0-beta-1
      */
-    Map<Plugin, PluginUpdatesDetails> lookupPluginsUpdates(Set<Plugin> plugins, boolean allowSnapshots)
+    Map<Plugin, PluginUpdatesDetails> lookupPluginsUpdates(Stream<Plugin> plugins, boolean allowSnapshots)
             throws VersionRetrievalException;
 
     /**

diff --git a/versions-common/src/test/java/org/codehaus/mojo/versions/api/AbstractVersionDetailsTest.java b/versions-common/src/test/java/org/codehaus/mojo/versions/api/AbstractVersionDetailsTest.java
@@ -30,6 +30,7 @@
 import static org.codehaus.mojo.versions.utils.ArtifactVersionUtils.version;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.fail;
 
 /**
  * Unit tests for {@link AbstractVersionDetails}
@@ -156,4 +157,24 @@ public void testRestrictionForUnchangedSegmentWithTwoNotConnectingRanges4()
                         .containsVersion(version("2.0.0-1")),
                 is(true));
     }
+
+    @Test
+    public void testRestrictionForUnchangedSegmentWithoutVersionInformation()
+            throws InvalidSegmentException, InvalidVersionSpecificationException {
+        instance.setCurrentVersionRange(VersionRange.createFromVersionSpec("[,0]"));
+        assertThat(
+                instance.restrictionForUnchangedSegment(null, empty(), false).containsVersion(version("1.0.0")),
+                is(true));
+    }
+
+    @Test
+    public void testGetSelectedRestrictionForNoVersion()
+            throws InvalidVersionSpecificationException, InvalidSegmentException {
+        instance.setCurrentVersionRange(VersionRange.createFromVersionSpec("[,0]"));
+        try {
+            instance.getSelectedRestriction(null);
+            fail("Expected a NullPointerException to be thrown or assertions are not enabled.");
+        } catch (AssertionError ignored) {
+        }
+    }
 }
diff --git a/versions-enforcer/src/main/java/org/apache/maven/plugins/enforcer/MaxDependencyUpdates.java b/versions-enforcer/src/main/java/org/apache/maven/plugins/enforcer/MaxDependencyUpdates.java
@@ -359,14 +359,15 @@ public void execute(EnforcerRuleHelper ruleHelper) throws EnforcerRuleException
             Optional<Segment> ignoredSegment = ignoreSubIncrementalUpdates
                     ? of(SUBINCREMENTAL)
                     : ignoreIncrementalUpdates ? of(INCREMENTAL) : ignoreMinorUpdates ? of(MINOR) : empty();
-            List<ArtifactVersions> upgradable =
-                    versionsHelper.lookupDependenciesUpdates(dependencies, false, allowSnapshots).values().stream()
-                            .filter(v -> v.getVersions(
-                                                    v.restrictionForIgnoreScope(v.getCurrentVersion(), ignoredSegment),
-                                                    true)
-                                            .length
-                                    > 0)
-                            .collect(Collectors.toList());
+            List<ArtifactVersions> upgradable = versionsHelper
+                    .lookupDependenciesUpdates(
+                            dependencies.stream().filter(d -> d.getVersion() != null), false, allowSnapshots)
+                    .values()
+                    .stream()
+                    .filter(v -> v.getVersions(v.restrictionForIgnoreScope(v.getCurrentVersion(), ignoredSegment), true)
+                                    .length
+                            > 0)
+                    .collect(Collectors.toList());
             if (upgradable.size() > maxUpdates) {
                 throw new EnforcerRuleException("More than " + maxUpdates + " upgradable artifacts detected: "
                         + upgradable.stream()

diff --git a/...aven-plugin/src/it/it-display-dependency-updates-issue-973-versionless/invoker.properties b/...aven-plugin/src/it/it-display-dependency-updates-issue-973-versionless/invoker.properties
@@ -0,0 +1,2 @@
+invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:display-dependency-updates
+invoker.mavenOpts = -Dversions.outputFile=./output.txt -DoutputEncoding=UTF-8
diff --git a/versions-maven-plugin/src/it/it-display-dependency-updates-issue-973-versionless/pom.xml b/versions-maven-plugin/src/it/it-display-dependency-updates-issue-973-versionless/pom.xml
@@ -0,0 +1,43 @@
+<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>localhost</groupId>
+  <artifactId>it-display-dependency-updates-002</artifactId>
+  <version>1.0</version>
+  <packaging>pom</packaging>
+  <name>display-dependency-updates-from-plugins</name>
+
+  <description>
+    Edge case: the pom.xml will fail when we try executing the actual plugin
+    as it is invalid -- lacks version, which caused an NPE (Issue #973)
+    when executing display-dependency-updates.
+  </description>
+
+  <dependencyManagement>
+    <dependencies>
+      <dependency>
+        <groupId>localhost</groupId>
+        <artifactId>dummy-api</artifactId>
+        <version>1.1</version>
+      </dependency>
+    </dependencies>
+  </dependencyManagement>
+
+  <build>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <groupId>localhost</groupId>
+          <artifactId>dummy-maven-plugin</artifactId>
+          <dependencies>
+            <dependency>
+              <groupId>localhost</groupId>
+              <artifactId>dummy-api</artifactId>
+            </dependency>
+          </dependencies>
+        </plugin>
+      </plugins>
+    </pluginManagement>
+  </build>
+
+</project>
diff --git a/...ons-maven-plugin/src/it/it-display-dependency-updates-issue-973-versionless/verify.groovy b/...ons-maven-plugin/src/it/it-display-dependency-updates-issue-973-versionless/verify.groovy
@@ -0,0 +1,4 @@
+def output = new File(basedir, "output.txt").text
+def matcher = output =~ /\Qlocalhost:dummy-api\E.*->\s+\Q3.0\E\b/
+assert matcher.find() : "localhost:dummy-api should have been bumped to version 3.0"
+assert !matcher.find() : "localhost:dummy-api may only appear once in the updates list"
diff --git a/...-plugin/src/main/java/org/codehaus/mojo/versions/AbstractDependencyUpdatesReportMojo.java b/...-plugin/src/main/java/org/codehaus/mojo/versions/AbstractDependencyUpdatesReportMojo.java
@@ -145,11 +145,16 @@ protected void doGenerateReport(Locale locale, Sink sink) throws MavenReportExce
 
         try {
 
-            Map<Dependency, ArtifactVersions> dependencyUpdates =
-                    getHelper().lookupDependenciesUpdates(dependencies, false, allowSnapshots);
+            Map<Dependency, ArtifactVersions> dependencyUpdates = getHelper()
+                    .lookupDependenciesUpdates(
+                            dependencies.stream().filter(d -> d.getVersion() != null), false, allowSnapshots);
 
             Map<Dependency, ArtifactVersions> dependencyManagementUpdates = processDependencyManagement
-                    ? getHelper().lookupDependenciesUpdates(dependencyManagement, false, allowSnapshots)
+                    ? getHelper()
+                            .lookupDependenciesUpdates(
+                                    dependencyManagement.stream().filter(d -> d.getVersion() != null),
+                                    false,
+                                    allowSnapshots)
                     : emptyMap();
 
             if (onlyUpgradable) {

diff --git a/...aven-plugin/src/main/java/org/codehaus/mojo/versions/AbstractPluginUpdatesReportMojo.java b/...aven-plugin/src/main/java/org/codehaus/mojo/versions/AbstractPluginUpdatesReportMojo.java
@@ -126,9 +126,9 @@ protected void doGenerateReport(Locale locale, Sink sink) throws MavenReportExce
         try {
 
             Map<Plugin, PluginUpdatesDetails> pluginUpdates =
-                    getHelper().lookupPluginsUpdates(plugins, getAllowSnapshots());
+                    getHelper().lookupPluginsUpdates(plugins.stream(), getAllowSnapshots());
             Map<Plugin, PluginUpdatesDetails> pluginManagementUpdates =
-                    getHelper().lookupPluginsUpdates(pluginManagement, getAllowSnapshots());
+                    getHelper().lookupPluginsUpdates(pluginManagement.stream(), getAllowSnapshots());
 
             if (onlyUpgradable) {
                 pluginUpdates = filter(pluginUpdates, p -> !p.isEmpty(allowSnapshots));
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:display-dependency-updates
		invoker.mavenOpts = -Dversions.outputFile=./output.txt -DoutputEncoding=UTF-8