address code review comments - javadoc, tests, and a few fixes to the more obscure config map methods
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/8156322b Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/8156322b Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/8156322b Branch: refs/heads/master Commit: 8156322b4362c059516505e9d952ae66a81cfbab Parents: 0d655e0 Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Thu Sep 22 12:45:47 2016 +0100 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Thu Sep 22 12:47:08 2016 +0100 ---------------------------------------------------------------------- .../apache/brooklyn/api/objs/Configurable.java | 2 +- .../brooklyn/ConfigInheritanceYamlTest.java | 111 ++++++++++--------- .../brooklyn/camp/brooklyn/ObjectsYamlTest.java | 2 +- .../core/config/BasicConfigInheritance.java | 49 +++++--- .../apache/brooklyn/core/config/ConfigKeys.java | 3 +- .../config/internal/AbstractConfigMapImpl.java | 107 +++++++++++++----- .../internal/LazyContainerAndKeyValue.java | 19 +++- .../core/entity/internal/EntityConfigMap.java | 8 ++ .../core/internal/BrooklynProperties.java | 6 +- .../core/internal/BrooklynPropertiesImpl.java | 2 +- .../location/internal/LocationConfigMap.java | 6 + .../internal/DeferredBrooklynProperties.java | 4 +- .../AbstractConfigurationSupportInternal.java | 2 +- .../brooklyn/core/objs/AdjunctConfigMap.java | 6 + .../core/objs/BasicConfigurableObject.java | 2 +- .../entity/ConfigEntityInheritanceTest.java | 69 +++++++++++- .../core/test/BrooklynAppUnitTestSupport.java | 8 +- .../util/core/internal/FlagUtilsTest.java | 2 +- .../brooklyn/config/ConfigInheritance.java | 4 +- .../brooklyn/config/ConfigInheritances.java | 31 ++++-- .../org/apache/brooklyn/config/ConfigMap.java | 2 +- .../brooklyn/config/ConfigValueAtContainer.java | 20 +++- 22 files changed, 325 insertions(+), 140 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java index ea36c4d..5957d62 100644 --- a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java +++ b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java @@ -98,6 +98,6 @@ public interface Configurable { */ <T> T set(HasConfigKey<T> key, Task<T> val); - Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> predicate); + Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> predicate); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java index 9a71aed..a6fe62c 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigInheritanceYamlTest.java @@ -20,8 +20,7 @@ package org.apache.brooklyn.camp.brooklyn; import static org.testng.Assert.assertEquals; -import java.nio.file.Files; -import java.nio.file.Path; +import java.io.File; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; @@ -38,6 +37,7 @@ import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool; +import org.apache.brooklyn.util.os.Os; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.annotations.AfterMethod; @@ -57,9 +57,9 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { @SuppressWarnings("unused") private static final Logger LOG = LoggerFactory.getLogger(ConfigInheritanceYamlTest.class); - private Path emptyFile; - private Path emptyFile2; - private Path emptyFile3; + private File emptyFile; + private File emptyFile2; + private File emptyFile3; private ExecutorService executor; @@ -70,9 +70,9 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { executor = Executors.newCachedThreadPool(); - emptyFile = Files.createTempFile("ConfigInheritanceYamlTest", ".txt"); - emptyFile2 = Files.createTempFile("ConfigInheritanceYamlTest2", ".txt"); - emptyFile3 = Files.createTempFile("ConfigInheritanceYamlTest3", ".txt"); + emptyFile = Os.newTempFile("ConfigInheritanceYamlTest", ".txt"); + emptyFile2 = Os.newTempFile("ConfigInheritanceYamlTest2", ".txt"); + emptyFile3 = Os.newTempFile("ConfigInheritanceYamlTest3", ".txt"); addCatalogItems( "brooklyn.catalog:", @@ -84,17 +84,17 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " shell.env:", " ENV1: myEnv1", " templates.preinstall:", - " "+emptyFile.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", " files.preinstall:", - " "+emptyFile.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", " templates.install:", - " "+emptyFile.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", " files.install:", - " "+emptyFile.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", " templates.runtime:", - " "+emptyFile.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", " files.runtime:", - " "+emptyFile.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", " provisioning.properties:", " mykey: myval", " templateOptions:", @@ -136,8 +136,9 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { public void tearDown() throws Exception { super.tearDown(); if (executor != null) executor.shutdownNow(); - if (emptyFile != null) Files.deleteIfExists(emptyFile); - if (emptyFile2 != null) Files.deleteIfExists(emptyFile2); + if (emptyFile != null) emptyFile.delete(); + if (emptyFile2 != null) emptyFile2.delete(); + if (emptyFile3 != null) emptyFile3.delete(); } @Test @@ -153,7 +154,7 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { assertEmptySoftwareProcessConfig( entity, ImmutableMap.of("ENV1", "myEnv1"), - ImmutableMap.of(emptyFile.toUri().toString(), "myfile"), + ImmutableMap.of(emptyFile.getAbsolutePath(), "myfile"), ImmutableMap.of("mykey", "myval", "templateOptions", ImmutableMap.of("myOptionsKey", "myOptionsVal"))); } @@ -167,17 +168,17 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " shell.env:", " ENV2: myEnv2", " templates.preinstall:", - " "+emptyFile2.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", " files.preinstall:", - " "+emptyFile2.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", " templates.install:", - " "+emptyFile2.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", " files.install:", - " "+emptyFile2.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", " templates.runtime:", - " "+emptyFile2.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", " files.runtime:", - " "+emptyFile2.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", " provisioning.properties:", " mykey2: myval2", " templateOptions:", @@ -189,7 +190,7 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { assertEmptySoftwareProcessConfig( entity, ImmutableMap.of("ENV1", "myEnv1", "ENV2", "myEnv2"), - ImmutableMap.of(emptyFile.toUri().toString(), "myfile", emptyFile2.toUri().toString(), "myfile2"), + ImmutableMap.of(emptyFile.getAbsolutePath(), "myfile", emptyFile2.getAbsolutePath(), "myfile2"), ImmutableMap.of("mykey", "myval", "mykey2", "myval2", "templateOptions", ImmutableMap.of("myOptionsKey", "myOptionsVal", "myOptionsKey2", "myOptionsVal2"))); } @@ -323,23 +324,23 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " ENV: myEnv", " ENV3: myEnv", " templates.preinstall:", - " "+emptyFile.toUri()+": myfile", - " "+emptyFile3.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", + " "+emptyFile3.getAbsolutePath()+": myfile3", " files.preinstall:", - " "+emptyFile.toUri()+": myfile", - " "+emptyFile3.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", + " "+emptyFile3.getAbsolutePath()+": myfile3", " templates.install:", - " "+emptyFile.toUri()+": myfile", - " "+emptyFile3.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", + " "+emptyFile3.getAbsolutePath()+": myfile3", " files.install:", - " "+emptyFile.toUri()+": myfile", - " "+emptyFile3.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", + " "+emptyFile3.getAbsolutePath()+": myfile3", " templates.runtime:", - " "+emptyFile.toUri()+": myfile", - " "+emptyFile3.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", + " "+emptyFile3.getAbsolutePath()+": myfile3", " files.runtime:", - " "+emptyFile.toUri()+": myfile", - " "+emptyFile3.toUri()+": myfile", + " "+emptyFile.getAbsolutePath()+": myfile", + " "+emptyFile3.getAbsolutePath()+": myfile3", " provisioning.properties:", " mykey: myval", " mykey3: myval", @@ -351,41 +352,41 @@ public class ConfigInheritanceYamlTest extends AbstractYamlTest { " brooklyn.config:", " shell.env:", " ENV2: myEnv2", - " ENV3: myEnv2", + " ENV3: myEnv3", " templates.preinstall:", - " "+emptyFile2.toUri()+": myfile2", - " "+emptyFile3.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", + " "+emptyFile3.getAbsolutePath()+": myfile3", " files.preinstall:", - " "+emptyFile2.toUri()+": myfile2", - " "+emptyFile3.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", + " "+emptyFile3.getAbsolutePath()+": myfile3", " templates.install:", - " "+emptyFile2.toUri()+": myfile2", - " "+emptyFile3.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", + " "+emptyFile3.getAbsolutePath()+": myfile3", " files.install:", - " "+emptyFile2.toUri()+": myfile2", - " "+emptyFile3.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", + " "+emptyFile3.getAbsolutePath()+": myfile3", " templates.runtime:", - " "+emptyFile2.toUri()+": myfile2", - " "+emptyFile3.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", + " "+emptyFile3.getAbsolutePath()+": myfile3", " files.runtime:", - " "+emptyFile2.toUri()+": myfile2", - " "+emptyFile3.toUri()+": myfile2", + " "+emptyFile2.getAbsolutePath()+": myfile2", + " "+emptyFile3.getAbsolutePath()+": myfile3", " provisioning.properties:", " mykey2: myval2", - " mykey3: myval2", + " mykey3: myval3", " templateOptions:", " myOptionsKey2: myOptionsVal2", - " myOptionsKey3: myOptionsVal2"); + " myOptionsKey3: myOptionsVal3"); Entity app = createStartWaitAndLogApplication(yaml); Entity entity = Iterables.getOnlyElement(app.getChildren()); assertEmptySoftwareProcessConfig( entity, - ImmutableMap.of("ENV", "myEnv", "ENV2", "myEnv2", "ENV3", "myEnv2"), - ImmutableMap.of(emptyFile.toUri().toString(), "myfile", emptyFile2.toUri().toString(), "myfile2", emptyFile3.toUri().toString(), "myfile2"), - ImmutableMap.of("mykey", "myval", "mykey2", "myval2", "mykey3", "myval2", - "templateOptions", ImmutableMap.of("myOptionsKey", "myOptionsVal", "myOptionsKey2", "myOptionsVal2", "myOptionsKey3", "myOptionsVal2"))); + ImmutableMap.of("ENV", "myEnv", "ENV2", "myEnv2", "ENV3", "myEnv3"), + ImmutableMap.of(emptyFile.getAbsolutePath(), "myfile", emptyFile2.getAbsolutePath(), "myfile2", emptyFile3.getAbsolutePath(), "myfile3"), + ImmutableMap.of("mykey", "myval", "mykey2", "myval2", "mykey3", "myval3", + "templateOptions", ImmutableMap.of("myOptionsKey", "myOptionsVal", "myOptionsKey2", "myOptionsVal2", "myOptionsKey3", "myOptionsVal3"))); } @Test http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java index f1ea8eb..a0f56ff 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java @@ -188,7 +188,7 @@ public class ObjectsYamlTest extends AbstractYamlTest { } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> predicate) { + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> predicate) { return MutableSet.copyOf(Iterables.filter(bag.getAllConfigAsConfigKeyMap().keySet(), predicate)); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/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 index 2ed7d48..51f2e7a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java @@ -20,6 +20,8 @@ package org.apache.brooklyn.core.config; import java.util.Map; +import javax.annotation.Nullable; + import org.apache.brooklyn.config.ConfigInheritance; import org.apache.brooklyn.config.ConfigInheritances; import org.apache.brooklyn.config.ConfigInheritances.BasicConfigValueAtContainer; @@ -56,43 +58,47 @@ public class BasicConfigInheritance implements ConfigInheritance { * (and often the descendant's value will simply overwrite). */ public static BasicConfigInheritance DEEP_MERGE = new BasicConfigInheritance(true,CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE,false); - // reinheritable? true/false; if false, children/descendants/inheritors will never see it; default true + /** 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 - // (overriding resolveConflict) - protected final String conflictResolutionStrategy; - // use-local-default-value? true/false; if true, overwrite above means "always ignore (even if null)"; default false - // whereas merge means "use local default (if non-null)" + /** conflict-resolution-strategy? in {@link BasicConfigInheritance} supported values are + * {@link #CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE} and {@link #CONFLICT_RESOLUTION_STRATEGY_OVERWRITE}. + * subclasses may pass null if they provide a custom implementaton of {@link #resolveWithParentCustomStrategy(ConfigValueAtContainer, ConfigValueAtContainer, org.apache.brooklyn.config.ConfigInheritance.ConfigInheritanceContext)} */ + @Nullable protected final String conflictResolutionStrategy; + /** use-local-default-value? true/false; if true, overwrite above means "always ignore (even if null)"; default false + * whereas merge means "use local default (if non-null)" */ protected final boolean useLocalDefaultValue; - protected BasicConfigInheritance(boolean isReinherited, String conflictResolutionStrategy, boolean useLocalDefaultValue) { + protected BasicConfigInheritance(boolean isReinherited, @Nullable String conflictResolutionStrategy, boolean useLocalDefaultValue) { super(); this.isReinherited = isReinherited; this.conflictResolutionStrategy = conflictResolutionStrategy; this.useLocalDefaultValue = useLocalDefaultValue; } - @SuppressWarnings("deprecation") - @Override + @Override @Deprecated public InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to) { return null; } + protected <TContainer, TValue> void checkInheritanceContext(ConfigValueAtContainer<TContainer, TValue> local, ConfigInheritanceContext context) { + ConfigInheritance rightInheritance = ConfigInheritances.findInheritance(local, context, this); + if (!equals(rightInheritance)) + throw new IllegalStateException("Low level inheritance computation error: caller should invoke on "+rightInheritance+" " + + "(the inheritance at "+local+"), not "+this); + } @Override public <TContainer, TValue> boolean isReinheritable(ConfigValueAtContainer<TContainer, TValue> parent, ConfigInheritanceContext context) { - if (!equals(ConfigInheritances.findInheritance(parent, context, this))) - throw new IllegalStateException("Method can only be invoked on inheritance at "+parent); + checkInheritanceContext(parent, context); return isReinherited(); } - + @Override public <TContainer,TValue> boolean considerParent( ConfigValueAtContainer<TContainer,TValue> local, ConfigValueAtContainer<TContainer,TValue> parent, ConfigInheritanceContext context) { - if (!equals(ConfigInheritances.findInheritance(local, context, this))) - throw new IllegalStateException("Method can only be invoked on inheritance at "+local); + checkInheritanceContext(local, context); if (parent==null) return false; if (CONFLICT_RESOLUTION_STRATEGY_OVERWRITE.equals(conflictResolutionStrategy)) { // overwrite means ignore if there's an explicit value, or we're using the local default @@ -107,6 +113,8 @@ public class BasicConfigInheritance implements ConfigInheritance { ConfigValueAtContainer<TContainer,TValue> parent, ConfigInheritanceContext context) { + checkInheritanceContext(local, context); + if (!parent.isValueExplicitlySet() && !getUseLocalDefaultValue()) return ReferenceWithError.newInstanceWithoutError(new BasicConfigValueAtContainer<TContainer,TValue>(local)); @@ -115,12 +123,13 @@ public class BasicConfigInheritance implements ConfigInheritance { if (!local.isValueExplicitlySet() && !getUseLocalDefaultValue()) return ReferenceWithError.newInstanceWithoutError(new BasicConfigValueAtContainer<TContainer,TValue>(parent)); - // both explicitly set or defaults applicable, and not overwriting; it should be merge + // both explicitly set or defaults applicable, and not overwriting; it should be merge, or something from child + if (CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE.equals(conflictResolutionStrategy)) { BasicConfigValueAtContainer<TContainer, TValue> result = new BasicConfigValueAtContainer<TContainer,TValue>(local); ReferenceWithError<Maybe<? extends TValue>> resolvedValue = deepMerge( local.isValueExplicitlySet() ? local.asMaybe() : local.getDefaultValue(), - parent.isValueExplicitlySet() ? parent.asMaybe() : parent.getDefaultValue()); + parent.isValueExplicitlySet() ? parent.asMaybe() : parent.getDefaultValue()); result.setValue(resolvedValue.getWithoutError()); return ReferenceWithError.newInstanceThrowingError(result, resolvedValue.getError()); } @@ -128,6 +137,8 @@ public class BasicConfigInheritance implements ConfigInheritance { return resolveWithParentCustomStrategy(local, parent, context); } + /** Provided for subclasses to override. Invoked by {@link #resolveWithParent(ConfigValueAtContainer, ConfigValueAtContainer, org.apache.brooklyn.config.ConfigInheritance.ConfigInheritanceContext)} + * if there is a value for the parent and the child. Default implementation throws {@link IllegalStateException}. */ protected <TContainer, TValue> ReferenceWithError<ConfigValueAtContainer<TContainer, TValue>> resolveWithParentCustomStrategy( ConfigValueAtContainer<TContainer, TValue> local, ConfigValueAtContainer<TContainer, TValue> parent, ConfigInheritanceContext context) { @@ -162,5 +173,9 @@ public class BasicConfigInheritance implements ConfigInheritance { public boolean getUseLocalDefaultValue() { return useLocalDefaultValue; } - + + @Override + public String toString() { + return super.toString()+"[reinherit="+isReinherited()+"; strategy="+getConflictResolutionStrategy()+"; useLocal="+getUseLocalDefaultValue()+"]"; + } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java index 6359907..cbc8658 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigKeys.java @@ -47,8 +47,7 @@ public class ConfigKeys { private static final Logger log = LoggerFactory.getLogger(ConfigKeys.class); - public enum InheritanceContext implements ConfigInheritanceContext - { TYPE_DEFINITION, RUNTIME_MANAGEMENT } + public enum InheritanceContext implements ConfigInheritanceContext { TYPE_DEFINITION, RUNTIME_MANAGEMENT } public static <T> ConfigKey<T> newConfigKey(Class<T> type, String name) { return new BasicConfigKey<T>(type, name); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java index e964d24..8716cec 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.core.config.internal; +import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -68,15 +69,19 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i private static final Logger LOG = LoggerFactory.getLogger(AbstractConfigMapImpl.class); - @SuppressWarnings("deprecation") + @Deprecated /** @deprecated since 0.10.0 - see method which uses it */ protected final transient org.apache.brooklyn.core.entity.internal.ConfigMapViewWithStringKeys mapViewWithStringKeys = new org.apache.brooklyn.core.entity.internal.ConfigMapViewWithStringKeys(this); + // TODO make final when not working with previously serialized instances + // (we shouldn't be, but just in case!) protected TContainer bo; /** * Map of configuration information that is defined at start-up time for the entity. These * configuration parameters are shared and made accessible to the "children" of this * entity. + * + * All iterator accesses (eg copying) should be synchronized. See {@link #putAllOwnConfigIntoSafely(Map)}. */ protected final Map<ConfigKey<?>,Object> ownConfig; @@ -117,9 +122,20 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i @Override public Map<ConfigKey<?>,Object> getAllConfigLocalRaw() { Map<ConfigKey<?>,Object> result = new LinkedHashMap<ConfigKey<?>,Object>(); - result.putAll(ownConfig); + putAllOwnConfigIntoSafely(result); return Collections.unmodifiableMap(result); } + protected Map<ConfigKey<?>, Object> putAllOwnConfigIntoSafely(Map<ConfigKey<?>, Object> result) { + synchronized (ownConfig) { + result.putAll(ownConfig); + } + return result; + } + protected ConfigBag putAllOwnConfigIntoSafely(ConfigBag bag) { + synchronized (ownConfig) { + return bag.putAll(ownConfig); + } + } /** an immutable copy of the config visible at this entity, local and inherited (preferring local) */ @Override @Deprecated @@ -127,14 +143,14 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i Map<ConfigKey<?>,Object> result = new LinkedHashMap<ConfigKey<?>,Object>(); if (getParent()!=null) result.putAll( getParentInternal().config().getInternalConfigMap().getAllConfig() ); - result.putAll(ownConfig); + putAllOwnConfigIntoSafely(result); return Collections.unmodifiableMap(result); } /** Creates an immutable copy of the config visible at this entity, local and inherited (preferring local), including those that did not match config keys */ @Deprecated public ConfigBag getAllConfigBag() { - ConfigBag result = ConfigBag.newInstance().putAll(ownConfig); + ConfigBag result = putAllOwnConfigIntoSafely(ConfigBag.newInstance()); if (getParent()!=null) { result.putIfAbsent( ((AbstractConfigMapImpl<?>)getParentInternal().config().getInternalConfigMap()).getAllConfigBag() ); @@ -144,7 +160,7 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i /** As {@link #getLocalConfigRaw()} but in a {@link ConfigBag} for convenience */ public ConfigBag getLocalConfigBag() { - return ConfigBag.newInstance().putAll(ownConfig).seal(); + return putAllOwnConfigIntoSafely(ConfigBag.newInstance()).seal(); } public Object setConfig(ConfigKey<?> key, Object v) { @@ -170,10 +186,6 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i @SuppressWarnings("unchecked") public void putAll(Map<?,?> vals) { -// ConfigBag ownConfigBag = ConfigBag.newInstance().putAll(vals); -// ownConfig.putAll(ownConfigBag.getAllConfigAsConfigKeyMap()); - // below seems more straightforward; should be the same. - // potential problem if clash of config key types? for (Map.Entry<?, ?> entry : vals.entrySet()) { if (entry.getKey()==null) throw new IllegalArgumentException("Cannot put null key into "+this); @@ -257,12 +269,14 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i 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") @@ -271,10 +285,17 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i } @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) { @@ -359,6 +380,7 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i } } + @Override public List<ConfigValueAtContainer<TContainer,?>> getConfigAllInheritedRaw(ConfigKey<?> queryKey) { List<ConfigValueAtContainer<TContainer, ?>> result = MutableList.of(); TContainer c = getContainer(); @@ -375,20 +397,24 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i 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; + 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; } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> filter) { + public Set<ConfigKey<?>> findKeys(Predicate<? super 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 @@ -398,6 +424,7 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i return result; } + @Override @SuppressWarnings("unchecked") public ReferenceWithError<ConfigValueAtContainer<TContainer,?>> getConfigInheritedRaw(ConfigKey<?> key) { return (ReferenceWithError<ConfigValueAtContainer<TContainer,?>>) (ReferenceWithError<?>) getConfigImpl(key, true); @@ -414,19 +441,27 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i } @Override public Map<ConfigKey<?>, ReferenceWithError<ConfigValueAtContainer<TContainer, ?>>> getAllConfigInheritedRawWithErrors() { - return getSelectedConfigInheritedRaw(false); + return getSelectedConfigInheritedRaw(null, false); } + @Override public Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>> getAllReinheritableConfigRaw() { - return getSelectedConfigInheritedRaw(true); + return getSelectedConfigInheritedRaw(null, true); } - protected Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>> getSelectedConfigInheritedRaw(boolean onlyReinheritable) { + protected Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>> getSelectedConfigInheritedRaw(Map<ConfigKey<?>,ConfigKey<?>> knownKeys, boolean onlyReinheritable) { + Map<ConfigKey<?>, ConfigKey<?>> knownKeysOnType = MutableMap.of(); + for (ConfigKey<?> k: getKeysAtContainer(getContainer())) knownKeysOnType.put(k, k); + + Map<ConfigKey<?>, ConfigKey<?>> knownKeysIncludingDescendants = MutableMap.copyOf(knownKeys); + knownKeysIncludingDescendants.putAll(knownKeysOnType); + 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(); + ((AbstractConfigMapImpl<?>)getParentInternal().config().getInternalConfigMap()) + .getSelectedConfigInheritedRaw(knownKeysIncludingDescendants, true); parents.putAll(po); } @@ -434,34 +469,48 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i 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); + for (ConfigKey<?> kSet: MutableSet.copyOf(local.keySet()).putAll(parents.keySet())) { + Maybe<Object> localValue = local.containsKey(kSet) ? Maybe.ofAllowingNull(local.get(kSet)) : Maybe.absent(); + ReferenceWithError<ConfigValueAtContainer<TContainer, ?>> vpr = parents.remove(kSet); + @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)); + + @Nullable ConfigKey<?> kOnType = knownKeysOnType.get(kSet); + @Nullable ConfigKey<?> kTypeOrDescendant = knownKeysIncludingDescendants.get(kSet); + assert kOnType==null || kOnType==kTypeOrDescendant; + + // if no key on type, we must use any descendant declared key here + // so that the correct descendant conflict resolution strategy is applied + ConfigInheritance inhHereOrDesc = ConfigInheritances.findInheritance(kTypeOrDescendant, InheritanceContext.RUNTIME_MANAGEMENT, getDefaultRuntimeInheritance()); + + // however for the purpose of qualifying we must not give any key except what is exactly declared here, + // else reinheritance will be incorrectly deduced + ConfigValueAtContainer<TContainer,Object> vl = new BasicConfigValueAtContainer<TContainer,Object>(getContainer(), kOnType, localValue); + ReferenceWithError<ConfigValueAtContainer<TContainer, Object>> vlr = null; - if (inh.considerParent(vl, vp, InheritanceContext.RUNTIME_MANAGEMENT)) { - vlr = inh.resolveWithParent(vl, vp, InheritanceContext.RUNTIME_MANAGEMENT); + if (inhHereOrDesc.considerParent(vl, vp, InheritanceContext.RUNTIME_MANAGEMENT)) { + vlr = inhHereOrDesc.resolveWithParent(vl, vp, InheritanceContext.RUNTIME_MANAGEMENT); } else { // no need to consider parent, just take vl + if (!vl.isValueExplicitlySet()) { + // inherited parent value NEVER_INHERIT ie overwritten by default value or null here + continue; + } vlr = ReferenceWithError.newInstanceWithoutError(vl); } if (onlyReinheritable) { - if (ConfigInheritances.findInheritance(k, InheritanceContext.RUNTIME_MANAGEMENT, getDefaultRuntimeInheritance()) - .isReinheritable(vl, InheritanceContext.RUNTIME_MANAGEMENT)) { - // continue - } else { + ConfigInheritance inhHere = ConfigInheritances.findInheritance(kOnType, InheritanceContext.RUNTIME_MANAGEMENT, getDefaultRuntimeInheritance()); + if (localValue.isAbsent() && !inhHere.isReinheritable(vl, InheritanceContext.RUNTIME_MANAGEMENT)) { // skip this one continue; } } @SuppressWarnings("unchecked") ReferenceWithError<ConfigValueAtContainer<TContainer, ?>> vlro = (ReferenceWithError<ConfigValueAtContainer<TContainer, ?>>) (ReferenceWithError<?>) vlr; - result.put(k, vlro); + result.put(kSet, vlro); } - result.putAll(parents); + assert parents.isEmpty(); return result; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/config/internal/LazyContainerAndKeyValue.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/LazyContainerAndKeyValue.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/LazyContainerAndKeyValue.java index fec13af..f72374e 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/internal/LazyContainerAndKeyValue.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/LazyContainerAndKeyValue.java @@ -18,27 +18,30 @@ */ package org.apache.brooklyn.core.config.internal; +import javax.annotation.Nullable; + import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigValueAtContainer; import org.apache.brooklyn.util.guava.Maybe; import com.google.common.base.Function; +import com.google.common.base.Preconditions; public class LazyContainerAndKeyValue<TContainer,TValue> implements ConfigValueAtContainer<TContainer,TValue> { - private final TContainer container; - private final ConfigKey<TValue> key; + @Nullable private final TContainer container; + @Nullable private final ConfigKey<TValue> key; private final Function<TContainer,Maybe<Object>> lookupResolutionFunction; private final Function<Maybe<Object>,Maybe<TValue>> conversionFunction; private Maybe<TValue> resolved; - public LazyContainerAndKeyValue(ConfigKey<TValue> key, TContainer container, + public LazyContainerAndKeyValue(@Nullable ConfigKey<TValue> key, @Nullable TContainer container, Function<TContainer, Maybe<Object>> lookupResolutionFunction, Function<Maybe<Object>, Maybe<TValue>> conversionFunction) { this.key = key; this.container = container; - this.lookupResolutionFunction = lookupResolutionFunction; - this.conversionFunction = conversionFunction; + this.lookupResolutionFunction = Preconditions.checkNotNull(lookupResolutionFunction); + this.conversionFunction = Preconditions.checkNotNull(conversionFunction); } protected synchronized Maybe<TValue> resolve() { @@ -81,4 +84,10 @@ public class LazyContainerAndKeyValue<TContainer,TValue> implements ConfigValueA if (key==null || !key.hasDefaultValue()) return Maybe.absent(); return conversionFunction.apply(Maybe.of((Object)key.getDefaultValue())); } + + @Override + public String toString() { + return super.toString()+"[key="+key+"; container="+container+"]"; + } + } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/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 400694e..480e1ed 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 @@ -21,14 +21,17 @@ package org.apache.brooklyn.core.entity.internal; import static com.google.common.base.Preconditions.checkNotNull; import java.util.Map; +import java.util.Set; 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.api.objs.BrooklynObject; +import org.apache.brooklyn.api.objs.EntityAdjunct; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.entity.EntityInternal; +import org.apache.brooklyn.core.objs.AbstractEntityAdjunct; import org.apache.brooklyn.core.objs.BrooklynObjectInternal.ConfigurationSupportInternal; import org.apache.brooklyn.util.guava.Maybe; import org.slf4j.Logger; @@ -103,6 +106,11 @@ public class EntityConfigMap extends AbstractConfigMapImpl<Entity> { return container.getEntityType().getConfigKey(queryKey.getName()); } + @Override + protected Set<ConfigKey<?>> getKeysAtContainer(Entity container) { + return container.getEntityType().getConfigKeys(); + } + @Override @Deprecated public EntityConfigMap submap(Predicate<ConfigKey<?>> filter) { EntityConfigMap m = new EntityConfigMap(getEntity(), Maps.<ConfigKey<?>, Object>newLinkedHashMap()); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynProperties.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynProperties.java b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynProperties.java index 1bed084..1cfbcbc 100644 --- a/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynProperties.java +++ b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynProperties.java @@ -311,11 +311,13 @@ public interface BrooklynProperties extends Map, StringConfigMap { public Maybe<Object> getConfigRaw(ConfigKey<?> key); - /** Inheritance is ignored here. Preferable to use {@link #getConfigRaw(ConfigKey)}. */ + /** Inheritance is ignored here. Preferable to use {@link #getConfigRaw(ConfigKey)}. + * @deprecated since 0.10.0 the second parameter is ignored for {@link BrooklynProperties}; use {@link #getConfigLocalRaw(ConfigKey)}. */ + @Deprecated // and confirmed no usages apart from internal @Override public Maybe<Object> getConfigRaw(ConfigKey<?> key, boolean includeInherited); - @Override + @Override @Deprecated public Map<ConfigKey<?>, Object> getAllConfig(); @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java index 03fb13d..89cb7d2 100644 --- a/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java @@ -471,7 +471,7 @@ public class BrooklynPropertiesImpl extends LinkedHashMap implements BrooklynPro } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> filter) { + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { Set<ConfigKey<?>> result = new LinkedHashSet<ConfigKey<?>>(); for (Object entry: entrySet()) { ConfigKey<?> k = new BasicConfigKey<Object>(Object.class, ""+((Map.Entry)entry).getKey()); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java index 076443a..9b38946 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.core.location.internal; +import java.util.Collection; import java.util.Map; import javax.annotation.Nonnull; @@ -94,6 +95,11 @@ public class LocationConfigMap extends AbstractConfigMapImpl<Location> { return ((AbstractLocation)container).getLocationTypeInternal().getConfigKey(queryKey.getName()); } + @Override + protected Collection<ConfigKey<?>> getKeysAtContainer(Location container) { + return ((AbstractLocation)container).getLocationTypeInternal().getConfigKeys().values(); + } + } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java index 69f2423..1bc737e 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java @@ -131,7 +131,7 @@ public class DeferredBrooklynProperties implements BrooklynProperties { return getConfigRaw(key, true); } - @Override + @Override @Deprecated public Maybe<Object> getConfigRaw(ConfigKey<?> key, boolean includeInherited) { Maybe<Object> result = delegate.getConfigRaw(key, includeInherited); return (result.isPresent()) ? Maybe.of(transform(key, result.get())) : Maybe.absent(); @@ -203,7 +203,7 @@ public class DeferredBrooklynProperties implements BrooklynProperties { return new DeferredBrooklynProperties(submap, mgmt); } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> filter) { + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { return delegate.findKeys(filter); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java index 402abb8..1b87d3c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java @@ -230,7 +230,7 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> filter) { + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { return getConfigsInternal().findKeys(filter); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java index fc13cd8..602d943 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java @@ -19,6 +19,7 @@ package org.apache.brooklyn.core.objs; import java.util.Map; +import java.util.Set; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.ExecutionContext; @@ -91,5 +92,10 @@ public class AdjunctConfigMap extends AbstractConfigMapImpl<EntityAdjunct> { protected <T> ConfigKey<?> getKeyAtContainerImpl(EntityAdjunct container, ConfigKey<T> queryKey) { return ((AbstractEntityAdjunct)container).getAdjunctType().getConfigKey(queryKey.getName()); } + + @Override + protected Set<ConfigKey<?>> getKeysAtContainer(EntityAdjunct container) { + return ((AbstractEntityAdjunct)container).getAdjunctType().getConfigKeys(); + } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java index d3e29b6..0e9bd87 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java @@ -123,7 +123,7 @@ public class BasicConfigurableObject implements Configurable, Identifiable, Mana } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> predicate) { + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> predicate) { return MutableSet.copyOf(Iterables.filter(config.getAllConfigAsConfigKeyMap().keySet(), predicate)); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java index d6a2f9f..ed44096 100644 --- a/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/entity/ConfigEntityInheritanceTest.java @@ -18,9 +18,14 @@ */ package org.apache.brooklyn.core.entity; +import java.util.List; +import java.util.Map; + import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.config.ConfigMap.ConfigMapWithInheritance; +import org.apache.brooklyn.config.ConfigValueAtContainer; import org.apache.brooklyn.core.config.BasicConfigInheritance; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.EntityConfigTest.MyOtherEntity; @@ -28,9 +33,14 @@ import org.apache.brooklyn.core.entity.EntityConfigTest.MyOtherEntityImpl; import org.apache.brooklyn.core.sensor.AttributeSensorAndConfigKey; import org.apache.brooklyn.core.sensor.BasicAttributeSensorAndConfigKey.IntegerAttributeSensorAndConfigKey; import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.collections.MutableSet; import org.testng.Assert; import org.testng.annotations.Test; +import com.google.common.collect.Iterables; + public class ConfigEntityInheritanceTest extends BrooklynAppUnitTestSupport { protected void checkKeys(Entity entity2, Integer value) { @@ -144,14 +154,23 @@ public class ConfigEntityInheritanceTest extends BrooklynAppUnitTestSupport { } // -------------------- + + @Override + protected Boolean shouldSkipOnBoxBaseDirResolution() { + return null; //because it adds extra config we do not want to test for + } - @Test - public void testConfigKeysInheritance() throws Exception { + protected Entity setupBasicInheritanceTest() { app.config().set(MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT, "heritable"); app.config().set(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE, "always_heritable"); app.config().set(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT, "uninheritable"); app.config().set(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE, "maybe"); - Entity child = app.addChild(EntitySpec.create(MyEntityWithPartiallyHeritableConfig.class)); + return app.addChild(EntitySpec.create(MyEntityWithPartiallyHeritableConfig.class)); + } + + @Test + public void testConfigKeysInheritance() throws Exception { + Entity child = setupBasicInheritanceTest(); Assert.assertNotNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT)); Assert.assertNotNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE)); @@ -163,6 +182,50 @@ public class ConfigEntityInheritanceTest extends BrooklynAppUnitTestSupport { Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE)); } + @SuppressWarnings("unchecked") + @Test + public void testConfigKeysAllInheritanceMethods() throws Exception { + Entity child = setupBasicInheritanceTest(); + + Assert.assertEquals( ((EntityInternal)child).config().getInternalConfigMap().getAllConfigLocalRaw(), MutableMap.of() ); + + Assert.assertEquals( ((EntityInternal)child).config().getInternalConfigMap().getAllConfigInheritedRawValuesIgnoringErrors(), + MutableMap.of().add(MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT, "heritable") + .add(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE, "always_heritable") + .add(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE, "maybe") ); + + Map<ConfigKey<?>, ?> rawReinherit = ((EntityInternal)child).config().getInternalConfigMap().getAllReinheritableConfigRaw(); + Assert.assertEquals(rawReinherit.keySet(), MutableSet.of( + MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT, + MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE)); + + + // check the hierarchical list of values is correct + + List<ConfigValueAtContainer<Entity, ?>> rawHierarchy = ((ConfigMapWithInheritance<Entity>)((EntityInternal)child).config().getInternalConfigMap()). + getConfigAllInheritedRaw(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE); + Assert.assertEquals(rawHierarchy.size(), 1); + Assert.assertEquals(Iterables.getOnlyElement(rawHierarchy).getContainer(), app); + + rawHierarchy = ((ConfigMapWithInheritance<Entity>)((EntityInternal)child).config().getInternalConfigMap()). + getConfigAllInheritedRaw(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT); + Assert.assertEquals(rawHierarchy, MutableList.of()); + + + // and try with and without NOT_RE declared at the app + + rawHierarchy = ((ConfigMapWithInheritance<Entity>)((EntityInternal)child).config().getInternalConfigMap()). + getConfigAllInheritedRaw(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE); + Assert.assertEquals(rawHierarchy.size(), 1); + Assert.assertEquals(Iterables.getOnlyElement(rawHierarchy).getContainer(), app); + + app.getMutableEntityType().addConfigKey(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE); + + rawHierarchy = ((ConfigMapWithInheritance<Entity>)((EntityInternal)child).config().getInternalConfigMap()). + getConfigAllInheritedRaw(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE); + Assert.assertEquals(rawHierarchy, MutableList.of()); + } + public static class MyEntityWithPartiallyHeritableConfig extends AbstractEntity { public static final ConfigKey<String> HERITABLE_BY_DEFAULT = ConfigKeys.builder(String.class, "herit.default").build(); public static final ConfigKey<String> NEVER_INHERIT = ConfigKeys.builder(String.class, "herit.never").runtimeInheritance(BasicConfigInheritance.NEVER_INHERITED).build(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java index cbe39d0..6a79a0f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java +++ b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java @@ -38,13 +38,15 @@ public class BrooklynAppUnitTestSupport extends BrooklynMgmtUnitTestSupport { setUpApp(); } - protected boolean shouldSkipOnBoxBaseDirResolution() { + /** set null not to set, or a boolean to set explicitly; default true */ + protected Boolean shouldSkipOnBoxBaseDirResolution() { return true; } protected void setUpApp() { - EntitySpec<TestApplication> appSpec = EntitySpec.create(TestApplication.class) - .configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, shouldSkipOnBoxBaseDirResolution()); + EntitySpec<TestApplication> appSpec = EntitySpec.create(TestApplication.class); + if (shouldSkipOnBoxBaseDirResolution()!=null) + appSpec.configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, shouldSkipOnBoxBaseDirResolution()); app = mgmt.getEntityManager().createEntity(appSpec); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java index 792b441..9180183 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java @@ -407,7 +407,7 @@ public class FlagUtilsTest { } @Override - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> predicate) { + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> predicate) { return MutableSet.copyOf(Iterables.filter(bag.getAllConfigAsConfigKeyMap().keySet(), predicate)); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/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 dffa548..5b96a7b 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 @@ -54,6 +54,7 @@ public interface ConfigInheritance extends Serializable { /** @deprecated since 0.10.0 see implementations of this interface */ @Deprecated public static final ConfigInheritance DEEP_MERGE = new Legacy.Merged(); + /** @deprecated since 0.10.0 more complex inheritance conditions now require other methods */ @Deprecated InheritanceMode isInherited(ConfigKey<?> key, Object from, Object to); @@ -65,7 +66,8 @@ public interface ConfigInheritance extends Serializable { * If there is a {@link ConfigInheritance} defined at this node, * this method must be called on that instance and that instance only. * In that case it is an error to invoke this method on any other {@link ConfigInheritance} instance. - * If there is not one, the config generally should be considered reinheritable. + * If there is not one, the config generally should be considered reinheritable; + * callers will typically not invoke this method from a descendant inheritance context. * <p> * Consumers will typically find the methods in {@link ConfigInheritances} more convenient. */ public <TContainer,TValue> boolean isReinheritable( http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritances.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritances.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritances.java index 95d80ff..bac207d 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritances.java +++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigInheritances.java @@ -20,6 +20,7 @@ package org.apache.brooklyn.config; import java.util.Iterator; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.brooklyn.config.ConfigInheritance.ConfigInheritanceContext; @@ -116,28 +117,38 @@ public class ConfigInheritances { public static class BasicConfigValueAtContainer<TContainer,TValue> implements ConfigValueAtContainer<TContainer,TValue> { - TContainer container = null; - Maybe<? extends TValue> value = Maybe.absent(); + @Nullable TContainer container = null; + @Nonnull Maybe<? extends TValue> value = Maybe.absent(); boolean valueWasExplicitlySet = false; - ConfigKey<? extends TValue> key = null; - Maybe<TValue> defaultValue = null; + @Nullable ConfigKey<? extends TValue> key = null; + @Nullable Maybe<TValue> defaultValue = null; public BasicConfigValueAtContainer() {} public BasicConfigValueAtContainer(ConfigValueAtContainer<TContainer,TValue> toCopy) { this(toCopy.getContainer(), toCopy.getKey(), toCopy.asMaybe(), toCopy.isValueExplicitlySet(), toCopy.getDefaultValue()); } - public BasicConfigValueAtContainer(TContainer container, ConfigKey<? extends TValue> key, Maybe<? extends TValue> value) { + public BasicConfigValueAtContainer(@Nullable TContainer container, @Nullable ConfigKey<? extends TValue> key, + @Nullable Maybe<? extends TValue> value) { this(container, key, value, value.isPresent()); } - public BasicConfigValueAtContainer(TContainer container, ConfigKey<? extends TValue> key, Maybe<? extends TValue> value, boolean isValueSet) { + public BasicConfigValueAtContainer(@Nullable TContainer container, @Nullable ConfigKey<? extends TValue> key, @Nullable Maybe<? extends TValue> value, boolean isValueSet) { this(container, key, value, isValueSet, null); } - public BasicConfigValueAtContainer(TContainer container, ConfigKey<? extends TValue> key, Maybe<? extends TValue> value, boolean isValueSet, Maybe<TValue> defaultValue) { + /** Creates an instance, configuring all parameters. + * + * @param container May be null as per contract. + * @param key May be null as per contract. + * @param value Null means always to take {@link #getDefaultValue()}; if absent and isValueSet is false, it will also take {@link #getDefaultValue()}. + * @param isValueSet + * @param defaultValue Null means to take a default value from the key ({@link #getKey()}), otherwise this {@link Maybe} will be preferred to that value + * (even if absent). + */ + public BasicConfigValueAtContainer(@Nullable TContainer container, @Nullable ConfigKey<? extends TValue> key, @Nullable Maybe<? extends TValue> value, boolean isValueSet, @Nullable Maybe<TValue> defaultValue) { this.container = container; this.key = key; this.valueWasExplicitlySet = isValueSet; this.defaultValue = defaultValue; - this.value = value!=null && (value.isPresent() || isValueSet || key==null || !key.hasDefaultValue()) ? value + this.value = value!=null && (value.isPresent() || isValueSet || getDefaultValue().isPresent()) ? value : getDefaultValue(); } @@ -166,6 +177,10 @@ public class ConfigInheritances { return key!=null && key.hasDefaultValue() ? Maybe.ofAllowingNull((TValue) key.getDefaultValue()) : Maybe.<TValue>absent(); } + @Override + public String toString() { + return super.toString()+"[key="+key+"; value="+value+"; container="+container+"]"; + } } /** determine whether a key is reinheritable from the point in the given inheritance hierarchy where it is introduced; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java index 7b0b576..dd805d1 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java +++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java @@ -81,7 +81,7 @@ public interface ConfigMap { public ConfigMap submap(Predicate<ConfigKey<?>> filter); /** returns all keys matching the given filter predicate; see ConfigPredicates for common predicates */ - public Set<ConfigKey<?>> findKeys(Predicate<ConfigKey<?>> filter); + public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter); /** returns a read-only map view which has string keys (corresponding to the config key names); * callers encouraged to use the typed keys (and so not use this method), http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8156322b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigValueAtContainer.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigValueAtContainer.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigValueAtContainer.java index a4c60ca..59168a3 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigValueAtContainer.java +++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigValueAtContainer.java @@ -18,6 +18,9 @@ */ package org.apache.brooklyn.config; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import org.apache.brooklyn.util.guava.Maybe; import com.google.common.base.Supplier; @@ -25,19 +28,24 @@ import com.google.common.base.Supplier; public interface ConfigValueAtContainer<TContainer,TValue> extends Supplier<TValue> { /** Returns the value for this key, or null. Technically this returns {@link #asMaybe()} or null. */ - TValue get(); + @Nullable TValue get(); /** Absent if no value can be found, typically meaning no default value, but in raw value lookups it may ignore default values. */ - Maybe<? extends TValue> asMaybe(); + @Nonnull Maybe<? extends TValue> asMaybe(); /** If false, any contents of {@link #get()} will have come from {@link #getDefaultValue()}. */ boolean isValueExplicitlySet(); - /** The container where the value was found (possibly an ancestor of the queried object) */ - TContainer getContainer(); + /** The container where the value was found (possibly an ancestor of the queried object). + * This may be null in internal uses to indicate an anonymous query. */ + @Nullable TContainer getContainer(); - ConfigKey<? extends TValue> getKey(); + /** The key whose value is being held here. + * <p> + * This may be null when working in a context where the query key is widely known + * to indicate that no key was defined at this container. */ + @Nullable ConfigKey<? extends TValue> getKey(); /** The default value on the key, if available and permitted, * possibly coerced or resolved in the scope of {@link #getContainer()}, * and possibly absent e.g. in raw value lookups */ - Maybe<TValue> getDefaultValue(); + @Nonnull Maybe<TValue> getDefaultValue(); }
