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 2014/08/07 23:55:41 UTC
[7/9] git commit: Move construction mess into AbstractBrooklynObject
Move construction mess into AbstractBrooklynObject
- delete duplication from AbstractEntity, AbstractLocation, etc
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/6cb55b50
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/6cb55b50
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/6cb55b50
Branch: refs/heads/master
Commit: 6cb55b500449c6d6dee033509d50e9a20cc16d1e
Parents: 0ab0c39
Author: Aled Sage <al...@gmail.com>
Authored: Thu Aug 7 13:24:16 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Aug 7 13:43:33 2014 +0100
----------------------------------------------------------------------
.../brooklyn/basic/AbstractBrooklynObject.java | 62 ++++++++++
.../brooklyn/entity/basic/AbstractEntity.java | 124 +++++++++----------
.../location/basic/AbstractLocation.java | 48 ++-----
.../AggregatingMachineProvisioningLocation.java | 4 +-
.../FixedListMachineProvisioningLocation.java | 4 +-
.../LocalhostMachineProvisioningLocation.java | 10 +-
.../location/basic/SshMachineLocation.java | 3 +-
.../policy/basic/AbstractEntityAdjunct.java | 80 +++---------
.../location/jclouds/JcloudsLocation.java | 4 +-
9 files changed, 167 insertions(+), 172 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java b/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
index 5ddefd0..8396ec7 100644
--- a/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
+++ b/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
@@ -18,18 +18,30 @@
*/
package brooklyn.basic;
+import java.util.Collections;
+import java.util.Map;
import java.util.Set;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import brooklyn.entity.proxying.InternalFactory;
import brooklyn.management.ManagementContext;
import brooklyn.management.internal.ManagementContextInternal;
+import brooklyn.util.config.ConfigBag;
import brooklyn.util.flags.SetFromFlag;
import brooklyn.util.text.Identifiers;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
+ private static final Logger log = LoggerFactory.getLogger(AbstractBrooklynObject.class);
+
+ private boolean _legacyConstruction;
+
@SetFromFlag(value="id")
private String id = Identifiers.makeRandomId(8);
@@ -39,6 +51,56 @@ public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
public abstract void setDisplayName(String newName);
+ public AbstractBrooklynObject() {
+ this(Maps.newLinkedHashMap());
+ }
+
+ public AbstractBrooklynObject(Map<?,?> properties) {
+ _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing();
+
+ if (!_legacyConstruction && properties!=null && !properties.isEmpty()) {
+ log.warn("Forcing use of deprecated old-style location construction for "+getClass().getName()+" because properties were specified ("+properties+"); instead use specs (e.g. LocationSpec, EntitySpec, etc)");
+ if (log.isDebugEnabled())
+ log.debug("Source of use of old-style construction", new Throwable("Source of use of old-style construction"));
+ _legacyConstruction = true;
+ }
+
+ // rely on sub-class to call configure(properties), because otherwise its fields will not have been initialised
+ }
+
+ /**
+ * See {@link #configure(Map)}
+ *
+ * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
+ */
+ @Deprecated
+ protected AbstractBrooklynObject configure() {
+ return configure(Collections.emptyMap());
+ }
+
+ /**
+ * Will set fields from flags, and put the remaining ones into the 'leftovers' map.
+ * For some types, you can find unused config via {@link ConfigBag#getUnusedConfig()}.
+ * <p>
+ * To be overridden by AbstractEntity, AbstractLoation, AbstractPolicy, AbstractEnricher, etc.
+ * <p>
+ * But should not be overridden by specific entity types. If you do, the entity may break in
+ * subsequent releases. Also note that if you require fields to be initialized you must do that
+ * in this method. You must *not* rely on field initializers because they may not run until *after*
+ * this method (this method is invoked by the constructor in this class, so initializers
+ * in subclasses will not have run when this overridden method is invoked.)
+ *
+ * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
+ */
+ @Deprecated
+ protected AbstractBrooklynObject configure(Map flags) {
+ return this;
+ }
+
+ protected boolean isLegacyConstruction() {
+ return _legacyConstruction;
+ }
+
/**
* Called by framework (in new-style instances where spec was used) after configuring etc,
* but before a reference to this instance is shared.
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
index 0f1a184..729a2ff 100644
--- a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
+++ b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
@@ -40,7 +40,6 @@ import brooklyn.entity.Entity;
import brooklyn.entity.EntityType;
import brooklyn.entity.Group;
import brooklyn.entity.proxying.EntitySpec;
-import brooklyn.entity.proxying.InternalFactory;
import brooklyn.entity.rebind.BasicEntityRebindSupport;
import brooklyn.entity.rebind.RebindManagerImpl;
import brooklyn.entity.rebind.RebindSupport;
@@ -80,6 +79,7 @@ import brooklyn.util.collections.MutableMap;
import brooklyn.util.collections.SetFromLiveMap;
import brooklyn.util.config.ConfigBag;
import brooklyn.util.flags.FlagUtils;
+import brooklyn.util.flags.TypeCoercions;
import brooklyn.util.guava.Maybe;
import brooklyn.util.task.DeferredSupplier;
import brooklyn.util.text.Strings;
@@ -212,8 +212,6 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E
protected transient SubscriptionTracker _subscriptionTracker;
- private final boolean _legacyConstruction;
-
public AbstractEntity() {
this(Maps.newLinkedHashMap(), null);
}
@@ -240,8 +238,25 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E
*/
@Deprecated
public AbstractEntity(@SuppressWarnings("rawtypes") Map flags, Entity parent) {
+ super(checkConstructorFlags(flags, parent));
+
+ // TODO Don't let `this` reference escape during construction
+ entityType = new EntityDynamicType(this);
+
+ if (isLegacyConstruction()) {
+ AbstractBrooklynObject checkWeGetThis = configure(flags);
+ assert this.equals(checkWeGetThis) : this+" configure method does not return itself; returns "+checkWeGetThis+" instead of "+this;
+
+ boolean deferConstructionChecks = (flags.containsKey("deferConstructionChecks") && TypeCoercions.coerce(flags.get("deferConstructionChecks"), Boolean.class));
+ if (!deferConstructionChecks) {
+ FlagUtils.checkRequiredFields(this);
+ }
+ }
+ }
+
+ private static Map<?,?> checkConstructorFlags(Map flags, Entity parent) {
if (flags==null) {
- throw new IllegalArgumentException("Flags passed to entity "+this+" must not be null (try no-arguments or empty map)");
+ throw new IllegalArgumentException("Flags passed to entity must not be null (try no-arguments or empty map)");
}
if (flags.get("parent") != null && parent != null && flags.get("parent") != parent) {
throw new IllegalArgumentException("Multiple parents supplied, "+flags.get("parent")+" and "+parent);
@@ -256,77 +271,18 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E
flags.put("parent", parent);
}
if (flags.get("owner") != null) {
- LOG.warn("Use of deprecated \"flags.owner\" instead of \"flags.parent\" for entity {}", this);
+ LOG.warn("Use of deprecated \"flags.owner\" instead of \"flags.parent\" for entity");
flags.put("parent", flags.get("owner"));
flags.remove("owner");
}
-
- // TODO Don't let `this` reference escape during construction
- entityType = new EntityDynamicType(this);
-
- _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing();
-
- if (_legacyConstruction) {
- LOG.warn("Deprecated use of old-style entity construction for "+getClass().getName()+"; instead use EntityManager().createEntity(spec)");
- AbstractEntity checkWeGetThis = configure(flags);
- assert this.equals(checkWeGetThis) : this+" configure method does not return itself; returns "+checkWeGetThis+" instead";
- }
-
- inConstruction = false;
+ return flags;
}
- @Override
- public int hashCode() {
- return getId().hashCode();
- }
-
- @Override
- public boolean equals(Object o) {
- return (o == this || o == selfProxy) ||
- (o instanceof Entity && Objects.equal(getId(), ((Entity)o).getId()));
- }
-
- protected boolean isLegacyConstruction() {
- return _legacyConstruction;
- }
-
- protected boolean isRebinding() {
- return RebindManagerImpl.RebindTracker.isRebinding();
- }
-
- public void setProxy(Entity proxy) {
- if (selfProxy != null) throw new IllegalStateException("Proxy is already set; cannot reset proxy for "+toString());
- selfProxy = checkNotNull(proxy, "proxy");
- }
-
- public Entity getProxy() {
- return selfProxy;
- }
-
/**
- * Returns the proxy, or if not available (because using legacy code) then returns the real entity.
- * This method will be deleted in a future release; it will be kept while deprecated legacy code
- * still exists that creates entities without setting the proxy.
- */
- @Beta
- public Entity getProxyIfAvailable() {
- return getProxy()!=null ? getProxy() : this;
- }
-
- /** sets fields from flags; can be overridden if needed, subclasses should
- * set custom fields before _invoking_ this super
- * (and they nearly always should invoke the super)
- * <p>
- * note that it is usually preferred to use the SetFromFlag annotation on relevant fields
- * so they get set automatically by this method and overriding it is unnecessary
- *
- * @return this entity, for fluent style initialization
+ * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
*/
- public AbstractEntity configure() {
- return configure(Maps.newLinkedHashMap());
- }
-
@Override
+ @Deprecated
public AbstractEntity configure(Map flags) {
if (!inConstruction && getManagementSupport().isDeployed()) {
LOG.warn("bulk/flag configuration being made to {} after deployment: may not be supported in future versions ({})",
@@ -387,6 +343,40 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E
return this;
}
+
+ @Override
+ public int hashCode() {
+ return getId().hashCode();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return (o == this || o == selfProxy) ||
+ (o instanceof Entity && Objects.equal(getId(), ((Entity)o).getId()));
+ }
+
+ protected boolean isRebinding() {
+ return RebindManagerImpl.RebindTracker.isRebinding();
+ }
+
+ public void setProxy(Entity proxy) {
+ if (selfProxy != null) throw new IllegalStateException("Proxy is already set; cannot reset proxy for "+toString());
+ selfProxy = checkNotNull(proxy, "proxy");
+ }
+
+ public Entity getProxy() {
+ return selfProxy;
+ }
+
+ /**
+ * Returns the proxy, or if not available (because using legacy code) then returns the real entity.
+ * This method will be deleted in a future release; it will be kept while deprecated legacy code
+ * still exists that creates entities without setting the proxy.
+ */
+ @Beta
+ public Entity getProxyIfAvailable() {
+ return getProxy()!=null ? getProxy() : this;
+ }
/**
* Sets a config key value, and returns this Entity instance for use in fluent-API style coding.
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/AbstractLocation.java b/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
index 8aea917..734b1c8 100644
--- a/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
@@ -36,7 +36,6 @@ import brooklyn.basic.AbstractBrooklynObject;
import brooklyn.config.ConfigKey;
import brooklyn.config.ConfigKey.HasConfigKey;
import brooklyn.entity.basic.EntityDynamicType;
-import brooklyn.entity.proxying.InternalFactory;
import brooklyn.entity.rebind.BasicLocationRebindSupport;
import brooklyn.entity.rebind.RebindManagerImpl;
import brooklyn.entity.rebind.RebindSupport;
@@ -83,7 +82,7 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
public static final ConfigKey<Location> PARENT_LOCATION = new BasicConfigKey<Location>(Location.class, "parentLocation");
- private final AtomicBoolean configured = new AtomicBoolean(false);
+ private final AtomicBoolean configured = new AtomicBoolean();
private Reference<Long> creationTimeUtc = new BasicReference<Long>(System.currentTimeMillis());
@@ -102,8 +101,6 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
private volatile boolean managed;
- private boolean _legacyConstruction;
-
private boolean inConstruction;
private final Map<Class<?>, Object> extensions = Maps.newConcurrentMap();
@@ -137,12 +134,8 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
* </ul>
*/
public AbstractLocation(Map properties) {
+ super(properties);
inConstruction = true;
- _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing();
- if (!_legacyConstruction && properties!=null && !properties.isEmpty()) {
- LOG.warn("Forcing use of deprecated old-style location construction for "+getClass().getName()+" because properties were specified ("+properties+")");
- _legacyConstruction = true;
- }
// When one calls getConfig(key), we want to use the default value specified on *this* location
// if it overrides the default config. The easiest way to look up all our config keys is to
@@ -150,13 +143,10 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
// entities). See {@link #getConfig(ConfigKey)}
entityType = new EntityDynamicType((Class)getClass());
- if (_legacyConstruction) {
- LOG.warn("Deprecated use of old-style location construction for "+getClass().getName()+"; instead use LocationManager().createLocation(spec)");
- if (LOG.isDebugEnabled())
- LOG.debug("Source of use of old-style location construction", new Throwable("Source of use of old-style location construction"));
-
- configure(properties);
-
+ if (isLegacyConstruction()) {
+ AbstractBrooklynObject checkWeGetThis = configure(properties);
+ assert this.equals(checkWeGetThis) : this+" configure method does not return itself; returns "+checkWeGetThis+" instead of "+this;
+
boolean deferConstructionChecks = (properties.containsKey("deferConstructionChecks") && TypeCoercions.coerce(properties.get("deferConstructionChecks"), Boolean.class));
if (!deferConstructionChecks) {
FlagUtils.checkRequiredFields(this);
@@ -213,21 +203,11 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
}
/**
- * Will set fields from flags. The unused configuration can be found via the
- * {@linkplain ConfigBag#getUnusedConfig()}.
- * This can be overridden for custom initialization but note the following.
- * <p>
- * For new-style locations (i.e. not calling constructor directly, this will
- * be invoked automatically by brooklyn-core post-construction).
- * <p>
- * For legacy location use, this will be invoked by the constructor in this class.
- * Therefore if over-riding you must *not* rely on field initializers because they
- * may not run until *after* this method (this method is invoked by the constructor
- * in this class, so initializers in subclasses will not have run when this overridden
- * method is invoked.) If you require fields to be initialized you must do that in
- * this method with a guard (as in FixedListMachineProvisioningLocation).
- */
- public void configure(Map properties) {
+ * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
+ */
+ @Override
+ @Deprecated
+ public AbstractLocation configure(Map properties) {
assertNotYetManaged();
boolean firstTime = !configured.getAndSet(true);
@@ -269,6 +249,8 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
}
configBag.put(LocationConfigKeys.ISO_3166, codes);
}
+
+ return this;
}
// TODO ensure no callers rely on 'remove' semantics, and don't remove;
@@ -307,10 +289,6 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements
}
}
- protected boolean isLegacyConstruction() {
- return _legacyConstruction;
- }
-
@Override
public String getDisplayName() {
return name.get();
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java b/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java
index d50aa7b..dbb52c5 100644
--- a/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/AggregatingMachineProvisioningLocation.java
@@ -78,13 +78,13 @@ public class AggregatingMachineProvisioningLocation<T extends MachineLocation> e
}
@Override
- public void configure(Map properties) {
+ public AbstractLocation configure(Map properties) {
if (lock == null) {
lock = new Object();
provisioners = Lists.<MachineProvisioningLocation<T>>newArrayList();
inUse = Maps.<T, MachineProvisioningLocation<T>>newLinkedHashMap();
}
- super.configure(properties);
+ return super.configure(properties);
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java b/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java
index 595ab8a..263e307 100644
--- a/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/FixedListMachineProvisioningLocation.java
@@ -121,11 +121,11 @@ implements MachineProvisioningLocation<T>, Closeable {
}
@Override
- public void configure(Map properties) {
+ public AbstractLocation configure(Map properties) {
if (machines == null) machines = Sets.newLinkedHashSet();
if (inUse == null) inUse = Sets.newLinkedHashSet();
if (pendingRemoval == null) pendingRemoval = Sets.newLinkedHashSet();
- super.configure(properties);
+ return super.configure(properties);
}
public FixedListMachineProvisioningLocation<T> newSubLocation(Map<?,?> newFlags) {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java b/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java
index 1562f9f..64a9345 100644
--- a/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/LocalhostMachineProvisioningLocation.java
@@ -118,7 +118,7 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
return LocationSpec.create(LocalhostMachineProvisioningLocation.class);
}
- public void configure(Map flags) {
+ public LocalhostMachineProvisioningLocation configure(Map flags) {
super.configure(flags);
if (!truth(getDisplayName())) { setDisplayName("localhost"); }
@@ -142,6 +142,8 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
if (getConfig(BrooklynConfigKeys.ONBOX_BASE_DIR)==null && (getManagementContext()==null || getManagementContext().getConfig().getConfig(BrooklynConfigKeys.ONBOX_BASE_DIR)==null)) {
setConfig(BrooklynConfigKeys.ONBOX_BASE_DIR, "/tmp/brooklyn-"+Os.user());
}
+
+ return this;
}
public static InetAddress getLocalhostInetAddress() {
@@ -253,11 +255,13 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
public LocalhostMachine(Map properties) {
super(MutableMap.builder().putAll(properties).put("mutexSupport", mutexSupport).build());
}
+
public boolean obtainSpecificPort(int portNumber) {
if (!isSudoAllowed() && portNumber <= 1024)
return false;
return LocalhostMachineProvisioningLocation.obtainSpecificPort(getAddress(), portNumber);
}
+
public int obtainPort(PortRange range) {
int r = LocalhostMachineProvisioningLocation.obtainPort(getAddress(), range);
synchronized (portsObtained) {
@@ -266,6 +270,7 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
LOG.debug("localhost.obtainPort("+range+"), returning "+r);
return r;
}
+
@Override
public void releasePort(int portNumber) {
synchronized (portsObtained) {
@@ -280,10 +285,11 @@ public class LocalhostMachineProvisioningLocation extends FixedListMachineProvis
}
@Override
- public void configure(Map properties) {
+ public LocalhostMachine configure(Map properties) {
if (address==null || !properties.containsKey("address"))
address = Networking.getLocalHost();
super.configure(properties);
+ return this;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java b/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java
index 47376d5..a88218e 100644
--- a/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java
@@ -309,7 +309,7 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat
}
@Override
- public void configure(Map properties) {
+ public SshMachineLocation configure(Map properties) {
super.configure(properties);
// TODO Note that check for addresss!=null is done automatically in super-constructor, in FlagUtils.checkRequiredFields
@@ -325,6 +325,7 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat
setDisplayName((truth(user) ? user+"@" : "") + address.getHostName());
}
}
+ return this;
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java b/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
index a1e2449..a8cf150 100644
--- a/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
+++ b/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
@@ -39,14 +39,12 @@ import brooklyn.entity.Entity;
import brooklyn.entity.Group;
import brooklyn.entity.basic.EntityInternal;
import brooklyn.entity.basic.EntityLocal;
-import brooklyn.entity.proxying.InternalFactory;
import brooklyn.entity.rebind.RebindManagerImpl;
import brooklyn.entity.trait.Configurable;
import brooklyn.event.AttributeSensor;
import brooklyn.event.Sensor;
import brooklyn.event.SensorEventListener;
import brooklyn.management.ExecutionContext;
-import brooklyn.management.ManagementContext;
import brooklyn.management.SubscriptionContext;
import brooklyn.management.SubscriptionHandle;
import brooklyn.management.internal.SubscriptionTracker;
@@ -68,14 +66,9 @@ import com.google.common.collect.Maps;
public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject implements BrooklynObjectInternal, EntityAdjunct, Configurable {
private static final Logger log = LoggerFactory.getLogger(AbstractEntityAdjunct.class);
- protected Map<String,Object> leftoverProperties = Maps.newLinkedHashMap();
-
- private boolean _legacyConstruction;
private boolean _legacyNoConstructionInit;
-
- // TODO not sure if we need this -- never read
- @SuppressWarnings("unused")
- private boolean inConstruction;
+
+ protected Map<String,Object> leftoverProperties = Maps.newLinkedHashMap();
protected transient ExecutionContext execution;
@@ -101,43 +94,28 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple
this(Collections.emptyMap());
}
- public AbstractEntityAdjunct(@SuppressWarnings("rawtypes") Map flags) {
- inConstruction = true;
- _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing();
- _legacyNoConstructionInit = (flags != null) && Boolean.TRUE.equals(flags.get("noConstructionInit"));
-
- if (!_legacyConstruction && flags!=null && !flags.isEmpty()) {
- log.debug("Using direct construction for "+getClass().getName()+" because properties were specified ("+flags+")");
- _legacyConstruction = true;
- }
+ public AbstractEntityAdjunct(@SuppressWarnings("rawtypes") Map properties) {
+ super(properties);
+ _legacyNoConstructionInit = (properties != null) && Boolean.TRUE.equals(properties.get("noConstructionInit"));
- if (_legacyConstruction) {
- log.debug("Using direct construction for "+getClass().getName()+"; calling configure(Map) immediately");
-
- configure(flags);
-
- boolean deferConstructionChecks = (flags.containsKey("deferConstructionChecks") && TypeCoercions.coerce(flags.get("deferConstructionChecks"), Boolean.class));
+ if (isLegacyConstruction()) {
+ AbstractBrooklynObject checkWeGetThis = configure(properties);
+ assert this.equals(checkWeGetThis) : this+" configure method does not return itself; returns "+checkWeGetThis+" instead of "+this;
+
+ boolean deferConstructionChecks = (properties.containsKey("deferConstructionChecks") && TypeCoercions.coerce(properties.get("deferConstructionChecks"), Boolean.class));
if (!deferConstructionChecks) {
FlagUtils.checkRequiredFields(this);
}
}
-
- inConstruction = false;
}
- /** will set fields from flags, and put the remaining ones into the 'leftovers' map.
- * can be subclassed for custom initialization but note the following.
- * <p>
- * if you require fields to be initialized you must do that in this method. You must
- * *not* rely on field initializers because they may not run until *after* this method
- * (this method is invoked by the constructor in this class, so initializers
- * in subclasses will not have run when this overridden method is invoked.) */
- protected void configure() {
- configure(Collections.emptyMap());
- }
-
+ /**
+ * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
+ */
+ @Override
+ @Deprecated
@SuppressWarnings({ "unchecked", "rawtypes" })
- public void configure(Map flags) {
+ public AbstractEntityAdjunct configure(Map flags) {
// TODO only set on first time through
boolean isFirstTime = true;
@@ -172,12 +150,9 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple
Preconditions.checkArgument(flags.get("displayName") instanceof CharSequence, "'displayName' property should be a string");
setDisplayName(flags.remove("displayName").toString());
}
+ return this;
}
- protected boolean isLegacyConstruction() {
- return _legacyConstruction;
- }
-
/**
* Used for legacy-style policies/enrichers on rebind, to indicate that init() should not be called.
* Will likely be deleted in a future release; should not be called apart from by framework code.
@@ -186,26 +161,7 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple
protected boolean isLegacyNoConstructionInit() {
return _legacyNoConstructionInit;
}
-
- /**
- * Called by framework (in new-style policies where PolicySpec was used) after configuring etc,
- * but before a reference to this policy is shared.
- *
- * To preserve backwards compatibility for if the policy is constructed directly, one
- * can call the code below, but that means it will be called after references to this
- * policy have been shared with other entities.
- * <pre>
- * {@code
- * if (isLegacyConstruction()) {
- * init();
- * }
- * }
- * </pre>
- */
- public void init() {
- // no-op
- }
-
+
protected boolean isRebinding() {
return RebindManagerImpl.RebindTracker.isRebinding();
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/6cb55b50/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
index 0290dc5..380e351 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
@@ -93,6 +93,7 @@ import brooklyn.location.MachineManagementMixins.MachineMetadata;
import brooklyn.location.MachineManagementMixins.RichMachineProvisioningLocation;
import brooklyn.location.NoMachinesAvailableException;
import brooklyn.location.access.PortForwardManager;
+import brooklyn.location.basic.AbstractLocation;
import brooklyn.location.basic.BasicMachineMetadata;
import brooklyn.location.basic.LocationConfigKeys;
import brooklyn.location.basic.LocationConfigUtils;
@@ -206,7 +207,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
}
@Override
- public void configure(Map properties) {
+ public JcloudsLocation configure(Map properties) {
super.configure(properties);
if (getLocalConfigBag().containsKey("providerLocationId")) {
@@ -231,6 +232,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
}
setConfig(MACHINE_CREATION_SEMAPHORE, new Semaphore(maxConcurrent, true));
}
+ return this;
}
@Override