You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sv...@apache.org on 2016/06/06 14:45:55 UTC

[1/9] brooklyn-server git commit: Makes config parent-inheritance “merge” configurable

Repository: brooklyn-server
Updated Branches:
  refs/heads/master 2276de020 -> 2edf7168d


Makes config parent-inheritance \u201cmerge\u201d configurable

Adds:
* ConfigKey.getParentInheritance (deprecating getInheritance)
* ConfigKey.getTypeInheritance (not yet implemented)
* Adds MapConfigKey.Builder
* Some SoftwareProcess config keys now set typeInheritance(MERGE)

Breaking backwards-compatibility changes:
* ConfigInheritance.isInherited return type changed (method was @Beta)
* VanillaSoftwareProcess keys changed to MapConfigKey, rather than
  ConfigKey<Map>
* BasicConfigKey.Builder: generics changed, to allow sub-typing by
  MapConfigKey.Builder
* AbstractStructuredConfigKey.subType field changed from public to
  protected

Change of semantics:
* Previously, when looking up entity config, the order of preference
  (in EntityConfigMap) was:
   1. look up own config, using key
   2. look up inherited config, using key
   3. look up \u201cown bag\u201d, using key\u2019s name
   4. look up \u201cinherited bag\u201d, using key\u2019s name.
  Now the order is (1), (3), (2), (4).


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/57beafa2
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/57beafa2
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/57beafa2

Branch: refs/heads/master
Commit: 57beafa24bcfa5ffe9db16fd8e9dcb51acba0eab
Parents: 97807f2
Author: Aled Sage <al...@gmail.com>
Authored: Fri May 27 09:43:53 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:08 2016 +0100

----------------------------------------------------------------------
 .../camp/brooklyn/BrooklynCampConstants.java    |   4 +-
 .../brooklyn/ConfigInheritanceYamlTest.java     | 244 ++++++++++++++++++-
 .../brooklyn/core/config/BasicConfigKey.java    | 169 +++++++++----
 .../apache/brooklyn/core/config/ConfigKeys.java |   8 +-
 .../brooklyn/core/config/MapConfigKey.java      |  77 +++++-
 .../internal/AbstractStructuredConfigKey.java   |   2 +-
 .../core/entity/BrooklynConfigKeys.java         |  11 +-
 .../core/entity/internal/EntityConfigMap.java   | 129 +++++++---
 .../entity/lifecycle/ServiceStateLogic.java     |   4 +-
 .../core/location/AbstractLocation.java         |   4 +-
 .../brooklyn/core/objs/BasicSpecParameter.java  |  15 +-
 .../sensor/AttributeSensorAndConfigKey.java     |   2 +-
 .../sensor/PortAttributeSensorAndConfigKey.java |   2 +-
 .../brooklyn/core/config/ConfigKeysTest.java    |   4 +-
 .../entity/ConfigEntityInheritanceTest.java     |   4 +-
 .../entity/software/base/SoftwareProcess.java   |  54 ++--
 .../brooklyn/config/ConfigInheritance.java      |  41 +++-
 .../org/apache/brooklyn/config/ConfigKey.java   |  12 +
 18 files changed, 643 insertions(+), 143 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/BrooklynCampConstants.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/BrooklynCampConstants.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/BrooklynCampConstants.java
index d3641f2..5a2d4e6 100644
--- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/BrooklynCampConstants.java
+++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/BrooklynCampConstants.java
@@ -34,12 +34,12 @@ public class BrooklynCampConstants {
     public static final ConfigKey<String> PLAN_ID = ConfigKeys.builder(String.class, "camp.plan.id")
             .description("Identifier supplied in the deployment plan for component to which this entity corresponds "
                         + "(human-readable, for correlating across plan, template, and instance)")
-            .inheritance(ConfigInheritance.NONE)
+            .parentInheritance(ConfigInheritance.NONE)
             .build();
 
     public static final ConfigKey<String> TEMPLATE_ID = ConfigKeys.builder(String.class, "camp.template.id")
             .description("UID of the component in the CAMP template from which this entity was created")
-            .inheritance(ConfigInheritance.NONE)
+            .parentInheritance(ConfigInheritance.NONE)
             .build();
 
     public static final ConfigKey<CampPlatform> CAMP_PLATFORM = ConfigKeys.newConfigKey(CampPlatform.class, "brooklyn.camp.platform",

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 a9bbcb0..1d37589 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,6 +31,7 @@ import java.util.concurrent.Executors;
 
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.MapConfigKey;
 import org.apache.brooklyn.core.entity.EntityAsserts;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.entity.TestEntity;
@@ -112,8 +113,8 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
     public void tearDown() throws Exception {
         super.tearDown();
         if (executor != null) executor.shutdownNow();
-        if (emptyFile != null) Files.delete(emptyFile);
-        if (emptyFile2 != null) Files.delete(emptyFile2);
+        if (emptyFile != null) Files.deleteIfExists(emptyFile);
+        if (emptyFile2 != null) Files.deleteIfExists(emptyFile2);
     }
     
     @Test
@@ -134,6 +135,43 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
     }
     
     @Test
+    public void testExtendsSuperTypeConfig() throws Exception {
+        String yaml = Joiner.on("\n").join(
+                "location: localhost-stub",
+                "services:",
+                "- type: EmptySoftwareProcess-with-conf",
+                "  brooklyn.config:",
+                "    shell.env:",
+                "      ENV2: myEnv2",
+                "    templates.preinstall:",
+                "      "+emptyFile2.toUri()+": myfile2",
+                "    files.preinstall:",
+                "      "+emptyFile2.toUri()+": myfile2",
+                "    templates.install:",
+                "      "+emptyFile2.toUri()+": myfile2",
+                "    files.install:",
+                "      "+emptyFile2.toUri()+": myfile2",
+                "    templates.runtime:",
+                "      "+emptyFile2.toUri()+": myfile2",
+                "    files.runtime:",
+                "      "+emptyFile2.toUri()+": myfile2",
+                "    provisioning.properties:",
+                "      mykey2: myval2",
+                "      templateOptions:",
+                "        myOptionsKey2: myOptionsVal2");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+        
+        assertEmptySoftwareProcessConfig(
+                entity,
+                ImmutableMap.of("ENV1", "myEnv1", "ENV2", "myEnv2"),
+                ImmutableMap.of(emptyFile.toUri().toString(), "myfile", emptyFile2.toUri().toString(), "myfile2"),
+                ImmutableMap.of("mykey", "myval", "mykey2", "myval2", "templateOptions", 
+                        ImmutableMap.of("myOptionsKey", "myOptionsVal", "myOptionsKey2", "myOptionsVal2")));
+    }
+    
+    @Test
     public void testInheritsParentConfig() throws Exception {
         String yaml = Joiner.on("\n").join(
                 "location: localhost-stub",
@@ -300,14 +338,212 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
                 ImmutableMap.of("mykey2", "myval2", "templateOptions", ImmutableMap.of("myOptionsKey2", "myOptionsVal2")));
     }
     
+    @Test
+    public void testEntityTypeInheritanceOptions() throws Exception {
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  itemType: entity",
+                "  items:",
+                "  - id: entity-with-keys",
+                "    item:",
+                "      type: org.apache.brooklyn.core.test.entity.TestEntity",
+                "      brooklyn.parameters:",
+                "      - name: map.type-merged",
+                "        type: java.util.Map",
+                "        inheritance.type: merge",
+                "        default: {myDefaultKey: myDefaultVal}",
+                "      - name: map.type-always",
+                "        type: java.util.Map",
+                "        inheritance.type: always",
+                "        default: {myDefaultKey: myDefaultVal}",
+                "      - name: map.type-never",
+                "        type: java.util.Map",
+                "        inheritance.type: none",
+                "        default: {myDefaultKey: myDefaultVal}",
+                "  - id: entity-with-conf",
+                "    item:",
+                "      type: entity-with-keys",
+                "      brooklyn.config:",
+                "        map.type-merged:",
+                "          mykey: myval",
+                "        map.type-always:",
+                "          mykey: myval",
+                "        map.type-never:",
+                "          mykey: myval");
+        
+        // Test retrieval of defaults
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: entity-with-keys");
+            
+            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("myDefaultKey", "myDefaultVal"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("myDefaultKey", "myDefaultVal"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("myDefaultKey", "myDefaultVal"));
+        }
+
+        // Test override defaults
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: entity-with-keys",
+                    "  brooklyn.config:",
+                    "    map.type-merged:",
+                    "      mykey2: myval2",
+                    "    map.type-always:",
+                    "      mykey2: myval2",
+                    "    map.type-never:",
+                    "      mykey2: myval2");
+            
+            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("mykey2", "myval2"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("mykey2", "myval2"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("mykey2", "myval2"));
+        }
+
+        // Test retrieval of explicit config
+        // TODO what should "never" mean here? Should we get the default value?
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: entity-with-conf");
+            
+            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("mykey", "myval"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("mykey", "myval"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("myDefaultKey", "myDefaultVal"));
+        }
+        
+        // Test merging/overriding of explicit config
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: entity-with-conf",
+                    "  brooklyn.config:",
+                    "    map.type-merged:",
+                    "      mykey2: myval2",
+                    "    map.type-always:",
+                    "      mykey2: myval2",
+                    "    map.type-never:",
+                    "      mykey2: myval2");
+            
+            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("mykey", "myval", "mykey2", "myval2"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("mykey2", "myval2"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("mykey2", "myval2"));
+        }
+    }
+    
+    @Test
+    public void testParentInheritanceOptions() throws Exception {
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  itemType: entity",
+                "  items:",
+                "  - id: entity-with-keys",
+                "    item:",
+                "      type: org.apache.brooklyn.core.test.entity.TestEntity",
+                "      brooklyn.parameters:",
+                "      - name: map.type-merged",
+                "        type: java.util.Map",
+                "        inheritance.parent: merge",
+                "        default: {myDefaultKey: myDefaultVal}",
+                "      - name: map.type-always",
+                "        type: java.util.Map",
+                "        inheritance.parent: always",
+                "        default: {myDefaultKey: myDefaultVal}",
+                "      - name: map.type-never",
+                "        type: java.util.Map",
+                "        inheritance.parent: none",
+                "        default: {myDefaultKey: myDefaultVal}");
+        
+        // Test retrieval of defaults
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: org.apache.brooklyn.entity.stock.BasicApplication",
+                    "  brooklyn.children:",
+                    "  - type: entity-with-keys");
+            
+            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("myDefaultKey", "myDefaultVal"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("myDefaultKey", "myDefaultVal"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("myDefaultKey", "myDefaultVal"));
+        }
+
+        // Test override defaults in parent entity
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: org.apache.brooklyn.entity.stock.BasicApplication",
+                    "  brooklyn.config:",
+                    "    map.type-merged:",
+                    "      mykey: myval",
+                    "    map.type-always:",
+                    "      mykey: myval",
+                    "    map.type-never:",
+                    "      mykey: myval",
+                    "  brooklyn.children:",
+                    "  - type: entity-with-keys");
+            
+            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("mykey", "myval"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("mykey", "myval"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("myDefaultKey", "myDefaultVal"));
+        }
+
+        // Test merging/overriding of explicit config
+        {
+            String yaml = Joiner.on("\n").join(
+                    "services:",
+                    "- type: org.apache.brooklyn.entity.stock.BasicApplication",
+                    "  brooklyn.config:",
+                    "    map.type-merged:",
+                    "      mykey: myval",
+                    "    map.type-always:",
+                    "      mykey: myval",
+                    "    map.type-never:",
+                    "      mykey: myval",
+                    "  brooklyn.children:",
+                    "  - type: entity-with-keys",
+                    "    brooklyn.config:",
+                    "      map.type-merged:",
+                    "        mykey2: myval2",
+                    "      map.type-always:",
+                    "        mykey2: myval2",
+                    "      map.type-never:",
+                    "        mykey2: myval2");
+            
+            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("mykey", "myval", "mykey2", "myval2"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-always")), ImmutableMap.of("mykey2", "myval2"));
+            assertEquals(entity.config().get(entity.getEntityType().getConfigKey("map.type-never")), ImmutableMap.of("mykey2", "myval2"));
+        }
+    }
+    
     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));
         
