This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 5114cb8779fd6f70b843b016d2558a05aca1b931
Author: Alex Heneveld <[email protected]>
AuthorDate: Tue Oct 20 14:57:51 2020 +0100

    minor tidies to tests and bundle resolution
    
    when taking bundle name from id, forgive version being unset
    
    again for legacy compatibility, where version was not always required
---
 .../brooklyn/api/catalog/BrooklynCatalog.java      | 15 +++++--
 .../brooklyn/camp/brooklyn/AbstractYamlTest.java   |  4 +-
 .../catalog/CatalogOsgiYamlEntityTest.java         |  4 +-
 .../CatalogYamlEntityOsgiTypeRegistryTest.java     |  7 +++-
 .../catalog/CatalogYamlVersioningTest.java         | 18 ++++----
 .../catalog/internal/BasicBrooklynCatalog.java     | 44 +++++++++++++++----
 .../mgmt/ha/BrooklynBomOsgiArchiveInstaller.java   | 49 +++++++++++++++++++---
 .../typereg/AbstractCatalogBundleResolver.java     |  2 +-
 .../core/typereg/BasicBrooklynTypeRegistry.java    |  1 +
 .../BrooklynBomBundleCatalogBundleResolver.java    | 14 +++----
 .../BrooklynBomYamlCatalogBundleResolver.java      | 16 +++----
 .../typereg/BrooklynCatalogBundleResolver.java     | 28 +++++++++++++
 .../rest/resources/BundleAndTypeResourcesTest.java |  2 +-
 .../rest/resources/CatalogResourceTest.java        |  2 +-
 14 files changed, 154 insertions(+), 52 deletions(-)

