Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/340#discussion_r80060372
  
    --- Diff: 
core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
 ---
    @@ -107,4 +264,264 @@ public int size() {
         public boolean isEmpty() {
             return ownConfig.isEmpty();
         }
    +    
    +    protected ConfigInheritance getDefaultRuntimeInheritance() {
    +        return BasicConfigInheritance.OVERWRITE; 
    +    }
    +
    +    @Override
    +    public <T> ReferenceWithError<ConfigValueAtContainer<TContainer,T>> 
getConfigAndContainer(ConfigKey<T> key) {
    +        return getConfigImpl(key, false);
    +    }
    +
    +    protected abstract TContainer getParentOfContainer(TContainer 
container);
    +    
    +    
    +    @Nullable protected final <T> ConfigKey<T> 
getKeyAtContainer(TContainer container, ConfigKey<T> queryKey) {
    +        if (container==null) return null;
    +        @SuppressWarnings("unchecked")
    +        ConfigKey<T> candidate = (ConfigKey<T>) 
getKeyAtContainerImpl(container, queryKey);
    +        return candidate;
    +    }
    +    
    +    @Nullable protected abstract <T> ConfigKey<?> 
getKeyAtContainerImpl(@Nonnull TContainer container, ConfigKey<T> queryKey);
    +    protected abstract Collection<ConfigKey<?>> 
getKeysAtContainer(@Nonnull TContainer container);
    +
    +    protected Maybe<Object> getRawValueAtContainer(TContainer container, 
ConfigKey<? extends Object> configKey) {
    +        return 
((BrooklynObjectInternal)container).config().getInternalConfigMap().getConfigLocalRaw(configKey);
    +    }
    +    /** finds the value at the given container/key, taking in to account 
any resolution expected by the key (eg for map keys).
    +     * the input is the value in the {@link #ownConfig} map taken from 
{@link #getRawValueAtContainer(BrooklynObject, ConfigKey)}, 
    +     * but the key may have other plans.
    +     * current impl just uses the key to extract again which is a little 
bit wasteful but simpler. 
    +     * <p>
    +     * this does not do any resolution with respect to ancestors. */
    +    protected Maybe<Object> resolveRawValueFromContainer(TContainer 
container, ConfigKey<?> key, Object value) {
    +        Map<ConfigKey<?>, Object> oc = ((AbstractConfigMapImpl<?>) 
((BrooklynObjectInternal)container).config().getInternalConfigMap()).ownConfig;
    +        if (key instanceof ConfigKeySelfExtracting) {
    +            if (((ConfigKeySelfExtracting<?>)key).isSet(oc)) {
    +                Map<ConfigKey<?>, ?> ownCopy;
    +                synchronized (oc) {
    +                    // wasteful to make a copy to look up; maybe try once 
opportunistically?
    +                    ownCopy = MutableMap.copyOf(oc);
    +                }
    +                return Maybe.of((Object) ((ConfigKeySelfExtracting<?>) 
key).extractValue(ownCopy, getExecutionContext(container)) );
    +            } else {
    +                return Maybe.absent();
    +            }
    +        } else {
    +            // all our keys are self-extracting
    +            LOG.warn("Unexpected key type "+key+" ("+key.getClass()+") in 
"+bo+"; ignoring value");
    +            return Maybe.absent();
    +        }
    +    }
    +
    +    @SuppressWarnings("unchecked")
    +    protected <T> T coerce(Object value, Class<T> type) {
    +        if (type==null || value==null) return (T) value;
    +        return (T) TypeCoercions.coerce(value, type);
    +    }
    +    
    +    protected <T> ReferenceWithError<ConfigValueAtContainer<TContainer,T>> 
getConfigImpl(final ConfigKey<T> queryKey, final boolean raw) {
    +        // In case this entity class has overridden the given key (e.g. to 
set default), then retrieve this entity's key
    +        Function<TContainer, ConfigKey<T>> keyFn = new 
Function<TContainer, ConfigKey<T>>() {
    +            @Override public ConfigKey<T> apply(TContainer input) {
    +                // should return null if the key is not known, to indicate 
selected inheritance rules from base key should take effect
    +                return getKeyAtContainer(input, queryKey);
    +            }
    +        };
    +        ConfigKey<T> ownKey = keyFn.apply(getContainer());
    +        if (ownKey==null) ownKey = queryKey;
    +        @SuppressWarnings("unchecked")
    +        final Class<T> type = (Class<T>) ownKey.getType();
    +        
    +        // takes type of own key (or query key if own key not available)
    +        // takes default of own key if available and has default, else of 
query key
    +        
    +        Function<Maybe<Object>, Maybe<T>> coerceFn = new 
Function<Maybe<Object>, Maybe<T>>() {
    +            @SuppressWarnings("unchecked") @Override public Maybe<T> 
apply(Maybe<Object> input) {
    +                if (raw || input==null || input.isAbsent()) return 
(Maybe<T>)input;
    +                return Maybe.ofAllowingNull(coerce(input.get(), type));
    +            }
    +        };
    +        // prefer default and type of ownKey
    +        Maybe<T> defaultValue = raw ? Maybe.<T>absent() :
    +            ownKey.hasDefaultValue() ? 
coerceFn.apply(Maybe.of((Object)ownKey.getDefaultValue())) : 
    +            queryKey.hasDefaultValue() ? 
coerceFn.apply(Maybe.of((Object)queryKey.getDefaultValue())) :
    +                Maybe.<T>absent();
    +            
    +        if (ownKey instanceof ConfigKeySelfExtracting) {
    +            
    +            Function<TContainer, Maybe<Object>> lookupFn = new 
Function<TContainer, Maybe<Object>>() {
    +                @Override public Maybe<Object> apply(TContainer input) {
    +                    Maybe<Object> result = getRawValueAtContainer(input, 
queryKey);
    +                    if (!raw) result = resolveRawValueFromContainer(input, 
queryKey, result);
    +                    return result;
    +                }
    +            };
    +            Function<TContainer, TContainer> parentFn = new 
Function<TContainer, TContainer>() {
    +                @Override public TContainer apply(TContainer input) {
    +                    return getParentOfContainer(input);
    +                }
    +            };
    +            AncestorContainerAndKeyValueIterator<TContainer, T> ckvi = new 
AncestorContainerAndKeyValueIterator<TContainer,T>(
    +                getContainer(), keyFn, lookupFn, coerceFn, parentFn);
    +            
    +            return ConfigInheritances.resolveInheriting(
    +                getContainer(), ownKey, 
coerceFn.apply(lookupFn.apply(getContainer())), defaultValue, 
    +                ckvi, InheritanceContext.RUNTIME_MANAGEMENT, 
getDefaultRuntimeInheritance());
    +            
    +        } else {
    +            String message = "Config key "+ownKey+" of 
"+getBrooklynObject()+" is not a ConfigKeySelfExtracting; cannot retrieve 
value; returning default";
    +            LOG.warn(message);
    +            return ReferenceWithError.newInstanceThrowingError(new 
BasicConfigValueAtContainer<TContainer,T>(getContainer(), ownKey, null, false,
    +                    defaultValue),
    +                new IllegalStateException(message));
    +        }
    +    }
    +    
    +    @Override
    +    public List<ConfigValueAtContainer<TContainer,?>> 
getConfigAllInheritedRaw(ConfigKey<?> queryKey) {
    +        List<ConfigValueAtContainer<TContainer, ?>> result = 
MutableList.of();
    +        TContainer c = getContainer();
    +        int count=0;
    +
    +        final InheritanceContext context = 
InheritanceContext.RUNTIME_MANAGEMENT;
    +        ConfigInheritance currentInheritance = 
ConfigInheritances.findInheritance(queryKey, context, 
getDefaultRuntimeInheritance());
    +        
    +        BasicConfigValueAtContainer<TContainer, Object> last = null;
    +        
    +        while (c!=null) {
    +            Maybe<Object> v = getRawValueAtContainer(c, queryKey);
    +            BasicConfigValueAtContainer<TContainer, Object> next = new 
BasicConfigValueAtContainer<TContainer, Object>(c, getKeyAtContainer(c, 
queryKey), v);
    +            
    +            if (last!=null && !currentInheritance.considerParent(last, 
next, context)) break;
    +            
    +            ConfigInheritance currentInheritanceExplicit = 
ConfigInheritances.findInheritance(next.getKey(), 
InheritanceContext.RUNTIME_MANAGEMENT, null);
    +            if (currentInheritanceExplicit!=null) {
    +                if (count>0 && 
!currentInheritanceExplicit.isReinheritable(next, context)) break;
    +                currentInheritance = currentInheritanceExplicit;
    +            }
    +            
    +            if (next.isValueExplicitlySet()) result.add(0, next);
    +
    +            last = next;
    +            c = getParentOfContainer(c);
    +            count++;
    +        }
    +        
    +        return result;
    +    }
    --- End diff --
    
    There feels like a scary looking number of methods on here now. I'm not 
suggesting any specific method(s) that should be removed, but it's something 
that we should be aware of - gut feel is that we have too many methods (even 
ignoring the deprecated ones).
    
    If we really need all these different ways of access it, then another would 
would be to present this as "views" - e.g. a raw view, a resolved view, a 
local-only view, etc.
    
    But any such change are much longer term, rather than part of this PR!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to