forgot to use deserialization routines when parsing catalog input also test and fix for some edge-cases in serializing
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/fdb2784a Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/fdb2784a Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/fdb2784a Branch: refs/heads/master Commit: fdb2784a8bdc7a3bc53508a381b258539f7aafc0 Parents: 5fec275 Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Sat Sep 22 01:24:09 2018 +0100 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Sat Sep 22 01:24:09 2018 +0100 ---------------------------------------------------------------------- .../brooklyn/core/config/ConfigConstraints.java | 9 ++++ .../brooklyn/core/objs/BasicSpecParameter.java | 51 +------------------- .../core/objs/ConstraintSerialization.java | 9 ++-- .../brooklyn/util/core/ResourcePredicates.java | 2 +- .../core/objs/ConstraintSerializationTest.java | 45 +++++++++++++---- 5 files changed, 52 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/fdb2784a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java index 4ee584d..c234aae 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java @@ -22,6 +22,7 @@ package org.apache.brooklyn.core.config; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Objects; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.location.Location; @@ -244,6 +245,14 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { public String toString() { return "required()"; } + @Override + public boolean equals(Object obj) { + return (obj instanceof RequiredPredicate) && obj.getClass().equals(getClass()); + } + @Override + public int hashCode() { + return Objects.hash(toString()); + } } private static abstract class OtherKeyPredicate implements BrooklynObjectPredicate<Object> { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/fdb2784a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java index 7c1bde0..7c777f8 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java @@ -18,8 +18,6 @@ */ package org.apache.brooklyn.core.objs; -import static com.google.common.base.Preconditions.checkArgument; - import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collection; @@ -55,7 +53,6 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.guava.Maybe; -import org.apache.brooklyn.util.text.StringPredicates; import org.apache.brooklyn.util.time.Duration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -70,7 +67,6 @@ import com.google.common.base.Predicates; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; import com.google.common.reflect.TypeToken; public class BasicSpecParameter<T> implements SpecParameter<T>{ @@ -221,18 +217,6 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ .put("port", PortRange.class) .build(); - private static final Map<String, Predicate<?>> BUILT_IN_CONSTRAINTS = ImmutableMap.<String, Predicate<?>>of( - "required", StringPredicates.isNonBlank()); - - private static final Map<String, Function<Object, Predicate<?>>> BUILT_IN_CONSTRAINT_FACTORIES = ImmutableMap.<String, Function<Object, Predicate<?>>>of( - "regex", new Function<Object, Predicate<?>>() { - @Override public Predicate<?> apply(Object input) { - // TODO Could try to handle deferred supplier as well? - checkArgument(input instanceof String, "Constraint regex value must be a string, but got %s (%s)", - (input == null ? "null" : input.getClass().getName()), input); - return StringPredicates.matchesRegex((String)input); - }}); - private static List<SpecParameter<?>> parseParameters(List<?> inputsRaw, Function<Object, Object> specialFlagTransformer, BrooklynClassLoadingContext loader) { if (inputsRaw == null) return ImmutableList.of(); List<SpecParameter<?>> inputs = new ArrayList<>(inputsRaw.size()); @@ -381,40 +365,7 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ } private static Predicate<?> parseConstraint(Object untypedConstraint, BrooklynClassLoadingContext loader) { - // TODO Could try to handle deferred supplier as well? - if (untypedConstraint instanceof Predicate) { - // An explicit predicate (e.g. via "$brooklyn:object: ...") - return (Predicate<?>) untypedConstraint; - } else if (untypedConstraint instanceof String) { - // build-in simple declaration, such as "required" - String constraint = (String)untypedConstraint; - if (BUILT_IN_CONSTRAINTS.containsKey(constraint)) { - return BUILT_IN_CONSTRAINTS.get(constraint); - } else { - throw new IllegalArgumentException("The constraint '" + constraint + "' for a catalog input is not " - + "recognized as a built-in (" + BUILT_IN_CONSTRAINTS.keySet() + " or " - + BUILT_IN_CONSTRAINT_FACTORIES.keySet() + ")"); - } - } else if (untypedConstraint instanceof Map) { - // For example "regex: foo.*" - Map<?,?> constraint = (Map<?,?>)untypedConstraint; - if (constraint.size() == 1) { - Object key = Iterables.getOnlyElement(constraint.keySet()); - Object val = constraint.get(key); - if (BUILT_IN_CONSTRAINT_FACTORIES.containsKey(key)) { - Function<Object, Predicate<?>> factory = BUILT_IN_CONSTRAINT_FACTORIES.get(key); - return factory.apply(val); - } else { - throw new IllegalArgumentException("The constraint '" + constraint + "' for a catalog input is not " - + "recognized as a built-in (" + BUILT_IN_CONSTRAINTS.keySet() + ")"); - } - } else { - throw new IllegalArgumentException("The config key constraint '" + constraint + "' is not supported - " - + "it can handle only single key:value constraint."); - } - } else { - throw new IllegalArgumentException("The constraint '" + untypedConstraint + "' for a catalog input is not recognized"); - } + return ConstraintSerialization.INSTANCE.toPredicateFromJson(untypedConstraint); } private static ConfigInheritance parseInheritance(Object obj, BrooklynClassLoadingContext loader) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/fdb2784a/core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java b/core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java index 6844a72..2fef0e8 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/ConstraintSerialization.java @@ -214,6 +214,9 @@ public class ConstraintSerialization { if (parser.result instanceof Map && ((Map)parser.result).size()==1 && ((Map)parser.result).containsKey("all")) { return (List<Object>) ((Map)parser.result).get("all"); } + if ("Predicates.alwaysTrue".equals(parser.result)) { + return Collections.emptyList(); + } return ImmutableList.of(parser.result); } @@ -291,7 +294,7 @@ public class ConstraintSerialization { } } - private void collectPredicateListFromJson(Object o, List<Predicate<?>> result) { + private void collectPredicateListFromJson(Object o, Collection<Predicate<?>> result) { if (o instanceof Collection) { ((Collection<?>)o).stream().forEach(i -> collectPredicateListFromJson(i, result)); return; @@ -361,9 +364,9 @@ public class ConstraintSerialization { return Predicates.and(preds); } public List<Predicate<?>> toPredicateListFromJsonList(Collection<?> o) { - List<Predicate<?>> result = MutableList.of(); + Set<Predicate<?>> result = MutableSet.of(); collectPredicateListFromJson(o, result); - return result; + return MutableList.copyOf(result); } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/fdb2784a/core/src/main/java/org/apache/brooklyn/util/core/ResourcePredicates.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/ResourcePredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/ResourcePredicates.java index 240ab52..effed58 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/ResourcePredicates.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/ResourcePredicates.java @@ -66,7 +66,7 @@ public class ResourcePredicates { @Override public String toString() { - return "ResourcePredicates.urlExists()"; + return "ResourcePredicates.exists()"; } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/fdb2784a/core/src/test/java/org/apache/brooklyn/core/objs/ConstraintSerializationTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/objs/ConstraintSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/objs/ConstraintSerializationTest.java index 539fc87..a8fdb02 100644 --- a/core/src/test/java/org/apache/brooklyn/core/objs/ConstraintSerializationTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/objs/ConstraintSerializationTest.java @@ -58,10 +58,10 @@ public class ConstraintSerializationTest extends BrooklynMgmtUnitTestSupport { @Test public void testAltName() { Predicate<String> p = StringPredicates.matchesGlob("???*"); - Assert.assertEquals(ConstraintSerialization.INSTANCE.toPredicateFromJson( - MutableList.of(MutableMap.of("matchesGlob", "???*"))).toString(), p.toString()); - Assert.assertEquals(ConstraintSerialization.INSTANCE.toPredicateFromJson( - MutableList.of(MutableMap.of("glob", "???*"))).toString(), p.toString()); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson( + MutableList.of(MutableMap.of("matchesGlob", "???*"))), p); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson( + MutableList.of(MutableMap.of("glob", "???*"))), p); Assert.assertEquals(ConstraintSerialization.INSTANCE.toJsonList(p), MutableList.of(MutableMap.of("glob", "???*"))); } @@ -69,13 +69,19 @@ public class ConstraintSerializationTest extends BrooklynMgmtUnitTestSupport { @Test public void testAcceptsMap() { Predicate<String> p = StringPredicates.matchesGlob("???*"); - Assert.assertEquals(ConstraintSerialization.INSTANCE.toPredicateFromJson(MutableMap.of("matchesGlob", "???*")).toString(), p.toString()); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson(MutableMap.of("matchesGlob", "???*")), p); + } + + @Test + public void testAcceptsForbiddenIfMap() { + Predicate<Object> p = ConfigConstraints.forbiddenIf("x"); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson(MutableMap.of("forbiddenIf", "x")), p); } @Test public void testAcceptsString() { Predicate<String> p = StringPredicates.matchesGlob("???*"); - Assert.assertEquals(ConstraintSerialization.INSTANCE.toPredicateFromJson("matchesGlob(\"???*\")").toString(), p.toString()); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson("matchesGlob(\"???*\")"), p); } @Test @@ -83,14 +89,33 @@ public class ConstraintSerializationTest extends BrooklynMgmtUnitTestSupport { Predicate<?> p = Predicates.notNull(); Assert.assertEquals(ConstraintSerialization.INSTANCE.toJsonList(p), MutableList.of("required")); - Assert.assertEquals(ConstraintSerialization.INSTANCE.toPredicateFromJson("required").toString(), - ConfigConstraints.required().toString()); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson("required"), + ConfigConstraints.required()); + } + + @Test + public void testFlattens() { + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson(MutableList.of("required", "required")), + ConfigConstraints.required()); + } + + @Test + public void testEmpty() { + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson(MutableList.of()), + Predicates.alwaysTrue()); + Assert.assertEquals(ConstraintSerialization.INSTANCE.toJsonList(Predicates.alwaysTrue()), + MutableList.of()); } private void assertPredJsonBidi(Predicate<?> pred, List<?> json) { Assert.assertEquals(ConstraintSerialization.INSTANCE.toJsonList(pred), json); + assertSamePredicate(ConstraintSerialization.INSTANCE.toPredicateFromJson(json), pred); + } + + private static void assertSamePredicate(Predicate<?> p1, Predicate<?> p2) { // some predicates don't support equals, but all (the ones we use) must support toString - Assert.assertEquals(ConstraintSerialization.INSTANCE.toPredicateFromJson(json).toString(), pred.toString()); + Assert.assertEquals(p1.toString(), p2.toString()); + Assert.assertEquals(p1.getClass(), p2.getClass()); } - + }