Skip to content

Fix @JsonAnyGetter keys not prefixed when inner bean uses @JsonUnwrapped(prefix=...) - #6123

Open
pjfanning wants to merge 15 commits into
FasterXML:3.xfrom
pjfanning:copilot/unwrapped-test
Open

Fix @JsonAnyGetter keys not prefixed when inner bean uses @JsonUnwrapped(prefix=...)#6123
pjfanning wants to merge 15 commits into
FasterXML:3.xfrom
pjfanning:copilot/unwrapped-test

Conversation

@pjfanning

@pjfanning pjfanning commented Jul 25, 2026

Copy link
Copy Markdown
Member

#6122
#6118

When @JsonAnyGetter appears on a bean referenced via @JsonUnwrapped(prefix="..."), the prefix was never applied to the any-getter map keys. Worse, AnyGetterWriter didn't override rename(NameTransformer), so the inherited path called _new(PropertyName) which throws IllegalStateException for subclasses — crashing serializer construction.

Example

class Outer {
    public String name = "aaa";
    @JsonUnwrapped(prefix = "a-")
    public Inner inner; // inner.getExtra() returns {"age": "64"}
}
// Before fix: IllegalStateException at mapper construction, or wrong output
// After fix:  {"name":"aaa","a-age":"64"}

Changes

AnyGetterWriter

  • Added NameTransformer _nameTransformer field (null in normal, non-unwrapped use)
  • Added protected copy constructor AnyGetterWriter(AnyGetterWriter src, NameTransformer transformer) with correct chaining: chainedTransformer(newTransformer, src._nameTransformer), matching the ordering in UnwrappingBeanPropertyWriter
  • Overrode rename(NameTransformer) to return a copy storing the transformer rather than attempting to rename the property itself (meaningless for any-getter)
  • getAndSerialize() and getAndFilter() now dispatch to transformer-aware helpers when _nameTransformer != null, applying it to each map/ObjectNode key individually

UnwrappedWithAnySetterTest

  • Added testUnwrappedWithPrefixWithAnyGetter covering the bug scenario: outer direct field serializes without prefix; inner any-getter entries get the "a-" prefix applied per key

@pjfanning pjfanning changed the title possible fix for 6118 Fix @JsonAnyGetter keys not prefixed when inner bean uses @JsonUnwrapped(prefix=...) Jul 25, 2026
@pjfanning

pjfanning commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

There is still an issue with deserialization of maps when unwrapped with prefix is used. The deserialization of the map doesn't remove the prefix. I want to treat that as a separate issue and can work out a fix for that if and when this serialization fix is merged.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.81% 📉 -0.030%
Branches branches 75.39% 📉 -0.040%

Coverage data generated from JaCoCo test results

@pjfanning

Copy link
Copy Markdown
Member Author

