More work on #1498

Author: cowtowncoder

src/main/java/com/fasterxml/jackson/databind/cfg/ConstructorDetector.java

@@ -1,11 +1,5 @@
 package com.fasterxml.jackson.databind.cfg;
 
-import java.io.IOException;
-import java.util.List;
-
-import com.fasterxml.jackson.databind.DatabindContext;
-import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor;
-
 /**
  * Configurable handler used to select aspects of selecting
  * constructor to use as "Creator" for POJOs.
@@ -62,18 +56,6 @@ public enum SingleArgConstructor {
         REQUIRE_MODE;
     }
 
-//    @FunctionalInterface
-    /**
-     * Simple interface for optionally defined handler that can select
-     * specific Constructor to use if more than one implicitly detected ones
-     * (and no explicitly annotated one) found.
-     */
-    public interface ConstructorSelector {
-        public AnnotatedConstructor select(DatabindContext ctxt,
-                List<AnnotatedConstructor> ctors)
-            throws IOException;
-    }
-
     /*
     /**********************************************************************
     /* Global default instances to use
@@ -85,9 +67,6 @@ public AnnotatedConstructor select(DatabindContext ctxt,
      *<ul>
      * <li>Uses {@link SingleArgConstructor#HEURISTIC} for single-argument constructor case
      *  </li>
-     * <li>Does not specify {@link ConstructorSelector} to solve ambiguous
-     *  (multiple implicitly discovered argument-taking Constructors}
-     *  </li>
      * <li>Does not require explicit {@code @JsonCreator} annotations (so allows
      * auto-detection of Visible constructors} (except for JDK types)
      *  </li>
@@ -128,8 +107,6 @@ public AnnotatedConstructor select(DatabindContext ctxt,
 
     protected final SingleArgConstructor _singleArgMode;
 
-    protected final ConstructorSelector _selector;
-
     /**
      * Whether explicit {@link com.fasterxml.jackson.annotation.JsonCreator}
      * is always required for detecting constructors (even if visible) other
@@ -139,7 +116,7 @@ public AnnotatedConstructor select(DatabindContext ctxt,
 
     /**
      * Whether auto-detection of constructors of "JDK types" (those in
-     * packages {@code java.} and {@code javax.}) is allowed or not (
+     * packages {@code java.} and {@code javax.}) is allowed or not
      */
     protected final boolean _allowJDKTypeCtors;
 
