Skip to content

Commit

Permalink
[MNG-7559] Fix version comparison with case insensitive lexical order
Browse files Browse the repository at this point in the history
  • Loading branch information
@sultan
sultan committed Dec 22, 2022
1 parent da4246a commit a275da9
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 40 deletions.
80 changes: 54 additions & 26 deletions maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java
View file
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
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
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

0 comments on commit a275da9

Please sign in to comment.