Support configKey.deprecatedNames
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/02b71ffb Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/02b71ffb Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/02b71ffb Branch: refs/heads/master Commit: 02b71ffb5f81abb6ad76095a64903335b9692fda Parents: 93b7e18 Author: Aled Sage <[email protected]> Authored: Thu Jun 8 14:58:46 2017 +0100 Committer: Aled Sage <[email protected]> Committed: Fri Jun 16 13:13:23 2017 +0100 ---------------------------------------------------------------------- .../ConfigParametersDeprecationYamlTest.java | 203 ++++++++++ .../brooklyn/core/config/BasicConfigKey.java | 20 + .../config/internal/AbstractConfigMapImpl.java | 31 +- .../brooklyn/core/entity/AbstractEntity.java | 12 +- .../entity/internal/ConfigUtilsInternal.java | 108 ++++++ .../core/location/AbstractLocation.java | 11 + .../mgmt/rebind/BasicLocationRebindSupport.java | 10 + .../mgmt/rebind/BasicPolicyRebindSupport.java | 24 +- .../core/objs/AbstractEntityAdjunct.java | 15 + .../brooklyn/core/objs/BasicSpecParameter.java | 2 + .../core/objs/BrooklynTypeSnapshot.java | 24 +- .../brooklyn/util/core/config/ConfigBag.java | 118 +++++- .../util/core/config/ResolvingConfigBag.java | 6 + .../brooklyn/util/core/flags/FlagUtils.java | 4 +- .../config/ConfigKeyDeprecationRebindTest.java | 373 +++++++++++++++++++ .../core/config/ConfigKeyDeprecationTest.java | 328 ++++++++++++++++ .../MapConfigKeyAndFriendsDeprecationTest.java | 124 ++++++ .../internal/ConfigUtilsInternalTest.java | 71 ++++ .../config-deprecated-enricher-j8rvs5fc16 | 36 ++ .../config-deprecated-enricherOwner-sb5w8w5tq0 | 36 ++ .../config/config-deprecated-feed-km6gu420a0 | 30 ++ .../config-deprecated-feedOwner-d8p4p8o4x7 | 36 ++ ...config-deprecated-flagNameOnField-e86eode5yy | 48 +++ ...precated-flagNameOnField-location-f4kj5hxcvx | 35 ++ ...deprecated-flagNameOnField-policy-alq7mtwv0m | 35 ++ ...cated-flagNameOnField-policyOwner-vfncjpljqf | 36 ++ .../config-deprecated-flagNameOnKey-ug77ek2tkd | 36 ++ .../config/config-deprecated-key-dyozzd948m | 48 +++ .../config/config-deprecated-key-pps2ttgijb | 36 ++ .../org/apache/brooklyn/config/ConfigKey.java | 9 + 30 files changed, 1882 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersDeprecationYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersDeprecationYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersDeprecationYamlTest.java new file mode 100644 index 0000000..4999b5f --- /dev/null +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersDeprecationYamlTest.java @@ -0,0 +1,203 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.camp.brooklyn; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; + +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.entity.stock.BasicEntity; +import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; + +public class ConfigParametersDeprecationYamlTest extends AbstractYamlRebindTest { + + private static final Logger LOG = LoggerFactory.getLogger(ConfigParametersDeprecationYamlTest.class); + + @BeforeMethod(alwaysRun=true) + @Override + public void setUp() throws Exception { + super.setUp(); + RecordingSshTool.clear(); + } + + @AfterMethod(alwaysRun=true) + @Override + public void tearDown() throws Exception { + try { + super.tearDown(); + } finally { + RecordingSshTool.clear(); + } + } + + @Test + public void testParameterDefinesDeprecatedNames() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: entity-with-keys", + " item:", + " type: "+BasicEntity.class.getName(), + " brooklyn.parameters:", + " - name: key1", + " deprecatedNames:", + " - oldKey1", + " - oldKey1b", + " description: myDescription", + " type: String", + " default: myDefaultVal"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: entity-with-keys"); + + Entity app = createStartWaitAndLogApplication(yaml); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + + ConfigKey<?> key = findKey(entity, "key1"); + assertEquals(key.getDeprecatedNames(), ImmutableList.of("oldKey1", "oldKey1b")); + + // Rebind, and then check again that the config key is listed + Entity newApp = rebind(); + Entity newEntity = Iterables.getOnlyElement(newApp.getChildren()); + ConfigKey<?> newKey = findKey(newEntity, "key1"); + assertEquals(newKey.getDeprecatedNames(), ImmutableList.of("oldKey1", "oldKey1b")); + } + + @Test + public void testUsingDeprecatedNameInCatalog() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: entity-with-keys", + " item:", + " type: "+BasicEntity.class.getName(), + " brooklyn.parameters:", + " - name: key1", + " deprecatedNames:", + " - oldKey1", + " description: myDescription", + " type: String", + " brooklyn.config:", + " oldKey1: myval"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: entity-with-keys"); + + Entity app = createStartWaitAndLogApplication(yaml); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(findKey(entity, "key1")), "myval"); + + // Rebind, and then check again the config key value + Entity newApp = rebind(); + Entity newEntity = Iterables.getOnlyElement(newApp.getChildren()); + assertEquals(newEntity.config().get(findKey(entity, "key1")), "myval"); + } + + @Test + public void testUsingDeprecatedNameInUsage() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: entity-with-keys", + " item:", + " type: "+BasicEntity.class.getName(), + " brooklyn.parameters:", + " - name: key1", + " deprecatedNames:", + " - oldKey1", + " description: myDescription", + " type: String"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: entity-with-keys", + " brooklyn.config:", + " oldKey1: myval"); + + Entity app = createStartWaitAndLogApplication(yaml); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(findKey(entity, "key1")), "myval"); + + // Rebind, and then check again the config key value + Entity newApp = rebind(); + Entity newEntity = Iterables.getOnlyElement(newApp.getChildren()); + assertEquals(newEntity.config().get(findKey(entity, "key1")), "myval"); + } + + @Test + public void testUsingDeprecatedNameInSubtype() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: entity-with-keys", + " item:", + " type: "+BasicEntity.class.getName(), + " brooklyn.parameters:", + " - name: key1", + " deprecatedNames:", + " - oldKey1", + " description: myDescription", + " type: String"); + + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: sub-entity", + " item:", + " type: entity-with-keys", + " brooklyn.config:", + " oldKey1: myval"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: sub-entity"); + + Entity app = createStartWaitAndLogApplication(yaml); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(findKey(entity, "key1")), "myval"); + + // Rebind, and then check again the config key value + Entity newApp = rebind(); + Entity newEntity = Iterables.getOnlyElement(newApp.getChildren()); + assertEquals(newEntity.config().get(findKey(entity, "key1")), "myval"); + } + + protected ConfigKey<?> findKey(Entity entity, String keyName) { + ConfigKey<?> key = entity.getEntityType().getConfigKey(keyName); + assertNotNull(key, "No key '"+keyName+"'; keys="+entity.getEntityType().getConfigKeys()); + return key; + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java index d3af701..8560361 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java @@ -47,6 +47,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.reflect.TypeToken; @@ -100,6 +101,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab public abstract static class Builder<T, B extends Builder<T,B>> { protected String name; + protected Collection<String> deprecatedNames = ImmutableList.of(); protected TypeToken<T> type; protected String description; protected T defaultValue; @@ -125,6 +127,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab public Builder(String newName, ConfigKey<T> key) { this.type = checkNotNull(key.getTypeToken(), "type"); this.name = checkNotNull(newName, "name"); + this.deprecatedNames = checkNotNull(key.getDeprecatedNames(), "deprecatedNames"); description(key.getDescription()); defaultValue(key.getDefaultValue()); reconfigurable(key.isReconfigurable()); @@ -135,6 +138,12 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab public B name(String val) { this.name = val; return self(); } + public B deprecatedNames(Collection<String> val) { + this.deprecatedNames = val; return self(); + } + public B deprecatedNames(String... val) { + return deprecatedNames(val == null ? ImmutableList.of() : ImmutableList.copyOf(val)); + } public B type(Class<T> val) { this.type = TypeToken.of(val); return self(); } @@ -187,6 +196,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab } protected String name; + protected Collection<String> deprecatedNames; protected TypeToken<T> typeToken; protected Class<? super T> type; protected String description; @@ -237,6 +247,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab public BasicConfigKey(TypeToken<T> type, String name, String description, T defaultValue) { this.description = description; this.name = checkNotNull(name, "name"); + this.deprecatedNames = ImmutableList.of(); this.type = TypeTokens.getRawTypeIfRaw(checkNotNull(type, "type")); this.typeToken = TypeTokens.getTypeTokenIfNotRaw(type); @@ -248,6 +259,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab public BasicConfigKey(Builder<T,?> builder) { this.name = checkNotNull(builder.name, "name"); + this.deprecatedNames = checkNotNull(builder.deprecatedNames, "deprecatedNames"); this.type = TypeTokens.getRawTypeIfRaw(checkNotNull(builder.type, "type")); this.typeToken = TypeTokens.getTypeTokenIfNotRaw(builder.type); this.description = builder.description; @@ -264,6 +276,13 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab /** @see ConfigKey#getName() */ @Override public String getName() { return name; } + /** @see ConfigKey#getDeprecatedNames() */ + @Override public Collection<String> getDeprecatedNames() { + // check for null, for backwards compatibility of serialized state + if (deprecatedNames == null) deprecatedNames = ImmutableList.of(); + return deprecatedNames; + } + /** @see ConfigKey#getTypeName() */ @Override public String getTypeName() { return getType().getName(); } @@ -365,6 +384,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab } /** @see ConfigKey#getNameParts() */ + @Deprecated @Override public Collection<String> getNameParts() { return Lists.newArrayList(dots.split(name)); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java index 091f874..bc4916b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java @@ -226,7 +226,19 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i @Override @Deprecated public Maybe<Object> getConfigRaw(ConfigKey<?> key, boolean includeInherited) { // does not currently respect inheritance modes - if (ownConfig.containsKey(key)) return Maybe.of(ownConfig.get(key)); + if (ownConfig.containsKey(key)) { + return Maybe.of(ownConfig.get(key)); + } + for (String deprecatedName : key.getDeprecatedNames()) { + // Unfortunately `config.putAll(map.of(string, val))` (for dynamic config keys, + // i.e. where the key is not pre-defined on the entity). Unfortunately that + // means subsequent lookup must synthesise keys for each deprecated name. + ConfigKey<?> deprecatedKey = ConfigKeys.newConfigKeyRenamed(deprecatedName, key); + if (ownConfig.containsKey(deprecatedKey)) { + LOG.warn("Retrieving value with deprecated config key name '"+deprecatedName+"' for key "+key); + return Maybe.of(ownConfig.get(deprecatedKey)); + } + } if (!includeInherited || getParent()==null) return Maybe.absent(); return getParentInternal().config().getInternalConfigMap().getConfigRaw(key, includeInherited); } @@ -305,6 +317,23 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i * <p> * this does not do any resolution with respect to ancestors. */ protected Maybe<Object> resolveRawValueFromContainer(TContainer container, ConfigKey<?> key, Maybe<Object> value) { + Maybe<Object> result = resolveRawValueFromContainerIgnoringDeprecatedNames(container, key, value); + if (result.isPresent()) return result; + + // See AbstractconfigMapImpl.getConfigRaw(ConfigKey<?> key, boolean includeInherited) for how/why it + // handles deprecated names + for (String deprecatedName : key.getDeprecatedNames()) { + ConfigKey<?> deprecatedKey = ConfigKeys.newConfigKeyRenamed(deprecatedName, key); + result = resolveRawValueFromContainerIgnoringDeprecatedNames(container, deprecatedKey, value); + if (result.isPresent()) { + LOG.warn("Retrieving value with deprecated config key name '"+deprecatedName+"' for key "+key); + return result; + } + } + return result; + } + + private Maybe<Object> resolveRawValueFromContainerIgnoringDeprecatedNames(TContainer container, ConfigKey<?> key, Maybe<Object> value) { Map<ConfigKey<?>, Object> oc = ((AbstractConfigMapImpl<?>) ((BrooklynObjectInternal)container).config().getInternalConfigMap()).ownConfig; if (key instanceof ConfigKeySelfExtracting) { if (((ConfigKeySelfExtracting<?>)key).isSet(oc)) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java index 9935633..d0d3873 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java @@ -64,6 +64,7 @@ import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.config.render.RendererHints; import org.apache.brooklyn.core.enricher.AbstractEnricher; +import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; import org.apache.brooklyn.core.entity.internal.EntityConfigMap; import org.apache.brooklyn.core.entity.lifecycle.PolicyDescriptor; import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic; @@ -393,11 +394,16 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E // shouldn't be used; CAMP parser leaves it as a top-level attribute which is converted to a tag tags().addTag(BrooklynTags.newIconUrlTag((String) flags.remove(BrooklynConfigKeys.ICON_URL.getName()))); } - + + // allow config keys to be set by name (or deprecated name) + flags = ConfigUtilsInternal.setAllConfigKeys(flags, getEntityType().getConfigKeys(), this); + // allow config keys, and fields, to be set from these flags if they have a SetFromFlag annotation // TODO the default values on flags are not used? (we should remove that support, since ConfigKeys gives a better way) - FlagUtils.setFieldsFromFlags(flags, this); - flags = FlagUtils.setAllConfigKeys(flags, this, false); + if (flags.size() > 0) { + FlagUtils.setFieldsFromFlags(flags, this); + flags = FlagUtils.setAllConfigKeys(flags, this, false); + } // finally all config keys specified in map should be set as config // TODO use a config bag and remove the ones set above in the code below http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/entity/internal/ConfigUtilsInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/internal/ConfigUtilsInternal.java b/core/src/main/java/org/apache/brooklyn/core/entity/internal/ConfigUtilsInternal.java new file mode 100644 index 0000000..fb3a0e0 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/entity/internal/ConfigUtilsInternal.java @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.entity.internal; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.brooklyn.api.objs.Configurable; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.guava.Maybe; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Iterables; + +/** + * Internal utility methods for getting/setting config, such that it handles any deprecated names + * defined in {@link ConfigKey#getDeprecatedNames()}. + */ +public class ConfigUtilsInternal { + + private static final Logger LOG = LoggerFactory.getLogger(ConfigUtilsInternal.class); + + public static Map<?,?> setAllConfigKeys(Map<?,?> flags, Iterable<? extends ConfigKey<?>> configKeys, Configurable obj) { + Map<?,?> unusedFlags = MutableMap.copyOf(flags); + for (ConfigKey<?> key : configKeys) { + ConfigValue values = getValue(unusedFlags, key); + Maybe<Object> valueToUse = values.preferredValue(); + if (valueToUse.isPresent()) { + setValue(obj, key, valueToUse.get()); + values.logIfDeprecatedValue(obj, key); + } + } + return unusedFlags; + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static void setValue(Configurable obj, ConfigKey<?> key, Object val) { + obj.config().set((ConfigKey)key, val); + } + + private static ConfigValue getValue(Map<?,?> flags, ConfigKey<?> key) { + Maybe<Object> val; + String keyName = key.getName(); + if (flags.containsKey(keyName)) { + val = Maybe.of(flags.get(keyName)); + flags.remove(keyName); + } else { + val = Maybe.absent(); + } + Map<String, Object> deprecatedValues = new LinkedHashMap<>(key.getDeprecatedNames().size()); + for (String deprecatedName : key.getDeprecatedNames()) { + if (flags.containsKey(deprecatedName)) { + deprecatedValues.put(deprecatedName, flags.get(deprecatedName)); + flags.remove(deprecatedName); + } + } + return new ConfigValue(val, deprecatedValues); + } + + private static class ConfigValue { + final Maybe<Object> val; + final Map<String, Object> deprecatedValues; + + ConfigValue(Maybe<Object> val, Map<String, Object> deprecatedValues) { + this.val = val; + this.deprecatedValues = deprecatedValues; + } + + Maybe<Object> preferredValue() { + if (val.isPresent()) return val; + return (deprecatedValues.isEmpty()) ? Maybe.absent() : Maybe.of(Iterables.get(deprecatedValues.values(), 0)); + } + + void logIfDeprecatedValue(Configurable obj, ConfigKey<?> key) { + if (deprecatedValues.isEmpty()) return; + + if (val.isPresent()) { + LOG.warn("Ignoring deprecated config value(s) on "+obj+" because contains value for " + +"'"+key.getName()+"', other deprecated name(s) present were: "+deprecatedValues.keySet()); + } else if (deprecatedValues.size() == 1) { + LOG.warn("Using deprecated config value on "+obj+", should use '"+key.getName()+"', but used " + +"'"+Iterables.getOnlyElement(deprecatedValues.keySet())+"'"); + } else { + LOG.warn("Using deprecated config value on "+obj+", should use '"+key.getName()+"', but used " + +"'"+Iterables.get(deprecatedValues.keySet(), 1)+"' and ignored values present for other " + +"deprecated name(s) "+Iterables.skip(deprecatedValues.keySet(), 1)); + } + } + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java index 268d2a4..58b2809 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java @@ -47,6 +47,7 @@ import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; +import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; import org.apache.brooklyn.core.internal.storage.BrooklynStorage; import org.apache.brooklyn.core.internal.storage.Reference; import org.apache.brooklyn.core.internal.storage.impl.BasicReference; @@ -215,6 +216,16 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements config().removeKey(PARENT_LOCATION); } + // allow config keys to be set by name (or deprecated name) + // + // Aled thinks it would be sensible to remove the consumed flags below (i.e. properties = ...). + // However, that caused ClockerDynamicLocationPatternTest to fail because there is a field of + // StubContainerLocation annotated with `@SetFromFlag("owner")`, as well as a config key with + // name "owner" (and with `@SetFromFlag("owner")`) in the super-type (DynamicLocation). + // However, that looks mad - do we really need to support it?! + // I've preserved that behaviour (for now). + ConfigUtilsInternal.setAllConfigKeys(properties, getLocationTypeInternal().getConfigKeys().values(), this); + // NB: flag-setting done here must also be done in BasicLocationRebindSupport ConfigBag configBag = ConfigBag.newInstance(properties); FlagUtils.setFieldsFromFlagsWithBag(this, properties, configBag, firstTime); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java index e603395..072f2c8 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java @@ -19,6 +19,7 @@ package org.apache.brooklyn.core.mgmt.rebind; import java.lang.reflect.Field; +import java.util.Collection; import java.util.Collections; import java.util.Map; import java.util.NoSuchElementException; @@ -27,6 +28,7 @@ import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.mgmt.rebind.RebindContext; import org.apache.brooklyn.api.mgmt.rebind.mementos.LocationMemento; import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; import org.apache.brooklyn.core.location.AbstractLocation; import org.apache.brooklyn.core.mgmt.rebind.dto.MementosGenerators; import org.apache.brooklyn.util.collections.MutableMap; @@ -70,10 +72,18 @@ public class BasicLocationRebindSupport extends AbstractBrooklynObjectRebindSupp // Note that the flags have been set in the constructor // Sept 2016 - now ignores unused and config description + Collection<ConfigKey<?>> configKeys = location.getLocationTypeInternal().getConfigKeys().values(); + for (Map.Entry<String, Object> entry : memento.getLocationConfig().entrySet()) { String flagName = entry.getKey(); Object value = entry.getValue(); + Map<?, ?> unused = ConfigUtilsInternal.setAllConfigKeys(MutableMap.of(flagName, value), configKeys, location); + if (unused.isEmpty()) { + // Config key was known explicitly; don't need to iterate over all the fields yet again! + continue; + } + Field field; try { field = FlagUtils.findFieldForFlag(flagName, location); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicPolicyRebindSupport.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicPolicyRebindSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicPolicyRebindSupport.java index a52df3a..6c06d00 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicPolicyRebindSupport.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicPolicyRebindSupport.java @@ -18,8 +18,13 @@ */ package org.apache.brooklyn.core.mgmt.rebind; +import java.util.Collection; +import java.util.Map; + import org.apache.brooklyn.api.mgmt.rebind.RebindContext; import org.apache.brooklyn.api.mgmt.rebind.mementos.PolicyMemento; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; import org.apache.brooklyn.core.policy.AbstractPolicy; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.FlagUtils; @@ -37,11 +42,20 @@ public class BasicPolicyRebindSupport extends AbstractBrooklynObjectRebindSuppor protected void addConfig(RebindContext rebindContext, PolicyMemento memento) { // TODO entity does config-lookup differently; the memento contains the config keys. // BasicEntityMemento.postDeserialize uses the injectTypeClass to call EntityTypes.getDefinedConfigKeys(clazz) - // - // Note that the flags may have been set in the constructor; but some policies have no-arg constructors - ConfigBag configBag = ConfigBag.newInstance(memento.getConfig()); - FlagUtils.setFieldsFromFlags(policy, configBag); - FlagUtils.setAllConfigKeys(policy, configBag, false); + + Collection<ConfigKey<?>> configKeys = policy.getAdjunctType().getConfigKeys(); + + Map<?, ?> flags = memento.getConfig(); + + // First set the config keys that are known explicitly (including with deprecated names). + flags = ConfigUtilsInternal.setAllConfigKeys(flags, configKeys, policy); + + // Note that the flags may have been set in the constructor; but non-legacy policies should have no-arg constructors + if (!flags.isEmpty()) { + ConfigBag configBag = ConfigBag.newInstance(flags); + FlagUtils.setFieldsFromFlags(policy, configBag); + FlagUtils.setAllConfigKeys(policy, configBag, false); + } } @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java index 69e51e6..7395d0a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java @@ -40,12 +40,14 @@ import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.api.sensor.Sensor; import org.apache.brooklyn.api.sensor.SensorEventListener; import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityInternal; +import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; import org.apache.brooklyn.core.mgmt.internal.SubscriptionTracker; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.FlagUtils; @@ -151,6 +153,19 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple } } + // Allow config keys to be set by name (or deprecated name). + // + // Aled thinks it would be sensible to remove the consumed flags below (i.e. flags = ...). + // However, that causes PolicyConfigTest.testConfigFlagsPassedInAtConstructionIsAvailable to fail. + // However, tht looks mad - do we really need to support it?! + // The policy is defined with one key using a name in SetFromFlag, and another key using the same name. + // It expects both of the config keys to have been set. + // @SetFromFlag("strKey") + // public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key"); + // public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default"); + // I've preserved that behaviour (for now). + ConfigUtilsInternal.setAllConfigKeys(flags, getAdjunctType().getConfigKeys(), this); + ConfigBag bag = new ConfigBag().putAll(flags); FlagUtils.setFieldsFromFlags(this, bag, isFirstTime); FlagUtils.setAllConfigKeys(this, bag, false); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/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 c890719..7c1bde0 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 @@ -253,6 +253,7 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ throw new IllegalArgumentException("Catalog input definition expected to be a map, but is " + obj.getClass() + " instead: " + obj); } String name = (String)inputDef.get("name"); + Collection<String> deprecatedNames = (Collection<String>)inputDef.get("deprecatedNames"); String label = (String)inputDef.get("label"); String description = (String)inputDef.get("description"); String type = (String)inputDef.get("type"); @@ -296,6 +297,7 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ Builder builder = BasicConfigKey.builder(typeToken) .name(name) + .deprecatedNames((deprecatedNames == null) ? ImmutableList.of() : deprecatedNames) .description(description) .defaultValue(immutableDefaultValue) .constraint(constraint) http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynTypeSnapshot.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynTypeSnapshot.java b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynTypeSnapshot.java index 77fcb11..904b3c6 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynTypeSnapshot.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynTypeSnapshot.java @@ -18,12 +18,15 @@ */ package org.apache.brooklyn.core.objs; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import org.apache.brooklyn.api.objs.BrooklynType; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.util.text.Strings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects.ToStringHelper; @@ -34,15 +37,32 @@ import com.google.common.collect.ImmutableSet; public class BrooklynTypeSnapshot implements BrooklynType { private static final long serialVersionUID = 4670930188951106009L; + private static final Logger LOG = LoggerFactory.getLogger(BrooklynTypeSnapshot.class); + private final String name; private transient volatile String simpleName; private final Map<String, ConfigKey<?>> configKeys; + private final Map<String, ConfigKey<?>> configKeysByDeprecatedName; + private final Set<ConfigKey<?>> configKeysSet; protected BrooklynTypeSnapshot(String name, Map<String, ConfigKey<?>> configKeys) { this.name = name; this.configKeys = ImmutableMap.copyOf(configKeys); this.configKeysSet = ImmutableSet.copyOf(this.configKeys.values()); + + this.configKeysByDeprecatedName = new LinkedHashMap<>(); + for (ConfigKey<?> key : configKeysSet) { + for (String deprecatedName : key.getDeprecatedNames()) { + if (configKeys.containsKey(deprecatedName)) { + LOG.warn("Conflicting config key name '"+deprecatedName+"' used in "+configKeys.get(deprecatedName)+" and as deprecated name of "+key+"; will prefer "+key+" but may cause problems"); + } else if (configKeysByDeprecatedName.containsKey(deprecatedName)) { + LOG.warn("Conflicting config key name '"+deprecatedName+"' used as deprecated name in both "+configKeysByDeprecatedName.get(deprecatedName)+" and "+key+"; may cause problems"); + } else { + configKeysByDeprecatedName.put(deprecatedName, key); + } + } + } } @Override @@ -73,7 +93,9 @@ public class BrooklynTypeSnapshot implements BrooklynType { @Override public ConfigKey<?> getConfigKey(String name) { - return configKeys.get(name); + ConfigKey<?> result = configKeys.get(name); + if (result == null) result = configKeysByDeprecatedName.get(name); + return result; } @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java b/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java index f34d881..cfef3b0 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java @@ -330,7 +330,14 @@ public class ConfigBag { } public boolean containsKey(ConfigKey<?> key) { - return containsKey(key.getName()); + boolean found = containsKey(key.getName()); + if (!found) { + for (String deprecatedName : key.getDeprecatedNames()) { + found = containsKey(deprecatedName); + if (found) break; + } + } + return found; } public synchronized boolean containsKey(String key) { @@ -357,8 +364,9 @@ public class ConfigBag { @Beta public Maybe<Object> getObjKeyMaybe(Object key) { if (key instanceof HasConfigKey<?>) key = ((HasConfigKey<?>)key).getConfigKey(); - if (key instanceof ConfigKey<?>) key = ((ConfigKey<?>)key).getName(); - if (key instanceof String) { + if (key instanceof ConfigKey<?>) { + return getKeyMaybe((ConfigKey<?>)key, true); + } else if (key instanceof String) { return getStringKeyMaybe((String)key, true); } else { logInvalidKey(key); @@ -370,8 +378,9 @@ public class ConfigBag { @Beta private Maybe<Object> getRawObjKeyMaybe(Object key) { if (key instanceof HasConfigKey<?>) key = ((HasConfigKey<?>)key).getConfigKey(); - if (key instanceof ConfigKey<?>) key = ((ConfigKey<?>)key).getName(); - if (key instanceof String) { + if (key instanceof ConfigKey<?>) { + return this.getRawKeyMaybe((ConfigKey<?>)key, true); + } else if (key instanceof String) { return this.getRawStringKeyMaybe((String)key, true); } else { logInvalidKey(key); @@ -398,7 +407,7 @@ public class ConfigBag { @Beta @SuppressWarnings("unchecked") public <T> ConfigBag copyKeyAs(ConfigBag source, ConfigKey<T> sourceKey, ConfigKey<T> targetKey) { - Maybe<Object> sourceValue = source.getRawObjKeyMaybe(sourceKey); + Maybe<Object> sourceValue = source.getRawKeyMaybe(sourceKey, true); if (sourceValue.isPresent()) { put(targetKey, (T) sourceValue.get()); } @@ -421,14 +430,22 @@ public class ConfigBag { return get(preferredKey); } - /** convenience for @see #getWithDeprecation(ConfigKey[], ConfigKey...) */ + /** + * Convenience for @see #getWithDeprecation(ConfigKey[], ConfigKey...). + * + * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()} + */ public Object getWithDeprecation(ConfigKey<?> key, ConfigKey<?> ...deprecatedKeys) { return getWithDeprecation(new ConfigKey[] { key }, deprecatedKeys); } - /** returns the value for the first key in the list for which a value is set, + /** + * Returns the value for the first key in the list for which a value is set, * warning if any of the deprecated keys have a value which is different to that set on the first set current key - * (including warning if a deprecated key has a value but no current key does) */ + * (including warning if a deprecated key has a value but no current key does). + * + * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()} + */ public synchronized Object getWithDeprecation(ConfigKey<?>[] currentKeysInOrderOfPreference, ConfigKey<?> ...deprecatedKeys) { // Get preferred key (or null) ConfigKey<?> preferredKeyProvidingValue = null; @@ -492,8 +509,9 @@ public class ConfigBag { protected <T> T get(ConfigKey<T> key, boolean markUsed) { // TODO for now, no evaluation -- maps / closure content / other smart (self-extracting) keys are NOT supported // (need a clean way to inject that behaviour, as well as desired TypeCoercions) - // this method, and the coercion, is not synchronized, nor does it need to be, because the "get" is synchronized. - return coerceFirstNonNullKeyValue(key, getStringKey(key.getName(), markUsed)); + // this method, and the coercion, is not synchronized, nor does it need to be, because the "get" is synchronized. + Maybe<Object> val = getKeyMaybe(key, markUsed); + return coerceFirstNonNullKeyValue(key, val.orNull()); } /** returns the first non-null value to be the type indicated by the key, or the keys default value if no non-null values are supplied */ @@ -507,6 +525,44 @@ public class ConfigBag { return getStringKeyMaybe(key, markUsed).orNull(); } + private synchronized Maybe<Object> getRawKeyMaybe(ConfigKey<?> key, boolean markUsed) { + Maybe<Object> val = getRawStringKeyMaybe(key.getName(), markUsed); + + String firstDeprecatedName = null; + Maybe<Object> firstDeprecatedVal = null; + for (String deprecatedName : key.getDeprecatedNames()) { + Maybe<Object> deprecatedVal = getRawStringKeyMaybe(deprecatedName, markUsed); + if (deprecatedVal.isPresent()) { + if (val.isPresent()) { + if (!Objects.equal(val.get(), deprecatedVal.get())) { + log.warn("Conflicting value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using value from preferred name "+key.getName()); + } else { + log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using same value from preferred name "+key.getName()); + } + } else if (firstDeprecatedVal.isPresent()) { + if (!Objects.equal(firstDeprecatedVal.get(), deprecatedVal.get())) { + log.warn("Conflicting value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using earlier deprecated name "+firstDeprecatedName); + } else { + log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using same value from earlier depreated name "+key.getName()); + } + } else { + // new value, from deprecated name + log.warn("Value for key "+key+" found with deprecated name '"+deprecatedName+"'; " + + "recommend changing to preferred name '"+key.getName()+"'; this will not be supported in future versions"); + firstDeprecatedName = deprecatedName; + firstDeprecatedVal = deprecatedVal; + } + } + } + + + return val.isPresent() ? val : (firstDeprecatedVal != null && firstDeprecatedVal.isPresent() ? firstDeprecatedVal : val); + } + private synchronized Maybe<Object> getRawStringKeyMaybe(String key, boolean markUsed) { if (config.containsKey(key)) { if (markUsed) markUsed(key); @@ -518,6 +574,46 @@ public class ConfigBag { /** * @return Unresolved configuration value. May be overridden to provide resolution - @see {@link ResolvingConfigBag#getStringKeyMaybe(String, boolean)} */ + protected synchronized Maybe<Object> getKeyMaybe(ConfigKey<?> key, boolean markUsed) { + Maybe<Object> val = getStringKeyMaybe(key.getName(), markUsed); + + String firstDeprecatedName = null; + Maybe<Object> firstDeprecatedVal = null; + for (String deprecatedName : key.getDeprecatedNames()) { + Maybe<Object> deprecatedVal = getStringKeyMaybe(deprecatedName, markUsed); + if (deprecatedVal.isPresent()) { + if (val.isPresent()) { + if (!Objects.equal(val.get(), deprecatedVal.get())) { + log.warn("Conflicting value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using value from preferred name "+key.getName()); + } else { + log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using same value from preferred name "+key.getName()); + } + } else if (firstDeprecatedVal != null) { + if (!Objects.equal(firstDeprecatedVal.get(), deprecatedVal.get())) { + log.warn("Conflicting value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using earlier deprecated name "+firstDeprecatedName); + } else { + log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; " + + "using same value from earlier depreated name "+key.getName()); + } + } else { + // new value, from deprecated name + log.warn("Value for key "+key+" found with deprecated name '"+deprecatedName+"'; " + + "recommend changing to preferred name '"+key.getName()+"'; this will not be supported in future versions"); + firstDeprecatedName = deprecatedName; + firstDeprecatedVal = deprecatedVal; + } + } + } + + return val.isPresent() ? val : (firstDeprecatedVal != null && firstDeprecatedVal.isPresent() ? firstDeprecatedVal : val); + } + + /** + * @return Unresolved configuration value. May be overridden to provide resolution - @see {@link ResolvingConfigBag#getStringKeyMaybe(String, boolean)} + */ protected synchronized Maybe<Object> getStringKeyMaybe(String key, boolean markUsed) { return getRawStringKeyMaybe(key, markUsed); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java b/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java index 53af175..5dcd90d 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java @@ -164,6 +164,12 @@ public class ResolvingConfigBag extends ConfigBag { } @Override + protected synchronized Maybe<Object> getKeyMaybe(ConfigKey<?> key, boolean markUsed) { + Maybe<Object> result = super.getKeyMaybe(key, markUsed); + return (result.isPresent()) ? Maybe.of(getTransformer().apply(result.get())) : result; + } + + @Override public Map<String,Object> getAllConfigMutable() { throw new UnsupportedOperationException(); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/main/java/org/apache/brooklyn/util/core/flags/FlagUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/flags/FlagUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/flags/FlagUtils.java index d160a93..27b75cd 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/flags/FlagUtils.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/flags/FlagUtils.java @@ -272,7 +272,7 @@ public class FlagUtils { FlagConfigKeyAndValueRecord result = new FlagConfigKeyAndValueRecord(); result.configKey = key; if (key!=null && input.containsKey(key)) - result.configKeyValue = Maybe.<Object>of(input.getStringKey(key.getName())); + result.configKeyValue = input.getObjKeyMaybe(key); if (f != null) { SetFromFlag flag = f.getAnnotation(SetFromFlag.class); if (flag!=null) { @@ -408,7 +408,7 @@ public class FlagUtils { // first check whether it is a key ConfigKey<?> key = getFieldAsConfigKey(o, f); if (key!=null && bag.containsKey(key)) { - Object uncoercedValue = bag.getStringKey(key.getName()); + Object uncoercedValue = bag.getObjKeyMaybe(key).get(); setField(o, f, uncoercedValue, optionalAnnotation); return; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java new file mode 100644 index 0000000..b20f574 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java @@ -0,0 +1,373 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.config; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; + +import org.apache.brooklyn.api.entity.Application; +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.api.entity.EntitySpec; +import org.apache.brooklyn.api.entity.ImplementedBy; +import org.apache.brooklyn.api.objs.BrooklynObjectType; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.enricher.AbstractEnricher; +import org.apache.brooklyn.core.entity.AbstractApplication; +import org.apache.brooklyn.core.entity.EntityInternal; +import org.apache.brooklyn.core.feed.AbstractFeed; +import org.apache.brooklyn.core.location.AbstractLocation; +import org.apache.brooklyn.core.mgmt.rebind.AbstractRebindHistoricTest; +import org.apache.brooklyn.core.policy.AbstractPolicy; +import org.testng.annotations.Test; + +import com.google.common.base.Joiner; +import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; + +public class ConfigKeyDeprecationRebindTest extends AbstractRebindHistoricTest { + + @Test + public void testUsingDeprecatedName() throws Exception { + MyApp entity = app().addChild(EntitySpec.create(MyApp.class) + .configure("oldKey1", "myval")); + assertEquals(entity.config().get(MyApp.KEY_1), "myval"); + + rebind(); + + Entity newEntity = mgmt().getEntityManager().getEntity(entity.getId()); + assertEquals(newEntity.config().get(MyApp.KEY_1), "myval"); + + // Expect the persisted state of the entity to have used the non-deprecated name. + String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, newEntity.getId()); + assertFalse(allLines.contains("oldKey1"), "contains 'oldKey1', allLines="+allLines); + assertTrue(allLines.contains("key1"), "contains 'key1', allLines="+allLines); + } + + /** + * Created with: + * <pre> + * {@code + * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class) + * .configure("oldKey1", "myval")); + * } + * </pre> + */ + @Test + public void testEntityPersistedWithDeprecatedKeyName() throws Exception { + String appId = "pps2ttgijb"; + addMemento(BrooklynObjectType.ENTITY, "config-deprecated-key", appId); + rebind(); + + Entity newApp = mgmt().getEntityManager().getEntity(appId); + assertEquals(newApp.config().get(MyApp.KEY_1), "myval"); + + // Expect the persisted state to have been re-written with the new key value. + switchOriginalToNewManagementContext(); + rebind(); + + String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, appId); + assertFalse(allLines.contains("oldKey1"), "should not contain 'oldKey1', allLines="+allLines); + assertTrue(allLines.contains("<key1>"), "should contain '<key1>', allLines="+allLines); + } + + /** + * Created with: + * <pre> + * {@code + * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class) + * .configure("oldFlagKey2", "myval")); + * } + * </pre> + */ + @Test + public void testEntityPersistedWithSetFromFlagNameOnKey() throws Exception { + String appId = "ug77ek2tkd"; + addMemento(BrooklynObjectType.ENTITY, "config-deprecated-flagNameOnKey", appId); + rebind(); + + Entity newApp = mgmt().getEntityManager().getEntity(appId); + assertEquals(newApp.config().get(MyApp.KEY_2), "myval"); + + // Expect the persisted state to have been re-written with the new key value. + switchOriginalToNewManagementContext(); + rebind(); + + String allLines = getPersistanceFileContents(BrooklynObjectType.ENTITY, appId); + assertFalse(allLines.contains("oldFlagKey2"), "should not contain 'oldFlagKey2', allLines="+allLines); + assertTrue(allLines.contains("<key2>"), "should contain '<key1>', allLines="+allLines); + } + + /** + * Created with: + * <pre> + * {@code + * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class) + * .policy(PolicySpec.create(MyPolicy.class) + * .configure("field1", "myval"))); + * } + * </pre> + */ + @Test + public void testPolicyPersistedWithSetFromFlagNameOnField() throws Exception { + String appId = "vfncjpljqf"; + String policyId = "alq7mtwv0m"; + addMemento(BrooklynObjectType.ENTITY, "config-deprecated-flagNameOnField-policyOwner", appId); + addMemento(BrooklynObjectType.POLICY, "config-deprecated-flagNameOnField-policy", policyId); + rebind(); + + MyApp newApp = (MyApp) mgmt().getEntityManager().getEntity(appId); + MyPolicy newPolicy = (MyPolicy) Iterables.find(newApp.policies(), Predicates.instanceOf(MyPolicy.class)); + assertEquals(newPolicy.getField1(), "myval"); + assertEquals(newPolicy.config().get(MyPolicy.REPLACEMENT_FOR_FIELD_1), "myval"); + + // Expect the persisted state to have been re-written with the new key value. + switchOriginalToNewManagementContext(); + rebind(); + + String allLines = getPersistanceFileContents(BrooklynObjectType.POLICY, policyId); + assertFalse(allLines.contains("<field1>"), "should not contain '<field1>', allLines="+allLines); + assertTrue(allLines.contains("<replacementForField1>"), "should contain '<replacementForField1>', allLines="+allLines); + } + + /** + * Created with: + * <pre> + * {@code + * Entity app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class) + * .enricher(EnricherSpec.create(MyEnricher.class) + * .configure("oldKey1", "myval1") + * .configure("field1", "myval2"))); + * } + * </pre> + */ + @Test + public void testEnricherPersisted() throws Exception { + String appId = "sb5w8w5tq0"; + String enricherId = "j8rvs5fc16"; + addMemento(BrooklynObjectType.ENTITY, "config-deprecated-enricherOwner", appId); + addMemento(BrooklynObjectType.ENRICHER, "config-deprecated-enricher", enricherId); + rebind(); + + MyApp newApp = (MyApp) mgmt().getEntityManager().getEntity(appId); + MyEnricher newEnricher = (MyEnricher) Iterables.find(newApp.enrichers(), Predicates.instanceOf(MyEnricher.class)); + assertEquals(newEnricher.config().get(MyEnricher.KEY_1), "myval1"); + assertEquals(newEnricher.getField1(), "myval2"); + assertEquals(newEnricher.config().get(MyEnricher.REPLACEMENT_FOR_FIELD_1), "myval2"); + + // Expect the persisted state to have been re-written with the new key value. + switchOriginalToNewManagementContext(); + rebind(); + + String allLines = getPersistanceFileContents(BrooklynObjectType.ENRICHER, enricherId); + assertFalse(allLines.contains("<field1>"), "should not contain '<field1>', allLines="+allLines); + assertFalse(allLines.contains("<oldKey1>"), "should not contain '<oldKey1>', allLines="+allLines); + assertTrue(allLines.contains("<key1>"), "should contain '<key1>', allLines="+allLines); + assertTrue(allLines.contains("<replacementForField1>"), "should contain '<replacementForField1>', allLines="+allLines); + } + + /** + * Created with: + * <pre> + * {@code + * MyApp app = mgmt().getEntityManager().createEntity(EntitySpec.create(MyApp.class)); + * MyFeed feed = app.feeds().add(new MyFeed()); + * feed.config().set(MyFeed.KEY_1, "myval1"); + * feed.config().set(ImmutableMap.of("oldKey2", "myval2")); + * } + * </pre> + */ + @Test + public void testFeedPersisted() throws Exception { + String appId = "d8p4p8o4x7"; + String feedId = "km6gu420a0"; + addMemento(BrooklynObjectType.ENTITY, "config-deprecated-feedOwner", appId); + addMemento(BrooklynObjectType.FEED, "config-deprecated-feed", feedId); + rebind(); + + MyApp newApp = (MyApp) mgmt().getEntityManager().getEntity(appId); + MyFeed newFeed = (MyFeed) Iterables.find(newApp.feeds().getFeeds(), Predicates.instanceOf(MyFeed.class)); + assertEquals(newFeed.config().get(MyFeed.KEY_1), "myval1"); + assertEquals(newFeed.config().get(MyFeed.KEY_2), "myval2"); + + // Expect the persisted state to have been re-written with the new key value. + switchOriginalToNewManagementContext(); + rebind(); + + String allLines = getPersistanceFileContents(BrooklynObjectType.FEED, feedId); + assertFalse(allLines.contains("<oldKey1>"), "should not contain '<oldKey1>', allLines="+allLines); + assertFalse(allLines.contains("<oldKey2>"), "should not contain '<oldKey2>', allLines="+allLines); + assertTrue(allLines.contains("<key1>"), "should contain '<key1>', allLines="+allLines); + assertTrue(allLines.contains("<key2>"), "should contain '<key2>', allLines="+allLines); + } + + /** + * Created with: + * <pre> + * {@code + * MyLocation loc = mgmt().getLocationManager().createLocation(LocationSpec.create(MyLocation.class) + * .configure("field1", "myval")); + * } + * </pre> + */ + @Test + public void testLocationPersistedWithSetFromFlagNameOnField() throws Exception { + String locId = "f4kj5hxcvx"; + addMemento(BrooklynObjectType.LOCATION, "config-deprecated-flagNameOnField-location", locId); + rebind(); + + MyLocation newLoc = (MyLocation) mgmt().getLocationManager().getLocation(locId); + assertEquals(newLoc.getField1(), "myval"); + assertEquals(newLoc.config().get(MyLocation.REPLACEMENT_FOR_FIELD_1), "myval"); + + // Expect the persisted state to have been re-written with the new key value. + switchOriginalToNewManagementContext(); + rebind(); + + String allLines = getPersistanceFileContents(BrooklynObjectType.LOCATION, locId); + assertFalse(allLines.contains("<field1>"), "should not contain '<field1>', allLines="+allLines); + assertTrue(allLines.contains("<replacementForField1>"), "should contain '<replacementForField1>', allLines="+allLines); + } + + protected String getPersistanceFileContents(BrooklynObjectType type, String id) throws Exception { + File file = getPersistanceFile(type, id); + List<String> lines = Files.readAllLines(file.toPath(), StandardCharsets.UTF_8); + return Joiner.on("\n").join(lines); + } + + /** + * Interface previously had: + * <pre> + * {@code + * ConfigKey<String> KEY_1 = ConfigKeys.newStringConfigKey("oldKey1"); + * + * @SetFromFlag("oldFlagKey2") + * ConfigKey<String> KEY_2 = ConfigKeys.newStringConfigKey("key2"); + * } + * </pre> + */ + @ImplementedBy(MyAppImpl.class) + public interface MyApp extends Application, EntityInternal { + ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1") + .build(); + + ConfigKey<String> KEY_2 = ConfigKeys.builder(String.class, "key2") + .deprecatedNames("oldFlagKey2") + .build(); + } + + public static class MyAppImpl extends AbstractApplication implements MyApp { + @Override + protected void initEnrichers() { + // no-op; no standard enrichers to keep state small and simple + } + } + + /** + * Class previously had: + * <pre> + * {@code + * @SetFromFlag("field1") + * private String field1; + * } + * </pre> + */ + public static class MyLocation extends AbstractLocation { + public static final ConfigKey<String> REPLACEMENT_FOR_FIELD_1 = ConfigKeys.builder(String.class, "replacementForField1") + .deprecatedNames("field1") + .build(); + + public String getField1() { + return config().get(REPLACEMENT_FOR_FIELD_1); + } + } + + /** + * Class previously had: + * <pre> + * {@code + * @SetFromFlag("field1") + * private String field1; + * } + * </pre> + */ + public static class MyPolicy extends AbstractPolicy { + public static final ConfigKey<String> REPLACEMENT_FOR_FIELD_1 = ConfigKeys.builder(String.class, "replacementForField1") + .deprecatedNames("field1") + .build(); + + + public String getField1() { + return config().get(REPLACEMENT_FOR_FIELD_1); + } + } + + /** + * Class previously had: + * <pre> + * {@code + * public static final ConfigKey<String> KEY_1 = ConfigKeys.newStringConfigKey("oldKey1"); + * @SetFromFlag("field1") + * private String field1; + * } + * </pre> + */ + public static class MyEnricher extends AbstractEnricher { + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1") + .build(); + + public static final ConfigKey<String> REPLACEMENT_FOR_FIELD_1 = ConfigKeys.builder(String.class, "replacementForField1") + .deprecatedNames("field1") + .build(); + + public String getField1() { + return config().get(REPLACEMENT_FOR_FIELD_1); + } + + public String getKey1() { + return config().get(KEY_1); + } + } + /** + * Class previously had: + * <pre> + * {@code + * public static final ConfigKey<String> KEY_1 = ConfigKeys.newStringConfigKey("oldKey1"); + * @SetFromFlag("oldKey2") + * public static final ConfigKey<String> KEY_2 = ConfigKeys.newStringConfigKey("key2"); + * } + * </pre> + */ + public static class MyFeed extends AbstractFeed { + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1") + .build(); + + public static final ConfigKey<String> KEY_2 = ConfigKeys.builder(String.class, "key2") + .deprecatedNames("oldKey2") + .build(); + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/02b71ffb/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java new file mode 100644 index 0000000..7ab57cd --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java @@ -0,0 +1,328 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.config; + +import static org.testng.Assert.assertEquals; + +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.api.entity.EntityInitializer; +import org.apache.brooklyn.api.entity.EntityLocal; +import org.apache.brooklyn.api.entity.EntitySpec; +import org.apache.brooklyn.api.entity.ImplementedBy; +import org.apache.brooklyn.api.location.LocationSpec; +import org.apache.brooklyn.api.policy.PolicySpec; +import org.apache.brooklyn.api.sensor.EnricherSpec; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.enricher.AbstractEnricher; +import org.apache.brooklyn.core.entity.AbstractEntity; +import org.apache.brooklyn.core.entity.EntityInternal; +import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; +import org.apache.brooklyn.core.location.AbstractLocation; +import org.apache.brooklyn.core.policy.AbstractPolicy; +import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.entity.stock.BasicEntity; +import org.apache.brooklyn.test.LogWatcher; +import org.apache.brooklyn.test.LogWatcher.EventPredicates; +import org.apache.brooklyn.util.core.config.ConfigBag; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.Test; + +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableMap; + +import ch.qos.logback.classic.spi.ILoggingEvent; + +public class ConfigKeyDeprecationTest extends BrooklynAppUnitTestSupport { + + @SuppressWarnings("unused") + private static final Logger log = LoggerFactory.getLogger(ConfigKeyDeprecationTest.class); + + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1", "oldKey1b") + .build(); + + @Test + public void testUsingDeprecatedName() throws Exception { + EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class) + .configure("oldSuperKey1", "myval")); + EntityInternal entity2 = app.addChild(EntitySpec.create(MyBaseEntity.class) + .configure("oldSuperKey1b", "myval")); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval"); + assertEquals(entity2.config().get(MyBaseEntity.SUPER_KEY_1), "myval"); + } + + @Test + public void testPrefersNonDeprecatedName() throws Exception { + EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class) + .configure("superKey1", "myval") + .configure("oldSuperKey1", "mywrongval")); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval"); + } + + @Test + public void testPrefersFirstDeprecatedNameIfMultiple() throws Exception { + EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class) + .configure("oldSuperKey1", "myval1") + .configure("oldSuperKey1b", "myval2")); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval1"); + } + + @Test + public void testInheritsDeprecatedKeyFromRuntimeParent() throws Exception { + EntityInternal entity = app.addChild(EntitySpec.create(TestEntity.class) + .configure("oldSuperKey1", "myval")); + EntityInternal child = entity.addChild(EntitySpec.create(MyBaseEntity.class)); + assertEquals(child.config().get(MyBaseEntity.SUPER_KEY_1), "myval"); + } + + @Test + public void testInheritsDeprecatedKeyFromSuperType() throws Exception { + EntityInternal entity = app.addChild(EntitySpec.create(MySubEntity.class) + .configure("oldSuperKey1", "myval") + .configure("oldSuperKey2", "myval2") + .configure("oldSubKey2", "myval3") + .configure("oldInterfaceKey1", "myval4")); + assertEquals(entity.config().get(MySubEntity.SUPER_KEY_1), "myval"); + assertEquals(entity.config().get(MySubEntity.SUPER_KEY_2), "myval2"); + assertEquals(entity.config().get(MySubEntity.SUB_KEY_2), "myval3"); + assertEquals(entity.config().get(MyInterface.INTERFACE_KEY_1), "myval4"); + } + + @Test + public void testUsingDeprecatedNameInLocation() throws Exception { + MyLocation loc = mgmt.getLocationManager().createLocation(LocationSpec.create(MyLocation.class) + .configure("oldKey1", "myval")); + assertEquals(loc.config().get(MyLocation.KEY_1), "myval"); + } + + @Test + public void testUsingDeprecatedNameInPolicy() throws Exception { + MyPolicy policy = app.policies().add(PolicySpec.create(MyPolicy.class) + .configure("oldKey1", "myval")); + assertEquals(policy.config().get(MyPolicy.KEY_1), "myval"); + } + + @Test + public void testUsingDeprecatedNameInEnricher() throws Exception { + MyEnricher enricher = app.enrichers().add(EnricherSpec.create(MyEnricher.class) + .configure("oldKey1", "myval")); + assertEquals(enricher.config().get(MyEnricher.KEY_1), "myval"); + } + + @Test + public void testUsingDeprecatedNameInEntityInitializer() throws Exception { + Entity entity = app.addChild(EntitySpec.create(BasicEntity.class) + .addInitializer(new MyEntityInitializer(ConfigBag.newInstance(ImmutableMap.of("oldKey1", "myval"))))); + assertEquals(entity.config().get(MyEntityInitializer.KEY_1), "myval"); + } + + @Test + public void testSetConfigUsingDeprecatedName() throws Exception { + EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class)); + + // Set using strongly typed key + entity.config().set(MyBaseEntity.SUPER_KEY_1, "myval"); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval"); + + // Set using correct string + entity.config().putAll(ImmutableMap.of("superKey1", "myval2")); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval2"); + + // Set using deprecated name + entity.config().putAll(ImmutableMap.of("oldSuperKey1", "myval3")); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval3"); + + // Set using pseudo-generated strongly typed key with deprecated name + entity.config().set(ConfigKeys.newConfigKey(Object.class, "oldSuperKey1"), "myval4"); + assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval4"); + } + + @Test + public void testSetAndGetDynamicConfigUsingDeprecatedName() throws Exception { + // Using BasicEntity, which doesn't know about KEY_1 + EntityInternal entity = (EntityInternal) app.addChild(EntitySpec.create(BasicEntity.class)); + + // Set using deprecated name + entity.config().putAll(ImmutableMap.of("oldKey1", "myval3")); + assertEquals(entity.config().get(KEY_1), "myval3"); + + // Set using pseudo-generated strongly typed key with deprecated name + entity.config().set(ConfigKeys.newConfigKey(Object.class, "oldKey1"), "myval4"); + assertEquals(entity.config().get(KEY_1), "myval4"); + } + + /* + // Setting the value of a "dynamic config key" when using the deprecated key will not overwrite + // any existing value that was set using the real config key name. I think this is because + // EntityDynamicType.addConfigKey() is not called when KEY_1 is first set, so it doesn't have + // access to the deprecated names on subsequent calls to set(String, val). It therefore stores + // in EntityConfigMap (backed by a ConfigBag) the original key-value and the deprecated key-value. + // When we subsequently call get(key), it retrieved the original key-value. + // + // Contrast this with EntityDynamicType.addSensorIfAbsent(). + // + * However, it's (probably) not straight forward to just add and call a addConfigKeyIfAbsent. This + * is because config().set() is used for dynamic config declard in yaml, which the entity doesn't + * understand but that will be inherited by runtime children. + */ + @Test(groups="Broken") + public void testSetAndGetDynamicConfigUsingDeprecatedNameOverwritesExistingValue() throws Exception { + // Using BasicEntity, which doesn't know about KEY_1 + EntityInternal entity = (EntityInternal) app.addChild(EntitySpec.create(BasicEntity.class)); + + // Set using strongly typed key + entity.config().set(KEY_1, "myval"); + assertEquals(entity.config().get(KEY_1), "myval"); + + // Set using correct string + entity.config().putAll(ImmutableMap.of("key1", "myval2")); + assertEquals(entity.config().get(KEY_1), "myval2"); + + // Set using deprecated name + entity.config().putAll(ImmutableMap.of("oldKey1", "myval3")); + assertEquals(entity.config().get(KEY_1), "myval3"); + + // Set using pseudo-generated strongly typed key with deprecated name + entity.config().set(ConfigKeys.newConfigKey(Object.class, "oldKey1"), "myval4"); + assertEquals(entity.config().get(KEY_1), "myval4"); + } + + @Test + public void testLogsIfDeprecatedNameUsed() throws Exception { + // FIXME Which logger? + String loggerName = ConfigUtilsInternal.class.getName(); + ch.qos.logback.classic.Level logLevel = ch.qos.logback.classic.Level.WARN; + Predicate<ILoggingEvent> filter = EventPredicates.containsMessages( + "Using deprecated config value on MyBaseEntity", + "should use 'superKey1', but used 'oldSuperKey1'"); + LogWatcher watcher = new LogWatcher(loggerName, logLevel, filter); + + watcher.start(); + try { + testUsingDeprecatedName(); + watcher.assertHasEvent(); + } finally { + watcher.close(); + } + } + + + @Test + public void testLogsWarningIfNonDeprecatedAndDeprecatedNamesUsed() throws Exception { + String loggerName = ConfigUtilsInternal.class.getName(); + ch.qos.logback.classic.Level logLevel = ch.qos.logback.classic.Level.WARN; + Predicate<ILoggingEvent> filter = EventPredicates.containsMessages( + "Ignoring deprecated config value(s) on MyBaseEntity", + "because contains value for 'superKey1', other deprecated name(s) present were: [oldSuperKey1]"); + LogWatcher watcher = new LogWatcher(loggerName, logLevel, filter); + + watcher.start(); + try { + testPrefersNonDeprecatedName(); + watcher.assertHasEvent(); + } finally { + watcher.close(); + } + } + + @Test + public void testLogsWarningIfMultipleDeprecatedNamesUsed() throws Exception { + String loggerName = ConfigUtilsInternal.class.getName(); + ch.qos.logback.classic.Level logLevel = ch.qos.logback.classic.Level.WARN; + Predicate<ILoggingEvent> filter = EventPredicates.containsMessages( + "Using deprecated config value on MyBaseEntity", + "should use 'superKey1', but used 'oldSuperKey1b' and ignored values present for other deprecated name(s) [oldSuperKey1b]"); + LogWatcher watcher = new LogWatcher(loggerName, logLevel, filter); + + watcher.start(); + try { + testPrefersFirstDeprecatedNameIfMultiple(); + watcher.assertHasEvent(); + } finally { + watcher.close(); + } + } + + @ImplementedBy(MyBaseEntityImpl.class) + public interface MyBaseEntity extends EntityInternal { + ConfigKey<String> SUPER_KEY_1 = ConfigKeys.builder(String.class, "superKey1") + .deprecatedNames("oldSuperKey1", "oldSuperKey1b") + .build(); + ConfigKey<String> SUPER_KEY_2 = ConfigKeys.builder(String.class, "superKey2") + .deprecatedNames("oldSuperKey2") + .build(); + } + + public static class MyBaseEntityImpl extends AbstractEntity implements MyBaseEntity { + } + + @ImplementedBy(MySubEntityImpl.class) + public interface MySubEntity extends MyBaseEntity, MyInterface { + ConfigKey<String> SUPER_KEY_1 = ConfigKeys.newConfigKeyWithDefault(MyBaseEntity.SUPER_KEY_1, "overridden superKey1 default"); + ConfigKey<String> SUB_KEY_2 = ConfigKeys.builder(String.class, "subKey2") + .deprecatedNames("oldSubKey2") + .build(); + } + + public static class MySubEntityImpl extends MyBaseEntityImpl implements MySubEntity { + } + + public interface MyInterface { + ConfigKey<String> INTERFACE_KEY_1 = ConfigKeys.builder(String.class, "interfaceKey1") + .deprecatedNames("oldInterfaceKey1") + .build(); + } + + public static class MyLocation extends AbstractLocation { + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1", "oldKey1b") + .build(); + } + + public static class MyPolicy extends AbstractPolicy { + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1", "oldKey1b") + .build(); + } + + public static class MyEnricher extends AbstractEnricher { + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1", "oldKey1b") + .build(); + } + + public static class MyEntityInitializer implements EntityInitializer { + public static final ConfigKey<String> KEY_1 = ConfigKeys.builder(String.class, "key1") + .deprecatedNames("oldKey1", "oldKey1b") + .build(); + + private String key1; + + public MyEntityInitializer(ConfigBag params) { + this.key1 = params.get(KEY_1); + } + + @Override + public void apply(EntityLocal entity) { + entity.config().set(KEY_1, key1); + } + } +}
