From e3529c95825c1c4e42b480222fda86f361384b2d Mon Sep 17 00:00:00 2001 From: huazhongming Date: Wed, 21 Jun 2023 21:07:56 +0800 Subject: [PATCH] [MNG-7714] Fixed a scenario in version sorting where sp1 is less than final (#1099) * Fixed a scenario in version sorting where sp1 is less than final --- .../versioning/ComparableVersion.java | 204 +++++++++++++++--- .../versioning/ComparableVersionTest.java | 46 ++-- 2 files changed, 206 insertions(+), 44 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 25cd255e19..41297f9168 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 @@ -26,13 +26,14 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.Properties; /** *

* Generic implementation of version comparison. *

- * + *

* Features: *

* - * @see "Versioning" in the POM reference * @author Kenney Westerhof * @author Hervé Boutemy + * @see "Versioning" in the POM reference */ public class ComparableVersion implements Comparable { private static final int MAX_INTITEM_LENGTH = 9; @@ -79,6 +80,7 @@ private interface Item { int BIGINTEGER_ITEM = 0; int STRING_ITEM = 1; int LIST_ITEM = 2; + int COMBINATION_ITEM = 5; int compareTo(Item item); @@ -128,6 +130,8 @@ public int compareTo(Item item) { return -1; case STRING_ITEM: + return 1; + case COMBINATION_ITEM: return 1; // 1.1 > 1-sp case LIST_ITEM: @@ -199,6 +203,8 @@ public int compareTo(Item item) { return -1; case STRING_ITEM: + return 1; + case COMBINATION_ITEM: return 1; // 1.1 > 1-sp case LIST_ITEM: @@ -269,6 +275,8 @@ public int compareTo(Item item) { return value.compareTo(((BigIntegerItem) item).value); case STRING_ITEM: + return 1; + case COMBINATION_ITEM: return 1; // 1.1 > 1-sp case LIST_ITEM: @@ -309,13 +317,11 @@ public String toString() { 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 Properties ALIASES = new Properties(); static { - ALIASES.put("ga", ""); - ALIASES.put("final", ""); - ALIASES.put("release", ""); ALIASES.put("cr", "rc"); } @@ -353,25 +359,31 @@ public int getType() { @Override public boolean isNull() { - return (comparableQualifier(value).compareTo(RELEASE_VERSION_INDEX) == 0); + return value == null || value.isEmpty(); } /** * Returns a comparable value for a qualifier. - * + *

* This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical * ordering. - * - * 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. + *

* * @param qualifier * @return an equivalent value that can be used with lexical comparison */ public static String comparableQualifier(String qualifier) { + if (RELEASE_QUALIFIERS.contains(qualifier)) { + return String.valueOf(QUALIFIERS.indexOf("")); + } + int i = QUALIFIERS.indexOf(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. return i == -1 ? (QUALIFIERS.size() + "-" + qualifier) : String.valueOf(i); } @@ -390,6 +402,13 @@ public int compareTo(Item item) { case STRING_ITEM: return comparableQualifier(value).compareTo(comparableQualifier(((StringItem) item).value)); + case COMBINATION_ITEM: + int result = this.compareTo(((CombinationItem) item).getStringPart()); + if (result == 0) { + return -1; + } + return result; + case LIST_ITEM: return -1; // 1.any < 1-1 @@ -422,6 +441,106 @@ public String toString() { } } + /** + * Represents a combination in the version item list. + * It is usually a combination of a string and a number, with the string first and the number second. + */ + private static class CombinationItem implements Item { + + StringItem stringPart; + + Item digitPart; + + CombinationItem(String value) { + int index = 0; + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (Character.isDigit(c)) { + index = i; + break; + } + } + + stringPart = new StringItem(value.substring(0, index), true); + digitPart = parseItem(true, value.substring(index)); + } + + @Override + public int compareTo(Item item) { + if (item == null) { + // 1-rc1 < 1, 1-ga1 > 1 + return stringPart.compareTo(item); + } + int result = 0; + switch (item.getType()) { + case INT_ITEM: + case LONG_ITEM: + case BIGINTEGER_ITEM: + return -1; + + case STRING_ITEM: + result = stringPart.compareTo(item); + if (result == 0) { + // X1 > X + return 1; + } + return result; + + case LIST_ITEM: + return -1; + + case COMBINATION_ITEM: + result = stringPart.compareTo(((CombinationItem) item).getStringPart()); + if (result == 0) { + return digitPart.compareTo(((CombinationItem) item).getDigitPart()); + } + return result; + default: + return 0; + } + } + + public StringItem getStringPart() { + return stringPart; + } + + public Item getDigitPart() { + return digitPart; + } + + @Override + public int getType() { + return COMBINATION_ITEM; + } + + @Override + public boolean isNull() { + return false; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + CombinationItem that = (CombinationItem) o; + return Objects.equals(stringPart, that.stringPart) && Objects.equals(digitPart, that.digitPart); + } + + @Override + public int hashCode() { + return Objects.hash(stringPart, digitPart); + } + + @Override + public String toString() { + return stringPart.toString() + digitPart.toString(); + } + } + /** * Represents a version list item. This class is used both for the global item list and for sub-lists (which start * with '-(number)' in the version specification). @@ -442,10 +561,14 @@ void normalize() { Item lastItem = get(i); if (lastItem.isNull()) { - // remove null trailing items: 0, "", empty list - remove(i); - } else if (!(lastItem instanceof ListItem)) { - break; + if (i == size() - 1 || get(i + 1).getType() == STRING_ITEM) { + remove(i); + } else if (get(i + 1).getType() == LIST_ITEM) { + Item item = ((ListItem) get(i + 1)).get(0); + if (item.getType() == COMBINATION_ITEM || item.getType() == STRING_ITEM) { + remove(i); + } + } } } } @@ -472,6 +595,8 @@ public int compareTo(Item item) { return -1; // 1-1 < 1.0.x case STRING_ITEM: + return 1; + case COMBINATION_ITEM: return 1; // 1-1 > 1-sp case LIST_ITEM: @@ -549,6 +674,8 @@ public final void parseVersion(String version) { boolean isDigit = false; + boolean isCombination = false; + int startIndex = 0; for (int i = 0; i < version.length(); i++) { @@ -558,43 +685,51 @@ public final void parseVersion(String version) { if (i == startIndex) { list.add(IntItem.ZERO); } else { - list.add(parseItem(isDigit, version.substring(startIndex, i))); + list.add(parseItem(isCombination, isDigit, version.substring(startIndex, i))); } + isCombination = false; startIndex = i + 1; } else if (c == '-') { if (i == startIndex) { list.add(IntItem.ZERO); } else { - list.add(parseItem(isDigit, version.substring(startIndex, i))); + // X-1 is going to be treated as X1 + if (!isDigit && i != version.length() - 1) { + char c1 = version.charAt(i + 1); + if (Character.isDigit(c1)) { + isCombination = true; + continue; + } + } + list.add(parseItem(isCombination, isDigit, version.substring(startIndex, i))); } startIndex = i + 1; - list.add(list = new ListItem()); - stack.push(list); + if (!list.isEmpty()) { + list.add(list = new ListItem()); + stack.push(list); + } + isCombination = false; } else if (Character.isDigit(c)) { if (!isDigit && i > startIndex) { - // 1.0.0.X1 < 1.0.0-X2 - // treat .X as -X for any string qualifier X + // X1 + isCombination = true; + if (!list.isEmpty()) { list.add(list = new ListItem()); stack.push(list); } - - list.add(new StringItem(version.substring(startIndex, i), true)); - startIndex = i; - - list.add(list = new ListItem()); - stack.push(list); } isDigit = true; } else { if (isDigit && i > startIndex) { - list.add(parseItem(true, version.substring(startIndex, i))); + list.add(parseItem(isCombination, true, version.substring(startIndex, i))); startIndex = i; list.add(list = new ListItem()); stack.push(list); + isCombination = false; } isDigit = false; @@ -609,7 +744,7 @@ public final void parseVersion(String version) { stack.push(list); } - list.add(parseItem(isDigit, version.substring(startIndex))); + list.add(parseItem(isCombination, isDigit, version.substring(startIndex))); } while (!stack.isEmpty()) { @@ -619,7 +754,13 @@ public final void parseVersion(String version) { } private static Item parseItem(boolean isDigit, String buf) { - if (isDigit) { + return parseItem(false, isDigit, buf); + } + + private static Item parseItem(boolean isCombination, boolean isDigit, String buf) { + if (isCombination) { + return new CombinationItem(buf.replace("-", "")); + } else if (isDigit) { buf = stripLeadingZeroes(buf); if (buf.length() <= MAX_INTITEM_LENGTH) { // lower than 2^31 @@ -674,6 +815,7 @@ public int hashCode() { } // CHECKSTYLE_OFF: LineLength + /** * Main to test version parsing and comparison. *

@@ -688,7 +830,7 @@ public int hashCode() { * * * @param args the version strings to parse and compare. You can pass arbitrary number of version strings and always - * two adjacent will be compared + * two adjacent will be compared. */ // CHECKSTYLE_ON: LineLength public static void main(String... args) { 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 c4d2125b64..f3fcb20d99 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 @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } + private void checkVersionsHaveSameOrder(String v1, String v2) { + ComparableVersion c1 = new ComparableVersion(v1); + ComparableVersion c2 = new ComparableVersion(v2); + assertEquals(0, c1.compareTo(c2), "expected " + v1 + " == " + v2); + assertEquals(0, c2.compareTo(c1), "expected " + v2 + " == " + v1); + } + private void checkVersionsArrayEqual(String[] array) { // compare against each other (including itself) for (int i = 0; i < array.length; ++i) @@ -145,11 +152,6 @@ void testVersionsEqual() { checkVersionsEqual("1x", "1.0.0-x"); checkVersionsEqual("1.0x", "1-x"); checkVersionsEqual("1.0.0x", "1-x"); - - // aliases - checkVersionsEqual("1ga", "1"); - checkVersionsEqual("1release", "1"); - checkVersionsEqual("1final", "1"); checkVersionsEqual("1cr", "1rc"); // special "aliases" a, b and m for alpha, beta and milestone @@ -162,14 +164,6 @@ void testVersionsEqual() { checkVersionsEqual("1A", "1a"); checkVersionsEqual("1B", "1b"); checkVersionsEqual("1M", "1m"); - checkVersionsEqual("1Ga", "1"); - checkVersionsEqual("1GA", "1"); - checkVersionsEqual("1RELEASE", "1"); - checkVersionsEqual("1release", "1"); - checkVersionsEqual("1RELeaSE", "1"); - checkVersionsEqual("1Final", "1"); - checkVersionsEqual("1FinaL", "1"); - checkVersionsEqual("1FINAL", "1"); checkVersionsEqual("1Cr", "1Rc"); checkVersionsEqual("1cR", "1rC"); checkVersionsEqual("1m3", "1Milestone3"); @@ -177,6 +171,21 @@ void testVersionsEqual() { checkVersionsEqual("1m3", "1MILESTONE3"); } + @Test + void testVersionsHaveSameOrderButAreNotEqual() { + checkVersionsHaveSameOrder("1ga", "1"); + checkVersionsHaveSameOrder("1release", "1"); + checkVersionsHaveSameOrder("1final", "1"); + checkVersionsHaveSameOrder("1Ga", "1"); + checkVersionsHaveSameOrder("1GA", "1"); + checkVersionsHaveSameOrder("1RELEASE", "1"); + checkVersionsHaveSameOrder("1release", "1"); + checkVersionsHaveSameOrder("1RELeaSE", "1"); + checkVersionsHaveSameOrder("1Final", "1"); + checkVersionsHaveSameOrder("1FinaL", "1"); + checkVersionsHaveSameOrder("1FINAL", "1"); + } + @Test void testVersionComparing() { checkVersionsOrder("1", "2"); @@ -383,4 +392,15 @@ void testMng7644() { checkVersionsEqual("2.0." + x, "2.0.0." + x); // previously ordered, now equals } } + + @Test + public void testMng7714() { + ComparableVersion f = new ComparableVersion("1.0.final-redhat"); + ComparableVersion sp1 = new ComparableVersion("1.0-sp1-redhat"); + ComparableVersion sp2 = new ComparableVersion("1.0-sp-1-redhat"); + ComparableVersion sp3 = new ComparableVersion("1.0-sp.1-redhat"); + assertTrue(f.compareTo(sp1) < 0, "expected " + f + " < " + sp1); + assertTrue(f.compareTo(sp1) < 0, "expected " + f + " < " + sp2); + assertTrue(f.compareTo(sp1) < 0, "expected " + f + " < " + sp3); + } }