From 97e8bf67113a9697ff8940b0b65b3e4d7bd3c28a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gw=C3=A9na=C3=ABl=20Ruelland?= <5782559+sultan@users.noreply.github.com> Date: Sat, 3 Dec 2022 20:34:24 +0100 Subject: [PATCH] fix version comparison (#845) --- .../versioning/ComparableVersion.java | 113 +++++++++++++----- .../versioning/ComparableVersionTest.java | 40 +++++-- .../DefaultArtifactVersionTest.java | 2 +- 3 files changed, 112 insertions(+), 43 deletions(-) diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java index d822471c8b..8d17c001be 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java @@ -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; /** *

@@ -40,22 +41,39 @@ * 1.0alpha1 => [1, 0, 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: - * Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive), - *
    • - *
  • a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.
    • + *
  • + * String qualifiers are ordered lexically (case insensitive), with the following exceptions: + * + * and alias -> replacement (all case insensitive): + * + *
    • + *
  • + * Following semver rules is encouraged, and some qualifiers are discouraged (no matter the case): + * + * For other qualifiers, natural ordering is used (case insensitive): + * + *
    • + *
  • a hyphen usually precedes a qualifier, and is always less important than digits/number, for example + * 1.0.RC2 < 1.0-RC3 < 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1'
    • * * - * @see "Versioning" on Maven Wiki + * @see Version Order Specification * @author Kenney Westerhof * @author HervĂ© Boutemy */ @@ -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 QUALIFIERS = - Arrays.asList("alpha", "beta", "milestone", "rc", "snapshot", "", "sp"); + private static final List QUALIFIERS = Arrays.asList("snapshot", "", "sp"); - private static final Properties ALIASES = new Properties(); + private static final Map ALIASES = new HashMap<>(4); static { - ALIASES.put("ga", ""); - ALIASES.put("final", ""); - ALIASES.put("release", ""); 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; @@ -340,7 +357,7 @@ private static class StringItem implements Item { default: } } - this.value = ALIASES.getProperty(value, value); + this.value = ALIASES.getOrDefault(value, value); } @Override @@ -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; } /** @@ -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); } @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: @@ -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 @@ -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; @@ -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))); } diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java index 4105569fb2..ac539b7807 100644 --- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java +++ b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java @@ -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", @@ -61,9 +64,6 @@ 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", @@ -71,8 +71,8 @@ private Comparable newComparable(String version) { }; 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) { @@ -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"); @@ -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 } /** @@ -346,4 +342,22 @@ public void testReuse() { assertEquals(c1, c2, "reused instance should be equivalent to new instance"); } + + /** + * Test MNG-7559 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. + } } diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java index b1de43b33b..fcb78d77a4 100644 --- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java +++ b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java @@ -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");