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


The following commit(s) were added to refs/heads/master by this push:
     new 38eb80a  minor tweaks to preserve config order
38eb80a is described below

commit 38eb80a16e2264d2a328601e842fcbcccdb95985
Author: Alex Heneveld <[email protected]>
AuthorDate: Wed Mar 10 01:42:51 2021 +0000

    minor tweaks to preserve config order
    
    * when camp processes keys and flags, it does it in several passes, 
changing the order
    * when declared keys are retrieved, the order of set keys was not respected
    
    both fixed, along with one thing confusing the ide
---
 .../BrooklynComponentTemplateResolver.java         | 83 +++++++++++++---------
 .../config/internal/AbstractConfigMapImpl.java     |  9 ++-
 .../brooklyn/core/entity/EntityAssertsTest.java    |  8 +--
 3 files changed, 60 insertions(+), 40 deletions(-)

diff --git 
a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
 
b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
index c3f8a80..97992f7 100644
--- 
a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
+++ 
b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
@@ -58,7 +58,6 @@ import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
 import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
 import 
org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
 import org.apache.brooklyn.core.resolve.entity.EntitySpecResolver;
-import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
@@ -72,6 +71,7 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.net.Urls;
 import org.apache.brooklyn.util.text.Strings;
+import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.yaml.snakeyaml.Yaml;
@@ -306,9 +306,9 @@ public class BrooklynComponentTemplateResolver {
 
         // now set configuration for all the items in the bag
         Map<String, ConfigKey<?>> entityConfigKeys = findAllConfigKeys(spec);
-
         Collection<FlagConfigKeyAndValueRecord> records = 
findAllFlagsAndConfigKeyValues(spec, bag);
-        Set<String> keyNamesUsed = new LinkedHashSet<String>();
+
+        Map<String, Pair<Object,Object>> configLookup = MutableMap.of();
         for (FlagConfigKeyAndValueRecord r: records) {
             // flags and config keys tracked separately, look at each (may be 
overkill but it's what we've always done)
 
@@ -319,21 +319,15 @@ public class BrooklynComponentTemplateResolver {
                 final Object ownValue1 = new SpecialFlagsTransformer(loader, 
encounteredRegisteredTypeIds).apply(r.getFlagMaybeValue().get());
                 final Object ownValueF = 
rawConvFn.apply(Maybe.ofAllowingNull(ownValue1), key.getTypeToken()).get();
 
-                Function<EntitySpec<?>, Maybe<Object>> rawEvalFn = new 
Function<EntitySpec<?>,Maybe<Object>>() {
-                    @Override
-                    public Maybe<Object> apply(EntitySpec<?> input) {
-                        return spec.getFlags().containsKey(flag) ? 
Maybe.of((Object)spec.getFlags().get(flag)) : Maybe.absent();
-                    }
-                };
+                Function<EntitySpec<?>, Maybe<Object>> rawEvalFn = input -> 
spec.getFlags().containsKey(flag) ? Maybe.of((Object)spec.getFlags().get(flag)) 
: Maybe.absent();
                 Iterable<? extends 
ConfigValueAtContainer<EntitySpec<?>,Object>> ckvi = MutableList.of(
-                        new 
LazyContainerAndKeyValue<EntitySpec<?>,Object>(key, null, rawEvalFn, 
rawConvFn));
+                        new LazyContainerAndKeyValue<>(key, null, rawEvalFn, 
rawConvFn));
 
                 ConfigValueAtContainer<EntitySpec<?>,Object> combinedVal = 
ConfigInheritances.resolveInheriting(
                         null, key, Maybe.ofAllowingNull(ownValueF), 
Maybe.<Object>absent(),
                         ckvi.iterator(), InheritanceContext.TYPE_DEFINITION, 
getDefaultConfigInheritance()).getWithoutError();
 
-                spec.configure(flag, combinedVal.get());
-                keyNamesUsed.add(flag);
+                configLookup.put(flag, Pair.of(flag, combinedVal.get()));
             }
 
             if (r.getConfigKeyMaybeValue().isPresent()) {
@@ -341,12 +335,7 @@ public class BrooklynComponentTemplateResolver {
                 final Object ownValue1 = new SpecialFlagsTransformer(loader, 
encounteredRegisteredTypeIds).apply(r.getConfigKeyMaybeValue().get());
                 final Object ownValueF = 
rawConvFn.apply(Maybe.ofAllowingNull(ownValue1), key.getTypeToken()).get();
 
-                Function<EntitySpec<?>, Maybe<Object>> rawEvalFn = new 
Function<EntitySpec<?>,Maybe<Object>>() {
-                    @Override
-                    public Maybe<Object> apply(EntitySpec<?> input) {
-                        return spec.getConfig().containsKey(key) ? 
Maybe.of(spec.getConfig().get(key)) : Maybe.absent();
-                    }
-                };
+                Function<EntitySpec<?>, Maybe<Object>> rawEvalFn = input -> 
spec.getConfig().containsKey(key) ? Maybe.of(spec.getConfig().get(key)) : 
Maybe.absent();
                 Iterable<? extends 
ConfigValueAtContainer<EntitySpec<?>,Object>> ckvi = MutableList.of(
                         new 
LazyContainerAndKeyValue<EntitySpec<?>,Object>(key, null, rawEvalFn, 
rawConvFn));
 
@@ -354,11 +343,47 @@ public class BrooklynComponentTemplateResolver {
                         null, key, Maybe.ofAllowingNull(ownValueF), 
Maybe.<Object>absent(),
                         ckvi.iterator(), InheritanceContext.TYPE_DEFINITION, 
getDefaultConfigInheritance()).getWithoutError();
 
-                spec.configure(key, combinedVal.get());
-                keyNamesUsed.add(key.getName());
+                configLookup.put(key.getName(), Pair.of(key, 
combinedVal.get()));
             }
         }
 
