Possible wrong use of _arrayDelegateDeserializer in BeanDeserializerBase::deserializeFromObjectUsingNonDefault

[Gems]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

Deserialisation doesn't work as expected in case of several Json Creators are defined on a class ( PROPERTIES and DELEGATED), and the delegating one is accepting a List as an argument and represents the single mandatory property of the class.

It's expected that the PROPERTIES deserializer will be used, but the deserialisation process opts for the delegating deserialiser despite JSON definition being provided as Object.

Version Information

2.16.1

Reproduction

I have the following class definition for a JSON object:

class MyDefinition {
  List<StepDefinition> steps;
  ExtraProperties extra;

  @JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
  public MyDefinition(@JsonProperty("steps") List<StepDefinition> steps, @JsonProperty("extra") ExtraProperties extra) {
     this.steps = steps;
     this.extra = extra;
  }

  @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
  public static MyDefinition of(List<StepDefinition> steps) {
    return new MyDefinition(steps, null);
  }
}

Classes StepDefinition and ExtraProperties are simple class with straight-forward mapping.

I annotated the MyDefinition class in such a way, so it could handle mapping for the following two cases of JSON:

Case 1:

{
  "myDefinition": {
    "steps": [ { "step": "some" } ], 
    "extra": { "prop": "value" }
  }
}

Case 2:

{ 
  "myDefinition": [
    { "step": "some" }
  ]
}

The deserialisation for the "PROPERTIES" case (Case 1) fails on trying to deserialise "extra" property value as StepDefinition type.

Expected behavior

I'd expect that the PROPERTIES deserialiser is used in case the object is provided with a bunch of properties.

Additional context

A similar mapping scenario works in case the delegating factory method accepts String as a value (not a list).

My mapper is created using YAMLFactory and the failure occurs on parsing YAML.
The mapper has DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY feature enabled, and is configured with ParameterNamesModule

I guess it could be important to mention that I also use Lombok.

[Gems]

I checked the BeanDeserializerBase::deserializeFromObjectUsingNonDefault method and noticed that if _delegateDeserializer() method which returns available _arrayDelegateDeserializer (II guess because of the delegating Json Creator) would return null and the deserialisation process would fallback to use _propertyBasedCreator, then everything would work as expected.

The JsonParser current token is FIELD_NAME, hence nothing suggests that current parsing context is an array and that it's a good idea to use _arrayDelegateDeserializer.

HTH

[cowtowncoder]

Quick note: would be good to verify with latest release (2.17.1). But more importantly, 2.18.0-SNAPSHOT has fully rewritten Bean Property Introspection, which includes Creator detection.
So would be good to see how this works with latest code from 2.18 branch.

[cowtowncoder]

Other than that, what would be useful is an actual unit test: all the pieces seem to be included. But reproduction would be most useful.

Reproduction would ideally be for using JSON (not yaml), without parameter-names module (just use annotations). If it turns out things work in JSON, then would move issue to yaml module repo etc.

[Gems]

@cowtowncoder thanks for your input

I checked it with 2.17.1 and 2.18.0-SNAPSHOT, no dice. The BeanDeserializerBase::deserializeFromObjectUsingNonDefault method implementation is the same for the mentioned versions, hence no change in the behaviour.

[Gems]

I added the test, you can find it in the linked PR #4605

[cowtowncoder]

Thank you @Gems!

[cowtowncoder]

Ok, so, just because current JSON value is Object does not automatically rule out use of delegating deserializer -- Delegation to, say, Map, would expect JSON Object. So logic is not (and cannot) use JSON value shape for that distinction.

But I wonder if _arrayDelegateDeserializer should indeed check for JSON Array (there are some features it might not work well with, like "accept single value as Array of 1" but we'll ignore that for now) before being used.

[cowtowncoder]

Turns out, "single value as Array of 1 element" case was indeed caught by one existing unit tests.

But I was able to hack a PR to fix this behavior, #4609, which while not as clean as I'd hope, would do the trick.
So assuming CI passes, will merge in 2.18 for 2.18.0 (this is slightly risky change so not planning to backport as a patch into 2.17 or earlier).

[cowtowncoder]

Thank you @Gems for reporting the issue and contributing the test! As per my note above, fix will go in 2.18.0 which will take a bit to release (development cycle not yet complete).

[Gems]

Thank you @cowtowncoder for the prompt response and for fixing it. I look forward to updating my dependency to 2.18.0.