Merge branch '2.18' into 2.19

FasterXML · Dec 21, 2024 · ec24929 · ec24929
2 parents 18d4874 + 7d94c2f
commit ec24929
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 5 deletions.
diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -1865,6 +1865,11 @@ Stanislav Shcherbakov (@glorrian)
  * Contributed #4844: Fix wrapped array hanlding wrt `null` by `StdDeserializer`
   (2.18.3)
 
+Tomáš Poledný (@Saljack)
+ * Reported #4860: `ConstructorDetector.USE_PROPERTIES_BASED` does not work with
+   multiple constructors since 2.18
+  (2.18.3)
+
 Liam Feid (@fxshlein)
  * Contributed #1467: Support `@JsonUnwrapped` with `@JsonCreator`
   (2.19.0)

diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -43,6 +43,10 @@ Project: jackson-databind
  (fix by Joo-Hyuk K)
 #4844: Fix wrapped array handling wrt `null` by `StdDeserializer`
  (fix by Stanislav S)
+#4860: `ConstructorDetector.USE_PROPERTIES_BASED` does not work with
+  multiple constructors since 2.18
+ (reported by Tomáš P)
+ (fix by Joo-Hyuk K, @cowtowncoder)
 
 2.18.2 (27-Nov-2024)
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java
@@ -534,7 +534,7 @@ private void _addSelectedPropertiesBasedCreator(DeserializationContext ctxt,
                 if ((name == null) && (injectId == null)) {
                     ctxt.reportBadTypeDefinition(beanDesc,
 "Argument #%d of Creator %s has no property name (and is not Injectable): can not use as property-based Creator",
-i, candidate);
+                        i, candidate);
                 }
             }
             properties[i] = constructCreatorProperty(ctxt, beanDesc, name, i, param, injectId);

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java
@@ -711,8 +711,7 @@ protected void _addCreators(Map<String, POJOPropertyBuilder> props)
         }
 
         // One more thing: if neither explicit (constructor or factory) nor
-        // canonical (constructor?), consider implicit Constructor with
-        // all named.
+        // canonical (constructor?), consider implicit Constructor with all named.
         final ConstructorDetector ctorDetector = _config.getConstructorDetector();
         if (!creators.hasPropertiesBasedOrDelegating()
                 && !ctorDetector.requireCtorAnnotation()) {
@@ -1009,10 +1008,17 @@ private boolean _addImplicitConstructor(PotentialCreators collector,
                 if (ctorDetector.singleArgCreatorDefaultsToDelegating()) {
                     return false;
                 }
+                // 20-Dec-2024, tatu: [databind#4860] Cannot detect as properties-based
+                //   without implicit name (Injectable was checked earlier)
+                String implicitParamName = ctor.implicitNameSimple(0);
+                if (implicitParamName == null) {
+                    return false;
+                }
+
                 // if not, prefer Properties-based if explicit preference OR
                 // property with same name with at least one visible accessor
                 if (!ctorDetector.singleArgCreatorDefaultsToProperties()) {
-                    POJOPropertyBuilder prop = props.get(ctor.implicitNameSimple(0));
+                    POJOPropertyBuilder prop = props.get(implicitParamName);
                     if ((prop == null) || !prop.anyVisible() || prop.anyIgnorals()) {
                         return false;
                     }

diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetectorTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetectorTest.java
@@ -3,6 +3,7 @@
 import org.junit.jupiter.api.Test;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 import com.fasterxml.jackson.annotation.JsonSetter;
 import com.fasterxml.jackson.annotation.Nulls;
 
@@ -139,9 +140,24 @@ public Boolean field() {
             return field;
         }
     }
-
+
+    // [databind#4860]
+    @JsonPropertyOrder({ "id", "name "})
+    static class Foo4860 {
+        public String id;
+        public String name;
+
+        public Foo4860() { }
+
+        public Foo4860(String id) {
+            // should not be called as of Jackson 2.x
+            throw new IllegalStateException("Should not auto-detect args-taking constructor");
+        }
+    }
+
     private final ObjectMapper MAPPER_PROPS = mapperFor(ConstructorDetector.USE_PROPERTIES_BASED);
     private final ObjectMapper MAPPER_DELEGATING = mapperFor(ConstructorDetector.USE_DELEGATING);
+    private final ObjectMapper MAPPER_DEFAULT = mapperFor(ConstructorDetector.DEFAULT);
     private final ObjectMapper MAPPER_EXPLICIT = mapperFor(ConstructorDetector.EXPLICIT_ONLY);
 
     private final ObjectMapper MAPPER_MUST_ANNOTATE = mapperFor(ConstructorDetector.DEFAULT
@@ -379,6 +395,42 @@ void nullHandlingCreator3241() throws Exception {
         }
     }
 
+    // [databind#4860]
+    @Test
+    public void testDeserialization4860PropsBased() throws Exception {
+        _test4680With(MAPPER_PROPS);
+    }
+
+    @Test
+    public void testDeserialization4860Delegating() throws Exception {
+        _test4680With(MAPPER_DELEGATING);
+    }
+
+    @Test
+    public void testDeserialization4860Default() throws Exception {
+        _test4680With(MAPPER_DEFAULT);
+    }
+
+    @Test
+    public void testDeserialization4860Explicit() throws Exception {
+        _test4680With(MAPPER_EXPLICIT);
+    }
+
+    private void _test4680With(ObjectMapper mapper) throws Exception
+    {
+        _test4680With(mapper, "{}", a2q("{'id':null,'name':null}"));
+        _test4680With(mapper, a2q("{'id':'something'}"),
+                a2q("{'id':'something','name':null}"));
+        _test4680With(mapper, a2q("{'id':'something','name':'name'}"),
+                a2q("{'id':'something','name':'name'}"));
+    }
+
+    private void _test4680With(ObjectMapper mapper, String input, String output) throws Exception
+    {
+        Foo4860 result = mapper.readValue(input, Foo4860.class);
+        assertEquals(output, mapper.writeValueAsString(result));
+    }
+
     /*
     /**********************************************************************
     /* Helper methods