+        Set<String> keyNamesUsed = MutableSet.copyOf(configLookup.keySet());
+        Set<String> unusedBagNames = 
MutableSet.copyOf(bag.getUnusedConfig().keySet());
+
+        // preserve the order
+        bag.forEach((k,v) -> {
+            Pair<Object,Object> toSet = configLookup.remove(k);
+            if (toSet!=null) {
+                if (toSet.getLeft() instanceof String)
+                    spec.configure((String)toSet.getLeft(), toSet.getRight());
+                else if (toSet.getLeft() instanceof ConfigKey)
+                    spec.configure((ConfigKey)toSet.getLeft(), 
toSet.getRight());
+                else
+                    // shouldn't come here
+                    throw new IllegalStateException();
+            } else if (unusedBagNames.remove(k)) {
+                // anonymous keys -- insert into the right order
+                Object transformed = new SpecialFlagsTransformer(loader, 
encounteredRegisteredTypeIds).apply(bag.getStringKey(k));
+                transformed = convertConfig(Maybe.of(transformed), 
TypeToken.of(Object.class)).get();
+                spec.configure(ConfigKeys.newConfigKey(Object.class, k), 
transformed);
+                keyNamesUsed.add(k);
+            } else {
+                // a deprecated key name (or alias) was supplied; ignore this 
entry in the bag,
+                // it's okay that it gets inserted later on
+            }
+        });
+
+        // now pick up any config keys we missed (due to aliases?)
+        configLookup.values().forEach(toSet -> {
+            if (toSet.getLeft() instanceof String)
+                spec.configure((String) toSet.getLeft(), toSet.getRight());
+            else if (toSet.getLeft() instanceof ConfigKey)
+                spec.configure((ConfigKey) toSet.getLeft(), toSet.getRight());
+            else
+                // shouldn't come here
+                throw new IllegalStateException();
+        });
+
         // For anything that should not be inherited, clear it from the spec 
(if not set above)
         // (very few things follow this, esp not on the spec; things like 
camp.id do;
         // the meaning here is essentially that the given config cannot be 
stored in a parent spec)
@@ -372,19 +397,6 @@ public class BrooklynComponentTemplateResolver {
                 spec.removeFlag(key.getName());
             }
         }
