Merge #4515 from 2.18 to master

src/main/java/tools/jackson/databind/introspect/POJOPropertiesCollector.java

@@ -609,6 +609,8 @@ protected void _addCreators(Map<String, POJOPropertyBuilder> props)
         // Next: remove creators marked as explicitly disabled
         _removeDisabledCreators(constructors);
         _removeDisabledCreators(factories);
+        // And then remove non-annotated static methods that do not look like factories
+        _removeNonFactoryStaticMethods(factories);
 
         // and use annotations to find explicitly chosen Creators
         if (_useAnnotations) { // can't have explicit ones without Annotation introspection
@@ -630,9 +632,30 @@ protected void _addCreators(Map<String, POJOPropertyBuilder> props)
         // (JDK 17 Record/Scala/Kotlin)
         if (!creators.hasPropertiesBased()) {
             // for Records:
-            if ((canonical != null) && constructors.contains(canonical)) {
-                constructors.remove(canonical);
-                creators.setPropertiesBased(_config, canonical, "canonical");
+            if (canonical != null) {
+                // ... but only process if still included as a candidate
+                if (constructors.remove(canonical)) {
+                    // But wait! Could be delegating
+                    if (_isDelegatingConstructor(canonical)) {
+                        creators.addExplicitDelegating(canonical);
+                    } else {
+                        creators.setPropertiesBased(_config, canonical, "canonical");
+                    }
+                }
+            }
+        }
+
+        // One more thing: if neither explicit (constructor or factory) nor
+        // canonical (constructor?), consider implicit Constructor with
+        // all named.
+        final ConstructorDetector ctorDetector = _config.getConstructorDetector();
+        if (!creators.hasPropertiesBasedOrDelegating()
+                && !ctorDetector.requireCtorAnnotation()) {
+            // But only if no default constructor available OR if we are configured
+            // to prefer properties-based Creators
+            if ((_classDef.getDefaultConstructor() == null)
+                    || ctorDetector.singleArgCreatorDefaultsToProperties()) {
+                _addImplicitConstructor(creators, constructors, props);
             }
         }
 
@@ -650,6 +673,20 @@ protected void _addCreators(Map<String, POJOPropertyBuilder> props)
         }
     }
 
