diff --git a/src/main/java/org/kiwiproject/base/Versions.java b/src/main/java/org/kiwiproject/base/Versions.java index 44922625..15494218 100644 --- a/src/main/java/org/kiwiproject/base/Versions.java +++ b/src/main/java/org/kiwiproject/base/Versions.java @@ -4,14 +4,14 @@ import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotBlank; import lombok.experimental.UtilityClass; -import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; +import java.util.Arrays; + /** * A few simple version comparison utilities. */ @UtilityClass -@Slf4j public class Versions { /** @@ -81,17 +81,29 @@ public static boolean isSameVersion(String left, String right) { } /** - * Performs a <=> comparison of numeric version numbers. When a section is determined to be non-numeric, a - * case-insensitive string comparison is performed. + * Performs a case-insensitive, segment by segment comparison of numeric and alphanumeric version numbers. Versions + * are split on periods and dashes. For example, the segments of "2.5.0" are "2", "5", and "0" while the segments of + * "1.0.0-alpha.3" are "1", "0", "0", "alpha", and "3". Returns -1, 0, or 1 as the "left" version is less than, + * equal to, or greater than the "right" version. (These return values correspond to the values returned by the + * {@link Integer#signum(int)} function.) + *

+ * When a segment is determined to be non-numeric, a case-insensitive string comparison is performed. When the + * number of segments in the version are different, then the general logic is that the shorter segment is + * the higher version. This covers commons situations such as 1.0.0-SNAPSHOT, 1.0.0-alpha, and 1.0.0-beta.2, which + * should all be lower versions than 1.0.0. * - * @param left the first version number (e.g. "1.2.3") - * @param right the second version number (e.g. "1.2.4") + * @param left the first version number (e.g. "1.2.3" or "2.0.0-alpha1") + * @param right the second version number (e.g. "1.2.4" or "2.0.0-alpha2") * @return -1 if "left" is less than "right", 0 if the versions are equal, and 1 if "left" is higher than "right" - * @implNote Current implementation works best when versions have the same number of segments, e.g. 2.1.0 vs 2.0.0. - * It works also with different number of segments when the different segments are numeric, - * e.g. 1.0.0 vs 1.0.0.42. It does NOT work so well right now when the last different segments are - * non-numeric, e.g. 1.0.0 vs 1.0.0-SNAPSHOT or 1.0.0-alpha (1.0.0-SNAPSHOT and 1.0.0-alpha are considered - * higher versions than 1.0.0 currently). See issue #45 which is intended to improve these comparisons. + * @implNote The current implementation works best when versions have the same number of segments, e.g. comparing + * 2.1.0 vs 2.0.0. It also works fine with different number of segments when those different segments are numeric, + * such as 1.0.0 vs 1.0.0.42 (the latter is higher). It also handles most normal cases when the last segments are + * different and are non-numeric, e.g. 1.0.0 should be considered a higher version than 1.0.0-SNAPSHOT or + * 1.0.0-alpha. There are various edge cases that might report results that might not be what you expect; for + * example, should 2.0.0-beta.1 be a higher or lower version than 2.0.0-beta? Currently 2.0.0-beta is reported as + * the higher version due to the simple implementation. However, note that 2.0.0-beta1 would be reported as higher + * than 2.0.0-beta (because the String "beta" is "greater than" the String "beta" using (Java) string comparison. + * @see Integer#signum(int) */ public static int versionCompare(String left, String right) { checkArgumentNotBlank(left, "left version cannot be blank"); @@ -102,23 +114,44 @@ public static int versionCompare(String left, String right) { return 0; } - // split versions on dot/period and dash - String[] leftParts = left.split("[\\.-]"); - String[] rightParts = right.split("[\\.-]"); + // 1. normalize versions to lowercase so all alphanumeric comparisons are case-insensitive + // 2. split versions on dot/period and dash + String[] leftParts = lowerCaseAndSplit(left); + String[] rightParts = lowerCaseAndSplit(right); - // find the first non-equal ordinal (or last segment of shortest version string) - int pos = 0; - while (pos < leftParts.length && pos < rightParts.length && leftParts[pos].equals(rightParts[pos])) { - pos++; - } + // find the first non-equal segment index (or the index of the last segment of the shorter version) + int pos = indexOfFirstUnequalOrLastCommonSegment(leftParts, rightParts); - // compare first non-equal value + // compare first non-equal value if we found an unequal segment before the end if (pos < leftParts.length && pos < rightParts.length) { return contextuallyCompare(leftParts[pos], rightParts[pos]); } - // the strings are so far equal or one is a substring of the other - return Integer.signum(leftParts.length - rightParts.length); + // one of the given version arguments is a substring of the other. e.g. 1.0.0-alpha1 contains 1.0.0 + + // if all segments are numeric, then whichever is longer is the higher version, + // e.g. 3.0.1 > 3.0 and 2.5.10.1 > 2.5.10 + if (allAreNumericIn(leftParts) && allAreNumericIn(rightParts)) { + return Integer.signum(leftParts.length - rightParts.length); + } + + // Not all segments are numeric. These segments are assumed to contain things like alpha, beta, SNAPSHOT, etc. + // Handle special cases such as alpha[.n], beta[.n], Mn (i.e. milestone, like M1, M2) in a generic manner such + // that whichever part is longer is considered the *lower* version. This simple logic means that 1.0.0.alpha1 + // is a lower version than 1.0.0, 2.5.0-SNAPSHOT is a lower version than 2.5.0, and so on. + return Integer.signum(rightParts.length - leftParts.length); + } + + private static int indexOfFirstUnequalOrLastCommonSegment(String[] leftParts, String[] rightParts) { + int pos = 0; + while (pos < leftParts.length && pos < rightParts.length && leftParts[pos].equals(rightParts[pos])) { + pos++; + } + return pos; + } + + private static String[] lowerCaseAndSplit(String version) { + return version.toLowerCase().split("[.-]"); } private static int contextuallyCompare(String leftPart, String rightPart) { @@ -135,7 +168,12 @@ private static int compareNumeric(String leftPart, String rightPart) { } private static int compareString(String leftPart, String rightPart) { - var result = StringUtils.compare(leftPart.toLowerCase(), rightPart.toLowerCase()); + // Both args should be lowercase, so can compare exactly + var result = StringUtils.compare(leftPart, rightPart); return Integer.compare(result, 0); } + + private static boolean allAreNumericIn(String[] elements) { + return Arrays.stream(elements).allMatch(StringUtils::isNumeric); + } } diff --git a/src/test/java/org/kiwiproject/base/VersionsTest.java b/src/test/java/org/kiwiproject/base/VersionsTest.java index 70e26117..df9a853e 100644 --- a/src/test/java/org/kiwiproject/base/VersionsTest.java +++ b/src/test/java/org/kiwiproject/base/VersionsTest.java @@ -59,10 +59,24 @@ void testVersions_Simple(String left, String right, int expectedResult) { @ParameterizedTest @CsvSource({ + "2.0.1, 2.0.1, 0", + "1.6.12, 1.6.12, 0", "2.0.1, 1.6.12, 1", + "1.6.12, 2.0.1, -1", "1.1.1-SNAPSHOT, 1.1.1-SNAPSHOT, 0", + "1.1.1-SNAPSHOT, 1.1.1-snapshot, 0", + "1.1.1-snapshot, 1.1.1-SNAPSHOT, 0", "1.0.1, 1.1.1, -1", - "1.1.10, 1.1.2, 1" + "1.1.10, 1.1.2, 1", + "1.4.2, 1.4.2, 0", + "1.4-2, 1.4.2, 0", + "1.4.2, 1.4-2, 0", + "2.0.0.0, 2.0.0.0, 0", + "2.0.0.1, 2.0.0.0, 1", + "2.0.0.0, 2.0.0.1, -1", + "2.5.10.1, 2.5.10.1, 0", + "2.5.10.2, 2.5.10.1, 1", + "2.5.10.1, 2.5.10.2, -1", }) void testVersions_SlightlyComplex(String left, String right, int expectedResult) { assertThat(Versions.versionCompare(left, right)).isEqualTo(expectedResult); @@ -72,6 +86,8 @@ void testVersions_SlightlyComplex(String left, String right, int expectedResult) @CsvSource({ "1.0.1a-SNAPSHOT, 1.0.1a-SNAPSHOT, 0", "0.a.1a, 0.a.1a, 0", + "0.a-1a, 0.a.1a, 0", + "0.a.1a, 0.a-1a, 0", "0.a.1a, 0.a.1b, -1", "0.a.1b, 0.a.1a, 1", "a.10.1, a.2.1, 1", @@ -83,10 +99,16 @@ void testVersions_SlightlyComplex(String left, String right, int expectedResult) "a.b.d, a.b.C, 1", "1.2.3.a, 1.2.b, -1", "1.2.b, 1.2.3.a, 1", + "2.5.0.a, 2.5.0.a, 0", + "2.5.0.b, 2.5.0.a, 1", + "2.5.0.a, 2.5.0.b, -1", + "5.4.1.FINAL, 5.4.1.final, 0", "5.4.1.Final, 5.4.1.Final, 0", "5.4.2.Final, 5.4.1.Final, 1", "5.5.2.Final, 5.4.1.Final, 1", "5.4.0.Final, 5.4.1.Final, -1", + "6.0.0.Alpha7, 5.4.30.Final, 1", + "5.4.30.Final, 6.0.0.Alpha7, -1", "1.0.0-alpha, 1.0.0-alpha, 0", "1.1.0-alpha, 1.0.0-alpha, 1", "1.0.0-alpha, 1.1.0-alpha, -1", @@ -98,7 +120,65 @@ void testVersions_SlightlyComplex(String left, String right, int expectedResult) "1.0.0-Alpha, 1.0.0-Beta, -1", "1.0.0-Beta, 1.0.0-Alpha, 1", "1.0.0-alpha, 1.0.0-Beta, -1", - "1.0.0-beta, 1.0.0-Alpha, 1" + "1.0.0-beta, 1.0.0-Alpha, 1", + "2.0.0-alpha2, 2.0.0-alpha1, 1", + "2.0.0-alpha1, 2.0.0-alpha2, -1", + "2.0.0-alpha2, 2.0.0-alpha2, 0", + "2.0.0-alpha.2, 2.0.0-alpha.1, 1", + "2.0.0-alpha.1, 2.0.0-alpha.2, -1", + "1.0.0-beta.2, 1.0.0-beta.1, 1", + "1.0.0-beta.1, 2.0.0-beta.2, -1", + "1.0.0-beta.2, 1.0.0-beta-1, 1", + "1.0.0-beta-1, 2.0.0-beta-2, -1", + "11.0.0-alpha0, 11.0.0-alpha0, 0", + "11.0.0-alpha1, 11.0.0-alpha0, 1", + "11.0.0-alpha0, 11.0.0-alpha1, -1", + "11.0.0-beta0, 11.0.0-alpha3, 1", + "11.0.0-beta2, 11.0.0-beta3, -1", + "5.8.0-M2, 5.8.0-M1, 1", + "5.8.0-M1, 5.8.0-M2, -1", + "2.0.0-Alpha.2, 2.0.0-alpha.1, 1", + "2.0.0-alpha.1, 2.0.0-Alpha.2, -1", + "1.0.0, 1.0.0-alpha, 1", + "1.0.0-alpha, 1.0.0, -1", + "2.0.0, 2.0.0-beta, 1", + "2.0.0-beta, 2.0.0, -1", + "2.0.0-beta1, 2.0.0-beta, 1", + "2.0.0-beta, 2.0.0-beta1, -1", + "2.0.0-beta.1, 2.0.0.beta.0, 1", + "2.0.0-beta.0, 2.0.0-beta.1, -1", + "2.0.0-beta.b, 2.0.0-beta.a, 1", + "2.0.0-beta.a, 2.0.0-beta.b, -1", + "2.0.0, 2.0.0-beta.1, 1", + "2.0.0-beta.1, 2.0.0, -1", + "2.0.0-beta.3, 2.0.0-beta.3, 0", + "2.0.0-beta.3, 2.0.0-beta-3, 0", + "2.0.0-beta-3, 2.0.0-beta.3, 0", + "1.0.0, 1.0.0-SNAPSHOT, 1", + "1.0.0-SNAPSHOT, 1.0.0, -1", + "5.8.0, 5.8.0-M1, 1", + "5.8.0-M1, 5.8.0-M1, 0", + "5.8.0-M1, 5.8.0, -1", + "10.4.39.v20210325, 9.4.39.v20210325, 1", + "9.4.39.v20210325, 10.4.39.v20210325, -1", + "9.3.40.v20210325, 9.3.39.v20210325, 1", + "9.3.39.v20210325, 9.3.40.v20210325, -1", + "9.4.39.v20210325, 9.4.39.v20210325, 0", + "9.4.39.v20210326, 9.4.39.v20210325, 1", + "9.4.39.v20210325, 9.4.39.v20210326, -1", + "42.2.19.jre7, 42.2.19.jre7, 0", + "42.2.19, 42.2.19.jre7, 1", + "42.2.19.jre7, 42.2.19, -1", + "30.1.1-jre, 30.1.1-jre, 0", + "30.1.2-jre, 30.1.1-jre, 1", + "30.1.1-jre, 30.1.2-jre, -1", + "30.2-jre, 30.1.1-jre, 1", + "30.1.1-jre, 30.2-jre, -1", + "2.2.11.RELEASE, 2.2.11.RELEASE, 0", + "2.2.12.RELEASE, 2.2.11.RELEASE, 1", + "2.2.11.RELEASE, 2.2.12.RELEASE, -1", + "3.0.0, 2.2.12.RELEASE, 1", + "2.2.12.RELEASE, 3.0.0, -1", }) void testVersions_ComplexWithAlphanumerics(String left, String right, int expectedResult) { assertThat(Versions.versionCompare(left, right)).isEqualTo(expectedResult);