-
-        // set unused keys as anonymous config keys -
-        // they aren't flags or known config keys, so must be passed as config 
keys in order for
-        // EntitySpec to know what to do with them (as they are passed to the 
spec as flags)
-        for (String key: MutableSet.copyOf(bag.getUnusedConfig().keySet())) {
-            // we don't let a flag with the same name as a config key override 
the config key
-            // (that's why we check whether it is used)
-            if (!keyNamesUsed.contains(key)) {
-                Object transformed = new SpecialFlagsTransformer(loader, 
encounteredRegisteredTypeIds).apply(bag.getStringKey(key));
-                transformed = convertConfig(Maybe.of(transformed), 
TypeToken.of(Object.class)).get();
-                spec.configure(ConfigKeys.newConfigKey(Object.class, 
key.toString()), transformed);
-            }
-        }
     }
 
     private <T> Maybe<T> convertConfig(Maybe<Object> input, TypeToken<T> type) 
{
@@ -439,6 +451,10 @@ public class BrooklynComponentTemplateResolver {
 
         // need to de-dupe? (can't use Set bc FCKAVR doesn't impl 
equals/hashcode)
         // TODO merge *bagFlags* with existing spec params, merge yaml w yaml 
parent params elsewhere
+
+        // optimization:
+        if (bagFlags.isEmpty()) return Collections.emptyList();
+
         List<FlagConfigKeyAndValueRecord> allKeys = MutableList.of();
         
allKeys.addAll(FlagUtils.findAllParameterConfigKeys(spec.getParameters(), 
bagFlags));
         if (spec.getImplementation() != null) {
@@ -448,6 +464,7 @@ public class BrooklynComponentTemplateResolver {
         for (Class<?> iface : spec.getAdditionalInterfaces()) {
             allKeys.addAll(FlagUtils.findAllFlagsAndConfigKeys(null, iface, 
bagFlags));
         }
+
         return allKeys;
     }
 
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 976a6dc..5bcc5f6 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
@@ -546,10 +546,8 @@ public abstract class AbstractConfigMapImpl<TContainer 
extends BrooklynObject> i
     @SuppressWarnings("deprecation")
     protected Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> 
filter, KeyFindingMode mode) {
         MutableSet<ConfigKey<?>> result = MutableSet.of();
-        if (mode==KeyFindingMode.DECLARED_OR_PRESENT) {
-            result.addAll( 
Iterables.filter(getKeysAtContainer(getContainer()), filter) );
-        }
 
+        // always put present ones first, in the order they were specified
         for (ConfigKey<?> k: Iterables.filter(ownConfig.keySet(), filter)) {
             if (result.contains(k)) continue;
             if (mode!=KeyFindingMode.PRESENT_NOT_RESOLVED) {
@@ -559,6 +557,11 @@ public abstract class AbstractConfigMapImpl<TContainer 
extends BrooklynObject> i
             result.add(k);
         }
 
+        // then add any additional ones declared on the type
+        if (mode==KeyFindingMode.DECLARED_OR_PRESENT) {
+            result.addAll( 
Iterables.filter(getKeysAtContainer(getContainer()), filter) );
+        }
+
         // due to set semantics local should be added first, it prevents equal 
items from parent from being added on top
         if (getParent()!=null) {
             // now take from runtime parents, but filtered
diff --git 
a/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java 
b/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java
index 2967f22..2508afc 100644
--- a/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java
@@ -84,7 +84,7 @@ public class EntityAssertsTest extends 
BrooklynAppUnitTestSupport {
         final String after = "after";
 
         Task<?> assertValue = entity.getExecutionContext().submit("assert attr 
equals", 
-            () -> EntityAsserts.assertAttributeEqualsEventually(entity, 
TestEntity.NAME, after));
+            () -> { EntityAsserts.assertAttributeEqualsEventually(entity, 
TestEntity.NAME, after); });
         entity.sensors().set(TestEntity.NAME, after);
         assertValue.get();
     }
@@ -128,11 +128,11 @@ public class EntityAssertsTest extends 
BrooklynAppUnitTestSupport {
     public void shouldAssertPredicateEventuallyTrue() throws Exception {
         final int testVal = 987654321;
         final CountDownLatch eventuallyEntered = new CountDownLatch(2);
-        Task<?> assertValue = entity.getExecutionContext().submit("assert 
predicate", () -> EntityAsserts.assertPredicateEventuallyTrue(entity, 
+        Task<?> assertValue = entity.getExecutionContext().submit("assert 
predicate", () -> { EntityAsserts.assertPredicateEventuallyTrue(entity,
             (input) -> {
                 eventuallyEntered.countDown();
                 return testVal == input.getSequenceValue();
-            }));
+            }); });
         eventuallyEntered.await();
         entity.setSequenceValue(testVal);
         assertValue.get();
@@ -150,7 +150,7 @@ public class EntityAssertsTest extends 
BrooklynAppUnitTestSupport {
     public void shouldFailAssertAttributeEqualsContinually() throws Throwable {
         final String myName = "myname";
         entity.sensors().set(TestEntity.NAME, myName);
-        Task<?> assertValue = entity.getExecutionContext().submit("check attr 
equals", () -> EntityAsserts.assertAttributeEqualsContinually(entity, 
TestEntity.NAME, myName));
+        Task<?> assertValue = entity.getExecutionContext().submit("check attr 
equals", () -> { EntityAsserts.assertAttributeEqualsContinually(entity, 
TestEntity.NAME, myName); });
         entity.sensors().set(TestEntity.NAME, "something");
         try {
             assertValue.get();

Reply via email to