Make FieldProperty skip nulls on set() method
#4455
Conversation
|
But what about On target branch: I would not make risky change like this for patch, esp. not 2.15. |
Great question, I added test for method also. Observation is that....
basically both field & method case null is passed. |
hmmm 2.17 while it's still RC then? |
|
2.17.0 was released 😀
…On Thu, Mar 28, 2024 at 7:01 PM Kim, Joo Hyuk ***@***.***> wrote:
On target branch: I would not make risky change like this for patch, esp.
not 2.15.
hmmm 2.17 while it's still RC then?
—
Reply to this email directly, view it on GitHub
<#4455 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANOGODVHXFXR7YKIF3MJLY2TDQVAVCNFSM6AAAAABFNAEZAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRWGQ3TAMZSG4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Oh... haha |
| private final List<Inner> listInner = new ArrayList<>(); | ||
| private final String field1; | ||
|
|
||
| @ConstructorProperties({"field1"}) |
There was a problem hiding this comment.
Let's change this to use @JsonCreator instead to reduce use of @ConstructorProperties (in JPMS it's in extra package)
| @@ -186,6 +186,12 @@ public Object deserializeSetAndReturn(JsonParser p, | |||
| @Override | |||
| public void set(Object instance, Object value) throws IOException | |||
| { | |||
| // [databind#4441] 2.17 Fix `@JsonSetter(Nulls.SKIP)` field is not skipped up to this point | |||
| if (value == null) { | |||
There was a problem hiding this comment.
Same also needed for setAndReturn() method, I think.
| @@ -0,0 +1,149 @@ | |||
| package com.fasterxml.jackson.databind.deser; | |||
There was a problem hiding this comment.
I think Nulls.SKIP tests are under src/test/java/com/fasterxml/jackson/databind/deser/filter/ typically?
|
@JooHyukKim Ok sorry, I think this could actually be added in 2.17(.1) if it is possible to rebase PR. Change is safer than I originally thought. And yeah looks like skipping was accidentally omitted, probably assuming that the other 2 combo-methods would always be called. But that is not true for buffering cases. |
Alright, will apply your review and try to create separate PR instead. PS: filed #4469 accordingly. |
fixes #4441
As per comment, the problem seems to be around buffering + ordering of fields (implementing current PR made me believe more so). But since buffer sort of buffers by natural ordering of input JSON, I figured maybe we can just skip nulls within the field itself.
PS : target branch TBD