Skip to content

Commit 4f0c93f

Browse files
sultansultan
committed
[MNG-7559] fix version comparison
1 parent 29f96a3 commit 4f0c93f

File tree

3 files changed

+112
-42
lines changed

3 files changed

+112
-42
lines changed

‎maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java‎

Lines changed: 84 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@
2323
import java.util.ArrayList;
2424
import java.util.Arrays;
2525
import java.util.Deque;
26+
import java.util.HashMap;
2627
import java.util.Iterator;
2728
import java.util.List;
2829
import java.util.Locale;
29-
import java.util.Properties;
30+
import java.util.Map;
3031

3132
/**
3233
* <p>
@@ -40,22 +41,40 @@
4041
* <code>1.0alpha1 =&gt; [1, 0, alpha, 1]</code></li>
4142
* <li>unlimited number of version components,</li>
4243
* <li>version components in the text can be digits or strings,</li>
43-
* <li>strings are checked for well-known qualifiers and the qualifier ordering is used for version ordering.
44-
* Well-known qualifiers (case insensitive) are:<ul>
45-
* <li><code>alpha</code> or <code>a</code></li>
46-
* <li><code>beta</code> or <code>b</code></li>
47-
* <li><code>milestone</code> or <code>m</code></li>
48-
* <li><code>rc</code> or <code>cr</code></li>
49-
* <li><code>snapshot</code></li>
50-
* <li><code>(the empty string)</code> or <code>ga</code> or <code>final</code></li>
51-
* <li><code>sp</code></li>
52-
* </ul>
53-
* Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive),
54-
* </li>
55-
* <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li>
44+
* <li>
45+
* String qualifiers are ordered lexically (case insensitive), with the following exceptions:
46+
* <ul>
47+
* <li><code> 'snapshot' &lt; '' &lt; 'sp' </code></li>
48+
* </ul>
49+
* and <code>'alias' -&gt; 'replacement'</code> (all case insensitive):
50+
* <ul>
51+
* <li><code> 'a' -&gt; 'alpha' </code></li>
52+
* <li><code> 'b' -&gt; 'beta' </code></li>
53+
* <li><code> 'm' -&gt; 'milestone' </code></li>
54+
* <li><code> 'cr' -&gt; 'rc' </code></li>
55+
* <li><code> 'final' -&gt; '' </code></li>
56+
* <li><code> 'ga' -&gt; '' </code></li>
57+
* <li><code> 'release' -&gt; '' </code></li>
58+
* </ul>
59+
* </li>
60+
* <li>
61+
* Following semver rules is encouraged, and some qualifiers are discouraged (no matter the case):
62+
* <ul>
63+
* <li> The usage of 'CR' qualifier is discouraged. Use 'RC' instead. </li>
64+
* <li> The usage of 'final', 'ga', and 'release' qualifiers is discouraged. Use no qualifier instead. </li>
65+
* <li> The usage of 'SP' qualifier is discouraged. Increment the patch version instead. </li>
66+
* </ul>
67+
* For other qualifiers, natural ordering is used (case insensitive):
68+
* <ul>
69+
* <li><code> alpha = a &lt; beta = b &lt; milestone = m &lt; rc = cr
70+
* &lt; snapshot &lt; '' = final = ga = release &lt; sp </code></li>
71+
* </ul>
72+
* </li>
73+
* <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
74+
* 1.0.RC2 &lt; 1.0-RC3 &lt; 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </li>
5675
* </ul>
5776
*
58-
* @see <a href="https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning">"Versioning" on Maven Wiki</a>
77+
* @see <a href="https://maven.apache.org/pom.html#Version_Order_Specification">Version Order Specification</a>
5978
* @author <a href="mailto:kenney@apache.org">Kenney Westerhof</a>
6079
* @author <a href="mailto:hboutemy@apache.org">Hervé Boutemy</a>
6180
*/
@@ -304,23 +323,22 @@ public String toString() {
304323
* Represents a string in the version item list, usually a qualifier.
305324
*/
306325
private static class StringItem implements Item {
307-
private static final List<String> QUALIFIERS =
308-
Arrays.asList("alpha", "beta", "milestone", "rc", "snapshot", "", "sp");
326+
private static final List<String> QUALIFIERS = Arrays.asList("snapshot", "", "sp");
309327

310-
private static final Properties ALIASES = new Properties();
328+
private static final Map<String, String> ALIASES = new HashMap<>(4);
311329

312330
static {
313-
ALIASES.put("ga", "");
331+
ALIASES.put("cr", "rc");
314332
ALIASES.put("final", "");
333+
ALIASES.put("ga", "");
315334
ALIASES.put("release", "");
316-
ALIASES.put("cr", "rc");
317335
}
318336

319337
/**
320-
* A comparable value for the empty-string qualifier. This one is used to determine if a given qualifier makes
338+
* An index value for the empty-string qualifier. This one is used to determine if a given qualifier makes
321339
* the version older than one without a qualifier, or more recent.
322340
*/
323-
private static final String RELEASE_VERSION_INDEX = String.valueOf(QUALIFIERS.indexOf(""));
341+
private static final int RELEASE_VERSION_INDEX = QUALIFIERS.indexOf("");
324342

325343
private final String value;
326344

@@ -340,7 +358,7 @@ private static class StringItem implements Item {
340358
default:
341359
}
342360
}
343-
this.value = ALIASES.getProperty(value, value);
361+
this.value = ALIASES.getOrDefault(value, value);
344362
}
345363

