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 2021/08/24 16:00:06 UTC
[brooklyn-server] 03/03: be stricter on coercions that are
otherwise lossy and/or platform-dependent
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 <al...@cloudsoftcorp.com>
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) {