diff --git 
a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java 
b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java
index dcdc367..1dbe39e 100644
--- a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java
+++ b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java
@@ -143,6 +143,8 @@ public interface BrooklynCatalog {
      * <p>
      * A null bundle can be supplied although that will cause oddness in 
production installations
      * that assume bundles for persistence, lookup and other capabilities. 
(But it's fine for tests.)
+     * <p>
+     * Most callers will want to use the OsgiManager to install a bundle. This 
is primarily to support that.
      */
     @Beta  // method may move elsewhere, or return type may change
     public void addTypesFromBundleBom(String yaml, @Nullable ManagedBundle 
bundle, boolean forceUpdate, Map<RegisteredType, RegisteredType> result);
@@ -154,6 +156,8 @@ public interface BrooklynCatalog {
      * and the type registry may have some of the items updated.  It is the 
caller's responsibility to clean up if required
      * (note, clean up is only possible if an empty results map is supplied to 
collect the items, and even then finding the bundle needs work;
      * or, most commonly, where the management context is just being thrown 
away, like in tests).
+     * <p>
+     * Most callers will want to use the OsgiManager to install a bundle. This 
is primarily to support that.
      */
     @Beta
     Collection<RegisteredType> addTypesAndValidateAllowInconsistent(String 
catalogYaml, @Nullable Map<RegisteredType, RegisteredType> result, boolean 
forceUpdate);
@@ -176,7 +180,7 @@ public interface BrooklynCatalog {
 
     /** As {@link #addItems(String, boolean, boolean)}
      *
-     * @deprecated since 1.1, use {@link 
#addTypesAndValidateAllowInconsistent(String, Map, boolean)}
+     * @deprecated since 1.1, use {@link 
#addTypesAndValidateAllowInconsistent(String, Map, boolean)} for direct access 
to catalog (non-OSGi, eg tests) or use the OsgiManager
      * (Used in tests.)
      */
     Iterable<? extends CatalogItem<?,?>> addItems(String yaml);
@@ -192,7 +196,7 @@ public interface BrooklynCatalog {
      *
      * @throws IllegalArgumentException if the yaml was invalid
      *
-     * @deprecated Since 1.1, use {@link #addTypesFromBundleBom(String, 
ManagedBundle, boolean, Map)};
+     * @deprecated Since 1.1, use {@link #addTypesFromBundleBom(String, 
ManagedBundle, boolean, Map)} for direct access to catalog (non/partial-OSGi, 
eg tests) or use the OsgiManager
      * (Used for REST API, a few tests, and legacy-compatibility additions.)
      */
     Iterable<? extends CatalogItem<?,?>> addItems(String yaml, boolean 
validate, boolean forceUpdate);
@@ -207,7 +211,7 @@ public interface BrooklynCatalog {
      *
      * @throws IllegalArgumentException if the yaml was invalid
      *
-     * @deprecated Since 1.1, use {@link #addTypesFromBundleBom(String, 
ManagedBundle, boolean, Map)};
+     * @deprecated Since 1.1, use {@link #addTypesFromBundleBom(String, 
ManagedBundle, boolean, Map)} for direct access to catalog (non/partial-OSGi, 
eg tests) or use the OsgiManager
      * (Only used for legacy OSGi bundles.) */
     Iterable<? extends CatalogItem<?,?>> addItems(String yaml, ManagedBundle 
bundle, boolean forceUpdate);
     
@@ -217,7 +221,6 @@ public interface BrooklynCatalog {
      *
      * @deprecated since 0.7.0 Construct catalogs with yaml (referencing OSGi 
bundles) instead
      */
-    // TODO maybe this should stay on the API? -AH Apr 2015 
     @Deprecated
     void addItem(CatalogItem<?,?> item);
 
@@ -249,6 +252,10 @@ public interface BrooklynCatalog {
     @VisibleForTesting
     CatalogItem<?,?> addItem(Class<?> clazz);
 
+    /** @deprecated since 1.1 remove all bundles, that should clear the catalog
+     * Used for legacy catalog initialization and rebind.
+     */
+    @Deprecated
     void reset(Collection<CatalogItem<?, ?>> entries);
 
 }
diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
index c4c5c60..34e18d1 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
@@ -251,9 +251,7 @@ public abstract class AbstractYamlTest {
     }
 
     protected void addCatalogItems(String catalogYaml) {
-        mgmt().getCatalog().
-                addTypesAndValidateAllowInconsistent(catalogYaml, null, 
forceUpdate);
-//                addItems(catalogYaml, true, forceUpdate);
+        mgmt().getCatalog().addTypesAndValidateAllowInconsistent(catalogYaml, 
null, forceUpdate);
     }
 
     /*
diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
index 1704e43..5abf347 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java
@@ -245,7 +245,7 @@ public class CatalogOsgiYamlEntityTest extends 
AbstractYamlTest {
                 "    type: " + SIMPLE_ENTITY_TYPE);
             Asserts.shouldHaveFailedPreviously();
         } catch (Exception e) {
-            Asserts.expectedFailureContainsIgnoreCase(e, "both name and 
version are required");
+            Asserts.expectedFailureContainsIgnoreCase(e, "name", "version");
         }
         try {
             addCatalogItems(
@@ -259,7 +259,7 @@ public class CatalogOsgiYamlEntityTest extends 
AbstractYamlTest {
                 "    type: " + SIMPLE_ENTITY_TYPE);
             Asserts.shouldHaveFailedPreviously();
         } catch (Exception e) {
-            Asserts.expectedFailureContainsIgnoreCase(e, "both name and 
version are required");
+            Asserts.expectedFailureContainsIgnoreCase(e, "name", "version");
         }
     }
 
diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
index 0e19359..2414bea 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
@@ -52,8 +52,11 @@ public class CatalogYamlEntityOsgiTypeRegistryTest extends 
CatalogYamlEntityTest
         switch (itemsInstallMode!=null ? itemsInstallMode : 
             // this is the default because some "bundles" aren't resolvable or 
library BOMs loadable in test context
             CatalogItemsInstallationMode.BUNDLE_BUT_NOT_STARTED) {
-        case ADD_YAML_ITEMS_UNBUNDLED: super.addCatalogItems(catalogYaml); 
break;
-        case BUNDLE_BUT_NOT_STARTED: 
+        case ADD_YAML_ITEMS_UNBUNDLED:
+            super.addCatalogItems(catalogYaml);
+            break;
+        case BUNDLE_BUT_NOT_STARTED:
+            // TODO if id/bundle set in catalog yaml use that
             addCatalogItemsAsOsgiWithoutStartingBundles(mgmt(), catalogYaml, 
new VersionedName(bundleName(), bundleVersion()), isForceUpdate());
             break;
         case USUAL_OSGI_WAY_AS_BUNDLE_WITH_DEFAULT_NAME:
diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
index 588b1d8..ee0c884 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
@@ -310,14 +310,16 @@ public class CatalogYamlVersioningTest extends 
AbstractYamlTest {
     private void addCatalogEntity(String symbolicName, String version, String 
type, String iconUrl) {
         addCatalogItems(
             "brooklyn.catalog:",
-            "  id: " + symbolicName,
-            (version != null ? "  version: " + version : ""),
-            "  itemType: entity",
-            "  name: My Catalog App",
-            "  description: My description",
-            "  icon_url: "+iconUrl,
-            "  item:",
-            "    type: " + type);
+//            "  items:",
+//            "  - ",
+            "    id: " + symbolicName,
+            (version != null ? "    version: " + version : ""),
+            "    itemType: entity",
+            "    name: My Catalog App",
+            "    description: My description",
+            "    icon_url: "+iconUrl,
+            "    item:",
+            "      type: " + type);
     }
 
     protected void addCatalogEntityWithoutBundle(String symbolicName, String 
version) {
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 35adfa3..5a1a9e2 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
@@ -500,26 +500,52 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
         return getFirstAsMap(itemDef, "brooklyn.catalog").orNull();        
     }
     
-    /** @deprecated since 0.12.0 - use {@link #getVersionedName(Map, boolean)} 
*/
-    @Deprecated
-    public static VersionedName getVersionedName(Map<?,?> catalogMetadata) {
-        return getVersionedName(catalogMetadata, true);
-    }
-    
     public static VersionedName getVersionedName(Map<?,?> catalogMetadata, 
boolean required) {
+        // for better legacy compatibility, if id specified at root use that 
for bundle symbolic name and optionally for version
+        VersionedName idV = null;
+        String idS = getFirstAs(catalogMetadata, String.class, "id").orNull();
+        if (Strings.isNonBlank(idS)) {
+            idV = VersionedName.fromString(idS);
+        }
+
         String version = getFirstAs(catalogMetadata, String.class, 
"version").orNull();
         String bundle = getFirstAs(catalogMetadata, String.class, 
"bundle").orNull();
+        if (bundle==null) {
+            // if bundle not specified, ID indicates bundle name, and version 
if specified must match
+            if (idV!=null) {
+                bundle = idV.getSymbolicName();
+                if (Strings.isNonBlank(idV.getVersionString())) {
+                    if (version!=null) {
+                        if (!Objects.equal(version, idV.getVersionString()) && 
!Objects.equal(version, idV.getOsgiVersionString())) {
+                            throw new IllegalStateException("Catalog BOM using 
ID '" + idV + "' to define bundle does not match declared version '" + version 
+ "'");
+                        }
+                    } else {
+                        version = idV.getVersionString();
+                    }
+                } else {
+                    if (required) {
+                        throw new IllegalStateException("Catalog BOM must 
define bundle name and version or include version as part of the id '" + bundle 
+ "' (eg '" + bundle + ":1.0')");
+                    } else {
+                        // allow null version
+                    }
+                }
+            }
+        }
+
         if (Strings.isBlank(bundle) && Strings.isBlank(version)) {
             if (!required) return null;
-            throw new IllegalStateException("Catalog BOM must define bundle 
and version");
+            throw new IllegalStateException("Catalog BOM must define bundle 
name and version");
         }
         if (Strings.isBlank(bundle)) {
             if (!required) return null;
-            throw new IllegalStateException("Catalog BOM must define bundle");
+            throw new IllegalStateException("Catalog BOM must define bundle 
(or id)");
         }
         if (Strings.isBlank(version)) {
-            throw new IllegalStateException("Catalog BOM must define version 
if bundle is defined");
+            if (required) {
+                throw new IllegalStateException("Catalog BOM must define 
version where bundle name '" + bundle + "' is defined");
+            }
         }
+
         return new VersionedName(bundle, version);
     }
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
index 27ffe6a..6a8fb05 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java
@@ -364,6 +364,12 @@ public class BrooklynBomOsgiArchiveInstaller {
 
     private void discoverManifestFromCatalogBom(boolean isCatalogBomRequired) {
         discoveredManifest = new BundleMaker(mgmt()).getManifest(zipFile);
+
+        if (Strings.isNonBlank(bomText)) {
+            discoveredBomVersionedName = 
BasicBrooklynCatalog.getVersionedName(BasicBrooklynCatalog.getCatalogMetadata(bomText),
 false );
+            return;
+        }
+
         ZipFile zf = null;
         try {
             try {
@@ -382,20 +388,19 @@ public class BrooklynBomOsgiArchiveInstaller {
                     return;
                 }
             }
-            String bomS;
             try {
-                bomS = Streams.readFullyString(zf.getInputStream(bom));
+                bomText = Streams.readFullyString(zf.getInputStream(bom));
             } catch (IOException e) {
                 throw new IllegalArgumentException("Error reading catalog.bom 
from ZIP/JAR archive: "+e);
             }
-            discoveredBomVersionedName = 
BasicBrooklynCatalog.getVersionedName( 
BasicBrooklynCatalog.getCatalogMetadata(bomS), false );
+            discoveredBomVersionedName = 
BasicBrooklynCatalog.getVersionedName( 
BasicBrooklynCatalog.getCatalogMetadata(bomText), false );
         } finally {
             Streams.closeQuietly(zf);
         }
     }
     
     private void updateManifestFromAllSourceInformation() {
-        if (discoveredBomVersionedName!=null) {
+        if (discoveredBomVersionedName !=null) {
             matchSetOrFail("catalog.bom in archive", 
discoveredBomVersionedName.getSymbolicName(), 
discoveredBomVersionedName.getVersionString());
         }
         
@@ -953,7 +958,22 @@ public class BrooklynBomOsgiArchiveInstaller {
         } else if (Strings.isBlank(inferredMetadata.getSymbolicName())) {
             ((BasicManagedBundle)inferredMetadata).setSymbolicName(name);
         } else if (!Objects.equal(inferredMetadata.getSymbolicName(), name)){
-            throw new IllegalArgumentException("Symbolic name mismatch 
'"+name+"' from "+source+" (expected 
'"+inferredMetadata.getSymbolicName()+"')");
+            // if bundle name is inferred from ID, it is allowed to be 
overridden by manifest, but with a warning, for legacy and non-bundle BOM 
compatibility.
+            // if bundle name is set using 'bundle' it must match (previous 
behaviour).
+            boolean inferredMetadataCanBeOverridden = false;
+            try {
+                Map<?, ?> catalogMetadata = 
BasicBrooklynCatalog.getCatalogMetadata(bomText);
+                if (!catalogMetadata.containsKey("bundle") && 
catalogMetadata.containsKey("id")) {
+                    inferredMetadataCanBeOverridden = true;
+                    log.warn("Installing bundle '" + inferredMetadata + "' 
from " + source + ", even though 'id' in its catalog BOM is different ('" + 
name + "', v '"+version+"'); strongly recommended that the BOM 'id' match the 
bundle symbolic name");
+                }
+            } catch (Exception e) {
+                Exceptions.propagateIfFatal(e);
+            }
+
+            if (!inferredMetadataCanBeOverridden) {
+                throw new IllegalArgumentException("Symbolic name mismatch '" 
+ name + "' from " + source + " (expected '" + 
inferredMetadata.getSymbolicName() + "')");
+            }
         }
         
         if (Strings.isBlank(version)) {
@@ -961,7 +981,24 @@ public class BrooklynBomOsgiArchiveInstaller {
         } else if 
(Strings.isBlank(inferredMetadata.getSuppliedVersionString())) {
             ((BasicManagedBundle)inferredMetadata).setVersion(version);
         } else if 
(!BrooklynVersionSyntax.equalAsOsgiVersions(inferredMetadata.getSuppliedVersionString(),
 version)) {
-            throw new IllegalArgumentException("Bundle version mismatch 
'"+version+"' from "+source+" (expected 
'"+inferredMetadata.getSuppliedVersionString()+"')");
+            boolean inferredMetadataCanBeOverridden = false;
+            try {
+                if (BasicBrooklynCatalog.NO_VERSION.equals(version)) {
+                    inferredMetadataCanBeOverridden = true;
+                } else {
+                    Map<?, ?> catalogMetadata = 
BasicBrooklynCatalog.getCatalogMetadata(bomText);
+                    if (!catalogMetadata.containsKey("bundle") && 
catalogMetadata.containsKey("id")) {
+                        inferredMetadataCanBeOverridden = true;
+                        log.warn("Installing bundle '" + inferredMetadata + "' 
from " + source + ", even though 'version' in its catalog BOM is different ('" 
+ name + "', v '" + version + "'); strongly recommended that the BOM 'version' 
match the bundle version");
+                    }
+                }
+            } catch (Exception e) {
+                Exceptions.propagateIfFatal(e);
+            }
+
+            if (!inferredMetadataCanBeOverridden) {
+                throw new IllegalArgumentException("Bundle version mismatch '" 
+ version + "' from " + source + " (expected '" + 
inferredMetadata.getSuppliedVersionString() + "')");
+            }
         }
         
         return suppliedIsComplete;
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCatalogBundleResolver.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCatalogBundleResolver.java
index bebffba..6d709a3 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCatalogBundleResolver.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractCatalogBundleResolver.java
@@ -106,7 +106,7 @@ public abstract class AbstractCatalogBundleResolver 
implements BrooklynCatalogBu
 
         private byte[] bytesRead = new byte[0];
 
-        protected FileTypeDetector(Supplier<InputStream> streamSupplier) {
+        public FileTypeDetector(Supplier<InputStream> streamSupplier) {
             this.streamSupplier = streamSupplier;
         }
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
index 2ea060d..916ba56 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
@@ -258,6 +258,7 @@ public class BasicBrooklynTypeRegistry implements 
BrooklynTypeRegistry {
                 }
                 type = mgmt.getTypeRegistry().get(type.getSymbolicName(), 
type.getVersion());
                 if (type==null || 
type.getKind()==RegisteredTypeKind.UNRESOLVED) {
+                    // TODO show the resolution error
                     throw new ReferencedUnresolvedTypeException(type);
                 }
                 return createSpec(type, constraint, specSuperType);
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomBundleCatalogBundleResolver.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomBundleCatalogBundleResolver.java
index 4805cad..1c300a5 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomBundleCatalogBundleResolver.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomBundleCatalogBundleResolver.java
@@ -55,18 +55,18 @@ public class BrooklynBomBundleCatalogBundleResolver extends 
AbstractCatalogBundl
 
     @Override
     public ReferenceWithError<OsgiBundleInstallationResult> 
install(Supplier<InputStream> input, BundleInstallationOptions options) {
-        LOG.debug("Installing bundle from stream - known details: 
"+(options==null ? null : options.knownBundleMetadata));
+        LOG.debug("Installing bundle from stream - known details: 
"+(options==null ? null : options.getKnownBundleMetadata()));
 
         BrooklynBomOsgiArchiveInstaller installer = new 
BrooklynBomOsgiArchiveInstaller(
                 ((ManagementContextInternal)mgmt).getOsgiManager().get(),
-                (options==null ? null : options.knownBundleMetadata), 
input.get());
+                (options==null ? null : options.getKnownBundleMetadata()), 
input.get());
         installer.setCatalogBomText(FORMAT, null);
         if (options!=null) {
-            installer.setStart(options.start);
-            installer.setLoadCatalogBom(options.loadCatalogBom);
-            installer.setForce(options.forceUpdateOfNonSnapshots);
-            installer.setDeferredStart(options.deferredStart);
-            installer.setValidateTypes(options.validateTypes);
+            installer.setStart(options.isStart());
+            installer.setLoadCatalogBom(options.isLoadCatalogBom());
+            installer.setForce(options.isForceUpdateOfNonSnapshots());
+            installer.setDeferredStart(options.isDeferredStart());
+            installer.setValidateTypes(options.isValidateTypes());
         }
 
         return installer.install();
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomYamlCatalogBundleResolver.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomYamlCatalogBundleResolver.java
index da94601..861b3a7 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomYamlCatalogBundleResolver.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynBomYamlCatalogBundleResolver.java
@@ -84,6 +84,8 @@ public class BrooklynBomYamlCatalogBundleResolver extends 
AbstractCatalogBundleR
         // throw if not valid yaml
         new FileTypeDetector(input).getYaml().get();
 
+        if (options==null) options = new BundleInstallationOptions();
+
         String yaml = Streams.readFullyString(input.get());
         Map<?, ?> cm = BasicBrooklynCatalog.getCatalogMetadata(yaml);
 
@@ -93,13 +95,11 @@ public class BrooklynBomYamlCatalogBundleResolver extends 
AbstractCatalogBundleR
 
         VersionedName vn = BasicBrooklynCatalog.getVersionedName(cm, false);
         if (vn == null) {
-            // for better legacy compatibiity, if id specified at root use that
-            String id = (String) cm.get("id");
-            if (Strings.isNonBlank(id)) {
-                vn = VersionedName.fromString(id);
-            }
-            vn = new VersionedName(vn != null && 
Strings.isNonBlank(vn.getSymbolicName()) ? vn.getSymbolicName() : 
"brooklyn-catalog-bom-" + Identifiers.makeRandomId(8),
-                    vn != null && vn.getVersionString() != null ? 
vn.getVersionString() : ConfigUtils.getFirstAs(cm, String.class, 
"version").or(BasicBrooklynCatalog.NO_VERSION));
+            vn = new VersionedName("brooklyn-catalog-bom-" + 
Identifiers.makeRandomId(8), (String)null);
+        }
+        if (Strings.isBlank(vn.getVersionString())) {
+            vn = new VersionedName(vn.getSymbolicName(),
+                    ConfigUtils.getFirstAs(cm, String.class, 
"version").or(BasicBrooklynCatalog.NO_VERSION));
         }
         LOG.debug("Wrapping supplied BOM as " + vn);
         Manifest mf = new Manifest();
@@ -118,7 +118,7 @@ public class BrooklynBomYamlCatalogBundleResolver extends 
AbstractCatalogBundleR
             result = 
((ManagementContextInternal)mgmt).getOsgiManager().get().installBrooklynBomBundle(
                     new BasicManagedBundle(vn.getSymbolicName(), 
vn.getVersionString(),
                             null, 
BrooklynBomBundleCatalogBundleResolver.FORMAT,
-                            null, null), InputStreamSource.of("ZIP generated 
for "+vn+": "+bf, bf), true, true, options.forceUpdateOfNonSnapshots).get();
+                            null, null), InputStreamSource.of("ZIP generated 
for "+vn+": "+bf, bf), options.isStart(), options.isLoadCatalogBom(), 
options.isForceUpdateOfNonSnapshots()).get();
         } finally {
             bf.delete();
         }
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolver.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolver.java
index 88d792d..6244331 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolver.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynCatalogBundleResolver.java
@@ -121,6 +121,34 @@ public interface BrooklynCatalogBundleResolver extends 
ManagementContextInjectab
         public void setKnownBundleMetadata(ManagedBundle knownBundleMetadata) {
             this.knownBundleMetadata = knownBundleMetadata;
         }
+
+        public String getFormat() {
+            return format;
+        }
+
+        public ManagedBundle getKnownBundleMetadata() {
+            return knownBundleMetadata;
+        }
+
+        public boolean isDeferredStart() {
+            return deferredStart;
+        }
+
+        public boolean isForceUpdateOfNonSnapshots() {
+            return forceUpdateOfNonSnapshots;
+        }
+
+        public boolean isLoadCatalogBom() {
+            return loadCatalogBom;
+        }
+
+        public boolean isStart() {
+            return start;
+        }
+
+        public boolean isValidateTypes() {
+            return validateTypes;
+        }
     }
 
 }
diff --git 
a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/BundleAndTypeResourcesTest.java
 
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/BundleAndTypeResourcesTest.java
index 8406dcd..de93001 100644
--- 
a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/BundleAndTypeResourcesTest.java
+++ 
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/BundleAndTypeResourcesTest.java
@@ -601,7 +601,7 @@ public class BundleAndTypeResourcesTest extends 
BrooklynRestResourceTest {
                 .post(Streams.readFully(new FileInputStream(f)));
 
         assertEquals(response.getStatus(), 
Response.Status.BAD_REQUEST.getStatusCode());
-        
Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), 
"Catalog BOM must define version");
+        
Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), 
"BOM", "bundle", "version");
     }
 
     @Test
diff --git 
a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
 
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
index fa62ece..286a4db 100644
--- 
a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
+++ 
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
@@ -660,7 +660,7 @@ public class CatalogResourceTest extends 
BrooklynRestResourceTest {
                 .post(Streams.readFully(new FileInputStream(f)));
 
         assertEquals(response.getStatus(), 
Response.Status.BAD_REQUEST.getStatusCode());
-        
Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), 
"Catalog BOM must define version");
+        
Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), 
"BOM", "bundle", "version");
     }
 
     @Test

Reply via email to