346364
@Override
@@ -350,7 +368,7 @@ public int getType() {
350368

351369
@Override
352370
public boolean isNull() {
353-
return (comparableQualifier(value).compareTo(RELEASE_VERSION_INDEX) == 0);
371+
return QUALIFIERS.indexOf(value) == RELEASE_VERSION_INDEX;
354372
}
355373

356374
/**
@@ -365,18 +383,42 @@ public boolean isNull() {
365383
*
366384
* @param qualifier
367385
* @return an equivalent value that can be used with lexical comparison
386+
* @deprecated Use {@link #compareQualifiers(String, String)} instead
368387
*/
388+
@Deprecated
369389
public static String comparableQualifier(String qualifier) {
370-
int i = QUALIFIERS.indexOf(qualifier);
390+
int index = QUALIFIERS.indexOf(qualifier) + 1;
371391

372-
return i == -1 ? (QUALIFIERS.size() + "-" + qualifier) : String.valueOf(i);
392+
return index == 0 ? ("0-" + qualifier) : String.valueOf(index);
393+
}
394+
395+
/**
396+
* Compare the qualifiers of two artifact versions.
397+
*
398+
* @param qualifier1 qualifier of first artifact
399+
* @param qualifier2 qualifier of second artifact
400+
* @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or
401+
* greater than the second
402+
*/
403+
public static int compareQualifiers(String qualifier1, String qualifier2) {
404+
int i1 = QUALIFIERS.indexOf(qualifier1);
405+
int i2 = QUALIFIERS.indexOf(qualifier2);
406+
407+
// if both pre-release, then use natural lexical ordering
408+
if (i1 == -1 && i2 == -1) {
409+
// alpha < beta < ea < milestone < preview < rc
410+
return qualifier1.compareTo(qualifier2);
411+
}
412+
413+
// 'other qualifier' < 'snapshot' < '' < 'sp'
414+
return Integer.compare(i1, i2);
373415
}
374416

375417
@Override
376418
public int compareTo(Item item) {
377419
if (item == null) {
378420
// 1-rc < 1, 1-ga > 1
379-
return comparableQualifier(value).compareTo(RELEASE_VERSION_INDEX);
421+
return Integer.compare(QUALIFIERS.indexOf(value), RELEASE_VERSION_INDEX);
380422
}
381423
switch (item.getType()) {
382424
case INT_ITEM:
@@ -385,7 +427,7 @@ public int compareTo(Item item) {
385427
return -1; // 1.any < 1.1 ?
386428

387429
case STRING_ITEM:
388-
return comparableQualifier(value).compareTo(comparableQualifier(((StringItem) item).value));
430+
return compareQualifiers(value, ((StringItem) item).value);
389431

390432
case LIST_ITEM:
391433
return -1; // 1.any < 1-1
@@ -570,6 +612,13 @@ public final void parseVersion(String version) {
570612
stack.push(list);
571613
} else if (Character.isDigit(c)) {
572614
if (!isDigit && i > startIndex) {
615+
// 1.0.0.RC1 < 1.0.0-RC2
616+
// treat .RC as -RC
617+
if (!list.isEmpty()) {
618+
list.add(list = new ListItem());
619+
stack.push(list);
620+
}
621+
573622
list.add(new StringItem(version.substring(startIndex, i), true));
574623
startIndex = i;
575624

@@ -592,6 +641,13 @@ public final void parseVersion(String version) {
592641
}
593642

594643
if (version.length() > startIndex) {
644+
// 1.0.0.RC1 < 1.0.0-RC2
645+
// treat .RC as -RC
646+
if (!isDigit && !list.isEmpty()) {
647+
list.add(list = new ListItem());
648+
stack.push(list);
649+
}
650+
595651
list.add(parseItem(isDigit, version.substring(startIndex)));
596652
}
597653

‎maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,16 @@ private Comparable newComparable(String version) {
4646
}
4747

4848
private static final String[] VERSIONS_QUALIFIER = {
49+
"1-abc",
4950
"1-alpha2snapshot",
5051
"1-alpha2",
5152
"1-alpha-123",
5253
"1-beta-2",
5354
"1-beta123",
55+
"1-def",
5456
"1-m2",
5557
"1-m11",
58+
"1-pom-1",
5659
"1-rc",
5760
"1-cr2",
5861
"1-rc123",
@@ -61,18 +64,15 @@ private Comparable newComparable(String version) {
6164
"1-sp",
6265
"1-sp2",
6366
"1-sp123",
64-
"1-abc",
65-
"1-def",
66-
"1-pom-1",
6767
"1-1-snapshot",
6868
"1-1",
6969
"1-2",
7070
"1-123"
7171
};
7272

7373
private static final String[] VERSIONS_NUMBER = {
74-
"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",
75-
"2.2", "2.123", "11.a2", "11.a11", "11.b2", "11.b11", "11.m2", "11.m11", "11", "11.a", "11b", "11c", "11m"
74+
"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",
75+
"2.123", "11.a", "11.a2", "11.a11", "11b", "11.b2", "11.b11", "11c", "11m", "11.m2", "11.m11", "11"
7676
};
7777

7878
private void checkVersionsOrder(String[] versions) {
@@ -202,7 +202,7 @@ public void testVersionComparing() {
202202

203203
checkVersionsOrder("2.0-1", "2.0.1");
204204
checkVersionsOrder("2.0.1-klm", "2.0.1-lmn");
205-
checkVersionsOrder("2.0.1", "2.0.1-xyz");
205+
checkVersionsOrder("2.0.1-xyz", "2.0.1"); // now 2.0.1-xyz < 2.0.1 as of MNG-7559
206206

207207
checkVersionsOrder("2.0.1", "2.0.1-123");
208208
checkVersionsOrder("2.0.1-xyz", "2.0.1-123");
@@ -217,13 +217,9 @@ public void testVersionComparing() {
217217
*/
218218
@Test
219219
public void testMng5568() {
220-
String a = "6.1.0";
221-
String b = "6.1.0rc3";
222-
String c = "6.1H.5-beta"; // this is the unusual version string, with 'H' in the middle
223-
224-
checkVersionsOrder(b, a); // classical
225-
checkVersionsOrder(b, c); // now b < c, but before MNG-5568, we had b > c
226-
checkVersionsOrder(a, c);
220+
checkVersionsOrder("6.1H.5-beta", "6.1.0rc3"); // now H < RC as of MNG-7559
221+
checkVersionsOrder("6.1.0rc3", "6.1.0"); // classical
222+
checkVersionsOrder("6.1H.5-beta", "6.1.0"); // transitivity
227223
}
228224

229225
/**
@@ -346,4 +342,22 @@ public void testReuse() {
346342

347343
assertEquals(c1, c2, "reused instance should be equivalent to new instance");
348344
}
345+
346+
/**
347+
* Test <a href="https://issues.apache.org/jira/browse/MNG-7559">MNG-7559</a> edge cases
348+
* 1.0.0.RC1 < 1.0.0-RC2
349+
* -pfd < final, ga, release
350+
* 2.0.1.MR < 2.0.1
351+
* 9.4.1.jre16 > 9.4.1.jre16-preview
352+
*/
353+
@Test
354+
public void testMng7559() {
355+
checkVersionsOrder("1.0.0.RC1", "1.0.0-RC2");
356+
checkVersionsOrder("4.0.0.Beta3", "4.0.0-RC2");
357+
checkVersionsOrder("2.3-pfd", "2.3");
358+
checkVersionsOrder("2.0.1.MR", "2.0.1");
359+
checkVersionsOrder("9.4.1.jre16-preview", "9.4.1.jre16");
360+
checkVersionsEqual("2.0.a", "2.0.0.a"); // previously ordered, now equals
361+
checkVersionsOrder("1-sp-1", "1-ga-1"); // proving website documentation right.
362+
}
349363
}

‎maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void testVersionComparing() {
119119
assertVersionOlder("2.0-1", "2.0.1");
120120

121121
assertVersionOlder("2.0.1-klm", "2.0.1-lmn");
122-
assertVersionOlder("2.0.1", "2.0.1-xyz");
122+
assertVersionOlder("2.0.1-xyz", "2.0.1"); // now 2.0.1-xyz < 2.0.1 as of MNG-7559
123123
assertVersionOlder("2.0.1-xyz-1", "2.0.1-1-xyz");
124124

125125
assertVersionOlder("2.0.1", "2.0.1-123");

0 commit comments

Comments
 (0)