Skip to content

Apply default null handling to @JsonAnySetter - #6171

Merged
cowtowncoder merged 13 commits into
FasterXML:3.xfrom
Dongnyoung:fix-6169-anysetter-null-handling
Sep 29, 2026
Merged

cowtowncoder merged 13 commits into
FasterXML:3.xfrom
Dongnyoung:fix-6169-anysetter-null-handling

Conversation

@Dongnyoung

@Dongnyoung Dongnyoung commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Mapper-level default null handling was not applied to properties handled by @JsonAnySetter.

For regular bean properties, valueNulls(Nulls.SKIP) is propagated through property metadata and resolved to a skipping NullValueProvider, so an input VALUE_NULL skips assignment.

@JsonAnySetter uses a separate path: its synthetic BeanProperty was created with PropertyMetadata.STD_OPTIONAL, without applying default null-handling configuration. As a result, null-valued unknown properties were still passed to the any-setter.

Issue fixed is #6170, discussed in #6169;

Fix

Apply the existing null-handling flow to @JsonAnySetter:

  • Resolve null-handling metadata for the synthetic any-setter BeanProperty using the existing setter-info logic.
  • Resolve and attach a NullValueProvider to SettableAnyProperty.
  • Skip any-setter assignment when the input token is VALUE_NULL and Nulls.SKIP is configured.
  • Apply the same behavior to buffered and creator-based any-setter paths.
  • Preserve existing custom value deserializer getNullValue() behavior when null skipping is not configured.

The skip decision is based on the input token being VALUE_NULL, not on the deserialized Java value being null. This preserves behavior for non-null input tokens that deserialize into Java null.

Tests

Added regression coverage for:

  • mapper-level valueNulls(Nulls.SKIP) with method-based @JsonAnySetter
  • buffered method-based any-setter handling
  • creator-based any-setter handling
  • non-null input tokens that deserialize to Java null
  • existing custom getNullValue() behavior

Fixes: #6170

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.89% 📉 -0.030%
Branches branches 75.54% 📉 -0.010%

Coverage data generated from JaCoCo test results

Comment thread src/main/java/tools/jackson/databind/deser/BeanDeserializerFactory.java Outdated
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.92% 📈 +0.000%
Branches branches 75.56% 📈 +0.010%

Coverage data generated from JaCoCo test results

Centralize null-provider preservation in a private helper and remove two JsonNode deserialize overrides that now duplicate the base behavior.

JAIPilot generated the cleanup. It was independently reviewed against the exact pull-request head, and the helper was kept private to avoid expanding the protected API.
@skrcode

skrcode commented Aug 22, 2026

Copy link
Copy Markdown

I ran JAIPilot Cloud against this exact PR head. It proposed a small follow-up that consolidates the new null-provider bookkeeping into one private helper without changing public behavior.

The cloud run passed 26 focused tests and the clean 6,188-test repository suite. This is a maintainability cleanup, not a performance claim.

Cloud-generated draft and evidence: skrcode#2
PR directly onto this source branch: Dongnyoung#1

Feel free to merge or cherry-pick if the consolidation is useful.

Consolidate SettableAnyProperty null-provider bookkeeping
@Dongnyoung

Dongnyoung commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor Author

@skrcode Thanks for the detailed follow-up. I reviewed the diff and the invariants around the JsonNode subclasses, and the cleanup looks good to me. I'll incorporate this into my PR branch.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.93% 📈 +0.020%
Branches branches 75.59% 📈 +0.030%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 82.36% 📈 +0.000%
Branches branches 75.99% 📈 +0.030%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 82.37% 📈 +0.030%
Branches branches 76.06% 📈 +0.060%

Coverage data generated from JaCoCo test results

@cowtowncoder cowtowncoder changed the title Apply default null handling to @JsonAnySetter Apply default null handling to @JsonAnySetter Sep 29, 2026
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 82.43% 📈 +0.010%
Branches branches 76.13% 📈 +0.040%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage
Instructions coverage 82.44%
Branches branches 76.13%

Coverage data generated from JaCoCo test results

@cowtowncoder

cowtowncoder commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

This is HUGE pr for sort of niche case... I wonder if there is any way to simplify code.

But on plus side, fixes look correct.

EDIT: I did some minor simplifying, am content with implementation now.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 82.45% 📈 +0.150%
Branches branches 76.14% 📈 +0.250%

Coverage data generated from JaCoCo test results

@gitar-bot

gitar-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 closed / 1 findings

🟡 Medium risk · Any-setter deserialization now honors configured null skip/fail behavior across creator paths.

Applies mapper-level default null handling to @JsonAnySetter properties by resolving null-handling metadata and attaching a NullValueProvider to skip null-valued unknown properties when configured. The fix addresses inconsistent null handling between method and field any-setters and covers buffered and creator-based paths, with regression tests for mapper-level valueNulls(Nulls.SKIP) and custom getNullValue() behavior.

✅ 1 closed
✅ Edge Case: Null-handling type differs between method vs field any-setters

📄 src/main/java/tools/jackson/databind/deser/BeanDeserializerFactory.java:877-891 📄 src/main/java/tools/jackson/databind/deser/BeanDeserializerFactory.java:977-988 📄 src/main/java/tools/jackson/databind/deser/BasicDeserializerFactory.java:653-664 📄 src/main/java/tools/jackson/databind/deser/bean/BeanDeserializerBase.java:657-662
_constructAnySetterProperty is passed the element valueType for method-based any-setters but the container fieldType/paramType for field/parameter-based ones. As a result, _getSetterInfo looks up per-type ConfigOverride null-handling (line 654: config.getConfigOverride(prop.getType().getRawClass())) against the value type in one case and against Map/JsonNode in the other, and _findNullProvider receives a prop whose getType() is the container type for field-based any-setters. This can make a ConfigOverride for e.g. Map.class (or Nulls.FAIL error metadata) apply inconsistently across the two any-setter styles. Consider constructing the synthetic property with the element/value type consistently (or documenting the intent) so config-override lookup and null-provider resolution behave the same for method- and field-based any-setters.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@cowtowncoder cowtowncoder added this to the 3.3.0 milestone Sep 29, 2026
@cowtowncoder
cowtowncoder merged commit 5a33919 into FasterXML:3.x Sep 29, 2026
5 checks passed
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 82.45% 📈 +0.150%
Branches branches 76.14% 📈 +0.250%

Coverage data generated from JaCoCo test results

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

Successfully merging this pull request may close these issues.

Allow skipping null values for properties handled by @JsonAnySetter

3 participants