Use key type factory methods ahead of ctors

[tbartley]

Fixes #2158

Simply swaps the order factory method and String
constructors are searched for in the key type.
Factory methods are now preferred.

[cowtowncoder]

What is the basis for change? Is this the order used for values too?

[tbartley]

That I had this PR going completely slipped my mind. Rebased it now that the unrelated issue I was hitting is merged and it now builds and tests cleanly.

[tbartley]

In answer to your question "Is this the order for values too?" - yes, where a single field constructor is present and not itself annotated with @JsonCreator.

If single argument constructor of the same type is present and annotated with @JsonCreator and there's a single argument @JsonCreator annotated factory method then, AFAICT, an exception is thrown (CreatorCollector.verifyNonDup:311)

                        throw new IllegalArgumentException(String.format(
                                "Conflicting %s creators: already had %s creator %s, encountered another: %s",
                                TYPE_DESCS[typeIndex],
                                explicit ? "explicitly marked"
                                        : "implicitly discovered",
                                oldOne, newOne));

[cowtowncoder]

Ok thanks -- I will need to re-read this, and will put it on my todo list.

[tbartley]

Thanks Tatu! [cid:67BA6677-8CED-46CB-910D-CFCFBF6C063E@gateway] On 19 Jul 2019, at 9:47 am, Tatu Saloranta <[email protected]<mailto:[email protected]>> wrote: Ok thanks -- I will need to re-read this, and will put it on my todo list. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#2180?email_source=notifications&email_token=AAZQNHGWGSAZDVCO46PQ5PTQAD6IHA5CNFSM4GCI46O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2KEXSI#issuecomment-513035209>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAZQNHAKIR3UAAWNDJHNV53QAD6IHANCNFSM4GCI46OQ>.

[cowtowncoder]

Hmm. Ok. This gets tricky. So, BasicDeserializerFactory comment actually says:

        // Important: first add factory methods; then constructors, so
        // latter can override former!
        _addDeserializerFactoryMethods(ctxt, beanDesc, vchecker, intr, creators, creatorDefs);
        if (beanDesc.getType().isConcrete()) {
            _addDeserializerConstructors(ctxt, beanDesc, vchecker, intr, creators, creatorDefs);
        }

which would suggest constructors are (or were intended) to have precedence over factory methods. And as far as I can tell, conflicts only matter with explicit (annotated conflicts) -- in case of implicitly found creators, later ones override earlier ones.

Given this, I don't think I want to change the lookup order: existing order has been default order since now so there need to be some really good reason to change it.

However. One thing I could add would be to check for explicit annotation (@JsonCreator), to determine if one has been explicitly specified. Problem with this would be that for some types creator to use for Keys might be different from that for values.
Alternatively it would be possible to create new method in AnnotationIntrospector to either:

1. Handle lookup for markers for possible key deserializer candidates OR
2. Resolve case where 2 alternate Creators are found (and in case of more than 2, call it pair-wise).