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 44d3f2103eefb87d89ace74886fc37ef539c3911 Author: Alex Heneveld <[email protected]> AuthorDate: Sat Mar 18 11:59:28 2023 +0000 fallback to ignoring type fields when deserializing if appropriate (fix tests just added) this is done if the guessed type cannot be read, or if the type that is read doesn't match bounds --- .../resolve/jackson/AsPropertyIfAmbiguous.java | 81 ++++++++++++++++------ .../core/resolve/jackson/BeanWithTypeUtils.java | 22 ++++-- .../jackson/BrooklynJacksonSerializationUtils.java | 12 ++++ .../brooklyn/util/core/flags/TypeCoercions.java | 23 ++++++ ...klynRegisteredTypeJacksonSerializationTest.java | 25 +++---- 5 files changed, 120 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java index 6cc34dd269..113b586960 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java @@ -248,25 +248,66 @@ public class AsPropertyIfAmbiguous { TokenBuffer tb = null; boolean ignoreCase = ctxt.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES); - // changed to look for conflicting property first - Pair<String, TokenBuffer> typeIdFindResult = findTypeIdOrUnambiguous(p, ctxt, t, tb, ignoreCase, mustUseConflictingTypePrefix); - tb = typeIdFindResult.getRight(); -// if (typeIdFindResult.getLeft()==null && !mustUseConflictingTypePrefix) { -// p = tb.asParserOnFirstToken(); -// tb = null; -// typeIdFindResult = findTypeId(p, ctxt, t, tb, ignoreCase, _typePropertyName); -// } -// tb = typeIdFindResult.getRight(); - - if (typeIdFindResult.getLeft()!=null) return _deserializeTypedForId(p, ctxt, tb, typeIdFindResult.getLeft()); - else return _deserializeTypedUsingDefaultImpl(p, ctxt, tb, _msgForMissingId); + // will look for conflicting property first + + // cache it in case we need to rollback the type (this is a bit expensive; but can optimize later if problematic) + TokenBuffer tb0 = BrooklynJacksonSerializationUtils.createBufferForParserCurrentObject(p, ctxt); + p = tb0.asParserOnFirstToken(); //BrooklynJacksonSerializationUtils.createParserFromTokenBufferAndParser(tb0, p); + + DiscoveredTypeAndCachedTokenBuffer typeIdFindResult = findTypeIdOrUnambiguous(p, ctxt, t, tb, ignoreCase, mustUseConflictingTypePrefix); + tb = typeIdFindResult.tb; + + IOException preferredError = null; + if (typeIdFindResult.type!=null) { + boolean canTryWithoutType = !typeIdFindResult.isUnambiguous; + try { + Object result = _deserializeTypedForId(p, ctxt, tb, typeIdFindResult.type); + if (_idResolver instanceof HasBaseType) { + JavaType baseType = ((HasBaseType) _idResolver).getBaseType(); + if (baseType != null) { + Class<?> rawClass = baseType.getRawClass(); + if (rawClass != null && !rawClass.isAssignableFrom(result.getClass())) { + canTryWithoutType = true; // will be allow to try without a type, but prefer our error + preferredError = new IOException("Invalid result: deserialized type "+result.getClass()+" when expected "+baseType); + throw preferredError; + } + } + } + return result; + } catch (Exception e) { + if (!canTryWithoutType) throw e; + // if ambiguous then deserialize using default, below; but reset the parser first + p = tb0.asParserOnFirstToken(); + tb = tb0; + } + } + + try { + return _deserializeTypedUsingDefaultImpl(p, ctxt, tb, _msgForMissingId); + } catch (Exception e2) { + if (preferredError!=null) throw preferredError; + throw e2; + } + + } + + static class DiscoveredTypeAndCachedTokenBuffer { + String type; + TokenBuffer tb; + boolean isUnambiguous; + + DiscoveredTypeAndCachedTokenBuffer(String type, TokenBuffer tb, boolean isUnambiguous) { + this.type = type; + this.tb = tb; + this.isUnambiguous = isUnambiguous; + } } - private Pair<String,TokenBuffer> findTypeIdOrUnambiguous(JsonParser p, DeserializationContext ctxt, JsonToken t, TokenBuffer tb, boolean ignoreCase, boolean mustUseConflictingTypePrefix) throws IOException { + private DiscoveredTypeAndCachedTokenBuffer findTypeIdOrUnambiguous(JsonParser p, DeserializationContext ctxt, JsonToken t, TokenBuffer tb, boolean ignoreCase, boolean mustUseConflictingTypePrefix) throws IOException { String typeUnambiguous1 = CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM.apply(_typePropertyName); String typeUnambiguous2 = CONFLICTING_TYPE_NAME_PROPERTY_TRANSFORM_ALT.apply(_typePropertyName); - JsonLocation loc = p.currentTokenLocation(); //p.getCurrentLocation(); + int fieldsRead = 0; for (; t == JsonToken.FIELD_NAME; t = p.nextToken()) { final String name = p.currentName(); p.nextToken(); // to point to the value @@ -336,7 +377,7 @@ public class AsPropertyIfAmbiguous { if (_idResolver instanceof HasBaseType) { JavaType baseType = ((HasBaseType) _idResolver).getBaseType(); if (baseType==null || baseType.getRawClass().equals(Object.class)) { - if (loc.getCharOffset()<=1) { + if (fieldsRead==0) { // 'type' should be treated as a normal key when an object is expected, if type it references has a field 'type', // except if it is the first key in the definition, to facilitate messy places where we say 'type: xxx' as the definition if (warnedAmbiguousTypeProperty.add(typeId)) { @@ -361,7 +402,7 @@ public class AsPropertyIfAmbiguous { } } if (!disallowed) { - return Pair.of(typeId, tb); + return new DiscoveredTypeAndCachedTokenBuffer(typeId, tb, unambiguousName); } } } @@ -372,9 +413,9 @@ public class AsPropertyIfAmbiguous { tb.copyCurrentStructure(p); // advance so we no longer think we are at the beginning - loc = p.getCurrentLocation(); + fieldsRead++; } - return Pair.of(null, tb); + return new DiscoveredTypeAndCachedTokenBuffer(null, tb, true); } private boolean presentAndNotJsonIgnored(Maybe<? extends AccessibleObject> fm) { @@ -440,8 +481,8 @@ public class AsPropertyIfAmbiguous { p = JsonParserSequence.createFlattened(false, tb.asParser(p), p); } - if (((JsonParser)p).currentToken() != JsonToken.END_OBJECT) { - ((JsonParser)p).nextToken(); + if (p.currentToken() != JsonToken.END_OBJECT) { + p.nextToken(); } boolean wasEndToken = (p.currentToken() == JsonToken.END_OBJECT); diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java index 11049c2842..2e0462e9b9 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java @@ -165,26 +165,32 @@ public class BeanWithTypeUtils { mapper = BeanWithTypeUtils.applyCommonMapperConfig(mapper, mgmt, allowRegisteredTypes, loader, allowJavaTypes); mapper = new ObjectReferencingSerialization().useAndApplytoMapper(mapper); - String serialization = mapper.writeValueAsString(mapOrListToSerializeThenDeserialize); + String serialization = type.getRawType().equals(Object.class) ? mapper.writeValueAsString(mapOrListToSerializeThenDeserialize) : mapper.writerFor(Object.class).writeValueAsString(mapOrListToSerializeThenDeserialize); return mapper.readValue(serialization, BrooklynJacksonType.asJavaType(mapper, type)); } @Beta public static <T> T convertDeeply(ManagementContext mgmt, Object mapOrListToSerializeThenDeserialize, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes) throws JsonProcessingException { // try full serialization - but won't work if things being written cannot be deserialized, eg due to unknown type - ObjectMapper m = newMapper(mgmt, allowRegisteredTypes, loader, allowJavaTypes); - String serialization = m.writeValueAsString(mapOrListToSerializeThenDeserialize); - return m.readValue(serialization, BrooklynJacksonType.asJavaType(m, type)); + ObjectMapper mapper = newMapper(mgmt, allowRegisteredTypes, loader, allowJavaTypes); + String serialization = type.getRawType().equals(Object.class) ? mapper.writeValueAsString(mapOrListToSerializeThenDeserialize) : mapper.writerFor(Object.class).writeValueAsString(mapOrListToSerializeThenDeserialize); + return mapper.readValue(serialization, BrooklynJacksonType.asJavaType(mapper, type)); } public static <T> Maybe<T> tryConvertOrAbsentUsingContext(Maybe<Object> input, TypeToken<T> type) { + return tryConvertOrAbsentUsingContext(input, type, false); + } + public static <T> Maybe<T> tryConvertOrAbsentUsingContext(Maybe<Object> input, TypeToken<T> type, boolean allowNonMapComplexInput) { Entity entity = BrooklynTaskTags.getContextEntity(Tasks.current()); ManagementContext mgmt = entity != null ? ((EntityInternal) entity).getManagementContext() : null; BrooklynClassLoadingContext loader = entity != null ? RegisteredTypes.getClassLoadingContext(entity) : null; - return BeanWithTypeUtils.tryConvertOrAbsent(mgmt, input, type, true, loader, true); + return BeanWithTypeUtils.tryConvertOrAbsent(mgmt, input, type, true, loader, true, allowNonMapComplexInput); } public static <T> Maybe<T> tryConvertOrAbsent(ManagementContext mgmt, Maybe<Object> inputMap, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes) { + return tryConvertOrAbsent(mgmt, inputMap, type, allowRegisteredTypes, loader, allowJavaTypes, false); + } + public static <T> Maybe<T> tryConvertOrAbsent(ManagementContext mgmt, Maybe<Object> inputMap, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes, boolean allowNonMapComplexInput) { if (inputMap.isAbsent()) return (Maybe<T>)inputMap; Object o = inputMap.get(); @@ -192,7 +198,9 @@ public class BeanWithTypeUtils { if (type.isSupertypeOf(o.getClass())) { return (Maybe<T>)inputMap; } else { - return Maybe.absent(() -> new RuntimeException("BeanWithType cannot convert from "+o.getClass()+" to "+type)); + if (!allowNonMapComplexInput) { + return Maybe.absent(() -> new RuntimeException("BeanWithType cannot convert from " + o.getClass() + " to " + type)); + } // else continue below } } @@ -204,7 +212,7 @@ public class BeanWithTypeUtils { // there isn't a 'type' key so little obvious point in converting .. might make a difference _inside_ a map or list, but we've not got any generics so it won't if (!(o instanceof Map) || !((Map<?, ?>) o).containsKey(BrooklynJacksonSerializationUtils.TYPE)) return fallback; - } else if (type.isSupertypeOf(Map.class)) { + } else if (type.isSupertypeOf(Map.class) && o instanceof Map) { // skip conversion for a map if it isn't an object return (Maybe<T>) inputMap; } diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonSerializationUtils.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonSerializationUtils.java index 240e914b64..4134a718c3 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonSerializationUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonSerializationUtils.java @@ -82,6 +82,18 @@ public class BrooklynJacksonSerializationUtils { return deser; } + static void dumpParser(String header, JsonParser p) { + try { + System.out.println(header); + while (true) { + System.out.println(p.getCurrentToken() + " - " + p.currentName() + " - " + p.getValueAsString()); + if (p.nextToken()==null) return; + } + } catch (IOException e) { + e.printStackTrace(); + } + } + /** per TokenBuffer.asCopyOfValue but if on a field _name_ it buffers all the key-values */ @Beta public static TokenBuffer createBufferForParserCurrentObject(JsonParser parser, DeserializationContext optionalCtxtForFeatures) throws IOException { diff --git a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java index 7dfbc11dd0..0e3fa09bd3 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java @@ -375,6 +375,29 @@ public class TypeCoercions { return null; } }); + + registerAdapter("81-wrong-bean-to-map-or-bean", new TryCoercer() { + + @Override + public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) { + if (input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrBoxedObject(input)) { + return null; + } + // input is a complex type / bean + boolean toMap = Map.class.isAssignableFrom(type.getRawType()); + boolean toBeanWithType = !toMap && Reflections.findFieldMaybe(type.getRawType(), "type").isPresentAndNonNull(); + if (!toMap && !toBeanWithType) { + return null; + } + try { + Maybe<Map> resultMap = BeanWithTypeUtils.tryConvertOrAbsentUsingContext(Maybe.of(input), new TypeToken<Map>() {}, true); + if (toMap || resultMap.isAbsentOrNull()) return (Maybe<T>) resultMap; + return BeanWithTypeUtils.tryConvertOrAbsentUsingContext(Maybe.cast(resultMap), type); + } catch (Exception e) { + return Maybe.absent(e); + } + } + }); } } diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java index 71429e36e4..dc9a429904 100644 --- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java @@ -218,22 +218,15 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt Asserts.assertFailsWith(()->deser(deser3, OtherBean.class), e->true); // expected as y doesn't exist on OtherBean Asserts.assertInstanceOf(deser(deser3, SampleBeanWithType.class), SampleBeanWithType.class); Asserts.assertInstanceOf(deser(deser3), Map.class); - - // might be nice to support this but that might break maps containing [type] ?? what happens if we serialize a map containing type as an object? -// String deser4 = "{\"[type]\":\"" + OtherBean.class.getName() + "\",\"y\":\"hello\"}"; -// Asserts.assertFailsWith(()->deser(deser3, OtherBean.class), e->true); -// Asserts.assertInstanceOf(deser(deser3, SampleBeanWithType.class), SampleBeanWithType.class); -// Asserts.assertInstanceOf(deser(deser3), Map.class); -// Asserts.assertEquals( ((Map)deser(deser3)).get("type"), OtherBean.class.getName()); -// Asserts.assertNull( ((Map)deser(deser3)).get("[type]") ); -// Asserts.assertInstanceOf(deser(deser3, Map.class), Map.class); -// Asserts.assertEquals( ((Map)deser(deser3)).get("[type]"), OtherBean.class.getName()); -// Asserts.assertNull( ((Map)deser(deser3)).get("type") ); - // above can't work due to ambiguity with below -// Asserts.assertEquals(ser(MutableList.of(MutableMap.of("type", OtherBean.class.getName(), "x", "hello"))), "[{\"type\":\""+OtherBean.class.getName()+"\",\"x\":\"hello\"}]"); - - // TODO however we could allow this - coercer may try to serialize first then deserialize second if given two complex types where the second one has a field 'type' -// SampleBeanWithType redeser = TypeCoercions.coerce(deser(deser), SampleBeanWithType.class); + Asserts.assertEquals( ((Map)deser(deser3)).get("type"), OtherBean.class.getName()); + + // we have no choice but to fallback to map deserialization + // however we should allow further coercion to Map (in case we read as typed something which should have been a map) + // and also coercion that serializes input if complex type then deserializes to intended type, if the intended type has a field 'type' + Map redeserMap = TypeCoercions.coerce(deser(deser), Map.class); + Asserts.assertEquals(redeserMap.get("type"), OtherBean.class.getName()); + SampleBeanWithType redeserObj = TypeCoercions.coerce(deser(deser), SampleBeanWithType.class); + Asserts.assertEquals(redeserObj.x, "hello"); } @Test
