Github user ahgittin commented on a diff in the pull request: https://github.com/apache/brooklyn-server/pull/962#discussion_r186257982 --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java --- @@ -934,15 +937,17 @@ private void collectCatalogItemsFromItemMetadataBlock(String sourceYaml, Managed String sourcePlanYaml = planInterpreter.getPlanYaml(); if (resultLegacyFormat==null) { - // horrible API but basically if `result` is null then add to local unpersisted registry instead, - // without forcing resolution and ignoring errors; this lets us deal with forward references, but - // we'll have to do a validation step subsequently. (already we let bundles deal with persistence, - // just need TODO to make sure we delete previously-persisted things which now come through this path.) - // NB: when everything is a bundle and we've removed all scanning then this can be the _only_ path - // and code can be massively simpler - // TODO allow these to be set in catalog.bom ? + // horrible API but basically `resultLegacyFormat==null` means use the new-style, + // adding from persisted bundles to type registry (which is not persisted) + // instead of old way which persisted catalog items (and not their bundles). + // this lets us deal with forward references, with a subsequent step to validate. + + Set<Object> tags = MutableSet.of().putAll(getFirstAs(catalogMetadata, Collection.class, "tags").orNull()); --- End diff -- Duplicate tags are never supported. See e.g. `TagSupport.getTags()` which returns a `Set`.. The only reason it wasn't noticed here is that we didn't use tags much on these objects.
---