always use the nearest as that is the defined strategy for now. If we change the strategy to newest, a different approach is needed than just adjusting the node range as it doesn't take into account the right children.

git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@280489 13f79535-47bb-0310-9956-ffa450edef68

maven-artifact/src/main/java/org/apache/maven/artifact/resolver/DefaultArtifactCollector.java

@@ -137,9 +137,10 @@ public class DefaultArtifactCollector
             }
             else
             {
-                VersionRange newRange = previousRange.restrict( currentRange );
+                // TODO: shouldn't need to double up on this work, only done for simplicity of handling recommended
-                previous.getArtifact().setVersionRange( newRange );
+                // version but the restriction is identical
-                node.getArtifact().setVersionRange( newRange );
+                previous.getArtifact().setVersionRange( previousRange.restrict( currentRange ) );
+                node.getArtifact().setVersionRange( currentRange.restrict( previousRange ) );
             }
 
             // previous one is more dominant

maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java

@@ -185,8 +185,6 @@ public class VersionRange
 
     public VersionRange restrict( VersionRange restriction )
     {
-        ArtifactVersion version = max( recommendedVersion, restriction.getRecommendedVersion() );
-
         List r1 = this.restrictions;
         List r2 = restriction.restrictions;
         List restrictions;
@@ -199,24 +197,33 @@ public class VersionRange
             restrictions = intersection( r1, r2 );
         }
 
-        if ( version != null && restrictions.size() > 0 )
+        ArtifactVersion version = null;
+        if ( restrictions.size() > 0 )
         {
             boolean found = false;
             for ( Iterator i = restrictions.iterator(); i.hasNext() && !found; )
             {
                 Restriction r = (Restriction) i.next();
 
-                if ( r.containsVersion( version ) )
+                if ( recommendedVersion != null && r.containsVersion( recommendedVersion ) )
                 {
+                    // if we find the original, use that
+                    version = recommendedVersion;
                     found = true;
                 }
-            }
+                else if ( version == null && restriction.getRecommendedVersion() != null &&
-
+                    r.containsVersion( restriction.getRecommendedVersion() ) )
-            if ( !found )
                 {
-                version = null;
+                    // use this if we can, but prefer the original if possible
+                    version = restriction.getRecommendedVersion();
                 }
             }
+        }
+        else
+        {
+            // no range, so the recommended version is valid
+            version = recommendedVersion;
+        }
 
         return new VersionRange( version, restrictions );
     }
@@ -504,7 +511,7 @@ public class VersionRange
         return matched;
     }
 
-    private boolean containsVersion( ArtifactVersion version )
+    public boolean containsVersion( ArtifactVersion version )
     {
         boolean matched = false;
         for ( Iterator i = restrictions.iterator(); i.hasNext() && !matched; )

maven-artifact/src/test/java/org/apache/maven/artifact/resolver/DefaultArtifactCollectorTest.java

@@ -123,7 +123,7 @@ public class DefaultArtifactCollectorTest
         assertEquals( "Check artifact list", createSet( new Object[]{a.artifact, c.artifact} ), res.getArtifacts() );
     }
 
-    public void testResolveNearest()
+    public void testResolveNearestNewestIsNearest()
         throws ArtifactResolutionException
     {
         ArtifactSpec a = createArtifact( "a", "1.0" );
@@ -138,6 +138,21 @@ public class DefaultArtifactCollectorTest
         assertEquals( "Check version", "3.0", getArtifact( "c", res.getArtifacts() ).getVersion() );
     }
 
+    public void testResolveNearestOldestIsNearest()
+        throws ArtifactResolutionException
+    {
+        ArtifactSpec a = createArtifact( "a", "1.0" );
+        ArtifactSpec b = a.addDependency( "b", "1.0" );
+        ArtifactSpec c = a.addDependency( "c", "2.0" );
+
+        b.addDependency( "c", "3.0" );
+
+        ArtifactResolutionResult res = collect( a );
+        assertEquals( "Check artifact list", createSet( new Object[]{a.artifact, b.artifact, c.artifact} ),
+                      res.getArtifacts() );
+        assertEquals( "Check version", "2.0", getArtifact( "c", res.getArtifacts() ).getVersion() );
+    }
+
     public void testResolveNearestWithRanges()
         throws ArtifactResolutionException
     {

maven-artifact/src/test/java/org/apache/maven/artifact/versioning/VersionRangeTest.java

@@ -145,7 +145,9 @@ public class VersionRangeTest
         VersionRange range1 = VersionRange.createFromVersionSpec( "1.0" );
         VersionRange range2 = VersionRange.createFromVersionSpec( "1.1" );
         VersionRange mergedRange = range1.restrict( range2 );
-        assertEquals( CHECK_VERSION_RECOMMENDATION, "1.1", mergedRange.getRecommendedVersion().toString() );
+        // TODO: current policy is to retain the original version - is this correct, do we need strategies or is that handled elsewhere?
+//        assertEquals( CHECK_VERSION_RECOMMENDATION, "1.1", mergedRange.getRecommendedVersion().toString() );
+        assertEquals( CHECK_VERSION_RECOMMENDATION, "1.0", mergedRange.getRecommendedVersion().toString() );
         List restrictions = mergedRange.getRestrictions();
         assertEquals( CHECK_NUM_RESTRICTIONS, 1, restrictions.size() );
         Restriction restriction = (Restriction) restrictions.get( 0 );