more enrichers ease-of-use aggregator doesn't require "fromMembers", instead it defaults sensibly, plus better errors and messages around enrichers and policies
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/e5f93cc8 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/e5f93cc8 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/e5f93cc8 Branch: refs/heads/master Commit: e5f93cc8d47ee8ab5755068f65e761c83aa974cc Parents: 089fe86 Author: Alex Heneveld <[email protected]> Authored: Sun Jun 21 10:47:12 2015 -0700 Committer: Alex Heneveld <[email protected]> Committed: Wed Jun 24 00:40:33 2015 -0700 ---------------------------------------------------------------------- .../enricher/basic/AbstractAggregator.java | 32 +++++++++++++++----- .../enricher/basic/AbstractTransformer.java | 5 --- .../brooklyn/enricher/basic/Aggregator.java | 18 ++++++++--- .../java/brooklyn/enricher/basic/Combiner.java | 1 - .../brooklyn/enricher/basic/Transformer.java | 1 + .../entity/basic/ServiceStateLogic.java | 2 +- .../policy/autoscaling/AutoScalerPolicy.java | 2 +- 7 files changed, 41 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java b/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java index a76a602..951a75c 100644 --- a/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java +++ b/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java @@ -58,9 +58,11 @@ public abstract class AbstractAggregator<T,U> extends AbstractEnricher implement public static final ConfigKey<Set<? extends Entity>> FROM_HARDCODED_PRODUCERS = ConfigKeys.newConfigKey(new TypeToken<Set<? extends Entity>>() {}, "enricher.aggregating.fromHardcodedProducers"); - public static final ConfigKey<Boolean> FROM_MEMBERS = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromMembers"); + public static final ConfigKey<Boolean> FROM_MEMBERS = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromMembers", + "Whether this enricher looks at members; only supported if a Group producer is supplier; defaults to true for Group entities"); - public static final ConfigKey<Boolean> FROM_CHILDREN = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromChildren"); + public static final ConfigKey<Boolean> FROM_CHILDREN = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromChildren", + "Whether this enricher looks at children; this is the default for non-Group producers"); public static final ConfigKey<Predicate<? super Entity>> ENTITY_FILTER = ConfigKeys.newConfigKey(new TypeToken<Predicate<? super Entity>>() {}, "enricher.aggregating.entityFilter"); @@ -85,8 +87,6 @@ public abstract class AbstractAggregator<T,U> extends AbstractEnricher implement if (fromHardcodedProducers == null && producer == null) producer = entity; checkState(fromHardcodedProducers != null ^ producer != null, "must specify one of %s (%s) or %s (%s)", PRODUCER.getName(), producer, FROM_HARDCODED_PRODUCERS.getName(), fromHardcodedProducers); - checkState(producer == null || Boolean.TRUE.equals(fromMembers) || Boolean.TRUE.equals(fromChildren), - "when specifying producer, must specify at least one of fromMembers (%s) or fromChildren (%s)", fromMembers, fromChildren); if (fromHardcodedProducers != null) { for (Entity producer : Iterables.filter(fromHardcodedProducers, entityFilter)) { @@ -94,13 +94,13 @@ public abstract class AbstractAggregator<T,U> extends AbstractEnricher implement } } - if (Boolean.TRUE.equals(fromMembers)) { + if (isAggregatingMembers()) { setEntityBeforeSubscribingProducerMemberEvents(entity); setEntitySubscribeProducerMemberEvents(); setEntityAfterSubscribingProducerMemberEvents(); } - if (Boolean.TRUE.equals(fromChildren)) { + if (isAggregatingChildren()) { setEntityBeforeSubscribingProducerChildrenEvents(); setEntitySubscribingProducerChildrenEvents(); setEntityAfterSubscribingProducerChildrenEvents(); @@ -132,7 +132,7 @@ public abstract class AbstractAggregator<T,U> extends AbstractEnricher implement } protected void setEntityBeforeSubscribingProducerMemberEvents(EntityLocal entity) { - checkState(producer instanceof Group, "must be a group when fromMembers true: producer=%s; entity=%s; " + checkState(producer instanceof Group, "Producer must be a group when fromMembers true: producer=%s; entity=%s; " + "hardcodedProducers=%s", getConfig(PRODUCER), entity, fromHardcodedProducers); } @@ -187,6 +187,24 @@ public abstract class AbstractAggregator<T,U> extends AbstractEnricher implement } } + /** true if this should aggregate members */ + protected boolean isAggregatingMembers() { + if (Boolean.TRUE.equals(fromMembers)) return true; + if (Boolean.TRUE.equals(fromChildren)) return false; + if (fromHardcodedProducers!=null) return false; + if (producer instanceof Group) return true; + return false; + } + + /** true if this should aggregate members */ + protected boolean isAggregatingChildren() { + if (Boolean.TRUE.equals(fromChildren)) return true; + if (Boolean.TRUE.equals(fromMembers)) return false; + if (fromHardcodedProducers!=null) return false; + if (producer instanceof Group) return false; + return true; + } + protected abstract void addProducerHardcoded(Entity producer); protected abstract void addProducerMember(Entity producer); protected abstract void addProducerChild(Entity producer); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/core/src/main/java/brooklyn/enricher/basic/AbstractTransformer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/enricher/basic/AbstractTransformer.java b/core/src/main/java/brooklyn/enricher/basic/AbstractTransformer.java index 28a6de1..47e3642 100644 --- a/core/src/main/java/brooklyn/enricher/basic/AbstractTransformer.java +++ b/core/src/main/java/brooklyn/enricher/basic/AbstractTransformer.java @@ -18,8 +18,6 @@ */ package brooklyn.enricher.basic; -import static com.google.common.base.Preconditions.checkArgument; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,9 +30,6 @@ import brooklyn.event.Sensor; import brooklyn.event.SensorEvent; import brooklyn.event.SensorEventListener; import brooklyn.event.basic.BasicSensorEvent; -import brooklyn.util.collections.MutableSet; -import brooklyn.util.task.Tasks; -import brooklyn.util.time.Duration; import com.google.common.base.Function; import com.google.common.reflect.TypeToken; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/core/src/main/java/brooklyn/enricher/basic/Aggregator.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/enricher/basic/Aggregator.java b/core/src/main/java/brooklyn/enricher/basic/Aggregator.java index d7f4fca..a58d4a3 100644 --- a/core/src/main/java/brooklyn/enricher/basic/Aggregator.java +++ b/core/src/main/java/brooklyn/enricher/basic/Aggregator.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,7 +61,7 @@ public class Aggregator<T,U> extends AbstractAggregator<T,U> implements SensorEv @SetFromFlag("transformation") public static final ConfigKey<Object> TRANSFORMATION_UNTYPED = ConfigKeys.newConfigKey(Object.class, "enricher.transformation.untyped", "Specifies a transformation, as a function from a collection to the value, or as a string matching a pre-defined named transformation, " - + "such as 'average' (for numbers), 'add' (for numbers), or 'list' (the default, putting any collection of items into a list)"); + + "such as 'average' (for numbers), 'sum' (for numbers), or 'list' (the default, putting any collection of items into a list)"); public static final ConfigKey<Function<? super Collection<?>, ?>> TRANSFORMATION = ConfigKeys.newConfigKey(new TypeToken<Function<? super Collection<?>, ?>>() {}, "enricher.transformation"); public static final ConfigKey<Boolean> EXCLUDE_BLANK = ConfigKeys.newBooleanConfigKey("enricher.aggregator.excludeBlank", "Whether explicit nulls or blank strings should be excluded (default false); this only applies if no value filter set", false); @@ -82,13 +83,20 @@ public class Aggregator<T,U> extends AbstractAggregator<T,U> implements SensorEv super.setEntityLoadingConfig(); this.sourceSensor = (Sensor<T>) getRequiredConfig(SOURCE_SENSOR); + this.transformation = (Function<? super Collection<T>, ? extends U>) config().get(TRANSFORMATION); + Object t1 = config().get(TRANSFORMATION_UNTYPED); - if (t1 instanceof String) t1 = lookupTransformation((String)t1); + Function<? super Collection<?>, ?> t2 = null; + if (t1 instanceof String) { + t2 = lookupTransformation((String)t1); + if (t2==null) { + LOG.warn("Unknown transformation '"+t1+"' for "+this+"; will use default transformation"); + } + } - this.transformation = (Function<? super Collection<T>, ? extends U>) config().get(TRANSFORMATION); if (this.transformation==null) { - this.transformation = (Function<? super Collection<T>, ? extends U>) t1; - } else if (t1!=null && !t1.equals(this.transformation)) { + this.transformation = (Function<? super Collection<T>, ? extends U>) t2; + } else if (t1!=null && !Objects.equals(t2, this.transformation)) { throw new IllegalStateException("Cannot supply both "+TRANSFORMATION_UNTYPED+" and "+TRANSFORMATION+" unless they are equal."); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/core/src/main/java/brooklyn/enricher/basic/Combiner.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/enricher/basic/Combiner.java b/core/src/main/java/brooklyn/enricher/basic/Combiner.java index cc37d8c..74a400c 100644 --- a/core/src/main/java/brooklyn/enricher/basic/Combiner.java +++ b/core/src/main/java/brooklyn/enricher/basic/Combiner.java @@ -30,7 +30,6 @@ import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import brooklyn.catalog.Catalog; import brooklyn.config.ConfigKey; import brooklyn.entity.Entity; import brooklyn.entity.basic.ConfigKeys; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/core/src/main/java/brooklyn/enricher/basic/Transformer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/enricher/basic/Transformer.java b/core/src/main/java/brooklyn/enricher/basic/Transformer.java index 2fa85fe..0f6c409 100644 --- a/core/src/main/java/brooklyn/enricher/basic/Transformer.java +++ b/core/src/main/java/brooklyn/enricher/basic/Transformer.java @@ -37,6 +37,7 @@ import com.google.common.reflect.TypeToken; @SuppressWarnings("serial") public class Transformer<T,U> extends AbstractTransformer<T,U> { + @SuppressWarnings("unused") private static final Logger LOG = LoggerFactory.getLogger(Transformer.class); // exactly one of these should be supplied to set a value http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java b/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java index cfabf1b..75bf9a6 100644 --- a/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java +++ b/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java @@ -402,7 +402,7 @@ public class ServiceStateLogic { fromMembers = true; // above sets default super.setEntityLoadingConfig(); - if (fromMembers && (!(entity instanceof Group))) { + if (isAggregatingMembers() && (!(entity instanceof Group))) { if (fromChildren) fromMembers=false; else throw new IllegalStateException("Cannot monitor only members for non-group entity "+entity+": "+this); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e5f93cc8/policy/src/main/java/brooklyn/policy/autoscaling/AutoScalerPolicy.java ---------------------------------------------------------------------- diff --git a/policy/src/main/java/brooklyn/policy/autoscaling/AutoScalerPolicy.java b/policy/src/main/java/brooklyn/policy/autoscaling/AutoScalerPolicy.java index 52bb943..b693955 100644 --- a/policy/src/main/java/brooklyn/policy/autoscaling/AutoScalerPolicy.java +++ b/policy/src/main/java/brooklyn/policy/autoscaling/AutoScalerPolicy.java @@ -684,7 +684,7 @@ public class AutoScalerPolicy extends AbstractPolicy { @Override public void setEntity(EntityLocal entity) { if (!config().getRaw(RESIZE_OPERATOR).isPresentAndNonNull()) { - Preconditions.checkArgument(entity instanceof Resizable, "Provided entity must be an instance of Resizable, because no custom-resizer operator supplied"); + Preconditions.checkArgument(entity instanceof Resizable, "Provided entity "+entity+" must be an instance of Resizable, because no custom-resizer operator supplied"); } super.setEntity(entity); this.poolEntity = entity;
