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();