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 <[email protected]> Authored: Mon Jul 3 12:10:15 2017 +0100 Committer: Alex Heneveld <[email protected]> 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()) {
