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.

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

Use @JsonSetter(nulls=...) to specify handling of null values during deserialization #1402

Closed
cowtowncoder opened this issue Oct 5, 2016 · 24 comments
Labels
most-wanted Tag to indicate that there is heavy user +1'ing action
Milestone

Comments

@cowtowncoder
Copy link
Member

(note: replaces #1269, earlier #988 and #830)

It looks like there is strong desire to define alternate methods for handling incoming null values; instead of just assigning as Java nulls to:

  1. Ignore value (avoid calling setter)
  2. Fail (throw exception)
  3. Convert to a default value

Since applicability is probably not universal for all cases (that is, different types could well follow different rules), it seems that approach similar to @JsonInclude would work.
However, I don't think use of @JsonInclude itself works here, as its settings are partially output-specific; so ideas are reusable, implementation not.

Instead, I think use of existing @JsonSetter by adding:

  • Nulls value enumeration for a small set of standard behaviors (and perhaps, in future, for custom handler?)
  • nulls property for dealing with value itself (POJO, Collection, wrapper type)
  • contentNulls for contents of structured types: array/collection elements, Map values, POJO properties

Changes to annotation are simple; changes to deserializers would be sizable.

@raindev
Copy link

raindev commented Dec 28, 2016

Is there any ongoing progress to be able to deserialize nulls to empty collections? We regularly suffer from NullPointerExecptions in our API clients. It would be great to solve the issue systematically.

@cowtowncoder
Copy link
Member Author

@raindev This would be option (3), although it may be that most users would prefer initializing empty collection values in constructor, and opt for simple skip (1) instead. Either way could solve the null problem; former would require bit more complex handling (deserializer implementation would need to know how to produce default value; and it may or may not be easily accessible).

@raindev
Copy link

raindev commented Jan 9, 2017

@cowtowncoder, but as I understand it is not implemented yet, right? Can I help, if it is not?

@cowtowncoder
Copy link
Member Author

@raindev It's very high on my todo list, but correct, it is not yet implemented.

Help is always welcome, although in this case I do have reasonable idea of how to proceed: basically it would use approach somewhat similar to #1399, modifying SettableBeanProperty to skip setting. There are some open questions on interaction for obtaining "empty" instance (if that is to be supported).

For what it is worth, annotation side has already been added, so information is already available during construction of BeanDeserializer / SettableBeanProperty.

So... if you want to have a look, feel free to: I rarely keep much uncommitted code locally, so everything implemented by me so far is in master. Rest are just ideas which I am happy to share.

@cowtowncoder cowtowncoder added this to the 2.9.0 milestone Feb 18, 2017
@cowtowncoder
Copy link
Member Author

Completed finally!

@raindev Feature is now complete to the degree I planned to implement it.

@raindev
Copy link

raindev commented Feb 20, 2017

@cowtowncoder, that's great news, many thanks!
I'm sorry to never get back to it, because of life 😞

@cowtowncoder
Copy link
Member Author

@raindev That's ok -- I know the feeling. I hope you get a chance to try out the feature at least.

@TheKeeperOfPie
Copy link

@cowtowncoder Does this handle DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT properly, so that "" with (1) won't call the setter?

I don't think that's the case on the latest snapshot, but this project I'm using it on is such a cobbled together mess that I can't be sure of anything.

Also, any plans to make this a global setting? I'd rather not annotate all of my fields with JsonSetter.

@cowtowncoder
Copy link
Member Author

@TheKeeperOfPie As to global setting, yes, that is already supported. There are three levels:

  1. Per-property annotation
  2. Per-type config override: ObjectMapper.configOverride(class).setSetterInfo(....)
  3. Global default: ObjectMapper.setDefaultSetterInfo(....)

and those are tested to some degree in unit tests.

As to empty String... unfortunately empty String is not null in this respect so it will not apply no. While it could be beneficial to do so, I am not sure if that is technically feasible because of complexities wrt coercion of empty String, and how null handling here is handled.
But I will think how things could work: this would be valuable thing to do.

Could you please file another issue for this, so I won't forget? You can add a ref to this issue as background.

@ryanprayogo
Copy link

Can anyone recommend any workaround until this issue is released in v2.9.0?

@cowtowncoder
Copy link
Member Author

@ryanprayogo This is a new feature so I am not quite sure what you mean by work around... but if I had to guess the traditional thing to do has been to add a setter method to do whatever conversion or skipping of null is needed.

@allienna
Copy link

Hi,

Does it works with xml deserialisation?

We would like to deserialise <tags /> with this kind of code:

@XmlElement(name = "tags")
@JsonSetter(nulls = Nulls.SKIP)
private Tags tags = new Tags();

And Tags class look like:

@Data
public class Tags {
  List<Tag> tags = new ArrayList<>();
}

but it's stay to null

@cowtowncoder
Copy link
Member Author

@allienna The feature itself should work with all formats as this is on databind level, not at streaming API.
So anything that would otherwise result in assignment of null should instead be handled as per annotation.
However, XML module has proven quite tricky (probably the most difficult format to support) so it is definitely good to try it out -- if handling does not work as per my comment, then file an issue.

One concern I have here is that since Tags is POJO, it is quite possible that empty <tags /> would map to empty POJO and not null. But I forget exact rule of how this works.

@maciejwalkowiak
Copy link

Is there a way to make it work - deserialize null or missing key to empty collection - for constructor based deserialization? I could not even make it work missing key work for setter.

Ideally I would like to set up ObjectMapper features in a way that all following test pass:

class SampleWithSetter {
    private List<Integer> list;

    List<Integer> getList() {
        return list;
    }

    @JsonSetter(nulls = Nulls.AS_EMPTY)
    void setList(List<Integer> list) {
        this.list = list;
    }
}
class SampleWithConstructor {
    private final List<Integer> list;

    SampleWithConstructor(@JsonProperty("list") List<Integer> list) {
        this.list = list;
    }

    List<Integer> getList() {
        return list;
    }
}
public class ListSerializationTest {

    private ObjectMapper objectMapper = new ObjectMapper();

    @Test // fails
    public void shouldDeserializeNullToEmptyWithConstructor() throws IOException {
        String json = "{\"list\":null}";

        SampleWithConstructor sample = objectMapper.readValue(json, SampleWithConstructor.class);

        assertThat(sample.getList()).isNotNull().isEmpty();
    }

    @Test // fails
    public void shouldDeserializeMissingToEmptyWithConstructor() throws IOException {
        String json = "{}";

        SampleWithConstructor sample = objectMapper.readValue(json, SampleWithConstructor.class);

        assertThat(sample.getList()).isNotNull().isEmpty();
    }

    @Test // passes
    public void shouldDeserializeNullToEmptyWithSetter() throws IOException {
        String json = "{\"list\":null}";

        SampleWithSetter sample = objectMapper.readValue(json, SampleWithSetter.class);

        assertThat(sample.getList()).isNotNull().isEmpty();
    }

    @Test // fails
    public void shouldDeserializeMissingKeyToEmptyWithSetter() throws IOException {
        String json = "{}";

        SampleWithSetter sample = objectMapper.readValue(json, SampleWithSetter.class);

        assertThat(sample.getList()).isNotNull().isEmpty();
    }
}

Help appreciated.

@cowtowncoder
Copy link
Member Author

@maciejwalkowiak What does "missing key" mean? That the property is not included? If so, no, that will not work -- missing property is NOT the same as one with null value. This is a fundamental difference and right now there is no action possible for case of data that is not there; only for values that are being included. In such cases I would recommend you to initialize property within class, and not rely on deserialization: if so, you may also want to use skip as null handling.

As to constructor properties: handling should work. If not, a new issue with reproduction should be filed -- it is easier to track smaller flaws (or missing functionality) that way.

@maciejwalkowiak
Copy link

Exactly - missing key meaning that the key is not present in JSON. In some cases it means something different, in some cases deserializing it to empty collection makes absolute sense - this is why it would be useful on the annotation level.

I will file separate issue with constructor properties.
Thanks for quick reply.

@cowtowncoder
Copy link
Member Author

@maciejwalkowiak Understood: missing information is important. Unfortunately its handling is... quite different, from impl perspective. But this is a known gap, and I hope it can be addressed. At this point it is only really handled in context of Creator properties, where some checking is possible.

@Ramblurr
Copy link

Reading through this issue it seems the only way to deserialize null arrays as empty collections is via the @JsonSetter(nulls = Nulls.AS_EMPTY) annotation on every setter.

Is there not a way to apply this to an object mapper as a deserialization feature?

@cowtowncoder
Copy link
Member Author

@Ramblurr you can define per-type and global config overrides for ObjectMapper; see tests under src/test/java/com/fasterxml/jackson/databind/deser/merge/. These are newer configuration mechanisms intended for providing defaults.

@lucasgonzagarosa
Copy link

lucasgonzagarosa commented Dec 8, 2017

Is there any way to do this at class level?
Like to annotate a class to assume a new Instance value when null?

@cowtowncoder
Copy link
Member Author

@Tharsalbird please use mailing lists for usage questions in future.

But I think what you are asking is possible by using Nulls.AS_EMPTY value and then implementing JsonDeserializer.getEmptyValue(...) method.
Luckily for POJOs it will work already, since this method is implemented to try to create an instance using default (0-arg) constructor (if one exists).

@ndori
Copy link

ndori commented Apr 25, 2018

Hi, I've noticed a behavior I think should be different with
@JsonSetter(value = "foo", nulls = Nulls.SKIP)
private String foo;
and with .configure(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL, true)

in that case, I would expect that if there is an unknown enum the setter will skip the null set, as I told him I don't want you to set null.

but what is probably happening is that it sees a value which is not null so it tries to set it, then when it can't set it, it becomes null.

I realize that it will be probably hard to fix, but on the other hand, the whole point of the nulls attribute is to do something else with nulls...

@cowtowncoder
Copy link
Member Author

@ndori Could you please file a new issue, explaining the problem as above. And if possible with really simple test case? It sounds like a flaw and I would like to keep track of it separately; you can add a reference to this issue for background.

@Brown-Dan
Copy link

Still not great, no way to use custom defaults other than overriding

@cowtowncoder cowtowncoder added the most-wanted Tag to indicate that there is heavy user +1'ing action label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
most-wanted Tag to indicate that there is heavy user +1'ing action
Projects
None yet
Development

No branches or pull requests

10 participants