[MRESOLVER-141] Review index-based access to collections

This closes #76

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

@@ -27,6 +27,8 @@
 import java.util.IdentityHashMap;
 import java.util.List;
 import static java.util.Objects.requireNonNull;
+
+import java.util.ListIterator;
 import java.util.Set;
 
 import javax.inject.Inject;
@@ -257,16 +259,16 @@ private DeployResult deploy( SyncContext syncContext, RepositorySystemSession se
                 processedMetadata.put( metadata, null );
             }
 
-            for ( int i = 0; i < artifacts.size(); i++ )
+            for ( ListIterator<Artifact> iterator = artifacts.listIterator(); iterator.hasNext(); )
             {
-                Artifact artifact = artifacts.get( i );
+                Artifact artifact = iterator.next();
 
                 for ( MetadataGenerator generator : generators )
                 {
                     artifact = generator.transformArtifact( artifact );
                 }
 
-                artifacts.set( i, artifact );
+                iterator.set( artifact );
 
                 Collection<FileTransformer> fileTransformers =
                         fileTransformerManager.getTransformersForArtifact( artifact );

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

@@ -27,6 +27,7 @@
 import java.util.Collection;
 import java.util.IdentityHashMap;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Set;
 
 import javax.inject.Inject;
@@ -170,16 +171,16 @@ private InstallResult install( SyncContext syncContext, RepositorySystemSession
             result.addMetadata( metadata );
         }
 
-        for ( int i = 0; i < artifacts.size(); i++ )
+        for ( ListIterator<Artifact> iterator = artifacts.listIterator(); iterator.hasNext(); )
         {
-            Artifact artifact = artifacts.get( i );
+            Artifact artifact = iterator.next();
 
             for ( MetadataGenerator generator : generators )
             {
                 artifact = generator.transformArtifact( artifact );
             }
 
-            artifacts.set( i, artifact );
+            iterator.set( artifact );
 
             install( session, trace, artifact );
             result.addArtifact( artifact );

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

@@ -133,13 +133,13 @@ public List<PrioritizedComponent<T>> getEnabled()
 
     public void list( StringBuilder buffer )
     {
-        for ( int i = 0; i < components.size(); i++ )
+        int i = 0;
+        for ( PrioritizedComponent<?> component : components )
         {
-            if ( i > 0 )
+            if ( i++ > 0 )
             {
                 buffer.append( ", " );
             }
-            PrioritizedComponent<?> component = components.get( i );
             buffer.append( component.getType().getSimpleName() );
             if ( component.isDisabled() )
             {

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

@@ -22,6 +22,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -320,10 +321,11 @@ private static boolean equals( List<RemoteRepository> repos1, List<RemoteReposit
             {
                 return false;
             }
-            for ( int i = 0, n = repos1.size(); i < n; i++ )
+            for ( Iterator<RemoteRepository> it1 = repos1.iterator(), it2 = repos2.iterator();
+                  it1.hasNext() && it2.hasNext(); )
             {
-                RemoteRepository repo1 = repos1.get( i );
-                RemoteRepository repo2 = repos2.get( i );
+                RemoteRepository repo1 = it1.next();
+                RemoteRepository repo2 = it2.next();
                 if ( repo1.isRepositoryManager() != repo2.isRepositoryManager() )
                 {
                     return false;

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

@@ -73,13 +73,14 @@ public List<Dependency> getCyclicDependencies()
     public String toString()
     {
         StringBuilder buffer = new StringBuilder( 256 );
-        for ( int i = 0, n = dependencies.size(); i < n; i++ )
+        int i = 0;
+        for ( Dependency dependency : dependencies )
         {
-            if ( i > 0 )
+            if ( i++ > 0 )
             {
                 buffer.append( " -> " );
             }
-            buffer.append( ArtifactIdUtils.toVersionlessId( dependencies.get( i ).getArtifact() ) );
+            buffer.append( ArtifactIdUtils.toVersionlessId( dependency.getArtifact() ) );
         }
         return buffer.toString();
     }

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

@@ -21,10 +21,8 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 import java.util.List;
-import java.util.NoSuchElementException;
 
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.collection.VersionFilter;
@@ -47,9 +45,7 @@ final class DefaultVersionFilterContext
 
     VersionRangeResult result;
 
-    int count;
-
-    byte[] deleted = new byte[64];
+    private List<Version> versions;
 
     DefaultVersionFilterContext( RepositorySystemSession session )
     {
@@ -60,40 +56,12 @@ public void set( Dependency dependency, VersionRangeResult result )
     {
         this.dependency = dependency;
         this.result = result;
-        count = result.getVersions().size();
-        if ( deleted.length < count )
-        {
-            deleted = new byte[count];
-        }
-        else
-        {
-            for ( int i = count - 1; i >= 0; i-- )
-            {
-                deleted[i] = (byte) 0;
-            }
-        }
+        this.versions = new ArrayList<>( result.getVersions() );
     }
 
     public List<Version> get()
     {
-        if ( count == result.getVersions().size() )
-        {
-            return result.getVersions();
-        }
-        if ( count <= 1 )
-        {
-            if ( count <= 0 )
-            {
-                return Collections.emptyList();
-            }
-            return Collections.singletonList( iterator().next() );
-        }
-        List<Version> versions = new ArrayList<>( count );
-        for ( Version version : this )
-        {
-            versions.add( version );
-        }
-        return versions;
+        return new ArrayList<>( versions );
     }
 
     public RepositorySystemSession getSession()
@@ -113,7 +81,7 @@ public VersionConstraint getVersionConstraint()
 
     public int getCount()
     {
-        return count;
+        return versions.size();
     }
 
     public ArtifactRepository getRepository( Version version )
@@ -128,88 +96,12 @@ public List<RemoteRepository> getRepositories()
 
     public Iterator<Version> iterator()
     {
-        return ( count > 0 ) ? new VersionIterator() : Collections.<Version>emptySet().iterator();
+        return versions.iterator();
     }
 
     @Override
     public String toString()
     {
         return dependency + " " + result.getVersions();
     }
-
-    private class VersionIterator
-        implements Iterator<Version>
-    {
-
-        private final List<Version> versions;
-
-        private final int size;
-
-        private int count;
-
-        private int index;
-
-        private int next;
-
-        VersionIterator()
-        {
-            count = DefaultVersionFilterContext.this.count;
-            index = -1;
-            next = 0;
-            versions = result.getVersions();
-            size = versions.size();
-            advance();
-        }
-
-        @SuppressWarnings( "StatementWithEmptyBody" )
-        private void advance()
-        {
-            for ( next = index + 1; next < size && deleted[next] != (byte) 0; next++ )
-            {
-                // just advancing index
-            }
-        }
-
-        public boolean hasNext()
-        {
-            return next < size;
-        }
-
-        public Version next()
-        {
-            if ( count != DefaultVersionFilterContext.this.count )
-            {
-                throw new ConcurrentModificationException();
-            }
-            if ( next >= size )
-            {
-                throw new NoSuchElementException();
-            }
-            index = next;
-            advance();
-            return versions.get( index );
-        }
-
-        public void remove()
-        {
-            if ( count != DefaultVersionFilterContext.this.count )
-            {
-                throw new ConcurrentModificationException();
-            }
-            if ( index < 0 || deleted[index] == (byte) 1 )
-            {
-                throw new IllegalStateException();
-            }
-            deleted[index] = (byte) 1;
-            count = --DefaultVersionFilterContext.this.count;
-        }
-
-        @Override
-        public String toString()
-        {
-            return ( index < 0 ) ? "null" : String.valueOf( versions.get( index ) );
-        }
-
-    }
-
 }

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

@@ -23,6 +23,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -112,4 +113,20 @@ public void testAdd_Disabled()
         assertSame( comp1, sorted.get( 0 ).getComponent() );
         assertSame( comp2, sorted.get( 1 ).getComponent() );
     }
+
+    @Test
+    public void testList()
+    {
+        Exception comp1 = new IllegalArgumentException();
+        Exception comp2 = new NullPointerException();
+
+        PrioritizedComponents<Exception> components = new PrioritizedComponents<>( Collections.emptyMap() );
+        components.add( comp1, 1 );
+        components.add( comp2, 0 );
+
+        StringBuilder stringBuilder = new StringBuilder();
+        components.list( stringBuilder );
+
+        assertEquals( "IllegalArgumentException, NullPointerException", stringBuilder.toString() );
+    }
 }

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

@@ -25,8 +25,11 @@
 import org.eclipse.aether.repository.RemoteRepository;
 import org.eclipse.aether.resolution.ArtifactDescriptorRequest;
 import org.eclipse.aether.resolution.ArtifactDescriptorResult;
+import org.eclipse.aether.resolution.VersionRangeRequest;
 import org.junit.Test;
 
+import java.util.Collections;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
@@ -64,4 +67,19 @@ public void testArtifactDescriptorCaching()
         assertEquals( result.getAliases(), cached.getAliases() );
     }
 
+    @Test
+    public void testConstraintKey()
+    {
+        VersionRangeRequest request = new VersionRangeRequest();
+        request.setRepositories(
+            Collections.singletonList( new RemoteRepository.Builder( "some-id", "some-type", "http://www.example.com" ).build() )
+        );
+        request.setArtifact( new DefaultArtifact("group:artifact:1.0") );
+
+        DataPool pool = newDataPool();
+
+        Object key1 = pool.toKey( request );
+        Object key2 = pool.toKey( request );
+        assertEquals(key1, key2);
+    }
 }

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

@@ -0,0 +1,44 @@
+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.artifact.DefaultArtifact;
+import org.eclipse.aether.graph.DefaultDependencyNode;
+import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.graph.DependencyCycle;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class DefaultDependencyCycleTest
+{
+    private static final Dependency FOO_DEPENDENCY = new Dependency( new DefaultArtifact( "group-id:foo:1.0" ), "test" );
+    private static final Dependency BAR_DEPENDENCY = new Dependency( new DefaultArtifact( "group-id:bar:1.0" ), "test" );
+
+    @Test
+    public void testToString()
+    {
+        NodeStack nodeStack = new NodeStack();
+        nodeStack.push( new DefaultDependencyNode( FOO_DEPENDENCY ) );
+        DependencyCycle cycle = new DefaultDependencyCycle( nodeStack, 1, BAR_DEPENDENCY );
+
+        assertEquals( "group-id:foo:jar -> group-id:bar:jar", cycle.toString() );
+    }
+}