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 <[email protected]> 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(); }
