Skip to content

[MNG-7559] Fix versions comparison, backport to 3.8.x #925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sultan wants to merge 1 commit into from
Closed

[MNG-7559] Fix versions comparison, backport to 3.8.x #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 54 additions & 26 deletions maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,28 @@
* <code>1.0alpha1 =&gt; [1, 0, alpha, 1]</code></li>
* <li>unlimited number of version components,</li>
* <li>version components in the text can be digits or strings,</li>
* <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering.
* Well-known qualifiers (case insensitive) are:<ul>
* <li><code>alpha</code> or <code>a</code></li>
* <li><code>beta</code> or <code>b</code></li>
* <li><code>milestone</code> or <code>m</code></li>
* <li><code>rc</code> or <code>cr</code></li>
* <li><code>snapshot</code></li>
* <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li>
* <li><code>sp</code></li>
* </ul>
* Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive),
* </li>
* <li>
* Following semver rules is encouraged, and some qualifiers are discouraged (no matter the case):
* <ul>
* <li> The usage of 'CR' qualifier is discouraged. Use 'RC' instead. </li>
* <li> The usage of 'Final', 'GA', and 'Release' qualifiers is discouraged. Use no qualifier instead. </li>
* <li> The usage of 'SP' qualifier is discouraged. Increment the patch version instead. </li>
* </ul>
* String qualifiers are ordered lexically (case insensitive), with the following exceptions:
* <ul>
* <li><code> alpha = a &lt; beta = b &lt; milestone = m &lt; rc = cr
* &lt; snapshot &lt; '' = final = ga = release &lt; sp </code></li>
* <li>Other qualifiers are ordered lexically (case insensitive),
* and considered less than Snapshot and Release.</li>
* </ul>
* </li>
* <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
* {@code 1.0.RC2 < 1.0-RC3 < 1.0.1}; but prefer {@code 1.0.0-RC1} over {@code 1.0.0.RC1}, and more
* generally: {@code 1.0.X2 < 1.0-X3 < 1.0.1} for any string {@code X}; but prefer {@code 1.0.0-X1}
* over {@code 1.0.0.X1}.</li>
* </ul>
*
* @see <a href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning">"Versioning" on Maven Wiki</a>
* @see <a href="https://maven.apache.org/pom.html#Version_Order_Specification">Version Order Specification</a>
* @author <a href="mailto:kenney@apache.org">Kenney Westerhof</a>
* @author <a href="mailto:hboutemy@apache.org">Hervé Boutemy</a>
*/
Expand Down Expand Up @@ -135,7 +138,7 @@ public int compareTo( Item item )
{
case INT_ITEM:
int itemValue = ( (IntItem) item ).value;
return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
return Integer.compare( value, itemValue );
case LONG_ITEM:
case BIGINTEGER_ITEM:
return -1;
Expand Down Expand Up @@ -221,7 +224,7 @@ public int compareTo( Item item )
return 1;
case LONG_ITEM:
long itemValue = ( (LongItem) item ).value;
return ( value < itemValue ) ? -1 : ( ( value == itemValue ) ? 0 : 1 );
return Long.compare( value, itemValue );
case BIGINTEGER_ITEM:
return -1;

Expand Down Expand Up @@ -356,23 +359,23 @@ public String toString()
private static class StringItem
implements Item
{
private static final List<String> QUALIFIERS =
Arrays.asList( "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" );
private static final List<String> QUALIFIERS = Arrays.asList( "snapshot", "", "sp" );

private static final Properties ALIASES = new Properties();

static
{
ALIASES.put( "ga", "" );
ALIASES.put( "cr", "rc" );
ALIASES.put( "final", "" );
ALIASES.put( "ga", "" );
ALIASES.put( "release", "" );
ALIASES.put( "cr", "rc" );
}

/**
* A comparable value for the empty-string qualifier. This one is used to determine if a given qualifier makes
* An index value for the empty-string qualifier. This one is used to determine if a given qualifier makes
* the version older than one without a qualifier, or more recent.
*/
private static final String RELEASE_VERSION_INDEX = String.valueOf( QUALIFIERS.indexOf( "" ) );
private static final int RELEASE_VERSION_INDEX = QUALIFIERS.indexOf( "" );

private final String value;

Expand Down Expand Up @@ -407,7 +410,7 @@ public int getType()
@Override
public boolean isNull()
{
return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 );
return QUALIFIERS.indexOf( value ) == RELEASE_VERSION_INDEX;
}

/**
Expand All @@ -422,12 +425,37 @@ public boolean isNull()
*
* @param qualifier
* @return an equivalent value that can be used with lexical comparison
* @deprecated Use {@link #compareQualifiers(String, String)} instead
*/
@Deprecated
public static String comparableQualifier( String qualifier )
{
int i = QUALIFIERS.indexOf( qualifier );
int index = QUALIFIERS.indexOf( qualifier ) + 1;

return index == 0 ? ( "0-" + qualifier ) : String.valueOf( index );
}

/**
* Compare the qualifiers of two artifact versions.
*
* @param qualifier1 qualifier of first artifact
* @param qualifier2 qualifier of second artifact
* @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
* greater than the second
*/
public static int compareQualifiers( String qualifier1, String qualifier2 )
{
int i1 = QUALIFIERS.indexOf( qualifier1 );
int i2 = QUALIFIERS.indexOf( qualifier2 );

// if both pre-release, then use natural lexical ordering
if ( i1 == -1 && i2 == -1 )
{
return qualifier1.compareTo( qualifier2 );
}

return i == -1 ? ( QUALIFIERS.size() + "-" + qualifier ) : String.valueOf( i );
// 'other qualifier' < 'snapshot' < '' < 'sp'
return Integer.compare( i1, i2 );
}

@Override
Expand All @@ -436,7 +464,7 @@ public int compareTo( Item item )
if ( item == null )
{
// 1-rc < 1, 1-ga > 1
return comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX );
return Integer.compare( QUALIFIERS.indexOf( value ), RELEASE_VERSION_INDEX );
}
switch ( item.getType() )
{
Expand All @@ -446,7 +474,7 @@ public int compareTo( Item item )
return -1; // 1.any < 1.1 ?

case STRING_ITEM:
return comparableQualifier( value ).compareTo( comparableQualifier( ( (StringItem) item ).value ) );
return compareQualifiers( value, ( ( StringItem ) item ).value );

case LIST_ITEM:
return -1; // 1.any < 1-1
Expand Down
41 changes: 28 additions & 13 deletions maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ private Comparable newComparable( String version )
}

