Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
}
Expand All @@ -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();
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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) {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
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
Original file line number Diff line number Diff line change
@@ -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>
slawekjaranowski marked this conversation as resolved.
Show resolved Hide resolved
</plugin>
</plugins>
</pluginManagement>
</build>

</project>
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading