introduce parameter to control the skipper benavior

maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DependencyResolutionSkipper.java

@@ -0,0 +1,57 @@
+package org.eclipse.aether.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.graph.DependencyNode;
+
+import java.util.List;
+
+/**
+ * A skipper that determines whether to skip resolving given node during the dependency collection.
+ *
+ * @noimplement This interface is not intended to be implemented by clients.
+ * @noextend This interface is not intended to be extended by clients.
+ * @provisional This type is provisional and can be changed, moved or removed without prior notice.
+ */
+public interface DependencyResolutionSkipper
+{
+    /**
+     * Check whether the resolution of current node can be skipped before resolving.
+     *
+     * @param node    Current node
+     * @param parents All parent nodes of current node
+     * @return
+     */
+    boolean skipResolution( DependencyNode node, List<DependencyNode> parents );
+
+    /**
+     * Cache the resolution result when a node is resolved by @See DependencyCollector after resolution.
+     *
+     * @param node    Current node
+     * @param parents All parent nodes of current node
+     */
+    void cache( DependencyNode node, List<DependencyNode> parents );
+
+    /**
+     * Print the skip/resolve status report for all nodes.
+     */
+    void report();
+
+}

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java

@@ -58,6 +58,7 @@
 import org.eclipse.aether.graph.Exclusion;
 import org.eclipse.aether.impl.ArtifactDescriptorReader;
 import org.eclipse.aether.impl.DependencyCollector;
+import org.eclipse.aether.impl.DependencyResolutionSkipper;
 import org.eclipse.aether.impl.RemoteRepositoryManager;
 import org.eclipse.aether.impl.VersionRangeResolver;
 import org.eclipse.aether.repository.ArtifactRepository;
@@ -85,6 +86,21 @@ public class DefaultDependencyCollector
     implements DependencyCollector, Service
 {
 
+    /**
+     * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties()
+     * configuration properties} used to store a {@link Boolean} flag controlling the resolver's skip mode.
+     *
+     * @since 1.7.3
+     */
+    public static final String CONFIG_PROP_USE_SKIP = "aether.dependencyCollector.useSkip";
+
+    /**
+     * The default value for {@link #CONFIG_PROP_USE_SKIP}, {@code true}.
+     *
+     * @since 1.7.3
+     */
+    public static final boolean CONFIG_PROP_USE_SKIP_DEFAULT = true;
+
     private static final String CONFIG_PROP_MAX_EXCEPTIONS = "aether.dependencyCollector.maxExceptions";
 
     private static final int CONFIG_PROP_MAX_EXCEPTIONS_DEFAULT = 50;
@@ -151,6 +167,14 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle
         requireNonNull( request, "request cannot be null" );
         session = optimizeSession( session );
 
+        boolean useSkip = ConfigUtils.getBoolean(
+                session, CONFIG_PROP_USE_SKIP_DEFAULT, CONFIG_PROP_USE_SKIP
+        );
+        if ( useSkip )
+        {
+            LOGGER.debug( "Collector skip mode enabled" );
+        }
+
         RequestTrace trace = RequestTrace.newChild( request.getTrace(), request );
 
         CollectResult result = new CollectResult( request );
@@ -255,7 +279,8 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle
 
             Args args =
                     new Args( session, trace, pool, context, versionContext, request,
-                            new DependencyResolutionSkipper() );
+                            useSkip ? new DefaultDependencyResolutionSkipper()
+                                    : NeverDependencyResolutionSkipper.INSTANCE );
             Results results = new Results( result, session );
 
             DependencySelector rootDepSelector =

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolutionSkipper.java → maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyResolutionSkipper.java

@@ -21,6 +21,7 @@
 
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.impl.DependencyResolutionSkipper;
 import org.eclipse.aether.util.artifact.ArtifactIdUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,40 +32,21 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
-final class DependencyResolutionSkipper
+final class DefaultDependencyResolutionSkipper implements DependencyResolutionSkipper
 {
-    private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolutionSkipper.class );
+    private static final Logger LOGGER = LoggerFactory.getLogger( DefaultDependencyResolutionSkipper.class );
 
     private Map<DependencyNode, DependencyResolutionResult> results = new LinkedHashMap<>( 256 );
     private CacheManager cacheManager = new CacheManager();
     private CoordinateManager coordinateManager = new CoordinateManager();
 
-    DependencyResolutionSkipper()
+    DefaultDependencyResolutionSkipper()
     {
         // enables default constructor
     }
 
-    void report()
-    {
-        if ( LOGGER.isTraceEnabled() )
-        {
-            LOGGER.trace( "Skipped {} nodes as duplicate",
-                    results.entrySet().stream().filter( n -> n.getValue().skippedAsDuplicate ).count() );
-            LOGGER.trace( "Skipped {} nodes as having version conflict",
-                    results.entrySet().stream().filter( n -> n.getValue().skippedAsVersionConflict ).count() );
-            LOGGER.trace( "Resolved {} nodes",
-                    results.entrySet().stream().filter( n -> n.getValue().resolve ).count() );
-            LOGGER.trace( "Forced resolving {} nodes for scope selection",
-                    results.entrySet().stream().filter( n -> n.getValue().forceResolution ).count() );
-        }
-    }
-
-    public Map<DependencyNode, DependencyResolutionResult> getResults()
-    {
-        return results;
-    }
-
-    boolean skipResolution( DependencyNode node, List<DependencyNode> parents )
+    @Override
+    public boolean skipResolution( DependencyNode node, List<DependencyNode> parents )
     {
         DependencyResolutionResult result = new DependencyResolutionResult( node );
         results.put( node, result );
@@ -132,7 +114,8 @@ else if ( cacheManager.isDuplicate( node ) )
         return true;
     }
 
-    void cache( DependencyNode node, List<DependencyNode> parents )
+    @Override
+    public void cache( DependencyNode node, List<DependencyNode> parents )
     {
         boolean parentForceResolution = parents.stream()
                 .anyMatch( n -> results.containsKey( n ) && results.get( n ).forceResolution );
@@ -151,6 +134,26 @@ void cache( DependencyNode node, List<DependencyNode> parents )
         }
     }
 
