brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sjcorb...@apache.org
Subject [12/16] incubator-brooklyn git commit: Address code review comments (PR 955)
Date Thu, 15 Oct 2015 14:02:33 GMT
Address code review comments (PR 955)


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

Branch: refs/heads/master
Commit: 5fe288f27ad9902f52ad6601c09f7361f822d433
Parents: d79fd90
Author: Svetoslav Neykov <svetoslav.neykov@cloudsoftcorp.com>
Authored: Thu Oct 15 12:13:30 2015 +0300
Committer: Svetoslav Neykov <svetoslav.neykov@cloudsoftcorp.com>
Committed: Thu Oct 15 12:13:30 2015 +0300

----------------------------------------------------------------------
 .../brooklyn/core/mgmt/EntityManagementUtils.java       |  3 +++
 .../brooklyn/core/plan/PlanToSpecTransformer.java       |  7 ++++++-
 .../core/resolve/CatalogServiceSpecResolver.java        |  9 +--------
 .../brooklyn/core/resolve/ServiceSpecResolver.java      | 12 ++++++++++++
 .../spi/creation/BrooklynComponentTemplateResolver.java |  6 +++---
 .../brooklyn/spi/creation/CampToSpecTransformer.java    |  7 ++++++-
 .../spi/creation/service/UrlServiceSpecResolver.java    |  2 +-
 7 files changed, 32 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/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 0dca83c..5536a1e 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
@@ -103,6 +103,9 @@ public class EntityManagementUtils {
     }
 
     public static <T,SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>>
