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

Lombok with Jackson Deserializer from camelCase #1981

Closed
ktalebian opened this issue Dec 14, 2018 · 8 comments
Closed

Lombok with Jackson Deserializer from camelCase #1981

ktalebian opened this issue Dec 14, 2018 · 8 comments

Comments

@ktalebian
Copy link

ktalebian commented Dec 14, 2018

Here is a simple Object

@Builder
@Value
@JsonDeserialize(builder = User.UserBuilder.class)
class User {
    @NotNull
    @JsonProperty("user_name")
    private final string userName;

    @JsonPOJOBuilder(withPrefix = "")
    public static final class UserBuilder {}
}

Now, if I want to use ObjectMapper to read a string to this, I do

final User user = MAPPER.readValue("{\"user_name\":\"my-name\"}", User.class);

But this does not read the user_name because when I look at the generated source code for the builder, I see

public static final class UserBuilder {
    @java.lang.SuppressWarnings("all")
    private String userName;
}

And so the camelCase doesn't match the snake_case.

Normally if I were to manually generate the Builder class, I would put @JsonProperty("user_name") on the builder property as well:

public static final class UserBuilder {
     @JsonProperty("user_name")
     private String userName;
}

How do I fix this? I want the Builder fields to also get the @JsonProperty annotation.

@janrieke
Copy link
Contributor

janrieke commented Dec 14, 2018

Are you sure this would work? I would expect Jackson to ignore that annotation on internal private fields of the builder class. At least it would be a bit surprising to me. But even if it works in Jackson, there is no easy way for Lombok to determine which annotation to put on the fields. For example, @NonNull-like annotations should not be put on the fields, because it's perfectly legit for such fields to be null during the lifetime of a builder instance.
So I would say the same as in #1961: That should be changed in Jackson.

@ktalebian
Copy link
Author

I would have hoped that lombok.copyableAnnotations+=com.fasterxml.jackson.annotation.JsonProperty would allow me to force Lombok to copy certain annotations.

@ktalebian
Copy link
Author

ktalebian commented Dec 14, 2018

Also looking at #1634, what if you have, annotations like @JsonSerialize and @JsonDeserialize should be copied to the fields.

@janrieke
Copy link
Contributor

Just for clarification: You want the annotation to be copied to the field, right? Because currently it is copied to the method's parameter, but Jackson does not recognise this.

@ktalebian
Copy link
Author

ktalebian commented Dec 14, 2018

Correct, I want my Builder to look like

public static final class UserBuilder {
     @JsonProperty("user_name")
     private String userName;
}

And it makes sense that annotations like @NotBlank should not exist on such fields, but then if I were to provide copyableAnnotations, it should override that.

@janrieke
Copy link
Contributor

I just verified that (surprisingly) it works if the internal private field of the builder has the @JsonProperty annotation, but the method has not. That's kind of weird, because only the public API of the builder (and not the implementation details of the builder) should be relevant to Jackson. On the other hand, Jackson is known for doing weird stuff. ;)

However, as stated above, IMO Lombok should not blindly copy all copyableAnnotations to the fields of the builder, because annotations like @NonNull (which are copyable by default) are semantically wrong there.

@rzwitserloot
Copy link
Collaborator

yes, the real issue is that 'copy that annotation where relevant' is insufficient information. With builder, there are at least 5 places we can copy it to:

  • The internal field in the builder that is used to track the property (which it should NEVER be copied to; that is basically an internal implementation detail and often doesn't even exist; for example, if you have an @Singular Map, there are 2 fields to 3 fields in the builder to track the status for that: 2 lists and a boolean. Surely any annotation that needs to be copied, should definitely not be copied to any of those 3!)
  • The parameter of the 'setter' method in the builder.
  • The entire method of the 'setter' method in the builder.
  • For @Singular: The parameter of the singular variant.
  • For @Singular: The method of the singular variant.

We could blow copyableAnnotations apart into 5 things, but, copyableAnnotations is by design used by more lombok features which come with their own 'actually this could apply to multiple locations', so unless we're all okay with like 10+ copyableAnnotations config keys (and to be clear, we are not), this is one of those 'this is impossible' feature requests.

What we can attempt to do is hardcode well known commonly used annotations, which is inelegant but them's the breaks.

I'll look into hardcoding special behaviour for @JsonProperty.

@phillipuniverse
Copy link

One unintended consequence of this is that mixins that worked before stopped working. Took a bit to figure out what was up, thought I would share with the class.

Given an original file:

@Data
public class Original {

    @JsonProperty(access = Access.READ_ONLY)
    private String someProperty;
}

And a mixin like this:

public class Mixin {
  
    @JsonProperty(access = Access.READ_WRITE)
    private String someProperty;
}

And an ObjectMapper configuration like this:

Original augmentation new ObjectMapper()
      .addMixIn(Original.class, Mixin.class)
     .readValue("{\"someProperty\":  \"someValue\"}", Original.class);

someProperty will always be ignored by Jackson and will always be null. This exact configuration worked fine in lombok 1.18.6.

Now Lombok creates this bytecode on Original:

@Data
public class Original {

    @JsonProperty(access = Access.READ_ONLY)
    private String someProperty;

    @JsonProperty(access = Access.READ_ONLY)
    public String getSomeProperty() {
        return someProperty;
    }

   @JsonProperty(access = Access.READ_ONLY)
   public String setSomeProperty(String someProperty) {
        this.somePropety = someProperty;
    }
}

Since the mixin does not have @Data on it it thus has no annotation on the getter/setter. Jackson ensures that getter/setter annotations take precedence, thus the mixin annotation is effectively ignored.

Fix is to ensure that your mixin also has @Getter and @Setter:

public class Mixin {
    @Getter
    @Setter
    @JsonProperty(access = Access.READ_WRITE)
    private String someProperty;
}

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

4 participants