From d354028fd658b95448f5f15e2e263515878be14a Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 17 Feb 2020 15:08:35 -0800 Subject: [PATCH] Minor tweaks to fix for #1983, update release notes --- release-notes/CREDITS-2.x | 5 ++ release-notes/VERSION-2.x | 3 + .../impl/AsPropertyTypeDeserializer.java | 12 ++-- .../jsontype/impl/TypeNameIdResolver.java | 15 +++- .../JsonTypeInfoCaseInsensitive1983Test.java | 68 +++++++++++++++++++ .../JsonTypeInfoCaseInsensitive1983Test.java | 44 ------------ 6 files changed, 95 insertions(+), 52 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/jsontype/JsonTypeInfoCaseInsensitive1983Test.java delete mode 100644 src/test/java/com/fasterxml/jackson/failing/JsonTypeInfoCaseInsensitive1983Test.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 36098cdf7d..68ad0bf41e 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -1083,3 +1083,8 @@ Oleksii Khomchenko (gagoman@github) * Reported, contributed fix for #2592: `ObjectMapper.setSerializationInclusion()` is ignored for `JsonAnyGetter` (2.11.0) + +Oleksandr Poslavskyi (alevskyi@github) + * Contributed fix for #1983: Polymorphic deserialization should handle case-insensitive Type Id + property name if `MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES` is enabled + (2.11.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 43ff4a3983..9d5715ad3e 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -8,6 +8,9 @@ Project: jackson-databind #953: i-I case conversion problem in Turkish locale with case-insensitive deserialization (reported by Máté R) +#1983: Polymorphic deserialization should handle case-insensitive Type Id property name + if `MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES` is enabled + (reported by soundvibe@github, fix contributed by Oleksandr P) #2049: TreeTraversingParser and UTF8StreamJsonParser create contexts differently (reported by Antonio P) #2352: Support use of `@JsonAlias` for enum values diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/AsPropertyTypeDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/AsPropertyTypeDeserializer.java index 08dc5607d9..414245ea03 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/AsPropertyTypeDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/AsPropertyTypeDeserializer.java @@ -32,7 +32,7 @@ public AsPropertyTypeDeserializer(JavaType bt, TypeIdResolver idRes, { this(bt, idRes, typePropertyName, typeIdVisible, defaultImpl, As.PROPERTY); } - + /** * @since 2.8 */ @@ -89,15 +89,13 @@ public Object deserializeTypedFromObject(JsonParser p, DeserializationContext ct } // Ok, let's try to find the property. But first, need token buffer... TokenBuffer tb = null; - boolean ignoreCase = ctxt.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES); + for (; t == JsonToken.FIELD_NAME; t = p.nextToken()) { - String name = p.getCurrentName(); - if (ignoreCase) { - name = name.toLowerCase(); - } + final String name = p.getCurrentName(); p.nextToken(); // to point to the value - if (name.equals(_typePropertyName)) { // gotcha! + if (name.equals(_typePropertyName) + || (ignoreCase && name.equalsIgnoreCase(_typePropertyName))) { // gotcha! return _deserializeTypedForId(p, ctxt, tb); } if (tb == null) { diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java index f817ac6a5b..601afb5a2c 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java @@ -30,6 +30,11 @@ public class TypeNameIdResolver extends TypeIdResolverBase */ protected final Map _idToType; + /** + * @since 2.11 + */ + protected final boolean _caseInsensitive; + protected TypeNameIdResolver(MapperConfig config, JavaType baseType, ConcurrentHashMap typeToId, HashMap idToType) @@ -38,6 +43,7 @@ protected TypeNameIdResolver(MapperConfig config, JavaType baseType, _config = config; _typeToId = typeToId; _idToType = idToType; + _caseInsensitive = config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_VALUES); } public static TypeNameIdResolver construct(MapperConfig config, JavaType baseType, @@ -61,6 +67,8 @@ public static TypeNameIdResolver construct(MapperConfig config, JavaType base // for a single value. typeToId = new ConcurrentHashMap<>(4); } + final boolean caseInsensitive = config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_VALUES); + if (subtypes != null) { for (NamedType t : subtypes) { // no name? Need to figure out default; for now, let's just @@ -71,6 +79,10 @@ public static TypeNameIdResolver construct(MapperConfig config, JavaType base typeToId.put(cls.getName(), id); } if (forDeser) { + // [databind#1983]: for case-insensitive lookups must canonicalize: + if (caseInsensitive) { + id = id.toLowerCase(); + } // One more problem; sometimes we have same name for multiple types; // if so, use most specific JavaType prev = idToType.get(id); @@ -139,7 +151,8 @@ public JavaType typeFromId(DatabindContext context, String id) { } protected JavaType _typeFromId(String id) { - if(_config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES)) { + // [databind#1983]: for case-insensitive lookups must canonicalize: + if (_caseInsensitive) { id = id.toLowerCase(); } // Now: if no type is found, should we try to locate it by diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/JsonTypeInfoCaseInsensitive1983Test.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/JsonTypeInfoCaseInsensitive1983Test.java new file mode 100644 index 0000000000..fd89373ca0 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/JsonTypeInfoCaseInsensitive1983Test.java @@ -0,0 +1,68 @@ +package com.fasterxml.jackson.databind.jsontype; + +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; + +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.exc.InvalidTypeIdException; + +// Tests wrt [databind#1983] +public class JsonTypeInfoCaseInsensitive1983Test extends BaseMapTest +{ + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXTERNAL_PROPERTY, + property = "Operation") + @JsonSubTypes({ + @JsonSubTypes.Type(value = Equal.class, name = "eq"), + @JsonSubTypes.Type(value = NotEqual.class, name = "notEq"), + }) + static abstract class Filter { + } + + static class Equal extends Filter { } + + static class NotEqual extends Filter { } + + // verify failures when exact matching required: + private final ObjectMapper MAPPER = newJsonMapper(); + + public void testReadMixedCaseSubclass() throws Exception + { + final String serialised = "{\"Operation\":\"NoTeQ\"}"; + + // first: mismatch with value unless case-sensitivity disabled: + try { + MAPPER.readValue(serialised, Filter.class); + fail("Should not pass"); + } catch (InvalidTypeIdException e) { + verifyException(e, "Could not resolve type id 'NoTeQ'"); + } + + ObjectMapper mapper = jsonMapperBuilder() + .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_VALUES) + .build(); + // Type id ("value") mismatch, should work now: + Filter result = mapper.readValue(serialised, Filter.class); + + assertEquals(NotEqual.class, result.getClass()); + } + + public void testReadMixedCasePropertyName() throws Exception + { + final String serialised = "{\"oPeRaTioN\":\"notEq\"}"; + // first: mismatch with property name unless case-sensitivity disabled: + try { + MAPPER.readValue(serialised, Filter.class); + fail("Should not pass"); + } catch (InvalidTypeIdException e) { + verifyException(e, "Missing type id when trying to resolve subtype"); + } + + ObjectMapper mapper = jsonMapperBuilder() + .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES) + .build(); + // Type property name mismatch (but value match); should work: + Filter result = mapper.readValue(serialised, Filter.class); + + assertEquals(NotEqual.class, result.getClass()); + } +} diff --git a/src/test/java/com/fasterxml/jackson/failing/JsonTypeInfoCaseInsensitive1983Test.java b/src/test/java/com/fasterxml/jackson/failing/JsonTypeInfoCaseInsensitive1983Test.java deleted file mode 100644 index 1f0fdacd96..0000000000 --- a/src/test/java/com/fasterxml/jackson/failing/JsonTypeInfoCaseInsensitive1983Test.java +++ /dev/null @@ -1,44 +0,0 @@ -package com.fasterxml.jackson.failing; - -import com.fasterxml.jackson.annotation.JsonSubTypes; -import com.fasterxml.jackson.annotation.JsonTypeInfo; -import com.fasterxml.jackson.databind.BaseMapTest; -import com.fasterxml.jackson.databind.MapperFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import java.io.IOException; - -public class JsonTypeInfoCaseInsensitive1983Test extends BaseMapTest { - - @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXTERNAL_PROPERTY, property = "operation") - @JsonSubTypes({ - @JsonSubTypes.Type(value = Equal.class, name = "eq"), - @JsonSubTypes.Type(value = NotEqual.class, name = "noteq"), - }) - static abstract class Filter { - } - - static class Equal extends Filter { - } - - static class NotEqual extends Filter { - } - - public void testReadMixedCaseSubclass() throws IOException { -// There is also "ACCEPT_CASE_INSENSITIVE_VALUES" feature. Is it more suitable here, or I should leave it as is? - ObjectMapper mapper = objectMapper().enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES); - String serialised = "{\"operation\":\"NoTeQ\"}"; - - Filter result = mapper.readValue(serialised, Filter.class); - - assertEquals(NotEqual.class, result.getClass()); - } - - public void testReadMixedCasePropertyName() throws IOException { - ObjectMapper mapper = objectMapper().enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES); - String serialised = "{\"oPeRaTioN\":\"noteq\"}"; - - Filter result = mapper.readValue(serialised, Filter.class); - - assertEquals(NotEqual.class, result.getClass()); - } -}