using new config inheritance evaluation, inserted in one place (entities)
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/29706561 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/29706561 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/29706561 Branch: refs/heads/master Commit: 29706561bfba35953deb420d478ae0cb9035863a Parents: 189677d Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Fri Sep 16 09:36:37 2016 +0100 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Tue Sep 20 08:48:27 2016 +0100 ---------------------------------------------------------------------- .../core/config/BasicConfigInheritance.java | 66 ++++++ .../core/entity/internal/EntityConfigMap.java | 150 +++++++------- .../brooklyn/core/objs/BasicSpecParameter.java | 3 +- .../brooklyn/config/ConfigInheritance.java | 207 +++++++++++++++---- 4 files changed, 311 insertions(+), 115 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/29706561/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java new file mode 100644 index 0000000..a238809 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java @@ -0,0 +1,66 @@ +/* + * 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.core.config; + +import java.util.Iterator; + +import org.apache.brooklyn.config.ConfigInheritance; +import org.apache.brooklyn.config.ConfigKey; + +public class BasicConfigInheritance implements ConfigInheritance { + + private static final long serialVersionUID = -5916548049057961051L; + + // TODO javadoc + public static BasicConfigInheritance NOT_REINHERITED = new BasicConfigInheritance(false,"overwrite",false); + public static BasicConfigInheritance NEVER_INHERITED = new BasicConfigInheritance(false,"overwrite",true); + public static BasicConfigInheritance OVERWRITE = new BasicConfigInheritance(true,"overwrite",false); + public static BasicConfigInheritance DEEP_MERGE = new BasicConfigInheritance(true,"deep_merge",false); + +// reinheritable? true/false; if false, children/descendants/inheritors will never see it; default true + protected final boolean isReinherited; +// conflict-resolution-strategy? overwrite or deep_merge or null; default null meaning caller supplies + protected final String conflictResolutionStrategy; +// use-local-default-value? true/false; if true, overwrite above means "always ignore"; default false + protected final boolean useLocalDefaultValue; + + protected BasicConfigInheritance(boolean isReinherited, String conflictResolutionStrategy, boolean useLocalDefaultValue) { + super(); + this.isReinherited = isReinherited; + this.conflictResolutionStrategy = conflictResolutionStrategy; + this.useLocalDefaultValue = useLocalDefaultValue; + } + + @SuppressWarnings("deprecation") + @Override + public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { + return null; + } + + @Override + public <T> ContainerAndValue<T> resolveInheriting( + Iterator<ContainerAndKeyValue<T>> containerAndDataThroughAncestors, + ConfigInheritanceContext context) { +// XXX; + return null; + } + +} + + http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/29706561/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 25d1561..ee31779 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 @@ -22,22 +22,21 @@ import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.brooklyn.util.groovy.GroovyJavaMethods.elvis; import java.util.Collections; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.base.Predicate; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; - +import org.apache.brooklyn.api.entity.Entity; 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.ContainerAndKeyValue; +import org.apache.brooklyn.config.ConfigInheritance.ContainerAndValue; import org.apache.brooklyn.config.ConfigInheritance.InheritanceMode; import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.config.ConfigKeys.InheritanceContext; import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.core.config.StructuredConfigKey; import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; @@ -50,6 +49,12 @@ import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.core.internal.ConfigKeySelfExtracting; import org.apache.brooklyn.util.guava.Maybe; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Predicate; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; public class EntityConfigMap extends AbstractConfigMapImpl { @@ -94,15 +99,6 @@ public class EntityConfigMap extends AbstractConfigMapImpl { // but that example doesn't have a default... ConfigKey<T> ownKey = entity!=null ? (ConfigKey<T>)elvis(entity.getEntityType().getConfigKey(key.getName()), key) : key; - // TODO Confirm desired behaviour: I switched this around to get ownKey.getParentInheritance first. - ConfigInheritance inheritance = ownKey.getParentInheritance(); - if (inheritance==null) inheritance = key.getParentInheritance(); - if (inheritance==null) { - // TODO we could warn by introducing a temporary "ALWAYS_BUT_WARNING" instance - 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). // Long term, we'll just persist tasks properly so the call to onConfigChanged will go! @@ -110,7 +106,8 @@ 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); - Maybe<T> result = getConfigImpl((ConfigKeySelfExtracting<T>)ownKey, parentInheritanceMode); + // TODO this no longer looks at key's inheritance + Maybe<T> result = getConfigImpl((ConfigKeySelfExtracting<T>)ownKey); if (rawval instanceof Task) { entity.getManagementSupport().getEntityChangeListener().onConfigChanged(key); @@ -125,8 +122,8 @@ public class EntityConfigMap extends AbstractConfigMapImpl { } @SuppressWarnings("unchecked") - private <T> Maybe<T> getConfigImpl(ConfigKeySelfExtracting<T> key, InheritanceMode parentInheritance) { - ExecutionContext exec = entity.getExecutionContext(); + private <T> Maybe<T> getConfigImpl(final ConfigKeySelfExtracting<T> key) { + final ExecutionContext exec = entity.getExecutionContext(); Maybe<T> ownValue; Maybe<T> parentValue; @@ -145,76 +142,75 @@ public class EntityConfigMap extends AbstractConfigMapImpl { } else { ownValue = Maybe.<T>absent(); } + final Maybe<T> ownValueF = ownValue; - // 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(); + ContainerAndValue<T> result = getDefaultRuntimeInheritance().resolveInheriting(new Iterator<ContainerAndKeyValue<T>>() { + int count = 0; + @Override + public boolean hasNext() { + return count < 2; } - break; - case DEEP_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(); + @Override + public ContainerAndKeyValue<T> next() { + if (count >= 2) throw new NoSuchElementException(); + final boolean isLookingAtInheritedBag = (count==1); + try { + return new ContainerAndKeyValue<T>() { + @Override + public Object getContainer() { + // TODO the current inheritedConfigBag is not good enough to detect the ancestor container + return !isLookingAtInheritedBag ? entity : entity.getParent(); + } + @Override + public T getValue() { + return peekValue().orNull(); + } + protected Maybe<T> peekValue() { + if (!isLookingAtInheritedBag) return ownValueF; + if (((ConfigKeySelfExtracting<T>)key).isSet(inheritedConfig)) { + return Maybe.of( ((ConfigKeySelfExtracting<T>)key).extractValue(inheritedConfig, exec) ); + } else if (inheritedConfigBag.containsKey(key)) { + return Maybe.of(inheritedConfigBag.get(key)); + } else { + return Maybe.absent(); + } + } + + @Override + public boolean isValueSet() { + return peekValue().isPresent(); + } + + @Override + public ConfigKey<T> getKey() { + return key; + } + + @Override + public T getDefaultValue() { + return key.getDefaultValue(); + } + }; + } finally { + count++; + } } - 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 DEEP_MERGE: - return (Maybe<T>) deepMerge(ownValue, parentValue, key); - case NONE: - return ownValue; - default: - throw new IllegalStateException("Unsupported parent-inheritance mode for "+key.getName()+": "+parentInheritance); - } + @Override public void remove() { throw new UnsupportedOperationException(); } + }, InheritanceContext.RUNTIME_MANAGEMENT); + + if (result.isValueSet()) return Maybe.of(result.getValue()); + return Maybe.absent(); } - 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.getParentInheritance()); } private <T> boolean isInherited(ConfigKey<T> key, ConfigInheritance inheritance) { - if (inheritance==null) inheritance = getDefaultParentInheritance(); + if (inheritance==null) inheritance = getDefaultRuntimeInheritance(); InheritanceMode mode = inheritance.isInherited(key, entity.getParent(), entity); return mode != null && mode != InheritanceMode.NONE; } - private ConfigInheritance getDefaultParentInheritance() { + private ConfigInheritance getDefaultRuntimeInheritance() { return ConfigInheritance.ALWAYS; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/29706561/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 dc5ba3e..db0b7c3 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 @@ -311,7 +311,8 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ private static ConfigInheritance parseInheritance(Object obj, BrooklynClassLoadingContext loader) { if (obj == null || obj instanceof String) { - return ConfigInheritance.fromString((String)obj); + // TODO + return ConfigInheritance.Legacy.fromString((String)obj); } else { throw new IllegalArgumentException ("The config-inheritance '" + obj + "' for a catalog input is invalid format - string supported"); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/29706561/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 4bdbdda..a07a9d2 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 @@ -19,66 +19,199 @@ package org.apache.brooklyn.config; import java.io.Serializable; +import java.util.Iterator; +import java.util.Map; +import org.apache.brooklyn.util.collections.CollectionMerger; +import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.Strings; import com.google.common.annotations.Beta; @SuppressWarnings("serial") -public abstract class ConfigInheritance implements Serializable { +public interface ConfigInheritance extends Serializable { /** marker interface for inheritance contexts, for keys which can define one or more inheritance patterns */ public interface ConfigInheritanceContext {} + /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated @Beta public enum InheritanceMode { NONE, IF_NO_EXPLICIT_VALUE, DEEP_MERGE } + + /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated + public static final ConfigInheritance NONE = new Legacy.None(); + /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated + public static final ConfigInheritance ALWAYS = new Legacy.Always(); + /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated + public static final ConfigInheritance DEEP_MERGE = new Legacy.Merged(); + + @Deprecated + InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to); - public static final ConfigInheritance NONE = new None(); - public static final ConfigInheritance ALWAYS = new Always(); - public static final ConfigInheritance DEEP_MERGE = new Merged(); + interface ContainerAndValue<T> { + Object getContainer(); + T getValue(); + /** if false, the contents of {@link #getValue()} will have come from the default */ + boolean isValueSet(); + } - 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 "deepmerge" : - case "deep_merge" : - return DEEP_MERGE; - default: - throw new IllegalArgumentException("Invalid config-inheritance '"+val+"' (legal values are none, always or merge)"); - } + interface ContainerAndKeyValue<T> extends ContainerAndValue<T> { + ConfigKey<T> getKey(); + T getDefaultValue(); } - private ConfigInheritance() {} + /** + * given an iterable of the config containers (eg an entity) and associated data, + * with the first entry being the container of immediate interest + * and the subsequent nodes being the ancestors, + * this finds the value set for a config key after all inheritance strategies are applied, + * for instance merging a map throughout a container hierarchy, + * or traversing up until a non-reinheritable key definition is found and then returning the default value of the key + * <p> + * this uses an interface on the input so that: + * - the caller can supply the hierarchy + * - hierarchy is only traversed as far as needed + * - caller can do full resolution as required for local values + * <p> + * this returns in interface so that caller can get the value, + * and if needed also find the container where the key is defined. + * that can be useful for when the value needs to be further resolved, + * e.g. a DSL function or a URL. the returned value may be evaluated lazily, + * i.e. the actual traversal and evaluation may be deferred until a method on + * the returned object is invoked. + * <p> + * this object is taken as the default inheritance and used if no inheritance is + * defined on the key. + * <p> + * so that the caller can determine if a key/value is to be exported to children from a container, + * this method should accept an iterable whose first entry has a null container + * and whose second entry gives the container, key, and potential value to be exported. + * if null is returned the caller knows nothing is to be exported to children. + */ + <T> ContainerAndValue<T> resolveInheriting( + Iterator<ContainerAndKeyValue<T>> containerAndDataThroughAncestors, + ConfigInheritanceContext context); - @Beta - public abstract InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to); - - private static class Always extends ConfigInheritance { - @Override - public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { - return InheritanceMode.IF_NO_EXPLICIT_VALUE; + /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated + public static class Legacy { + 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 "deepmerge" : + case "deep_merge" : + return DEEP_MERGE; + default: + throw new IllegalArgumentException("Invalid config-inheritance '"+val+"' (legal values are none, always or merge)"); + } } - } - - private static class None extends ConfigInheritance { - @Override - public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { - return InheritanceMode.NONE; + private abstract static class LegacyAbstractConversion implements ConfigInheritance { + private static class Result<T> implements ContainerAndValue<T> { + Object container = null; + T value = null; + boolean isValueSet = false; + @Override public Object getContainer() { return container; } + @Override public T getValue() { return value; } + @Override public boolean isValueSet() { return isValueSet; } + } + @Override + public <T> ContainerAndValue<T> resolveInheriting( + Iterator<ContainerAndKeyValue<T>> containerAndLocalValues, + ConfigInheritanceContext context) { + if (containerAndLocalValues.hasNext()) { + ContainerAndKeyValue<T> c = containerAndLocalValues.next(); + if (c==null) return resolveInheriting(containerAndLocalValues, context); + + ConfigKey<T> key = c.getKey(); + ConfigInheritance ci = null; + if (key!=null) ci = key.getInheritanceByContext(context); + if (ci==null) ci = this; + + if (getMode()==InheritanceMode.NONE) { + // don't inherit, fall through to below + } else if (!c.isValueSet()) { + // no value here, try to inherit + ContainerAndValue<T> ri = ci.resolveInheriting(containerAndLocalValues, context); + if (ri.isValueSet()) { + // value found, return it + return ri; + } + // else no inherited, fall through to below + } else { + if (getMode()==InheritanceMode.IF_NO_EXPLICIT_VALUE) { + // don't inherit, fall through to below + } else { + // merging + Maybe<?> mr = deepMerge(asMaybe(c), + asMaybe(ci.resolveInheriting(containerAndLocalValues, context))); + if (mr.isPresent()) { + Result<T> r = new Result<T>(); + r.container = c.getContainer(); + r.isValueSet = true; + @SuppressWarnings("unchecked") + T vt = (T) mr.get(); + r.value = vt; + return r; + } + } + } + Result<T> r = new Result<T>(); + r.container = c.getContainer(); + r.isValueSet = c.isValueSet(); + r.value = r.isValueSet ? c.getValue() : c.getDefaultValue(); + return r; + } + return new Result<T>(); + } + @Override + public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { + return getMode(); + } + protected abstract InheritanceMode getMode(); + private static <T> Maybe<T> asMaybe(ContainerAndValue<T> cv) { + if (cv.isValueSet()) return Maybe.of(cv.getValue()); + return Maybe.absent(); + } + private static <T> Maybe<?> deepMerge(Maybe<? extends T> val1, Maybe<? extends T> val2) { + 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 + return val1; + } + } } - } - - private static class Merged extends ConfigInheritance { - @Override - public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { - return InheritanceMode.DEEP_MERGE; + private static class Always extends LegacyAbstractConversion { + @Override + public InheritanceMode getMode() { + return InheritanceMode.IF_NO_EXPLICIT_VALUE; + } + } + private static class None extends LegacyAbstractConversion { + @Override + public InheritanceMode getMode() { + return InheritanceMode.NONE; + } + } + private static class Merged extends LegacyAbstractConversion { + @Override + public InheritanceMode getMode() { + return InheritanceMode.DEEP_MERGE; + } } } + }
