Skip to content

Fix #5238 #5239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: 2.19
Choose a base branch
from
Open

Fix #5238 #5239

wants to merge 7 commits into from

Conversation

JooHyukKim
Copy link
Member

No description provided.

@JooHyukKim JooHyukKim changed the base branch from 2.x to 2.19 July 31, 2025 15:03
@cowtowncoder cowtowncoder marked this pull request as draft August 1, 2025 03:50
@JooHyukKim JooHyukKim marked this pull request as ready for review August 2, 2025 14:08
@JooHyukKim JooHyukKim changed the title Reproduce #5238 Fix #5238 Aug 2, 2025
public final String name;

@JsonCreator
public ThingPojo(@JsonProperty("id") int id, @JsonProperty("name") String name) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this test should make sure id is only set via Constructor and NOT direct set on Field.
(JVM does allow forced settting of final Fields).

One way would be to make constructor change "id" value by prefix or suffix.

I know the end result does not look different, but it would explain discrepancy.

@@ -335,7 +336,12 @@ public Object handleIdValue(final DeserializationContext ctxt, Object bean) thro
// also: may need to set a property value as well
SettableBeanProperty idProp = _objectIdReader.idProperty;
if (idProp != null) {
return idProp.setAndReturn(bean, _idValue);
// [databind#5328] Records do not have setters, skip...to set id value
if (ClassUtil.isRecordType(bean.getClass())) {
Copy link
Member

Choose a reason for hiding this comment

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

I'll have to think about this: ideally, setAndReturn() would not be called because id was passed via Constructor and no check was needed here.

Or maybe check could be done that if idProp is a CreatorProperty, no call would be made?

This is related to ensuring that we also do not try forcibly setting Field in regular POJO case, if id was already passed via Creator (see my note on test class).

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.

2 participants