brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From geomacy <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #740: New versioning rules prep
Date Tue, 27 Jun 2017 16:06:15 GMT
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/740#discussion_r124278481
  
    --- 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);
    +        if (uq!=0) return uq;
             
    -        for (int i = 0; ; i++) {
    -            if (i >= v1Parts.length && i >= v2Parts.length) {
    -                // end of both
    -                return 0;
    -            }
    -            if (i == v1Parts.length) {
    -                // shorter set always wins here; i.e.
    -                // 1-qualifier < 1
    -                return 1;
    -            }
    -            if (i == v2Parts.length) {
    -                // as above but inverted
    -                return -1;
    -            }
    -            // not at end; compare this dot split part
    -            
    -            String v1p = v1Parts[i];
    -            String v2p = v2Parts[i];
    -            
    -            if (v1p.equals(v2p)) continue;
    -            
    -            if (isNumberInFirstChar(v1p) || isNumberInFirstChar(v2p)) {
    -                // something starting with a number is higher than something not
    -                if (!isNumberInFirstChar(v1p)) return -1;
    -                if (!isNumberInFirstChar(v2p)) return 1;
    -                
    -                // both start with numbers; can use natural order comparison *unless*
    -                // one is purely a number AND the other *begins* with that number,
    -                // followed by non-digit chars, in which case prefer the pure number
    -                // ie:
    -                //           1beta < 1
    -                // but note
    -                //            1 < 2beta < 11beta
    -                if (isNumber(v1p) || isNumber(v2p)) {
    -                    if (!isNumber(v1p)) {
    -                        if (v1p.startsWith(v2p)) {
    -                            if (!isNumberInFirstChar(Strings.removeFromStart(v1p, v2p)))
{
    -                                // v2 is a number, and v1 is the same followed by non-numbers
    -                                return -1;
    -                            }
    -                        }
    -                    }
    -                    if (!isNumber(v2p)) {
    -                        // as above but inverted
    -                        if (v2p.startsWith(v1p)) {
    -                            if (!isNumberInFirstChar(Strings.removeFromStart(v2p, v1p)))
{
    -                                return 1;
    -                            }
    -                        }
    -                    }
    -                    // both numbers, skip to natural order comparison
    -                }
    -            }
    -            
    -            // otherwise it is in-order
    -            int result = NaturalOrderComparator.INSTANCE.compare(v1p, v2p);
    -            if (result!=0) return result;
    -        }
    -    }
    +        // qualified items are lower than unqualified items
    +        TwoBooleans no_qualifier = TwoBooleans.of(u1.equals(v1), u2.equals(v2));
    +        if (no_qualifier.different()) return no_qualifier.compare(false);
     
    -    @VisibleForTesting
    -    static String[] splitOnNonWordChar(String v) {
    -        Collection<String> parts = new ArrayList<String>();
    -        String remaining = v;
    +        // now compare qualifiers
    +        
    +        // allow -, ., and _ as qualifier offsets; if used, compare qualifer without
that
    +        String q1 = v1.substring(u1.length());
    +        String q2 = v2.substring(u2.length());
    +        
    +        String qq1 = q1, qq2 = q2;
    +        if (q1.startsWith("-") || q1.startsWith(".") || q1.startsWith("_")) q1 = q1.substring(1);
    +        if (q2.startsWith("-") || q2.startsWith(".") || q2.startsWith("_")) q2 = q2.substring(1);
    +        uq = NaturalOrderComparator.INSTANCE.compare(q1, q2);
    --- End diff --
    
    I don't think we should be using `NaturalOrderComparator` for this purpose, see my comments
[here](https://github.com/apache/brooklyn-docs/pull/198/files#r124223906).  We should just
use simple lexicographic comparison.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message