Skip to content

Commit 9dd0035

Browse files
andrzejj0andrzejj0
authored
Resolves#1066: Fixed a possible NPE in interpolateVersion if a depdendencyManagement entry has no version. (#1166)
Added a couple unit tests to interpolateVersion. interpolateVersion does not mutate the dependency anymore but returns a modified copy if it needs to modify it.
1 parent 26628bb commit 9dd0035

File tree

4 files changed

+142
-35
lines changed

4 files changed

+142
-35
lines changed

‎versions-common/src/main/java/org/codehaus/mojo/versions/utils/MavenProjectUtils.java

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,15 @@
3232

3333
import static java.util.Collections.emptyList;
3434
import static java.util.Optional.ofNullable;
35+
import static org.apache.commons.lang3.StringUtils.startsWith;
3536

3637
/**
3738
* Utility methods for extracting dependencies from a {@link org.apache.maven.project.MavenProject}
3839
*/
3940
public class MavenProjectUtils {
4041
/**
4142
* Retrieves dependencies from the plugins section
43+
*
4244
* @param project {@link MavenProject} instance
4345
* @return set of {@link Dependency} objects
4446
* or an empty set if none have been retrieveddependencies or an empty set if none have been retrieved
@@ -56,9 +58,10 @@ public static Set<Dependency> extractPluginDependenciesFromPluginsInPluginManage
5658

5759
/**
5860
* Retrieves dependencies from plugin management
61+
*
5962
* @param project {@link MavenProject} instance
6063
* @return set of {@link Dependency} objects
61-
* or an empty set if none have been retrieveddependencies or an empty set if none have been retrieved
64+
* or an empty set if none have been retrieveddependencies or an empty set if none have been retrieved
6265
*/
6366
public static Set<Dependency> extractDependenciesFromPlugins(MavenProject project) {
6467
return project.getBuildPlugins().stream()
@@ -71,14 +74,15 @@ public static Set<Dependency> extractDependenciesFromPlugins(MavenProject projec
7174
* Retrieves dependencies from the dependency management of the project
7275
* as well as its immediate parent project.
7376
*
74-
* @param project {@link MavenProject} instance
77+
* @param project {@link MavenProject} instance
7578
* @param processDependencyManagementTransitive if {@code true}, the original model will be considered
7679
* instead of the interpolated model, which does not contain
7780
* imported dependencies
78-
* @param log {@link Log} instance (may not be null)
81+
* @param log {@link Log} instance (may not be null)
7982
* @return set of {@link Dependency} objects
8083
* @throws VersionRetrievalException thrown if version information retrieval fails
81-
* or an empty set if none have been retrieveddependencies or an empty set if none have been retrieved
84+
* or an empty set if none have been retrieveddependencies or an empty set if none
85+
* have been retrieved
8286
*/
8387
public static Set<Dependency> extractDependenciesFromDependencyManagement(
8488
MavenProject project, boolean processDependencyManagementTransitive, Log log)
@@ -119,8 +123,8 @@ public static Set<Dependency> extractDependenciesFromDependencyManagement(
119123
throw new VersionRetrievalException(message);
120124
}
121125
} else {
122-
dependency = interpolateVersion(dependency, project);
123-
dependencyManagement.add(dependency);
126+
dependencyManagement.remove(dependency);
127+
dependencyManagement.add(interpolateVersion(dependency, project));
124128
}
125129
}
126130
}
@@ -131,22 +135,24 @@ public static Set<Dependency> extractDependenciesFromDependencyManagement(
131135
* Attempts to interpolate the version from model properties.
132136
*
133137
* @param dependency the dependency
134-
* @param project the maven project
138+
* @param project the maven project
135139
* @return the dependency with interpolated property (as far as possible)
136140
* @since 2.14.0
137141
*/
138142
public static Dependency interpolateVersion(final Dependency dependency, final MavenProject project) {
139-
140143
// resolve version from model properties if necessary (e.g. "${mycomponent.myversion}"
141-
if (dependency.getVersion().startsWith("${")) {
142-
final String resolvedVersion = project.getOriginalModel()
143-
.getProperties()
144-
.getProperty(dependency
145-
.getVersion()
146-
.substring(2, dependency.getVersion().length() - 1));
147-
if (resolvedVersion != null && !resolvedVersion.isEmpty()) {
148-
dependency.setVersion(resolvedVersion);
149-
}
144+
if (startsWith(dependency.getVersion(), "${")) {
145+
return ofNullable(project.getOriginalModel()
146+
.getProperties()
147+
.getProperty(dependency
148+
.getVersion()
149+
.substring(2, dependency.getVersion().length() - 1)))
150+
.map(v -> {
151+
Dependency result = dependency.clone();
152+
result.setVersion(v);
153+
return result;
154+
})
155+
.orElse(dependency);
150156
}
151157
return dependency;
152158
}

‎versions-common/src/test/java/org/codehaus/mojo/versions/utils/MavenProjectUtilsTest.java

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package org.codehaus.mojo.versions.utils;
2+
3+
import java.util.Properties;
4+
5+
import org.apache.maven.model.Dependency;
6+
import org.apache.maven.model.Model;
7+
import org.apache.maven.project.MavenProject;
8+
import org.junit.jupiter.api.Test;
9+
10+
import static org.codehaus.mojo.versions.utils.MavenProjectUtils.interpolateVersion;
11+
import static org.hamcrest.MatcherAssert.assertThat;
12+
import static org.hamcrest.Matchers.not;
13+
import static org.hamcrest.Matchers.nullValue;
14+
import static org.hamcrest.Matchers.sameInstance;
15+
import static org.hamcrest.core.Is.is;
16+
17+
public class MavenProjectUtilsTest {
18+
19+
@Test
20+
public void testInterpolateVersion() {
21+
Dependency dep = DependencyBuilder.newBuilder()
22+
.withGroupId("groupA")
23+
.withArtifactId("artifactA")
24+
.withVersion("${param}")
25+
.build();
26+
MavenProject proj = new MavenProject() {
27+
{
28+
setOriginalModel(new Model() {
29+
{
30+
setProperties(new Properties() {
31+
{
32+
setProperty("param", "1.0.0");
33+
}
34+
});
35+
}
36+
});
37+
}
38+
};
39+
Dependency result = interpolateVersion(dep, proj);
40+
assertThat(result.getVersion(), is("1.0.0"));
41+
}
42+
43+
@Test
44+
public void testImmutability() {
45+
Dependency dep = DependencyBuilder.newBuilder()
46+
.withGroupId("groupA")
47+
.withArtifactId("artifactA")
48+
.withVersion("${param}")
49+
.build();
50+
MavenProject proj = new MavenProject() {
51+
{
52+
setOriginalModel(new Model() {
53+
{
54+
setProperties(new Properties() {
55+
{
56+
setProperty("param", "1.0.0");
57+
}
58+
});
59+
}
60+
});
61+
}
62+
};
63+
assertThat(interpolateVersion(dep, proj), not(sameInstance(dep)));
64+
}
65+
66+
@Test
67+
public void testVersionlessDependency() {
68+
Dependency dep = DependencyBuilder.newBuilder()
69+
.withGroupId("groupA")
70+
.withArtifactId("artifactA")
71+
.build();
72+
MavenProject proj = new MavenProject() {
73+
{
74+
setOriginalModel(new Model() {
75+
{
76+
setProperties(new Properties() {
77+
{
78+
setProperty("param", "1.0.0");
79+
}
80+
});
81+
}
82+
});
83+
}
84+
};
85+
Dependency result = interpolateVersion(dep, proj);
86+
assertThat(result.getVersion(), nullValue());
87+
}
88+
}

‎versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/AbstractDependencyUpdatesReportMojo.java

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.maven.artifact.versioning.ArtifactVersion;
3434
import org.apache.maven.doxia.sink.Sink;
3535
import org.apache.maven.model.Dependency;
36+
import org.apache.maven.model.DependencyManagement;
3637
import org.apache.maven.plugins.annotations.Parameter;
3738
import org.apache.maven.project.MavenProject;
3839
import org.apache.maven.reporting.MavenReportException;
@@ -47,6 +48,7 @@
4748
import org.eclipse.aether.RepositorySystem;
4849

4950
import static java.util.Collections.emptyMap;
51+
import static java.util.Optional.ofNullable;
5052
import static org.codehaus.mojo.versions.utils.MavenProjectUtils.interpolateVersion;
5153
import static org.codehaus.mojo.versions.utils.MiscUtils.filter;
5254

@@ -189,29 +191,29 @@ protected void handleDependencyManagementTransitive(
189191
MavenProject project, Set<Dependency> dependencyManagementCollector) throws MavenReportException {
190192
if (processDependencyManagementTransitive) {
191193
if (hasDependencyManagement(project)) {
192-
for (Dependency dep : project.getDependencyManagement().getDependencies()) {
193-
getLog().debug("Dpmg: " + dep.getGroupId() + ":" + dep.getArtifactId() + ":" + dep.getVersion()
194-
+ ":" + dep.getType() + ":" + dep.getScope());
194+
if (getLog().isDebugEnabled()) {
195+
project.getDependencyManagement().getDependencies().forEach(dep -> getLog().debug(
196+
"Dpmg: " + dep.getGroupId() + ":" + dep.getArtifactId() + ":" + dep.getVersion()
197+
+ ":" + dep.getType() + ":" + dep.getScope()));
195198
}
196199
dependencyManagementCollector.addAll(
197200
project.getDependencyManagement().getDependencies());
198201
}
199202
} else {
200-
if (project.getOriginalModel().getDependencyManagement() != null
201-
&& project.getOriginalModel().getDependencyManagement().getDependencies() != null) {
202-
// Using the original model to getModel the original dependencyManagement entries and
203-
// not the interpolated model.
204-
// TODO: I'm not 100% sure if this will work correctly in all cases.
205-
for (Dependency dep :
206-
project.getOriginalModel().getDependencyManagement().getDependencies()) {
207-
dep = interpolateVersion(dep, project);
208-
209-
getLog().debug("Original Dpmg: " + dep.getGroupId() + ":" + dep.getArtifactId() + ":"
210-
+ dep.getVersion() + ":" + dep.getType() + ":" + dep.getScope());
211-
}
212-
dependencyManagementCollector.addAll(
213-
project.getOriginalModel().getDependencyManagement().getDependencies());
214-
}
203+
// Using the original model to getModel the original dependencyManagement entries and
204+
// not the interpolated model.
205+
// TODO: I'm not 100% sure if this will work correctly in all cases.
206+
ofNullable(project.getOriginalModel().getDependencyManagement())
207+
.map(DependencyManagement::getDependencies)
208+
.ifPresent(list -> list.stream()
209+
.map(dep -> interpolateVersion(dep, project))
210+
.peek(dep -> {
211+
if (getLog().isDebugEnabled()) {
212+
getLog().debug("Original Dpmg: " + dep.getGroupId() + ":" + dep.getArtifactId()
213+
+ ":" + dep.getVersion() + ":" + dep.getType() + ":" + dep.getScope());
214+
}
215+
})
216+
.forEach(dependencyManagementCollector::add));
215217
}
216218
}
217219

‎versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/DependencyUpdatesReportMojoTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,4 +364,15 @@ public void testResolvedVersionsWithoutTransitiveDependencyManagement()
364364
String output = os.toString();
365365
assertThat(output, Matchers.stringContainsInOrder("artifactA", "1.0.0", "artifactB", "1.2.3"));
366366
}
367+
368+
@Test
369+
public void testVersionlessDependency() throws IOException, MavenReportException {
370+
OutputStream os = new ByteArrayOutputStream();
371+
SinkFactory sinkFactory = new Xhtml5SinkFactory();
372+
new TestDependencyUpdatesReportMojo()
373+
.withOriginalDependencyManagement(dependencyOf("artifactA", null))
374+
.withProcessDependencyManagement(true)
375+
.withProcessDependencyManagementTransitive(false)
376+
.generate(sinkFactory.createSink(os), sinkFactory, Locale.getDefault());
377+
}
367378
}

0 commit comments

Comments
 (0)