You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/08/21 17:23:38 UTC

[brooklyn-server] 02/07: replace "unflattened" DSL predicate test with a cleaner "has-element" test

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit faecd8370a3b7989b5f40d73b4224b2e5e7422c6
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Aug 19 12:20:49 2022 +0100

    replace "unflattened" DSL predicate test with a cleaner "has-element" test
---
 .../util/core/predicates/DslPredicates.java        | 84 +++++++++++++---------
 .../util/core/predicates/DslPredicateTest.java     | 72 +++++++++++++++----
 2 files changed, 109 insertions(+), 47 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
index 210bfbcc7f..43074054a5 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
@@ -84,7 +84,7 @@ public class DslPredicates {
         init();
     }
 
-    public static enum WhenPresencePredicate {
+    public enum WhenPresencePredicate {
         /** value cannot be resolved (not even as null) */ ABSENT,
         /** value is null or cannot be resolved */ ABSENT_OR_NULL,
         /** value is available, but might be null */ PRESENT,
@@ -179,6 +179,10 @@ public class DslPredicates {
         public List<DslPredicate> any;
         public List<DslPredicate> all;
 
+        public @JsonProperty("has-element") DslPredicate hasElement;
+//        public @JsonProperty("at-key") DslPredicate containsKey;
+//        public @JsonProperty("at-index") DslPredicate containsValue;
+
         public WhenPresencePredicate when;
 
         public @JsonProperty("in-range") Range inRange;
@@ -265,6 +269,32 @@ public class DslPredicates {
 
             checkWhen(when, result, checker);
 
+            checker.check(hasElement, result, (test,value) -> {
+                if (value instanceof Map) value = ((Map)value).entrySet();
+                if (value instanceof Iterable) {
+                    for (Object v : ((Iterable) value)) {
+                        if (test.apply((T) v)) return true;
+                    }
+                }
+                return false;
+            });
+
+            // TODO at-key, at-value; preserver order
+//            checker.check(element, test -> nestedPredicateCheck(test, result));
+//
+//            if (result.isPresent() && result.get() instanceof Iterable) {
+//                // iterate through lists
+//                for (Object v : ((Iterable) result.get())) {
+//                    CheckCounts checker2 = new CheckCounts();
+//                    applyToResolved(v instanceof Maybe ? (Maybe) v : Maybe.of(v), checker2);
+//                    if (checker2.allPassed(true)) {
+//                        checker.add(checker2);
+//                        return;
+//                    }
+//                }
+//                // did not pass when flattened; try with unflattened
+//            }
+
             checker.check(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
             checker.check(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
 
@@ -302,7 +332,12 @@ public class DslPredicates {
         }
 
         protected boolean nestedPredicateCheck(DslPredicate p, Maybe<Object> result) {
-            return result.isPresent() ? p.apply(result.get()) : p instanceof DslEntityPredicateDefault ? ((DslEntityPredicateDefault)p).applyToResolved(result) : false;
+            return result.isPresent()
+                    ? p.apply(result.get())
+                    : p instanceof DslPredicateBase
+                    // in case it does a when: absent check
+                    ? ((DslEntityPredicateDefault)p).applyToResolved(result)
+                    : false;
         }
     }
 
@@ -324,9 +359,6 @@ public class DslPredicates {
 
         public Object target;
 
-        /** test to be applied prior to any flattening of lists (eg if targetting children */
-        public DslPredicate unflattened;
-
         public String config;
         public String sensor;
         public DslPredicate tag;
@@ -345,36 +377,6 @@ public class DslPredicates {
                 result = resolver.getMaybe();
             }
 
-            return result;
-        }
-
-        protected Maybe<Object> resolveTargetStringAgainstInput(String target, Object input) {
-            if ("location".equals(target) && input instanceof Entity) return Maybe.of( Locations.getLocationsCheckingAncestors(null, (Entity)input) );
-            if ("children".equals(target) && input instanceof Entity) return Maybe.of( ((Entity)input).getChildren() );
-            return Maybe.absent("Unsupported target '"+target+"' on input "+input);
-        }
-
-        @Override
-        public void applyToResolved(Maybe<Object> result, CheckCounts checker) {
-            checker.check(unflattened, test -> nestedPredicateCheck(test, result));
-
-            if (result.isPresent() && result.get() instanceof Iterable) {
-                // iterate through lists
-                for (Object v : ((Iterable) result.get())) {
-                    CheckCounts checker2 = new CheckCounts();
-                    applyToResolvedFlattened(v instanceof Maybe ? (Maybe) v : Maybe.of(v), checker2);
-                    if (checker2.allPassed(true)) {
-                        checker.add(checker2);
-                        return;
-                    }
-                }
-                // did not pass when flattened; try with unflattened
-            }
-
-            applyToResolvedFlattened(result, checker);
-        }
-
-        public void applyToResolvedFlattened(Maybe<Object> result, CheckCounts checker) {
             if (config!=null) {
                 if (sensor!=null) {
                     throw new IllegalStateException("One predicate cannot specify to test both config key '"+config+"' and sensor '"+sensor+"'; use 'all' with children");
@@ -406,7 +408,19 @@ public class DslPredicates {
                 }
             }
 
+            return result;
+        }
+
+        protected Maybe<Object> resolveTargetStringAgainstInput(String target, Object input) {
+            if ("location".equals(target) && input instanceof Entity) return Maybe.of( Locations.getLocationsCheckingAncestors(null, (Entity)input) );
+            if ("children".equals(target) && input instanceof Entity) return Maybe.of( ((Entity)input).getChildren() );
+            return Maybe.absent("Unsupported target '"+target+"' on input "+input);
+        }
+
+        @Override
+        public void applyToResolved(Maybe<Object> result, CheckCounts checker) {
             super.applyToResolved(result, checker);
+
             checker.check(tag, result, this::checkTag);
 
             if (checker.checksDefined==0) {
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
index ff88c116d0..ba06b7e4ef 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
@@ -55,18 +55,16 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
     }
 
     @Test
-    public void testImplicitEquals() {
+    public void testImplicitEqualsAsDslPredicate() {
         Predicate p = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
         Asserts.assertTrue(p.test("x"));
         Asserts.assertFalse(p.test("y"));
     }
 
     @Test
-    public void testImplicitEqualsAsPredicateNotSupported() {
+    public void testImplicitEqualsAsRawPredicateNotSupported() {
         Asserts.assertFailsWith(() -> {
                 Predicate p = TypeCoercions.coerce("x", Predicate.class);
-//                Asserts.assertTrue(p.test("x"));
-//                Asserts.assertFalse(p.test("y"));
         }, e -> {
                 Asserts.assertStringContainsIgnoreCase(e.toString(), "cannot coerce", "string", "predicate");
                 return true;
@@ -217,8 +215,43 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         }
     }
 
+    // auto-unflattening was too weird; instead prompt for "element", also "map-key", "map-value"
+//    @Test
+//    public void testAllWithListWithVariousFlattening() {
+//        Asserts.assertTrue(SetAllowingEqualsToList.of(MutableSet.of("y", "x")).equals(MutableList.of("x", "y")));
+//
+//        DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("equals", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y")));
+//        // list not equal because of order and equal
+//        Asserts.assertFalse(p.test(Arrays.asList("y", "x")));
+//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
+//        // set equality _does_ match without order
+//        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
+//
+//        // "all" works because it attempts unflattened at all, then flattens on each test
+//        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("y", "x")));
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+//        // set equality _does_ match!
+//        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
+//
+//        // specify unflattening also works, but is unnecessary
+//        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("all", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+//
+//        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("equals", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
+//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y")));
+//
+//        p = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+//
+////        DslPredicates.DslPredicate
+//                p = TypeCoercions.coerce(MutableMap.of("unflattened", "x"), DslPredicates.DslPredicate.class);
+//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
+//    }
     @Test
-    public void testAllWithListWithVariousFlattening() {
+    public void testListsAndElement() {
         Asserts.assertTrue(SetAllowingEqualsToList.of(MutableSet.of("y", "x")).equals(MutableList.of("x", "y")));
 
         DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("equals", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
@@ -229,16 +262,31 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         // set equality _does_ match without order
         Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
 
-        // "all" works because it attempts unflattened at all, then flattens on each test
-        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
+        // no longer automatically unflatted
+        p = TypeCoercions.coerce(MutableMap.of("equals", "x"), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse(p.test(Arrays.asList("y", "x")));
+        Asserts.assertFalse(p.test(Arrays.asList("x")));
+        Asserts.assertTrue(p.test("x"));
+        // and implicit equals gives error
+        Asserts.assertFailsWith(() -> {
+                    DslPredicates.DslPredicate p2 = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
+                    Asserts.assertFalse(p2.test(Arrays.asList("x")));
+                }, e -> Asserts.expectedFailureContainsIgnoreCase(e, "explicit", "equals"));
+
+        // "has-element" now supported
+        p = TypeCoercions.coerce(MutableMap.of("has-element", "x"), DslPredicates.DslPredicate.class);
         Asserts.assertTrue(p.test(Arrays.asList("y", "x")));
-        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
-        // set equality _does_ match!
-        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
+        Asserts.assertFalse(p.test(Arrays.asList("y", "z")));
 
-        // specify unflattening also works, but is unnecessary
-        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("all", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
+        // and can be nested to require multiple elements
+        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of(MutableMap.of("has-element", "x"), MutableMap.of("has-element","y"))), DslPredicates.DslPredicate.class);
         Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+        Asserts.assertFalse(p.test(Arrays.asList("x", "z")));
+
+        // and can take another predicate
+        p = TypeCoercions.coerce(MutableMap.of("has-element", MutableMap.of("glob", "?")), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(Arrays.asList("xx", "y")));
+        Asserts.assertFalse(p.test(Arrays.asList("xx", "yy")));
     }
 
     @Test