Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/740#discussion_r124511380
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java
---
@@ -56,149 +56,69 @@ public static boolean isSnapshot(String version) {
if (version==null) return false;
return version.toUpperCase().contains(SNAPSHOT);
}
+
+ @SuppressWarnings("unused")
+ private static class TwoBooleans {
+ private final boolean b1, b2;
+ public TwoBooleans(boolean b1, boolean b2) { this.b1 = b1; this.b2
= b2; }
+ boolean bothTrue() { return b1 && b2; }
+ boolean eitherTrue() { return b1 || b2; }
+ boolean bothFalse() { return !eitherTrue(); }
+ boolean same() { return b1==b2; }
+ boolean different() { return b1!=b2; }
+ int compare(boolean trueIsLess) { return same() ? 0 :
b1==trueIsLess ? -1 : 1; }
+ public static TwoBooleans of(boolean v1, boolean v2) {
+ return new TwoBooleans(v1, v2);
+ }
+ }
@Override
public int compare(String v1, String v2) {
- if (v1==null && v2==null) return 0;
- if (v1==null) return -1;
- if (v2==null) return 1;
+ if (Objects.equal(v1, v2)) return 0;
- boolean isV1Snapshot = isSnapshot(v1);
- boolean isV2Snapshot = isSnapshot(v2);
- if (isV1Snapshot == isV2Snapshot) {
- // if snapshot status is the same, look at dot-split parts
first
- return compareDotSplitParts(splitOnDot(v1), splitOnDot(v2));
- } else {
- // snapshot goes first
- return isV1Snapshot ? -1 : 1;
- }
- }
+ TwoBooleans nulls = TwoBooleans.of(v1==null, v2==null);
+ if (nulls.eitherTrue()) return nulls.compare(true);
- @VisibleForTesting
- static String[] splitOnDot(String v) {
- return v.split("(?<=\\.)|(?=\\.)");
- }
-
- private int compareDotSplitParts(String[] v1Parts, String[] v2Parts) {
- for (int i = 0; ; i++) {
- if (i >= v1Parts.length && i >= v2Parts.length) {
- // end of both
- return 0;
- }
- if (i == v1Parts.length) {
- // sequence depends whether the extra part *starts with* a
number
- // ie
- // 2.0 < 2.0.0
- // and
- // 2.0.qualifier < 2.0 < 2.0.0qualifier <
2.0.0-qualifier < 2.0.0.qualifier < 2.0.0 < 2.0.9-qualifier
- return isNumberInFirstCharPossiblyAfterADot(v2Parts, i) ?
-1 : 1;
- }
- if (i == v2Parts.length) {
- // as above but inverted
- return isNumberInFirstCharPossiblyAfterADot(v1Parts, i) ?
1 : -1;
- }
- // not at end; compare this dot split part
-
- int result = compareDotSplitPart(v1Parts[i], v2Parts[i]);
- if (result!=0) return result;
- }
- }
-
- private int compareDotSplitPart(String v1, String v2) {
- String[] v1Parts = splitOnNonWordChar(v1);
- String[] v2Parts = splitOnNonWordChar(v2);
+ TwoBooleans snapshots = TwoBooleans.of(isSnapshot(v1),
isSnapshot(v2));
+ if (snapshots.different()) return snapshots.compare(true);
+
+ String u1 = versionWithQualifier(v1);
+ String u2 = versionWithQualifier(v2);
+ int uq = NaturalOrderComparator.INSTANCE.compare(u1, u2);
--- End diff --
i think it's important for this comparator not to return `0` unless strings
are truly equal; otherwise we risk indeterminate ordering for things which
other routines consider different.
therefore in perverse cases we return a consistent ordering, even if it
seems a little arbitrary.
in this case, if someone did perversely use `1.01`, `1.1`, `1.09`, `1.9`,
and `1.10` as versions, i think that order -- which is what we return -- is the
least surprising.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
