allow "addition" of a catalog item which already exists if it's exactly the same
adds equals and hashCode to CatlogItemDto and CatalogBundleDto Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/fab1caf2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/fab1caf2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/fab1caf2 Branch: refs/heads/master Commit: fab1caf220f0d6eae661c4951683ee46b3eedf77 Parents: 48ce0df Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Wed May 6 23:02:03 2015 +0100 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Fri May 8 18:22:22 2015 +0100 ---------------------------------------------------------------------- .../catalog/internal/BasicBrooklynCatalog.java | 22 ++++++++++++-- .../catalog/internal/CatalogBundleDto.java | 19 ++++++++++++ .../internal/CatalogItemDtoAbstract.java | 32 ++++++++++++++++++++ .../brooklyn/catalog/CatalogYamlEntityTest.java | 20 ++++++++++-- .../catalog/CatalogYamlVersioningTest.java | 11 +++++-- usage/cli/src/main/java/brooklyn/cli/Main.java | 2 +- 6 files changed, 96 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fab1caf2/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java index 2e31532..7e4763d 100644 --- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java @@ -1045,7 +1045,12 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } private CatalogItem<?,?> addItemDto(CatalogItemDtoAbstract<?, ?> itemDto, boolean forceUpdate) { - checkItemNotExists(itemDto, forceUpdate); + CatalogItem<?, ?> existingDto = checkItemIsDuplicateOrDisallowed(itemDto, true, forceUpdate); + if (existingDto!=null) { + // it's a duplicate, and not forced, just return it + log.trace("Using existing duplicate for catalog item {}", itemDto.getId()); + return existingDto; + } if (manualAdditionsCatalog==null) loadManualAdditionsCatalog(); manualAdditionsCatalog.addEntry(itemDto); @@ -1067,8 +1072,19 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { return itemDto; } - private void checkItemNotExists(CatalogItem<?,?> itemDto, boolean forceUpdate) { - if (!forceUpdate && getCatalogItemDo(itemDto.getSymbolicName(), itemDto.getVersion()) != null) { + /** returns item DTO if item is an allowed duplicate, null if it should be added, or false if the item is an allowed duplicate, + * throwing if item cannot be added */ + private CatalogItem<?, ?> checkItemIsDuplicateOrDisallowed(CatalogItem<?,?> itemDto, boolean allowDuplicates, boolean forceUpdate) { + if (forceUpdate) return null; + CatalogItemDo<?, ?> existingItem = getCatalogItemDo(itemDto.getSymbolicName(), itemDto.getVersion()); + if (existingItem == null) return null; + // check if they are equal + CatalogItem<?, ?> existingDto = existingItem.getDto(); + if (existingDto.equals(itemDto)) { + if (allowDuplicates) return existingItem; + throw new IllegalStateException("Updating existing catalog entries, even with the same content, is forbidden: " + + itemDto.getSymbolicName() + ":" + itemDto.getVersion() + ". Use forceUpdate argument to override."); + } else { throw new IllegalStateException("Updating existing catalog entries is forbidden: " + itemDto.getSymbolicName() + ":" + itemDto.getVersion() + ". Use forceUpdate argument to override."); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fab1caf2/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java b/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java index 35353c4..5cbd23a 100644 --- a/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java +++ b/core/src/main/java/brooklyn/catalog/internal/CatalogBundleDto.java @@ -71,4 +71,23 @@ public class CatalogBundleDto implements CatalogBundle { .add("url", url) .toString(); } + + @Override + public int hashCode() { + return Objects.hashCode(symbolicName, version, url); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + CatalogBundleDto other = (CatalogBundleDto) obj; + if (!Objects.equal(symbolicName, other.symbolicName)) return false; + if (!Objects.equal(version, other.version)) return false; + if (!Objects.equal(url, other.url)) return false; + return true; + } + + } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fab1caf2/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java index 523cd79..c6ff97e 100644 --- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java +++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java @@ -39,6 +39,7 @@ import brooklyn.util.collections.MutableList; import brooklyn.util.flags.FlagUtils; import brooklyn.util.flags.SetFromFlag; +import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -162,6 +163,37 @@ public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynO } @Override + public int hashCode() { + return Objects.hashCode(symbolicName, planYaml, javaType, nullIfEmpty(libraries), version, getCatalogItemId()); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + CatalogItemDtoAbstract<?,?> other = (CatalogItemDtoAbstract<?,?>) obj; + if (!Objects.equal(symbolicName, other.symbolicName)) return false; + if (!Objects.equal(planYaml, other.planYaml)) return false; + if (!Objects.equal(javaType, other.javaType)) return false; + if (!Objects.equal(nullIfEmpty(libraries), nullIfEmpty(other.libraries))) return false; + if (!Objects.equal(getCatalogItemId(), other.getCatalogItemId())) return false; + if (!Objects.equal(version, other.version)) return false; + if (!Objects.equal(deprecated, other.deprecated)) return false; + if (!Objects.equal(description, other.description)) return false; + if (!Objects.equal(displayName, other.displayName)) return false; + if (!Objects.equal(iconUrl, other.iconUrl)) return false; + if (!Objects.equal(tags, other.tags)) return false; + // 'type' not checked, because deprecated, we might want to allow removal in future + return true; + } + + private static <T> Collection<T> nullIfEmpty(Collection<T> coll) { + if (coll==null || coll.isEmpty()) return null; + return coll; + } + + @Override public String toString() { return getClass().getSimpleName()+"["+getId()+"/"+getDisplayName()+"]"; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fab1caf2/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java index 1e321dc..9353108 100644 --- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java @@ -486,14 +486,23 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL + "}"); } } + + @Test + public void testUpdatingItemAllowedIfSame() { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); + + String id = "my.catalog.app.id.duplicate"; + addCatalogOSGiEntity(id); + addCatalogOSGiEntity(id); + } @Test(expectedExceptions = IllegalStateException.class) - public void testUpdatingItemFails() { + public void testUpdatingItemFailsIfDifferent() { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); String id = "my.catalog.app.id.duplicate"; addCatalogOSGiEntity(id); - addCatalogOSGiEntity(id); + addCatalogOSGiEntity(id, SIMPLE_ENTITY_TYPE, true); } @Test @@ -620,6 +629,10 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { } private void addCatalogOSGiEntity(String symbolicName, String serviceType) { + addCatalogOSGiEntity(symbolicName, serviceType, false); + } + + private void addCatalogOSGiEntity(String symbolicName, String serviceType, boolean extraLib) { addCatalogItem( "brooklyn.catalog:", " id: " + symbolicName, @@ -628,7 +641,8 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { " icon_url: classpath://path/to/myicon.jpg", " version: " + TEST_VERSION, " libraries:", - " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, + " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL + + (extraLib ? "\n"+" - url: "+OsgiStandaloneTest.BROOKLYN_OSGI_TEST_A_0_1_0_URL : ""), " item:", " type: " + serviceType); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fab1caf2/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java index c1176ea..dda60dc 100644 --- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java @@ -66,12 +66,13 @@ public class CatalogYamlVersioningTest extends AbstractYamlTest { } @Test - public void testAddSameVersionFails() { + public void testAddSameVersionFailsWhenIconIsDifferent() { String symbolicName = "sampleId"; String version = "0.1.0"; addCatalogEntity(symbolicName, version); + addCatalogEntity(symbolicName, version); try { - addCatalogEntity(symbolicName, version); + addCatalogEntity(symbolicName, version, BasicEntity.class.getName(), "classpath:/another/icon.png"); fail("Expected to fail"); } catch (IllegalStateException e) { assertEquals(e.getMessage(), "Updating existing catalog entries is forbidden: " + symbolicName + ":" + version + ". Use forceUpdate argument to override."); @@ -238,12 +239,16 @@ public class CatalogYamlVersioningTest extends AbstractYamlTest { } private void addCatalogEntity(String symbolicName, String version, String type) { + addCatalogEntity(symbolicName, version, type, "classpath://path/to/myicon.jpg"); + } + + private void addCatalogEntity(String symbolicName, String version, String type, String iconUrl) { addCatalogItem( "brooklyn.catalog:", " id: " + symbolicName, " name: My Catalog App", " description: My description", - " icon_url: classpath://path/to/myicon.jpg", + " icon_url: "+iconUrl, (version != null ? " version: " + version : ""), "", "services:", http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fab1caf2/usage/cli/src/main/java/brooklyn/cli/Main.java ---------------------------------------------------------------------- diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java index ad6aeb1..e6330ea 100644 --- a/usage/cli/src/main/java/brooklyn/cli/Main.java +++ b/usage/cli/src/main/java/brooklyn/cli/Main.java @@ -219,7 +219,7 @@ public class Main extends AbstractMain { @Option(name = { "--catalogInitial" }, title = "catalog initial bom URI", description = "Specifies a catalog.bom URI to be used to populate the initial catalog, " - + "if nothing is yet persisted in the catalog (or if it is reset)") + + "loaded on first run, or when persistence is off/empty or the catalog is reset") public String catalogInitial; @Option(name = { "--catalogReset" },
