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 11d9f10d85235e625594f2d1a3e6d21ea299bc64 Author: Alex Heneveld <[email protected]> AuthorDate: Thu Jul 14 13:17:24 2022 +0100 fix bug where new DslPredicate didn't properly test sensors and passed if no checks where applicable --- .../util/core/predicates/DslPredicates.java | 35 ++++++++++++++++------ .../core/predicates/DslPredicateEntityTest.java | 27 ++++++++++++++--- 2 files changed, 49 insertions(+), 13 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 8a079e2c3e..210bfbcc7f 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 @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.util.core.predicates; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.*; @@ -168,6 +169,7 @@ public class DslPredicates { return isStringOrPrimitive(value) ? test.test(value.toString()) : false; } + @JsonInclude(JsonInclude.Include.NON_NULL) public static class DslPredicateBase<T> { public Object implicitEquals; public Object equals; @@ -189,25 +191,39 @@ public class DslPredicates { public @JsonProperty("java-instance-of") Object javaInstanceOf; public static class CheckCounts { - int checksTried = 0; + int checksDefined = 0; + int checksApplicable = 0; int checksPassed = 0; public <T> void check(T test, java.util.function.Predicate<T> check) { if (test!=null) { - checksTried++; + checksDefined++; + checksApplicable++; if (check.test(test)) checksPassed++; } } public <T> void check(T test, Maybe<Object> value, java.util.function.BiPredicate<T,Object> check) { - if (value.isPresent()) check(test, t -> check.test(t, value.get())); + if (value.isPresent()) { + check(test, t -> check.test(t, value.get())); + } else { + if (test!=null) { + checksDefined++; + } + } } + @Deprecated /** @deprecated since 1.1 when introduced; pass boolean */ public boolean allPassed() { - return checksPassed == checksTried; + return checksPassed == checksDefined; + } + + public boolean allPassed(boolean requireAtLeastOne) { + if (checksDefined ==0 && requireAtLeastOne) return false; + return checksPassed == checksDefined; } public void add(CheckCounts other) { checksPassed += other.checksPassed; - checksTried += other.checksTried; + checksDefined += other.checksDefined; } } @@ -223,7 +239,8 @@ public class DslPredicates { public boolean applyToResolved(Maybe<Object> result) { CheckCounts counts = new CheckCounts(); applyToResolved(result, counts); - return counts.allPassed(); + if (counts.checksDefined==0) throw new IllegalStateException("Predicate does not define any checks; if always true or always false is desired, use 'when'"); + return counts.allPassed(true); } public void applyToResolved(Maybe<Object> result, CheckCounts checker) { @@ -346,7 +363,7 @@ public class DslPredicates { for (Object v : ((Iterable) result.get())) { CheckCounts checker2 = new CheckCounts(); applyToResolvedFlattened(v instanceof Maybe ? (Maybe) v : Maybe.of(v), checker2); - if (checker2.allPassed()) { + if (checker2.allPassed(true)) { checker.add(checker2); return; } @@ -392,8 +409,8 @@ public class DslPredicates { super.applyToResolved(result, checker); checker.check(tag, result, this::checkTag); - if (checker.checksTried==0) { - if (target!=null || config!=null) { + if (checker.checksDefined==0) { + if (target!=null || config!=null || sensor!=null) { // if no test specified, but test or config is, then treat as implicit presence check checkWhen(WhenPresencePredicate.PRESENT_NON_NULL, result, checker); } diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java index 24e3b06ee3..2465003544 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java @@ -26,12 +26,14 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.core.entity.EntityPredicates; import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; +import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.flags.TypeCoercions; +import org.apache.brooklyn.util.core.task.Tasks; import org.apache.brooklyn.util.yaml.Yamls; import org.testng.annotations.Test; @@ -50,11 +52,28 @@ public class DslPredicateEntityTest extends BrooklynAppUnitTestSupport { "config", TestEntity.CONF_OBJECT.getName(), "equals", "x"), DslPredicates.DslPredicate.class); - app.config().set(TestEntity.CONF_OBJECT, "x"); - Asserts.assertTrue(p.test(app)); + app.getExecutionContext().submit(Tasks.create("test", () -> { + app.config().set(TestEntity.CONF_OBJECT, "x"); + Asserts.assertTrue(p.test(app)); - app.config().set(TestEntity.CONF_OBJECT, "y"); - Asserts.assertFalse(p.test(app)); + app.config().set(TestEntity.CONF_OBJECT, "y"); + Asserts.assertFalse(p.test(app)); + })).getUnchecked(); + } + + @Test + public void testSensorEquals() { + DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of( + "sensor", "sensor1", + "equals", "x"), DslPredicates.DslPredicate.class); + + app.getExecutionContext().submit(Tasks.create("test", () -> { + app.sensors().set(Sensors.newStringSensor("sensor1"), "x"); + Asserts.assertTrue(p.test(app)); + + app.sensors().set(Sensors.newStringSensor("sensor1"), "y"); + Asserts.assertFalse(p.test(app)); + })).getUnchecked(); } @Test