@cowtowncoder I fixed the deserialization issue with AI assistance. The issue I described above (#6123 (comment))

* can be reverse-transformed before being passed to
* {@code @JsonAnySetter}.
*
* @since 3.3

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have put version 3.3 on all the since markers but can change them to 3.1 or 3.2 if this is deemed safe for backporting

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.81% 📉 -0.030%
Branches branches 75.39% 📉 -0.040%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.82% 📉 -0.030%
Branches branches 75.40% 📉 -0.060%

Coverage data generated from JaCoCo test results

@cowtowncoder

cowtowncoder commented Jul 29, 2026

Copy link
Copy Markdown
Member

Ok. AI review pointed out a seemingly serious issue:


Correctness issue — confirmed regression (high severity)

AnyGetterWriter._serializeMapEntriesWithTransformer() (the new fallback used whenever _nameTransformer != null) completely bypasses _mapSerializer for real java.util.Map values, replacing it with a hand-rolled loop: rawKey.toString() for keys and ctxt.writeValue(gen, entryValue) for values. This silently drops everything MapSerializer normally provides:

  • Content inclusion filtering (@JsonInclude(NON_NULL)/NON_EMPTY etc. on the any-getter map) — I reproduced this concretely: with @JsonInclude(NON_NULL) on the extra map, a null-valued entry that's correctly suppressed when the bean is not unwrapped is emitted ("a-nickname":null) once the same bean is referenced via @JsonUnwrapped(prefix="a-").
  • Custom key serializers (_keySerializer, e.g. @JsonSerialize(keyUsing=...), non-String keys with custom formatting) — silently downgraded to Object.toString().
  • Whatever else MapSerializer.serializeFilteredAnyProperties()/serializeWithoutTypeInfo() do beyond the naive loop (polymorphic value typing, etc.).

This means: for any bean that combines @JsonAnyGetter + @JsonUnwrapped(prefix/suffix=...), Map serialization semantics silently regress the moment a name-transform is in play — trading one bug (missing prefix) for a different, arguably worse one (silently wrong output for filtered/keyed maps). A better approach would push the key transformation into the existing MapSerializer path (e.g., wrap/rename the key after _keySerializer runs, or pass the transformer into serializeWithoutTypeInfo/serializeFilteredAnyProperties) rather than routing around it.


I'll see if that can be fixed; that cannot be merged as-is.

@pjfanning

Copy link
Copy Markdown
Member Author

@cowtowncoder I got AI to sort some of the issues but there might be more.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.81% 📉 -0.040%
Branches branches 75.38% 📉 -0.080%

Coverage data generated from JaCoCo test results

@cowtowncoder

Copy link
Copy Markdown
Member

@pjfanning Hoping to pick this up soon, maybe tomorrow.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.88% 📉 -0.040%
Branches branches 75.48% 📉 -0.080%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.87% 📉 -0.040%
Branches branches 75.47% 📉 -0.060%

Coverage data generated from JaCoCo test results

@pjfanning
pjfanning force-pushed the copilot/unwrapped-test branch from 3381453 to 06834b4 Compare August 21, 2026 19:43
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.91% 📉 -0.010%
Branches branches 75.53% 📉 -0.020%

Coverage data generated from JaCoCo test results

@pjfanning

Copy link
Copy Markdown
Member Author

I got Claude Opus to write extra tests and it has 7 that fail and they are annotated so they don't run.

Test coverage for @JsonUnwrapped(prefix) + @JsonAnyGetter/@JsonAnySetter

Added 24 tests across two files. All pass; the 7 in tofix/ pass because they fail — each is a real
edge case where the PR (#6123) breaks down.

What works — struct/UnwrappedWithAnyGetterPrefixTest.java (17 tests)

  • prefix / suffix / prefix+suffix round-trips
  • plain @JsonUnwrapped (NOP transformer) unchanged
  • keys that already contain the prefix (a-agea-a-agea-age)
  • empty any-getter map writes nothing
  • any-getter/any-setter declared on a field
  • ObjectNode-valued any-getter ([databind#3604]) — nested node keys correctly left alone
  • enum map keys
  • custom (non-MapSerializer) any-getter serializer via @JsonSerialize(using=...)
  • property-based creator on the unwrapped bean
  • @JsonIdentityInfo outer, polymorphic (@JsonTypeInfo) outer
  • nested @JsonUnwrapped prefixes chaining on write (a-b-age)
  • two prefixed unwrapped beans on write
  • handler-cache safety: same inner type used with different prefixes and standalone

What doesn't — tofix/UnwrappedAnyGetterPrefixTest.java (7 tests)

# Failure Cause
1 {"a-obj":{"a-x":1,"a-y":2}} — nested Map/Array keys prefixed NameTransformingGenerator intercepts every writeName, at any depth, not just the any-getter's own keys
2 {"a-pt":{"a-x":1,"a-y":2}} — nested POJO properties prefixed same
3 {"a-pet":{"a-@type":"dog",…}} — polymorphic type id prefixed same
4 {"3":"x"}Integer/Long keys get no prefix those go through JsonGenerator.writePropertyId(long), which the delegate doesn't override (String/enum/byte[] keys use writeName and do work)
5 Builder-based inner: any-setter receives a-age BuilderBasedDeserializer.unwrappingDeserializer() doesn't retain the NameTransformer the way BeanDeserializer now does
6 Two prefixed unwrapped beans: each any-setter collects the other's props (a.ea = {p=1, b-q=2}) when reverse() returns null the raw name is passed through instead of being skipped — writes are correct, so the value doesn't round-trip
7 Nested @JsonUnwrapped prefixes: a-b-age silently dropped on read pre-existing, but write now emits a-b-age, so the new serialization side has no working read side

Provenance: items 1–4 are new with this branch — on the base commit (30e603d3f) this whole scenario
threw IllegalStateException: Method must be overridden by class AnyGetterWriter. Items 5 and 6 are
unchanged-but-wrong. Item 7 predates the branch (verified in a worktree at 30e603d3f).
Items 1–3 were checked against a non-unwrapped baseline, which correctly produces {"obj":{"x":1}}.

Suggested fixes

  1. 1–3 (the serious one) — a generator wrapper can't tell depth apart. Either make
    NameTransformingGenerator transform only at the depth it was installed (track
    writeStartObject/writeEndObject nesting, transform only when depth == 0), or drop the wrapper
    and transform keys explicitly in a MapSerializer entry loop.
  2. 4 — override writePropertyId(long) in NameTransformingGenerator:
    delegate.writeName(_transformer.transform(String.valueOf(id))).
  3. 5 — mirror the BeanDeserializer change in BuilderBasedDeserializer.
  4. 6 — skip the property (rather than passing the raw name) when reverse() returns null.
    Safe, since the no-prefix case uses NameTransformer.NOP, whose reverse() never returns null.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.89% 📉 -0.030%
Branches branches 75.53% 📉 -0.020%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.90% 📉 -0.020%
Branches branches 75.56% 📈 +0.010%

Coverage data generated from JaCoCo test results

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

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.90% 📉 -0.010%
Branches branches 75.56% 📈 +0.000%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.90% 📉 -0.010%
Branches branches 75.55% 📉 -0.010%

Coverage data generated from JaCoCo test results

@gitar-bot

gitar-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Fixes @JsonAnyGetter map key prefixing under @JsonUnwrapped(prefix=...) by updating AnyGetterWriter to support NameTransformer, addressing the name transforming and double reverse computation findings. No issues found.

✅ 2 resolved
Bug: NameTransformingGenerator prefixes nested object keys too

📄 src/main/java/tools/jackson/databind/ser/NameTransformingGenerator.java:26-36 📄 src/main/java/tools/jackson/databind/ser/AnyGetterWriter.java:128-131 📄 src/main/java/tools/jackson/databind/ser/AnyGetterWriter.java:171-173
NameTransformingGenerator overrides writeName unconditionally, so every field name written to the wrapped generator gets the prefix/suffix — not just the any-getter map's top-level keys. Because MapSerializer serializes each map value on the same generator (MapSerializer.java:645-656), any-getter map values that are themselves POJOs or nested Maps get their inner keys transformed as well. For example an entry {"info": {"city": "x"}} under @JsonUnwrapped(prefix="a-") produces {"a-info": {"a-city": "x"}} instead of {"a-info": {"city": "x"}}, diverging from how @JsonUnwrapped prefixing works for ordinary bean properties (only the direct level is prefixed). The added tests only use scalar map values, so this is uncovered. Track nesting depth and only transform at the top level.

Performance: reverse() computed twice per accepted any-setter property

📄 src/main/java/tools/jackson/databind/deser/bean/BeanDeserializerBase.java:2009-2023
For each unknown property that reaches the any-setter on an unwrapped-with-prefix bean, _unwrappingNameTransformer.reverse(propName) is evaluated once in _anySetterAccepts (to decide acceptance) and again in _anySetterKey (to compute the stripped key). This is redundant work on the deserialization path, though negligible for typical inputs. If desired, _anySetterAccepts could return the reversed key (or callers could reuse a single computed value) to avoid the double transformation.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

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

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

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

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.

3 participants