brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #340: ConfigMap refactor, respecting inheritanc...
Date Thu, 22 Sep 2016 07:56:34 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/340#discussion_r79990250
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
---
    @@ -107,4 +252,227 @@ public int size() {
         public boolean isEmpty() {
             return ownConfig.isEmpty();
         }
    +    
    +    protected ConfigInheritance getDefaultRuntimeInheritance() {
    +        return BasicConfigInheritance.OVERWRITE; 
    +    }
    +
    +    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 Maybe<Object> getRawValueAtContainer(TContainer container, ConfigKey<?
extends Object> configKey) {
    +        return ((BrooklynObjectInternal)container).config().getInternalConfigMap().getConfigLocalRaw(configKey);
    +    }
    +    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));
    +        }
    +    }
    +    
    +    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;
    +            
    +            currentInheritance = ConfigInheritances.findInheritance(next.getKey(), InheritanceContext.RUNTIME_MANAGEMENT,
currentInheritance);
    +            if (count>0 && !currentInheritance.isReinheritable(next, context))
break;
    +            
    +            if (next.isValueExplicitlySet()) result.add(0, next);
    +
    +            last = next;
    +            c = getParentOfContainer(c);
    +        }
    +        
    +        return result;
    +    }
    +    
    +    @Override
    +    public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>>
filter) {
    +        MutableSet<ConfigKey<?>> result = MutableSet.of();
    +        result.addAll(Iterables.filter(ownConfig.keySet(), filter));
    +        // due to set semantics local should be added first, it prevents equal items
from parent from being added on top
    +        if (getParent()!=null) {
    +            result.addAll( getParentInternal().config().getInternalConfigMap().findKeys(filter)
);
    +        }
    +        return result;
    +    }
    +    
    +    @SuppressWarnings("unchecked")
    +    public ReferenceWithError<ConfigValueAtContainer<TContainer,?>> getConfigInheritedRaw(ConfigKey<?>
key) {
    +        return (ReferenceWithError<ConfigValueAtContainer<TContainer,?>>)
(ReferenceWithError<?>) getConfigImpl(key, true);
    +    }
    +    
    +    @Override
    +    public Map<ConfigKey<?>, Object> getAllConfigInheritedRawValuesIgnoringErrors()
{
    +        Map<ConfigKey<?>, ReferenceWithError<ConfigValueAtContainer<TContainer,
?>>> input = getAllConfigInheritedRawWithErrors();
    +        Map<ConfigKey<?>, Object> result = MutableMap.of();
    +        for (Map.Entry<ConfigKey<?>, ReferenceWithError<ConfigValueAtContainer<TContainer,
?>>> pair: input.entrySet()) {
    +            result.put(pair.getKey(), pair.getValue().getWithoutError().get());
    +        }
    +        return result;
    +    }
    +    @Override
    +    public Map<ConfigKey<?>, ReferenceWithError<ConfigValueAtContainer<TContainer,
?>>> getAllConfigInheritedRawWithErrors() {
    +        return getSelectedConfigInheritedRaw(false);
    +    }
    +    
    +    public Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
getAllReinheritableConfigRaw() {
    +        return getSelectedConfigInheritedRaw(true);
    +    }
    +
    +    protected Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
getSelectedConfigInheritedRaw(boolean onlyReinheritable) {
    +        Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
parents = MutableMap.of();
    +        if (getParent()!=null) {
    +            @SuppressWarnings("unchecked")
    +            Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
po = (Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>)
(Map<?,?>)
    +                getParentInternal().config().getInternalConfigMap().getAllReinheritableConfigRaw();
    +            parents.putAll(po);
    +        }
    +        
    +        Map<ConfigKey<?>, Object> local = getAllConfigLocalRaw();
    +        
    +        Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
result = MutableMap.of();
    +
    +        for (ConfigKey<?> k: local.keySet()) {
    +            Object v = local.get(k);
    +            ReferenceWithError<ConfigValueAtContainer<TContainer, ?>> vpr
= parents.remove(k);
    +            @SuppressWarnings("unchecked")
    +            ConfigValueAtContainer<TContainer, Object> vp = vpr==null ? null :
(ConfigValueAtContainer<TContainer,Object>) vpr.getWithoutError();
    +            ConfigInheritance inh = ConfigInheritances.findInheritance(k, InheritanceContext.RUNTIME_MANAGEMENT,
getDefaultRuntimeInheritance());
    +            ConfigValueAtContainer<TContainer,Object> vl = new BasicConfigValueAtContainer<TContainer,Object>(getContainer(),
k, Maybe.ofAllowingNull(v));
    +            ReferenceWithError<ConfigValueAtContainer<TContainer, Object>>
vlr = null;
    +            if (inh.considerParent(vl, vp, InheritanceContext.RUNTIME_MANAGEMENT)) {
    --- End diff --
    
    It feels strange to insist that the caller always calls `considerParent` before calling
`resolveWithParent` (and to likely have an incorrect result if `resolveWithParent` is called
without first checking `considerParent`).
    
    Would it make sense to have a single method call that does this: a resolve method which
would do the internal considerParent check (i.e. the logic here, but inside the `BasicConfigInheritance`)?


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