brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s...@apache.org
Subject [07/10] brooklyn-server git commit: BROOKLYN-382: use catalog.peekSpec()
Date Wed, 16 Nov 2016 18:15:51 GMT
BROOKLYN-382: use catalog.peekSpec()

* Deprecates catalog.createSpec()
* Adds catalog.peekSpec(), to return a spec object that is not intended
  for creating apps, but instead for just inspecting additional info
  of the item.
* Tidies up the BasicBrooklynCatalog.specCache
  (e.g. invalidate it all if anything changes in the catalog).
* Simplifies the generics of catalog.peekSpec, so that callers don’t
  do horrible casting to remove generics!


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

Branch: refs/heads/master
Commit: 405ddb98d1e7df984b0dfcd13aeb06053818e2d1
Parents: 5ed7b81
Author: Aled Sage <aled.sage@gmail.com>
Authored: Mon Nov 14 13:29:10 2016 +0000
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Wed Nov 16 18:09:14 2016 +0000

----------------------------------------------------------------------
 .../brooklyn/api/catalog/BrooklynCatalog.java   | 24 +++++-
 .../catalog/SpecParameterUnwrappingTest.java    | 28 +++----
 .../qa/performance/CatalogPerformanceTest.java  | 16 ++--
 .../brooklyn/test/lite/CampYamlLiteTest.java    |  3 +-
 .../catalog/internal/BasicBrooklynCatalog.java  | 80 ++++++++++++++++----
 .../core/catalog/internal/CatalogScanTest.java  |  3 +-
 .../rest/transform/CatalogTransformer.java      |  4 +-
 .../main/java/org/apache/brooklyn/cli/Main.java |  4 +-
 .../brooklyn/cli/lister/ItemDescriptors.java    |  5 +-
 9 files changed, 111 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/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 b47d4b1..248b06b 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
@@ -22,6 +22,7 @@ import java.util.Collection;
 import java.util.NoSuchElementException;
 
 import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
