[GitHub] brooklyn-server issue #462: Inherit config default values

[geomacy <gi...@git.apache.org>]

Github user geomacy commented on the issue:

    https://github.com/apache/brooklyn-server/pull/462
  
    
    
    Wrote a few extra tests to try out the change. The only one that didn't perform as I'd expected was
    
    ```java
        @Test
        public void testConfigDefaultIsNotInheritedWith_LocalDefaultResolvesWithAncestorValue_SetToTrue() throws Exception {
    
            addCatalogItems(
                "brooklyn.catalog:",
                "  itemType: entity",
                "  items:",
                "  - id: entity-with-keys",
                "    item:",
                "      type: "+TestEntity.class.getName(),
                "      brooklyn.parameters:",
                "      - name: my.param.key",
                "        type: string",
                "        inheritance.parent: never",
                "        description: description one",
                "        default: myDefaultVal",
                "      brooklyn.config:",
                "        my.other.key: $brooklyn:config(\"my.param.key\")");
    
            addCatalogItems(
                "brooklyn.catalog:",
                "  itemType: entity",
                "  items:",
                "  - id: wrapper-entity",
                "    item:",
                "      brooklyn.parameters:",
                "      - name: my.param.key",
                "        description: description two",
                "      type: entity-with-keys");
    
            String yaml = Joiner.on("\n").join(
                "services:",
                "- type: wrapper-entity");
    
            Entity app = createStartWaitAndLogApplication(yaml);
            final TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren());
            LOG.info("Config keys declared on "+entity+": "+entity.config().findKeysDeclared(Predicates.alwaysTrue()));
            ConfigKey<?> key = Iterables.getOnlyElement( entity.config().findKeysDeclared(ConfigPredicates.nameEqualTo("my.param.key")) );
            assertEquals(key.getDescription(), "description two");
            assertEquals(entity.config().get(key), null);
        }
    ```
    noting that the last assertion above expects the value to be null because the "inheritance.parent" 
    has been set to "never".  Actually the failure is 
    ```
    java.lang.AssertionError: expected [null] but found [myDefaultVal]
    Expected :null
    Actual   :myDefaultVal
     <Click to see difference>
    ```
    
    I debugged to see what goes on here, and found that the BasicConfigInheritance class is 
    doing what I expected, and the default value is *not* inherited.   Instead, the default
    is set higher up in the stack, in AbstractConfigurationSupportInternal#getNonBlockingResolvingSimple,
    at the [line](https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java#L139)
    ```
            Object unresolved = getRaw(key).or(key.getDefaultValue());
    ```
    
    so in this case the higher level code is ignoring and overriding the instructions of the 
    way the inheritance is configured on the key.  Maybe now that the inheritance code is 
    written this line and any others like it should be changed, to avoid "second-guessing" 
    the specified inheritance behaviour.
    
    
    
    
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---