brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From geom...@apache.org
Subject [21/39] brooklyn-server git commit: tidy tests, add more OSGi-variants of existing tests, weaken same-blueprint check
Date Wed, 19 Jul 2017 16:25:51 GMT
tidy tests, add more OSGi-variants of existing tests, weaken same-blueprint check

now permit uploading the same BOM twice, forgive different containing bundle in this case
(some cleverness to allow that, but preserves compatibility there;
code needed changing anyway to allow adding unresovled items then replacing as resolved items)


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

Branch: refs/heads/master
Commit: 520fb6a602702202fcb768842101ddd04cd56e2b
Parents: ce85007
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Mon Jul 3 12:10:15 2017 +0100
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Mon Jul 3 12:51:29 2017 +0100

----------------------------------------------------------------------
 .../brooklyn/api/catalog/BrooklynCatalog.java   |   1 +
 .../camp/brooklyn/ApplicationsYamlOsgiTest.java |  31 ++++++
 .../camp/brooklyn/ApplicationsYamlTest.java     |  29 ++++--
 .../catalog/CatalogOsgiYamlVersioningTest.java  |  30 +++---
 .../brooklyn/catalog/CatalogScanOsgiTest.java   |   5 +-
 .../catalog/CatalogYamlAppOsgiTest.java         |  31 ++++++
 .../brooklyn/catalog/CatalogYamlAppTest.java    |  15 ++-
 .../catalog/CatalogYamlVersioningTest.java      |   6 +-
 .../brooklyn/test/lite/CampYamlLiteTest.java    |  35 +++----
 .../catalog/internal/BasicBrooklynCatalog.java  |  23 ++--
 .../core/typereg/BasicBrooklynTypeRegistry.java | 104 +++++++++++++++++--
 .../brooklyn/core/typereg/RegisteredTypes.java  |  47 +++++++++
 .../rest/resources/CatalogResource.java         |   6 +-
 13 files changed, 291 insertions(+), 72 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java