+import org.apache.brooklyn.api.mgmt.ManagementContext;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Predicate;
@@ -65,10 +66,29 @@ public interface BrooklynCatalog {
      * so it is safe for callers to keep a handle on this. */
     public ClassLoader getRootClassLoader();
 
-    /** creates a spec for the given catalog item, throwing exceptions if any problems */
-    // TODO this should be cached on the item and renamed getSpec(...), else we re-create
it too often (every time catalog is listed)
+    /**
+     * Creates a spec for the given catalog item, throwing exceptions if any problems.
+     * 
+     * Use of this method is strongly discouraged. It causes the catalog item to be re-parsed

+     * every time, which is very inefficient.
+     * 
+     * @deprecated since 0.10.0; use {@link #peekSpec(CatalogItem)} for a preview of what
the item
+     *             corresponds to.
+     */
     <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT createSpec(CatalogItem<T,
SpecT> item);
 
+    /** 
+     * Creates a spec for the given catalog item, throwing exceptions if any problems. The
returned 
+     * spec is intended for getting additional information about the item. It should not
be used 
+     * for creating entities/apps!
+     * 
+     * See {@code EntityManagementUtils.createEntitySpecForApplication(ManagementContext
mgmt, String plan)}
+     * for how apps are deployed.
+     * 
+     * @since 0.10.0
+     */
+    AbstractBrooklynObjectSpec<?, ?> peekSpec(CatalogItem<?, ?> item);
+
     /**
      * Adds an item (represented in yaml) to the catalog.
      * Fails if the same version exists in catalog.

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
index 4b7a5f7..c0eb8ef 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
@@ -111,7 +111,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
         ConfigKey<String> SIMPLE_CONFIG = ConfigKeys.newStringConfigKey("simple");
         SpecParameter<String> SIMPLE_PARAM = new BasicSpecParameter<>("simple",
true, SIMPLE_CONFIG);
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         assertTrue(Iterables.tryFind(spec.getParameters(), Predicates.<SpecParameter<?>>equalTo(SIMPLE_PARAM)).isPresent());
     }
 
@@ -126,7 +126,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
             "    type: "+ testClass.getName());
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?, ?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?, ?> spec = catalog.peekSpec(item);
         assertEquals(ImmutableSet.copyOf(spec.getParameters()), ImmutableSet.copyOf(BasicSpecParameter.fromClass(mgmt(),testClass)));
     }
 
@@ -144,7 +144,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "    - simple");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         assertEquals(params.size(), NUM_APP_DEFAULT_CONFIG_KEYS + 1, "params="+params);
         assertTrue(Iterables.tryFind(params, nameEqualTo("simple")).isPresent());
@@ -167,7 +167,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "      type: paramItem");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         switch (type.getSimpleName()) {
             case "ConfigEntityForTest":
@@ -203,7 +203,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "      - override");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         switch (type.getSimpleName()) {
             case "ConfigEntityForTest":
@@ -240,7 +240,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "        label: override");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         switch (type.getSimpleName()) {
             case "ConfigEntityForTest":
@@ -279,7 +279,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "        default: rabbits");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         assertTrue(Iterables.tryFind(params, nameEqualTo("simple")).isPresent());
         Optional<ConfigKey<?>> config = Iterables.tryFind(spec.getConfig().keySet(),
ConfigPredicates.nameEqualTo("simple"));
@@ -304,7 +304,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "        simple: value");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         assertEquals(params.size(), 3);
         assertTrue(Iterables.tryFind(params, nameEqualTo("simple")).isPresent());
@@ -333,7 +333,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "        default: rabbits");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         assertTrue(Iterables.tryFind(params, nameEqualTo("simple")).isPresent());
         Optional<ConfigKey<?>> config = Iterables.tryFind(spec.getConfig().keySet(),
ConfigPredicates.nameEqualTo("simple"));
@@ -357,8 +357,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "      - simple");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(SYMBOLIC_NAME, TEST_VERSION);
-        @SuppressWarnings({ "rawtypes", "unchecked"})
-        EntitySpec<?> parentSpec = (EntitySpec<?>) catalog.createSpec((CatalogItem)item);
+        EntitySpec<?> parentSpec = (EntitySpec<?>) catalog.peekSpec(item);
         EntitySpec<?> spec = parentSpec.getChildren().get(0);
         List<SpecParameter<?>> params = spec.getParameters();
         assertEquals(params.size(), 3, "params="+params);
@@ -539,7 +538,7 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
                 "              test: $brooklyn:config(\"num\")");
 
         CatalogItem<?, ?> item = catalog.getCatalogItem(ConfigEntityForTest.class.getSimpleName()
+ "WithParams", TEST_VERSION);
-        AbstractBrooklynObjectSpec<?,?> spec = createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<SpecParameter<?>> params = spec.getParameters();
         assertEquals(params.size(), 3);
         assertTrue(Iterables.tryFind(params, nameEqualTo("num")).isPresent());
@@ -596,11 +595,6 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
         assertEquals(firstInput.getLabel(), "simple");
     }
 
-    @SuppressWarnings({"unchecked", "rawtypes"})
-    private AbstractBrooklynObjectSpec<?, ?> createSpec(CatalogItem<?, ?> item)
{
-        return (AbstractBrooklynObjectSpec<?,?>) catalog.createSpec((CatalogItem)item);
-    }
-
     private EntitySpec<? extends Application> createAppSpec(String... lines) {
         return EntityManagementUtils.createEntitySpecForApplication(mgmt(), joinLines(lines));
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/qa/performance/CatalogPerformanceTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/qa/performance/CatalogPerformanceTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/qa/performance/CatalogPerformanceTest.java
index 5382cc5..dac44d8 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/qa/performance/CatalogPerformanceTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/qa/performance/CatalogPerformanceTest.java
@@ -114,7 +114,7 @@ public class CatalogPerformanceTest extends AbstractYamlTest {
     }
     
     @Test(groups={"Integration"})
-    public void testCreateSpecs() {
+    public void testPeekSpecs() {
         final AtomicInteger counter = new AtomicInteger();
         final AtomicReference<List<CatalogItem<?,?>>> items = new AtomicReference<>();
         
@@ -125,11 +125,9 @@ public class CatalogPerformanceTest extends AbstractYamlTest {
             }
         };
         Runnable job = new Runnable() {
-            @SuppressWarnings({"unchecked", "rawtypes"})
             public void run() {
                 for (CatalogItem<?, ?> item : items.get()) {
-                    CatalogItem castItem = (CatalogItem) item;
-                    mgmt().getCatalog().createSpec(castItem);
+                    mgmt().getCatalog().peekSpec(item);
                 }
             }
         };
@@ -142,23 +140,21 @@ public class CatalogPerformanceTest extends AbstractYamlTest {
                 }
             }
         };
-        runPerformanceTest("testCreateSpecs", preJob, job, postJob);
+        runPerformanceTest("testPeekSpecs", preJob, job, postJob);
     }
     
     @Test(groups={"Integration"})
-    public void testCreateSameSpecsRepeatedly() {
+    public void testPeekSameSpecsRepeatedly() {
         final List<CatalogItem<?, ?>> items = addItems(0);
         
         Runnable job = new Runnable() {
-            @SuppressWarnings({"unchecked", "rawtypes"})
             public void run() {
                 for (CatalogItem<?, ?> item : items) {
-                    CatalogItem castItem = (CatalogItem) item;
-                    mgmt().getCatalog().createSpec(castItem);
+                    mgmt().getCatalog().peekSpec(item);
                 }
             }
         };
-        runPerformanceTest("testCreateSpecs", null, job, null);
+        runPerformanceTest("testPeekSameSpecsRepeatedly", null, job, null);
     }
     
     protected void runPerformanceTest(String methodName, Runnable preJob, Runnable job, Runnable
postJob) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/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 89cbe88..1f259a2 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
@@ -171,8 +171,7 @@ public class CampYamlLiteTest {
         Assert.assertEquals(bundle.getUrl(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL);
         Assert.assertEquals(bundle.getVersion(), "0.1.0");
 
-        @SuppressWarnings({ "unchecked", "rawtypes" })
-        EntitySpec<?> spec1 = (EntitySpec<?>) mgmt.getCatalog().createSpec((CatalogItem)retrievedItem);
+        EntitySpec<?> spec1 = (EntitySpec<?>) mgmt.getCatalog().peekSpec(retrievedItem);
         assertNotNull(spec1);
         Assert.assertEquals(spec1.getConfig().get(TestEntity.CONF_NAME), "sample");
         

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/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 1a48d27..641e7f6 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
@@ -22,6 +22,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
@@ -66,6 +67,7 @@ import org.slf4j.LoggerFactory;
 import org.yaml.snakeyaml.Yaml;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.Collections2;
@@ -108,11 +110,18 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     private final AggregateClassLoader rootClassLoader = AggregateClassLoader.newInstanceWithNoLoaders();
     
     /**
-     * Cache from CatalogItem id to the resolved spec for that catalog item.
+     * Cache of specs (used by {@link #peekSpec(CatalogItem)}).
      * We assume that no-one is modifying the catalog items (once added) without going through
the
      * correct accessor methods here (e.g. no-one calling {@code getCatalogItemDo().getDto().setXyz()}).
+     * 
+     * As discussed in https://github.com/apache/brooklyn-server/pull/423 and BROOKLYN-382,
there  
+     * are things outside of the control of the catalog that a spec depends on - like non-catalog

+     * locations, type registry, adding bundles, etc. However, because this cache is only
used for
+     * {@link #peekSpec(CatalogItem)}, it is considered good enough.
+     * 
+     * A longer term improvement is to focus on our YAML parsing, to make that faster and
better!
      */
-    private final Map<String, AbstractBrooklynObjectSpec<?,?>> specCache;
+    private final SpecCache specCache;
 
     public BasicBrooklynCatalog(ManagementContext mgmt) {
         this(mgmt, CatalogDto.newNamedInstance("empty catalog", "empty catalog", "empty catalog,
expected to be reset later"));
@@ -121,7 +130,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     public BasicBrooklynCatalog(ManagementContext mgmt, CatalogDto dto) {
         this.mgmt = checkNotNull(mgmt, "managementContext");
         this.catalog = new CatalogDo(mgmt, dto);
-        this.specCache = Maps.newLinkedHashMap();
+        this.specCache = new SpecCache();
     }
 
     public boolean blockIfNotLoaded(Duration timeout) {
@@ -137,7 +146,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
 
     public void reset(CatalogDto dto, boolean failOnLoadError) {
-        specCache.clear();
+        specCache.invalidate();
         // Unregister all existing persisted items.
         for (CatalogItem<?, ?> toRemove : getCatalogItems()) {
             if (log.isTraceEnabled()) {
@@ -254,7 +263,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         manualAdditionsCatalog.deleteEntry(itemDto);
         
         // Ensure the caches are de-populated
-        specCache.remove(itemDto.getCatalogItemId());
+        specCache.invalidate();
         getCatalog().deleteEntry(itemDto);
 
         // And indicate to the management context that it should be removed.
@@ -296,7 +305,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
 
     private void resetRootClassLoader() {
-        specCache.clear();
+        specCache.invalidate();
         rootClassLoader.reset(ImmutableList.of(catalog.getRootClassLoader()));
     }
 
@@ -312,20 +321,21 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
 
     @Override
-    @SuppressWarnings("unchecked")
-    public <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>>
SpecT createSpec(CatalogItem<T, SpecT> item) {
+    public AbstractBrooklynObjectSpec<?, ?> peekSpec(CatalogItem<?, ?> item)
{
         if (item == null) return null;
-        CatalogItemDo<T,SpecT> loadedItem = (CatalogItemDo<T, SpecT>) getCatalogItemDo(item.getSymbolicName(),
item.getVersion());
+        CatalogItemDo<?, ?> loadedItem = (CatalogItemDo<?, ?>) getCatalogItemDo(item.getSymbolicName(),
item.getVersion());
         if (loadedItem == null) throw new RuntimeException(item+" not in catalog; cannot
create spec");
         if (loadedItem.getSpecType()==null) return null;
         String itemId = item.getCatalogItemId();
         
-        if (specCache.containsKey(itemId)) {
-            return (SpecT) specCache.get(itemId);
+        Optional<AbstractBrooklynObjectSpec<?, ?>> cachedSpec = specCache.getSpec(itemId);
+        if (cachedSpec.isPresent()) {
+            return cachedSpec.get();
         } else {
-            SpecT spec = internalCreateSpecLegacy(mgmt, loadedItem, MutableSet.<String>of(),
true);
+            @SuppressWarnings({ "rawtypes", "unchecked" })
+            AbstractBrooklynObjectSpec<?, ?> spec = internalCreateSpecLegacy(mgmt,
(CatalogItem)loadedItem, MutableSet.<String>of(), true);
             if (spec != null) {
-                specCache.put(itemId, spec);
+                specCache.addSpec(itemId, spec);
                 return spec;
             }
         }
@@ -333,6 +343,23 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         throw new IllegalStateException("No known mechanism to create instance of "+item);
     }
     
+    @Override
+    @Deprecated
+    @SuppressWarnings("unchecked")
+    public <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>>
SpecT createSpec(CatalogItem<T, SpecT> item) {
+        if (item == null) return null;
+        CatalogItemDo<T,SpecT> loadedItem = (CatalogItemDo<T, SpecT>) getCatalogItemDo(item.getSymbolicName(),
item.getVersion());
+        if (loadedItem == null) throw new RuntimeException(item+" not in catalog; cannot
create spec");
+        if (loadedItem.getSpecType()==null) return null;
+        
+        SpecT spec = internalCreateSpecLegacy(mgmt, loadedItem, MutableSet.<String>of(),
true);
+        if (spec != null) {
+            return spec;
+        }
+        
+        throw new IllegalStateException("No known mechanism to create instance of "+item);
+    }
+    
     /** @deprecated since introduction in 0.9.0, only used for backwards compatibility, can
be removed any time;
      * uses the type-creation info on the item.
      * deprecated transformers must be included by routines which don't use {@link BrooklynTypePlanTransformer}
instances;
@@ -962,7 +989,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         }
 
         // Clear spec cache (in-case overwriting existing)
-        specCache.remove(itemDto.getCatalogItemId());
+        specCache.invalidate();
         
         if (manualAdditionsCatalog==null) loadManualAdditionsCatalog();
         manualAdditionsCatalog.addEntry(itemDto);
@@ -1011,7 +1038,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     @Override @Deprecated /** @deprecated see super */
     public void addItem(CatalogItem<?,?> item) {
         // Clear spec-cache (in-case overwriting)
-        specCache.remove(item.getCatalogItemId());
+        specCache.invalidate();
         
         //assume forceUpdate for backwards compatibility
         log.debug("Adding manual catalog item to "+mgmt+": "+item);
@@ -1031,7 +1058,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         CatalogItem<?, ?> result = manualAdditionsCatalog.classpath.addCatalogEntry(type);
         
         // Clear spec-cache (in-case overwriting)
-        specCache.remove(result.getCatalogItemId());
+        specCache.invalidate();
         
         return result;
     }
@@ -1144,4 +1171,25 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
             serializer = new CatalogXmlSerializer();
     }
 
+    private static class SpecCache {
+        private final Map<String, AbstractBrooklynObjectSpec<?,?>> cache = Collections.synchronizedMap(
+                Maps.<String, AbstractBrooklynObjectSpec<?,?>>newLinkedHashMap());
+
+        /**
+         * Whenever anything in the catalog is modified, the entire cache should be invalidated.

+         * This is because items in the cache can refer to each other, which can impact the
Spec
+         * created for a given catalog item.
+         */
+        public void invalidate() {
+            cache.clear();
+        }
+        
+        public Optional<AbstractBrooklynObjectSpec<?,?>> getSpec(String itemId)
{
+            return Optional.<AbstractBrooklynObjectSpec<?,?>>fromNullable(cache.get(itemId));
+        }
+        
+        public void addSpec(String itemId, AbstractBrooklynObjectSpec<?,?> spec) {
+            cache.put(itemId, spec);
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogScanTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogScanTest.java
b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogScanTest.java
index da5ca53..f08b771 100644
--- a/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogScanTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/catalog/internal/CatalogScanTest.java
@@ -121,8 +121,7 @@ public class CatalogScanTest {
         
         Assert.assertEquals(s1.getDescription(), "Some silly app test");
         
-        @SuppressWarnings({ "unchecked", "rawtypes" })
-        Class<?> app = c.createSpec((CatalogItem)s1).getType();
+        Class<?> app = c.peekSpec(s1).getType();
         Assert.assertEquals(MySillyAppTemplate.class, app);
         
         String xml = ((BasicBrooklynCatalog)c).toXmlString();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java
index cdbef0a..e74bbcd 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/CatalogTransformer.java
@@ -79,7 +79,7 @@ public class CatalogTransformer {
         EntitySpec<?> spec = null;
 
         try {
-            spec = (EntitySpec<?>) b.getCatalog().createSpec((CatalogItem) item);
+            spec = (EntitySpec<?>) b.getCatalog().peekSpec(item);
             EntityDynamicType typeMap = BrooklynTypes.getDefinedEntityType(spec.getType());
             EntityType type = typeMap.getSnapshot();
 
@@ -136,7 +136,7 @@ public class CatalogTransformer {
     public static CatalogPolicySummary catalogPolicySummary(BrooklynRestResourceUtils b,
CatalogItem<? extends Policy,PolicySpec<?>> item, UriBuilder ub) {
         final Set<PolicyConfigSummary> config = Sets.newLinkedHashSet();
         try{
-            final PolicySpec<?> spec = (PolicySpec<?>) b.getCatalog().createSpec((CatalogItem)
item);
+            final PolicySpec<?> spec = (PolicySpec<?>) b.getCatalog().peekSpec(item);
             for (final SpecParameter<?> input : spec.getParameters()){
                 config.add(EntityTransformer.policyConfigSummary(input));
             }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
index 140916a..51a35bb 100644
--- a/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
+++ b/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
@@ -39,6 +39,7 @@ import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
@@ -641,8 +642,7 @@ public class Main extends AbstractMain {
                         // and additionally they might contain multiple items in which case
                         // the validation below won't work anyway (you need to go via a deployment
plan)
                     } else {
-                        @SuppressWarnings({ "unchecked", "rawtypes" })
-                        Object spec = catalog.createSpec((CatalogItem)item);
+                        AbstractBrooklynObjectSpec<?, ?> spec = catalog.peekSpec(item);
                         if (spec instanceof EntitySpec) {
                             BrooklynTypes.getDefinedEntityType(((EntitySpec<?>)spec).getType());
                         }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/405ddb98/server-cli/src/main/java/org/apache/brooklyn/cli/lister/ItemDescriptors.java
----------------------------------------------------------------------
diff --git a/server-cli/src/main/java/org/apache/brooklyn/cli/lister/ItemDescriptors.java
b/server-cli/src/main/java/org/apache/brooklyn/cli/lister/ItemDescriptors.java
index ba644e0..f245d06 100644
--- a/server-cli/src/main/java/org/apache/brooklyn/cli/lister/ItemDescriptors.java
+++ b/server-cli/src/main/java/org/apache/brooklyn/cli/lister/ItemDescriptors.java
@@ -181,10 +181,9 @@ public class ItemDescriptors {
         return resolver.getPrefix();
     }
     
-    @SuppressWarnings({ "rawtypes", "unchecked" }) 
-    public static Map<String, Object> toItemDescriptor(BrooklynCatalog catalog, CatalogItem
item, boolean headingsOnly) {
+    public static Map<String, Object> toItemDescriptor(BrooklynCatalog catalog, CatalogItem<?,
?> item, boolean headingsOnly) {
         Map<String,Object> itemDescriptor = MutableMap.of();
-        AbstractBrooklynObjectSpec<?,?> spec = catalog.createSpec(item);
+        AbstractBrooklynObjectSpec<?,?> spec = catalog.peekSpec(item);
         List<EntityConfigSummary> config = new ArrayList<>();
 
         if (item.getDisplayName() != null) {


Mime
View raw message