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-7714] Fixed a scenario in version sorting where sp1 is less than final #1099

Merged
merged 6 commits into from
Jun 21, 2023
Merged

[MNG-7714] Fixed a scenario in version sorting where sp1 is less than final #1099

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5d01c4
Fixed a scenario in version sorting where sp1 is less than final
CrazyHZM May 2, 2023
3bd95eb
Split the return value of different cases
CrazyHZM May 22, 2023
d37c121
Inlining newComparable in checkVersionsEqualOrder method
CrazyHZM Jun 19, 2023
bf5b463
Add version object inequality test
CrazyHZM Jun 19, 2023
5384c7f
Make the test clearer
CrazyHZM Jun 20, 2023
09d7220
Making the comments clearer.
CrazyHZM Jun 20, 2023
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
192 changes: 164 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 @@ -26,13 +26,14 @@
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Properties;

/**
* <p>
* Generic implementation of version comparison.
* </p>
*
* <p>
* Features:
* <ul>
* <li>mixing of '<code>-</code>' (hyphen) and '<code>.</code>' (dot) separators,</li>
Expand All @@ -58,9 +59,9 @@
* over {@code 1.0.0.X1}.</li>
* </ul>
*
* @see <a href="https://maven.apache.org/pom.html#version-order-specification">"Versioning" in the POM reference</a>
* @author <a href="mailto:kenney@apache.org">Kenney Westerhof</a>
* @author <a href="mailto:hboutemy@apache.org">Hervé Boutemy</a>
* @see <a href="https://maven.apache.org/pom.html#version-order-specification">"Versioning" in the POM reference</a>
*/
public class ComparableVersion implements Comparable<ComparableVersion> {
private static final int MAX_INTITEM_LENGTH = 9;
Expand All @@ -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);

Expand Down Expand Up @@ -128,6 +130,7 @@ public int compareTo(Item item) {
return -1;

case STRING_ITEM:
case COMBINATION_ITEM:
return 1; // 1.1 > 1-sp

case LIST_ITEM:
Expand Down Expand Up @@ -199,6 +202,7 @@ public int compareTo(Item item) {
return -1;

case STRING_ITEM:
case COMBINATION_ITEM:
gnodet marked this conversation as resolved.
Show resolved Hide resolved
return 1; // 1.1 > 1-sp

case LIST_ITEM:
Expand Down Expand Up @@ -269,6 +273,7 @@ public int compareTo(Item item) {
return value.compareTo(((BigIntegerItem) item).value);

case STRING_ITEM:
case COMBINATION_ITEM:
return 1; // 1.1 > 1-sp

case LIST_ITEM:
Expand Down Expand Up @@ -309,13 +314,11 @@ public String toString() {
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> 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");
}

Expand Down Expand Up @@ -353,15 +356,15 @@ 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.
*
* <p>
* This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical
* ordering.
*
* <p>
* just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved
* 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.
Expand All @@ -370,6 +373,10 @@ public boolean isNull() {
* @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);

return i == -1 ? (QUALIFIERS.size() + "-" + qualifier) : String.valueOf(i);
Expand All @@ -390,6 +397,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).getStringValue());
if (result == 0) {
return -1;
}
return result;

case LIST_ITEM:
return -1; // 1.any < 1-1

Expand Down Expand Up @@ -422,6 +436,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
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved
*/
private static class CombinationItem implements Item {

StringItem stringValue;
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved

Item digitValue;

CombinationItem(String v) {
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved
int index = 0;
for (int i = 0; i < v.length(); i++) {
char c = v.charAt(i);
if (Character.isDigit(c)) {
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved
index = i;
break;
}
}

stringValue = new StringItem(v.substring(0, index), true);
digitValue = parseItem(true, v.substring(index));
}

@Override
public int compareTo(Item item) {
if (item == null) {
// 1-rc1 < 1, 1-ga1 > 1
return stringValue.compareTo(item);
}
int result = 0;
switch (item.getType()) {
case INT_ITEM:
case LONG_ITEM:
case BIGINTEGER_ITEM:
return -1;

case STRING_ITEM:
result = stringValue.compareTo(item);
if (result == 0) {
// X1 > X
return 1;
}
return result;

case LIST_ITEM:
return -1;

case COMBINATION_ITEM:
result = stringValue.compareTo(((CombinationItem) item).getStringValue());
if (result == 0) {
return digitValue.compareTo(((CombinationItem) item).getDigitValue());
}
return result;
default:
return 0;
}
}

public StringItem getStringValue() {
return stringValue;
}

public Item getDigitValue() {
return digitValue;
}

@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(stringValue, that.stringValue) && Objects.equals(digitValue, that.digitValue);
}

@Override
public int hashCode() {
return Objects.hash(stringValue, digitValue);
}

@Override
public String toString() {
return stringValue.toString() + digitValue.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).
Expand All @@ -442,10 +556,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);
}
}
}
}
}
Expand All @@ -472,6 +590,7 @@ public int compareTo(Item item) {
return -1; // 1-1 < 1.0.x

case STRING_ITEM:
case COMBINATION_ITEM:
return 1; // 1-1 > 1-sp

case LIST_ITEM:
Expand Down Expand Up @@ -549,6 +668,8 @@ public final void parseVersion(String version) {

boolean isDigit = false;

boolean isCombination = false;

int startIndex = 0;

for (int i = 0; i < version.length(); i++) {
Expand All @@ -558,43 +679,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;
Expand All @@ -609,7 +738,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()) {
Expand All @@ -619,7 +748,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) {
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved
if (isCombination) {
return new CombinationItem(buf.replace("-", ""));
} else if (isDigit) {
buf = stripLeadingZeroes(buf);
if (buf.length() <= MAX_INTITEM_LENGTH) {
// lower than 2^31
Expand Down Expand Up @@ -674,6 +809,7 @@ public int hashCode() {
}

// CHECKSTYLE_OFF: LineLength

/**
* Main to test version parsing and comparison.
* <p>
Expand All @@ -688,7 +824,7 @@ public int hashCode() {
* </pre>
*
* @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
CrazyHZM marked this conversation as resolved.
Show resolved Hide resolved
*/
// CHECKSTYLE_ON: LineLength
public static void main(String... args) {
Expand Down
Loading