----------------------------------------------------------------------
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 6599f6d..65c2b88 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
@@ -60,6 +60,7 @@ public interface BrooklynCatalog {
     <T,SpecT> Iterable<CatalogItem<T,SpecT>> getCatalogItems();
 
     /** convenience for filtering items in the catalog; see CatalogPredicates for useful
filters */
+//    XXX
     <T,SpecT> Iterable<CatalogItem<T,SpecT>> getCatalogItems(Predicate<?
super CatalogItem<T,SpecT>> filter);
 
     /** persists the catalog item to the object store, if persistence is enabled */

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlOsgiTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlOsgiTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlOsgiTest.java
new file mode 100644
index 0000000..a93ee64
--- /dev/null
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlOsgiTest.java
@@ -0,0 +1,31 @@
+/*
+ * 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.camp.brooklyn;
+
+import org.testng.annotations.Test;
+
+@Test
+public class ApplicationsYamlOsgiTest extends ApplicationsYamlTest {
+    
+    @Override
+    protected boolean disableOsgi() {
+        return false;
+    }
+    
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
index bcda335..27f7927 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ApplicationsYamlTest.java
@@ -24,21 +24,24 @@ import static org.testng.Assert.assertTrue;
 
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.camp.brooklyn.TestSensorAndEffectorInitializer.TestConfigurableInitializer;
 import org.apache.brooklyn.camp.brooklyn.catalog.CatalogYamlLocationTest;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
-import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.core.test.policy.TestEnricher;
 import org.apache.brooklyn.core.test.policy.TestPolicy;
+import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.apache.brooklyn.test.Asserts;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 
@@ -46,12 +49,6 @@ import com.google.common.collect.Iterables;
 public class ApplicationsYamlTest extends AbstractYamlTest {
     private static final Logger log = LoggerFactory.getLogger(ApplicationsYamlTest.class);
 
-    @Override
-    protected LocalManagementContext newTestManagementContext() {
-        // Don't need osgi
-        return LocalManagementContextForTests.newInstance();
-    }
-
     @Test
     public void testWrapsEntity() throws Exception {
         Entity app = createAndStartApplication(
@@ -403,6 +400,22 @@ public class ApplicationsYamlTest extends AbstractYamlTest {
         assertDoesNotWrap(app1, BasicApplication.class, "My name within item");
     }
     
+    @Test
+    /** Tests catalog.bom format where service is defined alongside brooklyn.catalog, IE
latter has no item/items */
+    public void testItemFromServicesSectionInCatalog() {
+        addCatalogItems(
+            "brooklyn.catalog:",
+            "  id: simple-test",
+            "  version: "+TEST_VERSION,
+            "services:",
+            "- type: org.apache.brooklyn.entity.stock.BasicEntity");
+        
+        Iterable<RegisteredType> retrievedItems = mgmt().getTypeRegistry()
+                .getMatching(RegisteredTypePredicates.symbolicName(Predicates.equalTo("simple-test")));
+        Asserts.assertSize(retrievedItems, 1);
+        Assert.assertEquals(Iterables.getOnlyElement(retrievedItems).getVersion(), TEST_VERSION);
+    }
+
     @Override
     protected Logger getLogger() {
         return log;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java
index 9a047d8..672c722 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java
@@ -34,19 +34,23 @@ public class CatalogOsgiYamlVersioningTest extends CatalogYamlVersioningTest
{
         return false;
     }
 
-    @Override
-    @Test
-    public void testAddSameVersionWithoutBundle() {
-        try {
-            // parent test should fail in OSGi - anonymous bundle is snapshot so updating
is attempted
-            // but item version is not snapshot and containing bundle is different, so ultimately
fails
-            super.testAddSameVersionWithoutBundle();
-            Asserts.shouldHaveFailedPreviously("Expected to fail because containing bundle
will be different when using OSGi");
-        } catch (Exception e) {
-            assertExpectedFailureSaysUpdatingExistingItemForbidden(e);
-            assertExpectedFailureIncludesSampleId(e);
-        }
-    }
+    // now parent version of this test passes also in OSGi;
+    // the "sameEnough" check in BasicBrooklynTypeRegistry.addToLocalUnpersisted
+    // does _not_ look at containing bundle
+    // TODO delete this when we're happy with this behaviour
+//    @Override
+//    @Test
+//    public void testAddSameVersionWithoutBundle() {
+//        try {
+//            // parent test should fail in OSGi - anonymous bundle is snapshot so updating
is attempted
+//            // but item version is not snapshot and containing bundle is different, so
ultimately fails
+//            super.testAddSameVersionWithoutBundle();
+//            Asserts.shouldHaveFailedPreviously("Expected to fail because containing bundle
will be different when using OSGi");
+//        } catch (Exception e) {
+//            assertExpectedFailureSaysDifferentIsBad(e);
+//            assertExpectedFailureIncludesSampleId(e);
+//        }
+//    }
     
     @Test
     public void testAddSameVersionWithoutBundleWorksIfItemIsSnapshot() {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java
index c8d5399..4d1dfd5 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java
@@ -32,6 +32,7 @@ import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
 import org.apache.brooklyn.camp.brooklyn.test.lite.CampYamlLiteTest;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.support.TestResourceUnavailableException;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -91,13 +92,13 @@ public class CatalogScanOsgiTest extends AbstractYamlTest {
 
         RegisteredType hereItem = mgmt().getTypeRegistry().get("here-item");
         assertEquals(hereItem.getVersion(), "2.0-test_java");
-        assertEquals(hereItem.getLibraries().size(), 3);
+        Asserts.assertSize(hereItem.getLibraries(), 2);
         assertEquals(hereItem.getContainingBundle(), "test-items:2.0-test_java");
         
         RegisteredType item = mgmt().getTypeRegistry().get(OsgiTestResources.BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY);
         // versions and libraries _are_ inherited in this legacy mode
         assertEquals(item.getVersion(), "2.0-test_java");
-        assertEquals(item.getLibraries().size(), 3);
+        Asserts.assertSize(hereItem.getLibraries(), 2);
         // and the containing bundle is recorded as the 
         assertEquals(item.getContainingBundle(), "test-items"+":"+"2.0-test_java");
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppOsgiTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppOsgiTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppOsgiTest.java
new file mode 100644
index 0000000..90133c7
--- /dev/null
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppOsgiTest.java
@@ -0,0 +1,31 @@
+/*
+ * 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.camp.brooklyn.catalog;
+
+import org.testng.annotations.Test;
+
+// OSGi variant of parent
+@Test
+public class CatalogYamlAppOsgiTest extends CatalogYamlAppTest {
+
+    @Override
+    protected boolean disableOsgi() {
+        return false;
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppTest.java
index cc9066c..e4a9bf9 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlAppTest.java
@@ -18,15 +18,15 @@
  */
 package org.apache.brooklyn.camp.brooklyn.catalog;
 
-import org.testng.annotations.Test;
-
 import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
 import org.apache.brooklyn.camp.brooklyn.ApplicationsYamlTest;
 import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.testng.annotations.Test;
 
 /**
- * Also see related tests in {@link ApplicationsYamlTest}.
- * TODO Not clear what the difference is between these two test classes!
+ * Tests a few obscure circular references.
+ * See primary tests in {@link ApplicationsYamlTest}.
+ * Also see OSGi subclass.
  */
 public class CatalogYamlAppTest extends AbstractYamlTest {
 
@@ -40,12 +40,8 @@ public class CatalogYamlAppTest extends AbstractYamlTest {
      * "org.apache.brooklyn.entity.stock.BasicEntity", but that has been defined in the
      * catalog as this new blueprint (which overrides the previous value of it
      * being a reference to the Java class).
-     * 
-     * We need to use an id that matches something else already on the classpath.
-     * Otherwise we'd get an error telling us "could not resolve item ..." when
-     * attempting to add the initial catalog item.
      */
-    @Test(groups="WIP") // TODO Fix this!
+    @Test   // already fixed prior to type-registry shift in 0.12.0, but also works with
OSGi
     public void testAddCatalogItemWithCircularReference() throws Exception {
         // Add a catalog item with a circular reference to its own id.
         addCatalogItems(
@@ -107,4 +103,5 @@ public class CatalogYamlAppTest extends AbstractYamlTest {
             deleteCatalogEntity("org.apache.brooklyn.entity.stock.BasicApplication");
         }
     }
+    
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
----------------------------------------------------------------------
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 cd4525a..2942828 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
@@ -89,7 +89,7 @@ public class CatalogYamlVersioningTest extends AbstractYamlTest {
     }
     
     protected void checkAddSameVersionFailsWhenIconIsDifferent(Exception e) {
-        assertExpectedFailureSaysUpdatingExistingItemForbidden(e);
+        assertExpectedFailureSaysDifferentIsBad(e);
         assertExpectedFailureIncludesSampleId(e);
     }
 
@@ -99,9 +99,9 @@ public class CatalogYamlVersioningTest extends AbstractYamlTest {
         Asserts.expectedFailureContainsIgnoreCase(e, 
             symbolicName + ":" + version);
     }
-    protected void assertExpectedFailureSaysUpdatingExistingItemForbidden(Exception e) {
+    protected void assertExpectedFailureSaysDifferentIsBad(Exception e) {
         Asserts.expectedFailureContainsIgnoreCase(e, 
-            "Updating existing catalog entries is forbidden");
+            "cannot add", "different");
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java
index 07a8098..92b053e 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java
@@ -29,7 +29,6 @@ import java.util.List;
 import java.util.Map;
 
 import org.apache.brooklyn.api.catalog.CatalogItem;
-import org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
@@ -40,7 +39,6 @@ import org.apache.brooklyn.camp.spi.Assembly;
 import org.apache.brooklyn.camp.spi.AssemblyTemplate;
 import org.apache.brooklyn.camp.spi.pdp.PdpYamlTest;
 import org.apache.brooklyn.camp.test.mock.web.MockWebPlatform;
-import org.apache.brooklyn.core.catalog.CatalogPredicates;
 import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.effector.AddChildrenEffector;
@@ -54,7 +52,9 @@ import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.core.typereg.BasicManagedBundle;
 import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts;
+import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
+import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.support.TestResourceUnavailableException;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.ResourceUtils;
@@ -155,29 +155,28 @@ public class CampYamlLiteTest {
     }
 
     @Test
+    /** Tests catalog.bom format where service is defined alongside brooklyn.catalog, IE
latter has no item/items */
     public void testYamlServiceForCatalog() {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
         installWithoutCatalogBom(mgmt, OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL);
 
-        CatalogItem<?, ?> realItem = Iterables.getOnlyElement(mgmt.getCatalog().addItems(Streams.readFullyStringAndClose(getClass().getResourceAsStream("test-app-service-blueprint.yaml"))));
-        Iterable<CatalogItem<Object, Object>> retrievedItems = mgmt.getCatalog()
-                .getCatalogItems(CatalogPredicates.symbolicName(Predicates.equalTo("catalog-name")));
+        Iterable<? extends CatalogItem<?, ?>> itemsInstalled = mgmt.getCatalog().addItems(Streams.readFullyStringAndClose(getClass().getResourceAsStream("test-app-service-blueprint.yaml")));
+        Asserts.assertSize(itemsInstalled, 1);
+        CatalogItem<?, ?> itemInstalled = Iterables.getOnlyElement(itemsInstalled);
+        Assert.assertEquals(itemInstalled.getSymbolicName(), "catalog-name");
+        Assert.assertEquals(itemInstalled.getVersion(), "0.9");
         
-        Assert.assertEquals(Iterables.size(retrievedItems), 1, "Wrong retrieved items: "+retrievedItems);
-        CatalogItem<Object, Object> retrievedItem = Iterables.getOnlyElement(retrievedItems);
-        Assert.assertEquals(retrievedItem, realItem);
-
-        Collection<CatalogBundle> bundles = retrievedItem.getLibraries();
-        Assert.assertEquals(bundles.size(), 1);
-        CatalogBundle bundle = Iterables.getOnlyElement(bundles);
+        Iterable<RegisteredType> retrievedItems = mgmt.getTypeRegistry()
+            .getMatching(RegisteredTypePredicates.symbolicName(Predicates.equalTo("catalog-name")));
+        Asserts.assertSize(retrievedItems, 1);
+        RegisteredType retrievedItem = Iterables.getOnlyElement(retrievedItems);
+        Assert.assertEquals(retrievedItem.getVersion(), "0.9");
+
+        Collection<OsgiBundleWithUrl> bundles = retrievedItem.getLibraries();
+        Asserts.assertSize(bundles, 1);
+        OsgiBundleWithUrl bundle = Iterables.getOnlyElement(bundles);
         Assert.assertEquals(bundle.getUrl(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL);
         Assert.assertEquals(bundle.getSuppliedVersionString(), "0.1.0");
-
-        EntitySpec<?> spec1 = (EntitySpec<?>) mgmt.getCatalog().peekSpec(retrievedItem);
-        assertNotNull(spec1);
-        Assert.assertEquals(spec1.getConfig().get(TestEntity.CONF_NAME), "sample");
-        
-        // TODO other assertions, about children
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/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 8beb3be..0d858e0 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
@@ -516,7 +516,12 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         catalogMetadata.remove("item");
         catalogMetadata.remove("items");
         if (!itemDef.isEmpty()) {
-            log.debug("Reading brooklyn.catalog peer keys as item ('top-level syntax')");
+            // AH - i forgot we even supported this. probably no point anymore,
+            // now that catalog defs can reference an item yaml and things can be bundled
together?
+            log.warn("Reading catalog item from sibling keys of `brooklyn.catalog` section,
"
+                + "instead of the more common appraoch of putting inside an `item` within
it. "
+                + "This behavior is not deprecated yet but it is being considered. "
+                + "If you find it useful please inform the community.");
             Map<String,?> rootItem = MutableMap.of("item", itemDef);
             String rootItemYaml = yaml;
             YamlExtract yamlExtract = Yamls.getTextOfYamlAtPath(rootItemYaml, "brooklyn.catalog");
@@ -849,7 +854,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         String description = getFirstAs(catalogMetadata, String.class, "description").orNull();
         description = setFromItemIfUnset(description, itemAsMap, "description");
 
-        // icon.url is discouraged, but kept for legacy compatibility; should deprecate this
+        // icon.url is discouraged (using '.'), but kept for legacy compatibility; should
deprecate this
         final String catalogIconUrl = getFirstAs(catalogMetadata, String.class, "iconUrl",
"icon_url", "icon.url").orNull();
 
         final String deprecated = getFirstAs(catalogMetadata, String.class, "deprecated").orNull();
@@ -935,7 +940,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
 
     private static boolean isLibrariesMoreThanJustContainingBundle(Collection<CatalogBundle>
library, ManagedBundle containingBundle) {
-        if (library==null) return false;
+        if (library==null || library.isEmpty()) return false;
         if (containingBundle==null) return !library.isEmpty();
         if (library.size()>1) return true;
         CatalogBundle li = Iterables.getOnlyElement(library);
@@ -1369,7 +1374,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
             if (result.getCode().isError()) {
                 throw new IllegalStateException(result.getMessage());
             }
-            return toItems(result.getCatalogItemsInstalled());
+            return toLegacyCatalogItems(result.getCatalogItemsInstalled());
 
             // if all items pertaining to an older anonymous catalog.bom bundle have been
overridden
             // we delete those later; see list of wrapper bundles kept in OsgiManager
@@ -1379,10 +1384,16 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
     
     @SuppressWarnings("deprecation")
-    private List<CatalogItem<?,?>> toItems(Iterable<String> itemIds) {
+    private List<CatalogItem<?,?>> toLegacyCatalogItems(Iterable<String>
itemIds) {
         List<CatalogItem<?,?>> result = MutableList.of();
         for (String id: itemIds) {
-            result.add(CatalogUtils.getCatalogItemOptionalVersion(mgmt, id));
+            CatalogItem<?, ?> item = CatalogUtils.getCatalogItemOptionalVersion(mgmt,
id);
+            if (item==null) {
+                // using new Type Registry (OSGi addition); 
+                result.add(RegisteredTypes.toPartialCatalogItem( mgmt.getTypeRegistry().get(id)
));
+            } else {
+                result.add(item);
+            }
         }
         return result;
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
----------------------------------------------------------------------
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 5095f68..2e1a1fa 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
@@ -19,6 +19,7 @@
 package org.apache.brooklyn.core.typereg;
 
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -35,6 +36,8 @@ import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext;
 import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
 import org.apache.brooklyn.core.catalog.internal.CatalogItemBuilder;
 import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
+import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -316,18 +319,101 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry
{
             log.debug("Inserting "+type+" into "+this);
             localRegisteredTypes.put(type.getId(), type);
         } else {
-            if (sameTypeAndPlan(oldType, type)) {
-                // ignore if same type and plan; other things can be changed while we sort
out replacements etc
-                return;
-            }
-            throw new IllegalStateException("Cannot add "+type+" to catalog; different "+oldType+"
is already present");
+            assertSameEnoughToAllowReplacing(oldType, type);
         }
     }
 
-    private boolean sameTypeAndPlan(RegisteredType oldType, RegisteredType type) {
-        if (!oldType.getVersionedName().equals(type.getVersionedName())) return false;
-        if (!oldType.getPlan().equals(type.getPlan())) return false;
-        return true;
+    /**
+     * Allow replacing even of non-SNAPSHOT versions if plans are "similar enough";
+     * ie, forgiving some metadata changes.
+     * <p>
+     * This is needed when replacing an unresolved item with a resolved one or vice versa;
+     * the {@link RegisteredType#equals(Object)} check is too strict.
+     */
+    private boolean assertSameEnoughToAllowReplacing(RegisteredType oldType, RegisteredType
type) {
+       /* The bundle checksum check prevents swapping a different bundle at the same name+version.
+        * For SNAPSHOT and forced-updates, this method doesn't apply, so we can assume here
that
+        * either the bundle checksums are the same,
+        * or it is a different bundle declaring an item which is already installed.
+        * <p>
+        * Thus if the containing bundle is the same, the items are necessarily the same,
+        * except for metadata we've mucked with (e.g. kind = unresolved).
+        * <p>
+        * If the containing bundle is different, it's possible someone is doing something
sneaky.
+        * If bundles aren't anonymous wrappers, then we should disallow --
+        * e.g. a bundle BAR is declaring and item already declared by a bundle FOO.
+        * 
+        * 
+        * It is the latter case we have to check.
+        * 
+        *  In the latter case we want to fail unless the old item comes from a wrapper bundle.
+        * 
+        * the only time this method 
+        * applies where there might be differences in the item is if installing a bundle
BAR which
+        * declares an item with same name and version as an item already installed by a bundle
FOO.
+        *  
+        * * uploading a bundle 
+        * * uploading a BOM (no bundle) where the item it is defining is the same as one
already defined   
+        *  
+        * (with the same non-SNAPSHOT version) bundle. So any changes to icons, plans, etc,
should have already been caught;
+        * this only applies if the BOM/bundle is identical, and in that case the only field
where the two types here
+        * could be different is the containing bundle metadata, viz. someone has uploaded
an anonymous BOM twice. 
+        */
+        
+        if (!oldType.getVersionedName().equals(type.getVersionedName())) {
+            // different name - shouldn't even come here
+            throw new IllegalStateException("Cannot add "+type+" to catalog; different "+oldType+"
is already present");
+        }
+        if (Objects.equals(oldType.getContainingBundle(), type.getContainingBundle())) {
+            // if named bundles equal then contents must be the same (due to bundle checksum);
bail out early
+            if (!oldType.getPlan().equals(type.getPlan())) {
+                // shouldn't come here, but check anyway (or maybe if item added twice in
the same catalog?)
+                throw new IllegalStateException("Cannot add "+type+" to catalog; different
plan in "+oldType+" from same bundle is already present");
+            }
+            if (oldType.getKind()!=RegisteredTypeKind.UNRESOLVED && type.getKind()!=RegisteredTypeKind.UNRESOLVED
&&
+                    !Objects.equals(oldType.getKind(), type.getKind())) {
+                throw new IllegalStateException("Cannot add "+type+" to catalog; different
kind in "+oldType+" from same bundle is already present");
+            }
+            return true;
+        }
+        
+        // different bundles, either anonymous or same item in two named bundles
+        if (!oldType.getPlan().equals(type.getPlan())) {
+            // if plan is different, fail
+            throw new IllegalStateException("Cannot add "+type+" in "+type.getContainingBundle()+"
to catalog; different plan in "+oldType+" from bundle "+
+                oldType.getContainingBundle()+" is already present");
+        }
+        if (oldType.getKind()!=RegisteredTypeKind.UNRESOLVED && type.getKind()!=RegisteredTypeKind.UNRESOLVED
&&
+                !Objects.equals(oldType.getKind(), type.getKind())) {
+            // if kind is different and both resolved, fail
+            throw new IllegalStateException("Cannot add "+type+" in "+type.getContainingBundle()+"
to catalog; different kind in "+oldType+" from bundle "+
+                oldType.getContainingBundle()+" is already present");
+        }
+
+        // now if old is a wrapper bundle (or old, no bundle), allow it -- metadata may be
different here
+        // but we'll allow that, probably the user is updating their catalog to the new format.
+        // icons might change, maybe a few other things (but any such errors can be worked
around),
+        // and more useful to be able to upload the same BOM or replace an anonymous BOM
with a named bundle.
+        // crucially if old is a wrapper bundle the containing bundle won't actually be needed
for anything,
+        // so this is safe in terms of search paths etc.
+        if (oldType.getContainingBundle()==null) {
+            // if old type wasn't from a bundle, let it be replaced by a bundle
+            return true;
+        }
+        // bundle is changing; was old bundle a wrapper?
+        OsgiManager osgi = ((ManagementContextInternal)mgmt).getOsgiManager().orNull();
+        if (osgi==null) {
+            // shouldn't happen, as we got a containing bundle, but just in case
+            return true;
+        }
+        if (BasicBrooklynCatalog.isNoBundleOrSimpleWrappingBundle(mgmt, 
+                osgi.getManagedBundle(VersionedName.fromString(oldType.getContainingBundle()))))
{
+            // old was a wrapper bundle; allow it 
+            return true;
+        }
+        
+        throw new IllegalStateException("Cannot add "+type+" in "+type.getContainingBundle()+"
to catalog; "
+            + "item  is already present in different bundle "+oldType.getContainingBundle());
     }
 
     @Beta // API stabilising

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
index 06014c6..eb761ad 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
@@ -19,8 +19,10 @@
 package org.apache.brooklyn.core.typereg;
 
 import java.lang.reflect.Method;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -30,6 +32,8 @@ import javax.annotation.Nullable;
 import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.mgmt.rebind.RebindSupport;
+import org.apache.brooklyn.api.mgmt.rebind.mementos.CatalogItemMemento;
 import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
 import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind;
@@ -119,6 +123,49 @@ public class RegisteredTypes {
         return type;
     }
 
+    /** @deprecated since introduced in 0.12.0; for backwards compatibility only, may be
removed at any point.
+     * Returns a partially-populated CatalogItem. Many methods throw {@link UnsupportedOperationException}
+     * but the basic ones work. */
+    @Deprecated
+    public static CatalogItem<?,?> toPartialCatalogItem(RegisteredType t) {
+        return new CatalogItemFromRegisteredType(t);
+    }
+    private static class CatalogItemFromRegisteredType implements CatalogItem<Object,Object>
{
+        private final RegisteredType type;
+        public CatalogItemFromRegisteredType(RegisteredType type) { this.type = type; }
+        @Override public String getDisplayName() { return type.getDisplayName(); }
+        @Override public String getCatalogItemId() { return type.getVersionedName().toString();
}
+        @Override public String getId() { return type.getId(); }
+        @Override public String getName() { return type.getSymbolicName(); }
+        @Override public String getSymbolicName() { return type.getSymbolicName(); }
+        @Override public String getRegisteredTypeName() { return type.getSymbolicName();
}
+        @Override public String getDescription() { return type.getDescription(); }
+        @Override public String getIconUrl() { return type.getIconUrl(); }
+        @Override public String getContainingBundle() { return type.getContainingBundle();
}
+        @Override public String getVersion() { return type.getVersion(); }
+
+        @Override public List<String> getCatalogItemIdSearchPath() { throw new UnsupportedOperationException();
}
+        @Override public TagSupport tags() { throw new UnsupportedOperationException(); }
+        @Override public RelationSupport<?> relations() { throw new UnsupportedOperationException();
}
+        @Override public <T> T setConfig(ConfigKey<T> key, T val) { throw new
UnsupportedOperationException(); }
+        @Override public <T> T getConfig(ConfigKey<T> key) { throw new UnsupportedOperationException();
}
+        @Override public ConfigurationSupport config() { throw new UnsupportedOperationException();
}
+        @Override public SubscriptionSupport subscriptions() { throw new UnsupportedOperationException();
}
+        @Override public org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType getCatalogItemType()
{ throw new UnsupportedOperationException(); }
+        @Override public Class<Object> getCatalogItemJavaType() { throw new UnsupportedOperationException();
}
+        @Override public Class<Object> getSpecType() { throw new UnsupportedOperationException();
}
+        @Override public String getJavaType() { throw new UnsupportedOperationException();
}
+        @Override public Collection<org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle>
getLibraries() {
+            throw new UnsupportedOperationException();
+        }
+        @Override public String getPlanYaml() { throw new UnsupportedOperationException();
}
+        @Override public RebindSupport<CatalogItemMemento> getRebindSupport() { throw
new UnsupportedOperationException(); }
+        @Override public void setDeprecated(boolean deprecated) { throw new UnsupportedOperationException();
}
+        @Override public void setDisabled(boolean disabled) { throw new UnsupportedOperationException();
}
+        @Override public boolean isDeprecated() { throw new UnsupportedOperationException();
}
+        @Override public boolean isDisabled() { throw new UnsupportedOperationException();
}
+    }
+    
     /** Preferred mechanism for defining a bean {@link RegisteredType}. 
      * Callers should also {@link #addSuperTypes(RegisteredType, Iterable)} on the result.*/
     public static RegisteredType bean(@Nonnull String symbolicName, @Nonnull String version,
@Nonnull TypeImplementationPlan plan) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/520fb6a6/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 ae22c79..6983e5d 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
@@ -201,10 +201,8 @@ public class CatalogResource extends AbstractBrooklynRestResource implements
Cat
 
         if (OsgiBundleInstallationResult.ResultCode.IGNORING_BUNDLE_AREADY_INSTALLED.equals(result.getWithoutError().getCode()))
{
             result = ReferenceWithError.newInstanceThrowingError(result.getWithoutError(),
new IllegalStateException(
-                    "Updating existing catalog entries is forbidden: " +
-                    result.getWithoutError().getMetadata().getSymbolicName() + ":" +
-                    result.getWithoutError().getMetadata().getSuppliedVersionString() +
-                    ". Use forceUpdate argument to override."));
+                    "Cannot add bundle" + result.getWithoutError().getMetadata().getVersionedName()
+
+                    "; different bundle with same name already installed"));
         }
         
         if (result.hasError()) {


Mime
View raw message