skip validation for "template" catalog items as that isn't always supported
prevents errors on startup and in log for catalog rest api calls Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/61d4821c Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/61d4821c Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/61d4821c Branch: refs/heads/master Commit: 61d4821cdcf157076e485c62ed72aab2f2a1fa8a Parents: e5f93cc Author: Alex Heneveld <[email protected]> Authored: Sat Jun 20 11:13:49 2015 -0700 Committer: Alex Heneveld <[email protected]> Committed: Wed Jun 24 00:40:33 2015 -0700 ---------------------------------------------------------------------- .../catalog/internal/BasicBrooklynCatalog.java | 3 ++- .../BrooklynAssemblyTemplateInstantiator.java | 2 +- .../spi/creation/BrooklynEntityMatcher.java | 6 ++--- usage/cli/src/main/java/brooklyn/cli/Main.java | 15 ++++++++--- .../rest/transform/CatalogTransformer.java | 27 ++++++++++++-------- .../src/main/java/brooklyn/util/yaml/Yamls.java | 10 +++++++- .../test/java/brooklyn/util/yaml/YamlsTest.java | 20 +++++++++++++++ 7 files changed, 62 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/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 31458f5..37af858 100644 --- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java @@ -325,8 +325,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { @SuppressWarnings("unchecked") @Override public <T, SpecT> SpecT createSpec(CatalogItem<T, SpecT> item) { + if (item == null) return null; CatalogItemDo<T,SpecT> loadedItem = (CatalogItemDo<T, SpecT>) getCatalogItemDo(item.getSymbolicName(), item.getVersion()); - if (loadedItem == null) return null; + if (loadedItem == null) throw new RuntimeException(item+" not in catalog; cannot create spec"); Class<SpecT> specType = loadedItem.getSpecType(); if (specType==null) return null; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java index 1519326..68136f2 100644 --- a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java +++ b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynAssemblyTemplateInstantiator.java @@ -183,7 +183,7 @@ public class BrooklynAssemblyTemplateInstantiator implements AssemblyTemplateSpe // TODO support https above // TODO this will probably be logged if we refer to chef:cookbook or other service types which BCTR accepts; // better would be to have BCTR supporting the calls above - log.warn("The reference " + brooklynType + " looks like an URL but the protocol " + + log.debug("The reference " + brooklynType + " looks like a URL (running the CAMP Brooklyn assembly-template instantiator) but the protocol " + protocol + " isn't white listed (" + BrooklynCampConstants.YAML_URL_PROTOCOL_WHITELIST + "). " + "Will try to load it as catalog item or java type."); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java index aae2b19..50b57e9 100644 --- a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java +++ b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityMatcher.java @@ -80,7 +80,7 @@ public class BrooklynEntityMatcher implements PdpMatcher { if (BrooklynCampConstants.YAML_URL_PROTOCOL_WHITELIST.contains(protocol)) { return serviceType; } else { - log.warn("The reference '" + serviceType + "' looks like an URL but the protocol '" + + log.debug("The reference '" + serviceType + "' looks like a URL (running the CAMP Brooklyn entity-matcher) but the protocol '" + protocol + "' isn't white listed " + BrooklynCampConstants.YAML_URL_PROTOCOL_WHITELIST + ". " + "Not recognized as catalog item or java item as well!"); } @@ -229,9 +229,7 @@ public class BrooklynEntityMatcher implements PdpMatcher { } catch (Exception e) { Exceptions.propagateIfFatal(e); if (e.toString().contains("Could not find")) { - // TODO currently we get this error if a catalog item is passed, giving stack trace is too scary; - // when we are doing catalog.createSpec let's remove this block - log.warn("Ignoring configuration attributes on "+typeName+", item probably loaded from catalog and flags are not yet supported here"); + // normal for catalog items, there will be no java type log.debug("Ignoring configuration attributes on "+typeName+", details: "+e); } else { log.warn("Ignoring configuration attributes on "+typeName+" due to "+e, e); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/usage/cli/src/main/java/brooklyn/cli/Main.java ---------------------------------------------------------------------- diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java index 0f93057..80d2b05 100644 --- a/usage/cli/src/main/java/brooklyn/cli/Main.java +++ b/usage/cli/src/main/java/brooklyn/cli/Main.java @@ -41,6 +41,7 @@ import brooklyn.BrooklynVersion; import brooklyn.basic.BrooklynTypes; import brooklyn.catalog.BrooklynCatalog; import brooklyn.catalog.CatalogItem; +import brooklyn.catalog.CatalogItem.CatalogItemType; import brooklyn.catalog.internal.CatalogInitialization; import brooklyn.cli.CloudExplorer.BlobstoreGetBlobCommand; import brooklyn.cli.CloudExplorer.BlobstoreListContainerCommand; @@ -633,11 +634,17 @@ public class Main extends AbstractMain { Iterable<CatalogItem<Object, Object>> items = catalog.getCatalogItems(); for (CatalogItem<Object, Object> item: items) { try { - Object spec = catalog.createSpec(item); - if (spec instanceof EntitySpec) { - BrooklynTypes.getDefinedEntityType(((EntitySpec<?>)spec).getType()); + if (item.getCatalogItemType()==CatalogItemType.TEMPLATE) { + // skip validation of templates, they might contain instructions, + // and additionally they might contain multiple items in which case + // the validation below won't work anyway (you need to go via a deployment plan) + } else { + Object spec = catalog.createSpec(item); + if (spec instanceof EntitySpec) { + BrooklynTypes.getDefinedEntityType(((EntitySpec<?>)spec).getType()); + } + log.debug("Catalog loaded spec "+spec+" for item "+item); } - log.debug("Catalog loaded spec "+spec+" for item "+item); } catch (Throwable throwable) { catInit.handleException(throwable, item); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/usage/rest-server/src/main/java/brooklyn/rest/transform/CatalogTransformer.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/transform/CatalogTransformer.java b/usage/rest-server/src/main/java/brooklyn/rest/transform/CatalogTransformer.java index 053a9c3..a9f9baf 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/transform/CatalogTransformer.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/transform/CatalogTransformer.java @@ -59,21 +59,28 @@ public class CatalogTransformer { private static final org.slf4j.Logger log = LoggerFactory.getLogger(CatalogTransformer.class); public static CatalogEntitySummary catalogEntitySummary(BrooklynRestResourceUtils b, CatalogItem<? extends Entity,EntitySpec<?>> item) { - EntitySpec<?> spec = b.getCatalog().createSpec(item); - EntityDynamicType typeMap = BrooklynTypes.getDefinedEntityType(spec.getType()); - EntityType type = typeMap.getSnapshot(); - Set<EntityConfigSummary> config = Sets.newTreeSet(SummaryComparators.nameComparator()); Set<SensorSummary> sensors = Sets.newTreeSet(SummaryComparators.nameComparator()); Set<EffectorSummary> effectors = Sets.newTreeSet(SummaryComparators.nameComparator()); - for (ConfigKey<?> x: type.getConfigKeys()) - config.add(EntityTransformer.entityConfigSummary(x, typeMap.getConfigKeyField(x.getName()))); - for (Sensor<?> x: type.getSensors()) - sensors.add(SensorTransformer.sensorSummaryForCatalog(x)); - for (Effector<?> x: type.getEffectors()) - effectors.add(EffectorTransformer.effectorSummaryForCatalog(x)); + try { + EntitySpec<?> spec = b.getCatalog().createSpec(item); + EntityDynamicType typeMap = BrooklynTypes.getDefinedEntityType(spec.getType()); + EntityType type = typeMap.getSnapshot(); + for (ConfigKey<?> x: type.getConfigKeys()) + config.add(EntityTransformer.entityConfigSummary(x, typeMap.getConfigKeyField(x.getName()))); + for (Sensor<?> x: type.getSensors()) + sensors.add(SensorTransformer.sensorSummaryForCatalog(x)); + for (Effector<?> x: type.getEffectors()) + effectors.add(EffectorTransformer.effectorSummaryForCatalog(x)); + + } catch (Exception e) { + // templates with multiple entities can't have spec created in the manner above; just ignore + if (log.isTraceEnabled()) + log.trace("Unable to create spec for "+item+": "+e, e); + } + return new CatalogEntitySummary(item.getSymbolicName(), item.getVersion(), item.getDisplayName(), item.getJavaType(), item.getPlanYaml(), item.getDescription(), tidyIconLink(b, item, item.getIconUrl()), http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/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 2ad9eee..b46e9d1 100644 --- a/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java +++ b/utils/common/src/main/java/brooklyn/util/yaml/Yamls.java @@ -44,6 +44,7 @@ import org.yaml.snakeyaml.nodes.SequenceNode; import brooklyn.util.collections.Jsonya; import brooklyn.util.collections.MutableList; import brooklyn.util.exceptions.Exceptions; +import brooklyn.util.exceptions.UserFacingException; import brooklyn.util.text.Strings; import com.google.common.annotations.Beta; @@ -350,7 +351,11 @@ public class Yamls { 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); + if (e instanceof UserFacingException) { + log.warn("Unable to match yaml text in "+this+": "+e.getMessage()); + } else { + log.warn("Unable to match yaml text in "+this+": "+e, e); + } } return null; } @@ -402,6 +407,9 @@ b: 1 */ List<String> result = MutableList.of(); if (includePrecedingComments) { + if (getEndOfPrevious() > getStartOfThis()) { + throw new UserFacingException("YAML not in expected format; when scanning, previous end "+getEndOfPrevious()+" exceeds this start "+getStartOfThis()); + } String[] preceding = yaml.substring(getEndOfPrevious(), getStartOfThis()).split("\n"); // suppress comments which are on the same line as the previous item or indented more than firstLineIndentation, // ensuring appropriate whitespace is added to preceding[0] if it starts mid-line http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/61d4821c/utils/common/src/test/java/brooklyn/util/yaml/YamlsTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/brooklyn/util/yaml/YamlsTest.java b/utils/common/src/test/java/brooklyn/util/yaml/YamlsTest.java index 36c146b..6cd90f8 100644 --- a/utils/common/src/test/java/brooklyn/util/yaml/YamlsTest.java +++ b/utils/common/src/test/java/brooklyn/util/yaml/YamlsTest.java @@ -28,6 +28,8 @@ import org.testng.TestNG; import org.testng.annotations.Test; import brooklyn.util.collections.MutableList; +import brooklyn.util.exceptions.UserFacingException; +import brooklyn.util.yaml.Yamls.YamlExtract; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -162,6 +164,24 @@ public class YamlsTest { "x: \n c: 1\n d: 2"); } + @Test + public void testExtractNoOOBE() { + // this might log a warning, as item not found, but won't throw + YamlExtract r1 = Yamls.getTextOfYamlAtPath( + "items:\n- id: sample2\n itemType: location\n item:\n type: jclouds:aws-ec2\n brooklyn.config:\n key2: value2\n\n", + "item"); + + // won't throw + r1.getMatchedYamlTextOrWarn(); + // will throw + try { + r1.getMatchedYamlText(); + Assert.fail(); + } catch (UserFacingException e) { + // expected, it should give a vaguely explanatory exception and no trace + } + } + // convenience, since running with older TestNG IDE plugin will fail (older snakeyaml dependency); // if you run as a java app it doesn't bring in the IDE TestNG jar version, and it works public static void main(String[] args) {
