This is an automated email from the ASF dual-hosted git repository.
heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit a85eff4fac6396e12935fbed718dc9ff2b100dc1
Author: Alex Heneveld <a...@cloudsoft.io>
AuthorDate: Tue Apr 9 10:47:27 2024 +0100
ensure keys saved as value only are if specified as NEVER_INHERITED are not
inherited
even if the type is removed from the entity
---
.../core/config/BasicConfigInheritance.java | 2 +-
.../config/internal/AbstractConfigMapImpl.java | 16 ++++-
.../core/entity/internal/EntityConfigMap.java | 20 +++++-
.../core/entity/ConfigEntityInheritanceTest.java | 71 +++++++++++++++++++++-
4 files changed, 101 insertions(+), 8 deletions(-)
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 943c9b1839..e2bd4a20a1 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
@@ -127,7 +127,7 @@ public class BasicConfigInheritance implements
ConfigInheritance {
@Override protected ConfigInheritance getDelegate() { return DELEGATE;
}
}
/** Indicates that a key's value should never be inherited, even if
inherited from a value set on a container that does not know the key.
- * (Most usages will prefer {@link #NOT_REINHERITED}.) */
+ * (Many usages will prefer {@link #NOT_REINHERITED}, to allow unknown
dynamic keys set in an ancestor accessible from descendants.) */
public static final ConfigInheritance NEVER_INHERITED = new
NeverInherited();
/** In case we deserialize an old copy; the last arg (ancestor inherit
default) is irrelevant
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 5d69a4bca6..80ba2f8c59 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
@@ -27,6 +27,7 @@ import org.apache.brooklyn.api.mgmt.ExecutionContext;
import org.apache.brooklyn.api.mgmt.TaskFactory;
import org.apache.brooklyn.api.objs.BrooklynObject;
import org.apache.brooklyn.config.ConfigInheritance;
+import org.apache.brooklyn.config.ConfigInheritance.ConfigInheritanceContext;
import org.apache.brooklyn.config.ConfigInheritances;
import
org.apache.brooklyn.config.ConfigInheritances.BasicConfigValueAtContainer;
import org.apache.brooklyn.config.ConfigKey;
@@ -613,11 +614,12 @@ public abstract class AbstractConfigMapImpl<TContainer
extends BrooklynObject> i
return getSelectedConfigInheritedRaw(null, true);
}
- protected
Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
getSelectedConfigInheritedRaw(Map<ConfigKey<?>,ConfigKey<?>> knownKeys, boolean
onlyReinheritable) {
+ protected
Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
getSelectedConfigInheritedRaw(Map<ConfigKey<?>,ConfigKey<?>>
knownKeysAtDescendants,
+ /* if true, only returns keys which are intended for inheritance
by our descendants */ boolean onlyReinheritable) {
Map<ConfigKey<?>, ConfigKey<?>> knownKeysOnType = MutableMap.of();
for (ConfigKey<?> k: getKeysAtContainer(getContainer()))
knownKeysOnType.put(k, k);
- Map<ConfigKey<?>, ConfigKey<?>> knownKeysIncludingDescendants =
MutableMap.copyOf(knownKeys);
+ Map<ConfigKey<?>, ConfigKey<?>> knownKeysIncludingDescendants =
MutableMap.copyOf(knownKeysAtDescendants);
knownKeysIncludingDescendants.putAll(knownKeysOnType);
Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>
parents = MutableMap.of();
@@ -645,7 +647,15 @@ public abstract class AbstractConfigMapImpl<TContainer
extends BrooklynObject> i
// 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());
+ ConfigInheritance inhHereOrDesc =
ConfigInheritances.findInheritance(kTypeOrDescendant,
InheritanceContext.RUNTIME_MANAGEMENT, null);
+ if (inhHereOrDesc==null) {
+ inhHereOrDesc =
kSet.getInheritanceByContext(InheritanceContext.RUNTIME_MANAGEMENT);
+ if (inhHereOrDesc != null) {
+ kOnType = kTypeOrDescendant = kSet; // prefer kset if it
has inheritance set (locally by value but not on type, e.g. because key was
removed from type while still present)
+ } else {
+ inhHereOrDesc = 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
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 c7b5532b64..f30f2fbf7a 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
@@ -19,15 +19,20 @@
package org.apache.brooklyn.core.entity.internal;
import java.util.Map;
+import java.util.Objects;
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.config.ConfigInheritance.ConfigInheritanceContext;
import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.BasicConfigInheritance;
import org.apache.brooklyn.core.config.ConfigConstraints;
+import org.apache.brooklyn.core.config.ConfigKeys.InheritanceContext;
import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl;
+import org.apache.brooklyn.core.entity.AbstractEntity;
import org.apache.brooklyn.core.entity.EntityInternal;
import org.apache.brooklyn.util.guava.Maybe;
import org.slf4j.Logger;
@@ -99,7 +104,20 @@ public class EntityConfigMap extends
AbstractConfigMapImpl<Entity> {
@Override
protected <T> ConfigKey<?> getKeyAtContainerImpl(Entity container,
ConfigKey<T> queryKey) {
if (queryKey==null) return null;
- return container.getEntityType().getConfigKey(queryKey.getName());
+ ConfigKey<?> kOnType =
container.getEntityType().getConfigKey(queryKey.getName());
+ if (kOnType!=null) return kOnType;
+ ConfigKey<?> kOnTypeUndeclared;
+ Map<ConfigKey<?>, Object> ownConfig = ((EntityConfigMap)
((EntityInternal) container).config().getInternalConfigMap()).ownConfig;
+ synchronized (ownConfig) {
+ kOnTypeUndeclared = ownConfig.keySet().stream().filter(ck ->
Objects.equals(queryKey.getName(), ck.getName())).findAny().orElse(null);
+ }
+ if (kOnTypeUndeclared!=null) {
+ // if a never inherited key is set, but not declared, it should be
returned
+ if
(BasicConfigInheritance.NEVER_INHERITED.equals(kOnTypeUndeclared.getInheritanceByContext(InheritanceContext.RUNTIME_MANAGEMENT)))
{
+ return kOnTypeUndeclared;
+ }
+ }
+ return null;
}
@Override
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 3ed05199be..07d7059a43 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
@@ -20,17 +20,22 @@ package org.apache.brooklyn.core.entity;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.stream.Collectors;
+import com.google.common.collect.Iterables;
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.BasicConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl;
import org.apache.brooklyn.core.entity.EntityConfigTest.MyOtherEntity;
import org.apache.brooklyn.core.entity.EntityConfigTest.MyOtherEntityImpl;
+import org.apache.brooklyn.core.entity.internal.EntityConfigMap;
import org.apache.brooklyn.core.sensor.AttributeSensorAndConfigKey;
import
org.apache.brooklyn.core.sensor.BasicAttributeSensorAndConfigKey.IntegerAttributeSensorAndConfigKey;
import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
@@ -42,8 +47,6 @@ 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) {
@@ -170,19 +173,81 @@ public class ConfigEntityInheritanceTest extends
BrooklynAppUnitTestSupport {
app.config().set(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE,
"maybe");
return
app.addChild(EntitySpec.create(MyEntityWithPartiallyHeritableConfig.class));
}
-
+
+ private boolean isKeyInInternalConfigMapInherited(Entity entity, ConfigKey
k) {
+ return isKeyInInternalConfigMapInherited(entity, k.getName());
+ }
+ private boolean isKeyInInternalConfigMapInherited(Entity entity, String s)
{
+ Map<ConfigKey<?>, Object> cfgMap = ((EntityInternal)
entity).config().getInternalConfigMap().getAllConfigInheritedRawValuesIgnoringErrors();
+ Set<String> cfgs = cfgMap.keySet().stream().map(k ->
k.getName()).collect(Collectors.toSet());
+ return cfgs.contains(s);
+ }
+
@Test
public void testConfigKeysInheritance() throws Exception {
+ ConfigKey<Object> WAS_NEVER_INHERIT_BUT_NOW_DEFAULT_INHERITANCE =
ConfigKeys.builder(Object.class,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT.getName()).build();
+
Entity child = setupBasicInheritanceTest();
Assert.assertNotNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT));
Assert.assertNotNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE));
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+
Assert.assertNull(child.getConfig(WAS_NEVER_INHERIT_BUT_NOW_DEFAULT_INHERITANCE));
// it's reinheritable unless explicitly declared
Assert.assertNotNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE));
+
+ Asserts.assertTrue(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT));
+ Asserts.assertTrue(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ Asserts.assertTrue(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE));
+
+ // if reinheritable is _declared_ at the app, it is null at the child
app.getMutableEntityType().addConfigKey(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE);
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE));
+ // and not found
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE));
+
+ // and returns to previous state when reverted
+
app.getMutableEntityType().removeConfigKey(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE);
+
Assert.assertNotNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE));
+ Asserts.assertTrue(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE));
+
+ // whereas never inherit is not found at child whether declared or not
on parent or child
+
app.getMutableEntityType().addConfigKey(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT);
+
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+
+
app.getMutableEntityType().removeConfigKey(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT);
+
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+
+
((EntityInternal)child).getMutableEntityType().addConfigKey(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT);
+
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+
Assert.assertNull(child.getConfig(WAS_NEVER_INHERIT_BUT_NOW_DEFAULT_INHERITANCE));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
WAS_NEVER_INHERIT_BUT_NOW_DEFAULT_INHERITANCE));
+
+
((EntityInternal)child).getMutableEntityType().removeConfigKey(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT);
+ // even if removed, if the key is set, it will keep that type
+
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ ((EntityInternal)
child).config().getInternalConfigMap().getAllConfigInheritedRawValuesIgnoringErrors();
+
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+
Assert.assertNull(child.getConfig(WAS_NEVER_INHERIT_BUT_NOW_DEFAULT_INHERITANCE));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
WAS_NEVER_INHERIT_BUT_NOW_DEFAULT_INHERITANCE));
+
+ // redefining at descendant doesn't change visibility
+ ConfigKey<Object> WAS_NEVER_INHERIT_BUT_NOW_OVERWRITE_AT_CHILD =
ConfigKeys.builder(Object.class,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT.getName())
+ .runtimeInheritance(BasicConfigInheritance.OVERWRITE).build();
+
((EntityInternal)child).getMutableEntityType().addConfigKey(WAS_NEVER_INHERIT_BUT_NOW_OVERWRITE_AT_CHILD);
+
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ // (but the key is known, of course)
+ Asserts.assertTrue(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+
+
((EntityInternal)child).getMutableEntityType().removeConfigKey(WAS_NEVER_INHERIT_BUT_NOW_OVERWRITE_AT_CHILD);
+
Assert.assertNull(child.getConfig(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
+ Asserts.assertFalse(isKeyInInternalConfigMapInherited(child,
MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT));
}
@SuppressWarnings("unchecked")
