Skip to content
Permalink
Browse files
for chained references, prefer outer catalog item id, and outer tags
as described in changes to release-notes.md
  • Loading branch information
ahgittin committed Jan 20, 2016
1 parent 7d2ae06 commit 1affd4d606d17136d3e334d08f401398f1ff3c96
Showing 7 changed files with 80 additions and 13 deletions.
@@ -45,7 +45,9 @@ can be used to traverse the hierarchy. It also means that when a type in the re
and a caller references it, that location will now take priority over a location defined in a parent.
Additionally, any locations specified in YAML extending the registered type will now *replace* locations on the referenced type;
this means in many cases an explicit `locations: []` when extending a type will cause locations to be taken from the
parent or application root in YAML.
parent or application root in YAML. Related to this, tags from referencing specs now preceed tags in the referenced types,
and the referencing catalog item ID also takes priority; this has no effect in most cases, but if you have a chain of
referenced types blueprint plan source code and the catalog item ID are now set correctly.

For changes in prior versions, please refer to the release notes for
[0.8.0](/v/0.8.0-incubating/misc/release-notes.html).
@@ -83,7 +83,7 @@ protected SpecT self() {

@Override
public String toString() {
return Objects.toStringHelper(this).add("type", getType()).toString();
return Objects.toStringHelper(this).add("type", getType()).toString()+"@"+Integer.toHexString(System.identityHashCode(this));
}

protected abstract void checkValidType(Class<? extends T> type);
@@ -97,6 +97,19 @@ public SpecT catalogItemId(String val) {
catalogItemId = val;
return self();
}
// TODO in many places (callers to this method) we prefer a wrapper item ID;
// that is right, because the wrapper's defn will refer to the wrapped,
// but we might need also to collect the item ID's so that *all* can be searched.
// e.g. if R3 references R2 which references R1 any one of these might supply config keys
// referencing resources or types in their local bundles.
@Beta
public SpecT catalogItemIdIfNotNull(String val) {
if (val!=null) {
catalogItemId = val;
}
return self();
}


public SpecT tag(Object tag) {
tags.add(tag);
@@ -105,9 +118,19 @@ public SpecT tag(Object tag) {

/** adds the given tags */
public SpecT tags(Iterable<Object> tagsToAdd) {
return tagsAdd(tagsToAdd);
}
/** adds the given tags */
public SpecT tagsAdd(Iterable<Object> tagsToAdd) {
Iterables.addAll(this.tags, tagsToAdd);
return self();
}
/** replaces the given tags */
public SpecT tagsReplace(Iterable<Object> tagsToReplace) {
this.tags.clear();
Iterables.addAll(this.tags, tagsToReplace);
return self();
}

// TODO which semantics are correct? replace has been the behaviour;
// add breaks tests and adds unwanted parameters,
@@ -114,7 +114,7 @@ public CampResolver(ManagementContext mgmt, RegisteredType type, RegisteredTypeL
throw new IllegalStateException("Creating spec from "+item+", got "+spec.getType()+" which is incompatible with expected "+expectedType);
}

((AbstractBrooklynObjectSpec<?, ?>)spec).catalogItemId(item.getId());
((AbstractBrooklynObjectSpec<?, ?>)spec).catalogItemIdIfNotNull(item.getId());

if (Strings.isBlank( ((AbstractBrooklynObjectSpec<?, ?>)spec).getDisplayName() ))
((AbstractBrooklynObjectSpec<?, ?>)spec).displayName(item.getDisplayName());
@@ -40,6 +40,7 @@
import org.apache.brooklyn.entity.stock.BasicApplication;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.test.support.TestResourceUnavailableException;
import org.apache.brooklyn.util.core.config.ConfigBag;
import org.apache.brooklyn.util.osgi.OsgiTestResources;
import org.python.google.common.collect.Iterables;
import org.testng.Assert;
@@ -209,6 +210,45 @@ public void testMetadataOnSpecCreatedFromItem() throws Exception {
Assert.assertEquals(child.getCatalogItemId(), "t1:"+TEST_VERSION);
}

@Test
public void testMetadataOnSpecCreatedFromItemReferencingAnApp() throws Exception {
// this nested ref to an app caused nested plan contents also to be recorded,
// due to how tags are merged. the *first* one is the most important, however,
// and ordering of tags should guarantee that.
// similarly ensure we get the right outermost non-null catalog item id.
addCatalogItems(
"brooklyn.catalog:",
" version: '1'",
" items:",
" - id: app1",
" name: myApp1",
" item:",
" type: org.apache.brooklyn.entity.stock.BasicApplication",
" brooklyn.config: { foo: bar }",
" - id: app1r",
" item_type: template",
" item:",
" services:",
" - type: app1",
" brooklyn.config:",
" foo: boo"
);

EntitySpec<? extends Application> spec = EntityManagementUtils.createEntitySpecForApplication(mgmt(),
"services: [ { type: app1r } ]\n" +
"location: localhost");

List<NamedStringTag> yamls = BrooklynTags.findAll(BrooklynTags.YAML_SPEC_KIND, spec.getTags());
Assert.assertTrue(yamls.size() >= 1, "Expected at least 1 yaml tag; instead had: "+yamls);
String yaml = yamls.iterator().next().getContents();
Asserts.assertStringContains(yaml, "services:", "type: app1r", "localhost");

Assert.assertEquals(spec.getChildren().size(), 0);
Assert.assertEquals(spec.getType(), BasicApplication.class);
Assert.assertEquals(ConfigBag.newInstance(spec.getConfig()).getStringKey("foo"), "boo");
Assert.assertEquals(spec.getCatalogItemId(), "app1r:1");
}

private RegisteredType makeItem() {
TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);

@@ -253,10 +253,9 @@ private static void mergeWrapperParentSpecToChildEntity(EntitySpec<? extends App

if (!wrapperParent.getParameters().isEmpty())
wrappedChild.parametersReplace(wrapperParent.getParameters());

if (wrappedChild.getCatalogItemId()==null) {
wrappedChild.catalogItemId(wrapperParent.getCatalogItemId());
}

// prefer the wrapper ID (change in 2016-01); see notes on the catalogItemIdIfNotNull method
wrappedChild.catalogItemIdIfNotNull(wrapperParent.getCatalogItemId());

// NB: this clobber's child config wherever they conflict; might prefer to deeply merge maps etc
// (or maybe even prevent the merge in these cases;
@@ -266,11 +265,12 @@ private static void mergeWrapperParentSpecToChildEntity(EntitySpec<? extends App
wrappedChild.configure(wrapperParent.getFlags());

// copying tags to all entities may be something the caller wants to control,
// e.g. if we're creating a list of entities which will be added,
// ignoring the parent Application holder;
// in that case each child's BrooklynTags.YAML_SPEC tag will show all entities;
// but in the normal case where we're unwrapping one, it's probably right.
wrappedChild.tags(wrapperParent.getTags());
// e.g. if we're adding multiple, the caller might not want to copy the parent
// (the BrooklynTags.YAML_SPEC tag will include the parents source including siblings),
// but OTOH they might because otherwise the parent's tags might get lost.
// also if we are unwrapping multiple registry references we will get the YAML_SPEC for each;
// putting the parent's tags first however causes the preferred (outer) one to be retrieved first.
wrappedChild.tagsReplace(MutableList.copyOf(wrapperParent.getTags()).appendAll(wrappedChild.getTags()));
}

public static EntitySpec<? extends Application> newWrapperApp() {
@@ -103,7 +103,8 @@ public Object create(final RegisteredType type, final RegisteredTypeLoadingConte
@Override protected Object visitSpec() {
try {
AbstractBrooklynObjectSpec<?, ?> result = createSpec(type, context);
result.catalogItemId(type.getId());
// see notes in CampResolver setting catalogItemId; we might need to keep both
result.catalogItemIdIfNotNull(type.getId());
return result;
} catch (Exception e) { throw Exceptions.propagate(e); }
}
@@ -64,6 +64,7 @@ public class BasicRegisteredType implements RegisteredType {

@Override
public String getId() {
if (symbolicName==null) return null;
return symbolicName + (version!=null ? ":"+version : "");
}

0 comments on commit 1affd4d

Please sign in to comment.