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

[BUG] Field-based Jackson mixins break when using @Setter #2769

Open
mjustin opened this issue Mar 12, 2021 · 2 comments
Open

[BUG] Field-based Jackson mixins break when using @Setter #2769

mjustin opened this issue Mar 12, 2021 · 2 comments

Comments

@mjustin
Copy link

mjustin commented Mar 12, 2021

Describe the bug

Jackson allows creating mixins which allow defining Jackson annotation-based configuration on a class mirroring the class to be (de)serialized. If the annotations are on the fields but not the getters/setters, then the "mixin" class must likewise be annotated on the fields.

For instance, take the following configuration:

public class Example {
    @JsonProperty(access = JsonProperty.Access.READ_ONLY)
    private String value;

    private String other;
}

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

In this case, deserializing the JSON value {"value": "Some Value", "other": "Other Value"} normally will produce an Example object with value set to null and other set to "Other Value", since the @JsonProperty annotation tells Jackson to skip the field during deserialization. Deserializing it with the mixin added, on the other hand, will produce an Example object with value set to "Some Value" and other set to "Other Value", as the mixin tells Jackson to deserialize it.

Annotating value in class Example with @Setter will cause the field-based mixin to effectively be ignored, wheras manually adding a setter will not.

// Deserializing the example JSON with ExampleMixin will result in a null value field.
public class Example {
    @Setter
    @JsonProperty(access = JsonProperty.Access.READ_ONLY)
    private String value;
}
// Deserializing the example JSON with ExampleMixin will result in a value field set to "Some Value".
public class Example {
    @JsonProperty(access = JsonProperty.Access.READ_ONLY)
    private String value;
    
    public void setValue(String value) {
        this.value = value;
    }
}

The reason for this discrepancy is that Lombok automatically copies a number of Jackson annotations from fields to their generated setters per @Setter:

https://github.com/rzwitserloot/lombok/blob/2e5ea518cc4041f2ef79d5d9cc0f970d805b6e64/src/core/lombok/core/handlers/HandlerUtil.java#L316-L329

With @Setter, the generated setter has the @JsonProperty(access = JsonProperty.Access.READ_ONLY). Since Jackson getter/setter annotations have priority over field annotations, the generated setter annotation wins out over both the annotated field on the class and on the mixin. Therefore, the field is not deserialized.

There are a number of ways this situation can be fixed by the user. The mixin can be changed to have an annotated setter (thus triggering an override of the Lombok setter), either manually or by annotating the mixin field using @Setter. The deserialized class can have an unannotated manual setter created for it in addition to the annotated field, thus preventing Lombok from creating an annotated one.

All of these fixes require the user to be aware of the fact that the generated setter method is being annotated with the Jackson annotation. However, the fact that Jackson annotations are copied over to the setter is not documented in the @Getter and @Setter page. That page only mentions nullability annotations as being copied over, and makes no mention of the copied Jackson annotations:

lombok.copyableAnnotations = [A list of fully qualified types] (default: empty list)
Lombok will copy any of these annotations from the field to the setter parameter, and to the getter method. Note that lombok ships with a bunch of annotations 'out of the box' which are known to be copyable: All popular nullable/nonnull annotations.

Various well-known annotations about nullability, such as org.eclipse.jdt.annotation.NonNull, are automatically copied over to the right place (method for getters, parameter for setters). You can specify additional annotations that should always be copied via lombok configuration key lombok.copyableAnnotations.

To Reproduce

ExampleFile.java

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.json.JsonMapper;
import lombok.Setter;

public class ExampleFile {
    public static void main(String[] args) throws Exception {
        JsonMapper mixinMapper = JsonMapper.builder().addMixIn(Example.class, ExampleMixin.class).build();
        Example example = mixinMapper.readValue("{\"value\": \"Some Value\"}", Example.class);
        if (!"Some Value".equals(example.value)) {
            throw new IllegalStateException("Wrong value; was: " + example.value);
        }
    }

    private static class Example {
        @Setter // Correct results if this is commented out or replaced with a manual setter
        @JsonProperty(access = JsonProperty.Access.READ_ONLY)
        private String value;
    }

    private abstract static class ExampleMixin {
//      @Setter // Correct results if this is uncommented
        @JsonProperty(access = JsonProperty.Access.READ_WRITE)
        private String value;
    }
}

Reproduction Steps

$ curl -s https://repo1.maven.org/maven2/org/projectlombok/lombok/1.18.18/lombok-1.18.18.jar -O
$ curl -s https://repo1.maven.org/maven2/com/fasterxml/jackson/core/jackson-annotations/2.12.2/jackson-annotations-2.12.2.jar -O
$ curl -s https://repo1.maven.org/maven2/com/fasterxml/jackson/core/jackson-core/2.12.2/jackson-core-2.12.2.jar -O
$ curl -s https://repo1.maven.org/maven2/com/fasterxml/jackson/core/jackson-databind/2.12.2/jackson-databind-2.12.2.jar -O
$ javac -cp "*" ExampleFile.java
$ java -cp ".:*" ExampleFile
Exception in thread "main" java.lang.IllegalStateException: Wrong value; was: null
	at ExampleFile.main(ExampleFile.java:10)

Expected behavior

At a minimum, the fact that Jackson annotations are copied over should be documented on the @Getter and @Setter page. Alternatively, the annotations shouldn't be copied by default, and the above reproduction steps should pass. Perhaps the Jackson annotations might not be copied by default, and the @Jacksonized annotation or the like could be used as a way of opting-into the Jackson annotation copying.

Version info (please complete the following information):

  • Lombok 1.18.18
  • javac 11.0.9.1
@mjustin
Copy link
Author

mjustin commented Mar 12, 2021

This issue was mentioned here: #1981 (comment):

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.

[...]

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

@janrieke
Copy link
Contributor

Is there any advantage in copying the annotations at all? Jackson considers field, getter, and setter as a unit; an annotation on one of them holds for all.

I can imagine one use-case: Jackson uses a slightly different algorithm for matching those than Lombok (crappy bean spec once again). Adding a `@JsonProperty("nAme") on the field would fix such a case. But is this worth it?

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