share code in AbstractConfigurationSupport, and fix location config issues from previous
support clearing, prevent writes to config bag, and allow coercion from class names Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/8405d877 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/8405d877 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/8405d877 Branch: refs/heads/master Commit: 8405d8774a0380a99b9dd1fbd2daf7fc15265cb8 Parents: ddc17d1 Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Tue Sep 20 13:07:22 2016 +0100 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Wed Sep 21 16:06:06 2016 +0100 ---------------------------------------------------------------------- .../config/internal/AbstractConfigMapImpl.java | 16 ++-- .../brooklyn/core/entity/AbstractEntity.java | 89 +++----------------- .../core/entity/internal/EntityConfigMap.java | 2 +- .../core/location/AbstractLocation.java | 72 +++------------- .../location/internal/LocationConfigMap.java | 11 ++- .../mgmt/internal/LocalLocationManager.java | 6 +- .../mgmt/rebind/BasicLocationRebindSupport.java | 8 +- .../AbstractConfigurationSupportInternal.java | 82 ++++++++++++++++++ .../core/objs/AbstractEntityAdjunct.java | 83 +++++------------- .../brooklyn/core/objs/AdjunctConfigMap.java | 2 +- .../core/objs/BrooklynObjectInternal.java | 3 + .../jclouds/JcloudsByonLocationResolver.java | 3 +- .../location/jclouds/JcloudsLocation.java | 26 +++--- .../jclouds/JcloudsLocationResolverTest.java | 34 ++++++-- 14 files changed, 193 insertions(+), 244 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java index 60760f6..6593aed 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java @@ -74,7 +74,7 @@ public abstract class AbstractConfigMapImpl implements ConfigMap { this.ownConfig = storage; } - protected BrooklynObjectInternal getBrooklynObject() { + public BrooklynObjectInternal getBrooklynObject() { return bo; } @@ -189,8 +189,8 @@ public abstract class AbstractConfigMapImpl implements ConfigMap { } public void removeFromLocalBag(String key) { - // FIXME probably never worked, config key vs string ? - ownConfig.remove(key); + // previously removed the string (?) + ownConfig.remove(ConfigKeys.newConfigKey(Object.class, key)); } public void removeFromLocalBag(ConfigKey<?> key) { @@ -228,21 +228,20 @@ public abstract class AbstractConfigMapImpl implements ConfigMap { return result.seal(); } protected Object coerceConfigVal(ConfigKey<?> key, Object v) { - Object val; if ((v instanceof Future) || (v instanceof DeferredSupplier)) { // no coercion for these (coerce on exit) - val = v; + return v; } else if (key instanceof StructuredConfigKey) { // no coercion for these structures (they decide what to do) - val = v; + return v; } else if ((v instanceof Map || v instanceof Iterable) && key.getType().isInstance(v)) { // don't do coercion on put for these, if the key type is compatible, // because that will force resolution deeply - val = v; + return v; } else { try { // try to coerce on input, to detect errors sooner - val = TypeCoercions.coerce(v, key.getTypeToken()); + return TypeCoercions.coerce(v, key.getTypeToken()); } catch (Exception e) { throw new IllegalArgumentException("Cannot coerce or set "+v+" to "+key, e); // if can't coerce, we could just log as below and *throw* the error when we retrieve the config @@ -253,7 +252,6 @@ public abstract class AbstractConfigMapImpl implements ConfigMap { // val = v; } } - return val; } @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java index 663c0ac..fe65cff 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java @@ -56,7 +56,6 @@ import org.apache.brooklyn.api.sensor.SensorEvent; import org.apache.brooklyn.api.sensor.SensorEventListener; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; -import org.apache.brooklyn.config.ConfigMap; import org.apache.brooklyn.core.BrooklynFeatureEnablement; import org.apache.brooklyn.core.BrooklynLogging; import org.apache.brooklyn.core.catalog.internal.CatalogUtils; @@ -100,7 +99,6 @@ import org.apache.brooklyn.util.collections.SetFromLiveMap; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.TypeCoercions; -import org.apache.brooklyn.util.core.task.DeferredSupplier; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.javalang.Equals; import org.apache.brooklyn.util.text.Strings; @@ -1209,65 +1207,13 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E // TODO revert to private when config() is reverted to return ConfigurationSupportInternal public class BasicConfigurationSupport extends AbstractConfigurationSupportInternal { - protected AbstractConfigMapImpl getConfigsInternal() { - return configsInternal; - } - - @Override - public <T> T get(ConfigKey<T> key) { - return getConfigsInternal().getConfig(key); - } - @Override - public <T> T set(ConfigKey<T> key, T val) { + protected <T> void assertValid(ConfigKey<T> key, T val) { ConfigConstraints.assertValid(AbstractEntity.this, key, val); - return setConfigInternal(key, val); - } - - @Override - public <T> T set(ConfigKey<T> key, Task<T> val) { - return setConfigInternal(key, val); - } - - @Override - public ConfigBag getLocalBag() { - return getConfigsInternal().getLocalConfigBag(); - } - - @Override - public Maybe<Object> getRaw(ConfigKey<?> key) { - return getConfigsInternal().getConfigRaw(key, true); - } - - @Override - public Maybe<Object> getLocalRaw(ConfigKey<?> key) { - return getConfigsInternal().getConfigRaw(key, false); - } - - @Override - public void addToLocalBag(Map<?, ?> vals) { - getConfigsInternal().addToLocalBag(vals); - } - - @Override - public void removeFromLocalBag(String key) { - getConfigsInternal().removeFromLocalBag(key); } - @Override - public void removeFromLocalBag(ConfigKey<?> key) { - getConfigsInternal().removeFromLocalBag(key); - } - - @Override - public ConfigMap getInternalConfigMap() { - return getConfigsInternal(); - } - - @Override - // TODO deprecate because key inheritance not respected - public ConfigBag getBag() { - return ((EntityConfigMap)getConfigsInternal()).getAllConfigBag(); + protected AbstractConfigMapImpl getConfigsInternal() { + return configsInternal; } @Override @@ -1284,22 +1230,23 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E } } - @SuppressWarnings("unchecked") - private <T> T setConfigInternal(ConfigKey<T> key, Object val) { + @Override + protected <T> void onConfigChanging(ConfigKey<T> key, Object val) { if (!inConstruction && getManagementSupport().isDeployed()) { // previously we threw, then warned, but it is still quite common; // so long as callers don't expect miracles, it should be fine. // i (Alex) think the way to be stricter about this (if that becomes needed) // would be to introduce a 'mutable' field on config keys LOG.debug("configuration being made to {} after deployment: {} = {}; change may not be visible in other contexts", - new Object[] { getContainer(), key, val }); + new Object[] { getContainer(), key, val }); } - T result = (T) getConfigsInternal().setConfig(key, val); - + } + + protected <T> void onConfigChanged(ConfigKey<T> key, Object val) { getManagementSupport().getEntityChangeListener().onConfigChanged(key); - return result; } + @Override protected BrooklynObject getContainer() { return AbstractEntity.this; } @@ -1351,14 +1298,6 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E return config().set(key, val); } - /** - * @deprecated since 0.7.0; use {@code config().set(key, task)}, with {@link Task} instead of {@link DeferredSupplier} - */ - @Deprecated - public <T> T setConfig(ConfigKey<T> key, DeferredSupplier val) { - return config.setConfigInternal(key, val); - } - @Override @Deprecated public <T> T setConfig(HasConfigKey<T> key, T val) { @@ -1371,14 +1310,6 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E return (T) config().set(key, val); } - /** - * @deprecated since 0.7.0; use {@code config().set(key, task)}, with {@link Task} instead of {@link DeferredSupplier} - */ - @Deprecated - public <T> T setConfig(HasConfigKey<T> key, DeferredSupplier val) { - return setConfig(key.getConfigKey(), val); - } - @SuppressWarnings("unchecked") public <T> T setConfigEvenIfOwned(ConfigKey<T> key, T val) { return (T) configsInternal.setConfig(key, val); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java index e2ff0be..c1314b3 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java @@ -62,7 +62,7 @@ public class EntityConfigMap extends AbstractConfigMapImpl { * @deprecated since 0.10.0 kept for serialization */ @Deprecated private EntityInternal entity; @Override - protected BrooklynObjectInternal getBrooklynObject() { + public BrooklynObjectInternal getBrooklynObject() { BrooklynObjectInternal result = super.getBrooklynObject(); if (result!=null) return result; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java index f89ae27..12253b6 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java @@ -38,6 +38,7 @@ import org.apache.brooklyn.api.mgmt.SubscriptionHandle; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.mgmt.rebind.RebindSupport; import org.apache.brooklyn.api.mgmt.rebind.mementos.LocationMemento; +import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.Configurable; import org.apache.brooklyn.api.sensor.Sensor; import org.apache.brooklyn.api.sensor.SensorEventListener; @@ -48,6 +49,7 @@ import org.apache.brooklyn.core.BrooklynFeatureEnablement; import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.internal.storage.BrooklynStorage; import org.apache.brooklyn.core.internal.storage.Reference; import org.apache.brooklyn.core.internal.storage.impl.BasicReference; @@ -387,72 +389,21 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements // Sept 2016 now uses AbstractConfigMapImpl like the other ConfigurationSupport implementations // (now gives us inheritance correctly -- for free!) - - protected LocationConfigMap getConfigMap() { - return configMap; - } - - @Override - public <T> T get(ConfigKey<T> key) { - return getConfigMap().getConfig(key); - - } @Override - public <T> T set(ConfigKey<T> key, T val) { + protected <T> void assertValid(ConfigKey<T> key, T val) { ConfigConstraints.assertValid(AbstractLocation.this, key, val); - @SuppressWarnings("unchecked") - T result = (T) getConfigMap().setConfig(key, val); - onChanged(); - return result; - } - - @Override - public <T> T set(ConfigKey<T> key, Task<T> val) { - // TODO Support for locations - throw new UnsupportedOperationException(); - } - - @Override - public ConfigBag getBag() { - ConfigBag result = getLocalBag(); - Location p = getParent(); - if (p!=null) result.putIfAbsent(((LocationInternal)p).config().getBag()); - return result; - } - - @Override - public ConfigBag getLocalBag() { - ConfigBag result = ConfigBag.newInstance(); - result.putAll(getConfigMap().getLocalConfig()); - return result; } @Override - public Maybe<Object> getRaw(ConfigKey<?> key) { - return getConfigMap().getConfigRaw(key, true); - } - - @Override - public Maybe<Object> getLocalRaw(ConfigKey<?> key) { - return getConfigMap().getConfigLocalRaw(key); - } - - @Override - public void addToLocalBag(Map<?, ?> vals) { - getConfigMap().addToLocalBag(vals); - } - - @Override - public void removeFromLocalBag(String key) { - getConfigMap().removeFromLocalBag(key); + protected <T> void onConfigChanging(ConfigKey<T> key, Object val) { } @Override - public void removeFromLocalBag(ConfigKey<?> key) { - getConfigMap().removeFromLocalBag(key); + protected <T> void onConfigChanged(ConfigKey<T> key, Object val) { + onChanged(); } - + @Override public void refreshInheritedConfig() { // no-op for location @@ -467,10 +418,15 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements protected ExecutionContext getContext() { return AbstractLocation.this.getManagementContext().getServerExecutionContext(); } + + @Override + protected AbstractConfigMapImpl getConfigsInternal() { + return configMap; + } @Override - public ConfigMap getInternalConfigMap() { - return getConfigMap(); + protected BrooklynObject getContainer() { + return AbstractLocation.this; } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java index 0e79668..4a854f6 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java @@ -132,5 +132,14 @@ public class LocationConfigMap extends AbstractConfigMapImpl { throw new UnsupportedOperationException("Location does not support submap"); } - + @Override + protected Object coerceConfigVal(ConfigKey<?> key, Object v) { + if ((Class.class.isAssignableFrom(key.getType()) || Function.class.isAssignableFrom(key.getType())) && v instanceof String) { + // strings can be written where classes/functions are permitted; this is a common pattern when configuring locations + // (bit sloppy to allow this; tests catch it, eg ImageChooser in jclouds) + return v; + } + + return super.coerceConfigVal(key, v); + } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalLocationManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalLocationManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalLocationManager.java index d73d2ce..709369d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalLocationManager.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalLocationManager.java @@ -406,9 +406,9 @@ public class LocalLocationManager implements LocationManagerInternal { // if not destroying, don't change the parent's children list ((AbstractLocation)loc).setParent(null, false); } - // clear config to help with GC; i know you're not supposed to, but this seems to help, else config bag is littered with refs to entities etc - // FIXME relies on config().getLocalBag() returning the underlying bag! - ((AbstractLocation)loc).config().getLocalBag().clear(); + // clear config to help with GC; everyone says you're not supposed to, but this really seems to help, + // else config bag is littered with refs to entities etc and some JVMs seem to keep them around much longer + ((AbstractLocation)loc).config().removeAllLocalConfig(); } /** http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java index 0f10672..9036e36 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicLocationRebindSupport.java @@ -35,8 +35,6 @@ import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.collect.Sets; - public class BasicLocationRebindSupport extends AbstractBrooklynObjectRebindSupport<LocationMemento> { private static final Logger LOG = LoggerFactory.getLogger(BasicLocationRebindSupport.class); @@ -70,11 +68,9 @@ public class BasicLocationRebindSupport extends AbstractBrooklynObjectRebindSupp // FIXME Treat config like we do for entities; this code will disappear when locations become entities. // Note that the flags have been set in the constructor - // FIXME Relies on location.config().getLocalBag() being mutable (to modify the location's own config) + // Sept 2016 - now ignores unused and config description - location.config().getLocalBag().putAll(memento.getLocationConfig()).markAll( - Sets.difference(memento.getLocationConfig().keySet(), memento.getLocationConfigUnused())). - setDescription(memento.getLocationConfigDescription()); + location.config().addToLocalBag(memento.getLocationConfig()); for (Map.Entry<String, Object> entry : memento.getLocationConfig().entrySet()) { String flagName = entry.getKey(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java index 9cb7a9e..2e18fc0 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java @@ -19,6 +19,7 @@ package org.apache.brooklyn.core.objs; +import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; @@ -27,12 +28,17 @@ import javax.annotation.Nullable; import org.apache.brooklyn.api.mgmt.ExecutionContext; import org.apache.brooklyn.api.mgmt.Task; +import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; +import org.apache.brooklyn.config.ConfigMap; import org.apache.brooklyn.core.config.MapConfigKey; import org.apache.brooklyn.core.config.StructuredConfigKey; import org.apache.brooklyn.core.config.SubElementConfigKey; +import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.core.task.Tasks; import org.apache.brooklyn.util.core.task.ValueResolver; @@ -149,6 +155,82 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb return set(key.getConfigKey(), val); } + protected abstract AbstractConfigMapImpl getConfigsInternal(); + protected abstract <T> void assertValid(ConfigKey<T> key, T val); + protected abstract BrooklynObject getContainer(); + protected abstract <T> void onConfigChanging(ConfigKey<T> key, Object val); + protected abstract <T> void onConfigChanged(ConfigKey<T> key, Object val); + + @Override + public <T> T get(ConfigKey<T> key) { + return getConfigsInternal().getConfig(key); + } + + @SuppressWarnings("unchecked") + protected <T> T setConfigInternal(ConfigKey<T> key, Object val) { + onConfigChanging(key, val); + T result = (T) getConfigsInternal().setConfig(key, val); + onConfigChanged(key, val); + return result; + } + + @Override + public <T> T set(ConfigKey<T> key, T val) { + assertValid(key, val); + return setConfigInternal(key, val); + } + + @Override + public <T> T set(ConfigKey<T> key, Task<T> val) { + return setConfigInternal(key, val); + } + + @Override + public ConfigBag getLocalBag() { + return getConfigsInternal().getLocalConfigBag(); + } + + @Override + public Maybe<Object> getRaw(ConfigKey<?> key) { + return getConfigsInternal().getConfigRaw(key, true); + } + + @Override + public Maybe<Object> getLocalRaw(ConfigKey<?> key) { + return getConfigsInternal().getConfigRaw(key, false); + } + + @Override + public void addToLocalBag(Map<?, ?> vals) { + getConfigsInternal().addToLocalBag(vals); + } + + @Override + public void removeFromLocalBag(String key) { + getConfigsInternal().removeFromLocalBag(key); + } + + @Override + public void removeFromLocalBag(ConfigKey<?> key) { + getConfigsInternal().removeFromLocalBag(key); + } + + @Override + public void removeAllLocalConfig() { + getConfigsInternal().setLocalConfig(MutableMap.<ConfigKey<?>,Object>of()); + } + + @Override + public ConfigMap getInternalConfigMap() { + return getConfigsInternal(); + } + + @Override + // TODO deprecate because key inheritance not respected + public ConfigBag getBag() { + return getConfigsInternal().getAllConfigBag(); + } + /** * @return An execution context for use by {@link #getNonBlocking(ConfigKey)} */ http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java index dd05c4d..c231157 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java @@ -33,7 +33,6 @@ import org.apache.brooklyn.api.entity.EntityLocal; import org.apache.brooklyn.api.entity.Group; import org.apache.brooklyn.api.mgmt.ExecutionContext; import org.apache.brooklyn.api.mgmt.SubscriptionHandle; -import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.Configurable; import org.apache.brooklyn.api.objs.EntityAdjunct; @@ -44,6 +43,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigMap; import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityInternal; @@ -52,7 +52,6 @@ import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.core.flags.TypeCoercions; -import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,11 +86,8 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple /** * The config values of this entity. Updating this map should be done * via {@link #config()}. - * - * @deprecated since 0.7.0; use {@link #config()} instead; this field may be made private or deleted in a future release. */ - @Deprecated - protected final AdjunctConfigMap configsInternal = new AdjunctConfigMap(this); + private final AdjunctConfigMap configsInternal = new AdjunctConfigMap(this); /** * @deprecated since 0.7.0; use {@link #getAdjunctType()} instead; this field may be made private or deleted in a future release. @@ -286,78 +282,27 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple private class BasicConfigurationSupport extends AbstractConfigurationSupportInternal { - @Override - public <T> T get(ConfigKey<T> key) { - return configsInternal.getConfig(key); - } - - @SuppressWarnings("unchecked") - @Override - public <T> T set(ConfigKey<T> key, T val) { - ConfigConstraints.assertValid(entity, key, val); - if (entity != null && isRunning()) { - doReconfigureConfig(key, val); - } - T result = (T) configsInternal.setConfig(key, val); - onChanged(); - return result; - } - @SuppressWarnings("unchecked") @Override - public <T> T set(ConfigKey<T> key, Task<T> val) { + protected <T> void onConfigChanging(ConfigKey<T> key, Object val) { if (entity != null && isRunning()) { - // TODO Support for AbstractEntityAdjunct - throw new UnsupportedOperationException(); + doReconfigureConfig(key, (T)val); } - T result = (T) configsInternal.setConfig(key, val); - onChanged(); - return result; - } - - @Override - public ConfigBag getBag() { - return getLocalBag(); - } - - @Override - public ConfigBag getLocalBag() { - return ConfigBag.newInstance(configsInternal.getAllConfig()); - } - - @Override - public Maybe<Object> getRaw(ConfigKey<?> key) { - return configsInternal.getConfigRaw(key, true); - } - - @Override - public Maybe<Object> getLocalRaw(ConfigKey<?> key) { - return configsInternal.getConfigRaw(key, false); - } - - @Override - public void addToLocalBag(Map<?, ?> vals) { - configsInternal.addToLocalBag(vals); } @Override - public void removeFromLocalBag(String key) { - configsInternal.removeFromLocalBag(key); + protected <T> void onConfigChanged(ConfigKey<T> key, Object val) { + onChanged(); } @Override - public void removeFromLocalBag(ConfigKey<?> key) { - configsInternal.removeFromLocalBag(key); - } - - @Override public void refreshInheritedConfig() { - // no-op for location + // no-op here } @Override public void refreshInheritedConfigOfChildren() { - // no-op for location + // no-op here } @Override @@ -366,9 +311,19 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple } @Override - public ConfigMap getInternalConfigMap() { + protected AbstractConfigMapImpl getConfigsInternal() { return configsInternal; } + + @Override + protected <T> void assertValid(ConfigKey<T> key, T val) { + ConfigConstraints.assertValid(entity, key, val); + } + + @Override + protected BrooklynObject getContainer() { + return AbstractEntityAdjunct.this; + } } @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java index 7ad9468..f3ecdea 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AdjunctConfigMap.java @@ -49,7 +49,7 @@ public class AdjunctConfigMap extends AbstractConfigMapImpl { * @deprecated since 0.10.0 kept for serialization */ @Deprecated private AbstractEntityAdjunct adjunct; @Override - protected BrooklynObjectInternal getBrooklynObject() { + public BrooklynObjectInternal getBrooklynObject() { BrooklynObjectInternal result = super.getBrooklynObject(); if (result!=null) return result; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java index 600104b..566f957 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java @@ -142,6 +142,9 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { @Beta ConfigMap getInternalConfigMap(); + + /** Clears all local config, e.g. on tear-down */ + void removeAllLocalConfig(); } @Beta http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java index 04c1126..f9a75be 100644 --- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java +++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java @@ -154,7 +154,8 @@ public class JcloudsByonLocationResolver extends AbstractLocationResolver implem JcloudsLocation jcloudsLocation = (JcloudsLocation) managementContext.getLocationManager().createLocation(jcloudsLocationSpec.get()); for (Map<?,?> machineFlags : machinesFlags) { try { - JcloudsMachineLocation machine = jcloudsLocation.registerMachine(jcloudsLocation.config().getBag().putAll(machineFlags)); + jcloudsLocation.config().addToLocalBag(machineFlags); + JcloudsMachineLocation machine = jcloudsLocation.registerMachine(jcloudsLocation.config().getBag()); result.add(machine); } catch (NoMachinesAvailableException e) { Map<?,?> sanitizedMachineFlags = Sanitizer.sanitize(machineFlags); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java index a1d1293..65ea8ec 100644 --- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java +++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java @@ -258,8 +258,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im (groovyTruth(getEndpoint()) ? ":"+getEndpoint() : "")); } - setCreationString(config().getLocalBag()); - if (getConfig(MACHINE_CREATION_SEMAPHORE) == null) { Integer maxConcurrent = getConfig(MAX_CONCURRENT_MACHINE_CREATIONS); if (maxConcurrent == null || maxConcurrent < 1) { @@ -583,13 +581,14 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im killMachine(m.getId()); } - /** attaches a string describing where something is being created - * (provider, region/location and/or endpoint, callerContext) */ - protected void setCreationString(ConfigBag config) { - config.setDescription(elvis(config.get(CLOUD_PROVIDER), "unknown")+ + /** can generate a string describing where something is being created + * (provider, region/location and/or endpoint, callerContext); + * previously set on the config bag, but not any longer (Sept 2016) as config is treated like entities */ + protected String getCreationString(ConfigBag config) { + return elvis(config.get(CLOUD_PROVIDER), "unknown")+ (config.containsKey(CLOUD_REGION_ID) ? ":"+config.get(CLOUD_REGION_ID) : "")+ (config.containsKey(CLOUD_ENDPOINT) ? ":"+config.get(CLOUD_ENDPOINT) : "")+ - (config.containsKey(CALLER_CONTEXT) ? "@"+config.get(CALLER_CONTEXT) : "")); + (config.containsKey(CALLER_CONTEXT) ? "@"+config.get(CALLER_CONTEXT) : ""); } // ----------------- obtaining a new machine ------------------------ @@ -651,7 +650,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im throw new IllegalStateException("Access controller forbids provisioning in "+this+": "+access.getMsg()); } - setCreationString(setup); boolean waitForSshable = !"false".equalsIgnoreCase(setup.get(WAIT_FOR_SSHABLE)); boolean waitForWinRmable = !"false".equalsIgnoreCase(setup.get(WAIT_FOR_WINRM_AVAILABLE)); boolean usePortForwarding = setup.get(USE_PORT_FORWARDING); @@ -728,10 +726,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im LOG.debug("jclouds using template {} / options {} to provision machine in {}", new Object[] {template, template.getOptions(), setup.getDescription()}); - if (!setup.getUnusedConfig().isEmpty()) - if (LOG.isDebugEnabled()) - LOG.debug("NOTE: unused flags passed to obtain VM in "+setup.getDescription()+": " - + Sanitizer.sanitize(setup.getUnusedConfig())); + // no longer supported because config is sealed, we use an underlying config map +// if (!setup.getUnusedConfig().isEmpty()) +// if (LOG.isDebugEnabled()) +// LOG.debug("NOTE: unused flags passed to obtain VM in "+setup.getDescription()+": " +// + Sanitizer.sanitize(setup.getUnusedConfig())); nodes = computeService.createNodesInGroup(groupId, 1, template); provisionTimestamp = Duration.of(provisioningStopwatch); } finally { @@ -2275,9 +2274,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im * @return */ protected NodeMetadata findNodeOrThrow(ConfigBag config) { - if (config.getDescription() == null) { - setCreationString(config); - } String user = checkNotNull(getUser(config), "user"); String rawId = (String) config.getStringKey("id"); String rawHostname = (String) config.getStringKey("hostname"); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8405d877/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationResolverTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationResolverTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationResolverTest.java index 6b568de..e18fd66 100644 --- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationResolverTest.java +++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationResolverTest.java @@ -345,8 +345,7 @@ public class JcloudsLocationResolverTest { @Test public void testResolvesListAndMapPropertiesWithoutMergeOnInheritance() throws Exception { - // when we have a yaml way to specify config we may wish to have different semantics; - // it could depend on the collection config key whether to merge on inheritance + // since prop2 does not specify DEEP_MERGE config inheritance, we overwrite brooklynProperties.put("brooklyn.location.jclouds.softlayer.prop1", "[ a, b ]"); brooklynProperties.put("brooklyn.location.jclouds.softlayer.prop2", "{ a: 1, b: 2 }"); brooklynProperties.put("brooklyn.location.named.foo", "jclouds:softlayer:ams01"); @@ -356,10 +355,6 @@ public class JcloudsLocationResolverTest { brooklynProperties.put("brooklyn.location.named.bar", "named:foo"); brooklynProperties.put("brooklyn.location.named.bar.prop2", "{ c: 4, d: 4 }"); - // these do NOT affect the maps - brooklynProperties.put("brooklyn.location.named.foo.prop2.z", "9"); - brooklynProperties.put("brooklyn.location.named.foo.prop3.z", "9"); - JcloudsLocation l = resolve("named:bar"); assertJcloudsEquals(l, "softlayer", "ams01"); @@ -376,6 +371,33 @@ public class JcloudsLocationResolverTest { assertEquals(prop3, null); } + @Test + public void testResolvesListAndMapPropertiesMergesWithDotQualifiedKeys() throws Exception { + brooklynProperties.put("brooklyn.location.jclouds.softlayer.prop1", "[ a, b ]"); + brooklynProperties.put("brooklyn.location.jclouds.softlayer.prop2", "{ a: 1, b: 2 }"); + brooklynProperties.put("brooklyn.location.named.foo", "jclouds:softlayer:ams01"); + + brooklynProperties.put("brooklyn.location.named.foo.prop1", "[ a: 1, c: 3 ]"); + brooklynProperties.put("brooklyn.location.named.foo.prop2", "{ b: 3, c: 3 }"); + brooklynProperties.put("brooklyn.location.named.bar", "named:foo"); + brooklynProperties.put("brooklyn.location.named.bar.prop2", "{ c: 4, d: 4 }"); + + // dot-qualified keys now DO get interpreted (sept 2016) + brooklynProperties.put("brooklyn.location.named.foo.prop2.z", 9); + brooklynProperties.put("brooklyn.location.named.foo.prop3.z", 10); + + JcloudsLocation l = resolve("named:bar"); + assertJcloudsEquals(l, "softlayer", "ams01"); + + Map<String, String> prop2 = l.config().get(new MapConfigKey<String>(String.class, "prop2")); + log.info("prop2: "+prop2); + assertEquals(prop2, MutableMap.of("c", 4, "d", 4, "z", "9")); + + Map<String, String> prop3 = l.config().get(new MapConfigKey<String>(String.class, "prop3")); + log.info("prop3: "+prop3); + assertEquals(prop3, MutableMap.of("z", "10")); + } + private void assertJcloudsEquals(JcloudsLocation loc, String expectedProvider, String expectedRegion) { assertEquals(loc.getProvider(), expectedProvider); assertEquals(loc.getRegion(), expectedRegion);
