[MNG-7854] Warn if imported dep is ignored

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;

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,15 +59,81 @@ public Model importManagement(
                 depMgmt = DependencyManagement.newInstance();
             }
 
+            Set<String> 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."));
+                    }
                 }
             }
 
             return target.withDependencyManagement(depMgmt.withDependencies(dependencies.values()));
         }
         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<Exclusion> ce1, Collection<Exclusion> ce2) {
+        if (ce1.size() == ce2.size()) {
+            Iterator<Exclusion> i1 = ce1.iterator();
+            Iterator<Exclusion> 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());
+    }
 }

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 <i>currently</i> 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 <i>currently</i> 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 <i>currently</i> 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 <i>currently</i> 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());
+    }
 }

maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.1.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd">
+    <modelVersion>4.1.0</modelVersion>
+
+    <groupId>test</groupId>
+    <artifactId>other</artifactId>
+    <version>0.1-SNAPSHOT</version>
+
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>org.junit</groupId>
+                <artifactId>bom</artifactId>
+                <version>0.1</version>
+                <type>pom</type>
+                <scope>import</scope>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
+</project>