brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [4/7] brooklyn-server git commit: don't persist anonymous keys
Date Fri, 10 Nov 2017 04:24:29 GMT
don't persist anonymous keys

another way of fixing the failing-before-this-commit test.
adding anonymous keys to the type means rebinded entities differ from original.
see comments, we still have that discrepancy for non-anonymous keys, but there it seems more
acceptable.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/1980b005
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/1980b005
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/1980b005

Branch: refs/heads/master
Commit: 1980b005a4081d7374af90ac2454f51d1763d20b
Parents: 82dfdd2
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Thu Nov 9 11:12:17 2017 +0000
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Thu Nov 9 11:38:29 2017 +0000

----------------------------------------------------------------------
 .../mgmt/rebind/dto/BasicEntityMemento.java     | 32 ++++++++++++++++++--
 .../core/mgmt/rebind/RebindEntityTest.java      | 17 +++++++++--
 .../org/apache/brooklyn/config/ConfigMap.java   |  1 +
 3 files changed, 46 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1980b005/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 939ad2d..38faab6 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
@@ -36,11 +36,13 @@ import org.apache.brooklyn.core.config.Sanitizer;
 import org.apache.brooklyn.core.entity.AbstractEntity;
 import org.apache.brooklyn.core.objs.BrooklynTypes;
 import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -78,6 +80,7 @@ public class BasicEntityMemento extends AbstractTreeNodeMemento implements
Entit
         protected List<String> members = Lists.newArrayList();
         protected List<Effector<?>> effectors = Lists.newArrayList();
         
+        /** @deprecated since 1.0.0 not used, and incomplete (eg doesn't copy configKeys)
*/
         public Builder from(EntityMemento other) {
             super.from((TreeNode)other);
             isTopLevelApp = other.isTopLevelApp();
@@ -163,8 +166,23 @@ public class BasicEntityMemento extends AbstractTreeNodeMemento implements
Entit
         if (configByKey != null) {
             for (Map.Entry<ConfigKey<?>, Object> entry : configByKey.entrySet())
{
                 ConfigKey<?> key = entry.getKey();
-                if (!configKeys.containsKey(key.getName()) && key != staticConfigKeys.get(key.getName()))
{
-                    configKeys.put(key.getName(), key);
+                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
+                    if (isAnonymous(key)) {
+                        // 2017-11 no longer persist these, wasteful and unhelpful, 
+                        // (can hurt if someone expects declared key to be meaningful) 
+                        log.debug("Skipping persistence of "+key+" on "+getId()+" because
it is anonymous");
+                    } else {
+                        if (log.isTraceEnabled()) {
+                            if (staticKey!=null) {
+                                log.trace("Persisting dynamic config key "+key+" on "+getId()+",
overriding key on type "+staticKey);
+                            } else {
+                                log.trace("Persisting dynamic config key "+key+" on "+getId());
+                            }
+                        }
+                        configKeys.put(key.getName(), key);
+                    }
                 }
                 config.put(key.getName(), entry.getValue());
             }
@@ -190,6 +208,16 @@ public class BasicEntityMemento extends AbstractTreeNodeMemento implements
Entit
         attributes = toPersistedMap(attributes);
     }
 
+    private static boolean isAnonymous(ConfigKey<?> key) {
+        Preconditions.checkNotNull(key, "key");
+        if (!Object.class.equals(key.getType())) return false;
+        if (!Strings.isBlank(key.getDescription())) return false;
+        if (key.getDefaultValue()!=null) return false;
+        // inheritance, constraints, reconfigurability could also be checked - but above
should catch any such
+        // (not even sure we need this method at all - see caller)
+        return true;
+    }
+
     protected synchronized Map<String, ConfigKey<?>> getStaticConfigKeys() {
         if (staticConfigKeys==null) {
             @SuppressWarnings("unchecked")

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1980b005/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 8481248..5eba386 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
@@ -29,6 +29,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Semaphore;
@@ -673,15 +674,27 @@ public class RebindEntityTest extends RebindTestFixtureWithApp {
         origApp.config().set(keyAsObject, (int) 1);
         // get the double when queried
         Asserts.assertInstanceOf(origApp.config().get(keyAsDouble), 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();
         // now (2017-11) this works because we check both types on lookup
         Asserts.assertInstanceOf(newApp.config().get(keyAsDouble), Double.class);
         
-        // but this still fails because the anonymous key definition is persisted
-        Optional<ConfigKey<?>> persistedKey = Iterables.tryFind(newApp.config().findKeysDeclared(Predicates.alwaysTrue()),
(k) -> k.getName().equals(doubleKeyName));
+        // 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());
     }
+
+    protected static Set<ConfigKey<?>> getTypeDeclaredKeys(Entity e) {
+        // thought this would work, but see comment on findKeysDeclared - it also includes
present ones
+        //return e.config().findKeysDeclared(Predicates.alwaysTrue());
+        
+        // this is the right way:
+        return e.getEntityType().getConfigKeys();
+    }
     
     /**
      * @deprecated since 0.7; support for rebinding old-style entities is deprecated

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1980b005/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java
----------------------------------------------------------------------
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 0e5d564..f29b444 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
@@ -92,6 +92,7 @@ public interface ConfigMap {
     /** returns all keys present in the map matching the given filter predicate; see ConfigPredicates
for common predicates.
      * if the map is associated with a container or type context where reference keys are
defined,
      * those keys are included in the result whether or not present in the map (unlike {@link
#findKeysPresent(Predicate)}) */
+    // TODO should be findKeysDeclaredOrPresent - if you want just the declared ones, look
up the type
     public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>>
filter);
 
     /** as {@link #findKeysDeclared(Predicate)} but restricted to keys actually present in
the map


Mime
View raw message