You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2017/06/30 14:02:58 UTC

[10/27] brooklyn-server git commit: add lots of text explaining BrooklynVersionSyntax, update comparator, tests, and usages

add lots of text explaining BrooklynVersionSyntax, update comparator, tests, and usages


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/45cb10a5
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/45cb10a5
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/45cb10a5

Branch: refs/heads/master
Commit: 45cb10a57af9e4aba0e7d624f1af6bfb25e5cee2
Parents: cc70392
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jun 21 17:04:04 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jun 21 17:04:04 2017 +0100

----------------------------------------------------------------------
 .../catalog/internal/BasicBrooklynCatalog.java  | 16 ++--
 .../internal/CatalogItemDtoAbstract.java        |  4 +-
 .../core/typereg/RegisteredTypeNaming.java      | 52 ++++++------
 .../internal/CatalogItemComparatorTest.java     |  9 +-
 .../catalog/internal/CatalogVersioningTest.java |  1 +
 .../core/typereg/RegisteredTypeNamingTest.java  | 59 +++++++------
 .../util/text/BrooklynVersionSyntax.java        | 87 ++++++++++++++++++++
 .../brooklyn/util/text/VersionComparator.java   | 11 ++-
 .../util/text/ComparableVersionTest.java        |  5 +-
 9 files changed, 180 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
