Configurable.config(): incorporate review comments
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/1ed5484e Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/1ed5484e Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/1ed5484e Branch: refs/heads/master Commit: 1ed5484e07b580e1daca7cbd3ce8ba9f9c2243fd Parents: 73c4985 Author: Aled Sage <[email protected]> Authored: Thu Jan 8 23:04:09 2015 +0000 Committer: Aled Sage <[email protected]> Committed: Wed Feb 18 11:02:35 2015 +0000 ---------------------------------------------------------------------- .../java/brooklyn/basic/BrooklynObject.java | 57 +----------------- .../java/brooklyn/entity/basic/EntityLocal.java | 2 +- .../brooklyn/entity/trait/Configurable.java | 59 ++++++++++++++++++- .../brooklyn/basic/BasicConfigurableObject.java | 55 +++++++++++++++--- .../catalog/internal/CatalogItemDo.java | 6 ++ .../internal/CatalogItemDtoAbstract.java | 8 ++- .../access/PortForwardManagerClient.java | 10 ++++ ...DynamicClusterWithAvailabilityZonesTest.java | 1 + .../brooklyn/util/internal/FlagUtilsTest.java | 45 ++++++++++++++- .../brooklyn/camp/brooklyn/ObjectsYamlTest.java | 61 +++++++++++++++++--- 10 files changed, 230 insertions(+), 74 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/api/src/main/java/brooklyn/basic/BrooklynObject.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/brooklyn/basic/BrooklynObject.java b/api/src/main/java/brooklyn/basic/BrooklynObject.java index 798d454..cb80695 100644 --- a/api/src/main/java/brooklyn/basic/BrooklynObject.java +++ b/api/src/main/java/brooklyn/basic/BrooklynObject.java @@ -22,18 +22,15 @@ import java.util.Set; import javax.annotation.Nonnull; -import brooklyn.config.ConfigKey; -import brooklyn.config.ConfigKey.HasConfigKey; +import brooklyn.entity.trait.Configurable; import brooklyn.entity.trait.Identifiable; -import brooklyn.management.Task; -import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableMap; /** * Super-type of entity, location, policy and enricher. */ -public interface BrooklynObject extends Identifiable { +public interface BrooklynObject extends Identifiable, Configurable { /** * A display name; recommended to be a concise single-line description. @@ -67,8 +64,6 @@ public interface BrooklynObject extends Identifiable { @Deprecated TagSupport getTagSupport(); - ConfigurationSupport config(); - public interface TagSupport { /** * @return An immutable copy of the set of tags on this entity. @@ -85,52 +80,4 @@ public interface BrooklynObject extends Identifiable { boolean removeTag(@Nonnull Object tag); } - - @Beta - public interface ConfigurationSupport { - - /** - * Gets the given configuration value for this entity, in the following order of preference: - * <ol> - * <li> value (including null) explicitly set on the entity - * <li> value (including null) explicitly set on an ancestor (inherited) - * <li> a default value (including null) on the best equivalent static key of the same name declared on the entity - * (where best equivalence is defined as preferring a config key which extends another, - * as computed in EntityDynamicType.getConfigKeys) - * <li> a default value (including null) on the key itself - * <li> null - * </ol> - */ - <T> T get(ConfigKey<T> key); - - /** - * @see {@link #getConfig(ConfigKey)} - */ - <T> T get(HasConfigKey<T> key); - - /** - * Sets the config to the given value. - */ - <T> T set(ConfigKey<T> key, T val); - - /** - * @see {@link #setConfig(HasConfigKey, Object)} - */ - <T> T set(HasConfigKey<T> key, T val); - - /** - * Sets the config to the value returned by the task. - * - * Returns immediately without blocking; subsequent calls to {@link #getConfig(ConfigKey)} - * will execute the task, and block until the task completes. - * - * @see {@link #setConfig(ConfigKey, Object)} - */ - <T> T set(ConfigKey<T> key, Task<T> val); - - /** - * @see {@link #setConfig(ConfigKey, Task)} - */ - <T> T set(HasConfigKey<T> key, Task<T> val); - } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/api/src/main/java/brooklyn/entity/basic/EntityLocal.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/brooklyn/entity/basic/EntityLocal.java b/api/src/main/java/brooklyn/entity/basic/EntityLocal.java index 266e572..dd7affc 100644 --- a/api/src/main/java/brooklyn/entity/basic/EntityLocal.java +++ b/api/src/main/java/brooklyn/entity/basic/EntityLocal.java @@ -48,7 +48,7 @@ import com.google.common.base.Function; * FIXME Add {@link setAttribute(AttributeSensorAndConfigKey<?,T>)} back in if/when move it back, * or if we extract an interface for AttributeSensorAndConfigKey. */ -public interface EntityLocal extends Entity, Configurable { +public interface EntityLocal extends Entity { // FIXME Rename to something other than EntityLocal. // Separate out what is specific to "local jvm", and what is here for an SPI rather than API. http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/api/src/main/java/brooklyn/entity/trait/Configurable.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/brooklyn/entity/trait/Configurable.java b/api/src/main/java/brooklyn/entity/trait/Configurable.java index 11f22ae..84f1922 100644 --- a/api/src/main/java/brooklyn/entity/trait/Configurable.java +++ b/api/src/main/java/brooklyn/entity/trait/Configurable.java @@ -19,6 +19,10 @@ package brooklyn.entity.trait; import brooklyn.config.ConfigKey; +import brooklyn.config.ConfigKey.HasConfigKey; +import brooklyn.management.Task; + +import com.google.common.annotations.Beta; /** * Something that has mutable config, such as an entity or policy. @@ -30,7 +34,60 @@ public interface Configurable { // FIXME Moved from core project to api project, as part of moving EntityLocal. // (though maybe it's fine here?) - /** returns the old value, or null if there was not one */ + /** + * @return the old value, or null if there was not one + * @deprecated since 0.7.0; use {@link ConfigurationSupport#set(ConfigKey, Object)}, such as {@code config().set(key, val)} + */ + @Deprecated public <T> T setConfig(ConfigKey<T> key, T val); + ConfigurationSupport config(); + + @Beta + public interface ConfigurationSupport { + + /** + * Gets the given configuration value for this entity, in the following order of preference: + * <ol> + * <li> value (including null) explicitly set on the entity + * <li> value (including null) explicitly set on an ancestor (inherited) + * <li> a default value (including null) on the best equivalent static key of the same name declared on the entity + * (where best equivalence is defined as preferring a config key which extends another, + * as computed in EntityDynamicType.getConfigKeys) + * <li> a default value (including null) on the key itself + * <li> null + * </ol> + */ + <T> T get(ConfigKey<T> key); + + /** + * @see {@link #getConfig(ConfigKey)} + */ + <T> T get(HasConfigKey<T> key); + + /** + * Sets the config to the given value. + */ + <T> T set(ConfigKey<T> key, T val); + + /** + * @see {@link #setConfig(HasConfigKey, Object)} + */ + <T> T set(HasConfigKey<T> key, T val); + + /** + * Sets the config to the value returned by the task. + * + * Returns immediately without blocking; subsequent calls to {@link #getConfig(ConfigKey)} + * will execute the task, and block until the task completes. + * + * @see {@link #setConfig(ConfigKey, Object)} + */ + <T> T set(ConfigKey<T> key, Task<T> val); + + /** + * @see {@link #setConfig(ConfigKey, Task)} + */ + <T> T set(HasConfigKey<T> key, Task<T> val); + } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/core/src/main/java/brooklyn/basic/BasicConfigurableObject.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/basic/BasicConfigurableObject.java b/core/src/main/java/brooklyn/basic/BasicConfigurableObject.java index f4910f1..5d1bc95 100644 --- a/core/src/main/java/brooklyn/basic/BasicConfigurableObject.java +++ b/core/src/main/java/brooklyn/basic/BasicConfigurableObject.java @@ -20,6 +20,8 @@ package brooklyn.basic; import brooklyn.camp.brooklyn.api.HasBrooklynManagementContext; import brooklyn.config.ConfigKey; +import brooklyn.config.ConfigKey.HasConfigKey; +import brooklyn.config.ConfigMap; import brooklyn.entity.trait.Configurable; import brooklyn.entity.trait.Identifiable; import brooklyn.management.ManagementContext; @@ -44,10 +46,10 @@ public class BasicConfigurableObject implements Configurable, Identifiable, Mana private String id = Identifiers.makeRandomId(8); private volatile ManagementContext managementContext; - private ConfigBag config; - + private BasicConfigurationSupport config; + public BasicConfigurableObject() { - config = ConfigBag.newInstance(); + config = new BasicConfigurationSupport(); } @Override @@ -65,14 +67,53 @@ public class BasicConfigurableObject implements Configurable, Identifiable, Mana } @Override + public ConfigurationSupport config() { + return config; + } + + @Override + @Deprecated public <T> T setConfig(ConfigKey<T> key, T value) { - T old = config.get(key); - config.configure(key, value); - return old; + return config().set(key, value); } public <T> T getConfig(ConfigKey<T> key) { - return config.get(key); + return config().get(key); } + private static class BasicConfigurationSupport implements ConfigurationSupport { + private final ConfigBag config = ConfigBag.newInstance(); + + @Override + public <T> T get(ConfigKey<T> key) { + return config.get(key); + } + + @Override + public <T> T get(HasConfigKey<T> key) { + return get(key.getConfigKey()); + } + + @Override + public <T> T set(ConfigKey<T> key, T val) { + T old = config.get(key); + config.configure(key, val); + return old; + } + + @Override + public <T> T set(HasConfigKey<T> key, T val) { + return set(key.getConfigKey(), val); + } + + @Override + public <T> T set(ConfigKey<T> key, Task<T> val) { + throw new UnsupportedOperationException(); + } + + @Override + public <T> T set(HasConfigKey<T> key, Task<T> val) { + return set(key.getConfigKey(), val); + } + } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/core/src/main/java/brooklyn/catalog/internal/CatalogItemDo.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDo.java index 494bfb0..b38d560 100644 --- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDo.java +++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDo.java @@ -25,6 +25,7 @@ import javax.annotation.Nullable; import brooklyn.basic.BrooklynObjectInternal; import brooklyn.catalog.CatalogItem; +import brooklyn.config.ConfigKey; import brooklyn.entity.rebind.RebindSupport; import brooklyn.management.ManagementContext; import brooklyn.mementos.CatalogItemMemento; @@ -56,6 +57,11 @@ public class CatalogItemDo<T,SpecT> implements CatalogItem<T,SpecT>, BrooklynObj } @Override + public <U> U setConfig(ConfigKey<U> key, U val) { + return config().set(key, val); + } + + @Override public CatalogItemType getCatalogItemType() { return itemDto.getCatalogItemType(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java index cf23c26..521f4d6 100644 --- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java +++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java @@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory; import brooklyn.basic.AbstractBrooklynObject; import brooklyn.catalog.CatalogItem; +import brooklyn.config.ConfigKey; import brooklyn.entity.rebind.BasicCatalogItemRebindSupport; import brooklyn.entity.rebind.RebindSupport; import brooklyn.mementos.CatalogItemMemento; @@ -69,7 +70,12 @@ public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynO public ConfigurationSupportInternal config() { throw new UnsupportedOperationException(); } - + + @Override + public <U> U setConfig(ConfigKey<U> key, U val) { + return config().set(key, val); + } + @Override public String getId() { return getCatalogItemId(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java b/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java index 7b7c091..ddd1040 100644 --- a/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java +++ b/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java @@ -387,4 +387,14 @@ public class PortForwardManagerClient implements PortForwardManager { public TagSupport getTagSupport() { return getDelegate().getTagSupport(); } + + @Override + public <T> T setConfig(ConfigKey<T> key, T val) { + return getDelegate().setConfig(key, val); + } + + @Override + public ConfigurationSupport config() { + return getDelegate().config(); + } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/core/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesTest.java b/core/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesTest.java index dc16335..c2b8dec 100644 --- a/core/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesTest.java +++ b/core/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesTest.java @@ -33,6 +33,7 @@ import org.testng.annotations.Test; import brooklyn.entity.BrooklynAppUnitTestSupport; import brooklyn.entity.Entity; +import brooklyn.entity.basic.EntityLocal; import brooklyn.entity.basic.EntityPredicates; import brooklyn.entity.group.zoneaware.ProportionalZoneFailureDetector; import brooklyn.entity.proxying.EntitySpec; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/core/src/test/java/brooklyn/util/internal/FlagUtilsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/util/internal/FlagUtilsTest.java b/core/src/test/java/brooklyn/util/internal/FlagUtilsTest.java index 2510eda..3dda990 100644 --- a/core/src/test/java/brooklyn/util/internal/FlagUtilsTest.java +++ b/core/src/test/java/brooklyn/util/internal/FlagUtilsTest.java @@ -33,9 +33,11 @@ import org.slf4j.LoggerFactory; import org.testng.annotations.Test; import brooklyn.config.ConfigKey; +import brooklyn.config.ConfigKey.HasConfigKey; import brooklyn.entity.basic.ConfigKeys; import brooklyn.entity.trait.Configurable; import brooklyn.event.basic.BasicConfigKey; +import brooklyn.management.Task; import brooklyn.util.collections.MutableMap; import brooklyn.util.config.ConfigBag; import brooklyn.util.flags.FlagUtils; @@ -264,8 +266,49 @@ public class FlagUtilsTest { int f1; ConfigBag bag = new ConfigBag(); + BasicConfigurationSupport configSupport = new BasicConfigurationSupport(); + + @Override + public ConfigurationSupport config() { + return configSupport; + } + public <T> T setConfig(ConfigKey<T> key, T val) { - return bag.put(key, val); + return config().set(key, val); + } + + private class BasicConfigurationSupport implements ConfigurationSupport { + @Override + public <T> T get(ConfigKey<T> key) { + return bag.get(key); + } + + @Override + public <T> T get(HasConfigKey<T> key) { + return get(key.getConfigKey()); + } + + @Override + public <T> T set(ConfigKey<T> key, T val) { + T old = bag.get(key); + bag.configure(key, val); + return old; + } + + @Override + public <T> T set(HasConfigKey<T> key, T val) { + return set(key.getConfigKey(), val); + } + + @Override + public <T> T set(ConfigKey<T> key, Task<T> val) { + throw new UnsupportedOperationException(); + } + + @Override + public <T> T set(HasConfigKey<T> key, Task<T> val) { + return set(key.getConfigKey(), val); + } } } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1ed5484e/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ObjectsYamlTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ObjectsYamlTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ObjectsYamlTest.java index 21eb7a6..6f5a968 100644 --- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ObjectsYamlTest.java +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ObjectsYamlTest.java @@ -26,9 +26,8 @@ import org.slf4j.LoggerFactory; import org.testng.Assert; import org.testng.annotations.Test; -import com.google.common.collect.Lists; - import brooklyn.config.ConfigKey; +import brooklyn.config.ConfigKey.HasConfigKey; import brooklyn.entity.Entity; import brooklyn.entity.basic.ConfigKeys; import brooklyn.entity.basic.Entities; @@ -36,10 +35,14 @@ import brooklyn.entity.proxy.ProxySslConfig; import brooklyn.entity.trait.Configurable; import brooklyn.management.ManagementContext; import brooklyn.management.ManagementContextInjectable; +import brooklyn.management.Task; import brooklyn.test.entity.TestEntity; +import brooklyn.util.config.ConfigBag; import brooklyn.util.flags.SetFromFlag; import brooklyn.util.flags.TypeCoercions; +import com.google.common.collect.Lists; + @Test public class ObjectsYamlTest extends AbstractYamlTest { private static final Logger log = LoggerFactory.getLogger(ObjectsYamlTest.class); @@ -81,7 +84,8 @@ public class ObjectsYamlTest extends AbstractYamlTest { private Integer number; private Object object; private Double value; - + BasicConfigurationSupport configSupport = new BasicConfigurationSupport(); + public ConfigurableObject() { } public String getString() { return string; } @@ -95,11 +99,52 @@ public class ObjectsYamlTest extends AbstractYamlTest { @Override public <T> T setConfig(ConfigKey<T> key, T value) { - log.info("Detected configuration injection for {}: {}", key.getName(), value); - configKeys.add(key.getName()); - if ("config.number".equals(key.getName())) number = TypeCoercions.coerce(value, Integer.class); - if ("config.object".equals(key.getName())) object = value; - return value; + return config().set(key, value); + } + + @Override + public ConfigurationSupport config() { + return configSupport; + } + + private class BasicConfigurationSupport implements ConfigurationSupport { + private final ConfigBag bag = new ConfigBag(); + + @Override + public <T> T get(ConfigKey<T> key) { + return bag.get(key); + } + + @Override + public <T> T get(HasConfigKey<T> key) { + return get(key.getConfigKey()); + } + + @Override + public <T> T set(ConfigKey<T> key, T val) { + log.info("Detected configuration injection for {}: {}", key.getName(), val); + configKeys.add(key.getName()); + if ("config.number".equals(key.getName())) number = TypeCoercions.coerce(val, Integer.class); + if ("config.object".equals(key.getName())) object = val; + T old = bag.get(key); + bag.configure(key, val); + return old; + } + + @Override + public <T> T set(HasConfigKey<T> key, T val) { + return set(key.getConfigKey(), val); + } + + @Override + public <T> T set(ConfigKey<T> key, Task<T> val) { + throw new UnsupportedOperationException(); + } + + @Override + public <T> T set(HasConfigKey<T> key, Task<T> val) { + return set(key.getConfigKey(), val); + } } }
