logging and tests for config-to-task early resolution

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

Branch: refs/heads/master
Commit: 3e266e62f535da4e9888d475a96e6e811bd4a1c7
Parents: 9b9f51d
Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com>
Authored: Tue Sep 20 09:48:48 2016 +0100
Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com>
Committed: Tue Sep 20 09:58:14 2016 +0100

----------------------------------------------------------------------
 .../apache/brooklyn/api/objs/Configurable.java  |  5 +--
 .../mgmt/rebind/dto/MementosGenerators.java     | 44 +++++++++++++++++---
 .../core/sensor/DependentConfiguration.java     | 10 +++--
 .../core/mgmt/rebind/RebindEntityTest.java      | 18 +++++---
 4 files changed, 59 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3e266e62/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java 
b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
index d7f2935..02eee54 100644
--- a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
+++ b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
@@ -31,9 +31,6 @@ import com.google.common.annotations.Beta;
  */
 public interface Configurable {
 
-    // FIXME Moved from core project to api project, as part of moving 
EntityLocal.
-    // (though maybe it's fine here?)
-
     /**
      * @return the old value, or null if there was not one
      * @deprecated since 0.7.0; use {@link ConfigurationSupport#set(ConfigKey, 
Object)}, such as {@code config().set(key, val)} 
@@ -76,7 +73,7 @@ public interface Configurable {
         /**
          * Sets the config to the given value.
          */
-        <T> T set(ConfigKey<T> key, T val);
+        <T> T set(ConfigKey<T> key, T val); 
         
         /**
          * @see {@link #setConfig(HasConfigKey, Object)}

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3e266e62/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java
index 934a8e8..266ed07 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java
@@ -61,8 +61,12 @@ import org.apache.brooklyn.core.mgmt.rebind.TreeUtils;
 import org.apache.brooklyn.core.objs.BrooklynTypes;
 import org.apache.brooklyn.core.policy.AbstractPolicy;
 import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.flags.FlagUtils;
+import org.apache.brooklyn.util.text.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
@@ -72,6 +76,8 @@ import com.google.common.collect.Sets;
 public class MementosGenerators {
 
     private MementosGenerators() {}
+
+    private static final Logger log = 
LoggerFactory.getLogger(MementosGenerators.class);
     
     /** @deprecated since 0.7.0 use {@link #newBasicMemento(BrooklynObject)} */
     public static Memento newMemento(BrooklynObject instance) {
@@ -175,7 +181,7 @@ public class MementosGenerators {
         Map<ConfigKey<?>, Object> localConfig = 
entity.getConfigMap().getLocalConfig();
         for (Map.Entry<ConfigKey<?>, Object> entry : localConfig.entrySet()) {
             ConfigKey<?> key = checkNotNull(entry.getKey(), localConfig);
-            Object value = configValueToPersistable(entry.getValue());
+            Object value = configValueToPersistable(entry.getValue(), 
entityRaw, key.getName());
             builder.config.put(key, value); 
         }
         
@@ -326,7 +332,7 @@ public class MementosGenerators {
         Map<ConfigKey<?>, Object> config = 
((AbstractPolicy)policy).getConfigMap().getAllConfig();
         for (Map.Entry<ConfigKey<?>, Object> entry : config.entrySet()) {
             ConfigKey<?> key = checkNotNull(entry.getKey(), "config=%s", 
config);
-            Object value = configValueToPersistable(entry.getValue());
+            Object value = configValueToPersistable(entry.getValue(), policy, 
key.getName());
             builder.config.put(key.getName(), value); 
         }
         
@@ -369,7 +375,7 @@ public class MementosGenerators {
         Map<ConfigKey<?>, Object> config = 
((AbstractEnricher)enricher).getConfigMap().getAllConfig();
         for (Map.Entry<ConfigKey<?>, Object> entry : config.entrySet()) {
             ConfigKey<?> key = checkNotNull(entry.getKey(), "config=%s", 
config);
-            Object value = configValueToPersistable(entry.getValue());
+            Object value = configValueToPersistable(entry.getValue(), 
enricher, key.getName());
             builder.config.put(key.getName(), value); 
         }
         
@@ -399,7 +405,7 @@ public class MementosGenerators {
         Map<ConfigKey<?>, Object> config = 
((AbstractFeed)feed).getConfigMap().getAllConfig();
         for (Map.Entry<ConfigKey<?>, Object> entry : config.entrySet()) {
             ConfigKey<?> key = checkNotNull(entry.getKey(), "config=%s", 
config);
-            Object value = configValueToPersistable(entry.getValue());
+            Object value = configValueToPersistable(entry.getValue(), feed, 
key.getName());
             builder.config.put(key.getName(), value); 
         }
         
@@ -457,17 +463,43 @@ public class MementosGenerators {
         }
     }
 
+    /** @deprecated since 0.10.0; use {@link #configValueToPersistable(Object, 
BrooklynObject, String)} */ @Deprecated
     protected static Object configValueToPersistable(Object value) {
+        return configValueToPersistable(value, null, null);
+    }
+    
+    private static Set<String> WARNED_ON_PERSISTING_TASK_CONFIG = 
MutableSet.of();
+    
+    protected static Object configValueToPersistable(Object value, 
BrooklynObject obj, String keyName) {
         // TODO Swapping an attributeWhenReady task for the actual value, if 
completed.
         // Long-term, want to just handle task-persistence properly.
         if (value instanceof Task) {
             Task<?> task = (Task<?>) value;
+            String contextName = "";
+            if (obj!=null) {
+                contextName = obj.getCatalogItemId();
+                if (Strings.isBlank(contextName)) contextName= 
obj.getDisplayName();
+            }
+            if (keyName!=null) {
+                if (Strings.isNonBlank(contextName)) contextName += ":";
+                contextName += keyName;
+            }
+            String message = "Persisting "+contextName+" - encountered task 
"+value;
+            Object result = null;
             if (task.isDone() && !task.isError()) {
-                return task.getUnchecked();
+                result = task.getUnchecked();
+                message += "; persisting result "+result;
             } else {
                 // TODO how to record a completed but errored task?
-                return null;
+                message += "; persisting as null";
+                result = null;
+            }
+            if (WARNED_ON_PERSISTING_TASK_CONFIG.add(contextName)) {
+                log.warn(message+" (subsequent values for this key will be at 
null)");
+            } else {
+                log.debug(message);
             }
+            return result;
         }
         return value;
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3e266e62/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
 
b/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
index b4709d9..868bdc1 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/sensor/DependentConfiguration.java
@@ -19,7 +19,6 @@
 package org.apache.brooklyn.core.sensor;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import groovy.lang.Closure;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -84,7 +83,9 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 
-/** Conveniences for making tasks which run in entity {@link 
ExecutionContext}s, subscribing to attributes from other entities, possibly 
transforming those;
+import groovy.lang.Closure;
+
+/** Conveniences for making tasks which run in entity {@link 
ExecutionContext}s, blocking on attributes from other entities, possibly 
transforming those;
  * these {@link Task} instances are typically passed in {@link 
EntityLocal#setConfig(ConfigKey, Object)}.
  * <p>
  * If using a lot it may be useful to:
@@ -93,6 +94,9 @@ import com.google.common.collect.Lists;
  *   import static org.apache.brooklyn.core.sensor.DependentConfiguration.*;
  * }
  * </pre>
+ * <p>
+ * Note that these methods return one-time tasks. The DslComponent methods 
return long-lasting pointers
+ * and should now normally be used instead.
  */
 public class DependentConfiguration {
 
@@ -346,7 +350,7 @@ public class DependentConfiguration {
                         throw new CompoundRuntimeException("Aborted waiting 
for ready from "+source+" "+sensor, abortionExceptions);
                     }
 
-                    nextPeriod = nextPeriod.times(2).upperBound(maxPeriod);
+                    nextPeriod = nextPeriod.multiply(2).upperBound(maxPeriod);
                 }
                 if (LOG.isDebugEnabled()) LOG.debug("Attribute-ready for {} in 
entity {}", sensor, source);
                 return postProcess(value);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3e266e62/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 eb5fb54..c6c2790 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
@@ -150,27 +150,35 @@ public class RebindEntityTest extends 
RebindTestFixtureWithApp {
     }
     
     @Test
-    public void testRestoresEntityDependentConfigCompleted() throws Exception {
+    public void 
testRestoresEntityLowLevelDependentConfigCompletedStoresValue() throws 
Exception {
         MyEntity origE = 
origApp.createAndManageChild(EntitySpec.create(MyEntity.class)
                 .configure("myconfig", 
DependentConfiguration.attributeWhenReady(origApp, 
TestApplication.MY_ATTRIBUTE)));
         origApp.sensors().set(TestApplication.MY_ATTRIBUTE, "myval");
         origE.getConfig(MyEntity.MY_CONFIG); // wait for it to be done
-        
+
+        // note it does not change; this is by design using DependentConfig 
(as opposed to DSL methods)
+        origApp.sensors().set(TestApplication.MY_ATTRIBUTE, "myval2");
+        assertEquals(origE.getConfig(MyEntity.MY_CONFIG), "myval");
+
         newApp = rebind();
         MyEntity newE = (MyEntity) Iterables.find(newApp.getChildren(), 
Predicates.instanceOf(MyEntity.class));
         assertEquals(newE.getConfig(MyEntity.MY_CONFIG), "myval");
+        
+        origApp.sensors().set(TestApplication.MY_ATTRIBUTE, "myval3");
+        assertEquals(newE.getConfig(MyEntity.MY_CONFIG), "myval");
     }
     
-    @Test(enabled=false) // not yet supported
+    @Test
     public void testRestoresEntityDependentConfigUncompleted() throws 
Exception {
         origApp.createAndManageChild(EntitySpec.create(MyEntity.class)
                 .configure("myconfig", 
DependentConfiguration.attributeWhenReady(origApp, 
TestApplication.MY_ATTRIBUTE)));
         
         newApp = rebind();
         MyEntity newE = (MyEntity) Iterables.find(newApp.getChildren(), 
Predicates.instanceOf(MyEntity.class));
+
+        // because Task is not persisted; should log a warning above
         newApp.sensors().set(TestApplication.MY_ATTRIBUTE, "myval");
-        
-        assertEquals(newE.getConfig(MyEntity.MY_CONFIG), "myval");
+        assertEquals(newE.getConfig(MyEntity.MY_CONFIG), null);
     }
     
     @Test

Reply via email to