brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rich...@apache.org
Subject [10/25] git commit: Policy/enricher rebind: use config
Date Sun, 01 Jun 2014 20:15:25 GMT
Policy/enricher rebind: use config

- use “config” instead of “flags”
- use no-arg constructor where available
- adds ManagementContextInternal.getPolicyFactory()
- refactor InternalPolicyFactory to be more like InternalLocationFactory


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/80b05c15
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/80b05c15
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/80b05c15

Branch: refs/heads/master
Commit: 80b05c15e23b3cb583f6625372f3353de804a289
Parents: 693a453
Author: Aled Sage <aled.sage@gmail.com>
Authored: Tue May 27 10:37:07 2014 +0100
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Fri May 30 10:24:38 2014 +0100

----------------------------------------------------------------------
 .../entity/proxying/InternalPolicyFactory.java  | 64 ++++++++++-----
 .../entity/rebind/RebindManagerImpl.java        | 85 ++++++++++++++++----
 .../entity/rebind/dto/BasicEnricherMemento.java |  2 +-
 .../entity/rebind/dto/MementosGenerators.java   | 59 ++++++++++----
 .../management/internal/LocalEntityManager.java |  5 ++
 .../internal/LocalManagementContext.java        |  6 ++
 .../internal/ManagementContextInternal.java     |  3 +
 .../NonDeploymentManagementContext.java         |  7 ++
 8 files changed, 177 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java b/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
