String-based Map key deserializer is not deterministic when there is no single arg constructor

[hisener]

Describe the bug

StdKeyDeserializers#findStringBasedKeyDeserializer uses a static factory method when there is no single arg constructor. It gets the factory method using BasicBeanDescription#findFactoryMethod that returns the first single-arg static method. However, the behavior is not deterministic as java.lang.Class#getDeclaredMethods Javadoc states:

The elements in the returned array are not sorted and are not in any particular order.

The factory methods are collected using AnnotatedCreatorCollector#_findPotentialFactories -> ClassUtil#getClassMethods -> java.lang.Class#getDeclaredMethods.

So in the case of two static factory methods, one with @JsonCreator annotation and one named #valueOf or #fromString, Jackson may use either of them depending on the array order.

The second method needs to be named #valueOf or #fromString because of the following BasicBeanDescription#isFactoryMethod logic:

jackson-databind/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java

Lines 631 to 645 in e85f7f0

	// 24-Oct-2016, tatu: As per [databind#1429] must ensure takes exactly one arg
	if ("valueOf".equals(name)) {
	if (am.getParameterCount() == 1) {
	return true;
	}
	}
	// [databind#208] Also accept "fromString()", if takes String or CharSequence
	if ("fromString".equals(name)) {
	if (am.getParameterCount() == 1) {
	Class<?> cls = am.getRawParameterType(0);
	if (cls == String.class || CharSequence.class.isAssignableFrom(cls)) {
	return true;
	}
	}
	}

Version information

2.12.2

To Reproduce

static class KeyTypeMultipleFactoryMethods {
    protected String value;

    private KeyTypeMultipleFactoryMethods(String v, boolean bogus) {
        value = v;
    }

    @JsonCreator
    public static KeyTypeMultipleFactoryMethods create(String v) {
        return new KeyTypeMultipleFactoryMethods(v, true);
    }

    public static KeyTypeMultipleFactoryMethods valueOf(String id) {
        return new KeyTypeMultipleFactoryMethods(id.toUpperCase(Locale.ROOT), false);
    }
}

public void testKeyWithCreatorAndMultipleFactoryMethods() throws Exception
{
    Map<KeyTypeMultipleFactoryMethods,Integer> map = MAPPER.readValue("{\"foo\":3}",
            new TypeReference<Map<KeyTypeMultipleFactoryMethods,Integer>>() {} );
    assertEquals(1, map.size());
    assertEquals("foo", map.keySet().iterator().next().value);
}

[ERROR] Failures: 
[ERROR]   MapDeserializationTest.testKeyWithCreatorAndMultipleFactoryMethods:486 expected:<[foo]> but was:<[FOO]>
[INFO] 
[ERROR] Tests run: 26, Failures: 1, Errors: 0, Skipped: 0

It's also available in this branch: 2.12...PicnicSupermarket:hsener/find-factory-method.

Expected behavior

@JsonCreator method has precedence over other static factory methods in String-based deserialization.

Additional context

We noticed this behavior in a custom key deserializer, which uses BasicBeanDescription#findFactoryMethod. The behavior was nondeterministic for an enum where we have Enum#valueOf and a static factory method with @JsonCreator annotation.

[cowtowncoder]

That does sound sub-optimal and should either throw an exception for ambiguous cases, or at least use stable tie-breaker -- possible latter for 2.12 (to prevent perceived regression for some usage), and exception for 2.13.

[hisener]

Would that mean #findFactoryMethod in 2.13 will always throw an exception for enums with a @JsonCreator annotation? 🤔

[cowtowncoder]

@hisener I'll have look into details but I think that rules should be similar to value deserializers:

1. Annotated constructor/factory method overrides any implicit ones
2. Only if there are multiple @JsonCreator annotated alternatives without precedence (I think there is a tie-breaker for factory vs constructor but I can't remember which is expected to win) -- that is, if there 2 annotated constructors, or 2 factory methods

For enums there is also @JsonValue to consider: there should be no exception, but I'd have to think of precedence here (since this is specific to Enums; for POJOs @JsonValue only affects serialization).

[cowtowncoder]

Added the failing test from above and can reproduce the issue. Method that selects Creator method is StdKeyDeserializers.findStringBasedKeyDeserializer() and needs to be changed to consider annotation(s) for precedence.