Rename âmergeâ to âdeep_mergeâ, and PR 173 comments
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/6dad5de0 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/6dad5de0 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/6dad5de0 Branch: refs/heads/master Commit: 6dad5de0663076fb0c44fde1fc69df9eb7cd0be6 Parents: 7a7f759 Author: Aled Sage <[email protected]> Authored: Mon Jun 6 15:09:27 2016 +0100 Committer: Aled Sage <[email protected]> Committed: Mon Jun 6 15:31:10 2016 +0100 ---------------------------------------------------------------------- .../BrooklynComponentTemplateResolver.java | 2 +- .../brooklyn/ConfigInheritanceYamlTest.java | 147 ++++++++++++++++++- .../ConfigLocationInheritanceYamlTest.java | 2 +- .../camp/brooklyn/ConfigParametersYamlTest.java | 2 +- .../core/entity/BrooklynConfigKeys.java | 2 +- .../core/entity/internal/EntityConfigMap.java | 4 +- .../entity/software/base/SoftwareProcess.java | 14 +- .../brooklyn/config/ConfigInheritance.java | 12 +- 8 files changed, 163 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java index ad5ca74..1654e5c 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java @@ -337,7 +337,7 @@ public class BrooklynComponentTemplateResolver { switch (mode) { case IF_NO_EXPLICIT_VALUE: return ownVal.isPresent() ? ownVal : superVal; - case MERGE: + case DEEP_MERGE: return deepMerge(ownVal, superVal, key); case NONE: return ownVal; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java index 9f52ded..36cd02b 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java @@ -31,10 +31,13 @@ import java.util.concurrent.Executors; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.core.config.MapConfigKey; +import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityAsserts; +import org.apache.brooklyn.core.entity.internal.EntityConfigMap; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess; +import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool; import org.slf4j.Logger; @@ -44,6 +47,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import com.google.api.client.repackaged.com.google.common.base.Joiner; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -396,7 +400,7 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " brooklyn.parameters:", " - name: map.type-merged", " type: java.util.Map", - " inheritance.type: merge", + " inheritance.type: deep_merge", " default: {myDefaultKey: myDefaultVal}", " - name: map.type-always", " type: java.util.Map", @@ -501,7 +505,7 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " brooklyn.parameters:", " - name: map.type-merged", " type: java.util.Map", - " inheritance.parent: merge", + " inheritance.parent: deep_merge", " default: {myDefaultKey: myDefaultVal}", " - name: map.type-always", " type: java.util.Map", @@ -636,9 +640,9 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, expectedEnv); } - // TODO Does not work, and probably hard to fix?! We need to figure out that "env" corresponds to the + // TODO Has never worked, and probably hard to fix?! We need to figure out that "env" corresponds to the // config key. Maybe FlagUtils could respect SetFromFlags when returning Map<String,ConfigKey>? - @Test(groups="WIP") + @Test(groups={"WIP", "Broken"}) public void testExtendsSuperTypeConfigMixingShortOverridingShortName() throws Exception { ImmutableMap<String, Object> expectedEnv = ImmutableMap.<String, Object>of("ENV1", "myEnv1", "ENV2", "myEnv2"); @@ -656,6 +660,141 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, expectedEnv); } + @Test + public void testExtendsSuperTypeMultipleLevels() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " id: EmptySoftwareProcess-level1", + " itemType: entity", + " item:", + " type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess", + " brooklyn.config:", + " shell.env:", + " ENV1: myEnv1"); + + addCatalogItems( + "brooklyn.catalog:", + " id: EmptySoftwareProcess-level2", + " itemType: entity", + " item:", + " type: EmptySoftwareProcess-level1", + " brooklyn.config:", + " shell.env:", + " ENV2: myEnv2"); + + addCatalogItems( + "brooklyn.catalog:", + " id: EmptySoftwareProcess-level3", + " itemType: entity", + " item:", + " type: EmptySoftwareProcess-level2", + " brooklyn.config:", + " shell.env:", + " ENV3: myEnv3"); + + String yaml = Joiner.on("\n").join( + "location: localhost-stub", + "services:", + "- type: EmptySoftwareProcess-level3"); + + Entity app = createStartWaitAndLogApplication(new StringReader(yaml)); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, + ImmutableMap.<String, Object>of("ENV1", "myEnv1", "ENV2", "myEnv2", "ENV3", "myEnv3")); + } + + @Test + public void testExtendsSuperTypeMultipleLevelsInheritanceOptions() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: TestEntity-level1", + " item:", + " type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.parameters:", + " - name: map.type-merged", + " type: java.util.Map", + " inheritance.type: deep_merge", + " brooklyn.config:", + " map.type-merged:", + " mykey1: myval1"); + + addCatalogItems( + "brooklyn.catalog:", + " id: TestEntity-level2", + " itemType: entity", + " item:", + " type: TestEntity-level1", + " brooklyn.config:", + " map.type-merged:", + " mykey2: myval2"); + + addCatalogItems( + "brooklyn.catalog:", + " id: TestEntity-level3", + " itemType: entity", + " item:", + " type: TestEntity-level2", + " brooklyn.config:", + " map.type-merged:", + " mykey3: myval3"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: TestEntity-level3"); + + Entity app = createStartWaitAndLogApplication(new StringReader(yaml)); + TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + + assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-merged")), + ImmutableMap.of("mykey1", "myval1", "mykey2", "myval2", "mykey3", "myval3")); + } + + /** + * TODO Has always failed, and probably hard to fix?! This is due to the way + * {@link EntityConfigMap#setInheritedConfig(Map, org.apache.brooklyn.util.core.config.ConfigBag)} works: + * the parent overrides the grandparent's config. So we only get mykey2+mykey3. + */ + @Test(groups={"Broken", "WIP"}) + public void testExtendsParentMultipleLevels() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: TestEntity-with-conf", + " item:", + " type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.parameters:", + " - name: map.type-merged", + " type: java.util.Map", + " inheritance.parent: deep_merge"); + + String yaml = Joiner.on("\n").join( + "location: localhost-stub", + "services:", + "- type: "+BasicApplication.class.getName(), + " brooklyn.config:", + " map.type-merged:", + " mykey1: myval1", + " brooklyn.children:", + " - type: "+BasicApplication.class.getName(), + " brooklyn.config:", + " map.type-merged:", + " mykey2: myval2", + " brooklyn.children:", + " - type: TestEntity-with-conf", + " brooklyn.config:", + " map.type-merged:", + " mykey3: myval3"); + + Entity app = createStartWaitAndLogApplication(new StringReader(yaml)); + Entity entity = Iterables.find(Entities.descendants(app), Predicates.instanceOf(TestEntity.class)); + + assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-merged")), + ImmutableMap.<String, Object>of("mykey1", "myval1", "mykey2", "myval2", "mykey3", "myval3")); + } + protected void assertEmptySoftwareProcessConfig(Entity entity, Map<String, ?> expectedEnv, Map<String, String> expectedFiles, Map<String, ?> expectedProvisioningProps) { EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, MutableMap.<String, Object>copyOf(expectedEnv)); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java index 0a40275..eaf0d22 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java @@ -322,7 +322,7 @@ public class ConfigLocationInheritanceYamlTest extends AbstractYamlTest { // TODO This doesn't work yet. Unfortunately the YAML parsing for entity and location items // is different (e.g. BrooklynComponentTemplateResolver.decorateSpec only deals with entities). // That is too big to deal with in this pull request that targets entity config! - @Test(groups="Live", enabled=false) + @Test(groups={"Live", "WIP", "Broken"}, enabled=false) public void testMergesCatalogLocationProperties() throws Exception { addCatalogItems( "brooklyn.catalog:", http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java index 0dce749..869558b 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java @@ -52,7 +52,7 @@ public class ConfigParametersYamlTest extends AbstractYamlTest { " - name: testConfigParametersListedInType.mykey", " description: myDescription", " type: java.util.Map", - " inheritance.type: merge", + " inheritance.type: deep_merge", " default: {myDefaultKey: myDefaultVal}"); String yaml = Joiner.on("\n").join( http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java b/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java index 1526df0..d9cb7ad 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java @@ -115,7 +115,7 @@ public class BrooklynConfigKeys { public static final MapConfigKey<Object> SHELL_ENVIRONMENT = new MapConfigKey.Builder<Object>(Object.class, "shell.env") .description("Map of environment variables to pass to the runtime shell") .defaultValue(ImmutableMap.<String,Object>of()) - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); public static final AttributeSensorAndConfigKey<String, String> INSTALL_DIR = new TemplatedStringAttributeSensorAndConfigKey("install.dir", "Directory for this software to be installed in", http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java index 7337bbf..97f61f7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java @@ -157,7 +157,7 @@ public class EntityConfigMap extends AbstractConfigMapImpl { parentValue = Maybe.absent(); } break; - case MERGE: + case DEEP_MERGE: if (((ConfigKeySelfExtracting<T>)key).isSet(inheritedConfig)) { parentValue = Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(inheritedConfig, exec)); } else if (inheritedConfigBag.containsKey(key)) { @@ -177,7 +177,7 @@ public class EntityConfigMap extends AbstractConfigMapImpl { switch (parentInheritance) { case IF_NO_EXPLICIT_VALUE: return ownValue.isPresent() ? ownValue : parentValue; - case MERGE: + case DEEP_MERGE: return (Maybe<T>) deepMerge(ownValue, parentValue, key); case NONE: return ownValue; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcess.java ---------------------------------------------------------------------- diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcess.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcess.java index 304b6c7..68a95ca 100644 --- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcess.java +++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcess.java @@ -175,7 +175,7 @@ public interface SoftwareProcess extends Entity, Startable { @SetFromFlag("preInstallFiles") MapConfigKey<String> PRE_INSTALL_FILES = new MapConfigKey.Builder<String>(String.class, "files.preinstall") .description("Mapping of files, to be copied before install, to destination name relative to installDir") - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); /** @@ -187,7 +187,7 @@ public interface SoftwareProcess extends Entity, Startable { @SetFromFlag("preInstallTemplates") MapConfigKey<String> PRE_INSTALL_TEMPLATES = new MapConfigKey.Builder<String>(String.class, "templates.preinstall") .description("Mapping of templates, to be filled in and copied before pre-install, to destination name relative to installDir") - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); /** @@ -202,7 +202,7 @@ public interface SoftwareProcess extends Entity, Startable { @SetFromFlag("installFiles") MapConfigKey<String> INSTALL_FILES = new MapConfigKey.Builder<String>(String.class, "files.install") .description("Mapping of files, to be copied before install, to destination name relative to installDir") - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); /** @@ -214,7 +214,7 @@ public interface SoftwareProcess extends Entity, Startable { @SetFromFlag("installTemplates") MapConfigKey<String> INSTALL_TEMPLATES = new MapConfigKey.Builder<String>(String.class, "templates.install") .description("Mapping of templates, to be filled in and copied before install, to destination name relative to installDir") - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); /** @@ -229,7 +229,7 @@ public interface SoftwareProcess extends Entity, Startable { @SetFromFlag("runtimeFiles") MapConfigKey<String> RUNTIME_FILES = new MapConfigKey.Builder<String>(String.class, "files.runtime") .description("Mapping of files, to be copied before customisation, to destination name relative to runDir") - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); /** @@ -241,14 +241,14 @@ public interface SoftwareProcess extends Entity, Startable { @SetFromFlag("runtimeTemplates") MapConfigKey<String> RUNTIME_TEMPLATES = new MapConfigKey.Builder<String>(String.class, "templates.runtime") .description("Mapping of templates, to be filled in and copied before customisation, to destination name relative to runDir") - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); @SetFromFlag("provisioningProperties") MapConfigKey<Object> PROVISIONING_PROPERTIES = new MapConfigKey.Builder<Object>(Object.class, "provisioning.properties") .description("Custom properties to be passed in when provisioning a new machine") .defaultValue(ImmutableMap.<String, Object>of()) - .typeInheritance(ConfigInheritance.MERGE) + .typeInheritance(ConfigInheritance.DEEP_MERGE) .build(); @SetFromFlag("maxRebindSensorsDelay") http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6dad5de0/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java index 81db3cc..3ff163f 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java +++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritance.java @@ -27,15 +27,16 @@ import com.google.common.annotations.Beta; @SuppressWarnings("serial") public abstract class ConfigInheritance implements Serializable { + @Beta public enum InheritanceMode { NONE, IF_NO_EXPLICIT_VALUE, - MERGE + DEEP_MERGE } public static final ConfigInheritance NONE = new None(); public static final ConfigInheritance ALWAYS = new Always(); - public static final ConfigInheritance MERGE = new Merged(); + public static final ConfigInheritance DEEP_MERGE = new Merged(); public static ConfigInheritance fromString(String val) { if (Strings.isBlank(val)) return null; @@ -44,8 +45,9 @@ public abstract class ConfigInheritance implements Serializable { return NONE; case "always": return ALWAYS; - case "merge" : - return MERGE; + case "deepmerge" : + case "deep_merge" : + return DEEP_MERGE; default: throw new IllegalArgumentException("Invalid config-inheritance '"+val+"' (legal values are none, always or merge)"); } @@ -73,7 +75,7 @@ public abstract class ConfigInheritance implements Serializable { private static class Merged extends ConfigInheritance { @Override public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { - return InheritanceMode.MERGE; + return InheritanceMode.DEEP_MERGE; } } }
