Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/740#discussion_r124512308
--- 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 --
But it's surely not correct for Versions - I agree `NaturalOrderComparator`
should return 0 only if the Strings are truly equal; I don't think it's the
right thing to use for the major/minor/patch - these are meant to be compared
numerically.
---
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.
---
