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 9a9cfc0c06cd763e9dfc964d7ae3a8ff90bcf53f Author: Alex Heneveld <[email protected]> AuthorDate: Tue Aug 24 16:50:51 2021 +0100 be stricter on coercions that are otherwise lossy and/or platform-dependent --- .../brooklyn/camp/brooklyn/ConfigYamlTest.java | 56 ++++++++++++++++++++++ .../brooklyn/core/test/entity/TestEntity.java | 2 + .../coerce/CommonAdaptorTypeCoercions.java | 19 ++++++-- .../coerce/PrimitiveStringTypeCoercions.java | 30 ++++++++---- 4 files changed, 94 insertions(+), 13 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java index 53a6be8..9c77478 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java @@ -419,4 +419,60 @@ public class ConfigYamlTest extends AbstractYamlTest { assertEquals(entity.config().getNonBlocking(TestEntity.CONF_LIST_PLAIN).get(), ImmutableList.of("myOther")); assertEquals(entity.config().getNonBlocking(TestEntity.CONF_SET_PLAIN).get(), ImmutableSet.of("myOther")); } + + @Test + public void testConfigGoodNumericCoercions() throws Exception { + String yaml = Joiner.on("\n").join( + "services:", + "- type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.config:", + " test.confDouble: 1.1", + " test.confInteger: 1.0"); + + final Entity app = createStartWaitAndLogApplication(yaml); + TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + + assertEquals(entity.config().get(TestEntity.CONF_INTEGER), (Integer)1); + assertEquals(entity.config().get(TestEntity.CONF_DOUBLE), (Double)1.1); + } + + @Test + public void testConfigVeryLargeIntegerCoercionFails() throws Exception { + String yaml = Joiner.on("\n").join( + "services:", + "- type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.config:", + " test.confInteger: 9999999999999999999999999999999999999332", + ""); + + Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml), + e -> e.toString().contains("9999332")); + } + + @Test + public void testConfigVeryLargeDoubleCoercionFails() throws Exception { + String yaml = Joiner.on("\n").join( + "services:", + "- type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.config:", + " test.confDouble: 999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999332", + ""); + + Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml), + e -> e.toString().contains("9999332")); + } + + @Test + public void testConfigFloatAsIntegerCoercionFails() throws Exception { + String yaml = Joiner.on("\n").join( + "services:", + "- type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.config:", + " test.confInteger: 1.5", + ""); + + Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml), + e -> e.toString().contains("1.5")); + } + } diff --git a/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java b/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java index bfada48..072a521 100644 --- a/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java +++ b/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java @@ -74,6 +74,8 @@ public interface TestEntity extends Entity, Startable, EntityLocal, EntityIntern public static final SetConfigKey<String> CONF_SET_THING = new SetConfigKey<String>(String.class, "test.confSetThing", "Configuration key that's a set thing"); public static final SetConfigKey<Object> CONF_SET_OBJ_THING = new SetConfigKey<Object>(Object.class, "test.confSetObjThing", "Configuration key that's a set thing, of objects"); public static final BasicConfigKey<Object> CONF_OBJECT = new BasicConfigKey<Object>(Object.class, "test.confObject", "Configuration key that's an object"); + public static final ConfigKey<Integer> CONF_INTEGER = ConfigKeys.newConfigKey(Integer.class, "test.confInteger", "Configuration key, an integer"); + public static final ConfigKey<Double> CONF_DOUBLE = ConfigKeys.newConfigKey(Double.class, "test.confDouble", "Configuration key, a double"); public static final ConfigKey<EntitySpec<? extends Entity>> CHILD_SPEC = ConfigKeys.newConfigKey(new TypeToken<EntitySpec<? extends Entity>>() {}, "test.childSpec", "Spec to be used for creating children"); public static final AttributeSensorAndConfigKey<String, String> ATTRIBUTE_AND_CONF_STRING = ConfigKeys.newStringSensorAndConfigKey( diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java index c49ea23..f865da8 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.util.javalang.coerce; +import com.google.common.annotations.Beta; import java.math.BigDecimal; import java.math.BigInteger; import java.net.InetAddress; @@ -58,6 +59,8 @@ import com.google.common.reflect.TypeToken; public class CommonAdaptorTypeCoercions { + @Beta public static final double DELTA_FOR_COERCION = 0.000001; + private final TypeCoercerExtensible coercer; public CommonAdaptorTypeCoercions(TypeCoercerExtensible coercer) { @@ -205,19 +208,19 @@ public class CommonAdaptorTypeCoercions { registerAdapter(BigDecimal.class, Double.class, new Function<BigDecimal,Double>() { @Override public Double apply(BigDecimal input) { - return input.doubleValue(); + return checkValidForConversion(input, input.doubleValue()); } }); registerAdapter(BigInteger.class, Long.class, new Function<BigInteger,Long>() { @Override public Long apply(BigInteger input) { - return input.longValue(); + return input.longValueExact(); } }); registerAdapter(BigInteger.class, Integer.class, new Function<BigInteger,Integer>() { @Override public Integer apply(BigInteger input) { - return input.intValue(); + return input.intValueExact(); } }); registerAdapter(String.class, BigDecimal.class, new Function<String,BigDecimal>() { @@ -317,7 +320,15 @@ public class CommonAdaptorTypeCoercions { } }); } - + + @Beta + public static double checkValidForConversion(BigDecimal input, double candidate) { + if (input.subtract(BigDecimal.valueOf(candidate)).abs().compareTo(BigDecimal.valueOf(DELTA_FOR_COERCION))>0) { + throw new IllegalStateException("Decimal value out of range; cannot convert "+ input +" to double"); + } + return candidate; + } + @SuppressWarnings("rawtypes") public void registerRecursiveIterableAdapters() { diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java index 844ef52..2cc6804 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java @@ -20,6 +20,8 @@ package org.apache.brooklyn.util.javalang.coerce; import java.lang.reflect.Method; +import java.math.BigDecimal; +import java.math.BigInteger; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.javalang.JavaClassNames; @@ -150,18 +152,28 @@ public class PrimitiveStringTypeCoercions { } else { isdouble = false; } + if (isdouble) { - if (targetWrapType == Character.class) return Maybe.of((T) Character.valueOf((char)d)); - if (targetWrapType == Byte.class) return Maybe.of((T) Byte.valueOf((byte)d)); - if (targetWrapType == Short.class) return Maybe.of((T) Short.valueOf((short)d)); - if (targetWrapType == Integer.class) return Maybe.of((T) Integer.valueOf((int)d)); - if (targetWrapType == Long.class) return Maybe.of((T) Long.valueOf((long)d)); - if (targetWrapType == Float.class) return Maybe.of((T) Float.valueOf((float)d)); if (targetWrapType == Double.class) return Maybe.of((T) Double.valueOf(d)); - return Maybe.absent(new IllegalStateException("Unexpected: sourceType="+sourceWrapType+"; targetType="+targetWrapType)); - } else { - return Maybe.absent(new IllegalStateException("Unexpected: sourceType="+sourceWrapType+"; targetType="+targetWrapType)); + + BigDecimal dd = BigDecimal.valueOf(d); + if (targetWrapType == Float.class) { + float candidate = (float)d; + if (dd.subtract(BigDecimal.valueOf(candidate)).abs().compareTo(BigDecimal.valueOf(CommonAdaptorTypeCoercions.DELTA_FOR_COERCION))>0) { + throw new IllegalStateException("Decimal value out of range; cannot convert "+ candidate +" to float"); + } + return Maybe.of((T) (Float) candidate); + } + + if (targetWrapType == Integer.class) return Maybe.of((T) Integer.valueOf((dd.intValueExact()))); + if (targetWrapType == Long.class) return Maybe.of((T) Long.valueOf(dd.longValueExact())); + if (targetWrapType == Short.class) return Maybe.of((T) Short.valueOf(dd.shortValueExact())); + if (targetWrapType == Byte.class) return Maybe.of((T) Byte.valueOf(dd.byteValueExact())); + + if (targetWrapType == Character.class) return Maybe.of((T) Character.valueOf((char)dd.intValueExact())); } + + return Maybe.absent(new IllegalStateException("Unexpected: sourceType="+sourceWrapType+"; targetType="+targetWrapType)); } public static boolean isPrimitiveOrBoxer(Class<?> type) {