@@ -150,12 +127,10 @@ public AnnotatedConstructor select(DatabindContext ctxt,
      */
 
     protected ConstructorDetector(SingleArgConstructor singleArgMode,
-            ConstructorSelector selector,
             boolean requireCtorAnnotation,
             boolean allowJDKTypeCtors)
     {
         _singleArgMode = singleArgMode;
-        _selector = selector;
         _requireCtorAnnotation = requireCtorAnnotation;
         _allowJDKTypeCtors = allowJDKTypeCtors;
     }
@@ -165,26 +140,21 @@ protected ConstructorDetector(SingleArgConstructor singleArgMode,
      * by {@code _singleArgMode}
      */
     protected ConstructorDetector(SingleArgConstructor singleArgMode) {
-        this(singleArgMode, null, false, false);
+        this(singleArgMode, false, false);
     }
 
     protected ConstructorDetector withSingleArgMode(SingleArgConstructor singleArgMode) {
-        return new ConstructorDetector(singleArgMode, _selector,
-                _requireCtorAnnotation, _allowJDKTypeCtors);
-    }
-
-    protected ConstructorDetector withSelector(ConstructorSelector selector) {
-        return new ConstructorDetector(_singleArgMode, selector,
+        return new ConstructorDetector(singleArgMode,
                 _requireCtorAnnotation, _allowJDKTypeCtors);
     }
 
     protected ConstructorDetector withRequireAnnotation(boolean state) {
-        return new ConstructorDetector(_singleArgMode, _selector,
+        return new ConstructorDetector(_singleArgMode,
                 state, _allowJDKTypeCtors);
     }
 
     protected ConstructorDetector withAllowJDKTypes(boolean state) {
-        return new ConstructorDetector(_singleArgMode, _selector,
+        return new ConstructorDetector(_singleArgMode,
                 _requireCtorAnnotation, state);
     }
 
@@ -198,10 +168,6 @@ public SingleArgConstructor singleArgMode() {
         return _singleArgMode;
     }
 
-    public ConstructorSelector constructorSelector() {
-        return _selector;
-    }
-
     public boolean requireCtorAnnotation() {
         return _requireCtorAnnotation;
     }

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

@@ -8,8 +8,6 @@
 import java.util.concurrent.atomic.AtomicReference;
 
 import com.fasterxml.jackson.annotation.*;
-import com.fasterxml.jackson.annotation.JacksonInject.Value;
-import com.fasterxml.jackson.annotation.JsonCreator.Mode;
 
 import com.fasterxml.jackson.core.JsonParser;
 
@@ -441,7 +439,7 @@ protected void _addExplicitConstructorCreators(DeserializationContext ctxt,
         // 21-Sep-2017, tatu: First let's handle explicitly annotated ones
         for (AnnotatedConstructor ctor : beanDesc.getConstructors()) {
             JsonCreator.Mode creatorMode = intr.findCreatorAnnotation(ctxt.getConfig(), ctor);
-            if (Mode.DISABLED == creatorMode) {
+            if (JsonCreator.Mode.DISABLED == creatorMode) {
                 continue;
             }
             if (creatorMode == null) {
@@ -645,7 +643,7 @@ protected void _addExplicitFactoryCreators(DeserializationContext ctxt,
                 }
                 continue;
             }
-            if (creatorMode == Mode.DISABLED) {
+            if (creatorMode == JsonCreator.Mode.DISABLED) {
                 continue;
             }
 

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

@@ -165,7 +165,10 @@ public Object getValue(Object pojo)
 
     @Override
     public String toString() {
-        return "[constructor for "+getName()+", annotations: "+_annotations+"]";
+        final int argCount = _constructor.getParameterCount();
+        return String.format("[constructor for %s (%d arg%s), annotations: %s",
+                ClassUtil.nameOf(_constructor.getDeclaringClass()), argCount,
+                (argCount == 1) ? "" : "s", _annotations);
     }
 
     @Override

src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java

@@ -36,24 +36,38 @@ public String findImplicitPropertyName(//MapperConfig<?> config,
     static class SingleArgNotAnnotated {
         protected int v;
 
-//        SingleArgNotAnnotated() { throw new Error("Should not be used"); }
+        SingleArgNotAnnotated() { v = -1; }
 
-        public SingleArgNotAnnotated(@ImplicitName("value") @com.fasterxml.jackson.annotation.JsonSetter int value) {
+        public SingleArgNotAnnotated(@ImplicitName("value") int value) {
             v = value;
         }
     }
 
     static class SingleArgNoMode {
         protected int v;
 
-//        SingleArgNoMode() { throw new Error("Should not be used"); }
+        SingleArgNoMode() { v = -1; }
 
         @JsonCreator
         public SingleArgNoMode(@ImplicitName("value") int value) {
             v = value;
         }
     }
 
+    static class SingleArg2CtorsNotAnnotated {
+        protected int v;
+
+        SingleArg2CtorsNotAnnotated() { v = -1; }
+
+        public SingleArg2CtorsNotAnnotated(@ImplicitName("value") int value) {
+            v = value;
+        }
+
+        public SingleArg2CtorsNotAnnotated(@ImplicitName("value") long value) {
+            v = (int) (value * 2);
+        }
+    }
+    
     static class SingleArg1498 {
         final int _bar;
 
@@ -97,6 +111,19 @@ public void test1ArgDefaultsToPropertiesIssue1498() throws Exception
         assertEquals(404, value._bar);
     }
 
+    // 18-Sep-2020, tatu: For now there is a problematic case of multiple eligible
+    //   choices; not cleanly solvable for 2.12
+    public void test1ArgDefaultsToPropsMultipleCtors() throws Exception
+    {
+        try {
+            MAPPER_PROPS.readValue(a2q("{'value' : 137 }"),
+                SingleArg2CtorsNotAnnotated.class);
+            fail("Should not pass");
+        } catch (InvalidDefinitionException e) {
+            verifyException(e, "Conflicting property-based creators");
+        }
+    }
+
     /*
     /**********************************************************************
     /* Test methods, selecting from 1-arg constructors, delegating
@@ -119,7 +146,7 @@ public void test1ArgDefaultsToDelegatingNoMode() throws Exception
 
     /*
     /**********************************************************************
-    /* Test methods, selecting from 1-arg constructors, other
+    /* Test methods, selecting from 1-arg constructors, heuristic (pre-2.12)
     /**********************************************************************
      */
 
@@ -137,9 +164,15 @@ public void test1ArgDefaultsToHeuristics() throws Exception
         assertEquals(13117, v2.v);
     }
 
-    // 15-Sep-2020, tatu: Tricky semantics... should this require turning
-    //    off of auto-detection?
     /*
+    /**********************************************************************
+    /* Test methods, selecting from 1-arg constructors, explicit fail
+    /**********************************************************************
+     */
+
+    // 15-Sep-2020, tatu: Tricky semantics... should this require turning
+    //    off of auto-detection? If there is 0-arg ctor, that is to be used
+/*
     public void test1ArgFailsNoAnnotation() throws Exception
     {
         // First: fail if nothing annotated (for 1-arg case)