index e4c9882..006d3e8 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
@@ -602,11 +602,14 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         // if symname not set, infer from: id, then name, then item id, then item name
         if (Strings.isBlank(symbolicName)) {
             if (Strings.isNonBlank(id)) {
-                if (RegisteredTypeNaming.isGoodTypeColonVersion(id)) {
+                if (RegisteredTypeNaming.isGoodBrooklynTypeColonVersion(id)) {
                     symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(id);
+                } else if (RegisteredTypeNaming.isValidOsgiTypeColonVersion(id)) {
+                    symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(id);
+                    log.warn("Discouraged version syntax in id '"+id+"'; version should comply with brooklyn recommendation (#.#.#-qualifier or portion) or specify symbolic name and version explicitly, not OSGi version syntax");
                 } else if (CatalogUtils.looksLikeVersionedId(id)) {
                     // use of above method is deprecated in 0.12; this block can be removed in 0.13
-                    log.warn("Discouraged version syntax in id '"+id+"'; version should comply with OSGi specs (#.#.#.qualifier or portion) or specify symbolic name and version explicitly");
+                    log.warn("Discouraged version syntax in id '"+id+"'; version should comply with brooklyn recommendation (#.#.#-qualifier or portion) or specify symbolic name and version explicitly");
                     symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(id);
                 } else if (RegisteredTypeNaming.isUsableTypeColonVersion(id)) {
                     log.warn("Deprecated type naming syntax in id '"+id+"'; colons not allowed in type name as it is used to indicate version");
@@ -618,7 +621,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
                     symbolicName = id;
                 }
             } else if (Strings.isNonBlank(name)) {
-                if (RegisteredTypeNaming.isGoodTypeColonVersion(name)) {
+                if (RegisteredTypeNaming.isGoodBrooklynTypeColonVersion(name) || RegisteredTypeNaming.isValidOsgiTypeColonVersion(name)) {
                     log.warn("Deprecated use of 'name' key to define '"+name+"'; version should be specified within 'id' key or with 'version' key, not this tag");
                     // deprecated in 0.12; remove in 0.13
                     symbolicName = CatalogUtils.getSymbolicNameFromVersionedId(name);
@@ -646,10 +649,13 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         }
 
         String versionFromId = null;
-        if (RegisteredTypeNaming.isGoodTypeColonVersion(id)) {
+        if (RegisteredTypeNaming.isGoodBrooklynTypeColonVersion(id)) {
+            versionFromId = CatalogUtils.getVersionFromVersionedId(id);
+        } else if (RegisteredTypeNaming.isValidOsgiTypeColonVersion(id)) {
             versionFromId = CatalogUtils.getVersionFromVersionedId(id);
+            log.warn("Discouraged version syntax in id '"+id+"'; version should comply with Brooklyn recommended version syntax (#.#.#-qualifier or portion) or specify symbolic name and version explicitly, not OSGi");
         } else if (CatalogUtils.looksLikeVersionedId(id)) {
-            log.warn("Discouraged version syntax in id '"+id+"'; version should comply with OSGi specs (#.#.#.qualifier or portion) or specify symbolic name and version explicitly");
+            log.warn("Discouraged version syntax in id '"+id+"'; version should comply with Brooklyn recommended version syntax (#.#.#-qualifier or portion) or specify symbolic name and version explicitly");
             // remove in 0.13
             versionFromId = CatalogUtils.getVersionFromVersionedId(id);
         } else if (RegisteredTypeNaming.isUsableTypeColonVersion(id)) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java
index 3f69e67..d098cdd 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java
@@ -420,7 +420,7 @@ public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynO
                     name = null;
                     version = null;
                     url = inlineRef;
-                } else if (RegisteredTypeNaming.isGoodTypeColonVersion(inlineRef)) {
+                } else if (RegisteredTypeNaming.isGoodBrooklynTypeColonVersion(inlineRef) || RegisteredTypeNaming.isValidOsgiTypeColonVersion(inlineRef)) {
                     //looks like a name+version ref
                     name = CatalogUtils.getSymbolicNameFromVersionedId(inlineRef);
                     version = CatalogUtils.getVersionFromVersionedId(inlineRef);
@@ -444,6 +444,8 @@ public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynO
                     url = inlineRef;
                 }
 
+                // TODO
+//                version = RegisteredTypeNaming.toOsgiVersion(version, "library reference "+name+":"+version);
                 dto.add(new CatalogBundleDto(name, version, url));
             } else {
                 LOG.debug("Unexpected entry in libraries list neither string nor map: " + object);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java
index 22e9e78..61b999e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypeNaming.java
@@ -18,6 +18,8 @@
  */
 package org.apache.brooklyn.core.typereg;
 
+import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
+
 /**
  * Methods for testing validity of names and decomposing them. 
  * Mostly based on OSGi, specifically sections 1.3.2 and 3.2.5 of 
@@ -31,16 +33,6 @@ public class RegisteredTypeNaming {
     public final static String OSGI_TOKEN_CHARS = "A-Za-z0-9_-";
     public final static String OSGI_TOKEN_REGEX = "[" + OSGI_TOKEN_CHARS + "]+";
     public final static String OSGI_SYMBOLIC_NAME_REGEX = OSGI_TOKEN_REGEX + "(" + DOT + OSGI_TOKEN_REGEX + ")*";
-    public final static String NUMBER = "[0-9]+";
-    public final static String QUALIFIER = OSGI_TOKEN_REGEX;
-    public final static String VERSION_REGEX = 
-        NUMBER + 
-            "(" + "\\." + NUMBER +  
-                "(" + "\\." + NUMBER +  
-                    "(" + "\\." + QUALIFIER +  
-                    ")?" +
-                ")?" +
-            ")?";
 
     private static boolean isUsable(String candidate) {
         return candidate!=null && candidate.matches(USABLE_REGEX);
@@ -71,22 +63,19 @@ public class RegisteredTypeNaming {
         return isUsable(candidate) && candidate.matches(OSGI_SYMBOLIC_NAME_REGEX);
     }
 
-    /** 
-     * For versions we currently work with any non-empty string that does not contain a ':' or whitespace.
-     * However we discourage things that are not OSGi versions; see {@link #isOsgiLegalVersion(String)}. 
-     * In some places (eg bundles) the use of OSGi version syntax may be enforced.  
-     */
+    /** @see BrooklynVersionSyntax#isUsableVersion(String) */
     public static boolean isUsableVersion(String candidate) {
-        return isUsable(candidate);
+        return BrooklynVersionSyntax.isUsableVersion(candidate);
     }
     
-    /** True if the argument matches the OSGi version spec, of the form 
-     * <code>MAJOR.MINOR.POINT.QUALIFIER</code> or part thereof (not ending in a period though),
-     * where the first three are whole numbers and the final piece is any valid OSGi token
-     * (something satisfying {@link #isGoodTypeName(String)} with no periods).
-     */
-    public static boolean isOsgiLegalVersion(String candidate) {
-        return candidate!=null && candidate.matches(VERSION_REGEX);
+    /** @see BrooklynVersionSyntax#isGoodBrooklynVersion(String) */
+    public static boolean isGoodBrooklynVersion(String candidate) {
+        return BrooklynVersionSyntax.isGoodBrooklynVersion(candidate);
+    }
+    
+    /** @see BrooklynVersionSyntax#isValidOsgiVersion(String) */
+    public static boolean isValidOsgiVersion(String candidate) {
+        return BrooklynVersionSyntax.isValidOsgiVersion(candidate);
     }
 
     /** True if the argument has exactly one colon, and the part before
@@ -98,14 +87,25 @@ public class RegisteredTypeNaming {
 
     /** True if the argument has exactly one colon, and the part before
      * satisfies {@link #isGoodTypeName(String)} and the part after 
-     * {@link #isOsgiLegalVersion(String)}. */
-    public static boolean isGoodTypeColonVersion(String candidate) {
+     * {@link #isGoodBrooklynVersion(String)}. */
+    public static boolean isGoodBrooklynTypeColonVersion(String candidate) {
         if (candidate==null) return false;
         int idx = candidate.indexOf(':');
         if (idx<=0) return false;
         if (!isGoodTypeName(candidate.substring(0, idx))) return false;
-        if (!isOsgiLegalVersion(candidate.substring(idx+1))) return false;
+        if (!isGoodBrooklynVersion(candidate.substring(idx+1))) return false;
         return true;
     }
 
+    /** True if the argument has exactly one colon, and the part before
+     * satisfies {@link #isGoodTypeName(String)} and the part after 
+     * {@link #isValidOsgiVersion(String)}. */
+    public static boolean isValidOsgiTypeColonVersion(String candidate) {
+        if (candidate==null) return false;
+        int idx = candidate.indexOf(':');
+        if (idx<=0) return false;
+        if (!isGoodTypeName(candidate.substring(0, idx))) return false;
+        if (!isValidOsgiVersion(candidate.substring(idx+1))) return false;
+        return true;
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogItemComparatorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogItemComparatorTest.java b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogItemComparatorTest.java
index 3c8ce83..9f2aa2c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogItemComparatorTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogItemComparatorTest.java
@@ -62,12 +62,15 @@ public class CatalogItemComparatorTest {
         compare(STABLE, STABLE, 0);
 
         compare(STABLE, "10.6", 1);
-        compare(STABLE, "10.5.8.1", 1);
 
-        compare("10.5.8-rc2", "10.5.8-rc3", 1) ;
-        compare("10.5.8-rc2", "10.5.8-rc1", -1);
+        compare(RC2, "10.5.8-rc3", 1) ;
+        compare(RC2, "10.5.8-rc1", -1);
 
         compare(STABLE, RC2, -1);
+        // surprising one, remember .1 is qualifier
+        compare(STABLE, "10.5.8.1", -1);
+        compare(STABLE, "10.5.8-1", -1);
+        compare(RC2, "10.5.8-1", -1);
 
         CatalogItemComparator cmp = CatalogItemComparator.INSTANCE;
         assertTrue(cmp.compare(v(RC2), v("10.5.8-beta1")) == cmp.compare(v(RC2), v("10.5.8-beta3")));

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java
index 3c09bb9..6748fdf 100644
--- a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogVersioningTest.java
@@ -69,6 +69,7 @@ public class CatalogVersioningTest {
         assertLegacyVersionParsesAs("http://foo:8080", null, null);
     }
 
+    @SuppressWarnings("deprecation")
     private static void assertLegacyVersionParsesAs(String versionedId, String id, String version) {
         if (version==null) {
             Assert.assertFalse(CatalogUtils.looksLikeVersionedId(versionedId));

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java b/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java
index 81b69bf..ec07c8a 100644
--- a/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/typereg/RegisteredTypeNamingTest.java
@@ -18,6 +18,7 @@
  */
 package org.apache.brooklyn.core.typereg;
 
+import org.apache.brooklyn.util.osgi.OsgiUtils;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -41,47 +42,55 @@ public class RegisteredTypeNamingTest {
     }
 
     public void testVersions() {
-        assertVersion("1", true, true);
-        assertVersion("1.0.0", true, true);
-        assertVersion("1.0.0.SNAPSHOT", true, true);
+        assertVersion("1", true, true, true);
+        assertVersion("1.0.0", true, true, true);
+        assertVersion("1.0.0.SNAPSHOT", true, true, false);
+        assertVersion("1.0.0-SNAPSHOT", true, false, true);
         
-        assertVersion("", false, false);
-        assertVersion(null, false, false);
-        assertVersion("1:1", false, false);
+        assertVersion("", false, false, false);
+        assertVersion(null, false, false, false);
+        assertVersion("1:1", false, false, false);
         
-        assertVersion("1.SNAPSHOT", true, false);
-        assertVersion("1.0.0_SNAPSHOT", true, false);
-        assertVersion(".1", true, false);
-        assertVersion("v1", true, false);
-        assertVersion("!$&", true, false);
+        assertVersion("1.SNAPSHOT", true, false, false);
+        assertVersion("1.0.0_SNAPSHOT", true, false, false);
+        assertVersion(".1", true, false, false);
+        assertVersion("v1", true, false, false);
+        assertVersion("!$&", true, false, false);
     }
     
     public void testNameColonVersion() {
-        assertNameColonVersion("foo:1", true, true);
-        assertNameColonVersion("1:1", true, true);
-        assertNameColonVersion("a-package.1-foo-bar:1.0.0.SNAPSHOT_dev", true, true);
+        assertNameColonVersion("foo:1", true, true, true);
+        assertNameColonVersion("1:1", true, true, true);
+        assertNameColonVersion("a-package.1-foo-bar:1.0.0.SNAPSHOT_dev", true, true, false);
+        assertNameColonVersion("a-package.1-foo-bar:1.0.0-SNAPSHOT_dev", true, false, true);
         
-        assertNameColonVersion("", false, false);
-        assertNameColonVersion(null, false, false);
-        assertNameColonVersion("foo:", false, false);
-        assertNameColonVersion(":1", false, false);
+        assertNameColonVersion("", false, false, false);
+        assertNameColonVersion(null, false, false, false);
+        assertNameColonVersion("foo:", false, false, false);
+        assertNameColonVersion(":1", false, false, false);
         
-        assertNameColonVersion("foo:1.SNAPSHOT", true, false);
-        assertNameColonVersion("foo:v1", true, false);
-        assertNameColonVersion("foo...bar!:1", true, false);
+        assertNameColonVersion("foo:1.SNAPSHOT", true, false, false);
+        assertNameColonVersion("foo:v1", true, false, false);
+        assertNameColonVersion("foo...bar!:1", true, false, false);
     }
     
     private void assertName(String candidate, boolean isUsable, boolean isGood) {
         Assert.assertEquals(RegisteredTypeNaming.isUsableTypeName(candidate), isUsable, "usable name '"+candidate+"'");
         Assert.assertEquals(RegisteredTypeNaming.isGoodTypeName(candidate), isGood, "good name '"+candidate+"'");
     }
-    private void assertVersion(String candidate, boolean isUsable, boolean isGood) {
+    private void assertVersion(String candidate, boolean isUsable, boolean isOsgi, boolean isGood) {
         Assert.assertEquals(RegisteredTypeNaming.isUsableVersion(candidate), isUsable, "usable version '"+candidate+"'");
-        Assert.assertEquals(RegisteredTypeNaming.isOsgiLegalVersion(candidate), isGood, "good version '"+candidate+"'");
+        Assert.assertEquals(RegisteredTypeNaming.isValidOsgiVersion(candidate), isOsgi, "osgi version '"+candidate+"'");
+        Assert.assertEquals(RegisteredTypeNaming.isGoodBrooklynVersion(candidate), isGood, "good version '"+candidate+"'");
     }
-    private void assertNameColonVersion(String candidate, boolean isUsable, boolean isGood) {
+    private void assertNameColonVersion(String candidate, boolean isUsable, boolean isOsgi, boolean isGood) {
         Assert.assertEquals(RegisteredTypeNaming.isUsableTypeColonVersion(candidate), isUsable, "usable name:version '"+candidate+"'");
-        Assert.assertEquals(RegisteredTypeNaming.isGoodTypeColonVersion(candidate), isGood, "good name:version '"+candidate+"'");
+        Assert.assertEquals(RegisteredTypeNaming.isValidOsgiTypeColonVersion(candidate), isOsgi, "osgi name:version '"+candidate+"'");
+        Assert.assertEquals(RegisteredTypeNaming.isGoodBrooklynTypeColonVersion(candidate), isGood, "good name:version '"+candidate+"'");
     }
 
+    public void testConvertToOsgiVersion() {
+        Assert.assertEquals(OsgiUtils.toOsgiVersion("1-foo"), "1.0.0.foo");
+        Assert.assertEquals(OsgiUtils.toOsgiVersion("1"), "1.0.0");
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/utils/common/src/main/java/org/apache/brooklyn/util/text/BrooklynVersionSyntax.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/BrooklynVersionSyntax.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/BrooklynVersionSyntax.java
new file mode 100644
index 0000000..9a59d11
--- /dev/null
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/BrooklynVersionSyntax.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.util.text;
+
+/** Utilities for parsing and working with versions following the recommended Brooklyn scheme,
+ * following <code>major.minor.patch-qualifier</code> syntax,
+ * with support for mapping to OSGi. 
+ * <p>
+ * See {@link VersionComparator} and its tests for examples.
+ */
+public class BrooklynVersionSyntax {
+
+    private static final String USABLE_REGEX = "[^:\\s/\\\\]+";
+
+    private final static String OSGI_TOKEN_CHARS = "A-Za-z0-9_-";
+    private final static String OSGI_TOKEN_REGEX = "[" + OSGI_TOKEN_CHARS + "]+";
+    private final static String NUMBER = "[0-9]+";
+    private final static String QUALIFIER = OSGI_TOKEN_REGEX;
+    
+    public final static String VALID_OSGI_VERSION_REGEX = 
+        NUMBER + 
+            "(" + "\\." + NUMBER +  
+                "(" + "\\." + NUMBER +  
+                    "(" + "\\." + QUALIFIER +  
+                    ")?" +
+                ")?" +
+            ")?";
+    
+    public final static String GOOD_BROOKLYN_VERSION_REGEX = 
+        NUMBER + 
+            "(" + "\\." + NUMBER +  
+                "(" + "\\." + NUMBER +  
+                    "(" + "-" + QUALIFIER +  
+                    ")?" +
+                ")?" +
+            ")?";
+    
+    private static boolean isUsable(String candidate) {
+        return candidate!=null && candidate.matches(USABLE_REGEX);
+    }
+    
+    /** 
+     * For versions we currently work with any non-empty string that does not contain a ':' or whitespace.
+     * However we discourage things that are not OSGi versions; see {@link #isValidOsgiVersion(String)}. 
+     * In some places (eg bundles) the use of OSGi version syntax may be enforced.  
+     */
+    public static boolean isUsableVersion(String candidate) {
+        return isUsable(candidate);
+    }
+    
+    /** True if the argument matches the Brooklyn version syntax, 
+     * <code>MAJOR.MINOR.POINT-QUALIFIER</code> or part thereof (not ending in a period though),
+     * where the first three are whole numbers and the final piece is any valid OSGi token
+     * (something satisfying {@link #isGoodTypeName(String)} with no periods).
+     * See also {@link #isValidOsgiVersion(String)} and note this _requires_ a different separator to OSGi.
+     */
+    public static boolean isGoodBrooklynVersion(String candidate) {
+        return candidate!=null && candidate.matches(GOOD_BROOKLYN_VERSION_REGEX);
+    }
+    
+    /** True if the argument matches the OSGi version spec, of the form 
+     * <code>MAJOR.MINOR.POINT.QUALIFIER</code> or part thereof (not ending in a period though),
+     * where the first three are whole numbers and the final piece is any valid OSGi token
+     * (something satisfying {@link #isGoodTypeName(String)} with no periods).
+     * See also {@link #isGoodVersion(String)}.
+     */
+    public static boolean isValidOsgiVersion(String candidate) {
+        return candidate!=null && candidate.matches(VALID_OSGI_VERSION_REGEX);
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java
index e5d1fbf..09f430d 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java
@@ -27,11 +27,18 @@ import com.google.common.base.Objects;
 /**
  * {@link Comparator} for version strings.
  * <p>
- * SNAPSHOT items always lowest rated, then looking for no qualifier, then doing natural order comparison.
- * This gives the desired semantics for our recommended version syntax.
+ * This gives the desired semantics for our recommended version syntax,
+ * following <code>major.minor.patch-qualifier</code> syntax, 
+ * doing numeric order of major/minor/patch (1.10 > 1.9),
+ * treating anything with a qualifier lower than the corresponding without but higher than items before
+ * (1.2 > 1.2-rc > 1.1),
+ * SNAPSHOT items always lowest rated (1.0 > 2-SNAPSHOT), and 
+ * qualifier sorting follows {@link NaturalOrderComparator} (1-M10 > 1-M9).
  * <p>
  * Impossible to follow semantics for all versioning schemes but 
  * does the obvious right thing for normal schemes and pretty well in fringe cases.
+ * Probably the most surprising fringe behaviours will be
+ * 1.2.3.4 < 1.2.3 (the ".4" is considered a qualifier).
  * <p>
  * See test case for lots of examples.
  */

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/45cb10a5/utils/common/src/test/java/org/apache/brooklyn/util/text/ComparableVersionTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/text/ComparableVersionTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/text/ComparableVersionTest.java
index a730ade..059e2e3 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/text/ComparableVersionTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/text/ComparableVersionTest.java
@@ -35,8 +35,9 @@ public class ComparableVersionTest {
         Assert.assertTrue(v.isLessThanAndNotEqualTo("10.6"));
         Assert.assertTrue(v.isLessThanOrEqualTo("10.5.8"));
         Assert.assertFalse(v.isLessThanAndNotEqualTo("10.5.8"));
-        
-        Assert.assertTrue(v.isLessThanAndNotEqualTo("10.5.8.1"));
+
+        // this one is surprising -- but we don't support sub-patch numbers, the ".1" is a qualifier
+        Assert.assertTrue(v.isGreaterThanAndNotEqualTo("10.5.8.1"));
         
         Assert.assertTrue(v_rc2.isLessThanAndNotEqualTo("10.5.8-rc3")) ;
         Assert.assertTrue(v_rc2.isGreaterThanAndNotEqualTo("10.5.8-rc1"));