diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java index fd0914d0fc..00cb37a1bf 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java @@ -136,16 +136,16 @@ public Exception getException() { @Override public String getMessage() { - String msg; + String msg = null; - if (message != null && message.length() > 0) { + if (message != null && !message.isEmpty()) { msg = message; - } else { + } else if (exception != null) { msg = exception.getMessage(); + } - if (msg == null) { - msg = ""; - } + if (msg == null) { + msg = ""; } return msg; diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java index 4456c541f5..613e8e1605 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java @@ -21,15 +21,16 @@ import javax.inject.Named; import javax.inject.Singleton; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import org.apache.maven.api.model.Dependency; import org.apache.maven.api.model.DependencyManagement; +import org.apache.maven.api.model.Exclusion; import org.apache.maven.api.model.Model; import org.apache.maven.model.building.ModelBuildingRequest; +import org.apache.maven.model.building.ModelProblem; import org.apache.maven.model.building.ModelProblemCollector; +import org.apache.maven.model.building.ModelProblemCollectorRequest; /** * Handles the import of dependency management from other models into the target model. @@ -58,10 +59,20 @@ public Model importManagement( depMgmt = DependencyManagement.newInstance(); } + Set directDependencies = new HashSet<>(dependencies.keySet()); + for (DependencyManagement source : sources) { for (Dependency dependency : source.getDependencies()) { String key = dependency.getManagementKey(); - dependencies.putIfAbsent(key, dependency); + Dependency present = dependencies.putIfAbsent(key, dependency); + if (present != null && !equals(dependency, present) && !directDependencies.contains(key)) { + // TODO: https://issues.apache.org/jira/browse/MNG-8004 + problems.add(new ModelProblemCollectorRequest( + ModelProblem.Severity.WARNING, ModelProblem.Version.V40) + .setMessage("Ignored POM import for: " + toString(dependency) + " as already imported " + + toString(present) + ". Add a the conflicting managed dependency directly " + + "to the dependencyManagement section of the POM.")); + } } } @@ -69,4 +80,60 @@ public Model importManagement( } return target; } + + private String toString(Dependency dependency) { + StringBuilder stringBuilder = new StringBuilder(); + stringBuilder + .append(dependency.getGroupId()) + .append(":") + .append(dependency.getArtifactId()) + .append(":") + .append(dependency.getType()); + if (dependency.getClassifier() != null && !dependency.getClassifier().isEmpty()) { + stringBuilder.append(":").append(dependency.getClassifier()); + } + stringBuilder + .append(":") + .append(dependency.getVersion()) + .append("@") + .append(dependency.getScope() == null ? "compile" : dependency.getScope()); + if (dependency.isOptional()) { + stringBuilder.append("[optional]"); + } + if (!dependency.getExclusions().isEmpty()) { + stringBuilder.append("[").append(dependency.getExclusions().size()).append(" exclusions]"); + } + return stringBuilder.toString(); + } + + private boolean equals(Dependency d1, Dependency d2) { + return Objects.equals(d1.getGroupId(), d2.getGroupId()) + && Objects.equals(d1.getArtifactId(), d2.getArtifactId()) + && Objects.equals(d1.getVersion(), d2.getVersion()) + && Objects.equals(d1.getType(), d2.getType()) + && Objects.equals(d1.getClassifier(), d2.getClassifier()) + && Objects.equals(d1.getScope(), d2.getScope()) + && Objects.equals(d1.getSystemPath(), d2.getSystemPath()) + && Objects.equals(d1.getOptional(), d2.getOptional()) + && equals(d1.getExclusions(), d2.getExclusions()); + } + + private boolean equals(Collection ce1, Collection ce2) { + if (ce1.size() == ce2.size()) { + Iterator i1 = ce1.iterator(); + Iterator i2 = ce2.iterator(); + while (i1.hasNext() && i2.hasNext()) { + if (!equals(i1.next(), i2.next())) { + return false; + } + } + return !i1.hasNext() && !i2.hasNext(); + } + return false; + } + + private boolean equals(Exclusion e1, Exclusion e2) { + return Objects.equals(e1.getGroupId(), e2.getGroupId()) + && Objects.equals(e1.getArtifactId(), e2.getArtifactId()); + } } diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java index 0c2d783496..4c98d88b50 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java @@ -30,8 +30,7 @@ import org.apache.maven.model.resolution.UnresolvableModelException; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.*; /** */ @@ -163,4 +162,229 @@ void testBuildRawModel() throws Exception { false); assertNotNull(res.get()); } + + /** + * This test has + * - a dependency directly managed to 0.2 + * - then a BOM import which manages that same dep to 0.1 + * This currently results in 0.2 and a no warning + */ + @Test + void testManagedDependencyBeforeImport() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-dep-first.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "test:import:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "test:mydep:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.2", dep.getVersion()); + assertEquals(0, result.getProblems().size()); + } + + /** + * This test has + * - a BOM import which manages the dep to 0.1 + * - then a directly managed dependency to 0.2 + * This currently results in 0.2 and no warning + */ + @Test + void testManagedDependencyAfterImport() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-dep-last.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "test:import:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "test:mydep:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.2", dep.getVersion()); + assertEquals(0, result.getProblems().size()); + } + + /** + * This test has + * - a BOM import which manages dep to 0.3 + * - another BOM import which manages the dep to 0.1 + * This currently results in 0.3 (first wins) and a warning + */ + @Test + void testManagedDependencyTwoImports() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-two-imports.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "test:import:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/import.xml") + .getFile())); + case "test:other:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/other-import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "test:mydep:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.3", dep.getVersion()); + assertEquals(1, result.getProblems().size()); + ModelProblem problem = result.getProblems().get(0); + assertTrue(problem.toString().contains("Ignored POM import")); + } + + /** + * This test has + * - a BOM import which itself imports the junit BOM which manages the dep to 0.1 + * - a BOM import to the junit BOM which manages the dep to 0.2 + * This currently results in 0.1 (first wins, unexpected as this is not the closest) and a warning + */ + @Test + void testManagedDependencyDistance() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setLocationTracking(true); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-distance.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "org.junit:bom:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/junit-" + dependency.getVersion() + ".xml") + .getFile())); + case "test:other:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/distant-import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "org.junit:junit:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.1", dep.getVersion()); + assertEquals(1, result.getProblems().size()); + ModelProblem problem = result.getProblems().get(0); + assertTrue(problem.toString().contains("Ignored POM import")); + } + + /** + * This test has + * - a BOM import which itself imports the junit BOM which manages the dep to 0.1 + * - a BOM import to the junit BOM which manages the dep to 0.2 + * - a direct managed dependency to the dep at 0.3 (as suggested by the warning) + * This results in 0.3 (explicit managed wins) and no warning + */ + @Test + void testManagedDependencyDistanceWithExplicit() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setLocationTracking(true); + request.setModelSource(new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/root-distance-explicit.xml") + .getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "org.junit:bom:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/junit-" + dependency.getVersion() + ".xml") + .getFile())); + case "test:other:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/distant-import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "org.junit:junit:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.3", dep.getVersion()); + assertEquals(0, result.getProblems().size()); + } } diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml b/maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml new file mode 100644 index 0000000000..2a19c7fed1 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml @@ -0,0 +1,23 @@ + + + 4.1.0 + + test + other + 0.1-SNAPSHOT + + + + + org.junit + bom + 0.1 + pom + import + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/import.xml b/maven-model-builder/src/test/resources/poms/depmgmt/import.xml new file mode 100644 index 0000000000..d4f2490cd3 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/import.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + test + import + 0.1-SNAPSHOT + pom + + + + + test + mydep + 0.1 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml new file mode 100644 index 0000000000..344fc69a97 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + org.junit + bom + 0.1 + pom + + + + + org.junit + junit + 0.1 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml new file mode 100644 index 0000000000..58db6d80f8 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + org.junit + bom + 0.2 + pom + + + + + org.junit + junit + 0.2 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml b/maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml new file mode 100644 index 0000000000..b110091a03 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + test + other-import + 0.1-SNAPSHOT + pom + + + + + test + mydep + 0.3 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml new file mode 100644 index 0000000000..eb57c4549c --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml @@ -0,0 +1,28 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + mydep + 0.2 + + + test + import + 0.1-SNAPSHOT + pom + import + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml new file mode 100644 index 0000000000..bac146e50e --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml @@ -0,0 +1,28 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + import + 0.1-SNAPSHOT + pom + import + + + test + mydep + 0.2 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml new file mode 100644 index 0000000000..737cfd7fab --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml @@ -0,0 +1,35 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + other + 0.1-SNAPSHOT + pom + import + + + org.junit + bom + 0.2 + pom + import + + + org.junit + junit + 0.3 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml new file mode 100644 index 0000000000..d56bbee188 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml @@ -0,0 +1,30 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + other + 0.1-SNAPSHOT + pom + import + + + org.junit + bom + 0.2 + pom + import + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml new file mode 100644 index 0000000000..198a885610 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml @@ -0,0 +1,30 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + other + 0.1-SNAPSHOT + pom + import + + + test + import + 0.1-SNAPSHOT + pom + import + + + + +