Refactoring JsonFormat detection, post #4409 (#4418)

FasterXML · Mar 7, 2024 · eeb5f2b · eeb5f2b
1 parent ef34d13
commit eeb5f2b
Show file tree

Hide file tree

Showing 12 changed files with 124 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -596,7 +596,7 @@ protected Map<String,List<PropertyName>> _collectAliases(Collection<SettableBean
     protected boolean _findCaseInsensitivity() {
         // 07-May-2020, tatu: First find combination of per-type config overrides (higher
         //   precedence) and per-type annotations (lower):
-        JsonFormat.Value format = _beanDesc.findExpectedFormat(null);
+        JsonFormat.Value format = _beanDesc.findExpectedFormat();
         // and see if any of those has explicit definition; if not, use global baseline default
         Boolean B = format.getFeature(JsonFormat.Feature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
         return (B == null) ? _config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES)

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java
@@ -406,7 +406,7 @@ protected JsonDeserializer<?> _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
@@ -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
@@ -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<Object, AnnotatedMember> _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<String, POJOPropertyBuilder> props)
     protected void _renameUsing(Map<String, POJOPropertyBuilder> 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
@@ -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
@@ -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
@@ -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
@@ -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);
+    }
+}