-        List<ConfigKey<Map<String,String>>> keys = ImmutableList.of(EmptySoftwareProcess.PRE_INSTALL_TEMPLATES, 
+        List<MapConfigKey<String>> keys = ImmutableList.of(EmptySoftwareProcess.PRE_INSTALL_TEMPLATES, 
                 EmptySoftwareProcess.PRE_INSTALL_FILES, EmptySoftwareProcess.INSTALL_TEMPLATES, 
                 EmptySoftwareProcess.INSTALL_FILES, EmptySoftwareProcess.RUNTIME_TEMPLATES, 
                 EmptySoftwareProcess.RUNTIME_FILES);
-        for (ConfigKey<Map<String,String>> key : keys) {
+        for (MapConfigKey<String> key : keys) {
             EntityAsserts.assertConfigEquals(entity, key, expectedFiles);
         }
         

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 f158c2c..a1285d8 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
@@ -54,66 +54,105 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
     
     private static final Splitter dots = Splitter.on('.');
 
-    public static <T> Builder<T> builder(TypeToken<T> type) {
-        return new Builder<T>().type(type);
+    public static <T> Builder<T,?> builder(TypeToken<T> type) {
+        return new ConcreteBuilder<T>().type(type);
     }
 
-    public static <T> Builder<T> builder(Class<T> type) {
-        return new Builder<T>().type(type);
+    public static <T> Builder<T,?> builder(Class<T> type) {
+        return new ConcreteBuilder<T>().type(type);
     }
 
-    public static <T> Builder<T> builder(TypeToken<T> type, String name) {
-        return new Builder<T>().type(type).name(name);
+    public static <T> Builder<T,?> builder(TypeToken<T> type, String name) {
+        return new ConcreteBuilder<T>(type, name);
     }
 
-    public static <T> Builder<T> builder(Class<T> type, String name) {
-        return new Builder<T>().type(type).name(name);
+    public static <T> Builder<T,?> builder(Class<T> type, String name) {
+        return new ConcreteBuilder<T>().type(type).name(name);
     }
 
-    public static <T> Builder<T> builder(ConfigKey<T> key) {
-        return new Builder<T>()
-            .name(checkNotNull(key.getName(), "name"))
-            .type(checkNotNull(key.getTypeToken(), "type"))
-            .description(key.getDescription())
-            .defaultValue(key.getDefaultValue())
-            .reconfigurable(key.isReconfigurable())
-            .inheritance(key.getInheritance())
-            .constraint(key.getConstraint());
+    public static <T> Builder<T,?> builder(ConfigKey<T> key) {
+        return new ConcreteBuilder<T>(key);
     }
 
-    public static class Builder<T> {
-        private String name;
-        private TypeToken<T> type;
-        private String description;
-        private T defaultValue;
-        private boolean reconfigurable;
-        private Predicate<? super T> constraint = Predicates.alwaysTrue();
-        private ConfigInheritance inheritance;
+    private static class ConcreteBuilder<T> extends Builder<T, ConcreteBuilder<T>> {
+        public ConcreteBuilder() {
+        }
+        public ConcreteBuilder(TypeToken<T> type, String name) {
+            super(type, name);
+        }
+        public ConcreteBuilder(ConfigKey<T> key) {
+            super(key);
+        }
+        @Override protected ConcreteBuilder<T> self() {
+            return this;
+        }
+    }
+
+    public abstract static class Builder<T, B extends Builder<T,B>> {
+        protected String name;
+        protected TypeToken<T> type;
+        protected String description;
+        protected T defaultValue;
+        protected boolean reconfigurable;
+        protected Predicate<? super T> constraint = Predicates.alwaysTrue();
+        protected ConfigInheritance parentInheritance;
+        protected ConfigInheritance typeInheritance;
         
-        public Builder<T> name(String val) {
-            this.name = val; return this;
+        protected abstract B self();
+
+        public Builder() {
+        }
+        public Builder(TypeToken<T> type, String name) {
+            this.type = type;
+            this.name = name;
+        }
+        public Builder(Class<T> type, String name) {
+            this(TypeToken.of(type), name);
+        }
+        public Builder(ConfigKey<T> key) {
+            this.type = checkNotNull(key.getTypeToken(), "type");
+            this.name = checkNotNull(key.getName(), "name");
+            description(key.getDescription());
+            defaultValue(key.getDefaultValue());
+            reconfigurable(key.isReconfigurable());
+            parentInheritance(key.getParentInheritance());
+            typeInheritance(key.getTypeInheritance());
+            constraint(key.getConstraint());
+        }
+        public B name(String val) {
+            this.name = val; return self();
+        }
+        public B type(Class<T> val) {
+            this.type = TypeToken.of(val); return self();
         }
-        public Builder<T> type(Class<T> val) {
-            this.type = TypeToken.of(val); return this;
+        public B type(TypeToken<T> val) {
+            this.type = val; return self();
         }
-        public Builder<T> type(TypeToken<T> val) {
-            this.type = val; return this;
+        public B description(String val) {
+            this.description = val; return self();
         }
-        public Builder<T> description(String val) {
-            this.description = val; return this;
+        public B defaultValue(T val) {
+            this.defaultValue = val; return self();
         }
-        public Builder<T> defaultValue(T val) {
-            this.defaultValue = val; return this;
+        public B reconfigurable(boolean val) {
+            this.reconfigurable = val; return self();
         }
-        public Builder<T> reconfigurable(boolean val) {
-            this.reconfigurable = val; return this;
+        public B parentInheritance(ConfigInheritance val) {
+            this.parentInheritance = val; return self();
         }
-        public Builder<T> inheritance(ConfigInheritance val) {
-            this.inheritance = val; return this;
+        public B typeInheritance(ConfigInheritance val) {
+            this.typeInheritance = val; return self();
+        }
+        /**
+         * @deprecated since 0.10.0; use {@link #parentInheritance(ConfigInheritance)}
+         */
+        @Deprecated
+        public B inheritance(ConfigInheritance val) {
+            return parentInheritance(val);
         }
         @Beta
-        public Builder<T> constraint(Predicate<? super T> constraint) {
-            this.constraint = checkNotNull(constraint, "constraint"); return this;
+        public B constraint(Predicate<? super T> constraint) {
+            this.constraint = checkNotNull(constraint, "constraint"); return self();
         }
         public BasicConfigKey<T> build() {
             return new BasicConfigKey<T>(this);
@@ -127,14 +166,22 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
         }
     }
     
-    private String name;
-    private TypeToken<T> typeToken;
-    private Class<? super T> type;
-    private String description;
-    private T defaultValue;
-    private boolean reconfigurable;
+    protected String name;
+    protected TypeToken<T> typeToken;
+    protected Class<? super T> type;
+    protected String description;
+    protected T defaultValue;
+    protected boolean reconfigurable;
+    protected ConfigInheritance typeInheritance;
+    protected ConfigInheritance parentInheritance;
+    protected Predicate<? super T> constraint;
+
+    /**
+     * Kept only for backwards compatibility with serialised state; when read, it's value is used 
+     * for {@link #parentInheritance} and then set to null.
+     */
+    @Deprecated
     private ConfigInheritance inheritance;
-    private Predicate<? super T> constraint;
 
     // FIXME In groovy, fields were `public final` with a default constructor; do we need the gson?
     public BasicConfigKey() { /* for gson */ }
@@ -171,14 +218,15 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
         this.constraint = Predicates.alwaysTrue();
     }
 
-    public BasicConfigKey(Builder<T> builder) {
+    public BasicConfigKey(Builder<T,?> builder) {
         this.name = checkNotNull(builder.name, "name");
         this.type = TypeTokens.getRawTypeIfRaw(checkNotNull(builder.type, "type"));
         this.typeToken = TypeTokens.getTypeTokenIfNotRaw(builder.type);
         this.description = builder.description;
         this.defaultValue = builder.defaultValue;
         this.reconfigurable = builder.reconfigurable;
-        this.inheritance = builder.inheritance;
+        this.parentInheritance = builder.parentInheritance;
+        this.typeInheritance = builder.typeInheritance;
         // Note: it's intentionally possible to have default values that are not valid
         // per the configured constraint. If validity were checked here any class that
         // contained a weirdly-defined config key would fail to initialise.
@@ -214,10 +262,23 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
         return reconfigurable;
     }
     
-    /** @see ConfigKey#getInheritance() */
-    @Override @Nullable
+    @Deprecated @Override @Nullable
     public ConfigInheritance getInheritance() {
-        return inheritance;
+        return getParentInheritance();
+    }
+
+    @Override @Nullable
+    public ConfigInheritance getTypeInheritance() {
+        return typeInheritance;
+    }
+
+    @Override @Nullable
+    public ConfigInheritance getParentInheritance() {
+        if (parentInheritance == null && inheritance != null) {
+            parentInheritance = inheritance;
+            inheritance = null;
+        }
+        return parentInheritance;
     }
 
     /** @see ConfigKey#getConstraint() */
@@ -306,7 +367,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
 
         /** builder here should be based on the same key passed in as parent */
         @Beta
-        public BasicConfigKeyOverwriting(Builder<T> builder, ConfigKey<T> parent) {
+        public BasicConfigKeyOverwriting(Builder<T,?> builder, ConfigKey<T> parent) {
             super(builder);
             parentKey = parent;
             Preconditions.checkArgument(Objects.equal(builder.name, parent.getName()), "Builder must use key of the same name.");

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java
index aeff698..f2bbc4a 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java
@@ -126,16 +126,16 @@ public class ConfigKeys {
                 name, description, defaultValue);
     }
 
-    public static <T> BasicConfigKey.Builder<T> builder(Class<T> type) {
+    public static <T> BasicConfigKey.Builder<T,?> builder(Class<T> type) {
         return BasicConfigKey.builder(type);
     }
-    public static <T> BasicConfigKey.Builder<T> builder(TypeToken<T> type) {
+    public static <T> BasicConfigKey.Builder<T,?> builder(TypeToken<T> type) {
         return BasicConfigKey.builder(type);
     }
-    public static <T> BasicConfigKey.Builder<T> builder(Class<T> type, String name) {
+    public static <T> BasicConfigKey.Builder<T,?> builder(Class<T> type, String name) {
         return BasicConfigKey.builder(type, name);
     }
-    public static <T> BasicConfigKey.Builder<T> builder(TypeToken<T> type, String name) {
+    public static <T> BasicConfigKey.Builder<T,?> builder(TypeToken<T> type, String name) {
         return BasicConfigKey.builder(type, name);
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/main/java/org/apache/brooklyn/core/config/MapConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/MapConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/config/MapConfigKey.java
index 96216ff..d06e160 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/MapConfigKey.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/MapConfigKey.java
@@ -18,6 +18,8 @@
  */
 package org.apache.brooklyn.core.config;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -35,6 +37,7 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Supplier;
 import com.google.common.collect.Maps;
+import com.google.common.reflect.TypeToken;
 
 /** A config key which represents a map, where contents can be accessed directly via subkeys.
  * Items added directly to the map must be of type map, and can be updated by:
@@ -49,11 +52,83 @@ import com.google.common.collect.Maps;
  * </ul>
  */
 //TODO Create interface
+//TODO The BasicConfigKey.builder(Class,name) methods mean we can't define them on this sub-type,
+//     because we want builder(Class<V>), which corresponds to BasicConfigKey<Map<String, V>.
 public class MapConfigKey<V> extends AbstractStructuredConfigKey<Map<String,V>,Map<String,Object>,V> {
     
     private static final long serialVersionUID = -6126481503795562602L;
     private static final Logger log = LoggerFactory.getLogger(MapConfigKey.class);
-    
+
+    public static <V> Builder<V> builder(MapConfigKey<V> key) {
+        return new Builder<V>(key);
+    }
+
+    public static class Builder<V> extends BasicConfigKey.Builder<Map<String, V>,Builder<V>> {
+        protected Class<V> subType;
+        
+        public Builder(TypeToken<V> subType, String name) {
+            super(new TypeToken<Map<String, V>>() {}, name);
+            this.subType = (Class<V>) subType.getRawType();
+        }
+        public Builder(Class<V> subType, String name) {
+            super(new TypeToken<Map<String, V>>() {}, name);
+            this.subType = checkNotNull(subType, "subType");
+        }
+        public Builder(MapConfigKey<V> key) {
+            super(checkNotNull(key.getTypeToken(), "type"), checkNotNull(key.getName(), "name"));
+            description(key.getDescription());
+            defaultValue(key.getDefaultValue());
+            reconfigurable(key.isReconfigurable());
+            parentInheritance(key.getParentInheritance());
+            typeInheritance(key.getTypeInheritance());
+            constraint(key.getConstraint());
+        }
+        public Builder<V> self() {
+            return this;
+        }
+        @Override
+        @Deprecated
+        public Builder<V> name(String val) {
+            throw new UnsupportedOperationException("Builder must be constructed with name");
+        }
+        @Override
+        @Deprecated
+        public Builder<V> type(Class<Map<String, V>> val) {
+            throw new UnsupportedOperationException("Builder must be constructed with type");
+        }
+        @Override
+        @Deprecated
+        public Builder<V> type(TypeToken<Map<String, V>> val) {
+            throw new UnsupportedOperationException("Builder must be constructed with type");
+        }
+        @Override
+        public MapConfigKey<V> build() {
+            return new MapConfigKey<V>(this);
+        }
+        
+        public String getName() {
+            return name;
+        }
+        public String getDescription() {
+            return description;
+        }
+    }
+
+    protected MapConfigKey(Builder<V> builder) {
+        super((Class)Map.class,
+                checkNotNull(builder.subType, "subType"),
+                checkNotNull(builder.name, "name"),
+                builder.description,
+                builder.defaultValue);
+        this.reconfigurable = builder.reconfigurable;
+        this.parentInheritance = builder.parentInheritance;
+        this.typeInheritance = builder.typeInheritance;
+        // Note: it's intentionally possible to have default values that are not valid
+        // per the configured constraint. If validity were checked here any class that
+        // contained a weirdly-defined config key would fail to initialise.
+        this.constraint = checkNotNull(builder.constraint, "constraint");
+    }
+
     public MapConfigKey(Class<V> subType, String name) {
         this(subType, name, name, null);
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractStructuredConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractStructuredConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractStructuredConfigKey.java
index 8c7c610..532e1da 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractStructuredConfigKey.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractStructuredConfigKey.java
@@ -34,7 +34,7 @@ public abstract class AbstractStructuredConfigKey<T,RawT,V> extends BasicConfigK
 
     private static final long serialVersionUID = 7806267541029428561L;
 
-    public final Class<V> subType;
+    protected final Class<V> subType;
 
     public AbstractStructuredConfigKey(Class<T> type, Class<V> subType, String name, String description, T defaultValue) {
         super(type, name, description, defaultValue);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 607a22b..1526df0 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
@@ -24,6 +24,7 @@ import static org.apache.brooklyn.core.config.ConfigKeys.newConfigKeyWithPrefix;
 import static org.apache.brooklyn.core.config.ConfigKeys.newStringConfigKey;
 
 import org.apache.brooklyn.api.location.Location;
+import org.apache.brooklyn.config.ConfigInheritance;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.MapConfigKey;
@@ -111,11 +112,11 @@ public class BrooklynConfigKeys {
     public static final ConfigKey<String> POST_LAUNCH_COMMAND = ConfigKeys.newStringConfigKey("post.launch.command",
             "Command to be run after the launch method being called on the driver");
 
-    public static final MapConfigKey<Object> SHELL_ENVIRONMENT = new MapConfigKey<Object>(
-            Object.class,
-            "shell.env", 
-            "Map of environment variables to pass to the runtime shell", 
-            ImmutableMap.<String,Object>of());
+    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)
+            .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/57beafa2/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 da209e1..7337bbf 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
@@ -21,7 +21,6 @@ package org.apache.brooklyn.core.entity.internal;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.apache.brooklyn.util.groovy.GroovyJavaMethods.elvis;
 
-import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -30,11 +29,14 @@ import java.util.Set;
 import org.apache.brooklyn.api.mgmt.ExecutionContext;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.config.ConfigInheritance;
+import org.apache.brooklyn.config.ConfigInheritance.InheritanceMode;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.Sanitizer;
 import org.apache.brooklyn.core.config.StructuredConfigKey;
 import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl;
 import org.apache.brooklyn.core.entity.AbstractEntity;
+import org.apache.brooklyn.core.location.AbstractLocation;
+import org.apache.brooklyn.util.collections.CollectionMerger;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.flags.FlagUtils;
@@ -86,23 +88,20 @@ public class EntityConfigMap extends AbstractConfigMapImpl {
 
     @SuppressWarnings("unchecked")
     public <T> T getConfig(ConfigKey<T> key, T defaultValue) {
-        // FIXME What about inherited task in config?!
-        //              alex says: think that should work, no?
-        // FIXME What if someone calls getConfig on a task, before setting parent app?
-        //              alex says: not supported (throw exception, or return the task)
-        
         // In case this entity class has overridden the given key (e.g. to set default), then retrieve this entity's key
         // TODO If ask for a config value that's not in our configKeys, should we really continue with rest of method and return key.getDefaultValue?
         //      e.g. SshBasedJavaAppSetup calls setAttribute(JMX_USER), which calls getConfig(JMX_USER)
         //           but that example doesn't have a default...
         ConfigKey<T> ownKey = entity!=null ? (ConfigKey<T>)elvis(entity.getEntityType().getConfigKey(key.getName()), key) : key;
         
-        ConfigInheritance inheritance = key.getInheritance();
-        if (inheritance==null) inheritance = ownKey.getInheritance(); 
+        // TODO Confirm desired behaviour: I switched this around to get ownKey.getParentInheritance first.
+        ConfigInheritance inheritance = ownKey.getParentInheritance();
+        if (inheritance==null) inheritance = key.getInheritance(); 
         if (inheritance==null) {
             // TODO we could warn by introducing a temporary "ALWAYS_BUT_WARNING" instance
-            inheritance = getDefaultInheritance(); 
+            inheritance = getDefaultParentInheritance(); 
         }
+        InheritanceMode parentInheritanceMode = inheritance.isInherited(ownKey, entity.getParent(), entity);
         
         // TODO We're notifying of config-changed because currently persistence needs to know when the
         // attributeWhenReady is complete (so it can persist the result).
@@ -111,47 +110,107 @@ public class EntityConfigMap extends AbstractConfigMapImpl {
         // Don't use groovy truth: if the set value is e.g. 0, then would ignore set value and return default!
         if (ownKey instanceof ConfigKeySelfExtracting) {
             Object rawval = ownConfig.get(key);
-            T result = null;
-            boolean complete = false;
-            if (((ConfigKeySelfExtracting<T>)ownKey).isSet(ownConfig)) {
-                ExecutionContext exec = entity.getExecutionContext();
-                result = ((ConfigKeySelfExtracting<T>)ownKey).extractValue(ownConfig, exec);
-                complete = true;
-            } else if (isInherited(ownKey, inheritance) && 
-                    ((ConfigKeySelfExtracting<T>)ownKey).isSet(inheritedConfig)) {
-                ExecutionContext exec = entity.getExecutionContext();
-                result = ((ConfigKeySelfExtracting<T>)ownKey).extractValue(inheritedConfig, exec);
-                complete = true;
-            } else if (localConfigBag.containsKey(ownKey)) {
-                // TODO configBag.get doesn't handle tasks/attributeWhenReady - it only uses TypeCoercions
-                result = localConfigBag.get(ownKey);
-                complete = true;
-            } else if (isInherited(ownKey, inheritance) && 
-                    inheritedConfigBag.containsKey(ownKey)) {
-                result = inheritedConfigBag.get(ownKey);
-                complete = true;
-            }
+            Maybe<T> result = getConfigImpl((ConfigKeySelfExtracting<T>)ownKey, parentInheritanceMode);
 
             if (rawval instanceof Task) {
                 entity.getManagementSupport().getEntityChangeListener().onConfigChanged(key);
             }
-            if (complete) {
-                return result;
+            if (result.isPresent()) {
+                return result.get();
             }
         } else {
             LOG.warn("Config key {} of {} is not a ConfigKeySelfExtracting; cannot retrieve value; returning default", ownKey, this);
         }
         return TypeCoercions.coerce((defaultValue != null) ? defaultValue : ownKey.getDefaultValue(), key.getTypeToken());
     }
+    
+    @SuppressWarnings("unchecked")
+    private <T> Maybe<T> getConfigImpl(ConfigKeySelfExtracting<T> key, InheritanceMode parentInheritance) {
+        ExecutionContext exec = entity.getExecutionContext();
+        Maybe<T> ownValue;
+        Maybe<T> parentValue;
+
+        // Get own value
+        if (((ConfigKeySelfExtracting<T>)key).isSet(ownConfig)) {
+            ownValue = Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(ownConfig, exec));
+        } else if (localConfigBag.containsKey(key)) {
+            // TODO configBag.get doesn't handle tasks/attributeWhenReady - it only uses TypeCoercions
+            // Precedence ordering has changed; previously we'd prefer an explicit isSet(inheritedConfig)
+            // over the localConfigBag.get(key).
+            ownValue = Maybe.of(localConfigBag.get(key));
+        } else {
+            ownValue = Maybe.<T>absent();
+        }
+        
+        // Get the parent-inheritance value (but only if we'll need it)
+        switch (parentInheritance) {
+        case IF_NO_EXPLICIT_VALUE:
+            if (ownValue.isAbsent()) {
+                if (((ConfigKeySelfExtracting<T>)key).isSet(inheritedConfig)) {
+                    parentValue = Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(inheritedConfig, exec));
+                } else if (inheritedConfigBag.containsKey(key)) {
+                    parentValue = Maybe.of(inheritedConfigBag.get(key));
+                } else {
+                    parentValue = Maybe.absent();
+                }
+            } else {
+                parentValue = Maybe.absent();
+            }
+            break;
+        case MERGE:
+            if (((ConfigKeySelfExtracting<T>)key).isSet(inheritedConfig)) {
+                parentValue = Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(inheritedConfig, exec));
+            } else if (inheritedConfigBag.containsKey(key)) {
+                parentValue = Maybe.of(inheritedConfigBag.get(key));
+            } else {
+                parentValue = Maybe.absent();
+            }
+            break;
+        case NONE:
+            parentValue = Maybe.absent();
+            break;
+        default:
+            throw new IllegalStateException("Unsupported parent-inheritance mode for "+key.getName()+": "+parentInheritance);
+        }
+
+        // Merge or override, as appropriate
+        switch (parentInheritance) {
+        case IF_NO_EXPLICIT_VALUE:
+            return ownValue.isPresent() ? ownValue : parentValue;
+        case MERGE:
+            return (Maybe<T>) deepMerge(ownValue, parentValue, key);
+        case NONE:
+            return ownValue;
+        default:
+            throw new IllegalStateException("Unsupported parent-inheritance mode for "+key.getName()+": "+parentInheritance);
+        }
+    }
+    
+    private <T> Maybe<?> deepMerge(Maybe<? extends T> val1, Maybe<? extends T> val2, ConfigKey<?> keyForLogging) {
+        if (val2.isAbsent() || val2.isNull()) {
+            return val1;
+        } else if (val1.isAbsent()) {
+            return val2;
+        } else if (val1.isNull()) {
+            return val1; // an explicit null means an override; don't merge
+        } else if (val1.get() instanceof Map && val2.get() instanceof Map) {
+            return Maybe.of(CollectionMerger.builder().build().merge((Map<?,?>)val1.get(), (Map<?,?>)val2.get()));
+        } else {
+            // cannot merge; just return val1
+            LOG.debug("Cannot merge values for "+keyForLogging.getName()+", because values are not maps: "+val1.get().getClass()+", and "+val2.get().getClass());
+            return val1;
+        }
+    }
 
     private <T> boolean isInherited(ConfigKey<T> key) {
-        return isInherited(key, key.getInheritance());
+        return isInherited(key, key.getParentInheritance());
     }
     private <T> boolean isInherited(ConfigKey<T> key, ConfigInheritance inheritance) {
-        if (inheritance==null) inheritance = getDefaultInheritance(); 
-        return inheritance.isInherited(key, entity.getParent(), entity);
+        if (inheritance==null) inheritance = getDefaultParentInheritance();
+        InheritanceMode mode = inheritance.isInherited(key, entity.getParent(), entity);
+        return mode != null && mode != InheritanceMode.NONE;
     }
-    private ConfigInheritance getDefaultInheritance() {
+    private ConfigInheritance getDefaultParentInheritance() {
         return ConfigInheritance.ALWAYS; 
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java b/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java
index c8f8400..33eb3bf 100644
--- a/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java
+++ b/core/src/main/java/org/apache/brooklyn/core/entity/lifecycle/ServiceStateLogic.java
@@ -387,12 +387,12 @@ public class ServiceStateLogic {
         public static final ConfigKey<QuorumCheck> UP_QUORUM_CHECK = ConfigKeys.builder(QuorumCheck.class, "enricher.service_state.children_and_members.quorum.up")
             .description("Logic for checking whether this service is up, based on children and/or members, defaulting to allowing none but if there are any requiring at least one to be up")
             .defaultValue(QuorumCheck.QuorumChecks.atLeastOneUnlessEmpty())
-            .inheritance(ConfigInheritance.NONE)
+            .parentInheritance(ConfigInheritance.NONE)
             .build();
         public static final ConfigKey<QuorumCheck> RUNNING_QUORUM_CHECK = ConfigKeys.builder(QuorumCheck.class, "enricher.service_state.children_and_members.quorum.running") 
             .description("Logic for checking whether this service is healthy, based on children and/or members running, defaulting to requiring none to be ON-FIRE")
             .defaultValue(QuorumCheck.QuorumChecks.all())
-            .inheritance(ConfigInheritance.NONE)
+            .parentInheritance(ConfigInheritance.NONE)
             .build();
         // TODO items below should probably also have inheritance NONE ?
         public static final ConfigKey<Boolean> DERIVE_SERVICE_NOT_UP = ConfigKeys.newBooleanConfigKey("enricher.service_state.children_and_members.service_up.publish", "Whether to derive a service-not-up indicator from children", true);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 d4e565e..1ca8c2f 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
@@ -44,6 +44,7 @@ import org.apache.brooklyn.api.objs.Configurable;
 import org.apache.brooklyn.api.sensor.Sensor;
 import org.apache.brooklyn.api.sensor.SensorEventListener;
 import org.apache.brooklyn.config.ConfigInheritance;
+import org.apache.brooklyn.config.ConfigInheritance.InheritanceMode;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
 import org.apache.brooklyn.core.BrooklynFeatureEnablement;
@@ -488,7 +489,8 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
         private boolean isInherited(ConfigKey<?> key) {
             ConfigInheritance inheritance = key.getInheritance();
             if (inheritance==null) inheritance = getDefaultInheritance();
-            return inheritance.isInherited(key, getParent(), AbstractLocation.this);
+            InheritanceMode mode = inheritance.isInherited(key, getParent(), AbstractLocation.this);
+            return mode != null && mode != InheritanceMode.NONE;
         }
 
         private ConfigInheritance getDefaultInheritance() {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 89b7cd0..d89c8cb 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
@@ -37,6 +37,7 @@ import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.api.objs.BrooklynType;
 import org.apache.brooklyn.api.objs.SpecParameter;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
+import org.apache.brooklyn.config.ConfigInheritance;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
 import org.apache.brooklyn.core.config.BasicConfigKey;
@@ -227,6 +228,8 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
             String type = (String)inputDef.get("type");
             Object defaultValue = inputDef.get("default");
             Predicate<?> constraints = parseConstraints(inputDef.get("constraints"), loader);
+            ConfigInheritance parentInheritance = parseInheritance(inputDef.get("inheritance.parent"), loader);
+            ConfigInheritance typeInheritance = parseInheritance(inputDef.get("inheritance.type"), loader);
 
             if (name == null) {
                 throw new IllegalArgumentException("'name' value missing from input definition " + obj + " but is required. Check for typos.");
@@ -240,7 +243,9 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
                 .name(name)
                 .description(description)
                 .defaultValue(defaultValue)
-                .constraint(constraints);
+                .constraint(constraints)
+                .parentInheritance(parentInheritance)
+                .typeInheritance(typeInheritance);
             
             if (PortRange.class.equals(typeToken.getRawType())) {
                 sensorType = new PortAttributeSensorAndConfigKey(builder);
@@ -299,6 +304,14 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
                 return Predicates.alwaysTrue();
             }
         }
+        
+        private static ConfigInheritance parseInheritance(Object obj, BrooklynClassLoadingContext loader) {
+            if (obj == null || obj instanceof String) {
+                return ConfigInheritance.fromString((String)obj);
+            } else {
+                throw new IllegalArgumentException ("The config-inheritance '" + obj + "' for a catalog input is invalid format - string supported");
+            }
+        }
     }
 
     private static final class ParseClassParameters {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeSensorAndConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeSensorAndConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeSensorAndConfigKey.java
index 2a26fa7..0580a77 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeSensorAndConfigKey.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/AttributeSensorAndConfigKey.java
@@ -88,7 +88,7 @@ public abstract class AttributeSensorAndConfigKey<ConfigType,SensorType> extends
         configKey = ConfigKeys.newConfigKeyWithDefault(orig.configKey, 
                 TypeCoercions.coerce(defaultValue, orig.configKey.getTypeToken()));
     }
-    public AttributeSensorAndConfigKey(Builder<ConfigType> configKeyBuilder, TypeToken<SensorType> sensorType) {
+    public AttributeSensorAndConfigKey(Builder<ConfigType,?> configKeyBuilder, TypeToken<SensorType> sensorType) {
         super(sensorType, configKeyBuilder.getName(), configKeyBuilder.getDescription());
         configKey = new BasicConfigKey<ConfigType>(configKeyBuilder);
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/main/java/org/apache/brooklyn/core/sensor/PortAttributeSensorAndConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/PortAttributeSensorAndConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/sensor/PortAttributeSensorAndConfigKey.java
index aa396d2..556e7b0 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/PortAttributeSensorAndConfigKey.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/PortAttributeSensorAndConfigKey.java
@@ -70,7 +70,7 @@ public class PortAttributeSensorAndConfigKey extends AttributeSensorAndConfigKey
     public PortAttributeSensorAndConfigKey(PortAttributeSensorAndConfigKey orig, Object defaultValue) {
         super(orig, TypeCoercions.coerce(defaultValue, PortRange.class));
     }
-    public PortAttributeSensorAndConfigKey(BasicConfigKey.Builder<PortRange> builder) {
+    public PortAttributeSensorAndConfigKey(BasicConfigKey.Builder<PortRange,?> builder) {
         super(builder, TypeToken.of(Integer.class));
     }
     

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeysTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeysTest.java b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeysTest.java
index bb5a323..9e82c1a 100644
--- a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeysTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeysTest.java
@@ -82,7 +82,7 @@ public class ConfigKeysTest {
         ConfigKey<String> key = ConfigKeys.builder(String.class, "mykey")
             .description("my descr")
             .defaultValue("my default val")
-            .inheritance(ConfigInheritance.NONE)
+            .parentInheritance(ConfigInheritance.NONE)
             .reconfigurable(true)
             .build();
         
@@ -98,7 +98,7 @@ public class ConfigKeysTest {
         assertEquals(key.getDescription(), "my descr");
         assertEquals(key.getDefaultValue(), "my default val");
         assertEquals(key.isReconfigurable(), true);
-        assertEquals(key.getInheritance(), ConfigInheritance.NONE);
+        assertEquals(key.getParentInheritance(), ConfigInheritance.NONE);
     }
 
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java
index a6b1602..3261d6f 100644
--- a/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java
@@ -167,9 +167,9 @@ public class ConfigEntityInheritanceTest extends BrooklynAppUnitTestSupport {
     
     public static class MyEntityWithPartiallyHeritableConfig extends AbstractEntity {
         public static final ConfigKey<String> HERITABLE = ConfigKeys.builder(String.class, "herit.default").build();
-        public static final ConfigKey<String> UNINHERITABLE = ConfigKeys.builder(String.class, "herit.none").inheritance(ConfigInheritance.NONE).build();
+        public static final ConfigKey<String> UNINHERITABLE = ConfigKeys.builder(String.class, "herit.none").parentInheritance(ConfigInheritance.NONE).build();
         // i find a strange joy in words where the prefix "in-" does not mean not, like inflammable 
-        public static final ConfigKey<String> ALWAYS_HERITABLE = ConfigKeys.builder(String.class, "herit.always").inheritance(ConfigInheritance.ALWAYS).build();
+        public static final ConfigKey<String> ALWAYS_HERITABLE = ConfigKeys.builder(String.class, "herit.always").parentInheritance(ConfigInheritance.ALWAYS).build();
     }
 
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 7ee393a..304b6c7 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
@@ -22,12 +22,14 @@ import java.util.Collection;
 import java.util.Map;
 
 import com.google.common.annotations.Beta;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.reflect.TypeToken;
 
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.location.MachineProvisioningLocation;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
+import org.apache.brooklyn.config.ConfigInheritance;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.annotation.Effector;
 import org.apache.brooklyn.core.config.ConfigKeys;
@@ -39,7 +41,6 @@ import org.apache.brooklyn.core.entity.lifecycle.Lifecycle.Transition;
 import org.apache.brooklyn.core.entity.trait.Startable;
 import org.apache.brooklyn.core.sensor.AttributeSensorAndConfigKey;
 import org.apache.brooklyn.core.sensor.Sensors;
-import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.flags.SetFromFlag;
 import org.apache.brooklyn.util.time.Duration;
 
@@ -50,6 +51,8 @@ public interface SoftwareProcess extends Entity, Startable {
     AttributeSensor<String> SUBNET_HOSTNAME = Attributes.SUBNET_HOSTNAME;
     AttributeSensor<String> SUBNET_ADDRESS = Attributes.SUBNET_ADDRESS;
 
+    // TODO Want this to have typeInheritance.merge as well, but currently only supported for maps
+    @SuppressWarnings("serial")
     ConfigKey<Collection<Integer>> REQUIRED_OPEN_LOGIN_PORTS = ConfigKeys.newConfigKey(
             new TypeToken<Collection<Integer>>() {},
             "requiredOpenLoginPorts",
@@ -169,10 +172,11 @@ public interface SoftwareProcess extends Entity, Startable {
      * @see #PRE_INSTALL_TEMPLATES
      */
     @Beta
-    @SuppressWarnings("serial")
     @SetFromFlag("preInstallFiles")
-    ConfigKey<Map<String, String>> PRE_INSTALL_FILES = ConfigKeys.newConfigKey(new TypeToken<Map<String, String>>() { },
-            "files.preinstall", "Mapping of files, to be copied before install, to destination name relative to installDir");
+    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)
+            .build();
 
     /**
      * Templates to be filled in and then copied to the server before install.
@@ -180,10 +184,11 @@ public interface SoftwareProcess extends Entity, Startable {
      * @see #PRE_INSTALL_FILES
      */
     @Beta
-    @SuppressWarnings("serial")
     @SetFromFlag("preInstallTemplates")
-    ConfigKey<Map<String, String>> PRE_INSTALL_TEMPLATES = ConfigKeys.newConfigKey(new TypeToken<Map<String, String>>() { },
-            "templates.preinstall", "Mapping of templates, to be filled in and copied before pre-install, to destination name relative to installDir");
+    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)
+            .build();
 
     /**
      * Files to be copied to the server before install.
@@ -194,10 +199,11 @@ public interface SoftwareProcess extends Entity, Startable {
      * @see #INSTALL_TEMPLATES
      */
     @Beta
-    @SuppressWarnings("serial")
     @SetFromFlag("installFiles")
-    ConfigKey<Map<String, String>> INSTALL_FILES = ConfigKeys.newConfigKey(new TypeToken<Map<String, String>>() { },
-            "files.install", "Mapping of files, to be copied before install, to destination name relative to installDir");
+    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)
+            .build();
 
     /**
      * Templates to be filled in and then copied to the server before install.
@@ -205,10 +211,11 @@ public interface SoftwareProcess extends Entity, Startable {
      * @see #INSTALL_FILES
      */
     @Beta
-    @SuppressWarnings("serial")
     @SetFromFlag("installTemplates")
-    ConfigKey<Map<String, String>> INSTALL_TEMPLATES = ConfigKeys.newConfigKey(new TypeToken<Map<String, String>>() { },
-            "templates.install", "Mapping of templates, to be filled in and copied before install, to destination name relative to installDir");
+    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)
+            .build();
 
     /**
      * Files to be copied to the server after customisation.
@@ -219,10 +226,11 @@ public interface SoftwareProcess extends Entity, Startable {
      * @see #RUNTIME_TEMPLATES
      */
     @Beta
-    @SuppressWarnings("serial")
     @SetFromFlag("runtimeFiles")
-    ConfigKey<Map<String, String>> RUNTIME_FILES = ConfigKeys.newConfigKey(new TypeToken<Map<String, String>>() { },
-            "files.runtime", "Mapping of files, to be copied before customisation, to destination name relative to runDir");
+    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)
+            .build();
 
     /**
      * Templates to be filled in and then copied to the server after customisation.
@@ -230,14 +238,18 @@ public interface SoftwareProcess extends Entity, Startable {
      * @see #RUNTIME_FILES
      */
     @Beta
-    @SuppressWarnings("serial")
     @SetFromFlag("runtimeTemplates")
-    ConfigKey<Map<String, String>> RUNTIME_TEMPLATES = ConfigKeys.newConfigKey(new TypeToken<Map<String, String>>() { },
-            "templates.runtime", "Mapping of templates, to be filled in and copied before customisation, to destination name relative to runDir");
+    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)
+            .build();
 
     @SetFromFlag("provisioningProperties")
-    MapConfigKey<Object> PROVISIONING_PROPERTIES = new MapConfigKey<Object>(Object.class,
-            "provisioning.properties", "Custom properties to be passed in when provisioning a new machine", MutableMap.<String,Object>of());
+    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)
+            .build();
 
     @SetFromFlag("maxRebindSensorsDelay")
     ConfigKey<Duration> MAXIMUM_REBIND_SENSOR_CONNECT_DELAY = ConfigKeys.newConfigKey(Duration.class,

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/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 4d581eb..81db3cc 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
@@ -20,31 +20,60 @@ package org.apache.brooklyn.config;
 
 import java.io.Serializable;
 
+import org.apache.brooklyn.util.text.Strings;
+
 import com.google.common.annotations.Beta;
 
 @SuppressWarnings("serial")
 public abstract class ConfigInheritance implements Serializable {
 
-    public static final ConfigInheritance ALWAYS = new Always();
+    public enum InheritanceMode {
+        NONE,
+        IF_NO_EXPLICIT_VALUE,
+        MERGE
+    }
+
     public static final ConfigInheritance NONE = new None();
+    public static final ConfigInheritance ALWAYS = new Always();
+    public static final ConfigInheritance MERGE = new Merged();
+    
+    public static ConfigInheritance fromString(String val) {
+        if (Strings.isBlank(val)) return null;
+        switch (val.toLowerCase().trim()) {
+        case "none":
+            return NONE;
+        case "always": 
+            return ALWAYS;
+        case "merge" :
+            return MERGE;
+        default:
+            throw new IllegalArgumentException("Invalid config-inheritance '"+val+"' (legal values are none, always or merge)");
+        }
+    }
     
     private ConfigInheritance() {}
     
     @Beta
-    public abstract boolean isInherited(ConfigKey<?> key, Object from, Object to);
+    public abstract InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to);
 
     private static class Always extends ConfigInheritance {
         @Override
-        public boolean isInherited(ConfigKey<?> key, Object from, Object to) {
-            return true;
+        public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) {
+            return InheritanceMode.IF_NO_EXPLICIT_VALUE;
         }
     }
 
     private static class None extends ConfigInheritance {
         @Override
-        public boolean isInherited(ConfigKey<?> key, Object from, Object to) {
-            return false;
+        public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) {
+            return InheritanceMode.NONE;
         }
     }
     
+    private static class Merged extends ConfigInheritance {
+        @Override
+        public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) {
+            return InheritanceMode.MERGE;
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/57beafa2/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
index 92f174f..3b66e98 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
@@ -84,8 +84,20 @@ public interface ConfigKey<T> {
     boolean isReconfigurable();
 
     /**
+     * @return The sub-typing inheritance model, or <code>null</code> for the default in any context.
+     */
+    @Nullable ConfigInheritance getTypeInheritance();
+
+    /**
+     * @return The inheritance-from-parent-entities model, or <code>null</code> for the default in any context.
+     */
+    @Nullable ConfigInheritance getParentInheritance();
+
+    /**
      * @return The inheritance model, or <code>null</code> for the default in any context.
+     * @deprecated since 0.10.0; use {@link #getParentInheritance()}.
      */
+    @Deprecated
     @Nullable ConfigInheritance getInheritance();
 
     /**


[7/9] brooklyn-server git commit: Location inheritance: test removing values with nulls

Posted by sv...@apache.org.
Location inheritance: test removing values with nulls

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/7a7f7592
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/7a7f7592
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/7a7f7592

Branch: refs/heads/master
Commit: 7a7f75925fbca513816bc77685ad82b6915b9322
Parents: e4e6004
Author: Aled Sage <al...@gmail.com>
Authored: Thu Jun 2 12:24:03 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:09 2016 +0100

----------------------------------------------------------------------
 .../ConfigLocationInheritanceYamlTest.java      | 24 ++++++++++++++++++++
 1 file changed, 24 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7a7f7592/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 a23b727..0a40275 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
@@ -40,6 +40,7 @@ import org.apache.brooklyn.location.jclouds.JcloudsLocationConfig;
 import org.apache.brooklyn.location.jclouds.JcloudsLocationResolver;
 import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry;
 import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.SingleNodeCreator;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
 import org.jclouds.compute.ComputeService;
@@ -230,6 +231,29 @@ public class ConfigLocationInheritanceYamlTest extends AbstractYamlTest {
     }
     
     @Test(groups="Live")
+    public void testRemoveLocationPropertiesByOverridingWithBlank() throws Exception {
+        String yaml = Joiner.on("\n").join(
+                "location: jclouds-config-test-with-conf",
+                "services:",
+                "- type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess",
+                "  brooklyn.config:",
+                "    provisioning.properties:",
+                "      minRam: ",
+                "      minCores: 2",
+                "      templateOptions:",
+                "        networks: ",
+                "        subnetId: mysubnet");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+
+        assertMachineConfig(
+                Machines.findUniqueMachineLocation(entity.getLocations()).get(),
+                MutableMap.of(JcloudsLocationConfig.MIN_RAM, null, JcloudsLocationConfig.MIN_CORES, 2),
+                MutableMap.of("networks", null, "subnetId", "mysubnet"));
+    }
+    
+    @Test(groups="Live")
     public void testMergeTemplateOptionsIsShallow() throws Exception {
         addCatalogItems(
                 "brooklyn.catalog:",


[5/9] brooklyn-server git commit: JcloudsLocation.templateOptions values merged

Posted by sv...@apache.org.
JcloudsLocation.templateOptions values merged

If passed templateOptions via an entity\u2019s provisioning.properties and
also have templateOptions in the location config, then merge them.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/03c98328
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/03c98328
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/03c98328

Branch: refs/heads/master
Commit: 03c9832891fb478b095a6575400662da0d20db71
Parents: e6be5b1
Author: Aled Sage <al...@gmail.com>
Authored: Wed Jun 1 17:37:48 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:09 2016 +0100

----------------------------------------------------------------------
 .../camp/brooklyn/AbstractYamlTest.java         |  11 +-
 .../ConfigLocationInheritanceYamlTest.java      | 312 +++++++++++++++++++
 .../location/jclouds/JcloudsLocation.java       |  22 ++
 3 files changed, 344 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/03c98328/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
index 39d6c50..a03df0d 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
@@ -37,6 +37,7 @@ import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
 import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests.Builder;
 import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.ResourceUtils;
@@ -81,7 +82,11 @@ public abstract class AbstractYamlTest {
     }
 
     protected LocalManagementContext newTestManagementContext() {
-        return LocalManagementContextForTests.builder(true).disableOsgi(disableOsgi()).build();
+        Builder builder = LocalManagementContextForTests.builder(true).disableOsgi(disableOsgi());
+        if (useDefaultProperties()) {
+            builder.useDefaultProperties();
+        }
+        return builder.build();
     }
 
     /** Override to enable OSGi in the management context for all tests in the class. */
@@ -89,6 +94,10 @@ public abstract class AbstractYamlTest {
         return true;
     }
     
+    protected boolean useDefaultProperties() {
+        return false;
+    }
+    
     @AfterMethod(alwaysRun = true)
     public void tearDown() throws Exception {
         if (brooklynMgmt != null) Entities.destroyAll(brooklynMgmt);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/03c98328/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
new file mode 100644
index 0000000..6db3b64
--- /dev/null
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java
@@ -0,0 +1,312 @@
+/*
+ * 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 java.io.StringReader;
+import java.net.URI;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.location.Location;
+import org.apache.brooklyn.api.location.LocationRegistry;
+import org.apache.brooklyn.api.location.LocationResolver;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.location.BasicLocationRegistry;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.location.jclouds.ComputeServiceRegistry;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsLocationConfig;
+import org.apache.brooklyn.location.jclouds.JcloudsLocationResolver;
+import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry;
+import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.SingleNodeCreator;
+import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Image;
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.domain.NodeMetadata.Status;
+import org.jclouds.compute.domain.OperatingSystem;
+import org.jclouds.compute.domain.OsFamily;
+import org.jclouds.compute.domain.Processor;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.domain.Volume;
+import org.jclouds.compute.domain.internal.HardwareImpl;
+import org.jclouds.compute.domain.internal.NodeMetadataImpl;
+import org.jclouds.domain.LocationScope;
+import org.jclouds.domain.LoginCredentials;
+import org.jclouds.domain.internal.LocationImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+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.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+/**
+ * Requires credentials for jclouds:aws-ec2 in the default brooklyn.properties.
+ */
+public class ConfigLocationInheritanceYamlTest extends AbstractYamlTest {
+    
+    @SuppressWarnings("unused")
+    private static final Logger LOG = LoggerFactory.getLogger(ConfigLocationInheritanceYamlTest.class);
+
+    private static final String CLOUD_PROVIDER = "aws-ec2";
+    private static final String CLOUD_REGION = "us-east-1";
+    private static final String CLOUD_IMAGE_ID = "us-east-1/ami-a96b01c0";
+    
+    private LocationImpl locImpl = new LocationImpl(
+            LocationScope.REGION, 
+            "myLocId", 
+            "myLocDescription", 
+            null, 
+            ImmutableList.<String>of(), // iso3166Codes 
+            ImmutableMap.<String,Object>of()); // metadata
+
+    private NodeMetadata node = new NodeMetadataImpl(
+            CLOUD_PROVIDER, 
+            "myname", 
+            "123", // ids in SoftLayer are numeric
+            locImpl,
+            URI.create("http://myuri.com"), 
+            ImmutableMap.<String, String>of(), // userMetadata 
+            ImmutableSet.<String>of(), // tags
+            "mygroup",
+            new HardwareImpl(
+                    "myHardwareProviderId", 
+                    "myHardwareName", 
+                    "myHardwareId", 
+                    locImpl, 
+                    URI.create("http://myuri.com"), 
+                    ImmutableMap.<String, String>of(), // userMetadata 
+                    ImmutableSet.<String>of(), // tags
+                    ImmutableList.<Processor>of(), 
+                    1024, 
+                    ImmutableList.<Volume>of(), 
+                    Predicates.<Image>alwaysTrue(), // supportsImage, 
+                    (String)null, // hypervisor
+                    false),
+            CLOUD_IMAGE_ID,
+            new OperatingSystem(
+                    OsFamily.CENTOS, 
+                    "myOsName", 
+                    "myOsVersion", 
+                    "myOsArch", 
+                    "myDescription", 
+                    true), // is64Bit
+            Status.RUNNING,
+            "myBackendStatus",
+            22, // login-port
+            ImmutableList.of("1.2.3.4"), // publicAddresses, 
+            ImmutableList.of("10.2.3.4"), // privateAddresses, 
+            LoginCredentials.builder().identity("myidentity").password("mypassword").build(), 
+            "myHostname");
+
+    private SingleNodeCreator nodeCreator;
+
+    @Override
+    protected boolean useDefaultProperties() {
+        return true;
+    }
+
+    public static class RecordingJcloudsLocation extends JcloudsLocation {
+        public final List<ConfigBag> templateConfigs = Lists.newCopyOnWriteArrayList();
+        
+        public Template buildTemplate(ComputeService computeService, ConfigBag config) {
+            templateConfigs.add(config);
+            return super.buildTemplate(computeService, config);
+        }
+    }
+
+    @BeforeMethod(alwaysRun = true)
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        
+        nodeCreator = new SingleNodeCreator(node);
+        final ComputeServiceRegistry computeServiceRegistry = new StubbedComputeServiceRegistry(nodeCreator);
+
+        LocationResolver resolver = new JcloudsLocationResolver() {
+            @Override
+            public String getPrefix() {
+                return "jclouds-config-test";
+            }
+
+            protected Class<? extends JcloudsLocation> getLocationClass() {
+                return RecordingJcloudsLocation.class;
+            }
+
+            @Override
+            public LocationSpec<?> newLocationSpecFromString(String spec, Map<?,?> locationFlags, LocationRegistry registry) {
+                LocationSpec<? extends Location> orig = super.newLocationSpecFromString(spec, locationFlags, registry);
+                return LocationSpec.create(orig)
+                        .configure(JcloudsLocationConfig.IMAGE_ID, CLOUD_IMAGE_ID)
+                        .configure(JcloudsLocationConfig.COMPUTE_SERVICE_REGISTRY, computeServiceRegistry)
+                        .configure(JcloudsLocationConfig.USE_JCLOUDS_SSH_INIT, false)
+                        .configure(JcloudsLocationConfig.WAIT_FOR_SSHABLE, "false")
+                        .configure(JcloudsLocationConfig.LOOKUP_AWS_HOSTNAME, false)
+                        .configure("sshToolClass", RecordingSshTool.class.getName());
+            }
+        };
+
+        ((BasicLocationRegistry)mgmt().getLocationRegistry()).registerResolver(resolver);
+
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  id: jclouds-config-test-with-conf",
+                "  name: stubbed-jclouds-gce",
+                "  itemType: location",
+                "  item:",
+                "    type: jclouds-config-test:"+CLOUD_PROVIDER+":"+CLOUD_REGION,
+                "    brooklyn.config:",
+                "      minRam: 1234",
+                "      templateOptions:",
+                "        networks:",
+                "        - mynetwork");
+    }
+    
+    @Test(groups="Live")
+    public void testUsesLocationProperties() throws Exception {
+        String yaml = Joiner.on("\n").join(
+                "location: jclouds-config-test-with-conf",
+                "services:",
+                "- type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+        
+        assertMachineConfig(
+                Machines.findUniqueMachineLocation(entity.getLocations()).get(),
+                ImmutableMap.of(JcloudsLocationConfig.MIN_RAM, 1234),
+                ImmutableMap.of("networks", ImmutableList.of("mynetwork")));
+    }
+    
+    @Test(groups="Live")
+    public void testMergesLocationProperties() throws Exception {
+        String yaml = Joiner.on("\n").join(
+                "location: jclouds-config-test-with-conf",
+                "services:",
+                "- type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess",
+                "  brooklyn.config:",
+                "    provisioning.properties:",
+                "      minCores: 2",
+                "      templateOptions:",
+                "        subnetId: mysubnet");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+
+        assertMachineConfig(
+                Machines.findUniqueMachineLocation(entity.getLocations()).get(),
+                ImmutableMap.of(JcloudsLocationConfig.MIN_RAM, 1234, JcloudsLocationConfig.MIN_CORES, 2),
+                ImmutableMap.of("networks", ImmutableList.of("mynetwork"), "subnetId", "mysubnet"));
+    }
+    
+    @Test(groups="Live")
+    public void testMergesCatalogEntityLocationProperties() throws Exception {
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  id: EmptySoftwareProcess-with-conf",
+                "  itemType: entity",
+                "  item:",
+                "    type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess",
+                "    brooklyn.config:",
+                "      provisioning.properties:",
+                "        minDisk: 10g",
+                "        templateOptions:",
+                "          placementGroup: myPlacementGroup");
+
+        String yaml = Joiner.on("\n").join(
+                "location: jclouds-config-test-with-conf",
+                "services:",
+                "- type: EmptySoftwareProcess-with-conf",
+                "  brooklyn.config:",
+                "    provisioning.properties:",
+                "      minCores: 2",
+                "      templateOptions:",
+                "        subnetId: mysubnet");
+
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+
+        assertMachineConfig(
+                Machines.findUniqueMachineLocation(entity.getLocations()).get(),
+                ImmutableMap.of(JcloudsLocationConfig.MIN_RAM, 1234, JcloudsLocationConfig.MIN_CORES, 2, JcloudsLocationConfig.MIN_DISK, "10g"),
+                ImmutableMap.of("networks", ImmutableList.of("mynetwork"), "subnetId", "mysubnet", "placementGroup", "myPlacementGroup"));
+    }
+
+    // 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)
+    public void testMergesCatalogLocationProperties() throws Exception {
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  id: extending-jclouds-config-test-with-conf",
+                "  name: jclouds-config-test-with-conf",
+                "  itemType: location",
+                "  item:",
+                "    type: jclouds-config-test:"+CLOUD_PROVIDER+":"+CLOUD_REGION,
+                "    brooklyn.config:",
+                "      minCores: 2",
+                "      templateOptions:",
+                "        subnetId: mysubnet");
+
+        String yaml = Joiner.on("\n").join(
+                "location: extending-jclouds-config-test-with-conf",
+                "services:",
+                "- type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess");
+
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+
+        assertMachineConfig(
+                Machines.findUniqueMachineLocation(entity.getLocations()).get(),
+                ImmutableMap.of(JcloudsLocationConfig.MIN_RAM, 1234, JcloudsLocationConfig.MIN_CORES, 2),
+                ImmutableMap.of("networks", ImmutableList.of("mynetwork"), "subnetId", "mysubnet"));
+    }
+
+    protected void assertMachineConfig(MachineLocation machine, Map<? extends ConfigKey<?>, ?> expectedTopLevel, Map<String, ?> expectedTemplateOptions) {
+        RecordingJcloudsLocation jcloudsLocation = (RecordingJcloudsLocation) machine.getParent();
+        ConfigBag conf = jcloudsLocation.templateConfigs.get(jcloudsLocation.templateConfigs.size()-1);
+        
+        Map<ConfigKey<?>, Object> subConf = Maps.newLinkedHashMap();
+        for (Map.Entry<? extends ConfigKey<?>, ?> entry : expectedTopLevel.entrySet()) {
+            subConf.put(entry.getKey(), conf.get(entry.getKey()));
+        }
+        
+        assertEquals(subConf, expectedTopLevel, "actual="+subConf);
+        
+        Map<String, Object> actualTemplateOptions = conf.get(JcloudsLocationConfig.TEMPLATE_OPTIONS);
+        for (Map.Entry<String, ?> entry : expectedTemplateOptions.entrySet()) {
+            assertEquals(actualTemplateOptions.get(entry.getKey()), entry.getValue(), "templateOptions="+actualTemplateOptions);
+        }
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/03c98328/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
index fb68819..a019ced 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
@@ -84,6 +84,7 @@ import org.apache.brooklyn.location.jclouds.templates.PortableTemplateBuilder;
 import org.apache.brooklyn.location.jclouds.zone.AwsAvailabilityZoneExtension;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.collections.CollectionMerger;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -608,6 +609,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         ConfigBag setupRaw = ConfigBag.newInstanceExtending(config().getBag(), flags);
         ConfigBag setup = ResolvingConfigBag.newInstanceExtending(getManagementContext(), setupRaw);
 
+        Map<String, Object> flagTemplateOptions = ConfigBag.newInstance(flags).get(TEMPLATE_OPTIONS);
+        Map<String, Object> baseTemplateOptions = config().get(TEMPLATE_OPTIONS);
+        Map<String, Object> templateOptions = (Map<String, Object>) deepMerge(Maybe.fromNullable(flagTemplateOptions), Maybe.fromNullable(baseTemplateOptions), TEMPLATE_OPTIONS).orNull();
+        setup.put(TEMPLATE_OPTIONS, templateOptions);
+        
         Integer attempts = setup.get(MACHINE_CREATE_ATTEMPTS);
         List<Exception> exceptions = Lists.newArrayList();
         if (attempts == null || attempts < 1) attempts = 1;
@@ -3322,4 +3328,20 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         return new JcloudsBlobStoreBasedObjectStore(this, container);
     }
 
+    // TODO Duplicate of EntityConfigMap.deepMerge
+    private <T> Maybe<?> deepMerge(Maybe<? extends T> val1, Maybe<? extends T> val2, ConfigKey<?> keyForLogging) {
+        if (val2.isAbsent() || val2.isNull()) {
+            return val1;
+        } else if (val1.isAbsent()) {
+            return val2;
+        } else if (val1.isNull()) {
+            return val1; // an explicit null means an override; don't merge
+        } else if (val1.get() instanceof Map && val2.get() instanceof Map) {
+            return Maybe.of(CollectionMerger.builder().build().merge((Map<?,?>)val1.get(), (Map<?,?>)val2.get()));
+        } else {
+            // cannot merge; just return val1
+            LOG.debug("Cannot merge values for "+keyForLogging.getName()+", because values are not maps: "+val1.get().getClass()+", and "+val2.get().getClass());
+            return val1;
+        }
+    }
 }


[8/9] brooklyn-server git commit: Rename “merge” to “deep_merge”, and PR 173 comments

Posted by sv...@apache.org.
Rename \u201cmerge\u201d to \u201cdeep_merge\u201d, 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 <al...@gmail.com>
Authored: Mon Jun 6 15:09:27 2016 +0100
Committer: Aled Sage <al...@gmail.com>
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;
         }
     }
 }


[4/9] brooklyn-server git commit: Implement ConfigKey.typeInheritance

Posted by sv...@apache.org.
Implement ConfigKey.typeInheritance


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/6a1ac3f2
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/6a1ac3f2
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/6a1ac3f2

Branch: refs/heads/master
Commit: 6a1ac3f2783d59d9cc9ff15f3e35614de864ed2d
Parents: 57beafa
Author: Aled Sage <al...@gmail.com>
Authored: Fri May 27 09:44:09 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:09 2016 +0100

----------------------------------------------------------------------
 .../internal/AbstractBrooklynObjectSpec.java    |   5 +
 .../BrooklynComponentTemplateResolver.java      | 105 ++++++++++++++--
 .../brooklyn/ConfigInheritanceYamlTest.java     | 124 ++++++++++++++++++-
 .../brooklyn/util/core/flags/FlagUtils.java     |  31 ++++-
 .../util/core/internal/FlagUtilsTest.java       |  96 +++++++++++++-
 5 files changed, 343 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6a1ac3f2/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
index ef99a27..a022883 100644
--- a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
+++ b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
@@ -303,6 +303,11 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly
         return self();
     }
 
+    public <V> SpecT removeFlag(String key) {
+        flags.remove( checkNotNull(key, "key") );
+        return self();
+    }
+
     /** Clears the config map, removing any config previously set. */
     public void clearConfig() {
         config.clear();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6a1ac3f2/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 e2eb9b6..ad5ca74 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
@@ -23,7 +23,6 @@ import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -31,10 +30,11 @@ import javax.annotation.Nullable;
 
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;
-import org.apache.brooklyn.api.location.LocationSpec;
 import org.apache.brooklyn.api.framework.FrameworkLookup;
+import org.apache.brooklyn.api.location.LocationSpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
+import org.apache.brooklyn.api.objs.SpecParameter;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.camp.brooklyn.BrooklynCampConstants;
 import org.apache.brooklyn.camp.brooklyn.BrooklynCampReservedKeys;
@@ -45,6 +45,8 @@ import org.apache.brooklyn.camp.spi.AbstractResource;
 import org.apache.brooklyn.camp.spi.ApplicationComponentTemplate;
 import org.apache.brooklyn.camp.spi.AssemblyTemplate;
 import org.apache.brooklyn.camp.spi.PlatformComponentTemplate;
+import org.apache.brooklyn.config.ConfigInheritance;
+import org.apache.brooklyn.config.ConfigInheritance.InheritanceMode;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
 import org.apache.brooklyn.core.config.ConfigKeys;
@@ -54,7 +56,9 @@ import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
 import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
 import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
 import org.apache.brooklyn.core.resolve.entity.EntitySpecResolver;
+import org.apache.brooklyn.util.collections.CollectionMerger;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.flags.FlagUtils;
@@ -257,14 +261,18 @@ public class BrooklynComponentTemplateResolver {
         // attrs will contain only brooklyn.xxx properties when coming from BrooklynEntityMatcher.
         // Any top-level flags will go into "brooklyn.flags". When resolving a spec from $brooklyn:entitySpec
         // top level flags remain in place. Have to support both cases.
-
+        //
+        // For config values inherited from the super-type (be that the Java type or another catalog item
+        // being extended), we lookup the config key to find out if the values should be merged, overridden or 
+        // cleared.
+        
         ConfigBag bag = ConfigBag.newInstance((Map<Object, Object>) attrs.getStringKey(BrooklynCampReservedKeys.BROOKLYN_CONFIG));
         ConfigBag bagFlags = ConfigBag.newInstanceCopying(attrs);
         if (attrs.containsKey(BrooklynCampReservedKeys.BROOKLYN_FLAGS)) {
             bagFlags.putAll((Map<String, Object>) attrs.getStringKey(BrooklynCampReservedKeys.BROOKLYN_FLAGS));
         }
 
-        Collection<FlagConfigKeyAndValueRecord> topLevelApparentConfig = findAllFlagsAndConfigKeys(spec, bagFlags);
+        Collection<FlagConfigKeyAndValueRecord> topLevelApparentConfig = findAllFlagsAndConfigKeyValues(spec, bagFlags);
         for (FlagConfigKeyAndValueRecord r: topLevelApparentConfig) {
             if (r.getConfigKeyMaybeValue().isPresent())
                 bag.putIfAbsent((ConfigKey)r.getConfigKey(), r.getConfigKeyMaybeValue().get());
@@ -272,22 +280,44 @@ public class BrooklynComponentTemplateResolver {
                 bag.putAsStringKeyIfAbsent(r.getFlagName(), r.getFlagMaybeValue().get());
         }
 
+        
         // now set configuration for all the items in the bag
-        Collection<FlagConfigKeyAndValueRecord> records = findAllFlagsAndConfigKeys(spec, bag);
+        Map<String, ConfigKey<?>> entityConfigKeys = findAllConfigKeys(spec);
+        
+        Collection<FlagConfigKeyAndValueRecord> records = findAllFlagsAndConfigKeyValues(spec, bag);
         Set<String> keyNamesUsed = new LinkedHashSet<String>();
         for (FlagConfigKeyAndValueRecord r: records) {
             if (r.getFlagMaybeValue().isPresent()) {
-                Object transformed = new SpecialFlagsTransformer(loader, encounteredRegisteredTypeIds).apply(r.getFlagMaybeValue().get());
-                spec.configure(r.getFlagName(), transformed);
-                keyNamesUsed.add(r.getFlagName());
+                String flag = r.getFlagName();
+                Object ownVal = new SpecialFlagsTransformer(loader, encounteredRegisteredTypeIds).apply(r.getFlagMaybeValue().get());
+                Maybe<?> superVal = spec.getFlags().containsKey(flag) ? Maybe.of(spec.getFlags().get(flag)) : Maybe.absent();
+                Object combinedVal = combineValues(entityConfigKeys.get(flag), spec, Maybe.of(ownVal), superVal).get();
+                spec.configure(flag, combinedVal);
+                keyNamesUsed.add(flag);
             }
             if (r.getConfigKeyMaybeValue().isPresent()) {
-                Object transformed = new SpecialFlagsTransformer(loader, encounteredRegisteredTypeIds).apply(r.getConfigKeyMaybeValue().get());
-                spec.configure((ConfigKey<Object>)r.getConfigKey(), transformed);
-                keyNamesUsed.add(r.getConfigKey().getName());
+                ConfigKey<Object> key = (ConfigKey<Object>) r.getConfigKey();
+                Object ownVal = new SpecialFlagsTransformer(loader, encounteredRegisteredTypeIds).apply(r.getConfigKeyMaybeValue().get());
+                Maybe<?> superVal = spec.getConfig().containsKey(key) ? Maybe.of(spec.getConfig().get(key)) : Maybe.absent();
+                Object combinedVal = combineValues(entityConfigKeys.get(key.getName()), spec, Maybe.of(ownVal), superVal).get();
+                spec.configure(key, combinedVal);
+                keyNamesUsed.add(key.getName());
             }
         }
 
+        // For anything that should not be inherited, clear if from the spec
+        for (Map.Entry<String, ConfigKey<?>> entry : entityConfigKeys.entrySet()) {
+            if (keyNamesUsed.contains(entry.getKey())) {
+                continue;
+            }
+            ConfigKey<?> key = entry.getValue();
+            InheritanceMode mode = getInheritanceMode(key, spec);
+            if (mode == InheritanceMode.NONE) {
+                spec.removeConfig(key);
+                spec.removeFlag(key.getName());
+            }
+        }
+        
         // set unused keys as anonymous config keys -
         // they aren't flags or known config keys, so must be passed as config keys in order for
         // EntitySpec to know what to do with them (as they are passed to the spec as flags)
@@ -301,10 +331,47 @@ public class BrooklynComponentTemplateResolver {
         }
     }
 
+    // TODO Duplicates some logic in EntityConfigMap.getConfig()
+    private Maybe<?> combineValues(ConfigKey<?> key, EntitySpec<?> spec, Maybe<?> ownVal, Maybe<?> superVal) {
+        InheritanceMode mode = getInheritanceMode(key, spec);
+        switch (mode) {
+        case IF_NO_EXPLICIT_VALUE:
+            return ownVal.isPresent() ? ownVal : superVal;
+        case MERGE:
+            return deepMerge(ownVal, superVal, key);
+        case NONE:
+            return ownVal;
+        default:
+            throw new IllegalStateException("Unsupported type-inheritance mode for "+key.getName()+": "+mode);
+        }
+    }
+
+    private InheritanceMode getInheritanceMode(ConfigKey<?> key, EntitySpec<?> spec) {
+        ConfigInheritance inheritance = (key != null && key.getTypeInheritance() != null) ? key.getTypeInheritance() : ConfigInheritance.ALWAYS;
+        return inheritance.isInherited(key, spec, attrs);
+    }
+
+    // TODO Duplicate of EntityConfigMap.deepMerge
+    private <T> Maybe<?> deepMerge(Maybe<? extends T> val1, Maybe<? extends T> val2, ConfigKey<?> keyForLogging) {
+        if (val2.isAbsent() || val2.isNull()) {
+            return val1;
+        } else if (val1.isAbsent()) {
+            return val2;
+        } else if (val1.isNull()) {
+            return val1; // an explicit null means an override; don't merge
+        } else if (val1.get() instanceof Map && val2.get() instanceof Map) {
+            return Maybe.of(CollectionMerger.builder().build().merge((Map<?,?>)val1.get(), (Map<?,?>)val2.get()));
+        } else {
+            // cannot merge; just return val1
+            log.debug("Cannot merge values for "+keyForLogging.getName()+", because values are not maps: "+val1.get().getClass()+", and "+val2.get().getClass());
+            return val1;
+        }
+    }
+
     /**
      * Searches for config keys in the type, additional interfaces and the implementation (if specified)
      */
-    private Collection<FlagConfigKeyAndValueRecord> findAllFlagsAndConfigKeys(EntitySpec<?> spec, ConfigBag bagFlags) {
+    private Collection<FlagConfigKeyAndValueRecord> findAllFlagsAndConfigKeyValues(EntitySpec<?> spec, ConfigBag bagFlags) {
         Set<FlagConfigKeyAndValueRecord> allKeys = MutableSet.of();
         allKeys.addAll(FlagUtils.findAllFlagsAndConfigKeys(null, spec.getType(), bagFlags));
         if (spec.getImplementation() != null) {
@@ -317,6 +384,20 @@ public class BrooklynComponentTemplateResolver {
         return allKeys;
     }
 
+    private Map<String, ConfigKey<?>> findAllConfigKeys(EntitySpec<?> spec) {
+        Set<Class<?>> types = MutableSet.<Class<?>>builder()
+                .add(spec.getType())
+                .add(spec.getImplementation())
+                .addAll(spec.getAdditionalInterfaces())
+                .remove(null)
+                .build();
+        MutableMap<String, ConfigKey<?>> result = MutableMap.copyOf(FlagUtils.findAllConfigKeys(null, types));
+        for (SpecParameter<?> param : spec.getParameters()) {
+            result.put(param.getConfigKey().getName(), param.getConfigKey());
+        }
+        return result;
+    }
+
     private static class SpecialFlagsTransformer implements Function<Object, Object> {
         protected final ManagementContext mgmt;
         /* TODO find a way to make do without loader here?

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6a1ac3f2/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 1d37589..9f52ded 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
@@ -30,7 +30,6 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
 import org.apache.brooklyn.api.entity.Entity;
-import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.MapConfigKey;
 import org.apache.brooklyn.core.entity.EntityAsserts;
 import org.apache.brooklyn.core.sensor.Sensors;
@@ -99,6 +98,26 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
         
         addCatalogItems(
                 "brooklyn.catalog:",
+                "  id: EmptySoftwareProcess-with-env",
+                "  itemType: entity",
+                "  item:",
+                "    type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess",
+                "    brooklyn.config:",
+                "      env:",
+                "        ENV1: myEnv1");
+        
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  id: EmptySoftwareProcess-with-shell.env",
+                "  itemType: entity",
+                "  item:",
+                "    type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess",
+                "    brooklyn.config:",
+                "      shell.env:",
+                "        ENV1: myEnv1");
+
+        addCatalogItems(
+                "brooklyn.catalog:",
                 "  id: localhost-stub",
                 "  name: Localhost (stubbed-SSH)",
                 "  itemType: location",
@@ -219,7 +238,7 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
      * attributeWhenReady apply to?
      */
     @Test(groups={"Broken", "WIP"}, enabled=false)
-    public void testInheritsParentConfigTask() throws Exception {
+    public void testInheritsParentConfigTaskWithSelfScope() throws Exception {
         String yaml = Joiner.on("\n").join(
                 "services:",
                 "- type: org.apache.brooklyn.entity.stock.BasicApplication",
@@ -241,6 +260,33 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
             public Object call() {
                 return app.sensors().set(Sensors.newStringSensor("myOtherSensor"), "myObject");
             }});
+        assertEquals(app.config().get(TestEntity.CONF_OBJECT), "myObject");
+    }
+    
+    @Test
+    public void testInheritsParentConfigTask() throws Exception {
+        String yaml = Joiner.on("\n").join(
+                "services:",
+                "- type: org.apache.brooklyn.entity.stock.BasicApplication",
+                "  id: app",
+                "  brooklyn.config:",
+                "    test.confName: $brooklyn:config(\"myOtherConf\")",
+                "    test.confObject: $brooklyn:component(\"app\").attributeWhenReady(\"myOtherSensor\")",
+                "    myOtherConf: myOther",
+                "  brooklyn.children:",
+                "  - type: org.apache.brooklyn.core.test.entity.TestEntity");
+
+        final Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren());
+     
+        // Task that resolves quickly
+        assertEquals(entity.config().get(TestEntity.CONF_NAME), "myOther");
+        
+        // Task that resolves slowly
+        executor.submit(new Callable<Object>() {
+            public Object call() {
+                return app.sensors().set(Sensors.newStringSensor("myOtherSensor"), "myObject");
+            }});
         assertEquals(entity.config().get(TestEntity.CONF_OBJECT), "myObject");
     }
     
@@ -536,6 +582,80 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest {
         }
     }
     
+    @Test
+    public void testExtendsSuperTypeConfigSimple() throws Exception {
+        ImmutableMap<String, Object> expectedEnv = ImmutableMap.<String, Object>of("ENV1", "myEnv1", "ENV2", "myEnv2");
+
+        // super-type has shell.env; sub-type shell.env
+        String yaml = Joiner.on("\n").join(
+                "location: localhost-stub",
+                "services:",
+                "- type: EmptySoftwareProcess-with-shell.env",
+                "  brooklyn.config:",
+                "    shell.env:",
+                "      ENV2: myEnv2");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+        EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, expectedEnv);
+    }
+
+    @Test
+    public void testExtendsSuperTypeConfigMixingLongOverridingShortNames() throws Exception {
+        ImmutableMap<String, Object> expectedEnv = ImmutableMap.<String, Object>of("ENV1", "myEnv1", "ENV2", "myEnv2");
+
+        // super-type has env; sub-type shell.env
+        String yaml = Joiner.on("\n").join(
+                "location: localhost-stub",
+                "services:",
+                "- type: EmptySoftwareProcess-with-env",
+                "  brooklyn.config:",
+                "    shell.env:",
+                "      ENV2: myEnv2");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+        EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, expectedEnv);
+    }
+        
+    @Test
+    public void testExtendsSuperTypeConfigMixingShortOverridingLongName() throws Exception {
+        ImmutableMap<String, Object> expectedEnv = ImmutableMap.<String, Object>of("ENV1", "myEnv1", "ENV2", "myEnv2");
+
+        // super-type has shell.env; sub-type env
+        String yaml = Joiner.on("\n").join(
+                "location: localhost-stub",
+                "services:",
+                "- type: EmptySoftwareProcess-with-shell.env",
+                "  brooklyn.config:",
+                "    env:",
+                "      ENV2: myEnv2");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+        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
+    // config key. Maybe FlagUtils could respect SetFromFlags when returning Map<String,ConfigKey>?
+    @Test(groups="WIP")
+    public void testExtendsSuperTypeConfigMixingShortOverridingShortName() throws Exception {
+        ImmutableMap<String, Object> expectedEnv = ImmutableMap.<String, Object>of("ENV1", "myEnv1", "ENV2", "myEnv2");
+
+        // super-type has env; sub-type env
+        String yaml = Joiner.on("\n").join(
+                "location: localhost-stub",
+                "services:",
+                "- type: EmptySoftwareProcess-with-env",
+                "  brooklyn.config:",
+                "    env:",
+                "      ENV2: myEnv2");
+
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+        EntityAsserts.assertConfigEquals(entity, EmptySoftwareProcess.SHELL_ENVIRONMENT, expectedEnv);
+    }
+    
     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/6a1ac3f2/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 3bc22e0..26f3e9d 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
@@ -218,6 +218,30 @@ public class FlagUtils {
         }
     }
     
+    /**
+     * Gets all the config keys for the given types (for duplicates, the first class in the list of types takes precedence).
+     */
+    public static Map<String, ConfigKey<?>> findAllConfigKeys(Object optionalInstance, Iterable<? extends Class<?>> types) {
+        Map<String, ConfigKey<?>> result = Maps.newLinkedHashMap();
+        for (Class<?> type : types) {
+            List<ConfigKey<?>> keys = FlagUtils.findAllConfigKeys(optionalInstance, type);
+            for (ConfigKey<?> key : keys) {
+                if (!result.containsKey(key.getName())) result.put(key.getName(), key);
+            }
+        }
+        return result;
+    }
+
+    /** gets all the config keys for the given type */
+    public static <T> List<ConfigKey<?>> findAllConfigKeys(T optionalInstance, Class<? extends T> type) {
+        List<ConfigKey<?>> output = new ArrayList<ConfigKey<?>>();
+        for (Field f: getAllFields(type)) {
+            ConfigKey<?> key = getFieldAsConfigKey(optionalInstance, f);
+            if (key != null) output.add(key);
+        }
+        return output;
+    }
+    
     /** gets all the flags/keys in the given config bag which are applicable to the given type's config keys and flags */
     public static <T> List<FlagConfigKeyAndValueRecord> findAllFlagsAndConfigKeys(T optionalInstance, Class<? extends T> type, ConfigBag input) {
         List<FlagConfigKeyAndValueRecord> output = new ArrayList<FlagUtils.FlagConfigKeyAndValueRecord>();
@@ -250,9 +274,10 @@ public class FlagUtils {
         if (f != null) {
             SetFromFlag flag = f.getAnnotation(SetFromFlag.class);
             if (flag!=null) {
-                result.flagName = flag.value();
-                if (input.containsKey(flag.value()))
-                    result.flagValue = Maybe.of(input.getStringKey(flag.value()));
+                String flagName = elvis(flag.value(), f.getName());
+                result.flagName = flagName;
+                if (input.containsKey(flagName))
+                    result.flagValue = Maybe.of(input.getStringKey(flagName));
             }
         }
         return result;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6a1ac3f2/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java
index a75db17..905674a 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.util.core.internal;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
 import java.lang.reflect.Field;
 import java.net.InetAddress;
@@ -34,15 +35,19 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
 import org.apache.brooklyn.core.config.BasicConfigKey;
 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.flags.FlagUtils;
+import org.apache.brooklyn.util.core.flags.FlagUtils.FlagConfigKeyAndValueRecord;
 import org.apache.brooklyn.util.core.flags.SetFromFlag;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Function;
+import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -220,6 +225,91 @@ public class FlagUtilsTest {
         assertEquals(unused, ImmutableMap.of("ck2", "dont-set"));
     }
 
+    @Test
+    public void testFindAllConfigKeys() {
+        Map<String, ConfigKey<?>> keys = FlagUtils.findAllConfigKeys(null, ImmutableList.of(SubFooCK.class));
+        assertEquals(keys, ImmutableMap.of(
+                FooCK.CK1.getName(), FooCK.CK1, 
+                FooCK.CK2.getName(), FooCK.CK2, 
+                FooCK.CK3.getName(), FooCK.CK3,
+                SubFooCK.CK4.getName(), SubFooCK.CK4));
+    }
+
+    @Test
+    public void testFindAllFlagsAndConfigKeysWithEmptyBag() {
+        ConfigBag bag = ConfigBag.newInstance(ImmutableMap.of());
+        List<FlagConfigKeyAndValueRecord> vals = FlagUtils.<SubFooCK>findAllFlagsAndConfigKeys(null, SubFooCK.class, bag);
+        assertRecordsEqual(vals, ImmutableList.<ExpectedFlagConfigKeyAndValueRecord>of());
+    }
+
+    @Test
+    public void testFindAllFlagsAndConfigKeysWithConfigVals() {
+        ConfigBag bag = ConfigBag.newInstance(ImmutableMap.of(FooCK.CK1, "ck1.myval"));
+        List<FlagConfigKeyAndValueRecord> vals = FlagUtils.<SubFooCK>findAllFlagsAndConfigKeys(null, SubFooCK.class, bag);
+        assertRecordsEqual(vals, ImmutableList.of(new ExpectedFlagConfigKeyAndValueRecord(FooCK.CK1, "CK1", "ck1.myval")));
+    }
+
+    @Test
+    public void testFindAllFlagsAndConfigKeysWithFlagVals() {
+        ConfigBag bag = ConfigBag.newInstance(ImmutableMap.of("f1", 123));
+        List<FlagConfigKeyAndValueRecord> vals = FlagUtils.<SubFooCK>findAllFlagsAndConfigKeys(null, SubFooCK.class, bag);
+        assertRecordsEqual(vals, ImmutableList.of(new ExpectedFlagConfigKeyAndValueRecord("f1", 123)));
+    }
+
+    protected void assertRecordsEqual(List<FlagConfigKeyAndValueRecord> actual, List<ExpectedFlagConfigKeyAndValueRecord> expected) {
+        List<FlagConfigKeyAndValueRecord> actualCopy = MutableList.copyOf(actual);
+        for (ExpectedFlagConfigKeyAndValueRecord record : expected) {
+            boolean found = false;
+            for (FlagConfigKeyAndValueRecord contender : actualCopy) {
+                if (isRecordEqual(contender, record)) {
+                    actualCopy.remove(contender);
+                    found = true;
+                    break;
+                }
+            }
+            if (!found) {
+                fail(record+" expected, but not found");
+            }
+        }
+        if (!actualCopy.isEmpty()) {
+            fail(actualCopy+" additional records found");
+        }
+    }
+
+    protected boolean isRecordEqual(FlagConfigKeyAndValueRecord val1, ExpectedFlagConfigKeyAndValueRecord val2) {
+        return Objects.equal(val1.getConfigKey(), val2.configKey)
+                && Objects.equal(val1.getFlagName(), val2.flagName)
+                && Objects.equal(val1.getConfigKeyMaybeValue(), val2.configKeyValue)
+                && Objects.equal(val1.getFlagMaybeValue(), val2.flagValue);
+    }
+
+    // The fields of FlagConfigKeyAndValueRecord are private; hence having this class to make assertions easier
+    static class ExpectedFlagConfigKeyAndValueRecord {
+        String flagName = "";
+        ConfigKey<?> configKey = null;
+        Maybe<Object> flagValue = Maybe.absent();
+        Maybe<Object> configKeyValue = Maybe.absent();
+        
+        ExpectedFlagConfigKeyAndValueRecord(String flagName, Object val) {
+            this.flagName = flagName;
+            this.flagValue = Maybe.of(val);
+        }
+        ExpectedFlagConfigKeyAndValueRecord(ConfigKey<?> key, String flagName, Object val) {
+            this.configKey = key;
+            this.flagName = flagName;
+            this.configKeyValue = Maybe.of(val);
+        }
+        @Override
+        public String toString() {
+            return Objects.toStringHelper(this).omitNullValues()
+                    .add("flag", flagName)
+                    .add("configKey", configKey)
+                    .add("flagValue", flagValue.orNull())
+                    .add("configKeyValue", configKeyValue.orNull())
+                    .toString();
+        }
+    }
+    
     public static class Foo {
         @SetFromFlag
         int w;
@@ -251,7 +341,7 @@ public class FlagUtilsTest {
         @SetFromFlag Set<?> set;
         @SetFromFlag InetAddress inet;
     }
-    
+
     public static class FooCK implements Configurable {
         @SetFromFlag
         public static ConfigKey<String> CK1 = ConfigKeys.newStringConfigKey("ck1");
@@ -315,4 +405,8 @@ public class FlagUtilsTest {
             }
         }
     }
+    
+    public static class SubFooCK extends FooCK {
+        public static ConfigKey<String> CK4 = ConfigKeys.newStringConfigKey("ck4");
+    }
 }
\ No newline at end of file


[6/9] brooklyn-server git commit: Change templateOptions merge to be shallow

Posted by sv...@apache.org.
Change templateOptions merge to be shallow

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/e4e60046
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/e4e60046
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/e4e60046

Branch: refs/heads/master
Commit: e4e60046e433c43b119c72cd01e0402251ca7626
Parents: 03c9832
Author: Aled Sage <al...@gmail.com>
Authored: Thu Jun 2 11:41:40 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:09 2016 +0100

----------------------------------------------------------------------
 .../ConfigLocationInheritanceYamlTest.java      | 33 ++++++++++++++++++++
 .../location/jclouds/JcloudsLocation.java       |  8 ++---
 2 files changed, 37 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e4e60046/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 6db3b64..a23b727 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
@@ -230,6 +230,39 @@ public class ConfigLocationInheritanceYamlTest extends AbstractYamlTest {
     }
     
     @Test(groups="Live")
+    public void testMergeTemplateOptionsIsShallow() throws Exception {
+        addCatalogItems(
+                "brooklyn.catalog:",
+                "  id: jclouds-config-test-with-tempateOptions-mapVal",
+                "  name: stubbed-jclouds-gce",
+                "  itemType: location",
+                "  item:",
+                "    type: jclouds-config-test:"+CLOUD_PROVIDER+":"+CLOUD_REGION,
+                "    brooklyn.config:",
+                "      templateOptions:",
+                "        mymap:",
+                "          key1: val1");
+        
+        String yaml = Joiner.on("\n").join(
+                "location: jclouds-config-test-with-tempateOptions-mapVal",
+                "services:",
+                "- type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess",
+                "  brooklyn.config:",
+                "    provisioning.properties:",
+                "      templateOptions:",
+                "        mymap:",
+                "          key2: val2");
+        
+        Entity app = createStartWaitAndLogApplication(new StringReader(yaml));
+        Entity entity = Iterables.getOnlyElement(app.getChildren());
+
+        assertMachineConfig(
+                Machines.findUniqueMachineLocation(entity.getLocations()).get(),
+                ImmutableMap.<ConfigKey<?>, Object>of(),
+                ImmutableMap.of("mymap", ImmutableMap.of("key2", "val2")));
+    }
+    
+    @Test(groups="Live")
     public void testMergesCatalogEntityLocationProperties() throws Exception {
         addCatalogItems(
                 "brooklyn.catalog:",

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e4e60046/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
index a019ced..ee67e9d 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
@@ -611,7 +611,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
         Map<String, Object> flagTemplateOptions = ConfigBag.newInstance(flags).get(TEMPLATE_OPTIONS);
         Map<String, Object> baseTemplateOptions = config().get(TEMPLATE_OPTIONS);
-        Map<String, Object> templateOptions = (Map<String, Object>) deepMerge(Maybe.fromNullable(flagTemplateOptions), Maybe.fromNullable(baseTemplateOptions), TEMPLATE_OPTIONS).orNull();
+        Map<String, Object> templateOptions = (Map<String, Object>) shallowMerge(Maybe.fromNullable(flagTemplateOptions), Maybe.fromNullable(baseTemplateOptions), TEMPLATE_OPTIONS).orNull();
         setup.put(TEMPLATE_OPTIONS, templateOptions);
         
         Integer attempts = setup.get(MACHINE_CREATE_ATTEMPTS);
@@ -3328,8 +3328,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         return new JcloudsBlobStoreBasedObjectStore(this, container);
     }
 
-    // TODO Duplicate of EntityConfigMap.deepMerge
-    private <T> Maybe<?> deepMerge(Maybe<? extends T> val1, Maybe<? extends T> val2, ConfigKey<?> keyForLogging) {
+    // TODO Very similar to EntityConfigMap.deepMerge
+    private <T> Maybe<?> shallowMerge(Maybe<? extends T> val1, Maybe<? extends T> val2, ConfigKey<?> keyForLogging) {
         if (val2.isAbsent() || val2.isNull()) {
             return val1;
         } else if (val1.isAbsent()) {
@@ -3337,7 +3337,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         } else if (val1.isNull()) {
             return val1; // an explicit null means an override; don't merge
         } else if (val1.get() instanceof Map && val2.get() instanceof Map) {
-            return Maybe.of(CollectionMerger.builder().build().merge((Map<?,?>)val1.get(), (Map<?,?>)val2.get()));
+            return Maybe.of(CollectionMerger.builder().deep(false).build().merge((Map<?,?>)val1.get(), (Map<?,?>)val2.get()));
         } else {
             // cannot merge; just return val1
             LOG.debug("Cannot merge values for "+keyForLogging.getName()+", because values are not maps: "+val1.get().getClass()+", and "+val2.get().getClass());


[9/9] brooklyn-server git commit: Closes #173

Posted by sv...@apache.org.
Closes #173

BROOKLYN-286: merge config keys map values


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/2edf7168
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/2edf7168
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/2edf7168

Branch: refs/heads/master
Commit: 2edf7168d6204c4b724eea95e2f6999232303f52
Parents: 2276de0 6dad5de
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Jun 6 17:45:44 2016 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Jun 6 17:45:44 2016 +0300

----------------------------------------------------------------------
 .../internal/AbstractBrooklynObjectSpec.java    |   5 +
 .../camp/brooklyn/BrooklynCampConstants.java    |   4 +-
 .../BrooklynComponentTemplateResolver.java      | 105 +++-
 .../camp/brooklyn/AbstractYamlTest.java         |  11 +-
 .../brooklyn/ConfigInheritanceYamlTest.java     | 507 ++++++++++++++++++-
 .../ConfigLocationInheritanceYamlTest.java      | 369 ++++++++++++++
 .../camp/brooklyn/ConfigParametersYamlTest.java |   2 +-
 .../brooklyn/core/config/BasicConfigKey.java    | 169 +++++--
 .../apache/brooklyn/core/config/ConfigKeys.java |   8 +-
 .../brooklyn/core/config/MapConfigKey.java      |  77 ++-
 .../internal/AbstractStructuredConfigKey.java   |   2 +-
 .../core/entity/BrooklynConfigKeys.java         |  11 +-
 .../core/entity/internal/EntityConfigMap.java   | 129 +++--
 .../entity/lifecycle/ServiceStateLogic.java     |   4 +-
 .../core/location/AbstractLocation.java         |   4 +-
 .../brooklyn/core/objs/BasicSpecParameter.java  |  15 +-
 .../sensor/AttributeSensorAndConfigKey.java     |   2 +-
 .../sensor/PortAttributeSensorAndConfigKey.java |   2 +-
 .../brooklyn/util/core/flags/FlagUtils.java     |  31 +-
 .../brooklyn/core/config/ConfigKeysTest.java    |   4 +-
 .../entity/ConfigEntityInheritanceTest.java     |   4 +-
 .../util/core/internal/FlagUtilsTest.java       |  96 +++-
 .../location/jclouds/JcloudsLocation.java       |  31 +-
 .../location/jclouds/JcloudsLocationConfig.java |   6 +
 .../entity/software/base/SoftwareProcess.java   |  54 +-
 .../brooklyn/config/ConfigInheritance.java      |  43 +-
 .../org/apache/brooklyn/config/ConfigKey.java   |  12 +
 .../util/collections/CollectionMerger.java      | 236 +++++++++
 .../util/collections/CollectionMergerTest.java  | 409 +++++++++++++++
 29 files changed, 2186 insertions(+), 166 deletions(-)
----------------------------------------------------------------------



[2/9] brooklyn-server git commit: Adds CollectionMerger, for merging maps

Posted by sv...@apache.org.
Adds CollectionMerger, for merging maps


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/97807f24
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/97807f24
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/97807f24

Branch: refs/heads/master
Commit: 97807f24efa49dceeaebfd71fdeb7766e2879ecd
Parents: 9f7a778
Author: Aled Sage <al...@gmail.com>
Authored: Fri May 27 09:40:03 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:08 2016 +0100

----------------------------------------------------------------------
 .../util/collections/CollectionMerger.java      | 236 +++++++++++
 .../util/collections/CollectionMergerTest.java  | 409 +++++++++++++++++++
 2 files changed, 645 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97807f24/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionMerger.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionMerger.java b/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionMerger.java
new file mode 100644
index 0000000..c9baac0
--- /dev/null
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionMerger.java
@@ -0,0 +1,236 @@
+/*
+ * 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 static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.brooklyn.util.guava.Maybe;
+
+import com.google.common.annotations.Beta;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
+
+@Beta
+public class CollectionMerger {
+    
+    public static class Builder {
+        protected int depth = Integer.MAX_VALUE;
+        protected boolean mergeNestedMaps = true;
+        protected boolean mergeNestedLists = false;
+        
+        public Builder deep(boolean val) {
+            return depth(val ? Integer.MAX_VALUE : 1);
+        }
+        /**
+         * Depth 1 means a shallow copy - i.e. only looking one layer down (e.g. at the values within the top-level map).
+         * Depth 2 would mean going one-deep into the values inside the top-level map/list/set.
+         * 
+         * By default, depth only applies to nested maps. One needs to set {@link #mergeNestedLists(boolean)} for 
+         * it to do this to nested iterables.
+         */
+        public Builder depth(int val) {
+            checkArgument(val > 0, "val %s must be positive", val);
+            this.depth = val;
+            return this;
+        }
+        public Builder mergeNestedMaps(boolean val) {
+            this.mergeNestedMaps = val;
+            return this;
+        }
+        public Builder mergeNestedLists(boolean val) {
+            this.mergeNestedLists = val;
+            return this;
+        }
+        public CollectionMerger build() {
+            return new CollectionMerger(this);
+        }
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    protected final int depth;
+    protected final boolean mergeNestedMaps;
+    protected final boolean mergeNestedLists;
+
+    protected CollectionMerger(Builder builder) {
+        this.depth = builder.depth;
+        this.mergeNestedMaps = builder.mergeNestedMaps;
+        this.mergeNestedLists = builder.mergeNestedLists;
+    }
+    
+    public Map<?, ?> merge(Map<?, ?> map1, Map<?, ?> map2) {
+        checkNotNull(map1, "map1");
+        checkNotNull(map2, "map2");
+        return (Map<?,?>) mergeImpl(Maybe.of(map1), Maybe.of(map2), depth, new Visited());
+    }
+    
+    protected Object mergeImpl(Maybe<?> val1, Maybe<?> val2, int depthRemaining, Visited visited) {
+        if (visited.isVisited(val1.orNull())) {
+            throw new IllegalStateException("Recursive self-reference, "+val1.get().getClass()+": "+val1.get());
+        }
+        if (visited.isVisited(val2.orNull())) {
+            throw new IllegalStateException("Recursive self-reference, "+val2.get().getClass()+": "+val2.get());
+        }
+        visited.recordVisit(val1.orNull());
+        visited.recordVisit(val2.orNull());
+        
+        if (depthRemaining < 0) {
+            throw new IllegalStateException("Invalid depth "+depthRemaining);
+        }
+        if (val2.isAbsent() || val2.isNull()) {
+            return (val1.isPresent() ? val1.get() : null);
+        }
+        if (val1.isAbsent()) {
+            return (val2.isPresent() ? val2.get() : null);
+        }
+        if (val1.isNull()) {
+            // An explicit null value is treated as a marker to mean "do-not-merge"
+            return val1.get();
+        }
+        
+        if (val1.get() instanceof Map) {
+            Map<?,?> map1 = (Map<?, ?>) val1.get();
+            if (val2.get() instanceof Map) {
+                return mergeMapsImpl(map1, (Map<?, ?>) val2.get(), depthRemaining, visited);
+            } else {
+                // incompatible types; not merging
+                return val1.get();
+            }
+        }
+        if (val1.get() instanceof Iterable) {
+            if (!mergeNestedLists) {
+                return val1.get();
+            }
+            Iterable<?> iter1 = (Iterable<?>) val1.get();
+            if (val2.get() instanceof Iterable) {
+                return mergeIterablesImpl(iter1, (Iterable<?>) val2.get(), depthRemaining, visited);
+            } else {
+                // incompatible types; not merging
+                return val1.get();
+            }
+        }
+        return val1.get();
+    }
+
+    private Map<?, ?> mergeMapsImpl(Map<?, ?> val1, Map<?, ?> val2, int depthRemaining, Visited visited) {
+        if (depthRemaining < 1) {
+            return val1;
+        }
+        MutableMap<Object, Object> result = MutableMap.of();
+        for (Object key : Sets.union(val1.keySet(), val2.keySet())) {
+            Maybe<?> sub1 = val1.containsKey(key) ? Maybe.of(val1.get(key)) : Maybe.absent();
+            Maybe<?> sub2 = val2.containsKey(key) ? Maybe.of(val2.get(key)) : Maybe.absent();
+            result.put(key, mergeImpl(sub1, sub2, depthRemaining-1, visited));
+        }
+        return result;
+    }
+    
+    private Iterable<?> mergeIterablesImpl(Iterable<?> val1, Iterable<?> val2, int depthRemaining, Visited visited) {
+        if (depthRemaining < 1) {
+            return val1;
+        }
+        if (val1 instanceof Set) {
+            return mergeSetsImpl((Set<?>)val1, MutableSet.copyOf(val2), depthRemaining, visited);
+        } else {
+            return mergeListsImpl(MutableList.copyOf(val1), val2, depthRemaining, visited);
+        }
+    }
+
+    private Set<?> mergeSetsImpl(Set<?> val1, Set<?> val2, int depthRemaining, Visited visited) {
+        return MutableSet.builder()
+                .addAll(val1)
+                .addAll(val2)
+                .build();
+    }
+
+    private List<?> mergeListsImpl(List<?> val1, Iterable<?> val2, int depthRemaining, Visited visited) {
+        return MutableList.builder()
+                .addAll(val1)
+                .addAll(val2)
+                .build();
+    }
+
+    /**
+     * For avoiding infinite loops, we need to know which objects we have already visited. 
+     * If we come across that object again, then want to return the same result (rather than
+     * re-visiting it). It is based on "same" (i.e. "==").
+     */
+    protected static class Visited {
+        private static final Set<Class<?>> TRIVIAL_CLASSES = ImmutableSet.<Class<?>>of(
+                Integer.class, Long.class, Boolean.class, Byte.class, Double.class, Float.class, Character.class, Short.class,
+                String.class, BigInteger.class, BigDecimal.class, Date.class);
+
+        protected static class Ref {
+            protected final Object obj;
+            
+            protected Ref(Object obj) {
+                this.obj = checkNotNull(obj, "ref");
+            }
+            
+            @Override
+            public boolean equals(Object o) {
+                if (!(o instanceof Ref)) {
+                    return false;
+                }
+                return ((Ref)o).obj == ((Ref)o).obj;
+            }
+            
+            @Override
+            public int hashCode() {
+                return System.identityHashCode(obj);
+            }
+            
+            @Override
+            public String toString() {
+                return "Ref["+obj+"]";
+            }
+        }
+
+        protected final Set<Ref> visited = Sets.newLinkedHashSet();
+
+        public boolean isVisited(Object o) {
+            if (isTrivial(o)) return false;
+            return visited.contains(new Ref(o));
+        }
+
+        public void recordVisit(Object o) {
+            if (isTrivial(o)) return;
+            visited.add(new Ref(o));
+        }
+        
+        protected boolean isTrivial(Object o) {
+            if (o == null)  return true;
+            if (o instanceof Map && ((Map)o).isEmpty()) return true;
+            if (o instanceof Iterable && Iterables.isEmpty(((Iterable)o))) return true;
+            Class<?> clazz = o.getClass();
+            return clazz.isEnum() || clazz.isPrimitive() || TRIVIAL_CLASSES.contains(clazz);
+        }
+    }
+}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/97807f24/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionMergerTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionMergerTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionMergerTest.java
new file mode 100644
index 0000000..7ff7bba
--- /dev/null
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/collections/CollectionMergerTest.java
@@ -0,0 +1,409 @@
+/*
+ * 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 static org.testng.Assert.assertEquals;
+
+import java.io.StringReader;
+import java.util.Map;
+
+import org.apache.brooklyn.test.Asserts;
+import org.testng.annotations.Test;
+import org.yaml.snakeyaml.Yaml;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
+public class CollectionMergerTest {
+    
+    @Test
+    public void testMergeMapsEmpty() {
+        Map<String, String> val1 = ImmutableMap.of();
+        Map<String, Object> val2 = ImmutableMap.of();
+        Map<?, ?> result = CollectionMerger.builder().build().merge(val1, val2);
+
+        assertEquals(result, ImmutableMap.of());
+    }
+
+    @Test
+    public void testMergeMapsSimple() {
+        Map<?, ?> val1 = ImmutableMap.of("key1", "val1a", "key2", "val2a");
+        Map<?, ?> val2 = ImmutableMap.of("key1", "val1b", "key3", "val3b");
+        Map<?, ?> resultDeep = CollectionMerger.builder().build().merge(val1, val2);
+        Map<?, ?> resultShallow = CollectionMerger.builder().deep(false).build().merge(val1, val2);
+        
+        assertEquals(resultDeep, ImmutableMap.of("key1", "val1a", "key2", "val2a", "key3", "val3b"));
+        assertEquals(resultShallow, ImmutableMap.of("key1", "val1a", "key2", "val2a", "key3", "val3b"));
+    }
+
+    @Test
+    public void testAvoidInfiniteLoop() {
+        {
+            Map<Object, Object> val1 = MutableMap.<Object, Object>of("key1", "val1a");
+            val1.put("key2", val1);
+            Map<Object, Object> val2 = MutableMap.<Object, Object>of("key3", "val3a");
+            try {
+                CollectionMerger.builder().build().merge(val1, val2);
+                Asserts.shouldHaveFailedPreviously();
+            } catch (IllegalStateException e) {
+                Asserts.expectedFailureContains(e, "Recursive self-reference");
+            }
+        }
+        
+        {
+            Map<Object, Object> val1 = MutableMap.<Object, Object>of("key1", "val1a");
+            Map<Object, Object> val2 = MutableMap.<Object, Object>of("key3", "val3a");
+            val1.put("key4", val2);
+            try {
+                CollectionMerger.builder().build().merge(val1, val2);
+                Asserts.shouldHaveFailedPreviously();
+            } catch (IllegalStateException e) {
+                Asserts.expectedFailureContains(e, "Recursive self-reference");
+            }
+        }
+    }
+
+    @Test
+    public void testMergeMapsWithDeepSubMaps() {
+        String yaml1 = Joiner.on("\n").join(
+                "key1: val1",
+                "key2:",
+                "  key2.1: val2.1a",
+                "  key2.2:", 
+                "    key2.2.1: val2.2.1a",
+                "    key2.2.2:",
+                "      key2.2.2.1: val2.2.2.1a",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: val2.2.2.2.1a");
+        String yaml2 = Joiner.on("\n").join(
+                "key1: override-ignored",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1: override-ignored",
+                "  key2.1b: val2.1b",
+                "  key2.2:", 
+                "    key2.2.1: override-ignored",
+                "    key2.2.1b: val2.2.1b",
+                "    key2.2.2:",
+                "      key2.2.2.1: override-ignored",
+                "      key2.2.2.1b: val2.2.2.1b",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: override-ignored",
+                "        key2.2.2.2.1b: val2.2.2.2.1b");
+        Map<?, ?> val1 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml1));
+        Map<?, ?> val2 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml2));
+        
+        Map<?, ?> resultDepth1 = CollectionMerger.builder().depth(1).build().merge(val1, val2);
+        Map<?, ?> resultDepth2 = CollectionMerger.builder().depth(2).build().merge(val1, val2);
+        Map<?, ?> resultDepth3 = CollectionMerger.builder().depth(3).build().merge(val1, val2);
+        Map<?, ?> resultDepth4 = CollectionMerger.builder().depth(4).build().merge(val1, val2);
+        Map<?, ?> resultDepth5 = CollectionMerger.builder().depth(5).build().merge(val1, val2);
+        Map<?, ?> resultShallow = CollectionMerger.builder().deep(false).build().merge(val1, val2);
+        Map<?, ?> resultDeep = CollectionMerger.builder().build().merge(val1, val2);
+        
+        assertEquals(resultDepth1, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1: val2.1a",
+                "  key2.2:", 
+                "    key2.2.1: val2.2.1a",
+                "    key2.2.2:",
+                "      key2.2.2.1: val2.2.2.1a",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: val2.2.2.2.1a"))));
+        assertEquals(resultDepth2, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1: val2.1a",
+                "  key2.1b: val2.1b",
+                "  key2.2:", 
+                "    key2.2.1: val2.2.1a",
+                "    key2.2.2:",
+                "      key2.2.2.1: val2.2.2.1a",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: val2.2.2.2.1a"))));
+        assertEquals(resultDepth3, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1: val2.1a",
+                "  key2.1b: val2.1b",
+                "  key2.2:", 
+                "    key2.2.1: val2.2.1a",
+                "    key2.2.1b: val2.2.1b",
+                "    key2.2.2:",
+                "      key2.2.2.1: val2.2.2.1a",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: val2.2.2.2.1a"))));
+        assertEquals(resultDepth4, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1: val2.1a",
+                "  key2.1b: val2.1b",
+                "  key2.2:", 
+                "    key2.2.1: val2.2.1a",
+                "    key2.2.1b: val2.2.1b",
+                "    key2.2.2:",
+                "      key2.2.2.1: val2.2.2.1a",
+                "      key2.2.2.1b: val2.2.2.1b",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: val2.2.2.2.1a"))));
+        assertEquals(resultDepth5, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1: val2.1a",
+                "  key2.1b: val2.1b",
+                "  key2.2:", 
+                "    key2.2.1: val2.2.1a",
+                "    key2.2.1b: val2.2.1b",
+                "    key2.2.2:",
+                "      key2.2.2.1: val2.2.2.1a",
+                "      key2.2.2.1b: val2.2.2.1b",
+                "      key2.2.2.2:",
+                "        key2.2.2.2.1: val2.2.2.2.1a",
+                "        key2.2.2.2.1b: val2.2.2.2.1b"))));
+        assertEquals(resultDeep, resultDepth5);
+        assertEquals(resultShallow, resultDepth1);
+    }
+
+    @Test
+    public void testMergeMapsWithNullOverridesOther() {
+        // Expect "key2:" to have a null value (rather than just empty).
+        String yaml1 = Joiner.on("\n").join(
+                "key1: val1",
+                "key2:");
+        String yaml2 = Joiner.on("\n").join(
+                "key1: override-ignored",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1b: val2.1b");
+        Map<?, ?> val1 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml1));
+        Map<?, ?> val2 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml2));
+        
+        Map<?, ?> resultDepth1 = CollectionMerger.builder().depth(1).build().merge(val1, val2);
+        Map<?, ?> resultDepth2 = CollectionMerger.builder().depth(2).build().merge(val1, val2);
+        
+        assertEquals(resultDepth1, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:"))));
+        assertEquals(resultDepth2, resultDepth1);
+    }
+    
+    @Test
+    public void testMergeMapsWithEmptyIsMerged() {
+        // Expect "key2:" to have a null value (rather than just empty).
+        String yaml1 = Joiner.on("\n").join(
+                "key1: val1",
+                "key2: {}");
+        String yaml2 = Joiner.on("\n").join(
+                "key1: override-ignored",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1b: val2.1b");
+        Map<?, ?> val1 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml1));
+        Map<?, ?> val2 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml2));
+        
+        Map<?, ?> resultDepth1 = CollectionMerger.builder().depth(1).build().merge(val1, val2);
+        Map<?, ?> resultDepth2 = CollectionMerger.builder().depth(2).build().merge(val1, val2);
+        
+        assertEquals(resultDepth1, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2: {}"))));
+        assertEquals(resultDepth2, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1: val1",
+                "key1b: val1b",
+                "key2:",
+                "  key2.1b: val2.1b"))));
+    }
+
+    @Test
+    public void testMergeMapsDefaultsToOverridingSubLists() {
+        Map<?, ?> val1 = ImmutableMap.of("key1", ImmutableList.of("val1a"));
+        Map<?, ?> val2 = ImmutableMap.of("key1", ImmutableList.of("val1b"));
+        
+        Map<?, ?> resultDepth1 = CollectionMerger.builder().depth(1).build().merge(val1, val2);
+        Map<?, ?> resultDepth2 = CollectionMerger.builder().depth(2).build().merge(val1, val2);
+        
+        assertEquals(resultDepth1, ImmutableMap.of("key1", ImmutableList.of("val1a")));
+        assertEquals(resultDepth2, resultDepth1);
+    }
+
+    @Test
+    public void testMergeMapsWithMergingSubListsRespectsTypes() {
+        Map<?, ?> val1 = ImmutableMap.of("key1", ImmutableList.of("val1a"));
+        Map<?, ?> val2 = ImmutableMap.of("key1", ImmutableList.of("val1b"));
+        Map<?, ?> result = CollectionMerger.builder().mergeNestedLists(true).build().merge(val1, val2);
+        
+        assertEquals(result, ImmutableMap.of("key1", ImmutableList.of("val1a", "val1b")));
+    }
+
+    @Test
+    public void testMergeMapsWithMergingSubSetsRespectsTypes() {
+        Map<?, ?> val1 = ImmutableMap.of("key1", ImmutableSet.of("val1a"));
+        Map<?, ?> val2 = ImmutableMap.of("key1", ImmutableSet.of("val1b"));
+        Map<?, ?> result = CollectionMerger.builder().mergeNestedLists(true).build().merge(val1, val2);
+        
+        assertEquals(result, ImmutableMap.of("key1", ImmutableSet.of("val1a", "val1b")));
+    }
+
+    @Test
+    public void testMergeMapsWithMergingSubLists() {
+        String yaml1 = Joiner.on("\n").join(
+                "key1:",
+                "- key1.1",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1");
+        String yaml2 = Joiner.on("\n").join(
+                "key1:",
+                "- key1.1b",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1b",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1b",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1b");
+        Map<?, ?> val1 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml1));
+        Map<?, ?> val2 = (Map<?, ?>) Iterables.getOnlyElement(parseYaml(yaml2));
+        
+        Map<?, ?> resultDepth1 = CollectionMerger.builder().mergeNestedLists(true).depth(1).build().merge(val1, val2);
+        Map<?, ?> resultDepth2 = CollectionMerger.builder().mergeNestedLists(true).depth(2).build().merge(val1, val2);
+        Map<?, ?> resultDepth3 = CollectionMerger.builder().mergeNestedLists(true).depth(3).build().merge(val1, val2);
+        Map<?, ?> resultDepth4 = CollectionMerger.builder().mergeNestedLists(true).depth(4).build().merge(val1, val2);
+        Map<?, ?> resultDepth5 = CollectionMerger.builder().mergeNestedLists(true).depth(5).build().merge(val1, val2);
+        Map<?, ?> resultShallow = CollectionMerger.builder().mergeNestedLists(true).deep(false).build().merge(val1, val2);
+        Map<?, ?> resultDeep = CollectionMerger.builder().mergeNestedLists(true).build().merge(val1, val2);
+        
+        assertEquals(resultDepth1, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1:",
+                "- key1.1",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1"))));
+        assertEquals(resultDepth2, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1:",
+                "- key1.1",
+                "- key1.1b",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1"))));
+        assertEquals(resultDepth3, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1:",
+                "- key1.1",
+                "- key1.1b",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1",
+                "  - key2.1.1b",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1"))));
+        assertEquals(resultDepth4, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1:",
+                "- key1.1",
+                "- key1.1b",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1",
+                "  - key2.1.1b",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1",
+                "    - key3.1.1.1b",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1"))));
+        assertEquals(resultDepth5, Iterables.getOnlyElement(parseYaml(Joiner.on("\n").join(
+                "key1:",
+                "- key1.1",
+                "- key1.1b",
+                "key2:",
+                "  key2.1:",
+                "  - key2.1.1",
+                "  - key2.1.1b",
+                "key3:",
+                "  key3.1:",
+                "    key3.1.1:",
+                "    - key3.1.1.1",
+                "    - key3.1.1.1b",
+                "key4:",
+                "  key4.1:",
+                "    key4.1.1:",
+                "      key4.1.1.1:",
+                "      - key4.1.1.1.1",
+                "      - key4.1.1.1.1b"))));
+        assertEquals(resultDeep, resultDepth5);
+        assertEquals(resultShallow, resultDepth1);
+    }
+
+    protected Iterable<?> parseYaml(String yaml) {
+        return new Yaml().loadAll(new StringReader(yaml));
+    }
+}


[3/9] brooklyn-server git commit: Adds JcloudsLocation.LOOKUP_AWS_HOSTNAME

Posted by sv...@apache.org.
Adds JcloudsLocation.LOOKUP_AWS_HOSTNAME

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/e6be5b1c
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/e6be5b1c
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/e6be5b1c

Branch: refs/heads/master
Commit: e6be5b1c10035e3c0b3a0773e1556f97ae321d69
Parents: 6a1ac3f
Author: Aled Sage <al...@gmail.com>
Authored: Wed Jun 1 17:34:17 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Jun 6 15:10:09 2016 +0100

----------------------------------------------------------------------
 .../apache/brooklyn/location/jclouds/JcloudsLocation.java   | 9 ++++++---
 .../brooklyn/location/jclouds/JcloudsLocationConfig.java    | 6 ++++++
 2 files changed, 12 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e6be5b1c/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
index c87678f..fb68819 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
@@ -3072,9 +3072,10 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
      */
     protected String getPublicHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort, LoginCredentials userCredentials, ConfigBag setup) {
         String provider = (setup != null) ? setup.get(CLOUD_PROVIDER) : null;
+        Boolean lookupAwsHostname = (setup != null) ? setup.get(LOOKUP_AWS_HOSTNAME) : null;
         if (provider == null) provider= getProvider();
 
-        if ("aws-ec2".equals(provider)) {
+        if ("aws-ec2".equals(provider) && Boolean.TRUE.equals(lookupAwsHostname)) {
             HostAndPort inferredHostAndPort = null;
             if (!sshHostAndPort.isPresent()) {
                 try {
@@ -3168,12 +3169,14 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
     
     protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort, LoginCredentials userCredentials, ConfigBag setup) {
         String provider = (setup != null) ? setup.get(CLOUD_PROVIDER) : null;
-        if (provider == null) provider= getProvider();
+        Boolean lookupAwsHostname = (setup != null) ? setup.get(LOOKUP_AWS_HOSTNAME) : null;
+        
+        if (provider == null) provider = getProvider();
 
         // TODO Discouraged to do cloud-specific things; think of this code for aws as an
         // exceptional situation rather than a pattern to follow. We need a better way to
         // do cloud-specific things.
-        if ("aws-ec2".equals(provider)) {
+        if ("aws-ec2".equals(provider) && Boolean.TRUE.equals(lookupAwsHostname)) {
             Maybe<String> result = getPrivateHostnameAws(node, sshHostAndPort, userCredentials, setup);
             if (result.isPresent()) return result.get();
         }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e6be5b1c/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
index 1403a9f..35da672 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
@@ -234,6 +234,12 @@ public interface JcloudsLocationConfig extends CloudLocationConfig {
                     + "This config is beta; its default could be changed and/or the option removed in an upcoming release.", 
             true);
     
+    @Beta
+    public static final ConfigKey<Boolean> LOOKUP_AWS_HOSTNAME = ConfigKeys.newBooleanConfigKey(
+            "lookupAwsHostname", 
+            "Whether to lookup the AWS hostname (via a command on the VM), or to just use the IP.", 
+            true);
+    
     public static final ConfigKey<JcloudsPortForwarderExtension> PORT_FORWARDER = ConfigKeys.newConfigKey(
             JcloudsPortForwarderExtension.class, "portforwarding.forwarder", "The port-forwarder to use");