Fix for #4860, ConstructorDetector with single arg constructor not working since 2.18 (#4861)

Author: JooHyukKim

release-notes/CREDITS-2.x

@@ -1861,3 +1861,8 @@ Jonathan Mesny (@jmesny)
 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)

release-notes/VERSION-2.x

@@ -12,6 +12,10 @@ Project: jackson-databind
  (fix by Joo-Hyuk K)
 #4844: Fix wrapped array hanlding 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)
 

src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java

@@ -548,7 +548,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);

src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java

@@ -715,8 +715,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()) {
@@ -1013,10 +1012,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;
                     }

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