Skip to content

Conversation

pjfanning
Copy link
Member

@pjfanning pjfanning commented Oct 6, 2025

@wendigo
Copy link

wendigo commented Oct 6, 2025

    record RecordWithOptionalsOnly(Optional<String> optional)
    {
        public RecordWithOptionalsOnly
        {
            requireNonNull(optional, "optional is null");
        }
    }

    record RecordWithOptionalField(RecordWithOptionalsOnly record)
    {
        public RecordWithOptionalField
        {
            requireNonNull(record, "record is null");
        }
    }

    @Test
    void deserializeNestedOptional() throws Exception
    {
        String json = a2q("{'record':{}}");
        JsonMapper mapper = JsonMapper.builder()
                .changeDefaultPropertyInclusion(
                        v -> JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS))
                .build();
        mapper.readValue(json, RecordWithOptionalField.class);
    }

fails, but String json = a2q("{'record':{'optional':null}}"); passes

@wendigo
Copy link

wendigo commented Oct 6, 2025

In your examples it will fail too if the json is:

String json = a2q("{'name':'v1'}");

@JooHyukKim
Copy link
Member

@wendigo Seems like changing to ALWAYS would make it pass (like below)

builder.changeDefaultPropertyInclusion(f -> JsonInclude.Value.construct(JsonInclude.Include.ALWAYS, JsonInclude.Include.ALWAYS));

Could you try? If so, something have changed wrt JsonInclude must've changed.

@wendigo
Copy link

wendigo commented Oct 6, 2025

This passes on Jackson 2.x:

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
import org.junit.jupiter.api.Test;

import java.util.Optional;

import static java.util.Objects.requireNonNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

public class RecordWithOptionalTest
{
    record RecordWithOptionalParam(String name, Optional<String> optional) {
        public RecordWithOptionalParam
        {
            requireNonNull(optional, "optional is null");
        }
    }

    record RecordWithOptionalsOnly(Optional<String> optional)
    {
        public RecordWithOptionalsOnly
        {
            requireNonNull(optional, "optional is null");
        }
    }

    record RecordWithOptionalField(RecordWithOptionalsOnly record)
    {
        public RecordWithOptionalField
        {
            requireNonNull(record, "record is null");
        }
    }

    private final ObjectMapper MAPPER = JsonMapper.builder()
            .addModule(new Jdk8Module())
            .build();

    @Test
    void serialize() throws Exception
    {
        RecordWithOptionalParam input = new RecordWithOptionalParam("v1", Optional.of("v2"));
        String json = MAPPER.writeValueAsString(input);
        String expected = a2q("{'name':'v1','optional':'v2'}");
        assertEquals(expected, json);
    }

    @Test
    void serializeEmpty() throws Exception
    {
        RecordWithOptionalParam input = new RecordWithOptionalParam("v1", Optional.empty());
        String json = MAPPER.writeValueAsString(input);
        String expected = a2q("{'name':'v1','optional':null}");
        assertEquals(expected, json);
    }

    @Test
    void deserializeNonEmpty() throws Exception
    {
        String json = a2q("{'name':'v1','optional':'v2'}");
        RecordWithOptionalParam output = MAPPER.readValue(json, RecordWithOptionalParam.class);
        assertEquals("v1", output.name());
        assertEquals(Optional.of("v2"), output.optional());
    }

    @Test
    void deserializeEmpty() throws Exception
    {
        String json = a2q("{'name':'v1','optional':null}");
        RecordWithOptionalParam output = MAPPER.readValue(json, RecordWithOptionalParam.class);
        assertEquals("v1", output.name());
        assertNotNull(output.optional());
        assertEquals(Optional.empty(), output.optional());
    }

    @Test
    void deserializeIssue5335() throws Exception
    {
        String json = a2q("{'name':'v1'}");
        JsonMapper mapper = JsonMapper.builder()
                .addModule(new Jdk8Module())
                .defaultPropertyInclusion(JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS))
                .build();
        RecordWithOptionalParam output = mapper.readValue(json, RecordWithOptionalParam.class);
        assertEquals("v1", output.name());
        assertNotNull(output.optional());
        assertEquals(Optional.empty(), output.optional());
    }

    @Test
    void deserializeNestedOptional() throws Exception
    {
        String json = a2q("{'record': {}}");
        JsonMapper mapper = JsonMapper.builder()
                .addModule(new Jdk8Module())
                .defaultPropertyInclusion(JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS))
                .build();
        mapper.readValue(json, RecordWithOptionalField.class);
    }

    public static String a2q(String json)
    {
        return json.replace('\'', '"');
    }
}

@wendigo
Copy link

wendigo commented Oct 6, 2025

@Test
    void deserializeIssue5335() throws Exception
    {
        String json = a2q("{'name':'v1'}");
        JsonMapper mapper = JsonMapper.builder()
                .changeDefaultPropertyInclusion(_ -> JsonInclude.Value.construct(JsonInclude.Include.ALWAYS, JsonInclude.Include.ALWAYS))
                .build();
        RecordWithOptionalParam output = mapper.readValue(json, RecordWithOptionalParam.class);
        assertEquals("v1", output.name());
        assertNotNull(output.optional());
        assertEquals(Optional.empty(), output.optional());
    }

still fails

@pjfanning
Copy link
Member Author

@wendigo I've updated the test to include some broken cases based on your suggestion. They are annotated with JacksonTestFailureExpected so that they don't break the CI build by failing but they can still be run in an IDE.

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 7, 2025

Same challenge as #5335, I think -- need to separate serialization and deserialization aspects. While related for end-to-end usage, handling internally fully decoupled and need to understand and process separately (isolate ser vs deser processing, invariants, behavior).

note: I realize test methods are appropriately split, but the issue doesn't seem to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants