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

Deserialization with builder ignores @JsonUnwrapped` in field of deserialized object type #3040

Closed
mjustin opened this issue Feb 2, 2021 · 4 comments

Comments

@mjustin
Copy link

mjustin commented Feb 2, 2021

Describe the bug
Deserialization fails when an object uses @JsonUnwrapped alongside a builder provided by @JsonDeserialize, unless that builder's "with" method also uses @JsonUnwrapped. Jackson appears to be using the builder's value instead of the object's value.

I am unsure if this is a bug or intentional, so filing as a bug in case this is not intended behavior.

I am running across this in Lombok, which by default does not transfer the @JsonUnwrapped annotation to its generated builder methods. It is very possible to work around this, but in either event, I'd like to know whether this is a bug or working as expected. If the behavior is intended, I will likely follow this up with a bug within Lombok itself if that seems appropriate.

Version information
Jackson 2.12.1

To Reproduce
The following unit test reproduces the issue.

import com.fasterxml.jackson.annotation.JsonUnwrapped;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.json.JsonMapper;
import org.junit.jupiter.api.Test;
import java.util.Objects;
import static org.junit.jupiter.api.Assertions.assertEquals;

public class JsonUnwrappedBuilderTest {
    private final ObjectMapper objectMapper = JsonMapper.builder().build();

    @Test
    public void testWithUnwrapped() throws JsonProcessingException {
        WithUnwrapped object = WithUnwrapped.builder().withNested(new NestedValue("A", "B")).build();
        String json = "{\"value1\":\"A\",\"value2\":\"B\"}";

        assertEquals(object, objectMapper.readValue(json, WithUnwrapped.class));
    }

    @JsonDeserialize(builder = WithUnwrapped.Builder.class) // Test passes if this is removed
    public static class WithUnwrapped {
        @JsonUnwrapped
        private NestedValue nested;

        public WithUnwrapped() { }
        public WithUnwrapped(NestedValue nested) { this.nested = nested; }

        public NestedValue getNested() { return nested; }
        public void setNested(NestedValue nested) { this.nested = nested; }
        public static Builder builder() { return new Builder(); }

        @Override 
        public boolean equals(Object o) {
            return o instanceof WithUnwrapped
                    && Objects.equals(nested, ((WithUnwrapped) o).nested);
        }

        @Override public int hashCode() { return Objects.hash(nested); }
        @Override public String toString() { return Objects.toString(nested); }

        private static class Builder {
            private NestedValue nested;

//            @JsonUnwrapped // Test passes if this is present
            public Builder withNested(NestedValue nested) {
                this.nested = nested;
                return this;
            }

            public WithUnwrapped build() {
                return new WithUnwrapped(nested);
            }
        }
    }

    public static class NestedValue {
        public NestedValue() { }

        public NestedValue(String value1, String value2) {
            this.value1 = value1;
            this.value2 = value2;
        }

        private String value1;
        private String value2;

        public String getValue1() { return value1; }
        public void setValue1(String value1) { this.value1 = value1; }
        public String getValue2() { return value2; }
        public void setValue2(String value2) { this.value2 = value2; }

        @Override
        public boolean equals(Object o) {
            return o instanceof NestedValue
                    && Objects.equals(value1, ((NestedValue) o).value1)
                    && Objects.equals(value2, ((NestedValue) o).value2);
        }

        @Override public int hashCode() { return Objects.hash(value1, value2); }
        @Override public String toString() { return "[" + value1 + "," + value2 + "]"; }
    }
}

Expected behavior
Deserializing an object with @JsonUnwrapped should work identically regardless of whether a builder is used or not.

@mjustin mjustin added the to-evaluate Issue that has been received but not yet evaluated label Feb 2, 2021
@cowtowncoder
Copy link
Member

This is intentional: none of target classes annotations are used in any way. To add support for trying to somehow combine annotations from value and builder classes would require significant new effort.

The easiest way to think about this is that builder class itself is essentially considered to be THE value being deserialized as far as Jackson is concerned; it is only at the end where "build method" is called and value it returns is returned as the final value.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Feb 2, 2021
@mjustin
Copy link
Author

mjustin commented Feb 2, 2021

When I looked at the @JsonDeserialize Javadocs prior to filing this bug report, I apparently failed to notice the bullet point that covers this point. I think I mostly was looking at the builder annotation element itself and missed it in the class-level documentation.

  • All other annotations regarding behavior during building should be on Builder class and NOT on target POJO class: for example @JsonIgnoreProperties should be on Builder to prevent "unknown property" errors.

@mjustin
Copy link
Author

mjustin commented Feb 2, 2021

It turns out that as of October, Lombok has the @Jacksonized annotation to automagically copy annotations to its builders. However, I discovered that it does not currently support @JsonUnwrapped so I filed a Lombok bug (projectlombok/lombok#2736).

@cowtowncoder
Copy link
Member

Thank you for your help here @mjustin -- yes, you are right, Lombok has those useful settings.
I'll close this issue but I hope it helps others find the help in case they are facing the same problem.

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

No branches or pull requests

2 participants