require bundle name/version defined in catalog.bom when POSTing a ZIP/JAR

accepts id, symbolicName, or symbolic-name under brooklyn.catalog;
if osgi bundle manifest supplied, it must match.


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

Branch: refs/heads/master
Commit: 67419b65c3976292ca35b7c26cd3804fca7cd182
Parents: 5df2824
Author: Alex Heneveld <[email protected]>
Authored: Thu Mar 9 09:59:56 2017 +0000
Committer: Alex Heneveld <[email protected]>
Committed: Thu Mar 9 10:02:33 2017 +0000

----------------------------------------------------------------------
 .../catalog/internal/BasicBrooklynCatalog.java  |  51 +++++++-
 .../apache/brooklyn/rest/api/CatalogApi.java    |  13 +-
 .../rest/resources/CatalogResource.java         | 126 +++++++++++--------
 .../rest/resources/CatalogResourceTest.java     |  19 +--
 4 files changed, 142 insertions(+), 67 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/67419b65/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 106240e..6e2a802 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
@@ -57,11 +57,13 @@ import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.AggregateClassLoader;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 import org.apache.brooklyn.util.javalang.LoadedClassLoader;
+import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.brooklyn.util.time.Time;
 import org.apache.brooklyn.util.yaml.Yamls;
 import org.apache.brooklyn.util.yaml.Yamls.YamlExtract;
+import org.osgi.framework.Version;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.yaml.snakeyaml.Yaml;
@@ -407,7 +409,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
     }
     
     @SuppressWarnings({ "unchecked", "rawtypes" })
-    private Maybe<Map<?,?>> getFirstAsMap(Map<?,?> map, String firstKey, 
String ...otherKeys) {
+    private static Maybe<Map<?,?>> getFirstAsMap(Map<?,?> map, String 
firstKey, String ...otherKeys) {
         return (Maybe) getFirstAs(map, Map.class, firstKey, otherKeys);
     }
 
@@ -417,6 +419,53 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
         return result;
     }
 
