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 2016/11/29 13:40:16 UTC
[02/13] brooklyn-server git commit: YAML config parsing: prefer
brooklyn.parameters keys
YAML config parsing: prefer brooklyn.parameters keys
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/92ff8201
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/92ff8201
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/92ff8201
Branch: refs/heads/master
Commit: 92ff8201714f4791137c63b836b128e734d82885
Parents: 9d120a8
Author: Aled Sage <al...@gmail.com>
Authored: Fri Nov 18 21:37:32 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Mon Nov 28 21:11:48 2016 +0000
----------------------------------------------------------------------
.../BrooklynComponentTemplateResolver.java | 36 ++++++++++++++++++--
.../catalog/SpecParameterUnwrappingTest.java | 9 +++--
2 files changed, 39 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/92ff8201/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
----------------------------------------------------------------------
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 ccf18c0..e968792 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
@@ -269,7 +269,7 @@ public class BrooklynComponentTemplateResolver {
// For config values inherited from the super-type (be that the Java type or another catalog item
// being extended), we lookup the config key to find out if the values should be merged, overridden or
// cleared.
-
+
ConfigBag bag = ConfigBag.newInstance((Map<Object, Object>) attrs.getStringKey(BrooklynCampReservedKeys.BROOKLYN_CONFIG));
ConfigBag bagFlags = ConfigBag.newInstanceCopying(attrs);
if (attrs.containsKey(BrooklynCampReservedKeys.BROOKLYN_FLAGS)) {
@@ -374,15 +374,45 @@ public class BrooklynComponentTemplateResolver {
* Searches for config keys in the type, additional interfaces and the implementation (if specified)
*/
private Collection<FlagConfigKeyAndValueRecord> findAllFlagsAndConfigKeyValues(EntitySpec<?> spec, ConfigBag bagFlags) {
+ // Matches the bagFlags against the names used in brooklyn.parameters, entity configKeys
+ // and entity fields with `@SetFromFlag`.
+ //
+ // Returns all config keys / flags that match things in bagFlags, including duplicates.
+ // For example, if a configKey in Java is re-declared in YAML `brooklyn.parameters`,
+ // then we'll get two records.
+ //
+ // Make some effort to have these returned in the right order. That is very important
+ // because they are added to the `EntitySpec.configure(key, val)`. If there is already
+ // a key in `EntitySpec.config`, then the put will replace the value and leave the key
+ // as-is (so the default-value and description of the key will remain as whatever the
+ // first put said).
+
+ // TODO We should remove duplicates, rather than just doing the `put` multiple times,
+ // relying on ordering. We should also respect the ordered returned by
+ // EntityDynamicType.getConfigKeys, which is much better (it respects `BasicConfigKeyOverwriting`
+ // etc).
+ //
+ // However, that is hard/fiddly because of the way a config key can be referenced by
+ // its real name or flag-name.
+ //
+ // I wonder if this is fundamentally broken (and I really do dislike our informal use
+ // of aliases). Consider a configKey with name A and alias B. The bagFlags could have
+ // {A: val1, B: val2}. There is no formal definition of which takes precedence. We'll add
+ // both to the entity's configBag, without any warning - it's up to the `config().get()`
+ // method to then figure out what to do. It gets worse if there is also a ConfigKey with
+ // name "B" the "val2" then applies to both!
+ //
+ // I plan to propose a change on dev@brooklyn, to replace `@SetFromFlag`!
+
Set<FlagConfigKeyAndValueRecord> allKeys = MutableSet.of();
- allKeys.addAll(FlagUtils.findAllFlagsAndConfigKeys(null, spec.getType(), bagFlags));
+ allKeys.addAll(FlagUtils.findAllParameterConfigKeys(spec.getParameters(), bagFlags));
if (spec.getImplementation() != null) {
allKeys.addAll(FlagUtils.findAllFlagsAndConfigKeys(null, spec.getImplementation(), bagFlags));
}
+ allKeys.addAll(FlagUtils.findAllFlagsAndConfigKeys(null, spec.getType(), bagFlags));
for (Class<?> iface : spec.getAdditionalInterfaces()) {
allKeys.addAll(FlagUtils.findAllFlagsAndConfigKeys(null, iface, bagFlags));
}
- allKeys.addAll(FlagUtils.findAllParameterConfigKeys(spec.getParameters(), bagFlags));
return allKeys;
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/92ff8201/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
index c0eb8ef..b332f8f 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/SpecParameterUnwrappingTest.java
@@ -244,13 +244,16 @@ public class SpecParameterUnwrappingTest extends AbstractYamlTest {
List<SpecParameter<?>> params = spec.getParameters();
switch (type.getSimpleName()) {
case "ConfigEntityForTest":
- assertEquals(params.size(), 4);
+ // expect: own "simple"; super-type's "sample.config"; and generic "defaultDisplayName"
+ assertEquals(params.size(), 3, "params="+params);
break;
case "ConfigPolicyForTest":
- assertEquals(params.size(), 3);
+ // expect: own "simple"; and super-type's "sample.config"
+ assertEquals(params.size(), 2, "params="+params);
break;
case "ConfigLocationForTest":
- assertEquals(params.size(), 8);
+ // Expect: own "simple"; super-type's "sample.config"; and generic "parentLocation" + "spec.final" + "spec.named.name" + "spec.original" + "temporaryLocation"
+ assertEquals(params.size(), 7, "params="+params);
break;
}
assertTrue(Iterables.tryFind(params, nameEqualTo("simple")).isPresent());