[MRESOLVER-357] Fix the code to deliver the promise in comment (apach…

…e#283) The check was wrong, it was "has ANY sibling", while it should be "has ANY RELATED sibling". --- https://issues.apache.org/jira/browse/MRESOLVER-357
larisabratean · May 2, 2023 · e2675b3 · e2675b3
1 parent b4cda77
commit e2675b3
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 1 deletion.
diff --git a/...solver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java b/...solver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java
@@ -356,7 +356,8 @@ private static void removeLosers(State state) {
                     DependencyNode child = childIt.next();
                     if (child == item.node) {
                         String childArtifactId = ArtifactIdUtils.toId(child.getArtifact());
-                        if (toRemoveIds.contains(childArtifactId) && item.parent.size() > 1) {
+                        if (toRemoveIds.contains(childArtifactId)
+                                && relatedSiblingsCount(child.getArtifact(), item.parent) > 1) {
                             childIt.remove();
                         }
                         break;
@@ -369,6 +370,14 @@ private static void removeLosers(State state) {
         // those will be nuked during future graph walks when we include the winner in the recursion
     }
 
+    private static long relatedSiblingsCount(Artifact artifact, List<DependencyNode> parent) {
+        String ga = artifact.getGroupId() + ":" + artifact.getArtifactId();
+        return parent.stream()
+                .map(DependencyNode::getArtifact)
+                .filter(a -> ga.equals(a.getGroupId() + ":" + a.getArtifactId()))
+                .count();
+    }
+
     static final class NodeInfo {
 
         /**

diff --git a/...er-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/...er-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java
@@ -278,6 +278,36 @@ public void versionRangeClashMixedOrderStandardVerbose() throws RepositoryExcept
         assertConflictedButSameAsOriginal(c2, b.getChildren().get(0));
     }
 
+    @Test
+    public void versionRangeClashMixedOrderStandardVerboseLeavesOne() throws RepositoryException {
+        // This is a bit different then others, is related to MRESOLVER-357 and makes sure that
+        // ConflictResolver fulfils the promise of "leaving 1 loser"
+        //
+        //  A -> B -> C[1..2]
+        //  |    \ -> D1
+        //  |--> C[1..2]
+        //  \--> D2
+        DependencyNode a = makeDependencyNode("some-group", "a", "1.0");
+        DependencyNode b = makeDependencyNode("some-group", "b", "1.0");
+        DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0");
+        DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0");
+        DependencyNode d1 = makeDependencyNode("some-group", "d", "1.0");
+        DependencyNode d2 = makeDependencyNode("some-group", "d", "2.0");
+        a.setChildren(mutableList(b, c2, c1, d2));
+        b.setChildren(mutableList(c1, c2, d1));
+
+        DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.STANDARD);
+
+        assertSame(a, ta);
+        assertEquals(3, a.getChildren().size());
+        assertSame(b, a.getChildren().get(0));
+        assertSame(c2, a.getChildren().get(1));
+        assertSame(d2, a.getChildren().get(2));
+        assertEquals(2, b.getChildren().size());
+        assertConflictedButSameAsOriginal(c2, b.getChildren().get(0));
+        assertConflictedButSameAsOriginal(d1, b.getChildren().get(1));
+    }
+
     @Test
     public void versionRangeClashMixedOrderFullVerbose() throws RepositoryException {
         //  A -> B -> C[1..2]