PR #734: address comments for 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/afa787c4 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/afa787c4 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/afa787c4 Branch: refs/heads/master Commit: afa787c45f153352feb1ddd5c7347f811f172dad Parents: 02b71ff Author: Aled Sage <[email protected]> Authored: Wed Jun 28 17:33:00 2017 +0100 Committer: Aled Sage <[email protected]> Committed: Wed Jun 28 22:08:52 2017 +0100 ---------------------------------------------------------------------- .../brooklyn/core/entity/AbstractEntity.java | 3 + .../core/location/AbstractLocation.java | 15 +- .../core/objs/AbstractEntityAdjunct.java | 16 +- .../brooklyn/util/core/config/ConfigBag.java | 6 +- .../config/ConfigKeyDeprecationRebindTest.java | 12 ++ .../core/config/ConfigKeyDeprecationTest.java | 6 +- .../core/enricher/BasicEnricherTest.java | 2 +- .../core/enricher/EnricherConfigTest.java | 6 +- .../core/entity/LocationSetFromFlagTest.java | 211 +++++++++++++++++++ .../core/policy/basic/BasicPolicyTest.java | 2 +- .../core/policy/basic/PolicyConfigTest.java | 6 +- .../util/core/config/ConfigBagTest.java | 82 ++++++- 12 files changed, 330 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/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 d0d3873..08b6591 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 @@ -396,6 +396,9 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E } // allow config keys to be set by name (or deprecated name) + // + // The resulting `flags` will no longer contain the keys that we matched; + // we will not also use them to for `SetFromFlag` etc. flags = ConfigUtilsInternal.setAllConfigKeys(flags, getEntityType().getConfigKeys(), this); // allow config keys, and fields, to be set from these flags if they have a SetFromFlag annotation http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/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 58b2809..32b01fa 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 @@ -216,17 +216,14 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements config().removeKey(PARENT_LOCATION); } - // allow config keys to be set by name (or deprecated name) + // 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); + // The resulting `properties` will no longer contain the keys that we matched; + // we will not also use them to for `SetFromFlag` etc. + properties = ConfigUtilsInternal.setAllConfigKeys(properties, getLocationTypeInternal().getConfigKeys().values(), this); // NB: flag-setting done here must also be done in BasicLocationRebindSupport + // Must call setFieldsFromFlagsWithBag, even if properites is empty, so defaults are extracted from annotations ConfigBag configBag = ConfigBag.newInstance(properties); FlagUtils.setFieldsFromFlagsWithBag(this, properties, configBag, firstTime); FlagUtils.setAllConfigKeys(this, configBag, false); @@ -252,7 +249,7 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements } else { codes = TypeCoercions.coerce(rawCodes, new TypeToken<Set<String>>() {}); } - configBag.put(LocationConfigKeys.ISO_3166, codes); + config().set(LocationConfigKeys.ISO_3166, codes); } return this; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/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 7395d0a..3faf2a6 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 @@ -153,19 +153,13 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple } } - // Allow config keys to be set by name (or deprecated name). + // 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); + // The resulting `flags` will no longer contain the keys that we matched; + // we will not also use them to for `SetFromFlag` etc. + flags = ConfigUtilsInternal.setAllConfigKeys(flags, getAdjunctType().getConfigKeys(), this); + // Must call setFieldsFromFlagsWithBag, even if properites is empty, so defaults are extracted from annotations 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/afa787c4/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 cfef3b0..9f9bf09 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 @@ -435,6 +435,7 @@ public class ConfigBag { * * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()} */ + @Deprecated public Object getWithDeprecation(ConfigKey<?> key, ConfigKey<?> ...deprecatedKeys) { return getWithDeprecation(new ConfigKey[] { key }, deprecatedKeys); } @@ -446,6 +447,7 @@ public class ConfigBag { * * @deprecated since 0.12.0; instead define deprecated names on key, see {@link ConfigKey#getDeprecatedNames()} */ + @Deprecated public synchronized Object getWithDeprecation(ConfigKey<?>[] currentKeysInOrderOfPreference, ConfigKey<?> ...deprecatedKeys) { // Get preferred key (or null) ConfigKey<?> preferredKeyProvidingValue = null; @@ -541,7 +543,7 @@ public class ConfigBag { log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; " + "using same value from preferred name "+key.getName()); } - } else if (firstDeprecatedVal.isPresent()) { + } else if (firstDeprecatedVal != null && 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); @@ -590,7 +592,7 @@ public class ConfigBag { log.warn("Duplicate value for key "+key+" from deprecated name '"+deprecatedName+"'; " + "using same value from preferred name "+key.getName()); } - } else if (firstDeprecatedVal != null) { + } else if (firstDeprecatedVal != null && 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); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/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 index b20f574..f25f544 100644 --- a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationRebindTest.java @@ -100,6 +100,18 @@ public class ConfigKeyDeprecationRebindTest extends AbstractRebindHistoricTest { * .configure("oldFlagKey2", "myval")); * } * </pre> + * + * Note that (with Brooklyn 0.11.0) the above code wrote the persisted state as "key2". + * That is, it switched the name used at configure-time for the actual key name, so that + * is what is in persisted state. + * + * Therefore this test is a bit pointless! The persisted state does not contain the word + * "oldFlagKey2" so of course it won't contain that word after rebind. + * + * The purpose of the test is to illustrate that we thought about and tested the code + * path of someone having used the alias in an old Brooklyn version, and now is upgrading + * to a version where the alias annotation has been replaced by + * {@code .deprecatedNames("oldFlagKey2")}. */ @Test public void testEntityPersistedWithSetFromFlagNameOnKey() throws Exception { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/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 index 7ab57cd..b85ad58 100644 --- a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyDeprecationTest.java @@ -80,8 +80,8 @@ public class ConfigKeyDeprecationTest extends BrooklynAppUnitTestSupport { @Test public void testPrefersFirstDeprecatedNameIfMultiple() throws Exception { EntityInternal entity = app.addChild(EntitySpec.create(MyBaseEntity.class) - .configure("oldSuperKey1", "myval1") - .configure("oldSuperKey1b", "myval2")); + .configure("oldSuperKey1b", "myval2") + .configure("oldSuperKey1", "myval1")); assertEquals(entity.config().get(MyBaseEntity.SUPER_KEY_1), "myval1"); } @@ -180,7 +180,7 @@ public class ConfigKeyDeprecationTest extends BrooklynAppUnitTestSupport { // 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 + * is because config().set() is used for dynamic config declared in yaml, which the entity doesn't * understand but that will be inherited by runtime children. */ @Test(groups="Broken") http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/core/src/test/java/org/apache/brooklyn/core/enricher/BasicEnricherTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/enricher/BasicEnricherTest.java b/core/src/test/java/org/apache/brooklyn/core/enricher/BasicEnricherTest.java index 98ab41c..5e47da3 100644 --- a/core/src/test/java/org/apache/brooklyn/core/enricher/BasicEnricherTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/enricher/BasicEnricherTest.java @@ -62,7 +62,7 @@ public class BasicEnricherTest extends BrooklynAppUnitTestSupport { @SetFromFlag("strKey") public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key"); public static final ConfigKey<Integer> INT_KEY_WITH_DEFAULT = new BasicConfigKey<Integer>(Integer.class, "ckey", "c key", 1); - public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default"); + public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKeyWithDefault", "str key", "strKeyDefaultVal"); MyEnricher(Map<?,?> flags) { super(flags); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/core/src/test/java/org/apache/brooklyn/core/enricher/EnricherConfigTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/enricher/EnricherConfigTest.java b/core/src/test/java/org/apache/brooklyn/core/enricher/EnricherConfigTest.java index aae382c..7dcd50c 100644 --- a/core/src/test/java/org/apache/brooklyn/core/enricher/EnricherConfigTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/enricher/EnricherConfigTest.java @@ -48,8 +48,7 @@ public class EnricherConfigTest extends BrooklynAppUnitTestSupport { assertEquals(enricher.getConfig(MyEnricher.STR_KEY), "aval"); assertEquals(enricher.getConfig(MyEnricher.INT_KEY), (Integer)2); - // this is set, because key name matches annotation on STR_KEY - assertEquals(enricher.getConfig(MyEnricher.STR_KEY_WITH_DEFAULT), "aval"); + assertEquals(enricher.getConfig(MyEnricher.STR_KEY_WITH_DEFAULT), MyEnricher.STR_KEY_WITH_DEFAULT.getDefaultValue()); } @Test @@ -74,7 +73,6 @@ public class EnricherConfigTest extends BrooklynAppUnitTestSupport { assertEquals(enricher.getConfig(MyEnricher.STR_KEY), "aval"); assertEquals(enricher.getConfig(MyEnricher.INT_KEY), (Integer)2); - // this is not set (contrast with above) assertEquals(enricher.getConfig(MyEnricher.STR_KEY_WITH_DEFAULT), MyEnricher.STR_KEY_WITH_DEFAULT.getDefaultValue()); } @@ -141,7 +139,7 @@ public class EnricherConfigTest extends BrooklynAppUnitTestSupport { public void testConfigReturnsDefaultValueIfNotSet() throws Exception { MyEnricher enricher = app.enrichers().add(EnricherSpec.create(MyEnricher.class)); - assertEquals(enricher.getConfig(MyEnricher.STR_KEY_WITH_DEFAULT), "str key default"); + assertEquals(enricher.getConfig(MyEnricher.STR_KEY_WITH_DEFAULT), MyEnricher.STR_KEY_WITH_DEFAULT.getDefaultValue()); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/core/src/test/java/org/apache/brooklyn/core/entity/LocationSetFromFlagTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/LocationSetFromFlagTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/LocationSetFromFlagTest.java new file mode 100644 index 0000000..30f35c3 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/entity/LocationSetFromFlagTest.java @@ -0,0 +1,211 @@ +/* + * 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; + +import static org.testng.Assert.assertEquals; + +import java.util.Map; + +import org.apache.brooklyn.api.location.PortRange; +import org.apache.brooklyn.api.location.LocationSpec; +import org.apache.brooklyn.core.location.PortRanges; +import org.apache.brooklyn.core.location.AbstractLocation; +import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.core.flags.SetFromFlag; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableMap; + +public class LocationSetFromFlagTest extends BrooklynAppUnitTestSupport { + + @Test + public void testSetFromFlagUsingFieldName() { + MyLocation loc = newLocation(MutableMap.of("str1", "myval")); + assertEquals(loc.str1, "myval"); + } + + @Test + public void testSetFromFlagUsingOverridenName() { + MyLocation loc = newLocation(MutableMap.of("altStr2", "myval")); + assertEquals(loc.str2, "myval"); + } + + @Test + public void testSetFromFlagWhenNoDefaultIsNull() { + MyLocation loc = newLocation(); + assertEquals(loc.str1, null); + } + + @Test + public void testSetFromFlagUsesDefault() { + MyLocation loc = newLocation(); + assertEquals(loc.str3, "default str3"); + } + + @Test + public void testSetFromFlagOverridingDefault() { + MyLocation loc = newLocation(MutableMap.of("str3", "overridden str3")); + assertEquals(loc.str3, "overridden str3"); + } + + @Test + public void testSetFromFlagCastsPrimitives() { + MyLocation loc = newLocation(MutableMap.of("double1", 1f)); + assertEquals(loc.double1, 1d); + } + + @Test + public void testSetFromFlagCastsDefault() { + MyLocation loc = newLocation(); + assertEquals(loc.byte1, (byte)1); + assertEquals(loc.short1, (short)2); + assertEquals(loc.int1, 3); + assertEquals(loc.long1, 4l); + assertEquals(loc.float1, 5f); + assertEquals(loc.double1, 6d); + assertEquals(loc.char1, 'a'); + assertEquals(loc.bool1, true); + + assertEquals(loc.byte2, Byte.valueOf((byte)1)); + assertEquals(loc.short2, Short.valueOf((short)2)); + assertEquals(loc.int2, Integer.valueOf(3)); + assertEquals(loc.long2, Long.valueOf(4l)); + assertEquals(loc.float2, Float.valueOf(5f)); + assertEquals(loc.double2, Double.valueOf(6d)); + assertEquals(loc.char2, Character.valueOf('a')); + assertEquals(loc.bool2, Boolean.TRUE); + } + + @Test + public void testSetFromFlagCoercesDefaultToPortRange() { + MyLocation loc = newLocation(); + assertEquals(loc.portRange1, PortRanges.fromInteger(1234)); + } + + @Test + public void testSetFromFlagCoercesStringValueToPortRange() { + MyLocation loc = newLocation(MutableMap.of("portRange1", "1-3")); + assertEquals(loc.portRange1, new PortRanges.LinearPortRange(1, 3)); + } + + @Test + public void testSetFromFlagCoercesStringValueToInt() { + MyLocation loc = newLocation(MutableMap.of("int1", "123")); + assertEquals(loc.int1, 123); + } + + @Test + public void testFailsFastOnInvalidCoercion() { + try { + newLocation(MutableMap.of("int1", "thisisnotanint")); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + if (Exceptions.getFirstThrowableOfType(e, IllegalArgumentException.class) == null) { + throw e; + } + } + } + + @Test + public void testSetFromFlagWithFieldThatIsExplicitySet() { + MyLocation loc = newLocation(MutableMap.of("str4", "myval")); + assertEquals(loc.str4, "myval"); + + MyLocation loc2 = newLocation(); + assertEquals(loc2.str4, "explicit str4"); + } + + private MyLocation newLocation() { + return newLocation(ImmutableMap.of()); + } + + private MyLocation newLocation(Map<?, ?> config) { + return mgmt.getLocationManager().createLocation(LocationSpec.create(MyLocation.class) + .configure(config)); + } + + public static class MyLocation extends AbstractLocation { + + @SetFromFlag(defaultVal="1234") + PortRange portRange1; + + @SetFromFlag + String str1; + + @SetFromFlag("altStr2") + String str2; + + @SetFromFlag(defaultVal="default str3") + String str3; + + @SetFromFlag + String str4 = "explicit str4"; + + @SetFromFlag(defaultVal="1") + byte byte1; + + @SetFromFlag(defaultVal="2") + short short1; + + @SetFromFlag(defaultVal="3") + int int1; + + @SetFromFlag(defaultVal="4") + long long1; + + @SetFromFlag(defaultVal="5") + float float1; + + @SetFromFlag(defaultVal="6") + double double1; + + @SetFromFlag(defaultVal="a") + char char1; + + @SetFromFlag(defaultVal="true") + boolean bool1; + + @SetFromFlag(defaultVal="1") + Byte byte2; + + @SetFromFlag(defaultVal="2") + Short short2; + + @SetFromFlag(defaultVal="3") + Integer int2; + + @SetFromFlag(defaultVal="4") + Long long2; + + @SetFromFlag(defaultVal="5") + Float float2; + + @SetFromFlag(defaultVal="6") + Double double2; + + @SetFromFlag(defaultVal="a") + Character char2; + + @SetFromFlag(defaultVal="true") + Boolean bool2; + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/core/src/test/java/org/apache/brooklyn/core/policy/basic/BasicPolicyTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/policy/basic/BasicPolicyTest.java b/core/src/test/java/org/apache/brooklyn/core/policy/basic/BasicPolicyTest.java index 020b274..26f3915 100644 --- a/core/src/test/java/org/apache/brooklyn/core/policy/basic/BasicPolicyTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/policy/basic/BasicPolicyTest.java @@ -48,7 +48,7 @@ public class BasicPolicyTest extends BrooklynAppUnitTestSupport { @SetFromFlag("strKey") public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key"); public static final ConfigKey<Integer> INT_KEY_WITH_DEFAULT = new BasicConfigKey<Integer>(Integer.class, "ckey", "c key", 1); - public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default"); + public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKeyWithDefault", "str key", "strKeyDefaultVal"); public static final ConfigKey<String> RECONFIGURABLE_KEY = ConfigKeys.builder(String.class, "reconfigurableKey") .reconfigurable(true) .build(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/core/src/test/java/org/apache/brooklyn/core/policy/basic/PolicyConfigTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/policy/basic/PolicyConfigTest.java b/core/src/test/java/org/apache/brooklyn/core/policy/basic/PolicyConfigTest.java index 784c076..c25122e 100644 --- a/core/src/test/java/org/apache/brooklyn/core/policy/basic/PolicyConfigTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/policy/basic/PolicyConfigTest.java @@ -55,8 +55,7 @@ public class PolicyConfigTest extends BrooklynAppUnitTestSupport { assertEquals(policy.getConfig(MyPolicy.STR_KEY), "aval"); assertEquals(policy.getConfig(MyPolicy.INT_KEY), (Integer)2); - // this is set, because key name matches annotation on STR_KEY - assertEquals(policy.getConfig(MyPolicy.STR_KEY_WITH_DEFAULT), "aval"); + assertEquals(policy.getConfig(MyPolicy.STR_KEY_WITH_DEFAULT), MyPolicy.STR_KEY_WITH_DEFAULT.getDefaultValue()); } @Test @@ -81,7 +80,6 @@ public class PolicyConfigTest extends BrooklynAppUnitTestSupport { assertEquals(policy.getConfig(MyPolicy.STR_KEY), "aval"); assertEquals(policy.getConfig(MyPolicy.INT_KEY), (Integer)2); - // this is not set (contrast with above) assertEquals(policy.getConfig(MyPolicy.STR_KEY_WITH_DEFAULT), MyPolicy.STR_KEY_WITH_DEFAULT.getDefaultValue()); } @@ -157,7 +155,7 @@ public class PolicyConfigTest extends BrooklynAppUnitTestSupport { public void testConfigReturnsDefaultValueIfNotSet() throws Exception { MyPolicy policy = app.policies().add(PolicySpec.create(MyPolicy.class)); - assertEquals(policy.getConfig(MyPolicy.STR_KEY_WITH_DEFAULT), "str key default"); + assertEquals(policy.getConfig(MyPolicy.STR_KEY_WITH_DEFAULT), MyPolicy.STR_KEY_WITH_DEFAULT.getDefaultValue()); } @Test http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/afa787c4/core/src/test/java/org/apache/brooklyn/util/core/config/ConfigBagTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/util/core/config/ConfigBagTest.java b/core/src/test/java/org/apache/brooklyn/util/core/config/ConfigBagTest.java index f85a117..bff9ca3 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/config/ConfigBagTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/config/ConfigBagTest.java @@ -19,6 +19,7 @@ package org.apache.brooklyn.util.core.config; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import java.util.List; import java.util.Map; @@ -29,8 +30,6 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; -import org.apache.brooklyn.util.core.config.ConfigBag; -import org.apache.brooklyn.util.core.config.ConfigBagTest; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.time.Duration; import org.slf4j.Logger; @@ -38,6 +37,8 @@ import org.slf4j.LoggerFactory; import org.testng.Assert; import org.testng.annotations.Test; +import com.google.common.collect.ImmutableMap; + public class ConfigBagTest { @SuppressWarnings("unused") @@ -136,6 +137,83 @@ public class ConfigBagTest { } @Test + public void testCopyKey() throws InterruptedException { + ConfigBag bag = ConfigBag.newInstance(); + bag.put(K1, "v1"); + + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKey(bag, K1); + + assertEquals(bag2.get(K1), "v1"); + } + + @Test + @SuppressWarnings("unchecked") + public void testCopyKeys() throws InterruptedException { + ConfigBag bag = ConfigBag.newInstance(); + bag.put(K1, "v1"); + bag.put(K2, "v2"); + + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKeys(bag, K1, K2, K3); + + assertEquals(bag2.get(K1), "v1"); + assertEquals(bag2.get(K2), "v2"); + assertFalse(bag2.containsKey(K3)); + } + + @Test + public void testCopyKeyAs() throws InterruptedException { + ConfigBag bag = ConfigBag.newInstance(); + bag.put(K1, "v1"); + + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKeyAs(bag, K1, K2); + + assertEquals(bag2.get(K2), "v1"); + assertFalse(bag2.containsKey(K1)); + } + + @Test + public void testCopyKeyWithDeprecatedNames() throws InterruptedException { + ConfigKey<String> k = ConfigKeys.builder(String.class, "k") + .deprecatedNames("kOld", "kOld2") + .build(); + + { + ConfigBag bag = ConfigBag.newInstance().putAll(ImmutableMap.of("k", "v1")); + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKey(bag, k); + assertEquals(bag2.getAllConfigRaw(), ImmutableMap.of("k", "v1")); + } + { + ConfigBag bag = ConfigBag.newInstance().putAll(ImmutableMap.of("kOld", "v1")); + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKey(bag, k); + assertEquals(bag2.getAllConfigRaw(), ImmutableMap.of("k", "v1")); + } + { + ConfigBag bag = ConfigBag.newInstance().putAll(ImmutableMap.of("kOld", "v1", "kOld2", "v2")); + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKey(bag, k); + assertEquals(bag2.getAllConfigRaw(), ImmutableMap.of("k", "v1")); + } + { + ConfigBag bag = ConfigBag.newInstance().putAll(ImmutableMap.of("k", "v1", "kOld", "v2")); + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKey(bag, k); + assertEquals(bag2.getAllConfigRaw(), ImmutableMap.of("k", "v1")); + } + { + ConfigBag bag = ConfigBag.newInstance().putAll(ImmutableMap.of("k", "v1", "kOld", "v2", "kOld2", "v3")); + ConfigBag bag2 = ConfigBag.newInstance(); + bag2.copyKey(bag, k); + assertEquals(bag2.getAllConfigRaw(), ImmutableMap.of("k", "v1")); + } + } + + + @Test public void testConcurrent() throws InterruptedException { ConfigBag bag = ConfigBag.newInstance(); bag.put(K1, "v1");
