From 6b36b895562d46a1d868324f9fda8e230007035e Mon Sep 17 00:00:00 2001 From: Benjamin Bentmann Date: Sat, 29 Dec 2012 22:14:07 +0100 Subject: [PATCH] Bug 397169 - Conflict resolution transformer can remove dependency completely from tree Partially reverted original fix and restored sharing of child node lists along with docs to explicitly call it out, not only can the new conflict resolver handle this data sharing, it's actually improving performance --- .../aether/graph/DefaultDependencyNode.java | 5 ---- .../eclipse/aether/graph/DependencyNode.java | 12 +++++++++- .../impl/DefaultDependencyCollector.java | 4 ++-- .../impl/DefaultDependencyCollectorTest.java | 23 ------------------- .../diamond/test_a_1_jar.ini | 2 -- .../diamond/test_b_1_jar.ini | 2 -- .../diamond/test_x_1_jar.ini | 1 - 7 files changed, 13 insertions(+), 36 deletions(-) delete mode 100644 aether-impl/src/test/resources/artifact-descriptions/diamond/test_a_1_jar.ini delete mode 100644 aether-impl/src/test/resources/artifact-descriptions/diamond/test_b_1_jar.ini delete mode 100644 aether-impl/src/test/resources/artifact-descriptions/diamond/test_x_1_jar.ini diff --git a/aether-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java b/aether-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java index 6d5b1e5e..72537caf 100644 --- a/aether-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java +++ b/aether-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java @@ -112,11 +112,6 @@ public final class DefaultDependencyNode return children; } - /** - * Sets the child nodes of this node. - * - * @param children The child nodes, may be {@code null} - */ public void setChildren( List children ) { if ( children == null ) diff --git a/aether-api/src/main/java/org/eclipse/aether/graph/DependencyNode.java b/aether-api/src/main/java/org/eclipse/aether/graph/DependencyNode.java index 63cd61e4..c983c3ca 100644 --- a/aether-api/src/main/java/org/eclipse/aether/graph/DependencyNode.java +++ b/aether-api/src/main/java/org/eclipse/aether/graph/DependencyNode.java @@ -33,12 +33,22 @@ public interface DependencyNode { /** - * Gets the child nodes of this node. + * Gets the child nodes of this node. To conserve memory, dependency nodes with equal dependencies may share the + * same child list instance. Hence clients mutating the child list need to be aware that these changes might affect + * more than this node. Where this is not desired, the child list should be copied before mutation if the client + * cannot be sure whether it might be shared with other nodes in the graph. * * @return The child nodes of this node, never {@code null}. */ List getChildren(); + /** + * Sets the child nodes of this node. + * + * @param children The child nodes, may be {@code null} + */ + void setChildren( List children ); + /** * Gets the dependency associated with this node. Note: For dependency graphs that have been constructed * without a root dependency, this method will yield {@code null} when invoked on the graph's root node. The root diff --git a/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDependencyCollector.java b/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDependencyCollector.java index 9d5c9f5c..6f4b5613 100644 --- a/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDependencyCollector.java +++ b/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDependencyCollector.java @@ -454,7 +454,7 @@ public class DefaultDependencyCollector if ( cycleNode != null ) { DefaultDependencyNode child = new DefaultDependencyNode( d ); - child.setChildren( new ArrayList( cycleNode.getChildren() ) ); + child.setChildren( cycleNode.getChildren() ); child.setPremanagedScope( premanagedScope ); child.setPremanagedVersion( premanagedVersion ); child.setRelocations( relocations ); @@ -537,7 +537,7 @@ public class DefaultDependencyCollector } else { - child.setChildren( new ArrayList( children ) ); + child.setChildren( children ); } } } diff --git a/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultDependencyCollectorTest.java b/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultDependencyCollectorTest.java index d5fe152d..949e779e 100644 --- a/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultDependencyCollectorTest.java +++ b/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultDependencyCollectorTest.java @@ -24,7 +24,6 @@ import java.util.Map; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.artifact.ArtifactProperties; -import org.eclipse.aether.artifact.DefaultArtifact; import org.eclipse.aether.collection.CollectRequest; import org.eclipse.aether.collection.CollectResult; import org.eclipse.aether.collection.DependencyCollectionContext; @@ -385,28 +384,6 @@ public class DefaultDependencyCollectorTest assertEquals( "managed", dep( node, 0, 0 ).getArtifact().getProperty( ArtifactProperties.LOCAL_PATH, null ) ); } - @Test - public void testDiamondWithSharedSinkNode() - throws Exception - { - CollectRequest request = new CollectRequest(); - request.addDependency( new Dependency( new DefaultArtifact( "test:a:1" ), "compile" ) ); - request.addDependency( new Dependency( new DefaultArtifact( "test:b:1" ), "compile" ) ); - request.addRepository( repository ); - collector.setArtifactDescriptorReader( new IniArtifactDescriptorReader( "artifact-descriptions/diamond/" ) ); - CollectResult result = collector.collectDependencies( session, request ); - DependencyNode root = result.getRoot(); - assertNotNull( root ); - assertEquals( 2, root.getChildren().size() ); - assertEquals( 1, root.getChildren().get( 0 ).getChildren().size() ); - assertEquals( 1, root.getChildren().get( 1 ).getChildren().size() ); - assertSame( root.getChildren().get( 0 ).getChildren().get( 0 ), - root.getChildren().get( 1 ).getChildren().get( 0 ) ); - root.getChildren().get( 0 ).getChildren().remove( 0 ); - assertEquals( 0, root.getChildren().get( 0 ).getChildren().size() ); - assertEquals( 1, root.getChildren().get( 1 ).getChildren().size() ); - } - /* */ public class TestDependencyManager implements DependencyManager diff --git a/aether-impl/src/test/resources/artifact-descriptions/diamond/test_a_1_jar.ini b/aether-impl/src/test/resources/artifact-descriptions/diamond/test_a_1_jar.ini deleted file mode 100644 index df1ec140..00000000 --- a/aether-impl/src/test/resources/artifact-descriptions/diamond/test_a_1_jar.ini +++ /dev/null @@ -1,2 +0,0 @@ -[dependencies] -test:x:jar:1 diff --git a/aether-impl/src/test/resources/artifact-descriptions/diamond/test_b_1_jar.ini b/aether-impl/src/test/resources/artifact-descriptions/diamond/test_b_1_jar.ini deleted file mode 100644 index 4d6e5c7b..00000000 --- a/aether-impl/src/test/resources/artifact-descriptions/diamond/test_b_1_jar.ini +++ /dev/null @@ -1,2 +0,0 @@ -[relocation] -test:a:jar:1 diff --git a/aether-impl/src/test/resources/artifact-descriptions/diamond/test_x_1_jar.ini b/aether-impl/src/test/resources/artifact-descriptions/diamond/test_x_1_jar.ini deleted file mode 100644 index 61a252c2..00000000 --- a/aether-impl/src/test/resources/artifact-descriptions/diamond/test_x_1_jar.ini +++ /dev/null @@ -1 +0,0 @@ -[dependencies] -- GitLab