Deduplicate filtered dependency graph (#2489)

* Deduplicate filtered dependency graph

* Add regression test

Author: alzimmermsft

maven-core/src/main/java/org/apache/maven/graph/FilteredProjectDependencyGraph.java

@@ -149,7 +149,24 @@ private List<MavenProject> applyFilter(
                 filtered.addAll(upstream ? getUpstreamProjects(project, false) : getDownstreamProjects(project, false));
             }
         }
-        return filtered;
+        if (filtered.isEmpty() || filtered.size() == 1) {
+            // Optimization to skip streaming, distincting, and collecting to a new list when there is zero or one
+            // project, aka there can't be duplicates.
+            return filtered;
+        }
+
+        // Distinct the projects to avoid duplicates.  Duplicates are possible in multi-module projects.
+        //
+        // Given a scenario where there is an aggregate POM with modules A, B, C, D, and E and project E depends on
+        // A, B, C, and D. If the aggregate POM is being filtered for non-transitive and downstream dependencies where
+        // only A, C, and E are whitelisted duplicates will occur. When scanning projects A, C, and E, those will be
+        // added to 'filtered' as they are whitelisted. When scanning B and D, they are not whitelisted, and since
+        // transitive is false, their downstream dependencies will be added to 'filtered'. E is a downstream dependency
+        // of A, B, C, and D, so when scanning B and D, E will be added again 'filtered'.
+        //
+        // Without de-duplication, the final list would contain E three times, once for E being in the projects and
+        // whitelisted, and twice more for E being a downstream dependency of B and D.
+        return filtered.stream().distinct().collect(Collectors.toList());
     }
 
     @Override

maven-core/src/test/java/org/apache/maven/graph/DefaultProjectDependencyGraphTest.java

@@ -39,6 +39,13 @@ public class DefaultProjectDependencyGraphTest extends TestCase {
 
     private final MavenProject cProject = createProject(Arrays.asList(toDependency(bProject)), "cProject");
 
+    private final MavenProject dProject = createProject(
+        Arrays.asList(toDependency(aProject), toDependency(bProject), toDependency(cProject)), "dProject");
+
+    private final MavenProject eProject = createProject(
+        Arrays.asList(toDependency(aProject), toDependency(bProject), toDependency(cProject), toDependency(dProject)),
+        "eProject");
+
     private final MavenProject depender1 = createProject(Arrays.asList(toDependency(aProject)), "depender1");
 
     private final MavenProject depender2 = createProject(Arrays.asList(toDependency(aProject)), "depender2");
@@ -61,6 +68,37 @@ public void testNonTransitiveFiltering() throws DuplicateProjectException, Cycle
         assertTrue(graph.getDownstreamProjects(aProject, false).contains(cProject));
     }
 
+    // Test verifying that getDownstreamProjects does not contain duplicates.
+    // This is a regression test for https://github.com/apache/maven/issues/2487.
+    //
+    // The graph is:
+    // aProject -> bProject
+    //        | -> dProject
+    //        | -> eProject
+    // bProject -> cProject
+    //        | -> dProject
+    //        | -> eProject
+    // cProject -> dProject
+    //        | -> eProject
+    // dProject -> eProject
+    //
+    // When getting the non-transitive, downstream projects of aProject with a whitelist of aProject, dProject,
+    // and eProject, we expect to get dProject, and eProject with no duplicates.
+    // Before the fix, this would return dProject and eProject twice, once from bProject and once from cProject. As
+    // aProject is whitelisted, it should not be returned as a downstream project for itself. bProject and cProject
+    // are not whitelisted, so they should return their downstream projects, both have dProject and eProject as
+    // downstream projects. Which would result in dProject and eProject being returned twice, but now the results are
+    // made unique.
+    public void testGetDownstreamDoesNotDuplicateProjects() throws CycleDetectedException, DuplicateProjectException {
+        ProjectDependencyGraph graph = new DefaultProjectDependencyGraph(
+            Arrays.asList(aProject, bProject, cProject, dProject, eProject));
+        graph = new FilteredProjectDependencyGraph(graph, Arrays.asList(aProject, dProject, eProject));
+        final List<MavenProject> downstreamProjects = graph.getDownstreamProjects(aProject, false);
+        assertEquals(2, downstreamProjects.size());
+        assertTrue(downstreamProjects.contains(dProject));
+        assertTrue(downstreamProjects.contains(eProject));
+    }
+
     public void testGetSortedProjects() throws DuplicateProjectException, CycleDetectedException {
         ProjectDependencyGraph graph = new DefaultProjectDependencyGraph(Arrays.asList(depender1, aProject));
         final List<MavenProject> sortedProjects = graph.getSortedProjects();