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:33 UTC
[12/16] incubator-brooklyn git commit: Address code review comments
(PR 955)
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 <sv...@cloudsoftcorp.com>
Authored: Thu Oct 15 12:13:30 2015 +0300
Committer: Svetoslav Neykov <sv...@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.