You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sj...@apache.org on 2015/10/15 16:02:29 UTC

[08/16] incubator-brooklyn git commit: Fix catalog items recursive check

Fix catalog items recursive check

Identical sibling items shouldn't trigger the recursive check abort.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/73d27909
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/73d27909
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/73d27909

Branch: refs/heads/master
Commit: 73d279097d7a15eb7902c587ee2d5b4400c207b1
Parents: 073c203
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Wed Oct 14 15:35:40 2015 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Oct 14 17:11:07 2015 +0300

----------------------------------------------------------------------
 .../core/mgmt/EntityManagementUtils.java        |  4 ++--
 .../BrooklynAssemblyTemplateInstantiator.java   |  4 ++--
 .../BrooklynComponentTemplateResolver.java      | 25 ++++++++++++++------
 .../brooklyn/spi/creation/CampCatalogUtils.java | 11 +++++++--
 .../service/CatalogServiceSpecResolver.java     | 16 +++++++++----
 .../brooklyn/catalog/CatalogYamlEntityTest.java | 21 ++++++++++++++++
 6 files changed, 64 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/73d27909/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
index cda3f5b..0dca83c 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
@@ -44,7 +44,6 @@ import org.apache.brooklyn.core.plan.PlanToSpecTransformer;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.task.TaskBuilder;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.text.Strings;
@@ -57,6 +56,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 
@@ -99,7 +99,7 @@ public class EntityManagementUtils {
     }
 
     public static <T,SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT createCatalogSpec(ManagementContext mgmt, CatalogItem<T, SpecT> item) {
-        return createCatalogSpec(mgmt, item, MutableSet.<String>of());
+        return createCatalogSpec(mgmt, item, ImmutableSet.<String>of());
     }
 
     public static <T,SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT createCatalogSpec(ManagementContext mgmt, final CatalogItem<T, SpecT> item, final Set<String> encounteredTypes) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/73d27909/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java
index 19aa100..f295e90 100644
--- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java
+++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java
@@ -39,11 +39,11 @@ import org.apache.brooklyn.core.mgmt.HasBrooklynManagementContext;
 import org.apache.brooklyn.core.mgmt.classloading.BrooklynClassLoadingContext;
 import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
 import org.apache.brooklyn.entity.stock.BasicApplicationImpl;
-import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 
@@ -90,7 +90,7 @@ public class BrooklynAssemblyTemplateInstantiator implements AssemblyTemplateSpe
 
         BrooklynComponentTemplateResolver resolver = BrooklynComponentTemplateResolver.Factory.newInstance(
             loader, buildWrapperAppTemplate(template));
-        EntitySpec<? extends Application> app = resolver.resolveSpec(MutableSet.<String>of());
+        EntitySpec<? extends Application> app = resolver.resolveSpec(ImmutableSet.<String>of());
         app.configure(EntityManagementUtils.WRAPPER_APP_MARKER, Boolean.TRUE);
 
         // first build the children into an empty shell app

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/73d27909/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
index 05252f5..b57d4df 100644
--- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
+++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
@@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.annotation.Nullable;
 
+import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.location.Location;
@@ -64,6 +65,7 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Function;
 import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 