index c51144d..1a5c4ed 100644
--- a/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
+++ b/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
@@ -113,14 +113,13 @@ public class InternalPolicyFactory {
         try {
             Class<? extends T> clazz = spec.getType();
             
-            FactoryConstructionTracker.setConstructing();
             T pol;
-            try {
-                pol = construct(clazz, spec);
-            } finally {
-                FactoryConstructionTracker.reset();
+            if (isNewStylePolicy(clazz)) {
+                pol = constructPolicy(clazz);
+            } else {
+                pol = constructOldStyle(clazz, MutableMap.copyOf(spec.getFlags()));
             }
-            
+
             if (spec.getDisplayName()!=null)
                 ((AbstractPolicy)pol).setName(spec.getDisplayName());
             
@@ -154,12 +153,11 @@ public class InternalPolicyFactory {
         try {
             Class<? extends T> clazz = spec.getType();
             
-            FactoryConstructionTracker.setConstructing();
             T enricher;
-            try {
-                enricher = construct(clazz, spec);
-            } finally {
-                FactoryConstructionTracker.reset();
+            if (isNewStyleEnricher(clazz)) {
+                enricher = constructEnricher(clazz);
+            } else {
+                enricher = constructOldStyle(clazz, MutableMap.copyOf(spec.getFlags()));
             }
             
             if (spec.getDisplayName()!=null)
@@ -186,19 +184,43 @@ public class InternalPolicyFactory {
         }
     }
     
-    private <T extends Policy> T construct(Class<? extends T> clazz, PolicySpec<T>
spec) throws InstantiationException, IllegalAccessException, InvocationTargetException {
-        if (isNewStylePolicy(clazz)) {
-            return clazz.newInstance();
-        } else {
-            return constructOldStyle(clazz, MutableMap.copyOf(spec.getFlags()));
+    /**
+     * Constructs a new-style policy (fails if no no-arg constructor).
+     */
+    public <T extends Policy> T constructPolicy(Class<T> clazz) {
+        try {
+            FactoryConstructionTracker.setConstructing();
+            try {
+                if (isNewStylePolicy(clazz)) {
+                    return clazz.newInstance();
+                } else {
+                    throw new IllegalStateException("Policy class "+clazz+" must have a no-arg
constructor");
+                }
+            } finally {
+                FactoryConstructionTracker.reset();
+            }
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
         }
     }
     
-    private <T extends Enricher> T construct(Class<? extends T> clazz, EnricherSpec<T>
spec) throws InstantiationException, IllegalAccessException, InvocationTargetException {
-        if (isNewStyleEnricher(clazz)) {
-            return clazz.newInstance();
-        } else {
-            return constructOldStyle(clazz, MutableMap.copyOf(spec.getFlags()));
+    /**
+     * Constructs a new-style enricher (fails if no no-arg constructor).
+     */
+    public <T extends Enricher> T constructEnricher(Class<T> clazz) {
+        try {
+            FactoryConstructionTracker.setConstructing();
+            try {
+                if (isNewStyleEnricher(clazz)) {
+                    return clazz.newInstance();
+                } else {
+                    throw new IllegalStateException("Enricher class "+clazz+" must have a
no-arg constructor");
+                }
+            } finally {
+                FactoryConstructionTracker.reset();
+            }
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
         }
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
index ab5a8eb..5cc6f6e 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -11,6 +11,7 @@ import java.util.concurrent.TimeoutException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import brooklyn.enricher.basic.AbstractEnricher;
 import brooklyn.entity.Application;
 import brooklyn.entity.Entity;
 import brooklyn.entity.basic.AbstractApplication;
@@ -21,6 +22,7 @@ import brooklyn.entity.proxying.EntitySpec;
 import brooklyn.entity.proxying.InternalEntityFactory;
 import brooklyn.entity.proxying.InternalLocationFactory;
 import brooklyn.entity.rebind.RebindExceptionHandlerImpl.RebindFailureMode;
+import brooklyn.entity.proxying.InternalPolicyFactory;
 import brooklyn.location.Location;
 import brooklyn.location.basic.AbstractLocation;
 import brooklyn.location.basic.LocationInternal;
@@ -36,6 +38,7 @@ import brooklyn.mementos.PolicyMemento;
 import brooklyn.mementos.TreeNode;
 import brooklyn.policy.Enricher;
 import brooklyn.policy.Policy;
+import brooklyn.policy.basic.AbstractPolicy;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.flags.FlagUtils;
@@ -513,35 +516,62 @@ public class RebindManagerImpl implements RebindManager {
     }
 
     /**
-     * Constructs a new policy, passing to its constructor the policy id and all of memento.getFlags().
+     * Constructs a new policy, passing to its constructor the policy id and all of memento.getConfig().
      */
     private Policy newPolicy(PolicyMemento memento, Reflections reflections) {
         String id = memento.getId();
-        String policyType = checkNotNull(memento.getType(), "policyType of "+id);
-        Class<?> policyClazz = reflections.loadClass(policyType);
+        String policyType = checkNotNull(memento.getType(), "policy type of %s must not be
null in memento", id);
+        Class<? extends Policy> policyClazz = (Class<? extends Policy>) reflections.loadClass(policyType);
+
+        if (InternalPolicyFactory.isNewStylePolicy(policyClazz)) {
+            InternalPolicyFactory policyFactory = managementContext.getPolicyFactory();
+            Policy policy = policyFactory.constructPolicy(policyClazz);
+            FlagUtils.setFieldsFromFlags(ImmutableMap.of("id", id), policy);
+            ((AbstractPolicy)policy).setManagementContext(managementContext);
+            
+            return policy;
 
-        Map<String, Object> flags = MutableMap.<String, Object>builder()
-                .put("id", id)
-                .putAll(memento.getConfig())
-                .build();
+        } else {
+            LOG.warn("Deprecated rebind of policy without no-arg constructor; this may not
be supported in future versions: id="+id+"; type="+policyType);
+            
+            // There are several possibilities for the constructor; find one that works.
+            // Prefer passing in the flags because required for Application to set the management
context
+            // TODO Feels very hacky!
+            Map<String, Object> flags = MutableMap.<String, Object>of("id", id,
"deferConstructionChecks", true);
+            flags.putAll(memento.getConfig());
 
-        return (Policy) invokeConstructor(reflections, policyClazz, new Object[] {flags});
+            return (Policy) invokeConstructor(reflections, policyClazz, new Object[] {flags});
+        }
     }
 
     /**
-     * Constructs a new enricher, passing to its constructor the enricher id and all of memento.getFlags().
+     * Constructs a new enricher, passing to its constructor the enricher id and all of memento.getConfig().
      */
     private Enricher newEnricher(EnricherMemento memento, Reflections reflections) {
         String id = memento.getId();
-        String enricherType = checkNotNull(memento.getType(), "enricherType of "+id);
-        Class<?> enricherClazz = reflections.loadClass(enricherType);
+        String enricherType = checkNotNull(memento.getType(), "enricher type of %s must not
be null in memento", id);
+        Class<? extends Enricher> enricherClazz = (Class<? extends Enricher>)
reflections.loadClass(enricherType);
+
+        if (InternalPolicyFactory.isNewStyleEnricher(enricherClazz)) {
+            InternalPolicyFactory policyFactory = managementContext.getPolicyFactory();
+            Enricher enricher = policyFactory.constructEnricher(enricherClazz);
+            FlagUtils.setFieldsFromFlags(ImmutableMap.of("id", id), enricher);
+            ((AbstractEnricher)enricher).setManagementContext(managementContext);
+            
+            return enricher;
 
-        Map<String, Object> flags = MutableMap.<String, Object>builder()
-                .put("id", id)
-                .putAll(memento.getConfig())
-                .build();
+        } else {
+            LOG.warn("Deprecated rebind of enricher without no-arg constructor; this may
not be supported in future versions: id="+id+"; type="+enricherType);
+            
+            // There are several possibilities for the constructor; find one that works.
+            // Prefer passing in the flags because required for Application to set the management
context
+            // TODO Feels very hacky!
+            Map<String, Object> flags = MutableMap.<String, Object>of("id", id,
"deferConstructionChecks", true);
+            flags.putAll(memento.getConfig());
+
+            return (Enricher) invokeConstructor(reflections, enricherClazz, new Object[]
{flags});
+        }
 
-        return (Enricher) invokeConstructor(reflections, enricherClazz, new Object[] {flags});
     }
 
     private <T> T invokeConstructor(Reflections reflections, Class<T> clazz,
Object[]... possibleArgs) {
@@ -558,6 +588,29 @@ public class RebindManagerImpl implements RebindManager {
         throw new IllegalStateException("Cannot instantiate instance of type "+clazz+"; expected
constructor signature not found");
     }
 
+    private static boolean isNewStylePolicy(Class<?> clazz) {
+        if (!Policy.class.isAssignableFrom(clazz)) {
+            throw new IllegalArgumentException("Class "+clazz+" is not a policy");
+        }
+        return hasNoArgConstructor(clazz);
+    }
+    
+    private static boolean isNewStyleEnricher(Class<?> clazz) {
+        if (!Enricher.class.isAssignableFrom(clazz)) {
+            throw new IllegalArgumentException("Class "+clazz+" is not an enricher");
+        }
+        return hasNoArgConstructor(clazz);
+    }
+    
+    private static boolean hasNoArgConstructor(Class<?> clazz) {
+        try {
+            clazz.getConstructor(new Class[0]);
+            return true;
+        } catch (NoSuchMethodException e) {
+            return false;
+        }
+    }
+    
     /**
      * Wraps a ChangeListener, to log and never propagate any exceptions that it throws.
      * 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/entity/rebind/dto/BasicEnricherMemento.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/dto/BasicEnricherMemento.java b/core/src/main/java/brooklyn/entity/rebind/dto/BasicEnricherMemento.java
index e1c751a..3642101 100644
--- a/core/src/main/java/brooklyn/entity/rebind/dto/BasicEnricherMemento.java
+++ b/core/src/main/java/brooklyn/entity/rebind/dto/BasicEnricherMemento.java
@@ -16,7 +16,7 @@ import com.google.common.collect.Maps;
  */
 public class BasicEnricherMemento extends AbstractMemento implements EnricherMemento, Serializable
{
 
-    private static final long serialVersionUID = -1; // FIXME
+    private static final long serialVersionUID = 3922505388588186311L;
 
     public static Builder builder() {
         return new Builder();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java b/core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java
index f3214df..9f744ba 100644
--- a/core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java
+++ b/core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java
@@ -7,6 +7,7 @@ import java.util.Map;
 import java.util.Set;
 
 import brooklyn.config.ConfigKey;
+import brooklyn.enricher.basic.AbstractEnricher;
 import brooklyn.entity.Application;
 import brooklyn.entity.Entity;
 import brooklyn.entity.Group;
@@ -27,6 +28,7 @@ import brooklyn.mementos.LocationMemento;
 import brooklyn.mementos.PolicyMemento;
 import brooklyn.policy.Enricher;
 import brooklyn.policy.Policy;
+import brooklyn.policy.basic.AbstractPolicy;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.config.ConfigBag;
 import brooklyn.util.flags.FlagUtils;
@@ -97,20 +99,7 @@ public class MementosGenerators {
         Map<ConfigKey<?>, Object> localConfig = ((EntityInternal)entity).getConfigMap().getLocalConfig();
         for (Map.Entry<ConfigKey<?>, Object> entry : localConfig.entrySet())
{
             ConfigKey<?> key = checkNotNull(entry.getKey(), localConfig);
-            Object value = entry.getValue();
-            
-            // 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;
-                if (task.isDone() && !task.isError()) {
-                    value = task.getUnchecked();
-                } else {
-                    // TODO how to record a completed but errored task?
-                    value = null;
-                }
-            }
-
+            Object value = configValueToPersistable(entry.getValue());
             builder.config.put(key, value); 
         }
         
@@ -217,8 +206,19 @@ public class MementosGenerators {
         builder.id = policy.getId();
         builder.displayName = policy.getName();
 
-        builder.flags.putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(policy, Modifier.STATIC
^ Modifier.TRANSIENT));
+        // TODO persist config keys as well? Or only support those defined on policy class;
+        // current code will lose the ConfigKey type on rebind for anything not defined on
class.
+        // Whereas entities support that.
+        // TODO Do we need the "nonPersistableFlagNames" that locations use?
+        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());
+            builder.config.put(key.getName(), value); 
+        }
         
+        builder.config.putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(policy, Modifier.STATIC
^ Modifier.TRANSIENT));
+
         return builder;
     }
     
@@ -237,8 +237,35 @@ public class MementosGenerators {
         builder.id = enricher.getId();
         builder.displayName = enricher.getName();
 
-        builder.flags.putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(enricher, Modifier.STATIC
^ Modifier.TRANSIENT));
+        // TODO persist config keys as well? Or only support those defined on policy class;
+        // current code will lose the ConfigKey type on rebind for anything not defined on
class.
+        // Whereas entities support that.
+        // TODO Do we need the "nonPersistableFlagNames" that locations use?
+        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());
+            builder.config.put(key.getName(), value); 
+        }
         
+        builder.config.putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(enricher, Modifier.STATIC
^ Modifier.TRANSIENT));
+
         return builder;
+
+    }
+    
+    protected static Object configValueToPersistable(Object value) {
+        // 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;
+            if (task.isDone() && !task.isError()) {
+                return task.getUnchecked();
+            } else {
+                // TODO how to record a completed but errored task?
+                return null;
+            }
+        }
+        return value;
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java b/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
index eaca3d2..c806645 100644
--- a/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
+++ b/core/src/main/java/brooklyn/management/internal/LocalEntityManager.java
@@ -92,6 +92,11 @@ public class LocalEntityManager implements EntityManagerInternal {
         return entityFactory;
     }
 
+    public InternalPolicyFactory getPolicyFactory() {
+        if (!isRunning()) throw new IllegalStateException("Management context no longer running");
+        return policyFactory;
+    }
+
     @Override
     public EntityTypeRegistry getEntityTypeRegistry() {
         if (!isRunning()) throw new IllegalStateException("Management context no longer running");

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/management/internal/LocalManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/LocalManagementContext.java b/core/src/main/java/brooklyn/management/internal/LocalManagementContext.java
index 643583b..96ccce7 100644
--- a/core/src/main/java/brooklyn/management/internal/LocalManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/LocalManagementContext.java
@@ -25,6 +25,7 @@ import brooklyn.entity.drivers.downloads.BasicDownloadsManager;
 import brooklyn.entity.effector.Effectors;
 import brooklyn.entity.proxying.InternalEntityFactory;
 import brooklyn.entity.proxying.InternalLocationFactory;
+import brooklyn.entity.proxying.InternalPolicyFactory;
 import brooklyn.internal.storage.DataGridFactory;
 import brooklyn.location.Location;
 import brooklyn.management.AccessController;
@@ -222,6 +223,11 @@ public class LocalManagementContext extends AbstractManagementContext
{
     }
 
     @Override
+    public InternalPolicyFactory getPolicyFactory() {
+        return getEntityManager().getPolicyFactory();
+    }
+
+    @Override
     public synchronized LocalLocationManager getLocationManager() {
         if (!isRunning()) throw new IllegalStateException("Management context no longer running");
         return locationManager;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
b/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
index fee519d..fd51427 100644
--- a/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
+++ b/core/src/main/java/brooklyn/management/internal/ManagementContextInternal.java
@@ -12,6 +12,7 @@ import brooklyn.entity.basic.BrooklynTaskTags;
 import brooklyn.entity.basic.ConfigKeys;
 import brooklyn.entity.proxying.InternalEntityFactory;
 import brooklyn.entity.proxying.InternalLocationFactory;
+import brooklyn.entity.proxying.InternalPolicyFactory;
 import brooklyn.internal.storage.BrooklynStorage;
 import brooklyn.location.Location;
 import brooklyn.management.ManagementContext;
@@ -60,6 +61,8 @@ public interface ManagementContextInternal extends ManagementContext {
     
     InternalLocationFactory getLocationFactory();
     
+    InternalPolicyFactory getPolicyFactory();
+    
     /**
      * Registers an entity that has been created, but that has not yet begun to be managed.
      * <p>

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/80b05c15/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
b/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
index fcc7d10..7f49629 100644
--- a/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
@@ -22,6 +22,7 @@ import brooklyn.entity.drivers.EntityDriverManager;
 import brooklyn.entity.drivers.downloads.DownloadResolverManager;
 import brooklyn.entity.proxying.InternalEntityFactory;
 import brooklyn.entity.proxying.InternalLocationFactory;
+import brooklyn.entity.proxying.InternalPolicyFactory;
 import brooklyn.entity.rebind.ChangeListener;
 import brooklyn.entity.rebind.RebindExceptionHandler;
 import brooklyn.entity.rebind.RebindManager;
@@ -134,6 +135,12 @@ public class NonDeploymentManagementContext implements ManagementContextInternal
     }
 
     @Override
+    public InternalPolicyFactory getPolicyFactory() {
+        checkInitialManagementContextReal();
+        return initialManagementContext.getPolicyFactory();
+    }
+
+    @Override
     public EntityManager getEntityManager() {
         return entityManager;
     }


Mime
View raw message