Skip to content
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

Jump to bottom

[MNG-7559] Fix versions comparison #845

Merged
merged 1 commit into from
Dec 3, 2022
Merged

[MNG-7559] Fix versions comparison #845

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
111 changes: 83 additions & 28 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 @@ -23,10 23,11 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Properties;
import java.util.Map;

/**
* <p>
Expand All @@ -40,22 41,39 @@
* <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>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li>
* <li>
* String qualifiers are ordered lexically (case insensitive), with the following exceptions:
* <ul>
* <li> 'snapshot' &lt; '' &lt; 'sp' </li>
* </ul>
* and alias -&gt; replacement (all case insensitive):
* <ul>
* <li> 'a' -&gt; 'alpha' </li>
* <li> 'b' -&gt; 'beta' </li>
* <li> 'm' -&gt; 'milestone' </li>
* <li> 'cr' -&gt; 'rc' </li>
* <li> 'final' -&gt; '' </li>
* <li> 'final' -&gt; '' </li>
* <li> 'final' -&gt; '' </li>
* </ul>
* </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>
sultan marked this conversation as resolved.
Show resolved Hide resolved
* </ul>
* For other qualifiers, natural ordering is used (case insensitive):
* <ul>
* <li> alpha = a &lt; beta = b &lt; milestone = m &lt; rc = cr &lt; snapshot &lt; '' = final = ga = release &lt; sp </li>
* </ul>
* </li>
* <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
* 1.0.RC2 &lt; 1.0-RC3 &lt; 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </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:[email protected]">Kenney Westerhof</a>
* @author <a href="mailto:[email protected]">Hervé Boutemy</a>
*/
Expand Down Expand Up @@ -304,23 322,22 @@ public String toString() {
* Represents a string in the version item list, usually a qualifier.
*/
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");

sultan marked this conversation as resolved.
Show resolved Hide resolved
private static final Properties ALIASES = new Properties();
private static final Map<String, String> ALIASES = new HashMap<>(4);

static {
ALIASES.put("ga", "");
ALIASES.put("cr", "rc");
ALIASES.put("final", "");
ALIASES.put("ga", "");
ALIASES.put("release", "");
ALIASES.put("cr", "rc");
}
sultan marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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 All @@ -340,7 357,7 @@ private static class StringItem implements Item {
default:
}
}
this.value = ALIASES.getProperty(value, value);
this.value = ALIASES.getOrDefault(value, value);
}

@Override
Expand All @@ -350,7 367,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 @@ -365,18 382,42 @@ 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 i == -1 ? (QUALIFIERS.size() "-" qualifier) : String.valueOf(i);
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) {
// alpha < beta < ea < milestone < preview < rc
return qualifier1.compareTo(qualifier2);
}

// 'other qualifier' < 'snapshot' < '' < 'sp'
return Integer.compare(i1, i2);
}
sultan marked this conversation as resolved.
Show resolved Hide resolved

@Override
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()) {
case INT_ITEM:
Expand All @@ -385,7 426,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 Expand Up @@ -570,6 611,13 @@ public final void parseVersion(String version) {
stack.push(list);
} else if (Character.isDigit(c)) {
if (!isDigit && i > startIndex) {
// 1.0.0.RC1 < 1.0.0-RC2
// treat .RC as -RC
if (!list.isEmpty()) {
list.add(list = new ListItem());
stack.push(list);
}

list.add(new StringItem(version.substring(startIndex, i), true));
startIndex = i;

Expand All @@ -592,6 640,13 @@ public final void parseVersion(String version) {
}

if (version.length() > startIndex) {
// 1.0.0.RC1 < 1.0.0-RC2
// treat .RC as -RC
if (!isDigit && !list.isEmpty()) {
list.add(list = new ListItem());
stack.push(list);
}

list.add(parseItem(isDigit, version.substring(startIndex)));
}

Expand Down
40 changes: 27 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,16 @@ private Comparable newComparable(String version) {
}

private static final String[] VERSIONS_QUALIFIER = {
"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",
Expand All @@ -61,18 64,15 @@ private Comparable newComparable(String version) {
"1-sp",
"1-sp2",
"1-sp123",
"1-abc",
"1-def",
"1-pom-1",
"1-1-snapshot",
"1-1",
"1-2",
"1-123"
};

private static final String[] VERSIONS_NUMBER = {
"2.0", "2-1", "2.0.a", "2.0.0.a", "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 @@ -202,7 202,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 @@ -217,13 217,9 @@ public void testVersionComparing() {
*/
@Test
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 @@ -346,4 342,22 @@ public void testReuse() {

assertEquals(c1, c2, "reused instance should be equivalent to new instance");
}

/**
* Test <a href="https://issues.apache.org/jira/browse/MNG-7559">MNG-7559</a> edge cases
* 1.0.0.RC1 < 1.0.0-RC2
* -pfd < final, ga, release
* 2.0.1.MR < 2.0.1
* 9.4.1.jre16 > 9.4.1.jre16-preview
*/
@Test
public void testMng7559() {
checkVersionsOrder("1.0.0.RC1", "1.0.0-RC2");
checkVersionsOrder("4.0.0.Beta3", "4.0.0-RC2");
checkVersionsOrder("2.3-pfd", "2.3");
checkVersionsOrder("2.0.1.MR", "2.0.1");
checkVersionsOrder("9.4.1.jre16-preview", "9.4.1.jre16");
checkVersionsEqual("2.0.a", "2.0.0.a"); // previously ordered, now equals
checkVersionsOrder("1-sp-1", "1-ga-1"); // proving website documentation right.
}
}
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 @@ -119,7 119,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