+    public static Map<?,?> getCatalogMetadata(String yaml) {
+        Map<?,?> itemDef = Yamls.getAs(Yamls.parseAll(yaml), Map.class);
+        return getFirstAsMap(itemDef, "brooklyn.catalog").orNull();        
+    }
+    
+    public static VersionedName getVersionedName(Map<?,?> catalogMetadata) {
+        String id = getFirstAs(catalogMetadata, String.class, "id").orNull();
+        String version = getFirstAs(catalogMetadata, String.class, 
"version").orNull();
+        String symbolicName = getFirstAs(catalogMetadata, String.class, 
"symbolicName").orNull();
+        symbolicName = findAssertingConsistentIfSet("symbolicName", 
symbolicName, getFirstAs(catalogMetadata, String.class, 
"symbolic-name").orNull());
+        // prefer symbolic name and version, if supplied
+        // else use id as symbolic name : version or just symbolic name
+        // (must match if both supplied)
+        if (Strings.isNonBlank(id)) {
+            if (CatalogUtils.looksLikeVersionedId(id)) {
+                symbolicName = findAssertingConsistentIfSet("symbolicName", 
symbolicName, CatalogUtils.getSymbolicNameFromVersionedId(id));
+                version = findAssertingConsistentIfSet("version", version, 
CatalogUtils.getVersionFromVersionedId(id));
+            } else {
+                symbolicName = findAssertingConsistentIfSet("symbolicName", 
symbolicName, id);
+            }
+        }
+        if (Strings.isBlank(symbolicName) && Strings.isBlank(version)) {
+            throw new IllegalStateException("Catalog BOM must define 
symbolicName and version");
+        }
+        if (Strings.isBlank(symbolicName)) {
+            throw new IllegalStateException("Catalog BOM must define 
symbolicName");
+        }
+        if (Strings.isBlank(version)) {
+            throw new IllegalStateException("Catalog BOM must define version");
+        }
+        return new VersionedName(symbolicName, Version.valueOf(version));
+    }
+    
+    private static String findAssertingConsistentIfSet(String context, String 
...values) {
+        String v = null;
+        for (String vi: values) {
+            if (Strings.isNonBlank(vi)) {
+                if (Strings.isNonBlank(v)) {
+                    if (!v.equals(vi)) throw new 
IllegalStateException("Mismatch in "+context+": '"+v+"' or '"+vi+"' supplied");
+                } else {
+                    v = vi;
+                }
+            }
+        }
+        return v;
+    }
+
     private void collectCatalogItems(String yaml, 
List<CatalogItemDtoAbstract<?, ?>> result, Map<?, ?> parentMeta) {
         Map<?,?> itemDef = Yamls.getAs(Yamls.parseAll(yaml), Map.class);
         Map<?,?> catalogMetadata = getFirstAsMap(itemDef, 
"brooklyn.catalog").orNull();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/67419b65/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java
----------------------------------------------------------------------
diff --git 
a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java 
b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java
index 93a812e..79b6186 100644
--- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java
+++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java
@@ -76,23 +76,20 @@ public interface CatalogApi {
                     name = "item",
                     value = "Item to install, as JAR/ZIP or Catalog YAML 
(autodetected)",
                     required = true)
-            byte[] item,
-            @ApiParam(name="name", value="Symbolic name to use for bundle", 
required=false, defaultValue="") String bundleName, 
-            @ApiParam(name="version", value="Version to set for bundle", 
required=false, defaultValue="") String bundleVersion);
+            byte[] item);
     
     @POST
     @Beta
     @Consumes({"application/x-zip", "application/x-jar"})
-    @ApiOperation(value = "Add a catalog item (e.g. new type of entity, policy 
or location) by uploading OSGi bundle JAR, or ZIP if bundle name and optionally 
version are supplied. "
+    @ApiOperation(value = "Add a catalog item (e.g. new type of entity, policy 
or location) by uploading OSGi bundle JAR, or ZIP which will be turned into 
bundle JAR, "
+            + "containing catalog.bom containing bundle name and version. "
             + "Return value is 201 CREATED if bundle could be added.", 
response = String.class)
     public Response createFromArchive(
             @ApiParam(
                     name = "archive",
-                    value = "Bundle to install, in JAR format, or ZIP if 
bundle name and optionally version are supplied, optionally with catalog.bom 
contained within",
+                    value = "Bundle to install, in ZIP or JAR format, 
requiring catalog.bom containing bundle name and version",
                     required = true)
-            byte[] archive,
-            @ApiParam(name="name", value="Symbolic name to use for bundle", 
required=false, defaultValue="") String bundleName, 
-            @ApiParam(name="version", value="Version to set for bundle", 
required=false, defaultValue="") String bundleVersion);
+            byte[] archive);
     
     @POST
     @Consumes(MediaType.APPLICATION_XML)

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/67419b65/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
----------------------------------------------------------------------
diff --git 
a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
 
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
index edc8b4d..9fdf977 100644
--- 
a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
+++ 
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
@@ -30,7 +30,6 @@ import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.jar.Attributes;
-import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 
 import javax.annotation.Nullable;
@@ -78,9 +77,13 @@ import org.apache.brooklyn.util.core.osgi.BundleMaker;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.os.Os;
+import org.apache.brooklyn.util.osgi.VersionedName;
+import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.StringPredicates;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.yaml.Yamls;
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.Constants;
 import org.slf4j.Logger;
@@ -112,10 +115,7 @@ public class CatalogResource extends 
AbstractBrooklynRestResource implements Cat
     static Set<String> missingIcons = MutableSet.of();
 
     @Override
