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 2017/11/10 04:24:27 UTC
[2/7] brooklyn-server git commit: more comments and test for
persisted non-type config
more comments and test for persisted non-type config
in particular the fact that such added config shifts to be on the type after rebind,
and tidy for other affected tests
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/31ff1855
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/31ff1855
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/31ff1855
Branch: refs/heads/master
Commit: 31ff1855fbd0334457e64b621376cd349c545992
Parents: 1980b00
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Nov 9 11:34:20 2017 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Nov 9 11:38:29 2017 +0000
----------------------------------------------------------------------
.../config/internal/AbstractConfigMapImpl.java | 3 +-
.../mgmt/rebind/dto/BasicEntityMemento.java | 6 +++-
.../core/mgmt/rebind/RebindEntityTest.java | 34 ++++++++++++++++++++
3 files changed, 41 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/31ff1855/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
----------------------------------------------------------------------
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 5fefbca..8ee78c8 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
@@ -404,7 +404,8 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i
if (ownKey1==null) ownKey1 = queryKey;
final ConfigKey<T> ownKey = ownKey1;
@SuppressWarnings("unchecked")
- final Class<T> type = (Class<T>) moreSpecificOrWarningPreferringFirst(ownKey, queryKey, ""+getContainer());
+ // NB: can't use ""+getContainerImpl() as this can loop in the case of locations looking up ports
+ final Class<T> type = (Class<T>) moreSpecificOrWarningPreferringFirst(ownKey, queryKey, ""+getContainer().getId()+"["+getContainer().getDisplayName()+"]");
// takes type of own key (or query key if own key not available)
// takes default of own key if available and has default, else of query key
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/31ff1855/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java
index 38faab6..e862b58 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicEntityMemento.java
@@ -168,7 +168,11 @@ public class BasicEntityMemento extends AbstractTreeNodeMemento implements Entit
ConfigKey<?> key = entry.getKey();
ConfigKey<?> staticKey = staticConfigKeys.get(key.getName());
if (!configKeys.containsKey(key.getName()) && key != staticKey) {
- // added a key (programmatically) not declared on the type; add if not anonymous
+ // user added a key (programmatically) not declared on the type; persist if not anonymous.
+ // on rebind this shows up in getEntityType() which is still a difference in the pre-rebind entity,
+ // but that's more understandable and has been the case for a long time.
+ // (the entity type is unclear in the yaml era anyway.)
+ // see RebindEntityTest.test*Key*
if (isAnonymous(key)) {
// 2017-11 no longer persist these, wasteful and unhelpful,
// (can hurt if someone expects declared key to be meaningful)
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/31ff1855/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
index 5eba386..ff3b703 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindEntityTest.java
@@ -674,6 +674,8 @@ public class RebindEntityTest extends RebindTestFixtureWithApp {
origApp.config().set(keyAsObject, (int) 1);
// get the double when queried
Asserts.assertInstanceOf(origApp.config().get(keyAsDouble), Double.class);
+ // but doesn't actually know it's a double
+ Asserts.assertInstanceOf(origApp.config().get(keyAsObject), Integer.class);
// also assert the key isn't included in declared list
Optional<ConfigKey<?>> declaredKey = Iterables.tryFind(getTypeDeclaredKeys(origApp), (k) -> k.getName().equals(doubleKeyName));
if (declaredKey.isPresent()) Assert.fail("Shouldn't have declared anonymous key, but had: "+declaredKey.get());
@@ -681,12 +683,44 @@ public class RebindEntityTest extends RebindTestFixtureWithApp {
newApp = rebind();
// now (2017-11) this works because we check both types on lookup
Asserts.assertInstanceOf(newApp.config().get(keyAsDouble), Double.class);
+ // if not querying double, we get the original type
+ Asserts.assertInstanceOf(newApp.config().get(keyAsObject), Integer.class);
// and this also succeeds because because now the anonymous key definition is not persisted
// (test changed, but confirmed it fails without the new BasicEntityMemento.isAnonymous check)
Optional<ConfigKey<?>> persistedKey = Iterables.tryFind(getTypeDeclaredKeys(newApp), (k) -> k.getName().equals(doubleKeyName));
if (persistedKey.isPresent()) Assert.fail("Shouldn't have persisted anonymous key, but had: "+persistedKey.get());
}
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void testRebindNonAnonymousKeyAlsoGivesCorrectType() throws Exception {
+ // happens if we write yaml and put an int where a double is expected
+ final String doubleKeyName = "double.key";
+ final ConfigKey<Object> keyAsObject = ConfigKeys.newConfigKey(Object.class, doubleKeyName);
+ final ConfigKey<Double> keyAsDouble = ConfigKeys.newDoubleConfigKey(doubleKeyName);
+ // set an int, casting to allow it, but should be recording doubleness internally
+ origApp.config().set((ConfigKey<Object>) (ConfigKey<?>) keyAsDouble, (int) 1);
+ // get the double when queried
+ Asserts.assertInstanceOf(origApp.config().get(keyAsObject), Double.class);
+ // also assert the key isn't included in declared list
+ Optional<ConfigKey<?>> declaredKey = Iterables.tryFind(getTypeDeclaredKeys(origApp), (k) -> k.getName().equals(doubleKeyName));
+ if (declaredKey.isPresent()) Assert.fail("Shouldn't have declared anonymous key, but had: "+declaredKey.get());
+
+ newApp = rebind();
+ // should know internally it's a double
+ Asserts.assertInstanceOf(newApp.config().get(keyAsDouble), Double.class);
+ Asserts.assertInstanceOf(newApp.config().get(keyAsObject), Double.class);
+
+ // this works because we add the key to the type-declared items on persistence;
+ // this has always been the case, even though it introduces an asymmetry,
+ // it's the only place we currently have on the persisted entity to store keys
+ // (whereas the in-memory can keep dynamic keys in its map, not on the type;
+ // ideally the in-memory map would just use strings, and type would be added
+ // on set to the dynamic type if it's non-anonymous; see further notes in BasicEntityMemento.isAnonymous)
+ Optional<ConfigKey<?>> persistedKey = Iterables.tryFind(getTypeDeclaredKeys(newApp), (k) -> k.getName().equals(doubleKeyName));
+ if (!persistedKey.isPresent()) Assert.fail("Should have persisted non-anonymous key: "+persistedKey.get());
+ }
protected static Set<ConfigKey<?>> getTypeDeclaredKeys(Entity e) {
// thought this would work, but see comment on findKeysDeclared - it also includes present ones