You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2018/09/24 11:30:26 UTC

[4/7] brooklyn-server git commit: forgot to use deserialization routines when parsing catalog input

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 <al...@cloudsoftcorp.com>
Authored: Sat Sep 22 01:24:09 2018 +0100
Committer: Alex Heneveld <al...@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());
     }
-    
+
 }