-    public Response createPoly(
-            byte[] item,
-            String bundleName, 
-            String bundleVersion) {
+    public Response createPoly(byte[] item) {
         Throwable yamlException = null;
         try {
             MutableList.copyOf( Yamls.parseAll(new InputStreamReader(new 
ByteArrayInputStream(item))) );
@@ -125,15 +125,12 @@ public class CatalogResource extends 
AbstractBrooklynRestResource implements Cat
         }
         
         if (yamlException==null) {
-            // treat as yaml
-            if (Strings.isNonBlank(bundleName) || 
Strings.isNonBlank(bundleVersion)) {
-                throw new IllegalArgumentException("Bundle name/version not 
permitted when installing catalog YAML");
-            }
+            // treat as yaml if it parsed
             return createFromYaml(new String(item));
         }
         
         try {
-            return createFromArchive(item, bundleName, bundleVersion);
+            return createFromArchive(item);
         } catch (Exception e) {
             throw Exceptions.propagate("Unable to handle input: not YAML or 
compatible ZIP. Specify Content-Type to clarify.", e);
         }
@@ -180,54 +177,85 @@ public class CatalogResource extends 
AbstractBrooklynRestResource implements Cat
     }
 
     @Override
-    public Response createFromArchive(byte[] zipInput, String bundleName, 
String bundleVersion) {
+    public Response createFromArchive(byte[] zipInput) {
         if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), 
Entitlements.ROOT, null)) {
             throw WebResourceUtils.forbidden("User '%s' is not authorized to 
add catalog item",
                 Entitlements.getEntitlementContext().user());
         }
 
         BundleMaker bm = new BundleMaker(mgmt());
-        File f = Os.newTempFile("brooklyn-posted-archive", "zip");
+        File f=null, f2=null;
         try {
-            Files.write(zipInput, f);
-        } catch (IOException e) {
-            Exceptions.propagate(e);
-        }
-        Manifest mf = bm.getManifest(f);
-        if (mf==null) {
-            mf = new Manifest();
-        }
-        String bundleNameInMF = 
mf.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
-        if (Strings.isBlank(bundleName) && Strings.isBlank(bundleNameInMF)) {
-            throw new IllegalStateException("Require either 
"+Constants.BUNDLE_SYMBOLICNAME+" in "+JarFile.MANIFEST_NAME+" or name supplied 
in REST API");
-        }
-        if (!Strings.isBlank(bundleName)) {
-            mf.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, 
bundleName);
-        }
-        if (!Strings.isBlank(bundleVersion) || 
Strings.isBlank(mf.getMainAttributes().getValue(Constants.BUNDLE_VERSION))) {
-            if (Strings.isBlank(bundleVersion)) {
-                bundleVersion = "0.0.0.SNAPSHOT";
+            f = Os.newTempFile("brooklyn-posted-archive", "zip");
+            try {
+                Files.write(zipInput, f);
+            } catch (IOException e) {
+                Exceptions.propagate(e);
             }
-            mf.getMainAttributes().putValue(Constants.BUNDLE_VERSION, 
bundleVersion);
-        }
-        if 
(mf.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION)==null) {
-            
mf.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), 
"1.0");
-        }
-        
-        File f2 = bm.copyAddingManifest(f, mf);
-        f.delete();
-        
-        Bundle bundle = bm.installBundle(f2, true);
-        f2.delete();
-        
-        if 
(!BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM))
 {
-            // if the above feature is not enabled, let's do it manually (as a 
contract of this method)
-            new CatalogBomScanner().new CatalogPopulator(
-                    ((LocalManagementContext) 
mgmt()).getOsgiManager().get().getFramework().getBundleContext(),
-                    mgmt()).addingBundle(bundle, null);
+            
+            ZipFile zf;
+            try {
+                zf = new ZipFile(f);
+            } catch (IOException e) {
+                throw WebResourceUtils.badRequest("Invalid ZIP/JAR archive: 
"+e);
+            }
+            ZipArchiveEntry bom = zf.getEntry("catalog.bom");
+            if (bom==null) {
+                bom = zf.getEntry("/catalog.bom");
+            }
+            if (bom==null) {
+                throw WebResourceUtils.badRequest("Archive must contain a 
catalog.bom file in the root");
+            }
+            String bomS;
+            try {
+                bomS = Streams.readFullyString(zf.getInputStream(bom));
+            } catch (IOException e) {
+                throw WebResourceUtils.badRequest("Error reading catalog.bom 
from ZIP/JAR archive: "+e);
+            }
+            VersionedName vn = BasicBrooklynCatalog.getVersionedName( 
BasicBrooklynCatalog.getCatalogMetadata(bomS) );
+            
+            Manifest mf = bm.getManifest(f);
+            if (mf==null) {
+                mf = new Manifest();
+            }
+            String bundleNameInMF = 
mf.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME);
+            if (Strings.isNonBlank(bundleNameInMF)) {
+                if (!bundleNameInMF.equals(vn.getSymbolicName())) {
+                    throw new IllegalStateException("JAR MANIFEST 
symbolic-name '"+bundleNameInMF+"' does not match '"+vn.getSymbolicName()+"' 
defined in BOM");
+                }
+            } else {
+                mf.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, 
vn.getSymbolicName());
+            }
+            
+            String bundleVersionInMF = 
mf.getMainAttributes().getValue(Constants.BUNDLE_VERSION);
+            if (Strings.isNonBlank(bundleVersionInMF)) {
+                if (!bundleVersionInMF.equals(vn.getVersion().toString())) {
+                    throw new IllegalStateException("JAR MANIFEST version 
'"+bundleVersionInMF+"' does not match '"+vn.getVersion()+"' defined in BOM");
+                }
+            } else {
+                mf.getMainAttributes().putValue(Constants.BUNDLE_VERSION, 
vn.getVersion().toString());
+            }
+            if 
(mf.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION)==null) {
+                
mf.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), 
"1.0");
+            }
+            
+            f2 = bm.copyAddingManifest(f, mf);
+            
+            Bundle bundle = bm.installBundle(f2, true);
+            
+            if 
(!BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM))
 {
+                // if the above feature is not enabled, let's do it manually 
(as a contract of this method)
+                new CatalogBomScanner().new CatalogPopulator(
+                        ((LocalManagementContext) 
mgmt()).getOsgiManager().get().getFramework().getBundleContext(),
+                        mgmt()).addingBundle(bundle, null);
+            }
+            
+            return Response.status(Status.CREATED).build();
+            
+        } finally {
+            if (f!=null) f.delete();
+            if (f2!=null) f2.delete();
         }
