FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.53k stars 1.38k forks source link

`@JsonInclude(value = JsonInclude.Include.CUSTOM,valueFilter = SomeFilter.class)` does not work for fields of record (fixed in 2.18.0) #4754

Closed swpalmer-cl closed 1 month ago

swpalmer-cl commented 1 month ago

Search before asking

Describe the bug

The valueFilter is not used at all when applied to a field of a record. An object of the filter class is not even constructed.

Version Information

2.17.0

Reproduction

public record MyRecord(
    @JsonInclude(
        value = JsonInclude.Include.CUSTOM,
        valueFilter = AttributeTypeSerializationFilter.class)
    String attributeType,
    String attributeValue) {

  // Never return null for the attributeType, instead return the default
  public MyRecord {
    if (attributeType == null) attributeType = "String";
  }

  static class AttributeTypeSerializationFilter {

    public AttributeTypeSerializationFilter() {
      System.out.println("AttributeTypeFilter.init");
    }

    @Override
    // return true to exclude
    public boolean equals(Object obj) {
      System.err.println("AttributeTypeFilter: " + obj);
      return obj == null || "String".equals(obj);
    }
  }
}

Expected behavior

@JsonInclude(value=JsonInclude.Include.CUSTOM, ... ) should work on record fields as it does for class fields.

Additional context

The idea is to reduce the JSON payload by excluding default values, but those values are non-null.

JooHyukKim commented 1 month ago

Did it work in previous versions? If so, could you share with us which?

yihtserns commented 1 month ago

More importantly, where are the steps to reproduce the issue?

cowtowncoder commented 1 month ago

Also: 2.17.0 is neither latest patch (2.17.2) (nor latest in general (2.18.0)). So should:

  1. Verify it occurs on 2.17.2 (slight possibility of fix)
  2. Ideally verify against 2.18.0 (reasonable chance might be fixed)

so we won't waste time looking into something that might already be fixed.

yihtserns commented 1 month ago

BTW I'm asking for steps to reproduce the issue because I cannot reproduce it with a 2.17.0 + Record + custom filter + ObjectMapper.writeValueAsString(...), so I highly suspect that the report is missing some context.

swpalmer-cl commented 1 month ago

I will try to create a more complete example from my code, but just for reference. I took the record I had, and re-wrote it as a class and only then did the filter get called. The ObjectMapper was configured exactly the same in both cases.

yihtserns commented 1 month ago

All passed:

public class JsonIncludeCustomTest {

    private ObjectMapper objectMapper = new ObjectMapper();

    @Test
    public void testBean() throws Exception {
        MyBean beanNone = new MyBean();
        beanNone.setName("N/A");
        beanNone.setAddress("N/A");

        MyBean beanDoe = new MyBean();
        beanDoe.setName("Doe");
        beanDoe.setAddress("Doe");

        assertEquals("{\"address\":\"N/A\"}", objectMapper.writeValueAsString(beanNone));
        assertEquals("{\"name\":\"Doe\",\"address\":\"Doe\"}", objectMapper.writeValueAsString(beanDoe));
    }

    @Test
    public void testRecord() throws Exception {
        MyRecord recordNone = new MyRecord("N/A", "N/A");
        MyRecord recordDoe = new MyRecord("Doe", "Doe");

        assertEquals("{\"address\":\"N/A\"}", objectMapper.writeValueAsString(recordNone));
        assertEquals("{\"name\":\"Doe\",\"address\":\"Doe\"}", objectMapper.writeValueAsString(recordDoe));
    }

    public static class MyBean {

        @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = NameFilter.class)
        private String name;
        private String address;

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }

        public String getAddress() {
            return address;
        }

        public void setAddress(String address) {
            this.address = address;
        }
    }

    record MyRecord(
            @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = NameFilter.class) String name,
            String address) {
    }

    public static class NameFilter {

        @Override
        public boolean equals(Object obj) {
            return "N/A".equals(obj);
        }
    }
}
swpalmer-cl commented 1 month ago

I think I found the issue... Change the record to:

    record MyRecord(
            @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = NameFilter.class) String name,
            String address) {
        @Override
        public String name() {
            if (name == null) return "N/A";
            return name;
        }
    }

You don't even need to do anything "odd" in the name() accessor method. This also fails:

        @Override
        public String name() {
            return name;
        }

So even if you just wanted to add logging or something it will skip the filter simply by overriding the default accessor method.

yihtserns commented 1 month ago

Right, I missed the accessor method override, relying too much on the issue title (which should maybe be updated to "@JsonInclude(value = JsonInclude.Include.CUSTOM,valueFilter = SomeFilter.class) does not work for Record field when the accessor is overridden")

The cause is the same as #4626, and has been fixed in 2.18.0.

cowtowncoder commented 1 month ago

@swpalmer-cl Did you try this with other versions like I requested? There is little value in working on non-latest patch (2.17.0) and there is strong indicates this might be fixed in 2.18.0 -- and if so, would be documented as such (fix unlikely to be backportable due to major rewrite in 2.18).

swpalmer-cl commented 1 month ago

@cowtowncoder I did not, I apologise. It is indeed fixed in 2.18.0

yihtserns commented 1 month ago

I forgot to mention an alternative/workaround if you can't/won't upgrade - move the annotation to the overridden accessor method:

record MyRecord(String name, String address) {

    @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = NameFilter.class) 
    @Override
    public String name() {
        if (name == null) return "N/A";
        return name;
    }
}
cowtowncoder commented 1 month ago

Thank you @swpalmer-cl!