@@ -150,13 +152,22 @@ public class BrooklynComponentTemplateResolver {
         EntitySpec<?> spec = serviceSpecResolver.resolve(type, loader, encounteredCatalogTypes);
 
         if (spec == null) {
-            String proto = Urls.getProtocol(type);
-            if (proto != null) {
-                log.debug("The reference " + type + " looks like a URL (running the CAMP Brooklyn assembly-template instantiator) but the protocol " +
-                        proto + " isn't white listed (" + BrooklynCampConstants.YAML_URL_PROTOCOL_WHITELIST + "). " +
-                        "Not a catalog item or java type as well.");
+            // Try to provide some troubleshooting details
+            String msgDetails = "";
+            CatalogItem<?, ?> item = CatalogUtils.getCatalogItemOptionalVersion(mgmt, Strings.removeFromStart(type, "catalog:"));
+            if (item != null && encounteredCatalogTypes.contains(item.getSymbolicName())) {
+                msgDetails = "Cycle between catalog items detected, starting from " + type +
+                        ". Other catalog items being resolved up the stack are " + encounteredCatalogTypes +
+                        ". Tried loading it as a Java class instead but failed.";
+            } else {
+                String proto = Urls.getProtocol(type);
+                if (proto != null) {
+                    msgDetails = "The reference " + type + " looks like a URL (running the CAMP Brooklyn assembly-template instantiator) but the protocol " +
+                            proto + " isn't white listed (" + BrooklynCampConstants.YAML_URL_PROTOCOL_WHITELIST + "). " +
+                            "Not a catalog item or java type as well.";
+                }
             }
-            throw new IllegalStateException("Unable to create spec for type " + type + ". No resolver knew how to handle it.");
+            throw new IllegalStateException("Unable to create spec for type " + type + ". No resolver knew how to handle it. " + msgDetails);
         }
 
         populateSpec(spec, encounteredCatalogTypes);
@@ -352,7 +363,7 @@ public class BrooklynComponentTemplateResolver {
                 @SuppressWarnings("unchecked")
                 Map<String, Object> resolvedConfig = (Map<String, Object>)transformSpecialFlags(specConfig.getSpecConfiguration());
                 specConfig.setSpecConfiguration(resolvedConfig);
-                return Factory.newInstance(getLoader(), specConfig.getSpecConfiguration()).resolveSpec(MutableSet.<String>of());
+                return Factory.newInstance(getLoader(), specConfig.getSpecConfiguration()).resolveSpec(ImmutableSet.<String>of());
             }
             if (flag instanceof ManagementContextInjectable) {
                 log.debug("Injecting Brooklyn management context info object: {}", flag);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/73d27909/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampCatalogUtils.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampCatalogUtils.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampCatalogUtils.java
index 4865241..29791dd 100644
--- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampCatalogUtils.java
+++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampCatalogUtils.java
@@ -31,10 +31,11 @@ import org.apache.brooklyn.core.mgmt.classloading.BrooklynClassLoadingContext;
 import org.apache.brooklyn.util.text.Strings;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
 
 public class CampCatalogUtils {
 
-    public static AbstractBrooklynObjectSpec<?, ?> createSpec(ManagementContext mgmt, CatalogItem<?, ?> item, Set<String> encounteredTypes) {
+    public static AbstractBrooklynObjectSpec<?, ?> createSpec(ManagementContext mgmt, CatalogItem<?, ?> item, Set<String> parentEncounteredTypes) {
         // preferred way is to parse the yaml, to resolve references late;
         // the parsing on load is to populate some fields, but it is optional.
         // TODO messy for location and policy that we need brooklyn.{locations,policies} root of the yaml, but it works;
@@ -43,9 +44,15 @@ public class CampCatalogUtils {
         BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(mgmt, item);
         Preconditions.checkNotNull(item.getCatalogItemType(), "catalog item type for "+item.getPlanYaml());
 
+        Set<String> encounteredTypes;
         // symbolicName could be null if coming from the catalog parser where it tries to load before knowing the id
         if (item.getSymbolicName() != null) {
-            encounteredTypes.add(item.getSymbolicName());
+            encounteredTypes = ImmutableSet.<String>builder()
+                    .addAll(parentEncounteredTypes)
+                    .add(item.getSymbolicName())
+                    .build();
+        } else {
+            encounteredTypes = parentEncounteredTypes;
         }
 
         AbstractBrooklynObjectSpec<?, ?> spec;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/73d27909/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/CatalogServiceSpecResolver.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/CatalogServiceSpecResolver.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/CatalogServiceSpecResolver.java
index 19a76b3..81207ee 100644
--- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/CatalogServiceSpecResolver.java
+++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/CatalogServiceSpecResolver.java
@@ -32,6 +32,8 @@ import org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.ImmutableSet;
+
 public class CatalogServiceSpecResolver extends AbstractServiceSpecResolver {
     private static final Logger log = LoggerFactory.getLogger(CatalogServiceSpecResolver.class);
 
@@ -61,7 +63,7 @@ public class CatalogServiceSpecResolver extends AbstractServiceSpecResolver {
     }
 
     @Override
-    public EntitySpec<?> resolve(String type, BrooklynClassLoadingContext loader, Set<String> encounteredTypes) {
+    public EntitySpec<?> resolve(String type, BrooklynClassLoadingContext loader, Set<String> parentEncounteredTypes) {
         String localType = getLocalType(type);
         CatalogItem<Entity, EntitySpec<?>> item = getCatalogItem(mgmt, localType);
         if (item != null) {
@@ -70,9 +72,15 @@ public class CatalogServiceSpecResolver extends AbstractServiceSpecResolver {
             //Take the symbolicName part of the catalog item only for recursion detection to prevent
             //cross referencing of different versions. Not interested in non-catalog item types.
             //Prevent catalog items self-referencing even if explicitly different version.
-            boolean firstOccurrence = encounteredTypes.add(item.getSymbolicName());
-            boolean nonRecursiveCall = firstOccurrence;
+            boolean nonRecursiveCall = !parentEncounteredTypes.contains(item.getSymbolicName());
             if (nonRecursiveCall) {
+                // Make a copy of the encountered types, so that we add the item being resolved for
+                // dependency items only. Siblings must not see we are resolving this item.
+                Set<String> encounteredTypes = ImmutableSet.<String>builder()
+                        .addAll(parentEncounteredTypes)
+                        .add(item.getSymbolicName())
+                        .build();
+
                 // CatalogItem generics are just getting in the way, better get rid of them, we
                 // are casting anyway.
                 @SuppressWarnings({ "rawtypes" })
@@ -84,7 +92,7 @@ public class CatalogServiceSpecResolver extends AbstractServiceSpecResolver {
                 return null;
             }
         } else {
-            return hardcodedResolver.resolve(type, loader, encounteredTypes);
+            return hardcodedResolver.resolve(type, loader, parentEncounteredTypes);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/73d27909/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
index 53cd146..9699b8e 100644
--- a/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
+++ b/usage/camp/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
@@ -670,6 +670,27 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     }
 
     @Test
+    public void testRecursiveCheckForDepenentsOnly() throws Exception {
+        String symbolicName = "my.catalog.app.id.basic";
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  id: " + symbolicName,
+                "  version: " + TEST_VERSION,
+                "",
+                "services:",
+                "- type: org.apache.brooklyn.entity.stock.BasicEntity");
+
+        createAndStartApplication(
+                "services:",
+                "- type: " + ver(symbolicName),
+                "  brooklyn.children:",
+                "  - type: " + ver(symbolicName),
+                "- type: " + ver(symbolicName),
+                "  brooklyn.children:",
+                "  - type: " + ver(symbolicName));
+    }
+
+    @Test
     public void testOsgiNotLeakingToParent() {
         addCatalogOSGiEntity(SIMPLE_ENTITY_TYPE);
         try {