-        
-        return Response.status(Status.CREATED).build();
     }
     
     @SuppressWarnings("deprecation")

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/67419b65/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
----------------------------------------------------------------------
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 d1a74ae..9e70156 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
@@ -499,16 +499,17 @@ public class CatalogResourceTest extends 
BrooklynRestResourceTest {
     @Test
     public void testOsgiBundleWithBom() throws Exception {
         
TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), 
OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
-        String bundleUrl = OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL;
+        final String symbolicName = 
OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_SYMBOLIC_NAME_FULL;
+        final String version = 
OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_VERSION;
+        final String bundleUrl = 
OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL;
         BundleMaker bm = new BundleMaker(manager);
         File f = Os.newTempFile("osgi", "jar");
         
Files.copyFile(ResourceUtils.create(this).getResourceFromUrl(bundleUrl), f);
         
-        String symbolicName = "my.catalog.entity.id.testOsgiBundleWithBom";
         String bom = Joiner.on("\n").join(
                 "brooklyn.catalog:",
                 "  id: " + symbolicName,
-                "  version: " + TEST_VERSION,
+                "  version: " + version,
                 "  itemType: entity",
                 "  name: My Catalog App",
                 "  description: My description",
@@ -525,18 +526,18 @@ public class CatalogResourceTest extends 
BrooklynRestResourceTest {
         
         assertEquals(response.getStatus(), 
Response.Status.CREATED.getStatusCode());
 
-        CatalogEntitySummary entityItem = 
client().path("/catalog/entities/"+symbolicName + "/" + TEST_VERSION)
+        CatalogEntitySummary entityItem = 
client().path("/catalog/entities/"+symbolicName + "/" + version)
                 .get(CatalogEntitySummary.class);
 
         Assert.assertNotNull(entityItem.getPlanYaml());
         
Assert.assertTrue(entityItem.getPlanYaml().contains("org.apache.brooklyn.core.test.entity.TestEntity"));
 
-        assertEquals(entityItem.getId(), ver(symbolicName));
+        assertEquals(entityItem.getId(), 
CatalogUtils.getVersionedId(symbolicName, version));
         assertEquals(entityItem.getSymbolicName(), symbolicName);
-        assertEquals(entityItem.getVersion(), TEST_VERSION);
+        assertEquals(entityItem.getVersion(), version);
 
         // and internally let's check we have libraries
-        RegisteredType item = 
getManagementContext().getTypeRegistry().get(symbolicName, TEST_VERSION);
+        RegisteredType item = 
getManagementContext().getTypeRegistry().get(symbolicName, version);
         Assert.assertNotNull(item);
         Collection<OsgiBundleWithUrl> libs = item.getLibraries();
         assertEquals(libs.size(), 1);
@@ -544,7 +545,7 @@ public class CatalogResourceTest extends 
BrooklynRestResourceTest {
         Assert.assertNull(lib.getUrl());
 
         assertEquals(lib.getSymbolicName(), 
"org.apache.brooklyn.test.resources.osgi.brooklyn-test-osgi-entities");
-        assertEquals(lib.getVersion(), "0.1.0");
+        assertEquals(lib.getVersion(), version);
 
         // now let's check other things on the item
         URI expectedIconUrl = URI.create(getEndpointAddress() + 
"/catalog/icon/" + symbolicName + "/" + entityItem.getVersion()).normalize();
@@ -564,7 +565,7 @@ public class CatalogResourceTest extends 
BrooklynRestResourceTest {
             assertTrue(actualInterfaces.contains(expectedInterface.getName()));
         }
 
-        byte[] iconData = client().path("/catalog/icon/" + symbolicName + "/" 
+ TEST_VERSION).get(byte[].class);
+        byte[] iconData = client().path("/catalog/icon/" + symbolicName + "/" 
+ version).get(byte[].class);
         assertEquals(iconData.length, 43);
     }
 

Reply via email to