+    // Method to determine if given non-explictly-annotated constructor
+    // looks like delegating one
+    private boolean _isDelegatingConstructor(PotentialCreator ctor)
+    {
+        // Only consider single-arg case, for now
+        if (ctor.paramCount() == 1) {
+            // Main thing: @JsonValue makes it delegating:
+            if ((_jsonValueAccessors != null) && !_jsonValueAccessors.isEmpty()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private List<PotentialCreator> _collectCreators(List<? extends AnnotatedWithParams> ctors)
     {
         if (ctors.isEmpty()) {
@@ -675,6 +712,35 @@ private void _removeDisabledCreators(List<PotentialCreator> ctors)
         }
     }
 
+    private void _removeNonFactoryStaticMethods(List<PotentialCreator> ctors)
+    {
+        final Class<?> rawType = _type.getRawClass();
+        Iterator<PotentialCreator> it = ctors.iterator();
+        while (it.hasNext()) {
+            // explicit mode? Retain (for now)
+            PotentialCreator ctor = it.next();
+            if (ctor.creatorMode() != null) {
+                continue;
+            }
+            // Copied from `BasicBeanDescription.isFactoryMethod()`
+            AnnotatedWithParams factory = ctor.creator();
+            if (rawType.isAssignableFrom(factory.getRawType())
+                    && ctor.paramCount() == 1) {
+                String name = factory.getName();
+
+                if ("valueOf".equals(name)) {
+                    continue;
+                } else if ("fromString".equals(name)) {
+                    Class<?> cls = factory.getRawParameterType(0);
+                    if (cls == String.class || CharSequence.class.isAssignableFrom(cls)) {
+                        continue;
+                    }
+                }
+            }
+            it.remove();
+        }
+    }
+
     private void _addExplicitlyAnnotatedCreators(PotentialCreators collector,
             List<PotentialCreator> ctors,
             Map<String, POJOPropertyBuilder> props,
@@ -690,48 +756,25 @@ private void _addExplicitlyAnnotatedCreators(PotentialCreators collector,
             if (ctor.creatorMode() == null) {
                 continue;
             }
+
             it.remove();
 
-            Boolean propsBased = null;
-            
+            boolean isPropsBased;
+
             switch (ctor.creatorMode()) {
             case DELEGATING:
-                propsBased = false;
+                isPropsBased = false;
                 break;
             case PROPERTIES:
-                propsBased = true;
+                isPropsBased = true;
                 break;
             case DEFAULT:
             default:
-                // First things first: if not single-arg Creator, must be Properties-based
-                // !!! Or does it? What if there's @JacksonInject etc?
-                if (ctor.paramCount() != 1) {
-                    propsBased = true;
-                }
-            }
-
-            // Must be 1-arg case:
-            if (propsBased == null) {
-                // Is ambiguity/heuristics allowed?
-                if (ctorDetector.requireCtorAnnotation()) {
-                    throw new IllegalArgumentException(String.format(
-                            "Ambiguous 1-argument Creator; `ConstructorDetector` requires specifying `mode`: %s",
-                            ctor));
-                }
-
-                // First things first: if explicit names found, is Properties-based
-                ctor.introspectParamNames(_config);
-                propsBased = ctor.hasExplicitNames()
-                        || ctorDetector.singleArgCreatorDefaultsToProperties();
-                // One more possibility: implicit name that maps to implied
-                // property based on Field/Getter/Setter
-                if (!propsBased) {
-                    String implName = ctor.implicitNameSimple(0);
-                    propsBased = (implName != null) && props.containsKey(implName);
-                }
+                isPropsBased = _isExplicitlyAnnotatedCreatorPropsBased(ctor,
+                        props, ctorDetector);
             }
 
-            if (propsBased) {
+            if (isPropsBased) {
                 // Skipping done if we already got higher-precedence Creator
                 if (!skipPropsBased) {
                     collector.setPropertiesBased(_config, ctor, "explicit");
@@ -742,6 +785,53 @@ private void _addExplicitlyAnnotatedCreators(PotentialCreators collector,
         }
     }
 
+    private boolean _isExplicitlyAnnotatedCreatorPropsBased(PotentialCreator ctor,
+            Map<String, POJOPropertyBuilder> props, ConstructorDetector ctorDetector)
+    {
+        if (ctor.paramCount() == 1) {
+            // Is ambiguity/heuristics allowed?
+            switch (ctorDetector.singleArgMode()) {
+            case DELEGATING:
+                return false;
+            case PROPERTIES:
+                return true;
+            case REQUIRE_MODE:
+                throw new IllegalArgumentException(String.format(
+"Single-argument constructor (%s) is annotated but no 'mode' defined; `ConstructorDetector`"
++ "configured with `SingleArgConstructor.REQUIRE_MODE`",
+ctor.creator()));
+            case HEURISTIC:
+            default:
+            }
+        }
+
+        // First: if explicit names found, is Properties-based
+        ctor.introspectParamNames(_config);
+        if (ctor.hasExplicitNames()) {
+            return true;
+        }
+        // Second: [databind#3180] @JsonValue indicates delegating
+        if ((_jsonValueAccessors != null) && !_jsonValueAccessors.isEmpty()) {
+            return false;
+        }
+        if (ctor.paramCount() == 1) {
+            // One more possibility: implicit name that maps to implied
+            // property based on Field/Getter/Setter
+            String implName = ctor.implicitNameSimple(0);
+            if ((implName != null) && props.containsKey(implName)) {
+                return true;
+            }
+            // Second: injectable also suffices
+            if ((_annotationIntrospector != null)
+                    && _annotationIntrospector.findInjectableValue(_config, ctor.param(0)) != null) {
+                return true;
+            }
+            return false;
+        }
+        // Trickiest case: rely on existence of implicit names and/or injectables
+        return ctor.hasNameOrInjectForAllParams(_config);
+    }
+
     private void _addCreatorsWithAnnotatedNames(PotentialCreators collector,
             List<PotentialCreator> ctors)
     {
@@ -760,6 +850,50 @@ private void _addCreatorsWithAnnotatedNames(PotentialCreators collector,
         }
     }
 
+    private boolean _addImplicitConstructor(PotentialCreators collector,
+            List<PotentialCreator> ctors, Map<String, POJOPropertyBuilder> props)
+    {
+        // Must have one and only one candidate
+        if (ctors.size() != 1) {
+            return false;
+        }
+        final PotentialCreator ctor = ctors.get(0);
+        // which needs to be visible
+        if (!_visibilityChecker.isCreatorVisible(ctor.creator())) {
+            return false;
+        }
+        ctor.introspectParamNames(_config);
+
+        // As usual, 1-param case is distinct
+        if (ctor.paramCount() != 1) {
+            if (!ctor.hasNameOrInjectForAllParams(_config)) {
+                return false;
+            }
+        } else {
+            // First things first: if only param has Injectable, must be Props-based
+            if ((_annotationIntrospector != null)
+                    && _annotationIntrospector.findInjectableValue(_config, ctor.param(0)) != null) {
+                // props-based, continue
+            } else {
+                // may have explicit preference
+                final ConstructorDetector ctorDetector = _config.getConstructorDetector();
+                if (ctorDetector.singleArgCreatorDefaultsToDelegating()) {
+                    return false;
+                }
+                // if not, prefer Properties-based if explicit preference OR
+                // property with same name
+                if (!ctorDetector.singleArgCreatorDefaultsToProperties()
+                        && !props.containsKey(ctor.implicitNameSimple(0))) {
+                    return false;
+                }
+            }
+        }
+
+        ctors.remove(0);
+        collector.setPropertiesBased(_config, ctor, "implicit");
+        return true;
+    }
+
     private void _addCreatorParams(Map<String, POJOPropertyBuilder> props,
             PotentialCreator ctor, List<POJOPropertyBuilder> creatorProps)
     {

src/test-jdk17/java/tools/jackson/databind/records/RecordExplicitCreatorsTest.java

@@ -199,18 +199,15 @@ public void testDeserializeUsingCanonicalConstructor_WhenJsonCreatorConstructorE
         }
     }
 
+    // 23-May-2024, tatu: Logic changed as part of [databind#4515]: explicit properties-based
+    //   Creator does NOT block implicit delegating Creators. So formerly (pre-2.18) failing
+    //   case is now expected to pass.
     @Test
     public void testDeserializeUsingImplicitFactoryMethod_WhenJsonCreatorConstructorExists_WillFail() throws Exception {
-        try {
-            MAPPER.readValue("123", RecordWithJsonPropertyWithJsonCreator.class);
-
-            fail("should not pass");
-        } catch (MismatchedInputException e) {
-            verifyException(e, "Cannot construct instance");
-            verifyException(e, "RecordWithJsonPropertyWithJsonCreator");
-            verifyException(e, "although at least one Creator exists");
-            verifyException(e, "no int/Int-argument constructor/factory method");
-        }
+        RecordWithJsonPropertyWithJsonCreator value = MAPPER.readValue("123",
+                RecordWithJsonPropertyWithJsonCreator.class);
+        assertEquals(123, value.id());
+        assertEquals("JsonCreatorConstructor", value.name());
     }
 
     /*

src/test/java/tools/jackson/databind/deser/creators/ConstructorDetector1498Test.java

@@ -9,6 +9,7 @@
 import tools.jackson.databind.*;
 import tools.jackson.databind.cfg.*;
 import tools.jackson.databind.exc.InvalidDefinitionException;
+import tools.jackson.databind.exc.UnrecognizedPropertyException;
 import tools.jackson.databind.introspect.AnnotatedMember;
 import tools.jackson.databind.introspect.JacksonAnnotationIntrospector;
 import tools.jackson.databind.json.JsonMapper;
@@ -147,13 +148,20 @@ public void testMultiArgAsProperties() throws Exception
     @Test
     public void test1ArgDefaultsToPropsMultipleCtors() throws Exception
     {
+        // 23-May-2024, tatu: Will fail differently with [databind#4515]; default
+        //   constructor available, implicit ones ignored
         try {
             MAPPER_PROPS.readValue(a2q("{'value' : 137 }"),
                 SingleArg2CtorsNotAnnotated.class);
             fail("Should not pass");
+        } catch (UnrecognizedPropertyException e) {
+            verifyException(e, "\"value\"");
+        }
+            /*
         } catch (InvalidDefinitionException e) {
             verifyException(e, "Conflicting property-based creators");
         }
+        */
     }
 
     /*

src/test/java/tools/jackson/databind/deser/creators/SingleArgCreatorTest.java

@@ -183,7 +183,12 @@ public void testSingleStringArgWithImplicitName() throws Exception
         final ObjectMapper mapper = jsonMapperBuilder()
                 .annotationIntrospector(new MyParamIntrospector("value"))
                 .build();
-        StringyBean bean = mapper.readValue(q("foobar"), StringyBean.class);
+        // 23-May-2024, tatu: [databind#4515] Clarifies handling to make
+        //   1-param Constructor with implicit name auto-discoverable
+        //   This is compatibility change so hopefully won't bite us but...
+        //   it seems like the right thing to do.
+//        StringyBean bean = mapper.readValue(q("foobar"), StringyBean.class);
+        StringyBean bean = mapper.readValue(a2q("{'value':'foobar'}"), StringyBean.class);
         assertEquals("foobar", bean.getValue());
     }
 

src/test/java/tools/jackson/databind/mixins/MixinForCreators2795Test.java

@@ -7,7 +7,6 @@
 import com.fasterxml.jackson.annotation.*;
 
 import tools.jackson.databind.*;
-import tools.jackson.databind.json.JsonMapper;
 import tools.jackson.databind.testutil.DatabindTestUtil;
 import tools.jackson.databind.testutil.NoCheckSubTypeValidator;
 
@@ -25,7 +24,7 @@ public UnmodifiableCollectionMixin(final Collection<?> collection) { }
     @Test
     public void testMixinWithUnmmodifiableCollection() throws Exception
     {
-        ObjectMapper mapper = JsonMapper.builder()
+        ObjectMapper mapper = jsonMapperBuilder()
                 .addMixIn(Collections.unmodifiableCollection(Collections.emptyList()).getClass(),
                         UnmodifiableCollectionMixin.class)
                 .activateDefaultTypingAsProperty(new NoCheckSubTypeValidator(),
@@ -36,8 +35,8 @@ public void testMixinWithUnmmodifiableCollection() throws Exception
         final Collection<String> unmodifiableCollection = Collections.unmodifiableCollection(strings);
         final byte[] bytes = mapper.writeValueAsBytes(unmodifiableCollection);
 
-        final Collection<?> collection = mapper.readValue(bytes, Collection.class);
+        final Collection<?> result = mapper.readValue(bytes, Collection.class);
 
-        assertEquals(2, collection.size());
+        assertEquals(2, result.size());
     }
 }

src/test/java/tools/jackson/failing/ObjectIdWithInjectables538Test.java

@@ -34,7 +34,7 @@ public B(@JacksonInject("i2") String injected) {
     /*****************************************************
      */
 
-    private final ObjectMapper MAPPER = new ObjectMapper();
+    private final ObjectMapper MAPPER = newJsonMapper();
 
     // 26-Sep-2017, tatu: With Jackson 3.x and inclusion of parameter-names for creators
     //   we face a new failure since explicit name of injectables is sort of ignored.