private static final String[] VERSIONS_QUALIFIER =
{ "1-alpha2snapshot", "1-alpha2", "1-alpha-123", "1-beta-2", "1-beta123", "1-m2", "1-m11", "1-rc", "1-cr2",
"1-rc123", "1-SNAPSHOT", "1", "1-sp", "1-sp2", "1-sp123", "1-abc", "1-def", "1-pom-1", "1-1-snapshot",
{ "1-abc", "1-alpha2snapshot", "1-alpha2", "1-alpha-123", "1-beta-2", "1-beta123", "1-def", "1-m2", "1-m11",
"1-pom-1", "1-rc", "1-cr2", "1-rc123", "1-SNAPSHOT", "1", "1-sp", "1-sp2", "1-sp123", "1-1-snapshot",
"1-1", "1-2", "1-123" };

private static final String[] VERSIONS_NUMBER =
{ "2.0", "2.0.a", "2-1", "2.0.2", "2.0.123", "2.1.0", "2.1-a", "2.1b", "2.1-c", "2.1-1", "2.1.0.1", "2.2",
"2.123", "11.a2", "11.a11", "11.b2", "11.b11", "11.m2", "11.m11", "11", "11.a", "11b", "11c", "11m" };
{ "2.0.a", "2.0", "2-1", "2.0.2", "2.0.123", "2.1-a", "2.1b", "2.1-c", "2.1.0", "2.1-1", "2.1.0.1", "2.2",
"2.123", "11.a", "11.a2", "11.a11", "11b", "11.b2", "11.b11", "11c", "11m", "11.m2", "11.m11", "11" };

private void checkVersionsOrder( String[] versions )
{
Expand Down Expand Up @@ -189,7 +189,7 @@ public void testVersionComparing()

checkVersionsOrder( "2.0-1", "2.0.1" );
checkVersionsOrder( "2.0.1-klm", "2.0.1-lmn" );
checkVersionsOrder( "2.0.1", "2.0.1-xyz" );
checkVersionsOrder( "2.0.1-xyz", "2.0.1" ); // now 2.0.1-xyz < 2.0.1 as of MNG-7559

checkVersionsOrder( "2.0.1", "2.0.1-123" );
checkVersionsOrder( "2.0.1-xyz", "2.0.1-123" );
Expand All @@ -204,13 +204,9 @@ public void testVersionComparing()
*/
public void testMng5568()
{
String a = "6.1.0";
String b = "6.1.0rc3";
String c = "6.1H.5-beta"; // this is the unusual version string, with 'H' in the middle

checkVersionsOrder( b, a ); // classical
checkVersionsOrder( b, c ); // now b < c, but before MNG-5568, we had b > c
checkVersionsOrder( a, c );
checkVersionsOrder( "6.1H.5-beta", "6.1.0rc3" ); // now H < RC as of MNG-7559
checkVersionsOrder( "6.1.0rc3", "6.1.0" ); // classical
checkVersionsOrder( "6.1H.5-beta", "6.1.0" ); // transitivity
}

/**
Expand Down Expand Up @@ -338,14 +334,33 @@ public void testReuse()
assertEquals( "reused instance should be equivalent to new instance", c1, c2 );
}

/**
* Test <a href="https://issues.apache.org/jira/browse/MNG-7559">MNG-7559</a> edge cases
* -pfd < final, ga, release
* 2.0.1.MR < 2.0.1
* 9.4.1.jre16 > 9.4.1.jre16-preview
*/
public void testMng7559()
{
// checking general cases
checkVersionsOrder(
new String[]{ "ab", "alpha", "beta", "cd", "ea", "milestone", "mr", "pfd", "preview", "RC" } );
// checking identified issues respect the general case
checkVersionsOrder( "2.3-pfd", "2.3" );
checkVersionsOrder( "2.0.1.MR", "2.0.1" );
checkVersionsOrder( "9.4.1.jre16-preview", "9.4.1.jre16" );
checkVersionsOrder( "1-sp-1", "1-ga-1" ); // proving website documentation right.
}

/**
* Test <a href="https://issues.apache.org/jira/browse/MNG-7644">MNG-7644</a> edge cases
* 1.0.0.RC1 < 1.0.0-RC2 and more generally:
* 1.0.0.X1 < 1.0.0-X2 for any string X
*/
public void testMng7644()
{
for ( String x : new String[]{ "abc", "alpha", "a", "beta", "b", "def", "milestone", "m", "RC" } ) {
for ( String x : new String[]{ "abc", "alpha", "a", "beta", "b", "def", "milestone", "m", "RC" } )
{
// 1.0.0.X1 < 1.0.0-X2 for any string x
checkVersionsOrder( "1.0.0." + x + "1", "1.0.0-" + x + "2" );
// 2.0.X == 2-X == 2.0.0.X for any string x
Expand Down
2 changes: 1 addition & 1 deletion ...tifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void testVersionComparing()
assertVersionOlder( "2.0-1", "2.0.1" );

assertVersionOlder( "2.0.1-klm", "2.0.1-lmn" );
assertVersionOlder( "2.0.1", "2.0.1-xyz" );
assertVersionOlder( "2.0.1-xyz", "2.0.1" ); // now 2.0.1-xyz < 2.0.1 as of MNG-7559
assertVersionOlder( "2.0.1-xyz-1", "2.0.1-1-xyz" );

assertVersionOlder( "2.0.1", "2.0.1-123" );
Expand Down