+    @Override
+    public void report()
+    {
+        if ( LOGGER.isTraceEnabled() )
+        {
+            LOGGER.trace( "Skipped {} nodes as duplicate",
+                    results.entrySet().stream().filter( n -> n.getValue().skippedAsDuplicate ).count() );
+            LOGGER.trace( "Skipped {} nodes as having version conflict",
+                    results.entrySet().stream().filter( n -> n.getValue().skippedAsVersionConflict ).count() );
+            LOGGER.trace( "Resolved {} nodes",
+                    results.entrySet().stream().filter( n -> n.getValue().resolve ).count() );
+            LOGGER.trace( "Forced resolving {} nodes for scope selection",
+                    results.entrySet().stream().filter( n -> n.getValue().forceResolution ).count() );
+        }
+    }
+
+    public Map<DependencyNode, DependencyResolutionResult> getResults()
+    {
+        return results;
+    }
 
     static final class DependencyResolutionResult
     {

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NeverDependencyResolutionSkipper.java

@@ -0,0 +1,51 @@
+package org.eclipse.aether.internal.impl.collect;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.impl.DependencyResolutionSkipper;
+
+import java.util.List;
+
+/**
+ * Skipper for Non-skip approach.
+ */
+final class NeverDependencyResolutionSkipper implements DependencyResolutionSkipper
+{
+    static final DependencyResolutionSkipper INSTANCE = new NeverDependencyResolutionSkipper();
+
+    @Override
+    public boolean skipResolution( DependencyNode node, List<DependencyNode> parents )
+    {
+        return false;
+    }
+
+    @Override
+    public void cache( DependencyNode node, List<DependencyNode> parents )
+    {
+
+    }
+
+    @Override
+    public void report()
+    {
+
+    }
+}

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java

@@ -69,7 +69,6 @@
 import org.eclipse.aether.util.graph.manager.DefaultDependencyManager;
 import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
 import org.eclipse.aether.util.graph.manager.TransitiveDependencyManager;
-import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
 import org.eclipse.aether.util.graph.version.HighestVersionFilter;
 import org.junit.Before;
 import org.junit.Test;
@@ -79,39 +78,39 @@
 public class DefaultDependencyCollectorTest
 {
 
-    private DefaultDependencyCollector collector;
+    protected DefaultDependencyCollector collector;
 
-    private DefaultRepositorySystemSession session;
+    protected DefaultRepositorySystemSession session;
 
-    private DependencyGraphParser parser;
+    protected DependencyGraphParser parser;
 
-    private RemoteRepository repository;
+    protected RemoteRepository repository;
 
-    private IniArtifactDescriptorReader newReader( String prefix )
+    protected IniArtifactDescriptorReader newReader( String prefix )
     {
         return new IniArtifactDescriptorReader( "artifact-descriptions/" + prefix );
     }
 
-    private Dependency newDep( String coords )
+    protected Dependency newDep( String coords )
     {
         return newDep( coords, "" );
     }
 
-    private Dependency newDep( String coords, String scope )
+    protected Dependency newDep( String coords, String scope )
     {
         return new Dependency( new DefaultArtifact( coords ), scope );
     }
 
-    private Dependency newDep( String coords, String scope, Collection<Exclusion> exclusions )
+    @Before
+    public void setup()
     {
-        Dependency d = new Dependency( new DefaultArtifact( coords ), scope );
-        return d.setExclusions( exclusions );
+        setupCollector(false);
     }
 
-    @Before
-    public void setup()
+    public void setupCollector(boolean useSkip)
     {
         session = TestUtils.newSession();
+        session.setConfigProperty(DefaultDependencyCollector.CONFIG_PROP_USE_SKIP, useSkip);
 
         collector = new DefaultDependencyCollector();
         collector.setArtifactDescriptorReader( newReader( "" ) );
@@ -161,12 +160,12 @@ private static void assertEqualSubtree( DependencyNode expected, DependencyNode
         parents.removeLast();
     }
 
-    private Dependency dep( DependencyNode root, int... coords )
+    protected Dependency dep( DependencyNode root, int... coords )
     {
         return path( root, coords ).getDependency();
     }
 
-    private DependencyNode path( DependencyNode root, int... coords )
+    protected DependencyNode path( DependencyNode root, int... coords )
     {
         try
         {
@@ -229,38 +228,6 @@ public void testMissingDependencyDescription()
         }
     }
 
-    @Test
-    public void testSkipperWithDifferentExclusion() throws DependencyCollectionException
-    {
-        collector.setArtifactDescriptorReader( newReader( "managed/" ) );
-        parser = new DependencyGraphParser( "artifact-descriptions/managed/" );
-        session.setDependencyManager( new TransitiveDependencyManager() );
-
-        ExclusionDependencySelector exclSel1 = new ExclusionDependencySelector();
-        session.setDependencySelector( exclSel1 );
-
-        Dependency root1 = newDep( "gid:root:ext:ver", "compile",
-                Collections.singleton( new Exclusion( "gid", "transitive-1", "", "ext" ) ) );
-        Dependency root2 = newDep( "gid:root:ext:ver", "compile",
-                Collections.singleton( new Exclusion( "gid", "transitive-2", "", "ext" ) ) );
-        List<Dependency> dependencies = Arrays.asList( root1, root2 );
-
-        CollectRequest request = new CollectRequest( dependencies, null, Arrays.asList( repository ) );
-        request.addManagedDependency( newDep( "gid:direct:ext:managed-by-dominant-request" ) );
-        request.addManagedDependency( newDep( "gid:transitive-1:ext:managed-by-root" ) );
-
-        CollectResult result = collector.collectDependencies( session, request );
-        assertEquals( 0, result.getExceptions().size() );
-        assertEquals( 2, result.getRoot().getChildren().size() );
-        assertEquals( root1, dep( result.getRoot(), 0 ) );
-        assertEquals( root2, dep( result.getRoot(), 1 ) );
-        //the winner has transitive-1 excluded
-        assertEquals( 1, path( result.getRoot(), 0 ).getChildren().size() );
-        assertEquals( 0, path( result.getRoot(), 0, 0 ).getChildren().size() );
-        //skipped
-        assertEquals( 0, path( result.getRoot(), 1 ).getChildren().size() );
-    }
-
     @Test
     public void testDuplicates()
         throws DependencyCollectionException