Fix #4938 Allow @JsonCreator to return null

[JooHyukKim]

resolves #4938

[cowtowncoder]

Need to address case of properties other than those matched as Creator properties: ones collected before Creator called and ones that may come after it's called.

[cowtowncoder]

This approach does not work: Jackson parsers buffer content so all content being read from InputStream does not actually necessarily mean all has been consumed.
There is a method in JsonParser for pushing back "unread" content but...

Instead of this, you could just enable DeserializationFeature.FAIL_ON_TRAILING_TOKENS to verify there is no more content to be read (tokens). That should catch the issue.

[JooHyukKim]

@cowtowncoder If you see current build failure there are two tests failing trying to handle unknown properties in following ways...

• One failing because of FAIL_ON_UNKNOWN_PROPERTIES enabled.
• The other because creator returned null and trying to set any-setter property on null bean

Both reaches BeanDeserializer.handleUnknownVanilla(), could it be what we already want?

Or are you saying we need the execution to continue to go on further down, to like ...

... here?

[cowtowncoder]

@cowtowncoder If you see current build failure there are two tests failing trying to handle unknown properties in following ways...

* One failing because of `FAIL_ON_UNKNOWN_PROPERTIES` enabled.

* The other because creator returned `null` and trying to set any-setter property on `null` bean

Both reaches BeanDeserializer.handleUnknownVanilla(), could it be what we already want?

Or are you saying we need the execution to continue to go on further down, to like ...

... here?

I think that if null is returned, we should probably do forced skipping and ignoring all properties until END_OBJECT.

But that handling should be extracted to a helper method just to keep logic cleaner and possibly change. And pass already collected information.

[JooHyukKim]

@cowtowncoder Just some updates,

• I added void testDeserializeReadingUntilEndObject() test to verify we read all the way to END_OBJECT.
• Still haven't managed to think of the RIGHT place to insert new method to handle unknown props 🥲

[JooHyukKim]

Alright, I think I'm stuck at trying to find the proper place/timing for method extraction.
May it be because current implementation doesn't have much changes tbh.

No rush for this work to be done, but any opinion much appreciated! 😆

[cowtowncoder]

Alright, I think I'm stuck at trying to find the proper place/timing for method extraction. May it be because current implementation doesn't have much changes tbh.

No rush for this work to be done, but any opinion much appreciated! 😆

I'm happy to have a look -- might take a while as I'm going for 1 week vacation tomorrow. But will help when I get back (unless I have extra time today).

[cowtowncoder]

LGTM

[cowtowncoder]

@JooHyukKim Finally had time to get back to this one -- changed things slightly, the way I think it should work, and I think this is ready to be merged. Will leave open, merge tomorrow, unless you want to change things further.

[cowtowncoder]

Assuming this is fine, merging.

[JooHyukKim]

Sorry late reply, yes, seems okay to merge!