merge default values in config inheritance as per (6) at https://issues.apache.org/jira/browse/BROOKLYN-329
(only does it for yaml parameters, but that's a limitation we can live with i think; notes included on doing it for java) also switches to new `fromString` which adjusts the interpretation of inheritance "none" to be NOT_INHERITED, with new keyword "never" available if caller really wants NEVER_INHERITED (this is in keeping with previous changes to the semantics) Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a2f8b207 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a2f8b207 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a2f8b207 Branch: refs/heads/master Commit: a2f8b207cc576c350a9a9e9d3431bc8c2df86e8d Parents: cc639c5 Author: Alex Heneveld <alex@Alexs-MacBook-Pro.local> Authored: Mon Dec 5 10:50:38 2016 +0000 Committer: Alex Heneveld <alex@Alexs-MacBook-Pro.local> Committed: Tue Dec 6 11:33:16 2016 +0000 ---------------------------------------------------------------------- .../internal/AbstractBrooklynObjectSpec.java | 29 +-- .../BrooklynComponentTemplateResolver.java | 65 ++++--- .../brooklyn/ConfigInheritanceYamlTest.java | 2 +- .../camp/brooklyn/ConfigParametersYamlTest.java | 45 +++++ .../core/config/BasicConfigInheritance.java | 25 +++ .../brooklyn/core/objs/BasicSpecParameter.java | 184 +++++++++++++++++-- .../brooklyn/config/ConfigInheritance.java | 1 + 7 files changed, 291 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java index dd65a44..e21d765 100644 --- a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java @@ -22,11 +22,20 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.io.Serializable; import java.lang.reflect.Modifier; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.brooklyn.api.mgmt.EntityManager; +import org.apache.brooklyn.api.mgmt.Task; +import org.apache.brooklyn.api.objs.BrooklynObject; +import org.apache.brooklyn.api.objs.SpecParameter; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.config.ConfigKey.HasConfigKey; +import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,15 +46,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; -import org.apache.brooklyn.api.mgmt.EntityManager; -import org.apache.brooklyn.api.mgmt.Task; -import org.apache.brooklyn.api.objs.BrooklynObject; -import org.apache.brooklyn.api.objs.SpecParameter; -import org.apache.brooklyn.config.ConfigKey; -import org.apache.brooklyn.config.ConfigKey.HasConfigKey; -import org.apache.brooklyn.util.collections.MutableSet; -import org.apache.brooklyn.util.exceptions.Exceptions; - /** * Defines a spec for creating a {@link BrooklynObject}. * <p> @@ -147,11 +147,13 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly // 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. + + // probably the thing to do is deprecate the ambiguous method in favour of an explicit @Beta public SpecT parameters(List<? extends SpecParameter<?>> parameters) { return parametersReplace(parameters); } - /** adds the given parameters */ + /** adds the given parameters, new ones first so they dominate subsequent ones */ @Beta public SpecT parametersAdd(List<? extends SpecParameter<?>> parameters) { // parameters follows immutable pattern, unlike the other fields @@ -159,15 +161,14 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly Set<SpecParameter<?>> current = MutableSet.<SpecParameter<?>>copyOf(this.parameters); current.removeAll(params); - this.parameters = ImmutableList.<SpecParameter<?>>builder() + return parametersReplace(ImmutableList.<SpecParameter<?>>builder() .addAll(params) .addAll(current) - .build(); - return self(); + .build()); } /** replaces parameters with the given */ @Beta - public SpecT parametersReplace(List<? extends SpecParameter<?>> parameters) { + public SpecT parametersReplace(Collection<? extends SpecParameter<?>> parameters) { this.parameters = ImmutableList.copyOf(checkNotNull(parameters, "parameters")); return self(); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java index 6e1bede..298196d 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java @@ -197,7 +197,7 @@ public class BrooklynComponentTemplateResolver { List<EntitySpecResolver> overrides = new ArrayList<>(); Iterable<ServiceTypeResolver> loader = FrameworkLookup.lookupAll(ServiceTypeResolver.class, mgmt.getCatalogClassLoader()); for (ServiceTypeResolver resolver : loader) { - overrides.add(new ServiceTypeResolverAdaptor(this, resolver)); + overrides.add(new ServiceTypeResolverAdaptor(this, resolver)); } return overrides; } @@ -222,7 +222,9 @@ public class BrooklynComponentTemplateResolver { for (Map<String,?> childAttrs : children) { BrooklynComponentTemplateResolver entityResolver = BrooklynComponentTemplateResolver.Factory.newInstance(loader, childAttrs); // encounteredRegisteredTypeIds must contain the items currently being loaded (the dependency chain), - // but not parent items in this type already resolved. + // but not parent items in this type already resolved (this is because an item's definition should + // not include itself here, as a defined child, as that would create an endless loop; + // use in a member spec is fine) EntitySpec<? extends Entity> childSpec = entityResolver.resolveSpec(encounteredRegisteredTypeIds); spec.child(EntityManagementUtils.unwrapEntity(childSpec)); } @@ -284,15 +286,15 @@ public class BrooklynComponentTemplateResolver { bag.putAsStringKeyIfAbsent(r.getFlagName(), r.getFlagMaybeValue().get()); } - + // now set configuration for all the items in the bag Map<String, ConfigKey<?>> entityConfigKeys = findAllConfigKeys(spec); - + Collection<FlagConfigKeyAndValueRecord> records = findAllFlagsAndConfigKeyValues(spec, bag); Set<String> keyNamesUsed = new LinkedHashSet<String>(); for (FlagConfigKeyAndValueRecord r: records) { // flags and config keys tracked separately, look at each (may be overkill but it's what we've always done) - + Function<Maybe<Object>, Maybe<Object>> rawConvFn = Functions.identity(); if (r.getFlagMaybeValue().isPresent()) { final String flag = r.getFlagName(); @@ -307,20 +309,20 @@ public class BrooklynComponentTemplateResolver { } }; Iterable<? extends ConfigValueAtContainer<EntitySpec<?>,Object>> ckvi = MutableList.of( - new LazyContainerAndKeyValue<EntitySpec<?>,Object>(key, null, rawEvalFn, rawConvFn)); - + new LazyContainerAndKeyValue<EntitySpec<?>,Object>(key, null, rawEvalFn, rawConvFn)); + ConfigValueAtContainer<EntitySpec<?>,Object> combinedVal = ConfigInheritances.resolveInheriting( - null, key, Maybe.ofAllowingNull(ownValueF), Maybe.<Object>absent(), - ckvi.iterator(), InheritanceContext.TYPE_DEFINITION, getDefaultConfigInheritance()).getWithoutError(); - + null, key, Maybe.ofAllowingNull(ownValueF), Maybe.<Object>absent(), + ckvi.iterator(), InheritanceContext.TYPE_DEFINITION, getDefaultConfigInheritance()).getWithoutError(); + spec.configure(flag, combinedVal.get()); keyNamesUsed.add(flag); } - + if (r.getConfigKeyMaybeValue().isPresent()) { final ConfigKey<Object> key = (ConfigKey<Object>) r.getConfigKey(); final Object ownValueF = new SpecialFlagsTransformer(loader, encounteredRegisteredTypeIds).apply(r.getConfigKeyMaybeValue().get()); - + Function<EntitySpec<?>, Maybe<Object>> rawEvalFn = new Function<EntitySpec<?>,Maybe<Object>>() { @Override public Maybe<Object> apply(EntitySpec<?> input) { @@ -328,12 +330,12 @@ public class BrooklynComponentTemplateResolver { } }; Iterable<? extends ConfigValueAtContainer<EntitySpec<?>,Object>> ckvi = MutableList.of( - new LazyContainerAndKeyValue<EntitySpec<?>,Object>(key, null, rawEvalFn, rawConvFn)); - + new LazyContainerAndKeyValue<EntitySpec<?>,Object>(key, null, rawEvalFn, rawConvFn)); + ConfigValueAtContainer<EntitySpec<?>,Object> combinedVal = ConfigInheritances.resolveInheriting( - null, key, Maybe.ofAllowingNull(ownValueF), Maybe.<Object>absent(), - ckvi.iterator(), InheritanceContext.TYPE_DEFINITION, getDefaultConfigInheritance()).getWithoutError(); - + null, key, Maybe.ofAllowingNull(ownValueF), Maybe.<Object>absent(), + ckvi.iterator(), InheritanceContext.TYPE_DEFINITION, getDefaultConfigInheritance()).getWithoutError(); + spec.configure(key, combinedVal.get()); keyNamesUsed.add(key.getName()); } @@ -352,7 +354,7 @@ public class BrooklynComponentTemplateResolver { spec.removeFlag(key.getName()); } } - + // set unused keys as anonymous config keys - // they aren't flags or known config keys, so must be passed as config keys in order for // EntitySpec to know what to do with them (as they are passed to the spec as flags) @@ -386,14 +388,14 @@ public class BrooklynComponentTemplateResolver { // a key in `EntitySpec.config`, then the put will replace the value and leave the key // as-is (so the default-value and description of the key will remain as whatever the // first put said). - + // TODO We should remove duplicates, rather than just doing the `put` multiple times, // relying on ordering. We should also respect the ordered returned by // EntityDynamicType.getConfigKeys, which is much better (it respects `BasicConfigKeyOverwriting` // etc). - // Or rather if the parameter fields are incomplete they might be merged with those defined - // on the type (eg description, default value) or ancestor, so that it isn't necessary for users - // to re-declare those in a parameter definition, just anything they wish to overwrite. + // Or rather if the parameter fields are incomplete they might be merged with those defined + // on the type (eg description, default value) or ancestor, so that it isn't necessary for users + // to re-declare those in a parameter definition, just anything they wish to overwrite. // // However, that is hard/fiddly because of the way a config key can be referenced by // its real name or flag-name. @@ -406,8 +408,9 @@ public class BrooklynComponentTemplateResolver { // name "B" the "val2" then applies to both! // // I plan to propose a change on dev@brooklyn, to replace `@SetFromFlag`! - - // need to de-dupe? (can't use Set bc FCKVR doesn't impl equals/hashcode) + + // need to de-dupe? (can't use Set bc FCKVR doesn't impl equals/hashcode) + // TODO merge *bagFlags* with existing spec params, merge yaml w yaml parent params elsewhere List<FlagConfigKeyAndValueRecord> allKeys = MutableList.of(); allKeys.addAll(FlagUtils.findAllParameterConfigKeys(spec.getParameters(), bagFlags)); if (spec.getImplementation() != null) { @@ -421,13 +424,23 @@ public class BrooklynComponentTemplateResolver { } private Map<String, ConfigKey<?>> findAllConfigKeys(EntitySpec<?> spec) { + // TODO use in BasicSpecParameter to resolve ancestor config keys ? Set<Class<?>> types = MutableSet.<Class<?>>builder() - .add(spec.getType()) .add(spec.getImplementation()) + .add(spec.getType()) .addAll(spec.getAdditionalInterfaces()) .remove(null) .build(); + // order above is important, respected below to take the first one defined MutableMap<String, ConfigKey<?>> result = MutableMap.copyOf(FlagUtils.findAllConfigKeys(null, types)); + + // put parameters atop config keys + // TODO currently at this point parameters have been merged with ancestor spec parameters, + // but *not* with config keys defined on the java type + // see comments in BasicSpecParameter; + // one way to fix would be to record in BasicSpecParameter which type fields are explicitly set + // and to do a further merge here with result.remove(param.getConfigKey().getName()); + // another way, probably simpler, would be to do the above result computation in BasicSpecParameter for (SpecParameter<?> param : spec.getParameters()) { result.put(param.getConfigKey().getName(), param.getConfigKey()); } @@ -497,7 +510,7 @@ public class BrooklynComponentTemplateResolver { Map<String, Object> resolvedConfig = (Map<String, Object>)transformSpecialFlags(specConfig.getSpecConfiguration()); specConfig.setSpecConfiguration(resolvedConfig); EntitySpec<?> entitySpec = Factory.newInstance(getLoader(), specConfig.getSpecConfiguration()).resolveSpec(encounteredRegisteredTypeIds); - + return EntityManagementUtils.unwrapEntity(entitySpec); } if (flag instanceof ManagementContextInjectable) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java index bccf750..408a846 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java @@ -519,7 +519,7 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " default: {myDefaultKey: myDefaultVal}", " - name: map.type-never", " type: java.util.Map", - " inheritance.parent: none", + " inheritance.parent: never", " default: {myDefaultKey: myDefaultVal}"); // Test retrieval of defaults http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java index 35d3a12..b001f27 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java @@ -31,6 +31,7 @@ import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.location.PortRange; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.ConfigPredicates; import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.location.PortRanges; import org.apache.brooklyn.core.sensor.Sensors; @@ -51,6 +52,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import com.google.common.base.Joiner; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -576,6 +578,49 @@ public class ConfigParametersYamlTest extends AbstractYamlRebindTest { } @Test + public void testConfigParameterInSubInheritsDefaultFromYaml() throws Exception { + // TODO note that the corresponding functionality to inherit config info from a *java* config key is not supported + // see notes in BasicParameterSpec + + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: entity-with-keys", + " item:", + " type: "+TestEntity.class.getName(), + " brooklyn.parameters:", + " - name: my.param.key", + " type: string", + " description: description one", + " default: myDefaultVal", + " brooklyn.config:", + " my.other.key: $brooklyn:config(\"my.param.key\")"); + + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: wrapper-entity", + " item:", + " brooklyn.parameters:", + " - name: my.param.key", + " description: description two", + " type: entity-with-keys"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: wrapper-entity"); + + Entity app = createStartWaitAndLogApplication(yaml); + final TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + LOG.info("Config keys declared on "+entity+": "+entity.config().findKeysDeclared(Predicates.alwaysTrue())); + ConfigKey<?> key = Iterables.getOnlyElement( entity.config().findKeysDeclared(ConfigPredicates.nameEqualTo("my.param.key")) ); + assertEquals(key.getDescription(), "description two"); + assertEquals(entity.config().get(key), "myDefaultVal"); + } + + @Test public void testSubTypeUsesDefaultsFromSuperInConfigMerging() throws Exception { RecordingSshTool.setCustomResponse(".*myCommand.*", new RecordingSshTool.CustomResponse(0, "myResponse", null)); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java index d064eb9..d63d330 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java @@ -35,6 +35,7 @@ import org.apache.brooklyn.util.collections.CollectionMerger; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.exceptions.ReferenceWithError; import org.apache.brooklyn.util.guava.Maybe; +import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -398,4 +399,28 @@ public class BasicConfigInheritance implements ConfigInheritance { public String toString() { return super.toString()+"[reinherit="+isReinherited()+"; strategy="+getConflictResolutionStrategy()+"; localDefaultResolvesWithAncestor="+localDefaultResolvesWithAncestorValue+"]"; } + + public static ConfigInheritance fromString(String val) { + if (Strings.isBlank(val)) return null; + // in all cases below the first value is preferred, but to preserve backwards compatibility we + // need to support some of the other names/strategies; yoml should give us a way to migrate to canonical form + switch (val.toLowerCase().trim()) { + case "not_reinherited": + case "notreinherited": + case "none": + return NOT_REINHERITED; + case "never": + return NEVER_INHERITED; + case "overwrite": + case "always": + return OVERWRITE; + case "deep_merge": + case "merge": + case "deepmerge": + return DEEP_MERGE; + default: + throw new IllegalArgumentException("Invalid config-inheritance '"+val+"' (legal values are none, always or deep_merge)"); + } + } + } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java index cc9d66a..5344bc1 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java @@ -20,6 +20,7 @@ package org.apache.brooklyn.core.objs; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.Date; @@ -40,10 +41,14 @@ import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.config.ConfigInheritance; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; +import org.apache.brooklyn.core.config.BasicConfigInheritance; import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.BasicConfigKey.Builder; +import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.ConfigKeys.InheritanceContext; import org.apache.brooklyn.core.sensor.PortAttributeSensorAndConfigKey; import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.StringPredicates; import org.apache.brooklyn.util.time.Duration; @@ -51,6 +56,7 @@ import org.apache.brooklyn.util.time.Duration; import com.google.common.annotations.Beta; import com.google.common.base.Function; import com.google.common.base.Objects; +import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.FluentIterable; @@ -62,11 +68,11 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ private static final long serialVersionUID = -4728186276307619778L; private final String label; - + /** pinning may become a priority or other more expansive indicator */ @Beta private final boolean pinned; - + private final ConfigKey<T> configKey; private final AttributeSensor<?> sensor; @@ -74,15 +80,16 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ // Automatically called by xstream (which is used under the covers by XmlMementoSerializer). // Required for those who have state from a version between // 29th October 2015 and 21st January 2016 (when this class was introduced, and then when it was changed). + /** @deprecated since 0.9.0 can be removed, along with readResolve, when field not used any further */ private ConfigKey<T> type; private Object readResolve() { if (type != null && configKey == null) { - return new BasicSpecParameter(label, pinned, type, sensor); + return new BasicSpecParameter<T>(label, pinned, type, sensor); } else { return this; } } - + @Beta public BasicSpecParameter(String label, boolean pinned, ConfigKey<T> config) { this(label, pinned, config, null); @@ -105,7 +112,7 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ public boolean isPinned() { return pinned; } - + @Override public ConfigKey<T> getConfigKey() { return configKey; @@ -227,13 +234,30 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ String label = (String)inputDef.get("label"); String description = (String)inputDef.get("description"); String type = (String)inputDef.get("type"); - Object defaultValue = inputDef.get("default"); Boolean pinned = (Boolean)inputDef.get("pinned"); + boolean hasDefaultValue = inputDef.containsKey("default"); + Object defaultValue = inputDef.get("default"); if (specialFlagTransformer != null) { defaultValue = specialFlagTransformer.apply(defaultValue); } + boolean hasConstraints = inputDef.containsKey("constraints"); Predicate<?> constraint = parseConstraints(inputDef.get("constraints"), loader); - ConfigInheritance parentInheritance = parseInheritance(inputDef.get("inheritance.parent"), loader); + + ConfigInheritance runtimeInheritance; + boolean hasRuntimeInheritance; + if (inputDef.containsKey("inheritance.runtime")) { + hasRuntimeInheritance = true; + runtimeInheritance = parseInheritance(inputDef.get("inheritance.runtime"), loader); + } else if (inputDef.containsKey("inheritance.parent")) { + // this alias will be deprecated + hasRuntimeInheritance = true; + runtimeInheritance = parseInheritance(inputDef.get("inheritance.parent"), loader); + } else { + hasRuntimeInheritance = false; + runtimeInheritance = parseInheritance(null, loader); + } + + boolean hasTypeInheritance = inputDef.containsKey("inheritance.type"); ConfigInheritance typeInheritance = parseInheritance(inputDef.get("inheritance.type"), loader); if (name == null) { @@ -242,15 +266,17 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ ConfigKey configType; AttributeSensor sensorType = null; - + + boolean hasType = type!=null; + TypeToken typeToken = inferType(type, loader); Builder builder = BasicConfigKey.builder(typeToken) - .name(name) - .description(description) - .defaultValue(defaultValue) - .constraint(constraint) - .runtimeInheritance(parentInheritance) - .typeInheritance(typeInheritance); + .name(name) + .description(description) + .defaultValue(defaultValue) + .constraint(constraint) + .runtimeInheritance(runtimeInheritance) + .typeInheritance(typeInheritance); if (PortRange.class.equals(typeToken.getRawType())) { sensorType = new PortAttributeSensorAndConfigKey(builder); @@ -258,7 +284,9 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ } else { configType = builder.build(); } - return new BasicSpecParameter(Objects.firstNonNull(label, name), !Boolean.FALSE.equals(pinned), configType, sensorType); + + return new SpecParameterForInheritance(label, pinned, configType, sensorType, + hasType, hasDefaultValue, hasConstraints, hasRuntimeInheritance, hasTypeInheritance); } @SuppressWarnings({ "rawtypes" }) @@ -309,11 +337,10 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ return Predicates.alwaysTrue(); } } - + private static ConfigInheritance parseInheritance(Object obj, BrooklynClassLoadingContext loader) { if (obj == null || obj instanceof String) { - // TODO - return ConfigInheritance.Legacy.fromString((String)obj); + return BasicConfigInheritance.fromString((String)obj); } else { throw new IllegalArgumentException ("The config-inheritance '" + obj + "' for a catalog input is invalid format - string supported"); } @@ -404,7 +431,126 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ spec.parametersAdd(BasicSpecParameter.fromSpec(loader.getManagementContext(), spec)); } if (explicitParams.size() > 0) { - spec.parametersAdd(explicitParams); + spec.parametersReplace(resolveParameters(explicitParams, spec.getParameters(), spec)); + } + } + + /** merge parameters against other parameters and known and type-inherited config keys */ + static Collection<SpecParameter<?>> resolveParameters(Collection<? extends SpecParameter<?>> newParams, Collection<? extends SpecParameter<?>> existingReferenceParamsToKeep, AbstractBrooklynObjectSpec<?,?> spec) { + Map<String,SpecParameter<?>> existingToKeep = MutableMap.of(); + if (existingReferenceParamsToKeep!=null) { + for (SpecParameter<?> p: existingReferenceParamsToKeep) { + existingToKeep.put(p.getConfigKey().getName(), p); + } + } + + List<SpecParameter<?>> result = MutableList.<SpecParameter<?>>of(); + for (SpecParameter<?> p: newParams) { + if (p instanceof SpecParameterForInheritance) { + SpecParameter<?> existingP = existingToKeep.remove(p.getConfigKey().getName()); + if (existingP!=null) { + p = ((SpecParameterForInheritance<?>)p).resolveWithAncestor(existingP); + } else { + // TODO find config keys on the type (not available as parameters) + /* we don't currently do this due to low priority; all it means if there is a config key in java, + * and a user wishes to expose it as a parameter, they have to redeclare everything; + * nothing from the config key in java will be inherited */ + p = ((SpecParameterForInheritance<?>)p).resolveWithAncestor((ConfigKey<?>)null); + } + result.add(p); + } else { + existingToKeep.remove(p.getConfigKey().getName()); + result.add(p); + } + } + result.addAll(existingToKeep.values()); + return result; + } + + /** instance of {@link SpecParameter} which includes information about what fields are explicitly set, + * for use with a subsequent merge */ + @SuppressWarnings("serial") + @Beta + static class SpecParameterForInheritance<T> extends BasicSpecParameter<T> { + + private final boolean hasType, hasLabelSet, hasPinnedSet, hasDefaultValue, hasConstraints, hasRuntimeInheritance, hasTypeInheritance; + + private <SensorType> SpecParameterForInheritance(String label, Boolean pinned, ConfigKey<T> config, AttributeSensor<SensorType> sensor, + boolean hasType, boolean hasDefaultValue, boolean hasConstraints, boolean hasRuntimeInheritance, boolean hasTypeInheritance) { + super(Preconditions.checkNotNull(label!=null ? label : config.getName(), "label or config name must be set"), + pinned==null ? true : pinned, config, sensor); + this.hasType = hasType; + this.hasLabelSet = label!=null; + this.hasPinnedSet = pinned!=null; + this.hasDefaultValue = hasDefaultValue; + this.hasConstraints = hasConstraints; + this.hasRuntimeInheritance = hasRuntimeInheritance; + this.hasTypeInheritance = hasTypeInheritance; + } + + /** used if yaml specifies a parameter, but possibly incomplete, and a spec supertype has a parameter */ + SpecParameter<?> resolveWithAncestor(SpecParameter<?> ancestor) { + if (ancestor==null) return new BasicSpecParameter<>(getLabel(), isPinned(), getConfigKey(), getSensor()); + + return new BasicSpecParameter<>( + hasLabelSet ? getLabel() : ancestor.getLabel(), + hasPinnedSet ? isPinned() : ancestor.isPinned(), + resolveWithAncestorConfigKey(ancestor.getConfigKey()), + hasType ? getSensor() : ancestor.getSensor()); + } + + /** as {@link #resolveWithAncestor(SpecParameter)} but where the param redefines/extends a config key coming from a java supertype, rather than a parameter */ + // TODO not used yet; see calls to the other resolveWithAncestor method, + // and see BrooklynComponentTemplateResolver.findAllConfigKeys; + // also note whilst it is easiest to do this here, logically it is messy, + // and arguably it should be done when converting the spec to an instance + SpecParameter<?> resolveWithAncestor(ConfigKey<?> ancestor) { + if (ancestor==null) return new BasicSpecParameter<>(getLabel(), isPinned(), getConfigKey(), getSensor()); + + // TODO probably want to do this (but it could get expensive!) + // Set<Class<?>> types = MutableSet.<Class<?>>builder() + // .add(spec.getImplementation()) + // .add(spec.getType()) + // .addAll(spec.getAdditionalInterfaces()) + // .remove(null) + // .build(); + // // order above is important, respected below to take the first one defined + // MutableMap<String, ConfigKey<?>> result = MutableMap.copyOf(FlagUtils.findAllConfigKeys(null, types)); + + return new BasicSpecParameter<>( + getLabel(), + isPinned(), + resolveWithAncestorConfigKey(ancestor), + // TODO port sensor will be lost (see messy code above which sets the port sensor) + getSensor()); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private ConfigKey<?> resolveWithAncestorConfigKey(ConfigKey<?> ancestor) { + ConfigKey<?> dominant = getConfigKey(); + return ConfigKeys.<Object>builder((TypeToken)(hasType ? dominant : ancestor).getTypeToken()) + .name(dominant.getName()) + .description((dominant.getDescription()!=null ? dominant : ancestor).getDescription()) + .defaultValue((hasDefaultValue ? dominant : ancestor).getDefaultValue()) + .constraint((Predicate<Object>) (hasConstraints ? dominant : ancestor).getConstraint()) + .runtimeInheritance( (hasRuntimeInheritance ? dominant : ancestor).getInheritanceByContext(InheritanceContext.RUNTIME_MANAGEMENT)) + .typeInheritance( (hasTypeInheritance ? dominant : ancestor).getInheritanceByContext(InheritanceContext.TYPE_DEFINITION)) + // not yet configurable from yaml + //.reconfigurable() + .build(); + } + + public boolean isHasDefaultValue() { + return hasDefaultValue; + } + public boolean isHasConstraints() { + return hasConstraints; + } + public boolean isHasRuntimeInheritance() { + return hasRuntimeInheritance; + } + public boolean isHasTypeInheritance() { + return hasTypeInheritance; } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a2f8b207/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java index 203348e..f089f96 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java +++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java @@ -130,6 +130,7 @@ public interface ConfigInheritance extends Serializable { /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated public static class Legacy { + /** @deprecated since 0.10.0 see fromString in selected subclasses of {@link ConfigInheritance} eg BasicConfigInheritance */ public static ConfigInheritance fromString(String val) { if (Strings.isBlank(val)) return null; switch (val.toLowerCase().trim()) {