forgive spaces and give better error if not quoted, in manifest headers

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

Branch: refs/heads/master
Commit: b05b8195eed5882d0ef80de268493a1f2b075f75
Parents: 9045005
Author: Alex Heneveld <[email protected]>
Authored: Fri Oct 27 11:01:10 2017 +0100
Committer: Alex Heneveld <[email protected]>
Committed: Fri Oct 27 11:57:28 2017 +0100

----------------------------------------------------------------------
 .../core/typereg/BundleUpgradeParser.java       | 11 ++++++-
 .../core/typereg/BundleUpgradeParserTest.java   | 34 +++++++++++++++++++-
 .../org/apache/brooklyn/util/text/Strings.java  |  7 ++++
 3 files changed, 50 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b05b8195/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
index 69bd583..72603e2 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
@@ -29,6 +29,7 @@ import java.util.Set;
 
 import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.typereg.RegisteredType;
+import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
 import org.apache.brooklyn.util.text.QuotedStringTokenizer;
@@ -297,7 +298,15 @@ public class BundleUpgradeParser {
         
         List<VersionRangedName> versionedItems = new ArrayList<>();
         for (String val : vals) {
-            versionedItems.add(VersionRangedName.fromString(val, 
singleVersionIsOsgiRange));
+            try {
+                versionedItems.add(VersionRangedName.fromString(val.trim(), 
singleVersionIsOsgiRange));
+            } catch (Exception e) {
+                if (Strings.containsAny(val, "(", ")", "[", "]") &&
+                        !Strings.containsAny(val, "'", "\"")) {
+                    throw Exceptions.propagateAnnotated("Entry cannot be 
parsed. If defining a range on an entry you must quote the entry.", e);
+                }
+                throw Exceptions.propagate(e);
+            }
         }
         return versionedItems;
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b05b8195/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
 
b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
index 341cccb..6e638d0 100644
--- 
a/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
+++ 
b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
@@ -151,6 +151,18 @@ public class BundleUpgradeParserTest {
         Bundle bundle = newMockBundle(ImmutableMap.of(
                 BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS, 
"\"foo:[0,1.0.0)\",\"bar:[0,1.0.0)\"",
                 BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_BUNDLES, 
"\"org.example.brooklyn.mybundle:[0,1.0.0)\""));
+        checkParse(bundle);
+    }
+
+    @Test
+    public void testParseBundleManifestWithSpaces() throws Exception {
+        Bundle bundle = newMockBundle(ImmutableMap.of(
+                BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS, 
"\"foo:[0,1.0.0)\", \"bar:[0,1.0.0)\"",
+                BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_BUNDLES, " 
\"org.example.brooklyn.mybundle:[0,1.0.0)\""));
+        checkParse(bundle);
+    }
+
+    protected void checkParse(Bundle bundle) {
         Supplier<Iterable<RegisteredType>> typeSupplier = 
Suppliers.ofInstance(ImmutableList.of());
         
         CatalogUpgrades upgrades = 
BundleUpgradeParser.parseBundleManifestForCatalogUpgrades(bundle, typeSupplier);
@@ -164,6 +176,20 @@ public class BundleUpgradeParserTest {
         assertFalse(upgrades.isLegacyItemRemoved(newMockCatalogItem("bar", 
"1.0.0")));
         
assertFalse(upgrades.isLegacyItemRemoved(newMockCatalogItem("different", 
"0.1.0")));
     }
+    
+    @Test
+    public void testForgetQuotesGivesNiceError() throws Exception {
+        Bundle bundle = newMockBundle(ImmutableMap.of(
+                BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS, 
"foo:[0,1.0.0),bar:[0,1.0.0)"));
+        Supplier<Iterable<RegisteredType>> typeSupplier = 
Suppliers.ofInstance(ImmutableList.of());
+        
+        try {
+            BundleUpgradeParser.parseBundleManifestForCatalogUpgrades(bundle, 
typeSupplier);
+            Asserts.shouldHaveFailedPreviously();
+        } catch (Exception e) {
+            Asserts.expectedFailureContainsIgnoreCase(e, "quote");
+        }
+    }
 
     @Test
     public void testStripQuotes() throws Exception {
@@ -183,7 +209,13 @@ public class BundleUpgradeParserTest {
     
     private Bundle newMockBundle(VersionedName name, Map<String, String> 
rawHeaders) {
         Dictionary<String, String> headers = new Hashtable<>(rawHeaders);
-        Bundle result = Mockito.mock(Bundle.class);
+        Bundle result;
+        try {
+            result = Mockito.mock(Bundle.class);
+        } catch (Exception e) {
+            throw new IllegalStateException("Java too old.  There is a bug in 
really early java 1.8.0 "
+                + "that causes mocks to fail, and has probably caused this.", 
e);
+        }
         Mockito.when(result.getHeaders()).thenReturn(headers);
         
Mockito.when(result.getSymbolicName()).thenReturn(name.getSymbolicName());
         
Mockito.when(result.getVersion()).thenReturn(Version.valueOf(name.getOsgiVersionString()));

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b05b8195/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java
----------------------------------------------------------------------
diff --git 
a/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java 
b/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java
index dc134bc..d7ffd4b 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/Strings.java
@@ -722,6 +722,13 @@ public class Strings {
         return false;
     }
 
+    public static boolean containsAny(CharSequence input, CharSequence 
...candidates) {
+        for (CharSequence candidate: candidates) {
+            if (containsLiteral(input, candidate)) return true;
+        }
+        return false;
+    }
+
     /** Returns a size string using metric suffixes from {@link 
ByteSizeStrings#metric()}, e.g. 23.5MB */
     public static String makeSizeString(long sizeInBytes) {
         return ByteSizeStrings.metric().makeSizeString(sizeInBytes);

Reply via email to