Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/173#discussion_r65543515
--- Diff:
core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java
---
@@ -111,47 +110,107 @@ public EntityConfigMap(AbstractEntity entity,
Map<ConfigKey<?>, Object> storage)
// 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);
- T result = null;
- boolean complete = false;
- if (((ConfigKeySelfExtracting<T>)ownKey).isSet(ownConfig)) {
- ExecutionContext exec = entity.getExecutionContext();
- result =
((ConfigKeySelfExtracting<T>)ownKey).extractValue(ownConfig, exec);
- complete = true;
- } else if (isInherited(ownKey, inheritance) &&
-
((ConfigKeySelfExtracting<T>)ownKey).isSet(inheritedConfig)) {
- ExecutionContext exec = entity.getExecutionContext();
- result =
((ConfigKeySelfExtracting<T>)ownKey).extractValue(inheritedConfig, exec);
- complete = true;
- } else if (localConfigBag.containsKey(ownKey)) {
- // TODO configBag.get doesn't handle
tasks/attributeWhenReady - it only uses TypeCoercions
- result = localConfigBag.get(ownKey);
- complete = true;
- } else if (isInherited(ownKey, inheritance) &&
- inheritedConfigBag.containsKey(ownKey)) {
- result = inheritedConfigBag.get(ownKey);
- complete = true;
- }
+ Maybe<T> result =
getConfigImpl((ConfigKeySelfExtracting<T>)ownKey, parentInheritanceMode);
if (rawval instanceof Task) {
entity.getManagementSupport().getEntityChangeListener().onConfigChanged(key);
}
- if (complete) {
- return result;
+ if (result.isPresent()) {
+ return result.get();
}
} else {
LOG.warn("Config key {} of {} is not a
ConfigKeySelfExtracting; cannot retrieve value; returning default", ownKey,
this);
}
return TypeCoercions.coerce((defaultValue != null) ? defaultValue
: ownKey.getDefaultValue(), key.getTypeToken());
}
+
+ @SuppressWarnings("unchecked")
+ private <T> Maybe<T> getConfigImpl(ConfigKeySelfExtracting<T> key,
InheritanceMode parentInheritance) {
+ ExecutionContext exec = entity.getExecutionContext();
+ Maybe<T> ownValue;
+ Maybe<T> parentValue;
+
+ // Get own value
+ if (((ConfigKeySelfExtracting<T>)key).isSet(ownConfig)) {
+ ownValue =
Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(ownConfig, exec));
+ } else if (localConfigBag.containsKey(key)) {
+ // TODO configBag.get doesn't handle tasks/attributeWhenReady
- it only uses TypeCoercions
+ // Precedence ordering has changed; previously we'd prefer an
explicit isSet(inheritedConfig)
+ // over the localConfigBag.get(key).
+ ownValue = Maybe.of(localConfigBag.get(key));
+ } else {
+ ownValue = Maybe.<T>absent();
+ }
+
+ // 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();
+ }
+ break;
+ case 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();
+ }
+ 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 MERGE:
+ return (Maybe<T>) deepMerge(ownValue, parentValue, key);
+ case NONE:
+ return ownValue;
+ default:
+ throw new IllegalStateException("Unsupported
parent-inheritance mode for "+key.getName()+": "+parentInheritance);
+ }
+ }
+
+ 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()));
--- End diff --
* +1 for `DEEP_MERGE` - better make it explicit
* I didn't realize lists are concatenated - default behaviour in
`CollectionMerger` is not to concatenate them and don't see where this is
explicitly enabled?
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
