Skip to content

fix: skip field processing when nested property has same name - #2693

Open
MikeEdgar wants to merge 1 commit into
smallrye:mainfrom
MikeEdgar:issue-2687
Open

MikeEdgar wants to merge 1 commit into
smallrye:mainfrom
MikeEdgar:issue-2687

Conversation

@MikeEdgar

Copy link
Copy Markdown
Member

Fixes #2687

@MikeEdgar MikeEdgar added this to the 4.4.0 milestone Oct 2, 2026
Signed-off-by: Michael Edgar <michael@xlate.io>
@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

@phillip-kruger phillip-kruger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I built the branch and ran the core tests (410 run, 0 failures). I also compared the output against main for a few variations:

Scenario main PR
Record from the issue: record Colliding(@JsonUnwrapped Inner foo) only bar bar, foo ✅
Bean with private @JsonUnwrapped PInner foo and public getFoo()/setFoo() only bar bar, foo ✅
@JsonUnwrapped(prefix = "x") PInner xfoo (collision after the prefix) only xbar xbar, xfoo ✅
@JsonUnwrapped @Schema(description = "outer") PInner foo only bar bar, foo ✅

I first suspected the outer accessor (foo()/getFoo() returning Inner) would attach to the inner foo resolver now that it's no longer ignored. updateTypeResolvers only stores the method when the types match, though, so it's fine.

Some minor, non-blocking comments:

  1. Edge case (FYI). If the outer class already has a property serialized as foo, for example @JsonProperty("foo") Integer other alongside @JsonUnwrapped PInner foo, the result is now foo: {type: [integer, string], format: int32}. main gave just integer, though it dropped the unwrapped property. Jackson would write a duplicate key here anyway, so this is a user error, but the merged schema is a bit odd. It could be handled in a follow-up, or maybe a warning.
  2. Comment wording. "Since the field will be ignored" could say more about why the early return matters, e.g. "...returning here avoids marking the unwrapped property as ignored."
  3. Javadoc placement in the test. The /** Issue ... */ block sits between the annotations and the method, so it isn't treated as Javadoc. Move it above @Test or make it a line comment.
  4. Test coverage. The issue's reproducer is a record, which can't be written in core's Java 11 tests. A record case in testsuite/data (next to RecordSchemaTest) and a prefix-collision case in core would round out the coverage.

This branch has not been deployed

No deployments
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.

@JsonUnwrapped handling removes field with same field name from produced schema

2 participants