brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From geomacy <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #462: Inherit config default values
Date Wed, 15 Feb 2017 15:25:06 GMT
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/462#discussion_r101298270
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/objs/BrooklynDynamicType.java ---
    @@ -175,20 +179,29 @@ protected static void buildConfigKeys(Class<? extends BrooklynObject>
clazz, Abs
                     LOG.warn("cannot access config key (skipping): "+f);
                 }
             }
    +        Collection<Class<?>> interfaces = MutableSet.copyOf(Arrays.asList(clazz.getInterfaces()));
             LinkedHashSet<String> keys = new LinkedHashSet<String>(configKeysAll.keys());
             for (String kn: keys) {
                 List<FieldAndValue<ConfigKey<?>>> kk = Lists.newArrayList(configKeysAll.get(kn));
    -            if (kk.size()>1) {
    -                // remove anything which extends another value in the list
    -                for (FieldAndValue<ConfigKey<?>> k: kk) {
    -                    ConfigKey<?> key = value(k);
    -                    if (key instanceof BasicConfigKeyOverwriting) {                 
          
    -                        ConfigKey<?> parent = ((BasicConfigKeyOverwriting<?>)key).getParentKey();
    -                        // find and remove the parent from consideration
    -                        for (FieldAndValue<ConfigKey<?>> k2: kk) {
    -                            if (value(k2) == parent)
    -                                configKeysAll.remove(kn, k2);
    -                        }
    +            // remove anything which isn't reinheritable, or which is overridden
    +            for (FieldAndValue<ConfigKey<?>> k: kk) {
    +                ConfigKey<?> key = value(k);
    +                if (!ConfigInheritances.isKeyReinheritable(key, InheritanceContext.TYPE_DEFINITION))
{
    +                    if (k.field.getDeclaringClass().equals(clazz)) {
    +                        // proceed if key is declared on this class
    +                    } else if (interfaces.contains(k.field.getDeclaringClass())) {
    +                        // proceed if key is declared on an exlicitly declared interface
    +                    } else {
    +                        // key not re-inheritable from parent so not visible here
    +                        configKeysAll.remove(k.value.getName(), k);
    +                    }
    +                }
    +                if (key instanceof BasicConfigKeyOverwriting) {                     
      
    +                    ConfigKey<?> parent = ((BasicConfigKeyOverwriting<?>)key).getParentKey();
    +                    // find and remove the parent from consideration
    +                    for (FieldAndValue<ConfigKey<?>> k2: kk) {
    +                        if (value(k2) == parent)
    +                            configKeysAll.remove(kn, k2);
                         }
                     }
                     kk = Lists.newArrayList(configKeysAll.get(kn));
    --- End diff --
    
    You've moved this update of `kk` inside the `for( ... k : kk)` loop, which doesn't look
right. Updating the list variable each time round an iterator initialised from that variable?


---
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.
---

Mime
View raw message