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
The following commit(s) were added to refs/heads/master by this push: new b05554acf0 better dsl predicate evaluation equality and comparison for numbers b05554acf0 is described below commit b05554acf01529dfa3e0346e1416b6c63bf0dec7 Author: Alex Heneveld <a...@cloudsoft.io> AuthorDate: Tue Jan 16 09:28:03 2024 +0000 better dsl predicate evaluation equality and comparison for numbers fixes equality of different numeric types, and equality of string double with integer (both of which should be true) --- .../util/core/predicates/DslPredicates.java | 11 +++-- .../util/core/predicates/DslPredicateTest.java | 49 +++++++++++++++++++--- .../org/apache/brooklyn/util/math/NumberMath.java | 8 ++-- 3 files changed, 55 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 574504cdad..76efad2c9b 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 @@ -61,6 +61,7 @@ import org.apache.brooklyn.util.guava.SerializablePredicate; import org.apache.brooklyn.util.javalang.Boxing; import org.apache.brooklyn.util.javalang.Reflections; import org.apache.brooklyn.util.javalang.coerce.TryCoercer; +import org.apache.brooklyn.util.math.NumberMath; import org.apache.brooklyn.util.text.NaturalOrderComparator; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.WildcardGlobs; @@ -161,13 +162,17 @@ public class DslPredicates { if (ma.equals(((Class<?>) mb).getName()) || ma.equals(((Class<?>) mb).getSimpleName())) return Maybe.of(true); } else if ((isJson(ma) && !isJson(mb)) || (ma instanceof String && Boxing.isPrimitiveOrBoxedClass(mb.getClass()))) { - Maybe<? extends Object> mma = TypeCoercions.tryCoerce(ma, mb.getClass()); + Class<?> clazz = mb instanceof Number ? Number.class : mb.getClass(); + Maybe<? extends Object> mma = TypeCoercions.tryCoerce(ma, clazz); if (mma.isPresent()) { - // repeat equality check - if (Objects.equals(mma.get(), mb) || Objects.equals(mb, mma.get())) return Maybe.of(true); +// // repeat equality check + return Maybe.of(coercedEqual(mma.get(), mb)); } return Maybe.absent("coercion not supported in equality check, to "+mb.getClass()); + } else if (a instanceof Number && b instanceof Number) { + return Maybe.of(new NumberMath((Number) a).withinTolerance((Number) b)); } + return Maybe.absent("coercion not permitted for equality check with these argument types"); }; return maybeCoercedEquals.apply(a, b) 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 61d5ca893c..016059ebbd 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 @@ -25,6 +25,7 @@ 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.core.flags.TypeCoercions; +import org.apache.brooklyn.util.core.predicates.DslPredicates.DslPredicate; import org.apache.brooklyn.util.time.Duration; import org.apache.brooklyn.util.time.Time; import org.testng.annotations.Test; @@ -222,12 +223,48 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport { @Test public void testGreaterThanOrEquals() { - DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of( - "greater-than-or-equal-to", "5"), DslPredicates.DslPredicate.class); - Asserts.assertFalse(p.test("4")); - Asserts.assertFalse(p.test("-0.2")); - Asserts.assertTrue(p.test("10")); - Asserts.assertTrue(p.test("5")); + Consumer<Object> check = (v) -> { + DslPredicate p = TypeCoercions.coerce(MutableMap.of("greater-than-or-equal-to", v), DslPredicate.class); + // numbers always compare as numbers + Asserts.assertTrue(p.test(5)); + Asserts.assertTrue(p.test(5.0)); + Asserts.assertTrue(p.test(5.1)); + Asserts.assertFalse(p.test(4.9)); + + // strings compare with numbers as numbers, with other strings using NaturalOrder -- + // so signed or unsigned digit sequences are compared numerically, but decimal parts not + Asserts.assertFalse(p.test("4")); + Asserts.assertFalse(p.test("-0.2")); + Asserts.assertTrue(p.test("10")); + Asserts.assertTrue(p.test("09")); + Asserts.assertFalse(p.test("04")); + Asserts.assertEquals(p.test("5"), !"5.0".equals(v)); + Asserts.assertEquals(p.test("5.0"), true); + }; + check.accept("5"); + check.accept("5.0"); + check.accept(5); + check.accept(5.0d); + } + + @Test + public void testEqualsNumber() { + Consumer<Object> check = (v) -> { + DslPredicate p = TypeCoercions.coerce(MutableMap.of("equals", v), DslPredicate.class); + Asserts.assertFalse(p.test("4")); + Asserts.assertFalse(p.test("-0.2")); + Asserts.assertFalse(p.test("10")); + Asserts.assertEquals(p.test("5"), !"5.0".equals(v)); + Asserts.assertEquals(p.test("5.0"), !"5".equals(v)); + Asserts.assertTrue(p.test(5)); + Asserts.assertTrue(p.test(5.0)); + Asserts.assertFalse(p.test(5.1)); + Asserts.assertFalse(p.test(4.9)); + }; + check.accept(5); + check.accept(5.0d); + check.accept("5"); + check.accept("5.0"); } @Test diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/math/NumberMath.java b/utils/common/src/main/java/org/apache/brooklyn/util/math/NumberMath.java index 023506fc66..eb2b2f2d97 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/math/NumberMath.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/math/NumberMath.java @@ -56,7 +56,7 @@ public class NumberMath<T extends Number> { public Optional<BigInteger> asBigIntegerWithinTolerance() { return asBigIntegerWithinTolerance(number); } public <T extends Number> T asTypeForced(Class<T> desiredType) { - return asTypeFirstMatching(number, desiredType, y -> withinTolerance(number, y)); + return asTypeFirstMatching(number, desiredType, y -> withinTolerance(y)); } public <T extends Number> Optional<T> asTypeWithinTolerance(Class<T> desiredType, Number tolerance) { return Optional.ofNullable(asTypeFirstMatching(number, desiredType, y -> withinTolerance(number, y, tolerance))); @@ -84,7 +84,7 @@ public class NumberMath<T extends Number> { public Optional<BigInteger> asBigIntegerWithinTolerance(Number number) { BigInteger candidate = asBigIntegerForced(number); - if (withinTolerance(number, candidate)) return Optional.of(candidate); + if (withinTolerance(candidate)) return Optional.of(candidate); return Optional.empty(); } @@ -112,8 +112,8 @@ public class NumberMath<T extends Number> { return null; } - public boolean withinTolerance(Number a, Number b) { - return withinTolerance(a, b, tolerance); + public boolean withinTolerance(Number b) { + return withinTolerance(number, b, tolerance); } public static boolean withinTolerance(Number a, Number b, Number tolerance) { return asBigDecimal(a).subtract(asBigDecimal(b)).abs().compareTo(asBigDecimal(tolerance)) <= 0;