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 50c54a39e88235a3b155e2ffaf701fae332d17af Author: Alex Heneveld <[email protected]> AuthorDate: Tue Mar 21 10:03:18 2023 +0000 fix workflow comparison bug where bigdecimal was used and not compared properly also avoid any E notation for numbers --- .../util/core/predicates/DslPredicates.java | 51 +++++++++++++- .../util/core/predicates/DslPredicateTest.java | 78 ++++++++++++++++++++++ .../brooklyn/util/text/NaturalOrderComparator.java | 19 +++++- 3 files changed, 144 insertions(+), 4 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 90622d1868..036a65b1aa 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 @@ -68,6 +68,8 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.io.IOException; import java.lang.reflect.Method; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiFunction; @@ -156,8 +158,35 @@ public class DslPredicates { // if either believes they're equal, believe it if (a.equals(b) || b.equals(a)) return 0; - if (isStringOrPrimitive(a) && isStringOrPrimitive(b)) { - return NaturalOrderComparator.INSTANCE.compare(a.toString(), b.toString()); + if (a instanceof Number || b instanceof Number) { + try { + // if either number is a string try to treat both as string + if (a instanceof String) a = new BigDecimal((String) a); + if (b instanceof String) b = new BigDecimal((String) b); + + if (a instanceof Number && b instanceof Number) { + if (a.getClass().equals(b.getClass())) { + if (a instanceof BigDecimal) return ((BigDecimal) a).compareTo((BigDecimal) b); + if (a instanceof BigInteger) return ((BigInteger) a).compareTo((BigInteger) b); + if (a instanceof Long) return ((Long) a).compareTo((Long) b); + if (a instanceof Integer) return ((Integer) a).compareTo((Integer) b); + return ((Double) (((Number) a).doubleValue())).compareTo(((Number) b).doubleValue()); + } + if (a instanceof Integer || a instanceof Long || a instanceof Byte) a = BigDecimal.valueOf(((Number) a).longValue()); + if (b instanceof Integer || b instanceof Long || a instanceof Byte) b = BigDecimal.valueOf(((Number) b).longValue()); + if (a instanceof Double || a instanceof Float) a = BigDecimal.valueOf(((Number) a).doubleValue()); + if (b instanceof Double || b instanceof Float) b = BigDecimal.valueOf(((Number) b).doubleValue()); + // all now normally big decimal + if (a instanceof BigDecimal && b instanceof BigDecimal) return ((BigDecimal) a).compareTo((BigDecimal) b); + // some weird type; proceed to string stuff below + } + } catch (Exception e) { + // one number is not parseable as a string; proceed to natural order comparator + } + } + + if (isStringOrPrimitiveOrNumber(a) && isStringOrPrimitiveOrNumber(b)) { + return NaturalOrderComparator.INSTANCE.compare(toStringForPrimitives(a), toStringForPrimitives(b)); } if (a instanceof DeferredSupplier || b instanceof DeferredSupplier) @@ -188,12 +217,28 @@ public class DslPredicates { return a!=null && (a instanceof String || Boxing.isPrimitiveOrBoxedClass(a.getClass())); } + private static boolean isStringOrPrimitiveOrNumber(Object a) { + return isStringOrPrimitive(a) || (a!=null && (a instanceof Number)); + } + private static boolean isJson(Object a) { return isStringOrPrimitive(a) || (a instanceof Map) || (a instanceof Collection); } + static String toStringForPrimitives(Object a) { + if (a==null) return null; + if (a instanceof Number) { + // print decimal numbers without E notation + if (a instanceof BigDecimal) return ((BigDecimal)a).toPlainString(); + if (a instanceof Double) return toStringForPrimitives(new BigDecimal((Double)a)); + if (a instanceof Float) return toStringForPrimitives(new BigDecimal((Float)a)); + // fall through to below + } + return a.toString(); + } + static boolean asStringTestOrFalse(Object value, Predicate<String> test) { - return isStringOrPrimitive(value) || value instanceof Throwable ? test.test(value.toString()) : value instanceof Class ? test.test(((Class)value).getName()) : false; + return isStringOrPrimitiveOrNumber(value) || value instanceof Throwable ? test.test(toStringForPrimitives(value)) : value instanceof Class ? test.test(((Class)value).getName()) : false; } @JsonInclude(JsonInclude.Include.NON_NULL) 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 a971d8a79e..74bc686501 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 @@ -121,6 +121,84 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport { Asserts.assertFalse(p.test("5")); } + @Test + public void testGreaterThanOtherTypes() { + DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of( + "greater-than", "25.45"), DslPredicates.DslPredicate.class); + Asserts.assertTrue(p.test(26.01d)); + Asserts.assertTrue(p.test(101.01d)); + Asserts.assertTrue(p.test(101)); + Asserts.assertFalse(p.test(24.9d)); + Asserts.assertFalse(p.test(3.3d)); + Asserts.assertTrue(p.test(25.55d)); + Asserts.assertFalse(p.test(25.35d)); + Asserts.assertFalse(p.test(1d)); + Asserts.assertFalse(p.test(1)); + Asserts.assertFalse(p.test("1")); + Asserts.assertFalse(p.test(-1d)); + Asserts.assertFalse(p.test(-1)); + Asserts.assertFalse(p.test("-1")); + Asserts.assertFalse(p.test(".")); + Asserts.assertTrue(p.test("A")); + + p = TypeCoercions.coerce(MutableMap.of( + "greater-than", 25.45d), DslPredicates.DslPredicate.class); + Asserts.assertTrue(p.test(26.01d)); + Asserts.assertTrue(p.test(101.01d)); + Asserts.assertTrue(p.test(101)); + Asserts.assertFalse(p.test(24.9d)); + Asserts.assertFalse(p.test(3.3d)); + Asserts.assertTrue(p.test(25.55d)); + Asserts.assertFalse(p.test(25.35d)); + Asserts.assertFalse(p.test(-25.55d)); + Asserts.assertFalse(p.test(1d)); + Asserts.assertFalse(p.test(1)); + Asserts.assertFalse(p.test("1")); + Asserts.assertFalse(p.test(-1)); + Asserts.assertFalse(p.test(-1d)); + Asserts.assertFalse(p.test("-1")); + Asserts.assertFalse(p.test(".")); + Asserts.assertTrue(p.test("A")); + + p = TypeCoercions.coerce(MutableMap.of( + "greater-than", -25.45), DslPredicates.DslPredicate.class); + Asserts.assertFalse(p.test(-26.01d)); + Asserts.assertFalse(p.test(-101.01d)); + Asserts.assertFalse(p.test(-101)); + Asserts.assertTrue(p.test(-24.9d)); + Asserts.assertTrue(p.test(-3.3d)); + Asserts.assertFalse(p.test(-25.55d)); + Asserts.assertTrue(p.test(-25.35d)); + Asserts.assertTrue(p.test(25.35d)); + Asserts.assertTrue(p.test(1d)); + Asserts.assertTrue(p.test(1)); + Asserts.assertTrue(p.test("1")); + Asserts.assertTrue(p.test(-1d)); + Asserts.assertTrue(p.test(-1)); + Asserts.assertTrue(p.test("-1")); + Asserts.assertTrue(p.test(".")); + Asserts.assertTrue(p.test("A")); + + p = TypeCoercions.coerce(MutableMap.of( + "greater-than", "-25.45"), DslPredicates.DslPredicate.class); + Asserts.assertFalse(p.test(-26.01d)); + Asserts.assertFalse(p.test(-101.01d)); + Asserts.assertFalse(p.test(-101)); + Asserts.assertTrue(p.test(-24.9d)); + Asserts.assertTrue(p.test(-3.3d)); + Asserts.assertFalse(p.test(-25.55d)); + Asserts.assertTrue(p.test(-25.35d)); + Asserts.assertTrue(p.test(25.35d)); + Asserts.assertTrue(p.test(1d)); + Asserts.assertTrue(p.test(1)); + Asserts.assertTrue(p.test("1")); + Asserts.assertTrue(p.test(-1d)); + Asserts.assertTrue(p.test(-1)); + Asserts.assertTrue(p.test("-1")); + Asserts.assertTrue(p.test(".")); // . comes first alphabetically + Asserts.assertTrue(p.test("A")); + } + @Test public void testLessThanOrEquals() { DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of( diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java index bade244942..9c456577fc 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java @@ -48,6 +48,9 @@ misrepresented as being the original software. */ package org.apache.brooklyn.util.text; +import org.apache.brooklyn.util.exceptions.Exceptions; + +import java.math.BigDecimal; import java.util.Comparator; /** comparator which takes two strings and puts them in an order @@ -109,6 +112,20 @@ public class NaturalOrderComparator implements Comparator<String> { @Override public int compare(String a, String b) { + if (a.startsWith("-") || b.startsWith("-")) { + try { + BigDecimal ba = new BigDecimal(a); + BigDecimal bd = new BigDecimal(b); + return ba.compareTo(bd); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + // otherwise ignore; not decimal + } + } + return compareInternal(a, b); + } + + public int compareInternal(String a, String b) { int ia = 0, ib = 0; int nza = 0, nzb = 0; @@ -167,7 +184,7 @@ public class NaturalOrderComparator implements Comparator<String> { if (ia >= a.length()) return -1; // only b has more chars if (ib >= b.length()) return 1; // only a has more chars // both have remaining chars; neither is numeric due to compareRight; recurse into remaining - if ((result = compare(a.substring(ia), b.substring(ib))) != 0) { + if ((result = compareInternal(a.substring(ia), b.substring(ib))) != 0) { return result; } }
