You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by ri...@apache.org on 2014/05/27 13:05:27 UTC
[11/16] git commit: Policy/enricher rebind: use config
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/dd86474d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/dd86474d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/dd86474d
Branch: refs/heads/feature/policy-persistence
Commit: dd86474d881658dafa0f12c85f429d213dedb458
Parents: ff16566
Author: Aled Sage <al...@gmail.com>
Authored: Tue May 27 10:37:07 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue May 27 11:22:45 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/dd86474d/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/dd86474d/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 55d42d5..1317bb2 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;
@@ -20,6 +21,7 @@ import brooklyn.entity.basic.EntityInternal;
import brooklyn.entity.proxying.EntitySpec;
import brooklyn.entity.proxying.InternalEntityFactory;
import brooklyn.entity.proxying.InternalLocationFactory;
+import brooklyn.entity.proxying.InternalPolicyFactory;
import brooklyn.location.Location;
import brooklyn.location.basic.AbstractLocation;
import brooklyn.location.basic.LocationInternal;
@@ -35,6 +37,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;
@@ -472,35 +475,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) {
@@ -517,6 +547,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/dd86474d/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/dd86474d/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/dd86474d/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/dd86474d/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/dd86474d/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/dd86474d/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 e9be4e1..8456bf8 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.RebindManager;
import brooklyn.internal.storage.BrooklynStorage;
@@ -133,6 +134,12 @@ public class NonDeploymentManagementContext implements ManagementContextInternal
}
@Override
+ public InternalPolicyFactory getPolicyFactory() {
+ checkInitialManagementContextReal();
+ return initialManagementContext.getPolicyFactory();
+ }
+
+ @Override
public EntityManager getEntityManager() {
return entityManager;
}