Repository: brooklyn-server Updated Branches: refs/heads/master b294f4170 -> 84eba7e31
BROOKLYN-406: log.warn if coerce collection<generics> fails Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/ab42d106 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/ab42d106 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/ab42d106 Branch: refs/heads/master Commit: ab42d1068223643f489874f77d35db4813fbbfc4 Parents: 3985115 Author: Aled Sage <aled.s...@gmail.com> Authored: Fri Dec 2 21:56:55 2016 +0000 Committer: Aled Sage <aled.s...@gmail.com> Committed: Thu Jan 5 16:50:39 2017 +0000 ---------------------------------------------------------------------- .../javalang/coerce/TypeCoercerExtensible.java | 13 ++++++++ .../util/javalang/coerce/TypeCoercionsTest.java | 33 ++++++++++++++++++++ 2 files changed, 46 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/ab42d106/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java index 2d5525f..366ae26 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java @@ -120,8 +120,21 @@ public class TypeCoercerExtensible implements TypeCoercer { if (targetTypeToken.getType() instanceof ParameterizedType) { if (value instanceof Collection && Collection.class.isAssignableFrom(targetType)) { result = tryCoerceCollection(value, targetTypeToken, targetType); + + if (result != null && result.isAbsent() && targetType.isInstance(value)) { + log.warn("Failed to coerce collection from " + value.getClass().getName() + " to " + targetTypeToken + + "; returning uncoerced result to preserve (deprecated) backwards compatibility", + Maybe.getException(result)); + } + } else if (value instanceof Map && Map.class.isAssignableFrom(targetType)) { result = tryCoerceMap(value, targetTypeToken); + + if (result != null && result.isAbsent() && targetType.isInstance(value)) { + log.warn("Failed to coerce map from " + value.getClass().getName() + " to " + targetTypeToken + + "; returning uncoerced result to preserve (deprecated) backwards compatibility", + Maybe.getException(result)); + } } } if (result!=null && result.isPresent()) return result; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/ab42d106/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java ---------------------------------------------------------------------- diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java index 78211e2..a1dc5a5 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java @@ -363,6 +363,39 @@ public class TypeCoercionsTest { } } + // TODO Asserting this undesirable behaviur, to preserve backwards compatibility! + // Would much prefer that we fail-fast. However, I worry that some entity's java declared + // ConfigKey<List<Foo>>, and rely on erasure so they can pass in other representations and + // coerce it themsevles in some way. + // I checked things like JcloudsLocation.JCLOUDS_LOCATION_CUSTOMIZERS - those look ok. + // + // Expect to get a log.warn about that now. Could assert that using LogWatcher. + @Test + public void testListOfFromThrowingException() { + TypeToken<List<WithFromThrowingException>> typeToken = new TypeToken<List<WithFromThrowingException>>() {}; + List<String> rawVal = ImmutableList.of("myval"); + + List<WithFromThrowingException> val = coercer.coerce(rawVal, typeToken); + assertEquals(val, rawVal); + + List<WithFromThrowingException> val2 = coercer.tryCoerce(rawVal, typeToken).get(); + assertEquals(val, rawVal); + } + + // See comment on testListOfFromThrowingException for why we're asserting this undesirable + // behaviour. + @Test + public void testMapOfFromThrowingException() { + TypeToken<Map<String, WithFromThrowingException>> typeToken = new TypeToken<Map<String, WithFromThrowingException>>() {}; + ImmutableMap<String, String> rawVal = ImmutableMap.of("mykey", "myval"); + + Map<String, WithFromThrowingException> val = coercer.coerce(rawVal, typeToken); + assertEquals(val, rawVal); + + Map<String, WithFromThrowingException> val2 = coercer.tryCoerce(rawVal, typeToken).get(); + assertEquals(val2, rawVal); + } + @Test public void testCoerceStringToNumber() { assertEquals(coerce("1", Number.class), (Number) Double.valueOf(1));
