jackson-databind
jackson-databind copied to clipboard
Filter method only got called once if the field is null when using `@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = SomeFieldFilter.class)`
Describe the bug
The filter method only get called once if the field value is NULL,
when using @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = SomeFieldFilter.class)
Version information 2.12.6.1
To Reproduce
- create an HTTP get endpoint to return an instance of
ObjectDtowhich has aNULLvalue forfieldDto:
public class ObjectDto {
@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = SomeFieldFilter.class)
private FieldDto fieldDto;
}
-
Send requests to the endpoint.
The first time, theequals()method ofSomeFieldFilteris called during the serialization. The same method never get called for the HTTP requests after. -
For instances that have a
non-NULLvalue forfieldDto, theequals()method of SomeFieldFilter` gets called all the time during the serialization
Expected behavior
For instances that have a NULL value for fieldDto, the equals() method of SomeFieldFilter also gets called all the time during the serialization
Additional context N/A
Thank you for reporting this issue. It sounds like possible bug.
It would be great to have a reproduction (unit test), if possible.
Ah, ok. This is a feature, not bug -- you are right, null check is only done once: this is an optimization to make null handling (more) efficient, partly, but also needed because value filtering has different code path for null values in general (many of default filters deal with nulls).
Calling filtering for repeated nulls would seem to serve no purpose, given that no meaningful arguments are passed to equals() (since it gets null). But I will try to see how to improve Javadocs, if possible.
Thank you very much for the prompt reply.
It's great that we have more optimized code running faster. I would hope the optimization does not change the behavior based on values.
I believe JsonInclude.Include.CUSTOM is a very cool feature that
the JsonInclude can use more complex logic in the filter class to decide
if a field shows in the serialized result or not,
no matter what the value of the field is.
The equals method of the filter class may return different results from time to time.
So, with this optimization, it behaves like this for non-null values,
but not for the null value.
Which version introduced this optimization? Is it possible to check the filter every time even if the value is null?
Thanks in advance.
This has always been the behavior, since the addition of CUSTOM, as far as I remember.
The skipping of call for null is partly due to implementation details -- null check is on distinct, separate code path all other types of values. So trying to make the call for every null is both unnecessary as far as I can see -- why would that ever need to vary? (keep in mind that filter instance is stateless and gets passed no contextual information) -- as well as a minor optimization, due to the way filtering code works.
Given above I don't think I have time to dig into whether change would be possible. But if you want to see if that is possible and propose a patch, I am not against that. I would, however, want to know why such a change is beneficial: that is, what exactly would be the use case?
I see. Thank you very much for the explanation.
I guess I didn't notice it before because the setting was only serializing non-null fields and
using JsonInclude.Include.CUSTOM to do further customization.
Now, the setting is always serializing everything as long as
JsonInclude.Include.CUSTOM allows or there's no JsonInclude.Include.CUSTOM.
Actually, the results of equals of the filter change from time to time.
For example, some fields are included during the weekdays, but not on weekends.
Yes, I can take a look to see if it's feasible. Could you show me where to start?
@AmiDavidW I don't remember specifically where, but you can search for Include.CUSTOM references in the codebase.
But as to filtering: if your filters are more complicated, you may want to check out use of @JsonFilter instead.
@JsonInclude is meant for somewhat static use cases, where the value alone determines the logic.
There are examples f.ex:
https://www.logicbig.com/tutorials/misc/jackson/json-filter-annotation.html
Thank you very much. I will check both
@cowtowncoder I provided a fix. Please review it and let me know if I need to make changes.
@cowtowncoder I signed and sent the CLA. Please let me know if there's anything else I need to do.
Thank you. I hope to have time to check the PR in near future -- thank you for contributing it and sending CLA!
Added a note on PR; apologies for slow followup.
No worries, @cowtowncoder. Thanks a lot for checking it.
I replied on the PR #3486