From eeb5f2b764a505c91fda5c55620e60da30676152 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 6 Mar 2024 19:53:09 -0800 Subject: [PATCH] Refactoring JsonFormat detection, post #4409 (#4418) --- release-notes/CREDITS-2.x | 15 ++++--- release-notes/VERSION-2.x | 4 ++ .../jackson/databind/BeanDescription.java | 18 ++++++++- .../databind/deser/BeanDeserializerBase.java | 2 +- .../deser/BeanDeserializerBuilder.java | 2 +- .../databind/deser/DeserializerCache.java | 4 +- .../introspect/BasicBeanDescription.java | 27 +++---------- .../introspect/POJOPropertiesCollector.java | 39 +++++++++++++++++++ .../introspect/POJOPropertyBuilder.java | 5 +-- .../databind/ser/BasicSerializerFactory.java | 10 ++--- .../databind/ser/std/BeanSerializerBase.java | 2 +- .../enums/EnumWithNamingStrategy4409Test.java | 38 ++++++++++++++++++ 12 files changed, 124 insertions(+), 42 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumWithNamingStrategy4409Test.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index b6190436be..79a108cf4a 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -1730,20 +1730,25 @@ Jan Pachol (janpacho@github) (2.16.0) Pieter Dirk Soels (Badbond@github) - * Reprted #4302: Problem deserializing some type of Enums when using + * Reported #4302: Problem deserializing some type of Enums when using `PropertyNamingStrategy` (2.16.2) +Stephane Bailliez (sbailliez@github) + * Reported #4409: Deserialization of enums with name defined with different cases + leads to `InvalidDefinitionException`: Multiple fields representing property + (2.16.2) + Muhammad Khalikov (mukham12@github) * Contributed fix for #4209: Make `BeanDeserializerModifier`/`BeanSerializerModifier` - implement `java.io.Serializable` - (2.17.0) + implement `java.io.Serializable` + (2.17.0) Eduard Dudar (edudar@github) * Contributed #4299: Some `Collection` and `Map` fallbacks don't work in GraalVM native image - (2.17.0) + (2.17.0) Jesper Blomquist (jebl01@github) * Contributed #4393: Deserialize `java.util.UUID` encoded as Base64 and base64Url with or without padding - (2.17.0) + (2.17.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index d9937bb0b1..c1cf248b5e 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -61,6 +61,10 @@ Project: jackson-databind #4355: Jackson 2.16 fails attempting to obtain `ObjectWriter` for an `Enum` of which some value returns null from `toString()` (reported by @YutaHiguchi-bsn) +#4409: Deserialization of enums with name defined with different cases leads to + `InvalidDefinitionException`: Multiple fields representing property + (reported by Stephane B) + (fix contributed by Joo-Hyuk K) 2.16.1 (24-Dec-2023) diff --git a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java index dc256d13b3..603ed87437 100644 --- a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java @@ -305,9 +305,25 @@ public AnnotatedMember findAnySetterField() { * defined by defaults and possible annotations. * Note that this may be further refined by per-property annotations. * + * @since 2.17 + */ + public abstract JsonFormat.Value findExpectedFormat(); + + /** * @since 2.1 + * @deprecated Since 2.17 use {@link #findExpectedFormat()} */ - public abstract JsonFormat.Value findExpectedFormat(JsonFormat.Value defValue); + @Deprecated // since 2.17 + public JsonFormat.Value findExpectedFormat(JsonFormat.Value defValue) { + JsonFormat.Value v = findExpectedFormat(); + if (defValue == null) { + return v; + } + if (v == null) { + return defValue; + } + return defValue.withOverrides(v); + } /** * Method for finding {@link Converter} used for serializing instances diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java index aa0c3fd1a2..3007714a1d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java @@ -251,7 +251,7 @@ protected BeanDeserializerBase(BeanDeserializerBuilder builder, ; // Any transformation we may need to apply? - final JsonFormat.Value format = beanDesc.findExpectedFormat(null); + final JsonFormat.Value format = beanDesc.findExpectedFormat(); _serializationShape = format.getShape(); _needViewProcesing = hasViews; diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBuilder.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBuilder.java index 87fd85d31f..f05666a764 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBuilder.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBuilder.java @@ -596,7 +596,7 @@ protected Map> _collectAliases(Collection _createDeserializer2(DeserializationContext ctxt, // Ideally we'd determine it bit later on (to allow custom handler checks) // but that won't work for other reasons. So do it here. // (read: rewrite for 3.0) - JsonFormat.Value format = beanDesc.findExpectedFormat(null); + JsonFormat.Value format = beanDesc.findExpectedFormat(); if (format.getShape() != JsonFormat.Shape.OBJECT) { MapLikeType mlt = (MapLikeType) type; if (mlt instanceof MapType) { @@ -421,7 +421,7 @@ protected JsonDeserializer _createDeserializer2(DeserializationContext ctxt, * (to allow custom handler checks), but that won't work for other * reasons. So do it here. */ - JsonFormat.Value format = beanDesc.findExpectedFormat(null); + JsonFormat.Value format = beanDesc.findExpectedFormat(); if (format.getShape() != JsonFormat.Shape.OBJECT) { CollectionLikeType clt = (CollectionLikeType) type; if (clt instanceof CollectionType) { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java index c03e71045a..20c79f6e7e 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java @@ -405,30 +405,13 @@ public AnnotatedMethod findMethod(String name, Class[] paramTypes) { /********************************************************** */ - @Override - public JsonFormat.Value findExpectedFormat(JsonFormat.Value defValue) + @Override // since 2.17 + public JsonFormat.Value findExpectedFormat() { - // 15-Apr-2016, tatu: Let's check both per-type defaults and annotations; per-type - // defaults having higher precedence, so start with that - if (_annotationIntrospector != null) { - JsonFormat.Value v = _annotationIntrospector.findFormat(_classInfo); - if (v != null) { - if (defValue == null) { - defValue = v; - } else { - defValue = defValue.withOverrides(v); - } - } + if (_propCollector == null) { + return JsonFormat.Value.empty(); } - JsonFormat.Value v = _config.getDefaultPropertyFormat(_classInfo.getRawType()); - if (v != null) { - if (defValue == null) { - defValue = v; - } else { - defValue = defValue.withOverrides(v); - } - } - return defValue; + return _propCollector.getFormatOverrides(); } @Override // since 2.9 diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 61961db4db..4f8d64b7a2 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.cfg.HandlerInstantiator; import com.fasterxml.jackson.databind.cfg.MapperConfig; @@ -144,6 +145,13 @@ public class POJOPropertiesCollector */ protected LinkedHashMap _injectables; + /** + * Lazily accessed information about POJO format overrides + * + * @since 2.17 + */ + protected JsonFormat.Value _formatOverrides; + // // // Deprecated entries to remove from 3.0 /** @@ -421,6 +429,31 @@ public Class findPOJOBuilderClass() { return _annotationIntrospector.findPOJOBuilder(_classDef); } + /** + * @since 2.17 + */ + public JsonFormat.Value getFormatOverrides() { + if (_formatOverrides == null) { + JsonFormat.Value format = null; + + // Let's check both per-type defaults and annotations; + // per-type defaults having higher precedence, so start with annotations + if (_annotationIntrospector != null) { + format = _annotationIntrospector.findFormat(_classDef); + } + JsonFormat.Value v = _config.getDefaultPropertyFormat(_type.getRawClass()); + if (v != null) { + if (format == null) { + format = v; + } else { + format = format.withOverrides(v); + } + } + _formatOverrides = (format == null) ? JsonFormat.Value.empty() : format; + } + return _formatOverrides; + } + /* /********************************************************** /* Public API: main-level collection @@ -1123,6 +1156,12 @@ protected void _renameProperties(Map props) protected void _renameUsing(Map propMap, PropertyNamingStrategy naming) { + // [databind#4409]: Need to skip renaming for Enums, unless Enums are handled as OBJECT format + if (_type.isEnumType()) { + if (getFormatOverrides().getShape() != JsonFormat.Shape.OBJECT) { + return; + } + } POJOPropertyBuilder[] props = propMap.values().toArray(new POJOPropertyBuilder[propMap.size()]); propMap.clear(); for (POJOPropertyBuilder prop : props) { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java index 50fd3fd7a0..75f201741c 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java @@ -3,10 +3,7 @@ import java.util.*; import java.util.stream.Collectors; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonSetter; -import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.annotation.*; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.cfg.ConfigOverride; diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java index bcccd9eaa0..e230d89529 100644 --- a/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java @@ -467,7 +467,7 @@ protected final JsonSerializer findSerializerByPrimaryType(SerializerProvider } if (Number.class.isAssignableFrom(raw)) { // 21-May-2014, tatu: Couple of alternatives actually - JsonFormat.Value format = beanDesc.findExpectedFormat(null); + JsonFormat.Value format = beanDesc.findExpectedFormat(); switch (format.getShape()) { case STRING: return ToStringSerializer.instance; @@ -713,7 +713,7 @@ protected JsonSerializer buildCollectionSerializer(SerializerProvider prov, if (ser == null) { // We may also want to use serialize Collections "as beans", if (and only if) // this is specified with `@JsonFormat(shape=Object)` - JsonFormat.Value format = beanDesc.findExpectedFormat(null); + JsonFormat.Value format = beanDesc.findExpectedFormat(); if (format.getShape() == JsonFormat.Shape.OBJECT) { return null; } @@ -803,7 +803,7 @@ protected JsonSerializer buildMapSerializer(SerializerProvider prov, { // [databind#467]: This is where we could allow serialization "as POJO": But! It's // nasty to undo, and does not apply on per-property basis. So, hardly optimal - JsonFormat.Value format = beanDesc.findExpectedFormat(null); + JsonFormat.Value format = beanDesc.findExpectedFormat(); if (format.getShape() == JsonFormat.Shape.OBJECT) { return null; } @@ -924,7 +924,7 @@ protected JsonSerializer buildMapEntrySerializer(SerializerProvider prov, // [databind#865]: Allow serialization "as POJO" -- note: to undo, declare // serialization as `Shape.NATURAL` instead; that's JSON Object too. JsonFormat.Value formatOverride = prov.getDefaultPropertyFormat(Map.Entry.class); - JsonFormat.Value formatFromAnnotation = beanDesc.findExpectedFormat(null); + JsonFormat.Value formatFromAnnotation = beanDesc.findExpectedFormat(); JsonFormat.Value format = JsonFormat.Value.merge(formatFromAnnotation, formatOverride); if (format.getShape() == JsonFormat.Shape.OBJECT) { return null; @@ -1202,7 +1202,7 @@ protected JsonSerializer buildEnumSerializer(SerializationConfig config, * POJO style serialization, so we must handle that special case separately; * otherwise pass it to EnumSerializer. */ - JsonFormat.Value format = beanDesc.findExpectedFormat(null); + JsonFormat.Value format = beanDesc.findExpectedFormat(); if (format.getShape() == JsonFormat.Shape.OBJECT) { // one special case: suppress serialization of "getDeclaringClass()"... ((BasicBeanDescription) beanDesc).removeProperty("declaringClass"); diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/std/BeanSerializerBase.java b/src/main/java/com/fasterxml/jackson/databind/ser/std/BeanSerializerBase.java index 1306ad4fd8..742d14d44e 100644 --- a/src/main/java/com/fasterxml/jackson/databind/ser/std/BeanSerializerBase.java +++ b/src/main/java/com/fasterxml/jackson/databind/ser/std/BeanSerializerBase.java @@ -128,7 +128,7 @@ protected BeanSerializerBase(JavaType type, BeanSerializerBuilder builder, _anyGetterWriter = builder.getAnyGetter(); _propertyFilterId = builder.getFilterId(); _objectIdWriter = builder.getObjectIdWriter(); - final JsonFormat.Value format = builder.getBeanDescription().findExpectedFormat(null); + final JsonFormat.Value format = builder.getBeanDescription().findExpectedFormat(); _serializationShape = format.getShape(); } } diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumWithNamingStrategy4409Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumWithNamingStrategy4409Test.java new file mode 100644 index 0000000000..3e411f41ef --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumWithNamingStrategy4409Test.java @@ -0,0 +1,38 @@ +package com.fasterxml.jackson.databind.deser.enums; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.PropertyNamingStrategies; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import static com.fasterxml.jackson.databind.BaseMapTest.jsonMapperBuilder; + +// [databind#4409]: PropertyNamingStrategy should not affect to Enums +class EnumWithNamingStrategy4409Test { + + enum ColorMode { + RGB, + RGBa, + RGBA + } + + static class Bug { + public ColorMode colorMode; + } + + @Test + public void testEnumAndPropertyNamingStrategy() throws Exception { + ObjectMapper mapper = jsonMapperBuilder() + .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE) + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); + + Bug bug = mapper.readValue("{ \"color_mode\": \"RGBa\"}", Bug.class); + + // fails + assertEquals(ColorMode.RGBa, bug.colorMode); + } +} \ No newline at end of file