diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/resolver/DefaultArtifactCollector.java b/maven-artifact/src/main/java/org/apache/maven/artifact/resolver/DefaultArtifactCollector.java index 142caaaa31..4c65f02b55 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/resolver/DefaultArtifactCollector.java +++ b/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 ); - previous.getArtifact().setVersionRange( newRange ); - node.getArtifact().setVersionRange( newRange ); + // TODO: shouldn't need to double up on this work, only done for simplicity of handling recommended + // version but the restriction is identical + previous.getArtifact().setVersionRange( previousRange.restrict( currentRange ) ); + node.getArtifact().setVersionRange( currentRange.restrict( previousRange ) ); } // previous one is more dominant diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java index 9b2351c590..a3bc1f2036 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java +++ b/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,23 +197,32 @@ 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() ) ) + { + // use this if we can, but prefer the original if possible + version = restriction.getRecommendedVersion(); + } } - - if ( !found ) - { - version = null; - } + } + 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; ) diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/resolver/DefaultArtifactCollectorTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/resolver/DefaultArtifactCollectorTest.java index dc6b8f4032..b23837e03f 100644 --- a/maven-artifact/src/test/java/org/apache/maven/artifact/resolver/DefaultArtifactCollectorTest.java +++ b/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 { diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/VersionRangeTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/VersionRangeTest.java index a77822e949..ce82d9bdd3 100644 --- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/VersionRangeTest.java +++ b/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 );