Skip to content
Permalink
Browse files
parameter-inheritance now marked with TODO, and prev behaviour largel…
…y restored
  • Loading branch information
ahgittin committed Jan 14, 2016
1 parent aa9a2d7 commit 7ffc21624ef6c1887f704207862381c1eb965dff
Showing 3 changed files with 36 additions and 4 deletions.
@@ -38,6 +38,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.annotations.Beta;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableList.Builder;
@@ -108,8 +109,23 @@ public SpecT tags(Iterable<Object> tagsToAdd) {
return self();
}

/** adds the given parameters */
// TODO which semantics are correct? replace has been the behaviour;
// add breaks tests and adds unwanted parameters,
// but replacing will cause some desired parameters to be lost.
// i (AH) think ideally the caller should remove any parameters which
// have been defined as config keys, and then add the others;
// or actually we should always add, since this is really defining the config keys,
// and maybe extend the SpecParameter object to be able to advertise whether
// it is a CatalogConfig or merely a config key, maybe introducing displayable, or even priority
// (but note part of the reason for CatalogConfig.priority is that java reflection doesn't preserve field order) .
// see also comments on the camp SpecParameterResolver.
@Beta
public SpecT parameters(List<? extends SpecParameter<?>> parameters) {
return parametersReplace(parameters);
}
/** adds the given parameters */
@Beta
public SpecT parametersAdd(List<? extends SpecParameter<?>> parameters) {
// parameters follows immutable pattern, unlike the other fields
Builder<SpecParameter<?>> result = ImmutableList.<SpecParameter<?>>builder();
if (this.parameters!=null)
@@ -118,6 +134,12 @@ public SpecT parameters(List<? extends SpecParameter<?>> parameters) {
this.parameters = result.build();
return self();
}
/** replaces parameters with the given */
@Beta
public SpecT parametersReplace(List<? extends SpecParameter<?>> parameters) {
this.parameters = ImmutableList.copyOf( checkNotNull(parameters, "parameters") );
return self();
}

/**
* @return The type (often an interface) this spec represents and which will be instantiated from it
@@ -186,6 +186,9 @@ public static class SpecParameterResolver extends BrooklynEntityDecorationResolv
@Override
public void decorate(EntitySpec<?> entitySpec, ConfigBag attrs) {
List<? extends SpecParameter<?>> explicitParams = buildListOfTheseDecorationsFromEntityAttributes(attrs);
// TODO see discussion at EntitySpec.parameters;
// maybe we should instead inherit always, or
// inherit except where it is set as config and then add the new explicit ones
if (!explicitParams.isEmpty()) {
entitySpec.parameters(explicitParams);
}
@@ -251,7 +251,8 @@ private static void mergeWrapperParentSpecToChildEntity(EntitySpec<? extends App

wrappedChild.locations(wrapperParent.getLocations());

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

if (wrappedChild.getCatalogItemId()==null) {
wrappedChild.catalogItemId(wrapperParent.getCatalogItemId());
@@ -302,12 +303,18 @@ public static boolean canPromoteWrappedApplication(EntitySpec<? extends Applicat
* @see #WRAPPER_APP_MARKER for an overview */
public static boolean canUnwrapEntity(EntitySpec<? extends Entity> spec) {
return isWrapperApp(spec) && hasSingleChild(spec) &&
//equivalent to no keys starting with "brooklyn."
// these "brooklyn.*" items on the app rather than the child absolutely prevent unwrapping
// as their semantics could well be different whether they are on the parent or the child
spec.getEnrichers().isEmpty() &&
spec.getEnricherSpecs().isEmpty() &&
spec.getInitializers().isEmpty() &&
spec.getPolicies().isEmpty() &&
spec.getPolicySpecs().isEmpty();
spec.getPolicySpecs().isEmpty() &&
// these items prevent merge only if they are defined at both levels
(spec.getLocations().isEmpty() || Iterables.getOnlyElement(spec.getChildren()).getLocations().isEmpty())
// TODO what should we do with parameters? currently clobbers due to EntitySpec.parameters(...) behaviour.
// && (spec.getParameters().isEmpty() || Iterables.getOnlyElement(spec.getChildren()).getParameters().isEmpty())
;
}
/** @deprecated since 0.9.0 use {@link #canUnwrapEntity(EntitySpec)} */ @Deprecated
public static boolean canPromoteChildrenInWrappedApplication(EntitySpec<? extends Application> spec) {

0 comments on commit 7ffc216

Please sign in to comment.