You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2023/02/10 00:43:12 UTC
[brooklyn-server] 02/02: correctly exclude some uninheritable keys in edge case
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 d3b3f7cf0ebd052c1f31db60bc1b4040f1bd9622
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Feb 9 23:19:12 2023 +0000
correctly exclude some uninheritable keys in edge case
---
.../config/internal/AbstractConfigMapImpl.java | 10 +++-
.../core/entity/ConfigEntityInheritanceTest.java | 70 +++++++++++++---------
.../java/org/apache/brooklyn/config/ConfigMap.java | 2 +-
3 files changed, 51 insertions(+), 31 deletions(-)
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 1c0ce763fe..9c1f4c4c01 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
@@ -655,8 +655,7 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i
if (getParent()!=null) {
@SuppressWarnings("unchecked")
Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>> po = (Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>>) (Map<?,?>)
- ((AbstractConfigMapImpl<?>)getParentInternal().config().getInternalConfigMap())
- .getSelectedConfigInheritedRaw(knownKeysIncludingDescendants, true);
+ ((AbstractConfigMapImpl<?>)getParentInternal().config().getInternalConfigMap()).getSelectedConfigInheritedRaw(knownKeysIncludingDescendants, true);
parents.putAll(po);
}
@@ -696,7 +695,12 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i
}
if (onlyReinheritable) {
ConfigInheritance inhHere = ConfigInheritances.findInheritance(kOnType, InheritanceContext.RUNTIME_MANAGEMENT, getDefaultRuntimeInheritance());
- if (localValue.isAbsent() && !inhHere.isReinheritable(vl, InheritanceContext.RUNTIME_MANAGEMENT)) {
+ if (
+ //2023-02 previously required the local value to be absent but that now seems wrong
+ //localValue.isAbsent() &&
+
+ !inhHere.isReinheritable(vl, InheritanceContext.RUNTIME_MANAGEMENT)) {
+
// skip this one
continue;
}
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 ed440960c5..3ed05199be 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,6 +20,7 @@ package org.apache.brooklyn.core.entity;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntitySpec;
@@ -33,6 +34,8 @@ 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.entity.stock.BasicEntity;
+import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.collections.MutableSet;
@@ -186,44 +189,57 @@ public class ConfigEntityInheritanceTest extends BrooklynAppUnitTestSupport {
@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(((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));
+ 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);
+
+ 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);
+
+ 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);
+
+ 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);
+
+ rawHierarchy = ((ConfigMapWithInheritance<Entity>) ((EntityInternal) child).config().getInternalConfigMap()).
+ getConfigAllInheritedRaw(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE);
Assert.assertEquals(rawHierarchy, MutableList.of());
+
+ Asserts.assertEquals(((EntityInternal) child).config().getInternalConfigMap().getAllConfigInheritedRawValuesIgnoringErrors().keySet(),
+ MutableSet.of(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE, MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT));
+ }
+
+ @Test
+ public void testAllInheritanceOnAnonymousChild() {
+ Entity child0 = setupBasicInheritanceTest();
+ Entity child = app.createAndManageChild(EntitySpec.create(BasicEntity.class));
+ app.getMutableEntityType().addConfigKey(MyEntityWithPartiallyHeritableConfig.NEVER_INHERIT);
+ app.getMutableEntityType().addConfigKey(MyEntityWithPartiallyHeritableConfig.NOT_REINHERITABLE);
+ Asserts.assertEquals(((EntityInternal) child).config().getInternalConfigMap().getAllConfigInheritedRawValuesIgnoringErrors().keySet(),
+ MutableSet.of(MyEntityWithPartiallyHeritableConfig.ALWAYS_HERITABLE, MyEntityWithPartiallyHeritableConfig.HERITABLE_BY_DEFAULT));
}
public static class MyEntityWithPartiallyHeritableConfig extends AbstractEntity {
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 f9f755c174..45f00424c8 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
@@ -158,7 +158,7 @@ public interface ConfigMap {
@Beta
public Map<ConfigKey<?>,Object> getAllConfigInheritedRawValuesIgnoringErrors();
- /** as {@link #getAllConfigInheritedRaw()} but removes any entries which should not be re-inheritable by descendants */
+ /** as {@link #getAllConfigInheritedRawWithErrors()} but removes any entries which should not be re-inheritable by descendants */
public Map<ConfigKey<?>,ReferenceWithError<ConfigValueAtContainer<TContainer,?>>> getAllReinheritableConfigRaw();
}