Skip to content

DataClassRowMapper should not override Kotlin init properties#26569

Closed
dennie170 wants to merge 1 commit intospring-projects:mainfrom
dennie170:master
Closed

DataClassRowMapper should not override Kotlin init properties#26569
dennie170 wants to merge 1 commit intospring-projects:mainfrom
dennie170:master

Conversation

@dennie170
Copy link
Copy Markdown

@dennie170 dennie170 commented Feb 18, 2021

Since Kotlin data classes are already constructed with values we don't want to override all the properties later on again.
Properties modified in the init { } block in a class would be reverted back to the value of the column again.

An example of this would be setting the default scale of a BigDecimal inside an init block. The init would be called and set the scale after constructing. However, after constructing the BeanPropertyRowMapper would override the BigDecimal again so the scale would be 0 again.

This commit fixes this issue for Kotlin data classes.

@dennie170
Fixed bug that would override Kotlin init properties
0e3dcca 
Since Kotlin data classes are already constructed with values we
don't want to override all the properties later on again.
Properties modified in the init { } block in a class would be reverted
back to the value of the column again.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 18, 2021
@dennie170
Copy link
Copy Markdown
Author

dennie170 commented Jun 28, 2021 •
edited by snicoll
Loading

Hi team, I see nobody has replies to my issue since February.
Are you gonna have a look at this? It is pretty "breaking" in Kotlin, and now demands some custom solutions.
Thanks!

@sbrannen sbrannen requested a review from jhoeller June 28, 2021 16:15
@jhoeller jhoeller self-assigned this Jul 6, 2021
@jhoeller jhoeller added in: data Issues in data modules (jdbc, orm, oxm, tx) type: bug A general bug labels Jul 6, 2021
@jhoeller jhoeller modified the milestones: 5.3.10, 5.3.9 Jul 6, 2021
@jhoeller jhoeller removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 8, 2021
@jhoeller jhoeller changed the title Fixed bug that would override Kotlin init properties DataClassRowMapper should not override Kotlin init properties Jul 8, 2021
@jhoeller jhoeller closed this in 4fe3ca1 Jul 9, 2021
@jhoeller
Copy link
Copy Markdown
Contributor

jhoeller commented Jul 9, 2021

It turns out that Kotlin var declarations implicitly generate setter methods for the affected properties, which leads to our DataClassRowMapper populating that setter as well (since we're expecting either a constructor argument or a setter method, not both, we haven't been checking for this explicitly). Mixing and matching between constructor arguments and setter methods is allowed by design, so we can't cut off the latter completely (like the original PR does). Instead, I've revised the DataClassRowMapper algorithm to suppress setter method calls for all properties that have been populated through constructor arguments already. In addition to a Java-based test for this behavior, I've also added your Kotlin test case.

Thanks for the PR, in any case!

@dennie170
Copy link
Copy Markdown
Author

dennie170 commented Jul 12, 2021

Thanks for figuring it out and the clear explanation! Glad to see it's fixed now.

@sync-by-unito sync-by-unito Bot mentioned this pull request Jul 14, 2021
Merged
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
@jhoeller @lxbzmy
DataClassRowMapper suppresses setter method calls for constructor-bou…
c352b71 
…nd properties

Closes spring-projectsgh-26569
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhoeller jhoeller Awaiting requested review from jhoeller

Assignees

@jhoeller jhoeller

Labels

in: data Issues in data modules (jdbc, orm, oxm, tx) type: bug A general bug

Projects

None yet

Milestone

5.3.9

Development

Successfully merging this pull request may close these issues.

3 participants

@dennie170 @jhoeller @spring-projects-issues