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

@JsonNaming on value class not applied to use via Builder #2712

Open
gbaso opened this issue May 6, 2020 · 6 comments
Open

@JsonNaming on value class not applied to use via Builder #2712

gbaso opened this issue May 6, 2020 · 6 comments
Labels
builder-related Issue related to handling of Builder-style deserialization

Comments

@gbaso
Copy link

gbaso commented May 6, 2020

Given an input json string like "{\"Id\":1,\"Title\":\"title\",\"ShortDescription\":\"desc\"}", I'd like to keep java naming conventions on my class, and use PropertyNamingStrategy.UpperCamelCaseStrategy for deserialization:

    @lombok.Data
    @lombok.Builder
    @lombok.NoArgsConstructor
    @lombok.AllArgsConstructor(access = AccessLevel.PACKAGE)
    @JsonNaming(PropertyNamingStrategy.UpperCamelCaseStrategy.class)
    public class Foo {
        private Long   id;
        private String title;
        private String shortDescription;
    }

While this works as intended, if I wanted to make the class (sort of) immutable I'd have to remove the no args constructor, and use the builder to deserialize:

    @lombok.Value
    @lombok.Builder
    @lombok.AllArgsConstructor(access = AccessLevel.PACKAGE)
    @JsonDeserialize(builder = Foo.FooBuilder.class)
    @JsonNaming(PropertyNamingStrategy.UpperCamelCaseStrategy.class)
    public class Foo {
        private Long   id;
        private String title;
        private String shortDescription;
    }

but this results in an empty bean. The builder setters are never invoked, Jackson cannot match them to the input properties:

ObjectMapper mapper = new ObjectMapper();
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
String json = "{\"Id\":1,\"Title\":\"title\",\"ShortDescription\":\"desc\"}";
mapper.readValue(json, Foo.class);
//Foo(id=null, title=null, shortDescription=null)

Currently on Jackson 2.10.3.

@gbaso
Copy link
Author

gbaso commented May 6, 2020

Delombokified source:

public class Foo {
    private Long   id;
    private String title;
    private String shortDescription;

    public Long getId() {
        return id;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public String getTitle() {
        return title;
    }

    public void setTitle(String title) {
        this.title = title;
    }

    public String getShortDescription() {
        return shortDescription;
    }

    public void setShortDescription(String shortDescription) {
        this.shortDescription = shortDescription;
    }

    public Foo() {
    }

    Foo(Long id, String title, String shortDescription) {
        super();
        this.id = id;
        this.title = title;
        this.shortDescription = shortDescription;
    }

    public static FooBuilder builder() {
        return new FooBuilder();
    }

    public static class FooBuilder {
        private Long   id;
        private String title;
        private String shortDescription;

        public FooBuilder id(Long id) {
            this.id = id;
            return this;
        }

        public FooBuilder title(String title) {
            this.title = title;
            return this;
        }

        public FooBuilder shortDescription(String shortDescription) {
            this.shortDescription = shortDescription;
            return this;
        }

        public Foo build() {
            return new Foo(id, title, shortDescription);
        }
    }

}

@cowtowncoder
Copy link
Member

@JsonNaming will need to be added to Builder class, not value class -- this is basically the rule for all builder-based functionality, mostly due to implementation details.
So that would solve the issue in your case, if Lombok can apply it there (or is configurable).

But I think it is reasonable to suggest that at least some annotations from value class should be sort of added as defaults to be used with builder.
So I think I can keep this as a placeholder for such request(s) -- I may create separate issue for more general functionality, but at least for now this works.

@cowtowncoder cowtowncoder changed the title @JsonNaming strategy ignored when deserializing with builder @JsonNaming on value class not applied to use via Builder May 6, 2020
@gbaso
Copy link
Author

gbaso commented May 6, 2020

There isn't a direct way to put annotations on generated builder classes, but you can write an empty builder class and lombok will fill it for you:

    @lombok.Value
    @lombok.Builder
    @JsonDeserialize(builder = Foo.FooBuilder.class)
    @JsonNaming(PropertyNamingStrategy.UpperCamelCaseStrategy.class)
    public class Foo {
        private Long   id;
        private String title;
        private String shortDescription;

		@JsonNaming(PropertyNamingStrategy.UpperCamelCaseStrategy.class)
		@JsonPOJOBuilder(withPrefix = "")
		public static class FooBuilder {
		}

    }

While this isn't perfect, and a solution like you mentioned (inheriting annotations from the value class) would be preferable, it is certainly acceptable as an alternative.

Thanks!

@cowtowncoder
Copy link
Member

@Ganondolf Ok good. And thank you for reporting this usability issue; related problems have come up occasionally but for different annotations. I hope to maybe address this in 2.12, depending on how things go. But at least it is more visible now.

@janrieke
Copy link

janrieke commented May 24, 2020

There will be a new annotation @Jacksonized (PR projectlombok/lombok#2387) in the next lombok release (it's also already in the current snapshot build, if you want to give it a try). It can be used in combination with @Builder and @SuperBuilder to automatically configure the generated builder for Jackson. This includes copying the @JsonNaming annotation (and several others, e.g. @JsonIgnoreProperties) to the builder class, so in the use-case described here you won't have to manually add that to the builder class any more.

@cowtowncoder
Copy link
Member

@janrieke Wow. That looks really really nice. And even if Jackson was to do this eventually on its own, it is good to have ability to customize handling (and obviously have this available now/soon, instead of "maybe in future").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder-related Issue related to handling of Builder-style deserialization
Projects
None yet
Development

No branches or pull requests

3 participants