From 11c9c34d9478b506fd68a7dfa13e1cef35d9affc Mon Sep 17 00:00:00 2001 From: Valentin Aitken Date: Mon, 23 May 2016 12:59:36 +0300 Subject: [PATCH] Merge Map values in config Merge location configuration and application's provisioning.properties --- .../EmptySoftwareProcessYamlTest.java | 34 +++++ .../core/location/AbstractLocation.java | 2 +- .../FixedListMachineProvisioningLocation.java | 3 +- .../brooklyn/util/core/config/ConfigBag.java | 13 +- .../util/collections/CollectionHelpers.java | 52 +++++++ .../collections/CollectionHelpersTest.java | 139 ++++++++++++++++++ 6 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionHelpers.java create mode 100644 utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionHelpersTest.java diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/EmptySoftwareProcessYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/EmptySoftwareProcessYamlTest.java index 35a896ae8b..20a9471eb1 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/EmptySoftwareProcessYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/EmptySoftwareProcessYamlTest.java @@ -21,13 +21,17 @@ import java.util.Iterator; import java.util.Map; +import com.google.common.collect.ImmutableMap; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.Attributes; import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityAsserts; import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess; +import org.apache.brooklyn.location.jclouds.JcloudsLocationConfig; import org.apache.brooklyn.location.ssh.SshMachineLocation; import org.apache.brooklyn.util.collections.Jsonya; import org.slf4j.Logger; @@ -59,6 +63,36 @@ public void testProvisioningProperties() throws Exception { Assert.assertEquals(pp.get("minRam"), 16384); } + public void testProvisioningPropertiesOverrideValues() throws Exception { + ConfigKey testConfig = ConfigKeys.newConfigKey(Map.class, "someOtherLocationOption"); + Entity app = createAndStartApplication( + "location:", + " localhost:", + " templateOptions:", + " testKey: testValue", + " someOtherLocationOption:", + " testKeyX1: testValueX1", + "services:", + "- type: "+EmptySoftwareProcess.class.getName(), + " provisioning.properties:", + " minRam: 16384", + " templateOptions:", + " overrideLoginUser: testUser", + " someOtherLocationOption:", + " testKeyX2: testValueX2"); + waitForApplicationTasks(app); + + log.info("App started:"); + Entities.dumpInfo(app); + + EmptySoftwareProcess entity = (EmptySoftwareProcess) app.getChildren().iterator().next(); + Map pp = entity.getConfig(EmptySoftwareProcess.PROVISIONING_PROPERTIES); + Assert.assertEquals(pp.get("minRam"), 16384); + Map templateOptions = entity.getLocations().iterator().next().getConfig(JcloudsLocationConfig.TEMPLATE_OPTIONS); + Assert.assertEquals(templateOptions, ImmutableMap.of("overrideLoginUser", "testUser", "testKey", "testValue")); + Assert.assertEquals(entity.getLocations().iterator().next().getConfig(testConfig), ImmutableMap.of("testKeyX2", "testValueX2")); + } + @Test(groups="Integration") public void testProvisioningPropertiesViaJsonya() throws Exception { Entity app = createAndStartApplication( 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 d4e565eaee..cd9f9a6031 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 @@ -459,7 +459,7 @@ public Maybe getLocalRaw(ConfigKey key) { @Override public void addToLocalBag(Map vals) { - configBag.putAll(vals); + configBag = ConfigBag.newInstanceExtending(configBag, vals); } @Override diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java index 6e88f15713..9544a75eba 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java @@ -39,6 +39,7 @@ import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.location.AbstractLocation; import org.apache.brooklyn.core.location.cloud.CloudLocationConfig; +import org.apache.brooklyn.core.objs.AbstractConfigurationSupportInternal; import org.apache.brooklyn.location.ssh.SshMachineLocation; import org.apache.brooklyn.util.collections.CollectionFunctionals; import org.apache.brooklyn.util.collections.MutableMap; @@ -279,7 +280,7 @@ public T obtain() throws NoMachinesAvailableException { public T obtain(Map flags) throws NoMachinesAvailableException { T machine; T desiredMachine = (T) flags.get("desiredMachine"); - ConfigBag allflags = ConfigBag.newInstanceExtending(config().getBag()).putAll(flags); + ConfigBag allflags = ConfigBag.newInstanceExtending(config().getBag(), flags); Function, MachineLocation> chooser = allflags.get(MACHINE_CHOOSER); synchronized (lock) { 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 23a7485c47..530b8a6ba2 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 @@ -29,8 +29,8 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.util.collections.CollectionHelpers; import org.apache.brooklyn.util.collections.MutableMap; -import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.javalang.JavaClassNames; @@ -123,10 +123,17 @@ public void markUsed(String key) { } } - /** As {@link #newInstanceExtending(ConfigBag)} but also putting the supplied values. */ + /** As {@link #newInstanceExtending(ConfigBag)} but also putting the supplied values. + * In Apache Brooklyn the method is used only for Location configuration. + * The method will do a shallow merge only for templateOptions key, other keys will override with optionalAdditionalValues. + */ @Beta public static ConfigBag newInstanceExtending(final ConfigBag configBag, Map optionalAdditionalValues) { - return newInstanceExtending(configBag).putAll(optionalAdditionalValues); + if (optionalAdditionalValues != null) { + return newInstance(CollectionHelpers.mergeMapsAndTheirInnerMapValues(configBag.getAllConfig(), (Map) optionalAdditionalValues, "templateOptions")); + } else { + return newInstanceExtending(configBag); + } } /** @deprecated since 0.7.0, not used; kept only for rebind compatibility where the inner class is used diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionHelpers.java b/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionHelpers.java new file mode 100644 index 0000000000..1c9100fa42 --- /dev/null +++ b/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionHelpers.java @@ -0,0 +1,52 @@ +/* + * 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.util.collections; + +import java.util.Map; + +public class CollectionHelpers { + public static Map mergeMapsAndTheirInnerMapValues(Map defaultEntries, Map overrideWith) { + return mergeMapsAndTheirInnerMapValues(defaultEntries, overrideWith, null); + } + + /** + * Does a shallow merge of two maps. + */ + public static Map mergeMapsAndTheirInnerMapValues(Map defaultEntries, Map overrideWith, String keyToWhichShallowMergeIsRestricted) { + Map result = MutableMap.of(); + result.putAll(defaultEntries); + for (Map.Entry newEntry: overrideWith.entrySet()) { + if (!defaultEntries.containsKey(newEntry.getKey())) { + result.put(newEntry.getKey(), newEntry.getValue()); + } else if (newEntry.getValue() instanceof Map) { + Map mergedValue = MutableMap.of(); + if (defaultEntries.get(newEntry.getKey()) instanceof Map) { + if (keyToWhichShallowMergeIsRestricted == null || keyToWhichShallowMergeIsRestricted.equals(newEntry.getKey())) { + mergedValue.putAll((Map) defaultEntries.get(newEntry.getKey())); + } + } + mergedValue.putAll((Map) newEntry.getValue()); + result.put(newEntry.getKey(), mergedValue); + } else { + result.put(newEntry.getKey(), newEntry.getValue()); + } + } + return result; + } +} diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionHelpersTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionHelpersTest.java new file mode 100644 index 0000000000..81030ad265 --- /dev/null +++ b/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionHelpersTest.java @@ -0,0 +1,139 @@ +/* + * 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.util.collections; + +import com.google.common.collect.ImmutableMap; +import org.testng.annotations.Test; + +import java.util.Map; + +import static org.apache.brooklyn.util.collections.CollectionHelpers.mergeMapsAndTheirInnerMapValues; +import static org.testng.Assert.assertEquals; + +public class CollectionHelpersTest { + @Test + public void testMergeMapsEmpty() { + Map targetMap = MutableMap.of(); + Map overrideWithNew = MutableMap.of(); + + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew), MutableMap.of()); + } + + @Test + public void testMergeMapsNonMapValues() { + Map targetMap = ImmutableMap.of("testKeyTarget", "testValTarget", "testKeyTarget1", "testVal1"); + Map overrideWithNew = ImmutableMap.of("testKeyNew1", "testValNew1", "testKeyTarget", "testValNew2"); + + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew), + ImmutableMap.of( + "testKeyTarget1", "testVal1", + "testKeyNew1", "testValNew1", + "testKeyTarget", "testValNew2")); + } + + /** + * Always override with the new value + */ + @Test + public void testMergeMapsOverridingMapAndNonMapValues() { + Map targetMap = ImmutableMap.of("testMapValue", ImmutableMap.of(), "testKeyTarget1", "testVal1"); + Map overrideWithNew = ImmutableMap.of("testKeyNew1", "testValNew1", "testMapValue", "testValNew2"); + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew), + ImmutableMap.of( + "testMapValue", "testValNew2", + "testKeyTarget1", "testVal1", + "testKeyNew1", "testValNew1")); + + targetMap = ImmutableMap.of("testMapValue", "testValNew2", "testKeyTarget1", "testVal1", "testKeyTarget2", "testVal2"); + overrideWithNew = MutableMap.of("testKeyNew1", "testValNew1", "testMapValue", ImmutableMap.of()); + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew), + ImmutableMap.of( + "testMapValue", ImmutableMap.of(), + "testKeyTarget1", "testVal1", + "testKeyNew1", "testValNew1", + "testKeyTarget2", "testVal2")); + } + + @Test + public void testMergeMapsOverridingMapValues() { + Map targetMap = ImmutableMap.of("testTargetKey", "testTargetValue", + "testTargetKey1", "testTargetValue1", + "templateOptions", ImmutableMap.of("locKey1", "locVal1", "locKey2", "locVal2", "locKey3", "locVal3")); + + Map overrideWithNew = ImmutableMap.of("testKeyNew1", "testValNew1", + "templateOptions", ImmutableMap.of("locKey1", "locValNew1", "locKey2", "locValNew2"), + "testTargetKey", "testNewValue"); + + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew), + ImmutableMap.of( + "testTargetKey1", "testTargetValue1", + "templateOptions", ImmutableMap.of("locKey1", "locValNew1", "locKey3", "locVal3", "locKey2", "locValNew2"), + "testKeyNew1", "testValNew1", + "testTargetKey", "testNewValue")); + } + + @Test + public void testMergeMapsOverridingMapValuesNested() { + Map targetMap = ImmutableMap.of( + "testTargetKey", "testTargetValue", + "testTargetKey1", "testTargetValue1", + "templateOptions", ImmutableMap.of( + "locKey2", ImmutableMap.of("locKeyNestedTarget", "locKey2UniqVal"), + "locKey1", "locVal1", + "locKey3", "locVal3")); + + Map overrideWithNew = ImmutableMap.of("testKeyNew1", "testValNew1", + "templateOptions", ImmutableMap.of( + "locKey1", "locValNew1", + "locKey2", ImmutableMap.of("locKey21", "locKeyVal21")), + "testTargetKey", "testNewValue"); + + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew), + ImmutableMap.of( + "testTargetKey1", "testTargetValue1", + "templateOptions", ImmutableMap.of( + "locKey1", "locValNew1", + "locKey3", "locVal3", + "locKey2", ImmutableMap.of("locKey21", "locKeyVal21")), // It will override Maps which are one level deeper + "testKeyNew1", "testValNew1", + "testTargetKey", "testNewValue")); + } + + @Test + public void testMergeMapsOverridingMapValuesRestrictedToAKey() { + Map targetMap = ImmutableMap.of("testTargetKey", "testTargetValue", + "testTargetKey1", "testTargetValue1", + "keyToReplace", ImmutableMap.of("locKey1", "locValNew1", "locKey3", "locVal3", "locKey2", "locValNew2"), + "keyToMerge", ImmutableMap.of("locKey1", "locValNew1", "locKey3", "locVal3", "locKey2", "locValNew2")); + + Map overrideWithNew = ImmutableMap.of("testKeyNew1", "testValNew1", + "keyToReplace", ImmutableMap.of("mlocKey1", "newLocValNew1", "locKey3", "newLocVal3", "locKey2", "newLocValNew2"), + "keyToMerge", ImmutableMap.of("mlocKey1", "newLocValNew1", "locKey3", "newLocVal3", "locKey2", "newLocValNew2"), + "testTargetKey", "testNewValue"); + + assertEquals(mergeMapsAndTheirInnerMapValues(targetMap, overrideWithNew, "keyToMerge"), + ImmutableMap.of( + "testTargetKey1", "testTargetValue1", + "keyToReplace", ImmutableMap.of("mlocKey1", "newLocValNew1", "locKey3", "newLocVal3", "locKey2", "newLocValNew2"), + "keyToMerge", ImmutableMap.of("locKey1", "locValNew1", + "mlocKey1", "newLocValNew1", "locKey3", "newLocVal3", "locKey2", "newLocValNew2"), + "testKeyNew1", "testValNew1", + "testTargetKey", "testNewValue")); + } +}