Skip to content

Ensure DataBinder once again binds null values for nullable/optional constructor arguments #31800

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Ensure DataBinder once again binds null values for nullable/optional constructor arguments #31800

wants to merge 1 commit into from

Conversation

HeartPattern
Copy link
Contributor

After ea398d7, DataBinder failed to bind nullable value through constructor.

We should check if param is nullable and do not trying to resolve nested constructor.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 9, 2023
@HeartPattern HeartPattern mentioned this pull request Dec 9, 2023
Closed
@sbrannen sbrannen requested a review from rstoyanchev December 10, 2023 18:03
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression and removed in: web Issues in web modules (web, webmvc, webflux, websocket) status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 10, 2023
@sbrannen sbrannen added this to the 6.1.2 milestone Dec 11, 2023
@sbrannen sbrannen changed the title fix DataBinder failed to bind nullable class value through constructor Ensure DataBinder once again binds null values for nullable/optional constructor arguments Dec 11, 2023
rstoyanchev
rstoyanchev reviewed Dec 12, 2023
View reviewed changes
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

We cannot always exclude optional arguments only because they are optional, or we would never be able to inject a successfully constructed nested argument.

The question I have is how was this working before in the success case? Such a nested class would not have been instantiated. Could you provide more details?

@bclozel bclozel modified the milestones: 6.1.2, 6.1.x Dec 12, 2023
@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on and removed type: regression A bug that is also a regression labels Dec 12, 2023
@snicoll snicoll removed this from the 6.1.x milestone Dec 12, 2023
@rstoyanchev rstoyanchev mentioned this pull request Dec 12, 2023
Closed
@rstoyanchev
Copy link
Contributor

See also #31821, which seems to be the same case. If you could, please clarify either way.

@rstoyanchev
Copy link
Contributor

Thanks for the report and a PR. I am preparing a fix for this under #31821.

@rstoyanchev rstoyanchev added status: superseded An issue that has been superseded by another and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rstoyanchev rstoyanchev rstoyanchev left review comments

Assignees
No one assigned
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@HeartPattern @rstoyanchev @bclozel @sbrannen @snicoll @spring-projects-issues