From fa21d7d1d359d1d4152dddd5c835c645df13aafd Mon Sep 17 00:00:00 2001 From: Swell <5782559+sultan@users.noreply.github.com> Date: Wed, 19 Nov 2025 21:19:41 +0100 Subject: [PATCH] [MNG-7559] Fix version comparison with case insensitive lexical order --- .../versioning/ComparableVersion.java | 114 +++++++++++------- .../versioning/ComparableVersionTest.java | 39 ++++-- .../DefaultArtifactVersionTest.java | 2 +- 3 files changed, 97 insertions(+), 58 deletions(-) diff --git a/compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java b/compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java index df3c0345288f..bd75c2f12829 100644 --- a/compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java +++ b/compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java @@ -23,11 +23,12 @@ 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.Map; import java.util.Objects; -import java.util.Properties; /** *

@@ -41,20 +42,21 @@ * 1.0alpha1 => [1, [alpha, 1]] *

  • Unlimited number of version components,
    • *
  • Version components in the text can be digits or strings,
    • - *
  • Strings are checked for well-known qualifiers, and the qualifier ordering is used for version ordering. - * Well-known qualifiers (case-insensitive) are, in order from least to greatest:
      - *
    1. alpha or a
      2. - *
    2. beta or b
      3. - *
    3. milestone or m
      4. - *
    4. rc or cr
      5. - *
    5. snapshot
      6. - *
    6. ga or final
      7. - *
    7. sp
      8. - *
    - * Unknown qualifiers are considered after known qualifiers, - * with lexical order (case-insensitive in the English locale). - * ga and final sort the same as not having a qualifier. - *
    • + *
  • + * Following semver rules is encouraged, and some qualifiers are discouraged (case insensitive): + * + * String qualifiers are ordered lexically (case insensitive in the English locale), with the following exceptions: + * + *
    • *
  • 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-RC2} over {@code 1.0.0.RC2}, 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} @@ -295,41 +297,37 @@ public String toString() { * Represents a string in the version item list, usually a qualifier. */ private static class StringItem implements Item { - private static final List QUALIFIERS = - Arrays.asList("alpha", "beta", "milestone", "rc", "snapshot", "", "sp"); - private static final List RELEASE_QUALIFIERS = Arrays.asList("ga", "final", "release"); + private static final List QUALIFIERS = Arrays.asList("snapshot", "", "sp"); + private static final List RELEASE_QUALIFIERS = Arrays.asList("final", "ga", "release"); - private static final Properties ALIASES = new Properties(); + private static final Map ALIASES = new HashMap<>(4); static { ALIASES.put("cr", "rc"); + ALIASES.put("final", ""); + ALIASES.put("ga", ""); + ALIASES.put("release", ""); } /** - * 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; StringItem(String value, boolean followedByDigit) { if (followedByDigit && value.length() == 1) { // a1 = alpha-1, b1 = beta-1, m1 = milestone-1 - switch (value.charAt(0)) { - case 'a': - value = "alpha"; - break; - case 'b': - value = "beta"; - break; - case 'm': - value = "milestone"; - break; - default: - } + value = switch (value.charAt(0)) { + case 'a' -> "alpha"; + case 'b' -> "beta"; + case 'm' -> "milestone"; + default -> value; + }; } - this.value = ALIASES.getProperty(value, value); + this.value = ALIASES.getOrDefault(value, value); } @Override @@ -351,27 +349,55 @@ 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) { + // Just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for + // -1 or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first + // character, so this is still fast. If more characters are needed then it requires a lexical sort anyway. + if (RELEASE_QUALIFIERS.contains(qualifier)) { - return String.valueOf(QUALIFIERS.indexOf("")); + qualifier = ""; } - int i = QUALIFIERS.indexOf(qualifier); + int index = QUALIFIERS.indexOf(qualifier) + 1; - // Just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for - // -1 - // or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first - // character, - // so this is still fast. If more characters are needed then it requires a lexical sort anyway. - 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) { + if (RELEASE_QUALIFIERS.contains(qualifier1)) { + qualifier1 = ""; + } + if (RELEASE_QUALIFIERS.contains(qualifier2)) { + 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); + } + + // 'other qualifier' < 'snapshot' < '' < 'sp' + return Integer.compare(i1, i2); } @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: @@ -380,7 +406,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 COMBINATION_ITEM: int result = this.compareTo(((CombinationItem) item).getStringPart()); diff --git a/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java b/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java index 11561aa16689..daac9b5313ad 100644 --- a/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java +++ b/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java @@ -44,13 +44,16 @@ private ComparableVersion 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", @@ -59,9 +62,6 @@ private ComparableVersion newComparable(String version) { "1-sp", "1-sp2", "1-sp123", - "1-abc", - "1-def", - "1-pom-1", "1-1-snapshot", "1-1", "1-2", @@ -69,8 +69,8 @@ private ComparableVersion newComparable(String version) { }; 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) { @@ -212,7 +212,7 @@ 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"); @@ -328,13 +328,9 @@ void testLexicographicOrder() { */ @Test 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 } /** @@ -458,6 +454,23 @@ void testReuse() { assertEquals(c1, c2, "reused instance should be equivalent to new instance"); } + /** + * Test MNG-7559 edge cases + * -pfd < final, ga, release + * 2.0.1.MR < 2.0.1 + * 9.4.1.jre16 > 9.4.1.jre16-preview + */ + @Test + 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-ga-1", "1-sp-1"); + } + /** * Test MNG-7644 edge cases * 1.0.0.RC1 < 1.0.0-RC2 and more generally: diff --git a/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java b/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java index c1530cbe202d..dbd77ca42ff2 100644 --- a/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java +++ b/compat/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java @@ -118,7 +118,7 @@ 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");