address code review for multi-item catalog * use `itemType` instead of `item.type` * infer itemType from the class - prompted major refactor of how items are parsed, but cleaner now (at least code; logic has some TODO improvements listed) * use service spec format for entity * add atomically (and lookup types against just-added items)
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/6f75f40b Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/6f75f40b Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/6f75f40b Branch: refs/heads/master Commit: 6f75f40bc3fa142d754afdffacc842b1f8da2654 Parents: a0ddec1 Author: Alex Heneveld <[email protected]> Authored: Tue Apr 14 13:17:57 2015 -0500 Committer: Alex Heneveld <[email protected]> Committed: Thu Apr 16 01:25:40 2015 -0500 ---------------------------------------------------------------------- .../main/java/brooklyn/catalog/CatalogItem.java | 3 +- .../io/brooklyn/camp/BasicCampPlatform.java | 7 +- .../catalog/internal/BasicBrooklynCatalog.java | 512 +++++++++++-------- .../location/geo/UtraceHostGeoLookup.java | 4 +- .../policy/basic/AbstractEntityAdjunct.java | 13 + docs/guide/ops/catalog/index.md | 38 +- .../creation/BrooklynYamlTypeInstantiator.java | 3 +- .../camp/brooklyn/AbstractYamlTest.java | 10 +- .../brooklyn/catalog/CatalogYamlCombiTest.java | 145 ++++++ .../brooklyn/catalog/CatalogYamlEntityTest.java | 141 ++++- .../brooklyn/catalog/CatalogYamlPolicyTest.java | 11 +- .../catalog/CatalogYamlTemplateTest.java | 33 +- .../src/test/resources/couchbase-w-loadgen.yaml | 2 +- .../src/main/java/brooklyn/util/yaml/Yamls.java | 33 +- 14 files changed, 672 insertions(+), 283 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/api/src/main/java/brooklyn/catalog/CatalogItem.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/brooklyn/catalog/CatalogItem.java b/api/src/main/java/brooklyn/catalog/CatalogItem.java index 8abaddc..a857a53 100644 --- a/api/src/main/java/brooklyn/catalog/CatalogItem.java +++ b/api/src/main/java/brooklyn/catalog/CatalogItem.java @@ -84,7 +84,8 @@ public interface CatalogItem<T,SpecT> extends BrooklynObject, Rebindable { public String toXmlString(); - /** @return The underlying YAML for this item, if known */ + /** @return The underlying YAML for this item, if known; + * currently including `services:` or `brooklyn.policies:` prefix (but this will likely be removed) */ @Nullable public String getPlanYaml(); @Override http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/camp/camp-base/src/main/java/io/brooklyn/camp/BasicCampPlatform.java ---------------------------------------------------------------------- diff --git a/camp/camp-base/src/main/java/io/brooklyn/camp/BasicCampPlatform.java b/camp/camp-base/src/main/java/io/brooklyn/camp/BasicCampPlatform.java index 7f6d4b0..16a3dee 100644 --- a/camp/camp-base/src/main/java/io/brooklyn/camp/BasicCampPlatform.java +++ b/camp/camp-base/src/main/java/io/brooklyn/camp/BasicCampPlatform.java @@ -131,8 +131,11 @@ public class BasicCampPlatform extends CampPlatform { @Override protected void finalize() throws Throwable { - if (!committed.get()) - log.warn("transaction "+this+" was never applied"); + if (!committed.get()) { + // normal, in the case of errors (which might occur when catalog tries to figure out the right plan format); shouldn't happen otherwise + // if we want log.warn visibility of these, then we will have to supply an abandon() method on this interface and ensure that is invoked on errors + log.debug("transaction "+this+" was never applied"); + } super.finalize(); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/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 b773247..680eaf1 100644 --- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java @@ -24,7 +24,6 @@ import io.brooklyn.camp.CampPlatform; import io.brooklyn.camp.spi.AssemblyTemplate; import io.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator; import io.brooklyn.camp.spi.pdp.DeploymentPlan; -import io.brooklyn.camp.spi.pdp.Service; import java.io.FileNotFoundException; import java.util.Collection; @@ -48,13 +47,11 @@ import brooklyn.catalog.CatalogItem.CatalogBundle; import brooklyn.catalog.CatalogItem.CatalogItemType; import brooklyn.catalog.CatalogPredicates; import brooklyn.config.BrooklynServerConfig; -import brooklyn.entity.proxying.EntitySpec; import brooklyn.location.Location; import brooklyn.location.LocationSpec; import brooklyn.location.basic.BasicLocationRegistry; import brooklyn.management.ManagementContext; import brooklyn.management.classloading.BrooklynClassLoadingContext; -import brooklyn.management.internal.EntityManagementUtils; import brooklyn.management.internal.ManagementContextInternal; import brooklyn.policy.Policy; import brooklyn.policy.PolicySpec; @@ -75,6 +72,7 @@ import brooklyn.util.yaml.Yamls; import brooklyn.util.yaml.Yamls.YamlExtract; import com.google.common.base.Function; +import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.base.Throwables; @@ -366,6 +364,18 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { return spec; } + private <T, SpecT> SpecT createSpec(String optionalId, CatalogItemType ciType, DeploymentPlan plan, BrooklynClassLoadingContext loader) { + Preconditions.checkNotNull(ciType, "catalog item type for "+plan); + switch (ciType) { + case TEMPLATE: + case ENTITY: + return createEntitySpec(optionalId, plan, loader); + case LOCATION: return createLocationSpec(plan, loader); + case POLICY: return createPolicySpec(plan, loader); + } + throw new IllegalStateException("Unknown CI Type "+ciType+" for "+plan); + } + @SuppressWarnings("unchecked") private <T, SpecT> SpecT createEntitySpec(String symbolicName, DeploymentPlan plan, BrooklynClassLoadingContext loader) { CampPlatform camp = BrooklynServerConfig.getCampPlatform(mgmt).get(); @@ -382,7 +392,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { try { AssemblyTemplateInstantiator instantiator = at.getInstantiator().newInstance(); if (instantiator instanceof AssemblyTemplateSpecInstantiator) { - return (SpecT) ((AssemblyTemplateSpecInstantiator)instantiator).createNestedSpec(at, camp, loader, MutableSet.of(symbolicName)); + return (SpecT) ((AssemblyTemplateSpecInstantiator)instantiator).createNestedSpec(at, camp, loader, + symbolicName==null ? MutableSet.<String>of() : MutableSet.of(symbolicName)); } throw new IllegalStateException("Unable to instantiate YAML; incompatible instantiator "+instantiator+" for "+at); } catch (Exception e) { @@ -405,17 +416,18 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { @SuppressWarnings("unchecked") private <T, SpecT> SpecT createPolicySpec(BrooklynClassLoadingContext loader, Object policy) { - Map<String, Object> config; + // TODO this (and LocationSpec) lack the loop-prevention which createEntitySpec has (hence different signature) + Map<String, Object> itemMap; if (policy instanceof String) { - config = ImmutableMap.<String, Object>of("type", policy); + itemMap = ImmutableMap.<String, Object>of("type", policy); } else if (policy instanceof Map) { - config = (Map<String, Object>) policy; + itemMap = (Map<String, Object>) policy; } else { throw new IllegalStateException("Policy expected to be string or map. Unsupported object type " + policy.getClass().getName() + " (" + policy.toString() + ")"); } - String type = (String) checkNotNull(Yamls.getMultinameAttribute(config, "policy_type", "policyType", "type"), "policy type"); - Map<String, Object> brooklynConfig = (Map<String, Object>) config.get("brooklyn.config"); + String type = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "policy_type", "policyType", "type"), "policy type"); + Map<String, Object> brooklynConfig = (Map<String, Object>) itemMap.get("brooklyn.config"); PolicySpec<? extends Policy> spec = PolicySpec.create(loader.loadClass(type, Policy.class)); if (brooklynConfig != null) { spec.configure(brooklynConfig); @@ -438,17 +450,17 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { @SuppressWarnings("unchecked") private <T, SpecT> SpecT createLocationSpec(BrooklynClassLoadingContext loader, Object location) { - Map<String, Object> config; + Map<String, Object> itemMap; if (location instanceof String) { - config = ImmutableMap.<String, Object>of("type", location); + itemMap = ImmutableMap.<String, Object>of("type", location); } else if (location instanceof Map) { - config = (Map<String, Object>) location; + itemMap = (Map<String, Object>) location; } else { throw new IllegalStateException("Location expected to be string or map. Unsupported object type " + location.getClass().getName() + " (" + location.toString() + ")"); } - String type = (String) checkNotNull(Yamls.getMultinameAttribute(config, "location_type", "locationType", "type"), "location type"); - Map<String, Object> brooklynConfig = (Map<String, Object>) config.get("brooklyn.config"); + String type = (String) checkNotNull(Yamls.getMultinameAttribute(itemMap, "location_type", "locationType", "type"), "location type"); + Map<String, Object> brooklynConfig = (Map<String, Object>) itemMap.get("brooklyn.config"); Maybe<Class<? extends Location>> javaClass = loader.tryLoadClass(type, Location.class); if (javaClass.isPresent()) { LocationSpec<?> spec = LocationSpec.create(javaClass.get()); @@ -534,17 +546,17 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { return (Maybe<Map<?,?>>)(Maybe) getFirstAs(map, Map.class, firstKey, otherKeys); } - private List<CatalogItemDtoAbstract<?,?>> addAbstractCatalogItems(String yaml, Boolean whenAddingAsDtoShouldWeForce) { + private List<CatalogItemDtoAbstract<?,?>> collectCatalogItems(String yaml) { Map<?,?> itemDef = Yamls.getAs(Yamls.parseAll(yaml), Map.class); - Map<?,?> catalogMetadata = getFirstAsMap(itemDef, "brooklyn.catalog", "catalog").orNull(); + Map<?,?> catalogMetadata = getFirstAsMap(itemDef, "brooklyn.catalog").orNull(); if (catalogMetadata==null) log.warn("No `brooklyn.catalog` supplied in catalog request; using legacy mode for "+itemDef); catalogMetadata = MutableMap.copyOf(catalogMetadata); List<CatalogItemDtoAbstract<?, ?>> result = MutableList.of(); - addAbstractCatalogItems(Yamls.getTextOfYamlAtPath(yaml, "brooklyn.catalog").getMatchedYamlTextOrWarn(), - catalogMetadata, result, null, whenAddingAsDtoShouldWeForce); + collectCatalogItems(Yamls.getTextOfYamlAtPath(yaml, "brooklyn.catalog").getMatchedYamlTextOrWarn(), + catalogMetadata, result, null); itemDef.remove("brooklyn.catalog"); catalogMetadata.remove("item"); @@ -559,34 +571,40 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { if (rootItemYaml.startsWith(match)) rootItemYaml = Strings.removeFromStart(rootItemYaml, match); else rootItemYaml = Strings.replaceAllNonRegex(rootItemYaml, "\n"+match, ""); } - addAbstractCatalogItems("item:\n"+makeAsIndentedObject(rootItemYaml), rootItem, result, catalogMetadata, whenAddingAsDtoShouldWeForce); + collectCatalogItems("item:\n"+makeAsIndentedObject(rootItemYaml), rootItem, result, catalogMetadata); } return result; } @SuppressWarnings("unchecked") - private void addAbstractCatalogItems(String sourceYaml, Map<?,?> itemMetadata, List<CatalogItemDtoAbstract<?, ?>> result, Map<?,?> parentMetadata, Boolean whenAddingAsDtoShouldWeForce) { + private void collectCatalogItems(String sourceYaml, Map<?,?> itemMetadata, List<CatalogItemDtoAbstract<?, ?>> result, Map<?,?> parentMetadata) { if (sourceYaml==null) sourceYaml = new Yaml().dump(itemMetadata); Map<Object,Object> catalogMetadata = MutableMap.builder().putAll(parentMetadata).putAll(itemMetadata).build(); // libraries we treat specially, to append the list, with the child's list preferred in classloading order - List<?> librariesL = MutableList.copyOf(getFirstAs(itemMetadata, List.class, "brooklyn.libraries", "libraries").orNull()) + List<?> librariesNew = MutableList.copyOf(getFirstAs(itemMetadata, List.class, "brooklyn.libraries", "libraries").orNull()); + Collection<CatalogBundle> libraryBundlesNew = CatalogItemDtoAbstract.parseLibraries(librariesNew); + + List<?> librariesCombined = MutableList.copyOf(librariesNew) .appendAll(getFirstAs(parentMetadata, List.class, "brooklyn.libraries", "libraries").orNull()); - if (!librariesL.isEmpty()) - catalogMetadata.put("brooklyn.libraries", librariesL); - Collection<CatalogBundle> libraries = CatalogItemDtoAbstract.parseLibraries(librariesL); + if (!librariesCombined.isEmpty()) + catalogMetadata.put("brooklyn.libraries", librariesCombined); + Collection<CatalogBundle> libraryBundles = CatalogItemDtoAbstract.parseLibraries(librariesCombined); + // TODO as this may take a while if downloading, the REST call should be async + CatalogUtils.installLibraries(mgmt, libraryBundlesNew); + Object items = catalogMetadata.remove("items"); Object item = catalogMetadata.remove("item"); if (items!=null) { int count = 0; for (Map<?,?> i: ((List<Map<?,?>>)items)) { - addAbstractCatalogItems(Yamls.getTextOfYamlAtPath(sourceYaml, "items", count).getMatchedYamlTextOrWarn(), - i, result, catalogMetadata, whenAddingAsDtoShouldWeForce); + collectCatalogItems(Yamls.getTextOfYamlAtPath(sourceYaml, "items", count).getMatchedYamlTextOrWarn(), + i, result, catalogMetadata); count++; } } @@ -598,7 +616,18 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { if (itemYaml!=null) sourceYaml = itemYaml; else sourceYaml = new Yaml().dump(item); - CatalogItemType itemType = TypeCoercions.coerce(getFirstAs(catalogMetadata, Object.class, "item.type", "itemType", "item_type").orNull(), CatalogItemType.class); + CatalogItemType itemType = TypeCoercions.coerce(getFirstAs(catalogMetadata, Object.class, "itemType", "item_type").orNull(), CatalogItemType.class); + BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(mgmt, "<load>:0", libraryBundles); + + PlanInterpreterGuessingType planInterpreter = new PlanInterpreterGuessingType(null, item, sourceYaml, itemType, loader, result).reconstruct(); + if (!planInterpreter.isResolved()) { + throw new IllegalStateException("Could not resolve plan: "+sourceYaml); + } + itemType = planInterpreter.getCatalogItemType(); + Map<?, ?> itemAsMap = planInterpreter.getItem(); + // the "plan yaml" includes the services: ... or brooklyn.policies: ... outer key, + // as opposed to the rawer { type: xxx } map without that outer key which is valid as item input + // TODO this plan yaml is needed for subsequent reconstruction; would be nicer if it weren't! String id = getFirstAs(catalogMetadata, String.class, "id").orNull(); String version = getFirstAs(catalogMetadata, String.class, "version").orNull(); @@ -611,164 +640,238 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { Strings.isNonBlank(name) && !name.equals(displayName)) { log.warn("Name property will be ignored due to the existence of displayName and at least one of id, symbolicName"); } - - DeploymentPlan plan = null; - try { - plan = makePlanFromYaml(sourceYaml); - } catch (Exception e) { - Exceptions.propagateIfFatal(e); - if (itemType==CatalogItemType.ENTITY || itemType==CatalogItemType.TEMPLATE) - log.warn("Could not parse item YAML for "+itemType+" (registering anyway): "+e+"\n"+sourceYaml); - } - - if (Strings.isBlank(id)) { - // let ID be inferred, especially from name, to support style where only "name" is specified, with inline version - if (Strings.isNonBlank(symbolicName) && Strings.isNonBlank(version)) { - id = symbolicName + ":" + version; + + // if symname not set, infer from: id, then name, then item id, then item name + if (Strings.isBlank(symbolicName)) { + if (Strings.isNonBlank(id)) { + if (CatalogUtils.looksLikeVersionedId(id)) { + symbolicName = CatalogUtils.getIdFromVersionedId(id); + } else { + symbolicName = id; + } } else if (Strings.isNonBlank(name)) { - id = name; + if (CatalogUtils.looksLikeVersionedId(name)) { + symbolicName = CatalogUtils.getIdFromVersionedId(name); + } else { + symbolicName = name; + } + } else { + symbolicName = setFromItemIfUnset(symbolicName, itemAsMap, "id"); + symbolicName = setFromItemIfUnset(symbolicName, itemAsMap, "name"); + if (Strings.isBlank(symbolicName)) { + log.error("Can't infer catalog item symbolicName from the following plan:\n" + sourceYaml); + throw new IllegalStateException("Can't infer catalog item symbolicName from catalog item metadata"); + } } } - final String catalogSymbolicName; - if (Strings.isNonBlank(symbolicName)) { - catalogSymbolicName = symbolicName; - } else if (Strings.isNonBlank(id)) { - if (Strings.isNonBlank(id) && CatalogUtils.looksLikeVersionedId(id)) { - catalogSymbolicName = CatalogUtils.getIdFromVersionedId(id); - } else { - catalogSymbolicName = id; + // if version not set, infer from: id, then from name, then item version + if (CatalogUtils.looksLikeVersionedId(id)) { + String versionFromId = CatalogUtils.getVersionFromVersionedId(id); + if (versionFromId != null && Strings.isNonBlank(version) && !versionFromId.equals(version)) { + throw new IllegalArgumentException("Discrepency between version set in id " + versionFromId + " and version property " + version); } - } else if (plan!=null && Strings.isNonBlank(plan.getName())) { - catalogSymbolicName = plan.getName(); - } else if (plan!=null && plan.getServices().size()==1) { - Service svc = Iterables.getOnlyElement(plan.getServices()); - if (Strings.isBlank(svc.getServiceType())) { - throw new IllegalStateException("CAMP service type not expected to be missing for " + svc); + version = versionFromId; + } + if (Strings.isBlank(version)) { + if (CatalogUtils.looksLikeVersionedId(name)) { + version = CatalogUtils.getVersionFromVersionedId(name); + } else if (Strings.isBlank(version)) { + version = setFromItemIfUnset(version, itemAsMap, "version"); + if (version==null) { + log.warn("No version specified for catalog item " + symbolicName + ". Using default value."); + version = null; + } } - catalogSymbolicName = svc.getServiceType(); - } else { - log.error("Can't infer catalog item symbolicName from the following plan:\n" + sourceYaml); - throw new IllegalStateException("Can't infer catalog item symbolicName from catalog item metadata"); } - - final String catalogVersion; - if (CatalogUtils.looksLikeVersionedId(id)) { - catalogVersion = CatalogUtils.getVersionFromVersionedId(id); - if (version != null && !catalogVersion.equals(version)) { - throw new IllegalArgumentException("Discrepency between version set in id " + catalogVersion + " and version property " + version); + + // if not set, ID can come from symname:version, failing that, from the plan.id, failing that from the sym name + if (Strings.isBlank(id)) { + // let ID be inferred, especially from name, to support style where only "name" is specified, with inline version + if (Strings.isNonBlank(symbolicName) && Strings.isNonBlank(version)) { + id = symbolicName + ":" + version; + } + id = setFromItemIfUnset(id, itemAsMap, "id"); + if (Strings.isBlank(id)) { + if (Strings.isNonBlank(symbolicName)) { + id = symbolicName; + } else { + log.error("Can't infer catalog item id from the following plan:\n" + sourceYaml); + throw new IllegalStateException("Can't infer catalog item id from catalog item metadata"); + } } - } else if (Strings.isNonBlank(version)) { - catalogVersion = version; - } else { - log.warn("No version specified for catalog item " + catalogSymbolicName + ". Using default value."); - catalogVersion = null; } - final String catalogDisplayName; - if (Strings.isNonBlank(displayName)) { - catalogDisplayName = displayName; - } else if (Strings.isNonBlank(name)) { - catalogDisplayName = name; - } else if (Strings.isNonBlank(plan.getName())) { - catalogDisplayName = plan.getName(); - } else { - catalogDisplayName = null; + if (Strings.isBlank(displayName)) { + if (Strings.isNonBlank(name)) displayName = name; + displayName = setFromItemIfUnset(displayName, itemAsMap, "name"); } - final String description = getFirstAs(catalogMetadata, String.class, "description").orNull(); - final String catalogDescription; - if (Strings.isNonBlank(description)) { - catalogDescription = description; - } else if (Strings.isNonBlank(plan.getDescription())) { - catalogDescription = plan.getDescription(); - } else { - catalogDescription = null; - } + String description = getFirstAs(catalogMetadata, String.class, "description").orNull(); + description = setFromItemIfUnset(description, itemAsMap, "description"); - final String catalogIconUrl = getFirstAs(catalogMetadata, String.class, "icon.url", "iconUrl", "icon_url").orNull(); + // icon.url is discouraged, but kept for legacy compatibility; should deprecate this + final String catalogIconUrl = getFirstAs(catalogMetadata, String.class, "iconUrl", "icon_url", "icon.url").orNull(); final String deprecated = getFirstAs(catalogMetadata, String.class, "deprecated").orNull(); final Boolean catalogDeprecated = Boolean.valueOf(deprecated); - CatalogUtils.installLibraries(mgmt, libraries); + // run again now that we know the ID + planInterpreter = new PlanInterpreterGuessingType(id, item, sourceYaml, itemType, loader, result).reconstruct(); + if (!planInterpreter.isResolved()) { + throw new IllegalStateException("Could not resolve plan once id and itemType are known (recursive reference?): "+sourceYaml); + } + String sourcePlanYaml = planInterpreter.getPlanYaml(); + + CatalogItemDtoAbstract<?, ?> dto = createItemBuilder(itemType, symbolicName, version) + .libraries(libraryBundles) + .displayName(displayName) + .description(description) + .deprecated(catalogDeprecated) + .iconUrl(catalogIconUrl) + .plan(sourcePlanYaml) + .build(); + + dto.setManagementContext((ManagementContextInternal) mgmt); + result.add(dto); + } + + private String setFromItemIfUnset(String oldValue, Map<?,?> item, String fieldAttr) { + if (Strings.isNonBlank(oldValue)) return oldValue; + if (item!=null) { + Object newValue = item.get(fieldAttr); + if (newValue instanceof String && Strings.isNonBlank((String)newValue)) + return (String)newValue; + } + return oldValue; + } + + + private class PlanInterpreterGuessingType { - String versionedId = CatalogUtils.getVersionedId(catalogSymbolicName, catalogVersion!=null ? catalogVersion : NO_VERSION); - BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(mgmt, versionedId, libraries); + final String id; + final Map<?,?> item; + final String itemYaml; + final BrooklynClassLoadingContext loader; + final List<CatalogItemDtoAbstract<?, ?>> itemsDefinedSoFar; - CatalogItemType inferredItemType = inferType(plan); - boolean usePlan; - if (inferredItemType!=null) { - if (itemType!=null) { - if (itemType==CatalogItemType.TEMPLATE && inferredItemType==CatalogItemType.ENTITY) { - // template - we use the plan, but coupled with the type to make a catalog template item - usePlan = true; - } else if (itemType==inferredItemType) { - // if the plan showed the type, it is either a parsed entity or a legacy spec type - usePlan = true; - if (itemType==CatalogItemType.TEMPLATE || itemType==CatalogItemType.ENTITY) { - // normal - } else { - log.warn("Legacy syntax used for "+itemType+" "+catalogSymbolicName+": should declare item.type and specify type"); - } - } else { - throw new IllegalStateException("Explicit type "+itemType+" "+catalogSymbolicName+" does not match blueprint inferred type "+inferredItemType); - } + CatalogItemType catalogItemType; + String planYaml; + @SuppressWarnings("unused") + DeploymentPlan plan; + AbstractBrooklynObjectSpec<?,?> spec; + boolean resolved = false; + + public PlanInterpreterGuessingType(@Nullable String id, Object item, String itemYaml, @Nullable CatalogItemType optionalCiType, + BrooklynClassLoadingContext loader, List<CatalogItemDtoAbstract<?,?>> itemsDefinedSoFar) { + // ID is useful to prevent recursive references (currently for entities only) + this.id = id; + + if (item instanceof String) { + // if just a string supplied, wrap as map + this.item = MutableMap.of("type", item); + this.itemYaml = "type:\n"+makeAsIndentedObject(itemYaml); } else { - // no type was declared, use the inferred type from the plan - itemType = inferredItemType; - usePlan = true; + this.item = (Map<?,?>)item; + this.itemYaml = itemYaml; } - } else if (itemType!=null) { - if (itemType==CatalogItemType.TEMPLATE || itemType==CatalogItemType.ENTITY) { - log.warn("Incomplete plan detected for "+itemType+" "+catalogSymbolicName+"; will likely fail subsequently"); - usePlan = true; - } else { - usePlan = false; + this.catalogItemType = optionalCiType; + this.loader = loader; + this.itemsDefinedSoFar = itemsDefinedSoFar; + } + + public PlanInterpreterGuessingType reconstruct() { + attemptType(null, CatalogItemType.ENTITY); + attemptType(null, CatalogItemType.TEMPLATE); + + attemptType("services", CatalogItemType.ENTITY); + attemptType(POLICIES_KEY, CatalogItemType.POLICY); + attemptType(LOCATIONS_KEY, CatalogItemType.LOCATION); + + if (!resolved && catalogItemType==CatalogItemType.TEMPLATE) { + // anything goes, for an explicit template, because we can't easily recurse into the types + planYaml = itemYaml; + resolved = true; } - } else { - throw new IllegalStateException("Unable to detect type for "+catalogSymbolicName+"; error in catalog metadata or blueprint"); + + return this; } - AbstractBrooklynObjectSpec<?, ?> spec; - if (usePlan) { - spec = createSpecFromPlan(catalogSymbolicName, plan, loader); - } else { - String key; - switch (itemType) { - // we don't actually need the spec here, since the yaml is what is used at load time, - // but it isn't a bad idea to confirm that it resolves - case POLICY: - spec = createPolicySpec(loader, item); - key = POLICIES_KEY; - break; - case LOCATION: - spec = createLocationSpec(loader, item); - key = LOCATIONS_KEY; - break; - default: throw new IllegalStateException("Cannot create "+itemType+" "+catalogSymbolicName+"; invalid metadata or blueprint"); + public boolean isResolved() { return resolved; } + + public CatalogItemType getCatalogItemType() { + return catalogItemType; + } + + public String getPlanYaml() { + return planYaml; + } + + private boolean attemptType(String key, CatalogItemType candidateCiType) { + if (resolved) return false; + if (catalogItemType!=null && catalogItemType!=candidateCiType) return false; + + final String candidateYaml; + if (key==null) candidateYaml = itemYaml; + else { + if (item.containsKey(key)) + candidateYaml = itemYaml; + else + candidateYaml = key + ":\n" + makeAsIndentedList(itemYaml); + } + // first look in collected items, if a key is given + String type = (String) item.get("type"); + if (type!=null && key!=null) { + for (CatalogItemDtoAbstract<?,?> candidate: itemsDefinedSoFar) { + if (type.equals(candidate.getSymbolicName()) || type.equals(candidate.getId())) { + // matched - exit + catalogItemType = candidateCiType; + planYaml = candidateYaml; + resolved = true; + return true; + } + } + } + + // then try parsing plan - this will use loader + try { + DeploymentPlan candidatePlan = makePlanFromYaml(candidateYaml); + spec = createSpec(id, candidateCiType, candidatePlan, loader); + if (spec!=null) { + catalogItemType = candidateCiType; + plan = candidatePlan; + planYaml = candidateYaml; + resolved = true; + } + return true; + } catch (Exception e) { + Exceptions.propagateIfFatal(e); } - // TODO currently we then convert yaml to legacy brooklyn.{location,policy} syntax for subsequent usage; - // would be better to (in the other path) convert to {type: ..., brooklyn.config: ... } format and expect that elsewhere - sourceYaml = key + ":\n" + makeAsIndentedList(sourceYaml); + + // finally try parsing a cut-down plan, in case there is a nested reference to a newly defined catalog item + if (type!=null && key!=null) { + try { + String cutDownYaml = key + ":\n" + makeAsIndentedList("type: "+type); + DeploymentPlan candidatePlan = makePlanFromYaml(cutDownYaml); + Object cutdownSpec = createSpec(id, candidateCiType, candidatePlan, loader); + if (cutdownSpec!=null) { + catalogItemType = candidateCiType; + planYaml = candidateYaml; + resolved = true; + } + return true; + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + } + } + + return false; } - - CatalogItemDtoAbstract<?, ?> dto = createItemBuilder(itemType, spec, catalogSymbolicName, catalogVersion) - .libraries(libraries) - .displayName(catalogDisplayName) - .description(catalogDescription) - .deprecated(catalogDeprecated) - .iconUrl(catalogIconUrl) - .plan(sourceYaml) - .build(); - - dto.setManagementContext((ManagementContextInternal) mgmt); - if (whenAddingAsDtoShouldWeForce!=null) { - addItemDto(dto, whenAddingAsDtoShouldWeForce); + public Map<?,?> getItem() { + return item; } - result.add(dto); } - + private String makeAsIndentedList(String yaml) { String[] lines = yaml.split("\n"); lines[0] = "- "+lines[0]; @@ -784,74 +887,41 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { return Strings.join(lines, "\n"); } - private CatalogItemType inferType(DeploymentPlan plan) { - if (plan==null) return null; - if (isEntityPlan(plan)) return CatalogItemType.ENTITY; - if (isPolicyPlan(plan)) return CatalogItemType.POLICY; - if (isLocationPlan(plan)) return CatalogItemType.LOCATION; - return null; - } - - private AbstractBrooklynObjectSpec<?,?> createSpecFromPlan(String symbolicName, DeploymentPlan plan, BrooklynClassLoadingContext loader) { - if (isPolicyPlan(plan)) { - return createPolicySpec(plan, loader); - } else if (isLocationPlan(plan)) { - return createLocationSpec(plan, loader); - } else { - return createEntitySpec(symbolicName, plan, loader); - } - } - - private CatalogItemBuilder<?> createItemBuilder(CatalogItemType itemType, AbstractBrooklynObjectSpec<?, ?> spec, String itemId, String version) { - if (itemType!=null) { - switch (itemType) { - case ENTITY: return CatalogItemBuilder.newEntity(itemId, version); - case TEMPLATE: return CatalogItemBuilder.newTemplate(itemId, version); - case POLICY: return CatalogItemBuilder.newPolicy(itemId, version); - case LOCATION: return CatalogItemBuilder.newLocation(itemId, version); - } - log.warn("Legacy syntax for "+itemId+"; unexpected item.type "+itemType); - } else { - log.warn("Legacy syntax for "+itemId+"; no item.type declared or inferred"); + private CatalogItemBuilder<?> createItemBuilder(CatalogItemType itemType, String itemId, String version) { + Preconditions.checkNotNull(itemType, "itemType required"); + switch (itemType) { + case ENTITY: return CatalogItemBuilder.newEntity(itemId, version); + case TEMPLATE: return CatalogItemBuilder.newTemplate(itemId, version); + case POLICY: return CatalogItemBuilder.newPolicy(itemId, version); + case LOCATION: return CatalogItemBuilder.newLocation(itemId, version); } - - // @deprecated - should not come here - if (spec instanceof EntitySpec) { - if (isApplicationSpec((EntitySpec<?>)spec)) { - return CatalogItemBuilder.newTemplate(itemId, version); - } else { - return CatalogItemBuilder.newEntity(itemId, version); - } - } else if (spec instanceof PolicySpec) { - return CatalogItemBuilder.newPolicy(itemId, version); - } else if (spec instanceof LocationSpec) { - return CatalogItemBuilder.newLocation(itemId, version); - } else { - throw new IllegalStateException("Unknown spec type " + spec.getClass().getName() + " (" + spec + ")"); - } - } - - private boolean isApplicationSpec(EntitySpec<?> spec) { - return !Boolean.TRUE.equals(spec.getConfig().get(EntityManagementUtils.WRAPPER_APP_MARKER)); + throw new IllegalStateException("Unexpected itemType: "+itemType); } - private boolean isEntityPlan(DeploymentPlan plan) { - return plan!=null && !plan.getServices().isEmpty() || !plan.getArtifacts().isEmpty(); - } - - private boolean isPolicyPlan(DeploymentPlan plan) { - return !isEntityPlan(plan) && plan.getCustomAttributes().containsKey(POLICIES_KEY); - } - - private boolean isLocationPlan(DeploymentPlan plan) { - return !isEntityPlan(plan) && plan.getCustomAttributes().containsKey(LOCATIONS_KEY); - } + // these kept as their logic may prove useful; Apr 2015 +// private boolean isApplicationSpec(EntitySpec<?> spec) { +// return !Boolean.TRUE.equals(spec.getConfig().get(EntityManagementUtils.WRAPPER_APP_MARKER)); +// } +// +// private boolean isEntityPlan(DeploymentPlan plan) { +// return plan!=null && !plan.getServices().isEmpty() || !plan.getArtifacts().isEmpty(); +// } +// +// private boolean isPolicyPlan(DeploymentPlan plan) { +// return !isEntityPlan(plan) && plan.getCustomAttributes().containsKey(POLICIES_KEY); +// } +// +// private boolean isLocationPlan(DeploymentPlan plan) { +// return !isEntityPlan(plan) && plan.getCustomAttributes().containsKey(LOCATIONS_KEY); +// } private DeploymentPlan makePlanFromYaml(String yaml) { CampPlatform camp = BrooklynServerConfig.getCampPlatform(mgmt).get(); return camp.pdp().parseDeploymentPlan(Streams.newReaderWithContents(yaml)); } + //------------------------ + @Override public CatalogItem<?,?> addItem(String yaml) { return addItem(yaml, false); @@ -871,11 +941,11 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { public List<? extends CatalogItem<?,?>> addItems(String yaml, boolean forceUpdate) { log.debug("Adding manual catalog item to "+mgmt+": "+yaml); checkNotNull(yaml, "yaml"); - List<CatalogItemDtoAbstract<?, ?>> result = addAbstractCatalogItems(yaml, forceUpdate); - // previously we did this here, but now we do it on each item, in case #2 refers to #1 -// for (CatalogItemDtoAbstract<?, ?> item: result) { -// addItemDto(item, forceUpdate); -// } + List<CatalogItemDtoAbstract<?, ?>> result = collectCatalogItems(yaml); + // do this at the end for atomic updates; if there are intra-yaml references, we handle them specially + for (CatalogItemDtoAbstract<?, ?> item: result) { + addItemDto(item, forceUpdate); + } return result; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/core/src/main/java/brooklyn/location/geo/UtraceHostGeoLookup.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/geo/UtraceHostGeoLookup.java b/core/src/main/java/brooklyn/location/geo/UtraceHostGeoLookup.java index 37ed9f7..10b21df 100644 --- a/core/src/main/java/brooklyn/location/geo/UtraceHostGeoLookup.java +++ b/core/src/main/java/brooklyn/location/geo/UtraceHostGeoLookup.java @@ -34,6 +34,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import brooklyn.util.exceptions.Exceptions; +import brooklyn.util.javalang.JavaClassNames; import brooklyn.util.net.Networking; import brooklyn.util.time.Duration; import brooklyn.util.time.Durations; @@ -129,7 +130,8 @@ Beyond this you get blacklisted and requests may time out, or return none. try { result.set(retrieveHostGeoInfo(address)); } catch (Exception e) { - throw Exceptions.propagate(e); + log.warn("Error computing geo info for "+address+"; internet issues or too many requests to (free) servers for "+JavaClassNames.simpleClassName(UtraceHostGeoLookup.this)+": "+e); + log.debug("Detail of host geo error: "+e, e); } } }; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java b/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java index bbd589d..1748de8 100644 --- a/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java +++ b/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java @@ -40,6 +40,7 @@ import brooklyn.config.ConfigMap; import brooklyn.enricher.basic.AbstractEnricher; import brooklyn.entity.Entity; import brooklyn.entity.Group; +import brooklyn.entity.basic.ConfigKeys; import brooklyn.entity.basic.Entities; import brooklyn.entity.basic.EntityInternal; import brooklyn.entity.basic.EntityLocal; @@ -58,6 +59,7 @@ import brooklyn.util.flags.FlagUtils; import brooklyn.util.flags.SetFromFlag; import brooklyn.util.flags.TypeCoercions; import brooklyn.util.guava.Maybe; +import brooklyn.util.text.Strings; import com.google.common.annotations.Beta; import com.google.common.base.Objects; @@ -168,6 +170,17 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple Preconditions.checkArgument(flags.get("displayName") instanceof CharSequence, "'displayName' property should be a string"); setDisplayName(flags.remove("displayName").toString()); } + + // set leftover flags should as config items; particularly useful when these have come from a brooklyn.config map + for (Object flag: flags.keySet()) { + ConfigKey<Object> key = ConfigKeys.newConfigKey(Object.class, Strings.toString(flag)); + if (config().getRaw(key).isPresent()) { + log.warn("Config '"+flag+"' on "+this+" conflicts with key already set; ignoring"); + } else { + config().set(key, flags.get(flag)); + } + } + return this; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/docs/guide/ops/catalog/index.md ---------------------------------------------------------------------- diff --git a/docs/guide/ops/catalog/index.md b/docs/guide/ops/catalog/index.md index 401979c..a2e4408 100644 --- a/docs/guide/ops/catalog/index.md +++ b/docs/guide/ops/catalog/index.md @@ -77,8 +77,9 @@ services: In addition to the above fields, exactly **one** of the following is also required: -- `item`: the blueprint (in the usual YAML format) for an entity or application template, - or a map containing `type` and optional `brooklyn.config` for policies and locations; **or** +- `item`: the YAML for a service or policy or location specification + (a map containing `type` and optional `brooklyn.config`) + or a full application blueprint (in the usual YAML format) for a template; **or* - `items`: a list of catalog items, where each entry in the map follows the same schema as the `brooklyn.catalog` value, and the keys in these map override any metadata specified as a sibling of this `items` key (or, in the case of `libraries` they add to the list); @@ -87,17 +88,17 @@ In addition to the above fields, exactly **one** of the following is also requir The following optional catalog metadata is supported: -- `item.type`: the type of the item being defined. - If omitted, all `item` definitions are taken to be entities; - for any type other than an entity, this field must be supplied. - The supported types are: +- `itemType`: the type of the item being defined. + When adding a template (see below) this must be set. + In most other cases this can be omitted and type type will be inferred. + The supported item types are: - `entity` - `template` - `policy` - `location` - `name`: a nicely formatted display name for the item, used when presenting it in a GUI - `description`: supplies an extended textual description for the item -- `icon.url`: points to an icon for the item, used when presenting it in a GUI. +- `iconUrl`: points to an icon for the item, used when presenting it in a GUI. The URL prefix `classpath` is supported but these URLs may *not* refer items in any OSGi bundle in the `libraries` section (to prevent requiring all OSGi bundles to be loaded at launch). Icons are instead typically installed either at the server from which the OSGi bundles or catalog items are supplied @@ -131,14 +132,13 @@ and its implementation will be the Java class `brooklyn.entity.nosql.riak.RiakNo brooklyn.catalog: id: datastore version: 1.0 - item.type: template - icon.url: classpath://brooklyn/entity/nosql/riak/riak.png + itemType: template + iconUrl: classpath://brooklyn/entity/nosql/riak/riak.png name: Datastore (Riak) description: Riak is an open-source NoSQL key-value data store. item: - services: - - type: brooklyn.entity.nosql.riak.RiakNode - name: Riak Node + type: brooklyn.entity.nosql.riak.RiakNode + name: Riak Node ``` @@ -149,22 +149,20 @@ This YAML will install three items: ```yaml brooklyn.catalog: version: 1.1 - icon.url: classpath://brooklyn/entity/nosql/riak/riak.png + iconUrl: classpath://brooklyn/entity/nosql/riak/riak.png description: Riak is an open-source NoSQL key-value data store. items: - id: riak-node item: - services: - - type: brooklyn.entity.nosql.riak.RiakNode - name: Riak Node + type: brooklyn.entity.nosql.riak.RiakNode + name: Riak Node - id: riak-cluster item: - services: - - type: brooklyn.entity.nosql.riak.RiakCluster - name: Riak Cluster + type: brooklyn.entity.nosql.riak.RiakCluster + name: Riak Cluster - id: datastore name: Datastore (Riak Cluster) - item.type: template + itemType: template item: services: - type: riak-cluster http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java index 2cde40e..c5f3dcb 100644 --- a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java +++ b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynYamlTypeInstantiator.java @@ -197,10 +197,9 @@ public abstract class BrooklynYamlTypeInstantiator { public <T> Class<? extends T> getType(@Nonnull Class<T> type) { try { return getClassLoadingContext().loadClass(getTypeName().get(), type); -// return loadClass(type, getTypeName().get(), factory.mgmt, factory.contextForLogging); } catch (Exception e) { Exceptions.propagateIfFatal(e); - log.warn("Unable to resolve " + type + " " + getTypeName().get() + " (rethrowing) in spec " + factory.contextForLogging); + log.debug("Unable to resolve " + type + " " + getTypeName().get() + " (rethrowing) in spec " + factory.contextForLogging); throw Exceptions.propagate(e); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java index b1e6817..41489ba 100644 --- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java @@ -104,7 +104,7 @@ public abstract class AbstractYamlTest { } protected Entity createAndStartApplication(String... multiLineYaml) throws Exception { - return createAndStartApplication(join(multiLineYaml)); + return createAndStartApplication(joinLines(multiLineYaml)); } protected Entity createAndStartApplication(String input) throws Exception { @@ -145,11 +145,11 @@ public abstract class AbstractYamlTest { } protected void addCatalogItem(Iterable<String> catalogYaml) { - addCatalogItem(join(catalogYaml)); + addCatalogItem(joinLines(catalogYaml)); } protected void addCatalogItem(String... catalogYaml) { - addCatalogItem(join(catalogYaml)); + addCatalogItem(joinLines(catalogYaml)); } protected void addCatalogItem(String catalogYaml) { @@ -164,11 +164,11 @@ public abstract class AbstractYamlTest { return LOG; } - private String join(Iterable<String> catalogYaml) { + private String joinLines(Iterable<String> catalogYaml) { return Joiner.on("\n").join(catalogYaml); } - private String join(String[] catalogYaml) { + private String joinLines(String[] catalogYaml) { return Joiner.on("\n").join(catalogYaml); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java new file mode 100644 index 0000000..cbb1d1e --- /dev/null +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package io.brooklyn.camp.brooklyn.catalog; + +import io.brooklyn.camp.brooklyn.AbstractYamlTest; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.Assert; +import org.testng.annotations.Test; + +import brooklyn.catalog.CatalogItem; +import brooklyn.entity.Entity; +import brooklyn.entity.basic.BasicEntity; +import brooklyn.entity.basic.BasicStartable; +import brooklyn.entity.basic.ConfigKeys; +import brooklyn.entity.basic.Entities; +import brooklyn.policy.Policy; +import brooklyn.policy.ha.ServiceRestarter; +import brooklyn.util.exceptions.Exceptions; + +import com.google.common.collect.Iterables; + + +public class CatalogYamlCombiTest extends AbstractYamlTest { + + private static final Logger log = LoggerFactory.getLogger(CatalogYamlCombiTest.class); + + @Test + public void testBRefEntityA() throws Exception { + addCatalogItem( + "brooklyn.catalog:", + " version: "+TEST_VERSION, + " items:", + " - item:", + " id: A", + " type: "+BasicEntity.class.getName(), + " brooklyn.config: { a: 1, b: 0 }", + " - item:", + " id: B", + " type: A", + " brooklyn.config: { b: 1 }"); + + CatalogItem<?, ?> item = mgmt().getCatalog().getCatalogItem("B", TEST_VERSION); + Assert.assertNotNull(item); + + Entity a = launchEntity("A"); + Assert.assertTrue(BasicEntity.class.isInstance(a), "Wrong type: "+a); + Assert.assertEquals(a.config().get(ConfigKeys.newIntegerConfigKey("a")), (Integer)1); + Assert.assertEquals(a.config().get(ConfigKeys.newIntegerConfigKey("b")), (Integer)0); + + Entity b = launchEntity("B"); + Assert.assertTrue(BasicEntity.class.isInstance(b), "Wrong type: "+b); + Assert.assertEquals(b.config().get(ConfigKeys.newIntegerConfigKey("a")), (Integer)1); + Assert.assertEquals(b.config().get(ConfigKeys.newIntegerConfigKey("b")), (Integer)1); + + deleteCatalogEntity("A"); + + // now loading B makes an error + try { + launchEntity("B"); + Assert.fail("B should not be launchable"); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + log.info("Got expected error: "+e); + } + + deleteCatalogEntity("B"); + } + + @Test + public void testBRefPolicyALocationZ() throws Exception { + addCatalogItem( + "brooklyn.catalog:", + " version: "+TEST_VERSION, + " id: Z", + " items:", + " - item: ", + " type: localhost", + " brooklyn.config: { z: 9 }"); + addCatalogItem( + "brooklyn.catalog:", + " version: "+TEST_VERSION, + " items:", + " - item_type: policy", + " item:", + " id: A", + " type: "+ServiceRestarter.class.getName(), + " brooklyn.config: { a: 99 }", + " - item:", + " id: B", + " type: "+BasicStartable.class.getName(), + " location: Z", + " brooklyn.policies:", + " - type: A"); + + CatalogItem<?, ?> item = mgmt().getCatalog().getCatalogItem("A", TEST_VERSION); + Assert.assertNotNull(item); + + Entity b = launchEntity("B", false); + Assert.assertTrue(BasicStartable.class.isInstance(b), "Wrong type: "+b); + Entities.dumpInfo(b); + + Assert.assertEquals(Iterables.getOnlyElement(b.getLocations()).getConfig(ConfigKeys.newIntegerConfigKey("z")), (Integer)9); + + Policy p = Iterables.getOnlyElement(b.getPolicies()); + Assert.assertTrue(ServiceRestarter.class.isInstance(p), "Wrong type: "+p); + Assert.assertEquals(p.getConfig(ConfigKeys.newIntegerConfigKey("a")), (Integer)99); + + deleteCatalogEntity("A"); + deleteCatalogEntity("B"); + deleteCatalogEntity("Z"); + } + + private Entity launchEntity(String symbolicName) throws Exception { + return launchEntity(symbolicName, true); + } + + private Entity launchEntity(String symbolicName, boolean includeLocation) throws Exception { + String yaml = "name: simple-app-yaml\n" + + (includeLocation ? "location: localhost\n" : "") + + "services: \n" + + " - type: "+ver(symbolicName); + Entity app = createAndStartApplication(yaml); + return Iterables.getOnlyElement(app.getChildren()); + } + + +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/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 1737718..e5eae35 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 @@ -49,6 +49,21 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { private static final String SIMPLE_ENTITY_TYPE = OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_SIMPLE_ENTITY; @Test + public void testAddCatalogItemVerySimple() throws Exception { + String symbolicName = "my.catalog.app.id.load"; + addCatalogItem( + "brooklyn.catalog:", + " id: " + symbolicName, + " version: " + TEST_VERSION, + " item:", + " type: "+ BasicEntity.class.getName()); + + CatalogItem<?, ?> item = mgmt().getCatalog().getCatalogItem(symbolicName, TEST_VERSION); + assertTrue(item.getPlanYaml().indexOf("services:")>=0, "expected 'services:' block: "+item+"\n"+item.getPlanYaml()); + + deleteCatalogEntity(symbolicName); + } + @Test public void testAddCatalogItem() throws Exception { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); @@ -61,7 +76,52 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { } @Test - public void testAddCatalogItemAsSiblingOfCatalogMetadata() throws Exception { + public void testAddCatalogItemTypeAsString() throws Exception { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); + + String symbolicName = "my.catalog.app.id.load"; + addCatalogItem( + "brooklyn.catalog:", + " id: " + symbolicName, + " name: My Catalog App", + " description: My description", + " icon_url: classpath://path/to/myicon.jpg", + " version: " + TEST_VERSION, + " libraries:", + " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, + " item: " + SIMPLE_ENTITY_TYPE); + + CatalogItem<?, ?> item = mgmt().getCatalog().getCatalogItem(symbolicName, TEST_VERSION); + assertEquals(item.getSymbolicName(), symbolicName); + + deleteCatalogEntity(symbolicName); + } + + @Test + public void testAddCatalogItemTypeExplicitTypeAsString() throws Exception { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); + + String symbolicName = "my.catalog.app.id.load"; + addCatalogItem( + "brooklyn.catalog:", + " id: " + symbolicName, + " name: My Catalog App", + " description: My description", + " icon_url: classpath://path/to/myicon.jpg", + " version: " + TEST_VERSION, + " item_type: entity", + " libraries:", + " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, + " item: " + SIMPLE_ENTITY_TYPE); + + CatalogItem<?, ?> item = mgmt().getCatalog().getCatalogItem(symbolicName, TEST_VERSION); + assertEquals(item.getSymbolicName(), symbolicName); + + deleteCatalogEntity(symbolicName); + } + + @Test + public void testAddCatalogItemTopLevelSyntax() throws Exception { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); String symbolicName = "my.catalog.app.id.load"; @@ -94,8 +154,8 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { " name: " + id, " libraries:", " - " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, - "services:", - "- type: " + SIMPLE_ENTITY_TYPE); + " item:", + " type: "+ SIMPLE_ENTITY_TYPE); CatalogItem<?, ?> catalogItem = mgmt().getCatalog().getCatalogItem(id, BrooklynCatalog.DEFAULT_VERSION); assertEquals(catalogItem.getVersion(), "0.0.0.SNAPSHOT"); mgmt().getCatalog().deleteCatalogItem(id, "0.0.0.SNAPSHOT"); @@ -151,6 +211,9 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { String referrerSymbolicName = "my.catalog.app.id.referring"; addCatalogOSGiEntities(referencedSymbolicName, SIMPLE_ENTITY_TYPE, referrerSymbolicName, ver(referencedSymbolicName)); + CatalogItem<?, ?> referrer = mgmt().getCatalog().getCatalogItem(referrerSymbolicName, TEST_VERSION); + Assert.assertTrue(referrer.getPlanYaml().indexOf("services")>=0, "expected services in: "+referrer.getPlanYaml()); + String yaml = "name: simple-app-yaml\n" + "location: localhost\n" + "services: \n" + @@ -199,7 +262,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { "name: simple-app-yaml", "location: localhost", "services:", - "- serviceType: "+BasicEntity.class.getName(), + "- type: "+BasicEntity.class.getName(), " brooklyn.children:", " - type: " + ver(referrerSymbolicName)); @@ -221,6 +284,40 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { } @Test + public void testLaunchApplicationChildWithCatalogReferencingOtherCatalogServicesBlock() throws Exception { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); + + String referencedSymbolicName = "my.catalog.app.id.child.referenced"; + String referrerSymbolicName = "my.catalog.app.id.child.referring"; + addCatalogOSGiEntity(referencedSymbolicName, SIMPLE_ENTITY_TYPE); + addCatalogChildOSGiEntityWithServicesBlock(referrerSymbolicName, ver(referencedSymbolicName)); + + Entity app = createAndStartApplication( + "name: simple-app-yaml", + "location: localhost", + "services:", + "- serviceType: "+BasicEntity.class.getName(), + " brooklyn.children:", + " - type: " + ver(referrerSymbolicName)); + + Collection<Entity> children = app.getChildren(); + assertEquals(children.size(), 1); + Entity child = Iterables.getOnlyElement(children); + assertEquals(child.getEntityType().getName(), BasicEntity.class.getName()); + Collection<Entity> grandChildren = child.getChildren(); + assertEquals(grandChildren.size(), 1); + Entity grandChild = Iterables.getOnlyElement(grandChildren); + assertEquals(grandChild.getEntityType().getName(), BasicEntity.class.getName()); + Collection<Entity> grandGrandChildren = grandChild.getChildren(); + assertEquals(grandGrandChildren.size(), 1); + Entity grandGrandChild = Iterables.getOnlyElement(grandGrandChildren); + assertEquals(grandGrandChild.getEntityType().getName(), SIMPLE_ENTITY_TYPE); + + deleteCatalogEntity(referencedSymbolicName); + deleteCatalogEntity(referrerSymbolicName); + } + + @Test public void testLaunchApplicationWithTypeUsingJavaColonPrefix() throws Exception { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); @@ -244,10 +341,11 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { String referrerSymbolicName = "my.catalog.app.id.child.referring"; try { - addCatalogChildOSGiEntity(referrerSymbolicName, ver(referrerSymbolicName)); - fail("Expected to throw IllegalStateException"); + // TODO only fails if using 'services', because that forces plan parsing; should fail in all cases + addCatalogChildOSGiEntityWithServicesBlock(referrerSymbolicName, ver(referrerSymbolicName)); + fail("Expected to throw"); } catch (IllegalStateException e) { - assertTrue(e.getMessage().contains("Could not find "+referrerSymbolicName)); + assertTrue(e.getMessage().contains(referrerSymbolicName), "message was: "+e); } } @@ -454,7 +552,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { "- type: " + symbolicName); fail("Catalog addition expected to fail due to non-existent java type " + symbolicName); } catch (IllegalStateException e) { - assertEquals(e.getMessage(), "Recursive reference to " + symbolicName + " (and cannot be resolved as a Java type)"); + assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e); } } @@ -480,7 +578,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { "- type: " + versionedId); fail("Catalog addition expected to fail due to non-existent java type " + versionedId); } catch (IllegalStateException e) { - assertEquals(e.getMessage(), "Recursive reference to " + versionedId + " (and cannot be resolved as a Java type)"); + assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e); } } @@ -497,7 +595,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { "- type: " + SIMPLE_ENTITY_TYPE); fail("Catalog addition expected to fail due to non-existent java type " + SIMPLE_ENTITY_TYPE); } catch (IllegalStateException e) { - assertEquals(e.getMessage(), "Recursive reference to " + SIMPLE_ENTITY_TYPE + " (and cannot be resolved as a Java type)"); + assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e); } } @@ -530,8 +628,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { " libraries:", " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, " item:", - " services:", - " - type: " + serviceType); + " type: " + serviceType); } private void addCatalogOSGiEntities(String ...namesAndTypes) { @@ -549,13 +646,12 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { lines.addAll(MutableList.of( " - id: " + namesAndTypes[i], " item:", - " services:", - " - type: " + namesAndTypes[i+1])); + " type: " + namesAndTypes[i+1])); } addCatalogItem(lines); } - private void addCatalogChildOSGiEntity(String symbolicName, String serviceType) { + private void addCatalogChildOSGiEntityWithServicesBlock(String symbolicName, String serviceType) { addCatalogItem( "brooklyn.catalog:", " id: " + symbolicName, @@ -571,5 +667,20 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { " brooklyn.children:", " - type: " + serviceType); } + private void addCatalogChildOSGiEntity(String symbolicName, String serviceType) { + addCatalogItem( + "brooklyn.catalog:", + " id: " + symbolicName, + " name: My Catalog App", + " description: My description", + " icon_url: classpath://path/to/myicon.jpg", + " version: " + TEST_VERSION, + " libraries:", + " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, + " item:", + " type: " + BasicEntity.class.getName(), + " brooklyn.children:", + " - type: " + serviceType); + } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java index b7370be..d38fee0 100644 --- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java @@ -139,7 +139,7 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest { String yaml = "name: simple-app-yaml\n" + "location: localhost\n" + "services: \n" + - " - serviceType: "+ ver(referrerSymbolicName); + "- type: "+ ver(referrerSymbolicName); Entity app = createAndStartApplication(yaml); @@ -150,7 +150,7 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest { deleteCatalogEntity(referencedSymbolicName); } - private void addCatalogOsgiPolicy(String symbolicName, String serviceType) { + private void addCatalogOsgiPolicy(String symbolicName, String policyType) { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); addCatalogItem( @@ -162,15 +162,14 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest { " version: " + TEST_VERSION, " libraries:", " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, - " item_type: policy", " item:", - " type: " + serviceType, + " type: " + policyType, " brooklyn.config:", " config1: config1", " config2: config2"); } - private void addCatalogOsgiPolicyTopLevelSyntax(String symbolicName, String serviceType) { + private void addCatalogOsgiPolicyTopLevelSyntax(String symbolicName, String policyType) { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); addCatalogItem( @@ -184,7 +183,7 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest { " - url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL, "", "brooklyn.policies:", - "- type: " + serviceType, + "- type: " + policyType, " brooklyn.config:", " config1: config1", " config2: config2"); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java index e473d2d..8fbcf65 100644 --- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java @@ -39,6 +39,31 @@ public class CatalogYamlTemplateTest extends AbstractYamlTest { @Test public void testAddCatalogItem() throws Exception { + CatalogItem<?, ?> item = makeItem(); + assertEquals(item.getCatalogItemType(), CatalogItemType.TEMPLATE); + Assert.assertTrue(item.getPlanYaml().indexOf("sample comment")>=0, + "YAML did not include original comments; it was:\n"+item.getPlanYaml()); + Assert.assertFalse(item.getPlanYaml().indexOf("description")>=0, + "YAML included metadata which should have been excluded; it was:\n"+item.getPlanYaml()); + + deleteCatalogEntity("t1"); + } + + @Test + public void testAddCatalogItemAndCheckSource() throws Exception { + // this will fail with the Eclipse TestNG plugin -- use the static main instead to run in eclipse! + // see Yamls.KnownClassVersionException for details + + CatalogItem<?, ?> item = makeItem(); + Assert.assertTrue(item.getPlanYaml().indexOf("sample comment")>=0, + "YAML did not include original comments; it was:\n"+item.getPlanYaml()); + Assert.assertFalse(item.getPlanYaml().indexOf("description")>=0, + "YAML included metadata which should have been excluded; it was:\n"+item.getPlanYaml()); + + deleteCatalogEntity("t1"); + } + + private CatalogItem<?, ?> makeItem() { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); addCatalogItem( @@ -57,13 +82,7 @@ public class CatalogYamlTemplateTest extends AbstractYamlTest { " - type: " + SIMPLE_ENTITY_TYPE); CatalogItem<?, ?> item = mgmt().getCatalog().getCatalogItem("t1", TEST_VERSION); - assertEquals(item.getCatalogItemType(), CatalogItemType.TEMPLATE); - Assert.assertTrue(item.getPlanYaml().indexOf("sample comment")>=0, - "YAML did not include original comments; it was:\n"+item.getPlanYaml()); - Assert.assertFalse(item.getPlanYaml().indexOf("description")>=0, - "YAML included metadata which should have been excluded; it was:\n"+item.getPlanYaml()); - - deleteCatalogEntity("t1"); + return item; } // convenience for running in eclipse when the TestNG plugin drags in old version of snake yaml http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/usage/launcher/src/test/resources/couchbase-w-loadgen.yaml ---------------------------------------------------------------------- diff --git a/usage/launcher/src/test/resources/couchbase-w-loadgen.yaml b/usage/launcher/src/test/resources/couchbase-w-loadgen.yaml index 70ed435..101511e 100644 --- a/usage/launcher/src/test/resources/couchbase-w-loadgen.yaml +++ b/usage/launcher/src/test/resources/couchbase-w-loadgen.yaml @@ -30,7 +30,7 @@ services: createBuckets: [ { bucket: default } ] brooklyn.config: provisioning.properties: - minRam: 16384 + minRam: 16g minCores: 4 brooklyn.policies: - type: brooklyn.policy.autoscaling.AutoScalerPolicy http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6f75f40b/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java b/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java index b2b81a9..2ad9eee 100644 --- a/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java +++ b/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; @@ -211,7 +212,31 @@ public class Yamls { try { return mark.getIndex(); } catch (NoSuchMethodError e) { - throw new IllegalStateException("Class version error. This can happen if using a TestNG plugin in your IDE " + try { + getClass().getClassLoader().loadClass("org.testng.TestNG"); + } catch (ClassNotFoundException e1) { + // not using TestNG + Exceptions.propagateIfFatal(e1); + throw e; + } + if (!LOGGED_TESTNG_WARNING.getAndSet(true)) { + log.warn("Detected TestNG/SnakeYAML version incompatibilities: " + + "some YAML source reconstruction will be unavailable. " + + "This can happen with TestNG plugins which force an older version of SnakeYAML " + + "which does not support Mark.getIndex. " + + "It should not occur from maven CLI runs. " + + "(Subsequent occurrences will be silently dropped, and source code reconstructed from YAML.)"); + } + // using TestNG + throw new KnownClassVersionException(e); + } + } + + static AtomicBoolean LOGGED_TESTNG_WARNING = new AtomicBoolean(); + static class KnownClassVersionException extends IllegalStateException { + private static final long serialVersionUID = -1620847775786753301L; + public KnownClassVersionException(Throwable e) { + super("Class version error. This can happen if using a TestNG plugin in your IDE " + "which is an older version, dragging in an older version of SnakeYAML which does not support Mark.getIndex.", e); } } @@ -322,7 +347,11 @@ public class Yamls { return getMatchedYamlText(); } catch (Exception e) { Exceptions.propagateIfFatal(e); - log.warn("Unable to match yaml text in "+this+": "+e, e); + if (e instanceof KnownClassVersionException) { + log.debug("Known class version exception; no yaml text being matched for "+this+": "+e); + } else { + log.warn("Unable to match yaml text in "+this+": "+e, e); + } return null; } }
