Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nulls.AS_EMPTY returns null in java.lang.Object #3489

Open
aivinog1 opened this issue May 18, 2022 · 3 comments · May be fixed by #3490
Open

Nulls.AS_EMPTY returns null in java.lang.Object #3489

aivinog1 opened this issue May 18, 2022 · 3 comments · May be fixed by #3490
Labels
to-evaluate Issue that has been received but not yet evaluated

Comments

@aivinog1
Copy link

Describe the bug
When I'm using JsonSetter.Value.forValueNulls(Nulls.AS_EMPTY, Nulls.AS_EMPTY) for nullable value with type java.lang.Object in a Map, I still getting the null in the deserialized object. I think that problem is that com.fasterxml.jackson.databind.deser.std.UntypedObjectDeserializer.Vanilla doesn't override com.fasterxml.jackson.databind.JsonDeserializer#getEmptyValue(com.fasterxml.jackson.databind.DeserializationContext). I think that the deserializer for object should return empty object in the getEmptyValue method.

Version information
2.13.2.2

To Reproduce
If you have a way to reproduce this with:

  1. Create a JSON, like this:
{
  "a": {
    "first": "second",
    "third": null
  }
}
  1. Create this class to deserialize with:
import java.util.Map;

public class Scratch {
    private Map<String, Object> a;

    public Map<String, Object> getA() {
        return a;
    }

    public void setA(Map<String, Object> a) {
        this.a = a;
    }
}
  1. Create the ObjectMapper and override nullable behavior:
        final ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.configOverride(Map.class).setSetterInfo(JsonSetter.Value.forValueNulls(Nulls.AS_EMPTY, Nulls.AS_EMPTY));
  1. Deserialize this JSON:
        String json = "...";
        final Scratch scratch = objectMapper.readValue(json, Scratch.class);
  1. Verify that in the a object we got a null in the third key:
        final Object third = scratch.getA().get("third");
        assert third != null; // fails

Expected behavior
I'm expecting that the empty value of the java.lang.Object is a new Object instance.
@aivinog1 aivinog1 added the to-evaluate Issue that has been received but not yet evaluated label May 18, 2022
aivinog1 added a commit to aivinog1/jackson-databind that referenced this issue May 18, 2022
@aivinog1
fix FasterXML#3489
d6d421c 
I've added the empty value to the Object deserializer in maps as `new Object();`.
@aivinog1 aivinog1 linked a pull request May 18, 2022 that will close this issue
Open
@cowtowncoder
Copy link
Member

I am not sure how this can really work unfortunately as there is no obvious "empty" representation of "untyped" value (nominal type of java.lang.Object). I guess theoretically it could be empty Map or empty List. Or maybe scripting language style would be to return empty String.
Returning plain new Object() would seem very surprising to me at least.

But... maybe it should be configurable? I could see that working. It would have to be a per-mapping configuration feature, and there'd be lots of wiring.
Alternative would be sub-classing of UntypedObjectDeserializer (or Vanilla) but that is very fragile so I would advise against that.

But configurable choice of "empty" value for java.lang.Object is something that I could see working.

@aivinog1
Copy link
Author

aivinog1 commented Jun 1, 2022

Thanks, @cowtowncoder for your insights! What do you think about implementing it like this:

  1. We create an interface EmptyValueProvider
  2. Right now I'm thinking about having exactly one method with the signature: Object getEmptyValue(DeserializationContext ctxt, JsonDeserializer<?> deser) throws JsonMappingException
  3. We can start with creating the default implementation - return the value from com.fasterxml.jackson.databind.JsonDeserializer#getNullValue(com.fasterxml.jackson.databind.DeserializationContext)
  4. We add the new method in the com.fasterxml.jackson.databind.annotation.JsonDeserialize with the name emptyValueProvider() and the return type Class<? extends EmptyValueProvider> and default value as the implementation that created in the 3. So a user can just add @JsonDeserialize(emptyValueProvider = MyCoolEmptyValueProvider.class.
  5. I think that the com.fasterxml.jackson.databind.JsonDeserializer should have a private field with the instance of EmptyValueProvider
  6. Wire up all the logic behind this 😅

Why I didn't use the same approach that exists for NullValueProvider you may ask? I think that for this case I should give a user an easy way to implement their implementation like you said: we can think differently about empty values :)

At this point I think that I need to negotiate the API first, then let's think about implementation :)

@cowtowncoder
Copy link
Member

Hmmh. Not sure about call flow, use of EmptyValueProvider... The core concept with databind is that serializers and deserializers have the main responsibility for handling, over centralized entities.
So I think call really should go through getEmptyValue(...), always. And from that, it would be necessary to make deserializer use something else, possibly provider.

But then again, adding all the logic to pipe in Yet Another Provider might be daunting.
I wonder if there was any chance of extending NullValueProvider to also support "empty" (and perhaps even "absent") value configuration. Possibly not, but that what springs to mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Return new Object() as default value for "empty" for java.lang.Object aivinog1/jackson-databind
2 participants
@cowtowncoder @aivinog1