Fix #2979 as suggested by PR (manual merge)

FasterXML · Dec 22, 2020 · f920a16 · f920a16
1 parent a2e914c
commit f920a16
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 13 deletions.
diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -1285,9 +1285,13 @@ Halil İbrahim Şener (hisener@github)
    (2.12.1)
 
 Faron Dutton (fdutton@github)
-
   * Contributed fix for #2990: Breaking API change in `BasicClassIntrospector` (2.12.0)
    (2.12.1)
 
---- END ---
+SunYiJun (xiaosunzhu@github)
 
+  * Reported, suggested fix for #2979: Conflicting in POJOPropertiesCollector when
+    having namingStrategy
+   (2.12.1)
+
+--- END ---
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -15,6 +15,8 @@ Project: jackson-databind
  (reported by zigzago@github)
 #2978: Fix failing `double` JsonCreators in jackson 2.12.0
  (contributed by Carter K)
+#2979: Conflicting in POJOPropertiesCollector when having namingStrategy
+ (reported, fix suggested by SunYiJun)
 #2990: Breaking API change in `BasicClassIntrospector` (2.12.0)
  (reported, fix contributed by Faron D)
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java
@@ -446,18 +446,19 @@ protected void collectAll()
             property.mergeAnnotations(_forSerialization);
         }
 
-        // And use custom naming strategy, if applicable...
-        PropertyNamingStrategy naming = _findNamingStrategy();
-        if (naming != null) {
-            _renameUsing(props, naming);
-        }
-
         // Sort by visibility (explicit over implicit); drop all but first of member
         // type (getter, setter etc) if there is visibility difference
         for (POJOPropertyBuilder property : props.values()) {
             property.trimByVisibility();
         }
 
+        // And use custom naming strategy, if applicable...
+        // As per [databind#2979], should be AFTER trimming
+        PropertyNamingStrategy naming = _findNamingStrategy();
+        if (naming != null) {
+            _renameUsing(props, naming);
+        }
+
         // and, if required, apply wrapper name: note, MUST be done after
         // annotations are merged.
         if (_config.isEnabled(MapperFeature.USE_WRAPPER_NAME_AS_PROPERTY_NAME)) {
@@ -1051,9 +1052,8 @@ protected void _renameUsing(Map<String, POJOPropertyBuilder> propMap,
                     } else if (prop.hasField()) {
                         rename = naming.nameForField(_config, prop.getField(), fullName.getSimpleName());
                     } else if (prop.hasGetter()) {
-                        /* Plus, when getter-as-setter is used, need to convert that too..
-                         * (should we verify that's enabled? For now, assume it's ok always)
-                         */
+                        // Plus, when getter-as-setter is used, need to convert that too..
+                        // (should we verify that's enabled? For now, assume it's ok always)
                         rename = naming.nameForGetterMethod(_config, prop.getGetter(), fullName.getSimpleName());
                     }
                 }

diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/SetterConflictTest.java b/src/test/java/com/fasterxml/jackson/databind/introspect/SetterConflictTest.java
@@ -1,5 +1,6 @@
 package com.fasterxml.jackson.databind.introspect;
 
+import com.fasterxml.jackson.annotation.JsonSetter;
 import com.fasterxml.jackson.databind.*;
 
 // mostly for [databind#1033]
@@ -15,18 +16,44 @@ public void setValue(Issue1033Bean foo) {
         }
     }
 
+    // [databind#2979]
+    static class DuplicateSetterBean2979 {
+        Object value;
+
+        public void setBloop(Boolean bloop) {
+            throw new Error("Wrong setter!");
+        }
+
+        @JsonSetter
+        public void setBloop(Object bloop) {
+            value = bloop;
+        }
+    }
+
     /*
     /**********************************************************
     /* Test methods
     /**********************************************************
      */
 
-    private final ObjectMapper MAPPER = objectMapper();
+    private final ObjectMapper MAPPER = newJsonMapper();
 
+    // [databind#1033]
     public void testSetterPriority() throws Exception
     {
-        Issue1033Bean bean = MAPPER.readValue(aposToQuotes("{'value':42}"),
+        Issue1033Bean bean = MAPPER.readValue(a2q("{'value':42}"),
                 Issue1033Bean.class);
         assertEquals(42, bean.value);
     }
+
+    // [databind#2979]
+    public void testConflictingSetters() throws Exception
+    {
+        ObjectMapper mapper = jsonMapperBuilder()
+                .propertyNamingStrategy(PropertyNamingStrategies.LOWER_CAMEL_CASE)
+                .build();
+        DuplicateSetterBean2979 result = mapper.readValue(a2q("{'bloop':true}"),
+                DuplicateSetterBean2979.class);
+        assertEquals(Boolean.TRUE, result.value);
+    }
 }