elharo commented on code in PR #281:
URL: https://github.com/apache/maven/pull/281#discussion_r3927131041
##########
impl/maven-core/src/test/java/org/apache/maven/RepositoryUtilsTest.java:
##########
@@ -18,11 +18,23 @@
*/
package org.apache.maven;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
import org.apache.maven.artifact.Artifact;
import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyFilter;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.internal.test.util.DependencyGraphParser;
Review Comment:
shouldn't depend on internal class from another package
##########
impl/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java:
##########
@@ -65,6 +68,50 @@
*/
public class RepositoryUtils {
+ @FunctionalInterface
+ private interface ArtifactOrderingStrategy {
+ void apply(
+ Collection<org.apache.maven.artifact.Artifact> artifacts,
+ Collection<? extends DependencyNode> nodes,
+ List<String> trail,
+ DependencyFilter filter);
+ }
+
+ public enum ArtifactOrdering {
+ /**
+ * Depth-first traversal of the dependency graph to convert nodes to
artifacts. This is the original behavior.
+ */
+ DFS(RepositoryUtils::toArtifactsDFS),
+ /**
+ * Breadth-first traversal of the dependency graph to convert nodes to
artifacts, using TreeMap to store artifacts by depth. This is the new behavior.
+ */
+ BFS(RepositoryUtils::toArtifactsBFS),
+ /**
+ * Breadth-first traversal of the dependency graph, using a List of
List to store artifacts by depth, and a stack to keep the dependency trail.
This is the new behavior.
Review Comment:
list of lists
##########
impl/maven-core/src/test/java/org/apache/maven/RepositoryUtilsTest.java:
##########
@@ -32,4 +44,113 @@ void
testToArtifactMethodsReturnNullWhenInputParameterIsNull() {
assertNull(RepositoryUtils.toArtifact((Artifact) null));
assertNull(RepositoryUtils.toArtifact((org.apache.maven.artifact.Artifact)
null));
}
+
+ @ParameterizedTest
+ @EnumSource(
+ value = RepositoryUtils.ArtifactOrdering.class,
+ names = {"BFS", "BFS2"})
+ public void
testToArtifactsCollectionOrderedByNodeDepth(RepositoryUtils.ArtifactOrdering
ordering) {
+ RepositoryUtils.setArtifactOrdering(ordering);
+
+ List<org.apache.maven.artifact.Artifact> artifacts = new ArrayList<>();
+
+ DependencyNode root = createDependencyTree();
+
+ List<String> trail = new ArrayList<>();
+ DependencyFilter filter = null;
+
+ RepositoryUtils.toArtifacts(artifacts, root.getChildren(), trail,
filter);
+
+ String expected =
+ "[gid:zlevel1:jar:1:, gid:ylevel1:jar:1:, gid:xlevel1:jar:1:,
gid:alevel2:jar:1:, gid:blevel2:jar:1:, gid:clevel2:jar:1:,
gid:alevel3:jar:1:]";
+ assertEquals(expected, artifacts.toString());
+
+ String[][] expectedTrails = {
+ {"gid:zlevel1:jar:1"},
+ {"gid:ylevel1:jar:1"},
+ {"gid:xlevel1:jar:1"},
+ {"gid:zlevel1:jar:1", "gid:alevel2:jar:1"},
+ {"gid:ylevel1:jar:1", "gid:blevel2:jar:1"},
+ {"gid:xlevel1:jar:1", "gid:clevel2:jar:1"},
+ {"gid:ylevel1:jar:1", "gid:blevel2:jar:1", "gid:alevel3:jar:1"}
+ };
+
+ assertDependencyTrails(artifacts, expectedTrails);
+ }
+
+ @ParameterizedTest
+ @EnumSource(
+ value = RepositoryUtils.ArtifactOrdering.class,
+ names = {"BFS", "BFS2"})
+ public void
testToArtifactsCollectionOrderedByNodeDepthWithFilter(RepositoryUtils.ArtifactOrdering
ordering) {
+ RepositoryUtils.setArtifactOrdering(ordering);
+
+ List<org.apache.maven.artifact.Artifact> artifacts = new ArrayList<>();
+
+ DependencyNode root = createDependencyTree();
+
+ List<String> trail = new ArrayList<>();
+ DependencyFilter filter = new DependencyFilter() {
+ @Override
+ public boolean accept(DependencyNode node, List<DependencyNode>
parents) {
+ // accept node if artifactId does NOT contain "level2"
+ return !node.getArtifact().getArtifactId().contains("level2");
+ }
+ };
+
+ RepositoryUtils.toArtifacts(artifacts, root.getChildren(), trail,
filter);
+
+ String expected = "[gid:zlevel1:jar:1:, gid:ylevel1:jar:1:,
gid:xlevel1:jar:1:, gid:alevel3:jar:1:]";
+ assertEquals(expected, artifacts.toString());
+
+ String[][] expectedTrails = {
+ {"gid:zlevel1:jar:1"},
+ {"gid:ylevel1:jar:1"},
+ {"gid:xlevel1:jar:1"},
+ {"gid:ylevel1:jar:1", "gid:blevel2:jar:1", "gid:alevel3:jar:1"}
+ };
+
+ assertDependencyTrails(artifacts, expectedTrails);
+ }
+
+ private void assertDependencyTrails(List<Artifact> artifacts, String[][]
expectedTrails) {
+ assertEquals(
+ expectedTrails.length,
+ artifacts.size(),
+ "Expected " + expectedTrails.length + " artifacts but got " +
artifacts.size());
+
+ for (int i = 0; i < artifacts.size(); i++) {
+ Artifact artifact = artifacts.get(i);
+ assertIterableEquals(
+ List.of(expectedTrails[i]),
+ artifact.getDependencyTrail(),
+ "Wrong dependency trail for artifact at index " + i + " ("
+ artifact.getId() + ")");
+ }
+ }
+
+ /**
+ * Create a dependency tree.
+ *
+ * @return the root node or the tree.
+ */
+ private DependencyNode createDependencyTree() {
+ DependencyGraphParser parser = new DependencyGraphParser();
+ String dependencyGraph = """
+ gid:root:1
+ +- gid:zlevel1:1
+ | \\- gid:alevel2:1
+ \\- gid:ylevel1:1
+ | \\- gid:blevel2:1
+ | \\- gid:alevel3:1
+ \\- gid:xlevel1:1
+ \\- gid:clevel2:1
+ """;
+
+ try {
+ return parser.parseLiteral(dependencyGraph);
+ } catch (IOException e) {
+ fail("Failed the parsing of the dependency node graph");
Review Comment:
Failed to parse the
##########
impl/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java:
##########
@@ -132,6 +187,97 @@ public static void toArtifacts(
}
}
+ private static void toArtifactsBFS(
Review Comment:
why do we have several different BFS methods? what's different about them?
Better names or comments or both are needed.
##########
impl/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java:
##########
@@ -132,6 +187,97 @@ public static void toArtifacts(
}
}
+ private static void toArtifactsBFS(
+ Collection<org.apache.maven.artifact.Artifact> artifacts,
+ Collection<? extends DependencyNode> nodes,
+ List<String> trail,
+ DependencyFilter filter) {
+ Map<Integer, List<org.apache.maven.artifact.Artifact>>
artifactsByDepth = new TreeMap<>();
+
+ List<org.apache.maven.artifact.Artifact> firstLevelArtifacts = new
ArrayList<>(nodes.size());
+ // we know there are at least direct dependencies
+ artifactsByDepth.put(1, firstLevelArtifacts);
+
+ toArtifactsBFS(artifactsByDepth, 1, nodes, trail, filter);
+
+ // list of artifacts ordered by depth level
+ for (List<org.apache.maven.artifact.Artifact> artifactsInDepth :
artifactsByDepth.values()) {
+ artifacts.addAll(artifactsInDepth);
+ }
+ }
+
+ private static void toArtifactsBFS(
+ Map<Integer, List<org.apache.maven.artifact.Artifact>>
artifactsByDepth,
+ int currentDepth,
+ Collection<? extends DependencyNode> nodes,
+ List<String> trail,
+ DependencyFilter filter) {
+ for (DependencyNode node : nodes) {
+ org.apache.maven.artifact.Artifact artifact =
toArtifact(node.getDependency());
+
+ List<String> nodeTrail = new ArrayList<>(trail.size() + 1);
+ nodeTrail.addAll(trail);
+ nodeTrail.add(artifact.getId());
+
+ if (filter == null || filter.accept(node,
Collections.emptyList())) {
+ artifact.setDependencyTrail(nodeTrail);
+
+ // add artifact in the list of the current depth
+ List<org.apache.maven.artifact.Artifact> artifactsCurrentDepth
= artifactsByDepth.get(currentDepth);
+ if (artifactsCurrentDepth == null) {
+ artifactsCurrentDepth = new ArrayList<>();
+ artifactsByDepth.put(currentDepth, artifactsCurrentDepth);
+ }
+ artifactsCurrentDepth.add(artifact);
+ }
+
+ toArtifactsBFS(artifactsByDepth, currentDepth + 1,
node.getChildren(), nodeTrail, filter);
+ }
+ }
+
+ public static void toArtifactsBFS2(
Review Comment:
This is public and the others are private?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