SpecT createCatalogSpec(ManagementContext mgmt, final CatalogItem<T, SpecT> item, final
Set<String> encounteredTypes) {
+        if (encounteredTypes.contains(item.getSymbolicName())) {
+            throw new IllegalStateException("Already encountered types " + encounteredTypes
+ " must not contain catalog item being resolver " + item.getSymbolicName());
+        }
         return PlanToSpecFactory.attemptWithLoaders(mgmt, new Function<PlanToSpecTransformer,
SpecT>() {
             @Override
             public SpecT apply(PlanToSpecTransformer input) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java b/core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java
index b9ca8ca..24753aa 100644
--- a/core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/plan/PlanToSpecTransformer.java
@@ -56,7 +56,12 @@ public interface PlanToSpecTransformer extends ManagementContextInjectable
{
      * the catalog item might be known by type, or its source plan fragment text might be
inspected and transformed.
      * implementations will typically look at the {@link CatalogItem#getCatalogItemType()}
first.
      * <p>
-     * should throw {@link PlanNotRecognizedException} if this transformer does not know
what to do with the plan. */
+     * should throw {@link PlanNotRecognizedException} if this transformer does not know
what to do with the plan.
+     * 
+     * @param item - The catalog item to convert to a spec. The item might not be fully populated
(i.e. missing {@code symbolicName} if called
+     *        from the catalog parser).
+     * @param encounteredTypes - The {@code symbolicName}s of catalog items being resolved
up the stack, but not including {@code item}.
+     */
     <T,SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>> SpecT createCatalogSpec(CatalogItem<T,
SpecT> item, Set<String> encounteredTypes) throws PlanNotRecognizedException;
     
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/core/src/main/java/org/apache/brooklyn/core/resolve/CatalogServiceSpecResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/CatalogServiceSpecResolver.java
b/core/src/main/java/org/apache/brooklyn/core/resolve/CatalogServiceSpecResolver.java
index f05355a..ded4ed3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/CatalogServiceSpecResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/CatalogServiceSpecResolver.java
@@ -73,19 +73,12 @@ public class CatalogServiceSpecResolver extends AbstractServiceSpecResolver
{
         //Prevent catalog items self-referencing even if explicitly different version.
         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" })
             CatalogItem rawItem = item;
             @SuppressWarnings({ "rawtypes", "unchecked" })
-            AbstractBrooklynObjectSpec rawSpec = EntityManagementUtils.createCatalogSpec(mgmt,
rawItem, encounteredTypes);
+            AbstractBrooklynObjectSpec rawSpec = EntityManagementUtils.createCatalogSpec(mgmt,
rawItem, parentEncounteredTypes);
             return (EntitySpec<?>) rawSpec;
         } else {
             return null;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/core/src/main/java/org/apache/brooklyn/core/resolve/ServiceSpecResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/ServiceSpecResolver.java
b/core/src/main/java/org/apache/brooklyn/core/resolve/ServiceSpecResolver.java
index 5e30cfa..64a6c66 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/ServiceSpecResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/ServiceSpecResolver.java
@@ -50,6 +50,18 @@ public interface ServiceSpecResolver extends ManagementContextInjectable
{
 
     /**
      * Create a spec for the service type
+     * 
+     * @param type - the string representation which should be converted to an EntitySpec
+     * @param loader - use it to load any Java classes
+     * @param encounteredTypes - an immutable set of the items which are currently being
resolved up the stack,
+     *        used to prevent cycles. Implementations should not try to resolve the type
if the symbolicName is
+     *        already contained in here. When resolving a type add it to a copy of the list
before
+     *        passing the new instance down the stack. See {@link CatalogServiceSpecResolver}
for example usage.
+     *
+     * @return The {@link EntitySpec} corresponding to the passed {@code type} argument,
possibly pre-configured
+     *         based on the information contained in {@code type}. Return {@code null} value
to indicate that
+     *         the implementation doesn't know how to convert {@code type} to an {@link EntitySpec}.
Throw an
+     *         exception if {@code type} looks like a supported value, but can't be loaded.
      */
     @Nullable EntitySpec<?> resolve(String type, BrooklynClassLoadingContext loader,
Set<String> encounteredTypes);
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/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 e6a866e..cfdfab2 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
@@ -163,7 +163,7 @@ public class BrooklynComponentTemplateResolver {
             } else 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.";
+                        "It's also neither a catalog item nor a java type.";
             } else {
                 msgDetails = "No resolver knew how to handle it. Using resolvers: " + serviceSpecResolver;
             }
@@ -226,10 +226,10 @@ public class BrooklynComponentTemplateResolver {
         if (childLocations != null)
             spec.locations(childLocations);
 
-        decoreateSpec(spec);
+        decorateSpec(spec);
     }
 
-    private <T extends Entity> void decoreateSpec(EntitySpec<T> spec) {
+    private <T extends Entity> void decorateSpec(EntitySpec<T> spec) {
         new BrooklynEntityDecorationResolver.PolicySpecResolver(yamlLoader).decorate(spec,
attrs);
         new BrooklynEntityDecorationResolver.EnricherSpecResolver(yamlLoader).decorate(spec,
attrs);
         new BrooklynEntityDecorationResolver.InitializerResolver(yamlLoader).decorate(spec,
attrs);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampToSpecTransformer.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampToSpecTransformer.java
b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampToSpecTransformer.java
index 9104f8f..60f00c6 100644
--- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampToSpecTransformer.java
+++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampToSpecTransformer.java
@@ -93,7 +93,12 @@ public class CampToSpecTransformer implements PlanToSpecTransformer {
     @Override
     public <T, SpecT extends AbstractBrooklynObjectSpec<? extends T, SpecT>>
SpecT createCatalogSpec(CatalogItem<T, SpecT> item, Set<String> encounteredTypes)
{
         // Ignore old-style java type catalog items
-        if (item.getPlanYaml() == null) return null;
+        if (item.getPlanYaml() == null) {
+            throw new PlanNotRecognizedException("Old style catalog item " + item + " not
supported.");
+        }
+        if (encounteredTypes.contains(item.getSymbolicName())) {
+            throw new IllegalStateException("Already encountered types " + encounteredTypes
+ " must not contain catalog item being resolver " + item.getSymbolicName());
+        }
 
         // Not really clear what should happen to the top-level attributes, ignored until
a good use case appears.
         return (SpecT) CampCatalogUtils.createSpec(mgmt, (CatalogItem)item, encounteredTypes);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5fe288f2/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/UrlServiceSpecResolver.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/UrlServiceSpecResolver.java
b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/UrlServiceSpecResolver.java
index b55c064..bde501b 100644
--- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/UrlServiceSpecResolver.java
+++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/service/UrlServiceSpecResolver.java
@@ -54,7 +54,7 @@ public class UrlServiceSpecResolver implements ServiceSpecResolver {
         try {
             yaml = ResourceUtils.create(this).getResourceAsString(type);
         } catch (Exception e) {
-            log.warn("AssemblyTemplate type " + type + " which looks like a URL can't be
fetched.", e);
+            log.warn("AssemblyTemplate type " + type + " looks like a URL that can't be fetched.",
e);
             return null;
         }
         // Referenced specs are expected to be CAMP format as well.


Mime
View raw message