code review part one, simplify AbstractBOSpec, and more minor tweaks based on @neykov's 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/90e8911f Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/90e8911f Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/90e8911f Branch: refs/heads/master Commit: 90e8911f64515d94ec52f21d9ebd06df4b404847 Parents: 42bf7c4 Author: Alex Heneveld <[email protected]> Authored: Tue Nov 17 10:24:34 2015 +0000 Committer: Alex Heneveld <[email protected]> Committed: Tue Nov 17 10:24:34 2015 +0000 ---------------------------------------------------------------------- .../apache/brooklyn/api/entity/EntitySpec.java | 91 -------------------- .../internal/AbstractBrooklynObjectSpec.java | 84 +++++++++++++++++- .../brooklyn/api/location/LocationSpec.java | 78 ++--------------- .../apache/brooklyn/api/policy/PolicySpec.java | 75 ---------------- .../brooklyn/api/sensor/EnricherSpec.java | 75 +--------------- .../api/typereg/BrooklynTypeRegistry.java | 15 ++-- .../core/location/CatalogLocationResolver.java | 3 +- .../core/plan/PlanNotRecognizedException.java | 1 - .../typereg/AbstractTypePlanTransformer.java | 13 ++- .../typereg/BrooklynTypePlanTransformer.java | 34 +++++--- .../spi/creation/CampTypePlanTransformer.java | 4 +- 11 files changed, 138 insertions(+), 335 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java b/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java index 500952d..a73298a 100644 --- a/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java @@ -20,7 +20,6 @@ package org.apache.brooklyn.api.entity; import static com.google.common.base.Preconditions.checkNotNull; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,22 +28,15 @@ import javax.annotation.Nullable; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.location.Location; -import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.policy.Policy; import org.apache.brooklyn.api.policy.PolicySpec; import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.EnricherSpec; -import org.apache.brooklyn.config.ConfigKey; -import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.util.collections.MutableList; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import com.google.common.base.Supplier; import com.google.common.base.Throwables; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; /** @@ -63,8 +55,6 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E private static final long serialVersionUID = -2247153452919128990L; - private static final Logger log = LoggerFactory.getLogger(EntitySpec.class); - /** * Creates a new {@link EntitySpec} instance for an entity of the given type. The returned * {@link EntitySpec} can then be customized. @@ -112,8 +102,6 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E private Class<? extends T> impl; private Entity parent; - private final Map<String, Object> flags = Maps.newLinkedHashMap(); - private final Map<ConfigKey<?>, Object> config = Maps.newLinkedHashMap(); private final List<Policy> policies = Lists.newArrayList(); private final List<PolicySpec<?>> policySpecs = Lists.newArrayList(); private final List<Enricher> enrichers = Lists.newArrayList(); @@ -133,11 +121,7 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E @Override protected EntitySpec<T> copyFrom(EntitySpec<T> otherSpec) { super.copyFrom(otherSpec) - .displayName(otherSpec.getDisplayName()) - .tags(otherSpec.getTags()) .additionalInterfaces(otherSpec.getAdditionalInterfaces()) - .configure(otherSpec.getConfig()) - .configure(otherSpec.getFlags()) .policySpecs(otherSpec.getPolicySpecs()) .policies(otherSpec.getPolicies()) .enricherSpecs(otherSpec.getEnricherSpecs()) @@ -146,7 +130,6 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E .children(otherSpec.getChildren()) .members(otherSpec.getMembers()) .groups(otherSpec.getGroups()) - .catalogItemId(otherSpec.getCatalogItemId()) .locations(otherSpec.getLocations()); if (otherSpec.getParent() != null) parent(otherSpec.getParent()); @@ -208,27 +191,7 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E public Entity getParent() { return parent; } - - /** - * @return Read-only construction flags - * @see SetFromFlag declarations on the entity type - */ - public Map<String, ?> getFlags() { - return Collections.unmodifiableMap(flags); - } - - /** - * @return Read-only configuration values - */ - public Map<ConfigKey<?>, Object> getConfig() { - return Collections.unmodifiableMap(config); - } - /** Clears the config map, removing any config previously set. */ - public void clearConfig() { - config.clear(); - } - public List<PolicySpec<?>> getPolicySpecs() { return policySpecs; } @@ -336,60 +299,6 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E parent = checkNotNull(val, "parent"); return this; } - - public EntitySpec<T> configure(Map<?,?> val) { - checkMutable(); - for (Map.Entry<?, ?> entry: val.entrySet()) { - if (entry.getKey()==null) throw new NullPointerException("Null key not permitted"); - if (entry.getKey() instanceof CharSequence) - flags.put(entry.getKey().toString(), entry.getValue()); - else if (entry.getKey() instanceof ConfigKey<?>) - config.put((ConfigKey<?>)entry.getKey(), entry.getValue()); - else if (entry.getKey() instanceof HasConfigKey<?>) - config.put(((HasConfigKey<?>)entry.getKey()).getConfigKey(), entry.getValue()); - else { - log.warn("Spec "+this+" ignoring unknown config key "+entry.getKey()); - } - } - return this; - } - - public EntitySpec<T> configure(CharSequence key, Object val) { - checkMutable(); - flags.put(checkNotNull(key, "key").toString(), val); - return this; - } - - public <V> EntitySpec<T> configure(ConfigKey<V> key, V val) { - checkMutable(); - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> EntitySpec<T> configure(ConfigKey<V> key, Task<? extends V> val) { - checkMutable(); - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> EntitySpec<T> configure(ConfigKey<V> key, Supplier<? extends V> val) { - checkMutable(); - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> EntitySpec<T> configure(HasConfigKey<V> key, V val) { - checkMutable(); - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - public <V> EntitySpec<T> configure(HasConfigKey<V> key, Task<? extends V> val) { - checkMutable(); - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - /** adds a policy to the spec */ public <V> EntitySpec<T> policy(Policy val) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java index de2954d..72fa96f 100644 --- a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java @@ -22,20 +22,27 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.io.Serializable; import java.lang.reflect.Modifier; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; import org.apache.brooklyn.api.mgmt.EntityManager; +import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.SpecParameter; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.exceptions.Exceptions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; /** Defines a spec for creating a {@link BrooklynObject}. * <p> @@ -49,6 +56,8 @@ import com.google.common.collect.Iterables; public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrooklynObjectSpec<T,SpecT>> implements Serializable { private static final long serialVersionUID = 3010955277740333030L; + + private static final Logger log = LoggerFactory.getLogger(AbstractBrooklynObjectSpec.class); private final Class<? extends T> type; private String displayName; @@ -56,6 +65,9 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly private Set<Object> tags = MutableSet.of(); private List<SpecParameter<?>> parameters = ImmutableList.of(); + protected final Map<String, Object> flags = Maps.newLinkedHashMap(); + protected final Map<ConfigKey<?>, Object> config = Maps.newLinkedHashMap(); + protected AbstractBrooklynObjectSpec(Class<? extends T> type) { checkValidType(type); this.type = type; @@ -150,6 +162,8 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly protected SpecT copyFrom(SpecT otherSpec) { return displayName(otherSpec.getDisplayName()) + .configure(otherSpec.getConfig()) + .configure(otherSpec.getFlags()) .tags(otherSpec.getTags()) .catalogItemId(otherSpec.getCatalogItemId()) .parameters(otherSpec.getParameters()); @@ -175,6 +189,74 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly /** strings inserted as flags, config keys inserted as config keys; * if you want to force one or the other, create a ConfigBag and convert to the appropriate map type */ - public abstract SpecT configure(Map<?,?> val); + public SpecT configure(Map<?,?> val) { + for (Map.Entry<?, ?> entry: val.entrySet()) { + if (entry.getKey()==null) throw new NullPointerException("Null key not permitted"); + if (entry.getKey() instanceof CharSequence) + flags.put(entry.getKey().toString(), entry.getValue()); + else if (entry.getKey() instanceof ConfigKey<?>) + config.put((ConfigKey<?>)entry.getKey(), entry.getValue()); + else if (entry.getKey() instanceof HasConfigKey<?>) + config.put(((HasConfigKey<?>)entry.getKey()).getConfigKey(), entry.getValue()); + else { + log.warn("Spec "+this+" ignoring unknown config key "+entry.getKey()); + } + } + return self(); + } + + public SpecT configure(CharSequence key, Object val) { + flags.put(checkNotNull(key, "key").toString(), val); + return self(); + } + public <V> SpecT configure(ConfigKey<V> key, V val) { + config.put(checkNotNull(key, "key"), val); + return self(); + } + + public <V> SpecT configureIfNotNull(ConfigKey<V> key, V val) { + return (val != null) ? configure(key, val) : self(); + } + + public <V> SpecT configure(ConfigKey<V> key, Task<? extends V> val) { + config.put(checkNotNull(key, "key"), val); + return self(); + } + + public <V> SpecT configure(HasConfigKey<V> key, V val) { + config.put(checkNotNull(key, "key").getConfigKey(), val); + return self(); + } + + public <V> SpecT configure(HasConfigKey<V> key, Task<? extends V> val) { + config.put(checkNotNull(key, "key").getConfigKey(), val); + return self(); + } + + public <V> SpecT removeConfig(ConfigKey<V> key) { + config.remove( checkNotNull(key, "key") ); + return self(); + } + + /** Clears the config map, removing any config previously set. */ + public void clearConfig() { + config.clear(); + } + + /** + * @return Read-only construction flags + * @see SetFromFlag declarations on the policy type + */ + public Map<String, ?> getFlags() { + return Collections.unmodifiableMap(flags); + } + + /** + * @return Read-only configuration values + */ + public Map<ConfigKey<?>, Object> getConfig() { + return Collections.unmodifiableMap(config); + } + } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/api/src/main/java/org/apache/brooklyn/api/location/LocationSpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/location/LocationSpec.java b/api/src/main/java/org/apache/brooklyn/api/location/LocationSpec.java index eca8964..b66ebea 100644 --- a/api/src/main/java/org/apache/brooklyn/api/location/LocationSpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/location/LocationSpec.java @@ -24,11 +24,7 @@ import java.util.Collections; import java.util.Map; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; -import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.config.ConfigKey; -import org.apache.brooklyn.config.ConfigKey.HasConfigKey; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.google.common.collect.Maps; @@ -46,8 +42,6 @@ public class LocationSpec<T extends Location> extends AbstractBrooklynObjectSpec // TODO Would like to add `configure(ConfigBag)`, but `ConfigBag` is in core rather than api - private static final Logger log = LoggerFactory.getLogger(LocationSpec.class); - private final static long serialVersionUID = 1L; /** @@ -81,29 +75,25 @@ public class LocationSpec<T extends Location> extends AbstractBrooklynObjectSpec @SuppressWarnings("unchecked") Class<T> exactType = (Class<T>)spec.getType(); - LocationSpec<T> result = create(exactType) - .displayName(spec.getDisplayName()) - .tags(spec.getTags()) - .configure(spec.getConfig()) - .configure(spec.getFlags()) - .catalogItemId(spec.getCatalogItemId()) - .extensions(spec.getExtensions()); - - if (spec.getParent() != null) result.parent(spec.getParent()); - - return (LocationSpec<T>) result; + return create(exactType).copyFrom(spec); } private String id; private Location parent; - private final Map<String, Object> flags = Maps.newLinkedHashMap(); - private final Map<ConfigKey<?>, Object> config = Maps.newLinkedHashMap(); private final Map<Class<?>, Object> extensions = Maps.newLinkedHashMap(); protected LocationSpec(Class<T> type) { super(type); } + @Override + protected LocationSpec<T> copyFrom(LocationSpec<T> otherSpec) { + LocationSpec<T> result = super.copyFrom(otherSpec).extensions(otherSpec.getExtensions()); + if (otherSpec.getParent() != null) result.parent(otherSpec.getParent()); + if (otherSpec.getId() != null) result.id(otherSpec.getId()); + return result; + } + protected void checkValidType(Class<? extends T> type) { checkIsImplementation(type, Location.class); checkIsNewStyleImplementation(type); @@ -123,56 +113,6 @@ public class LocationSpec<T extends Location> extends AbstractBrooklynObjectSpec return this; } - public LocationSpec<T> configure(Map<?,?> val) { - for (Map.Entry<?, ?> entry: val.entrySet()) { - if (entry.getKey()==null) throw new NullPointerException("Null key not permitted"); - if (entry.getKey() instanceof CharSequence) - flags.put(entry.getKey().toString(), entry.getValue()); - else if (entry.getKey() instanceof ConfigKey<?>) - config.put((ConfigKey<?>)entry.getKey(), entry.getValue()); - else if (entry.getKey() instanceof HasConfigKey<?>) - config.put(((HasConfigKey<?>)entry.getKey()).getConfigKey(), entry.getValue()); - else { - log.warn("Spec "+this+" ignoring unknown config key "+entry.getKey()); - } - } - return this; - } - - public LocationSpec<T> configure(CharSequence key, Object val) { - flags.put(checkNotNull(key, "key").toString(), val); - return this; - } - - public <V> LocationSpec<T> configure(ConfigKey<V> key, V val) { - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> LocationSpec<T> configureIfNotNull(ConfigKey<V> key, V val) { - return (val != null) ? configure(key, val) : this; - } - - public <V> LocationSpec<T> configure(ConfigKey<V> key, Task<? extends V> val) { - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> LocationSpec<T> configure(HasConfigKey<V> key, V val) { - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - public <V> LocationSpec<T> configure(HasConfigKey<V> key, Task<? extends V> val) { - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - public <V> LocationSpec<T> removeConfig(ConfigKey<V> key) { - config.remove( checkNotNull(key, "key") ); - return this; - } - public <E> LocationSpec<T> extension(Class<E> extensionType, E extension) { extensions.put(checkNotNull(extensionType, "extensionType"), checkNotNull(extension, "extension")); return this; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/api/src/main/java/org/apache/brooklyn/api/policy/PolicySpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/policy/PolicySpec.java b/api/src/main/java/org/apache/brooklyn/api/policy/PolicySpec.java index 92caba9..a139d5d 100644 --- a/api/src/main/java/org/apache/brooklyn/api/policy/PolicySpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/policy/PolicySpec.java @@ -18,19 +18,9 @@ */ package org.apache.brooklyn.api.policy; -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.Collections; import java.util.Map; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; -import org.apache.brooklyn.api.mgmt.Task; -import org.apache.brooklyn.config.ConfigKey; -import org.apache.brooklyn.config.ConfigKey.HasConfigKey; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.collect.Maps; /** * Gives details of a policy to be created. It describes the policy's configuration, and is @@ -44,8 +34,6 @@ import com.google.common.collect.Maps; */ public class PolicySpec<T extends Policy> extends AbstractBrooklynObjectSpec<T,PolicySpec<T>> { - private static final Logger log = LoggerFactory.getLogger(PolicySpec.class); - private final static long serialVersionUID = 1L; @@ -71,9 +59,6 @@ public class PolicySpec<T extends Policy> extends AbstractBrooklynObjectSpec<T,P return PolicySpec.create(type).configure(config); } - private final Map<String, Object> flags = Maps.newLinkedHashMap(); - private final Map<ConfigKey<?>, Object> config = Maps.newLinkedHashMap(); - protected PolicySpec(Class<T> type) { super(type); } @@ -87,65 +72,5 @@ public class PolicySpec<T extends Policy> extends AbstractBrooklynObjectSpec<T,P flags.put("uniqueTag", uniqueTag); return this; } - - public PolicySpec<T> configure(Map<?,?> val) { - for (Map.Entry<?, ?> entry: val.entrySet()) { - if (entry.getKey()==null) throw new NullPointerException("Null key not permitted"); - if (entry.getKey() instanceof CharSequence) - flags.put(entry.getKey().toString(), entry.getValue()); - else if (entry.getKey() instanceof ConfigKey<?>) - config.put((ConfigKey<?>)entry.getKey(), entry.getValue()); - else if (entry.getKey() instanceof HasConfigKey<?>) - config.put(((HasConfigKey<?>)entry.getKey()).getConfigKey(), entry.getValue()); - else { - log.warn("Spec "+this+" ignoring unknown config key "+entry.getKey()); - } - } - return this; - } - - public PolicySpec<T> configure(CharSequence key, Object val) { - flags.put(checkNotNull(key, "key").toString(), val); - return this; - } - - public <V> PolicySpec<T> configure(ConfigKey<V> key, V val) { - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> PolicySpec<T> configureIfNotNull(ConfigKey<V> key, V val) { - return (val != null) ? configure(key, val) : this; - } - - public <V> PolicySpec<T> configure(ConfigKey<V> key, Task<? extends V> val) { - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> PolicySpec<T> configure(HasConfigKey<V> key, V val) { - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - public <V> PolicySpec<T> configure(HasConfigKey<V> key, Task<? extends V> val) { - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - /** - * @return Read-only construction flags - * @see SetFromFlag declarations on the policy type - */ - public Map<String, ?> getFlags() { - return Collections.unmodifiableMap(flags); - } - - /** - * @return Read-only configuration values - */ - public Map<ConfigKey<?>, Object> getConfig() { - return Collections.unmodifiableMap(config); - } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/api/src/main/java/org/apache/brooklyn/api/sensor/EnricherSpec.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/sensor/EnricherSpec.java b/api/src/main/java/org/apache/brooklyn/api/sensor/EnricherSpec.java index 87dc2ef..ae50e2d 100644 --- a/api/src/main/java/org/apache/brooklyn/api/sensor/EnricherSpec.java +++ b/api/src/main/java/org/apache/brooklyn/api/sensor/EnricherSpec.java @@ -18,19 +18,12 @@ */ package org.apache.brooklyn.api.sensor; -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.Collections; import java.util.Map; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.collect.Maps; /** * Gives details of an enricher to be created. It describes the enricher's configuration, and is @@ -44,10 +37,7 @@ import com.google.common.collect.Maps; */ public class EnricherSpec<T extends Enricher> extends AbstractBrooklynObjectSpec<T,EnricherSpec<T>> { - private static final Logger log = LoggerFactory.getLogger(EnricherSpec.class); - - private final static long serialVersionUID = 1L; - + private static final long serialVersionUID = -6012873926010992062L; /** * Creates a new {@link EnricherSpec} instance for an enricher of the given type. The returned @@ -71,9 +61,6 @@ public class EnricherSpec<T extends Enricher> extends AbstractBrooklynObjectSpec return EnricherSpec.create(type).configure(config); } - private final Map<String, Object> flags = Maps.newLinkedHashMap(); - private final Map<ConfigKey<?>, Object> config = Maps.newLinkedHashMap(); - protected EnricherSpec(Class<? extends T> type) { super(type); } @@ -88,66 +75,6 @@ public class EnricherSpec<T extends Enricher> extends AbstractBrooklynObjectSpec return this; } - public EnricherSpec<T> configure(Map<?,?> val) { - for (Map.Entry<?, ?> entry: val.entrySet()) { - if (entry.getKey()==null) throw new NullPointerException("Null key not permitted"); - if (entry.getKey() instanceof CharSequence) - flags.put(entry.getKey().toString(), entry.getValue()); - else if (entry.getKey() instanceof ConfigKey<?>) - config.put((ConfigKey<?>)entry.getKey(), entry.getValue()); - else if (entry.getKey() instanceof HasConfigKey<?>) - config.put(((HasConfigKey<?>)entry.getKey()).getConfigKey(), entry.getValue()); - else { - log.warn("Spec "+this+" ignoring unknown config key "+entry.getKey()); - } - } - return this; - } - - public EnricherSpec<T> configure(CharSequence key, Object val) { - flags.put(checkNotNull(key, "key").toString(), val); - return this; - } - - public <V> EnricherSpec<T> configure(ConfigKey<V> key, V val) { - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> EnricherSpec<T> configureIfNotNull(ConfigKey<V> key, V val) { - return (val != null) ? configure(key, val) : this; - } - - public <V> EnricherSpec<T> configure(ConfigKey<V> key, Task<? extends V> val) { - config.put(checkNotNull(key, "key"), val); - return this; - } - - public <V> EnricherSpec<T> configure(HasConfigKey<V> key, V val) { - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - public <V> EnricherSpec<T> configure(HasConfigKey<V> key, Task<? extends V> val) { - config.put(checkNotNull(key, "key").getConfigKey(), val); - return this; - } - - /** - * @return Read-only construction flags - * @see SetFromFlag declarations on the enricher type - */ - public Map<String, ?> getFlags() { - return Collections.unmodifiableMap(flags); - } - - /** - * @return Read-only configuration values - */ - public Map<ConfigKey<?>, Object> getConfig() { - return Collections.unmodifiableMap(config); - } - public abstract static class ExtensibleEnricherSpec<T extends Enricher,K extends ExtensibleEnricherSpec<T,K>> extends EnricherSpec<T> { private static final long serialVersionUID = -3649347642882809739L; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java b/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java index 476034f..ec5db91 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/BrooklynTypeRegistry.java @@ -41,29 +41,30 @@ public interface BrooklynTypeRegistry { Iterable<RegisteredType> getAll(); Iterable<RegisteredType> getAll(Predicate<? super RegisteredType> filter); - + + // XXX remove `context` parameter? /** @return The item matching the given given * {@link RegisteredType#getSymbolicName() symbolicName} * and optionally {@link RegisteredType#getVersion()}, * filtered for the optionally supplied {@link RegisteredTypeLoadingContext}, * taking the best version if the version is null or a default marker, * returning null if no matches are found. */ - RegisteredType get(String symbolicName, String version, @Nullable RegisteredTypeLoadingContext constraint); + RegisteredType get(String symbolicName, String version, @Nullable RegisteredTypeLoadingContext context); /** as {@link #get(String, String, RegisteredTypeLoadingContext)} with no constraints */ RegisteredType get(String symbolicName, String version); /** as {@link #get(String, String, RegisteredTypeLoadingContext)} but allows <code>"name:version"</code> * (the {@link RegisteredType#getId()}) in addition to the unversioned name, * using a default marker if no version can be inferred */ - RegisteredType get(String symbolicNameWithOptionalVersion, @Nullable RegisteredTypeLoadingContext constraint); + RegisteredType get(String symbolicNameWithOptionalVersion, @Nullable RegisteredTypeLoadingContext context); /** as {@link #get(String, RegisteredTypeLoadingContext)} but with no constraints */ RegisteredType get(String symbolicNameWithOptionalVersion); // NB the seemingly more correct generics <T,SpecT extends AbstractBrooklynObjectSpec<T,SpecT>> // cause compile errors, not in Eclipse, but in maven (?) - <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpec(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalConstraint, Class<SpecT> optionalSpecSuperType); - <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpecFromPlan(String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalConstraint, Class<SpecT> optionalSpecSuperType); - - <T> T createBean(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalConstraint, Class<T> optionalResultSuperType); + // TODO do these belong here, or in a separate master TypePlanTransformer ? + <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpec(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalContext, Class<SpecT> optionalSpecSuperType); + <SpecT extends AbstractBrooklynObjectSpec<?,?>> SpecT createSpecFromPlan(String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalContext, Class<SpecT> optionalSpecSuperType); + <T> T createBean(RegisteredType type, @Nullable RegisteredTypeLoadingContext optionalContext, Class<T> optionalResultSuperType); <T> T createBeanFromPlan(String planFormat, Object planData, @Nullable RegisteredTypeLoadingContext optionalConstraint, Class<T> optionalBeanSuperType); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java b/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java index 5db14b7..194e946 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java @@ -22,7 +22,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.util.Map; -import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationRegistry; import org.apache.brooklyn.api.location.LocationResolver; @@ -60,7 +59,7 @@ public class CatalogLocationResolver implements LocationResolver { log.warn("Use of deprecated catalog item "+item.getSymbolicName()+":"+item.getVersion()); } - LocationSpec origLocSpec = (LocationSpec) managementContext.getTypeRegistry().createSpec(item, null, LocationSpec.class); + LocationSpec<?> origLocSpec = (LocationSpec) managementContext.getTypeRegistry().createSpec(item, null, LocationSpec.class); LocationSpec locSpec = LocationSpec.create(origLocSpec) .configure(locationFlags); return managementContext.getLocationManager().createLocation(locSpec); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java b/core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java index dd5c93d..4010cff 100644 --- a/core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java +++ b/core/src/main/java/org/apache/brooklyn/core/plan/PlanNotRecognizedException.java @@ -25,7 +25,6 @@ import org.apache.brooklyn.core.typereg.UnsupportedTypePlanException; @Deprecated public class PlanNotRecognizedException extends RuntimeException { - /** {@link UnsupportedTypePlanException} */ private static final long serialVersionUID = -5590108442839125317L; public PlanNotRecognizedException(String message, Throwable cause) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java index b4d79c2..2900e20 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java @@ -29,6 +29,12 @@ import org.slf4j.LoggerFactory; /** * Convenience supertype for {@link BrooklynTypePlanTransformer} instances. + * <p> + * This supplies a default {@link #scoreForType(RegisteredType, RegisteredTypeLoadingContext)} + * method which returns 1 if the format code matches, + * and otherwise branches to two methods {@link #scoreForNullFormat(Object, RegisteredType, RegisteredTypeLoadingContext)} + * and {@link #scoreForNonmatchingNonnullFormat(String, Object, RegisteredType, RegisteredTypeLoadingContext)} + * which subclasses can implement. (Often the implementation of the latter is 0.) */ public abstract class AbstractTypePlanTransformer implements BrooklynTypePlanTransformer { @@ -107,12 +113,11 @@ public abstract class AbstractTypePlanTransformer implements BrooklynTypePlanTra } }.visit(type), type, context); - } catch (UnsupportedTypePlanException e) { - // no logging - throw Exceptions.propagate(e); } catch (Exception e) { Exceptions.propagateIfFatal(e); - log.debug("Could not instantiate "+type+" (rethrowing): "+Exceptions.collapseText(e)); + if (!(e instanceof UnsupportedTypePlanException)) { + log.debug("Could not instantiate "+type+" (rethrowing): "+Exceptions.collapseText(e)); + } throw Exceptions.propagate(e); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java index 224aca2..e185d3f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BrooklynTypePlanTransformer.java @@ -21,6 +21,9 @@ package org.apache.brooklyn.core.typereg; import java.util.List; import java.util.ServiceLoader; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; import org.apache.brooklyn.api.typereg.RegisteredType; @@ -28,7 +31,8 @@ import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; import org.apache.brooklyn.core.mgmt.ManagementContextInjectable; /** - * Interface for use by schemes which with to be able to transform plans. + * Interface for use by schemes which provide the capability to transform plans + * (serialized descriptions) to brooklyn objecs and specs. * <p> * To add a new plan transformation scheme, simply create an implementation and declare it * as a java service (cf {@link ServiceLoader}). @@ -37,19 +41,29 @@ import org.apache.brooklyn.core.mgmt.ManagementContextInjectable; */ public interface BrooklynTypePlanTransformer extends ManagementContextInjectable { - /** @return a code to identify type implementations created specifying the use of this plan transformer. */ + /** @return An identifier for the transformer. + * This may be used by RegisteredType instances to target a specific transformer. */ String getFormatCode(); - /** @return a display name for this transformer. */ + /** @return A display name for this transformer. + * This may be used to prompt a user what type of plan they are supplying. */ String getFormatName(); - /** @return a description for this transformer */ + /** @return A description for this transformer */ String getFormatDescription(); - /** @return how appropriate is this transformer for the {@link RegisteredType#getPlan()} of the type; - * 0 (or less) if not, 1 for absolutely, and in some autodetect cases a value between 0 and 1 indicate a ranking. - * <p> + /** + * Determines how appropriate is this transformer for the {@link RegisteredType#getPlan()} of the type. * The framework guarantees arguments are nonnull, and that the {@link RegisteredType#getPlan()} is also not-null. - * However the format in that plan may be null. */ - double scoreForType(RegisteredType type, RegisteredTypeLoadingContext context); + * However the format in that plan may be null. + * @return A co-ordinated score / confidence value in the range 0 to 1. + * 0 means not compatible, + * 1 means this is clearly the intended transformer and no others need be tried + * (for instance because the format is explicitly specified), + * and values between 0 and 1 indicate how likely a transformer believes it should be used. + * Values greater than 0.5 are generally reserved for the presence of marker tags or files + * which strongly indicate that the format is compatible. + * <p> + * */ + double scoreForType(@Nonnull RegisteredType type, @Nonnull RegisteredTypeLoadingContext context); /** Creates a new instance of the indicated type, or throws if not supported; * this method is used by the {@link BrooklynTypeRegistry} when it creates instances, * so implementations must respect the {@link RegisteredTypeKind} semantics and the {@link RegisteredTypeLoadingContext} @@ -60,7 +74,7 @@ public interface BrooklynTypePlanTransformer extends ManagementContextInjectable * <p> * Implementations should either return null or throw {@link UnsupportedTypePlanException} * if the {@link RegisteredType#getPlan()} is not supported. */ - Object create(RegisteredType type, RegisteredTypeLoadingContext context); + @Nullable Object create(@Nonnull RegisteredType type, @Nonnull RegisteredTypeLoadingContext context); double scoreForTypeDefinition(String formatCode, Object catalogData); List<RegisteredType> createFromTypeDefinition(String formatCode, Object catalogData); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/90e8911f/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java index 56eeb99..f287fec 100644 --- a/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java +++ b/usage/camp/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/CampTypePlanTransformer.java @@ -35,7 +35,9 @@ import com.google.common.collect.ImmutableList; public class CampTypePlanTransformer extends AbstractTypePlanTransformer { - private static final List<String> FORMATS = ImmutableList.of("brooklyn-camp", "camp", "brooklyn"); + private static final List<String> FORMATS = ImmutableList.of("brooklyn-camp"); + // TODO any use in having these formats? if not, remove. Nov 2015. + // , "camp", "brooklyn"); public static final String FORMAT = FORMATS.get(0);
