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

Allow @JsonCreator annotated Creator to retun null #4938

Open
f-aubert opened this issue Jan 28, 2025 · 7 comments · May be fixed by #4948
Open

Allow @JsonCreator annotated Creator to retun null #4938

f-aubert opened this issue Jan 28, 2025 · 7 comments · May be fixed by #4948
Labels
2.19 Issues planned at 2.19 or later

Comments

@f-aubert
Copy link

f-aubert commented Jan 28, 2025

Is your feature request related to a problem? Please describe.

When using JsonCreator on a factory (static) method, we have the possibility of returning null (because for example all the arguments are empty). However during deserialization an exception is thrown, java.lang.NullPointerException: JSON Creator returned nul.

I think it would make sense to allow deserialization to null

Describe the solution you'd like

Improve the code robustness in the BeanEntityDeserializer to allow Deserialization to null if a static JsonCreator explicitly allows it.

Usage example

@JsonCreator
public static Localized3 of(@JsonProperty("en") String en, @JsonProperty("de") String de, @JsonProperty("fr") String fr) {
        if (en == null && de == null && fr == null) {
            return null;
        }

        return new Localized3(en, de, fr);
}

should be allowed for Deserialization and { en: null, de: null, fr: null } should deserialize to null

Additional context

No response

@f-aubert f-aubert added the to-evaluate Issue that has been received but not yet evaluated label Jan 28, 2025
@JooHyukKim
Copy link
Member

Seems like reasonable request, but this request conflicts with current behavior.
According to #597, users might be actually building things on top of current behavior.

So either of ....

  1. Intentionally change current behavior
  2. introduce a new configuration f.ex DeserializationFeature.ALLOW_NULL_RETURNING_CREATOR
  3. TBD

@JooHyukKim
Copy link
Member

Here is reproduction

public class JsonCreatoreReturningNull4938Test
    extends DatabindTestUtil
{
    // The class with the JsonCreator static factory method
    static class Localized3 {
        public final String en;
        public final String de;
        public final String fr;

        @JsonCreator
        public static Localized3 of(
                @JsonProperty("en") String en,
                @JsonProperty("de") String de,
                @JsonProperty("fr") String fr) {
            if (en == null && de == null && fr == null) {
                return null; // Explicitly return null when all arguments are null
            }
            return new Localized3(en, de, fr);
        }

        private Localized3(String en, String de, String fr) {
            this.en = en;
            this.de = de;
            this.fr = fr;
        }
    }

    private final ObjectMapper MAPPER = newJsonMapper();

    @Test
    void testDeserializeToNullWhenAllPropertiesAreNull() throws Exception {
        // JSON input where all properties are null
        String json = "{ \"en\": null, \"de\": null, \"fr\": null }";

        // Deserialize JSON to Localized3 object
        Localized3 result = MAPPER.readValue(json, Localized3.class);

        // Verify that the result is null
        assertNull(result, "Deserialization should result in null when all properties are null");
    }

}

@f-aubert
Copy link
Author

@JooHyukKim thanks for considering my request and adding the missing test case! That's very nice. I also read the referred issue 597 but no one there seems to demonstrate a usage of the current behavior and I could not think of one either. My guess would be a better exception helped debugging code or simply throwing better error message. But a DeserializationFeat would also work. But you seems to limit them?

@JooHyukKim
Copy link
Member

@cowtowncoder WDYT?

@cowtowncoder
Copy link
Member

What is the current exception? To me, ability to return null seems reasonable and I am not sure why it should be considered error case. I don't remember past reasoning.

If we do want it to be optional, DeserializationFeature seems reasonable.

@cowtowncoder
Copy link
Member

Oh. I think I may remember why null was blocked: for Properties-based case there's now nothing to bind possible extra properties to -- so code was being bit opinionated.

However, I think in that case we should just skip following properties and take null to mean nothing more is to be used from that level. At least that seems reasonable enough to me now.

So I think I'm ok actually changing behavior for 2.19, without configurability.

That is, unless someone can come up with a scenario where this should be prevented (configurably or not).

@JooHyukKim JooHyukKim linked a pull request Feb 2, 2025 that will close this issue
@cowtowncoder cowtowncoder added 2.19 Issues planned at 2.19 or later and removed to-evaluate Issue that has been received but not yet evaluated labels Feb 5, 2025
@cowtowncoder
Copy link
Member

So just make sure: I am +1 for allowing this, and working with @JooHyukKim on a pr to get implemented for 2.19.

@cowtowncoder cowtowncoder changed the title Feat. Deserialization to null Allow @JsonCreator annotated Creator to retun null Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.19 Issues planned at 2.19 or later
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants