Skip to content
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

ModelAttributeMethodArgumentResolver does not support custom field binding for immutable objects #28284

Closed
seabamirum opened this issue Apr 4, 2022 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@seabamirum
Copy link

The paramName value for the constructor field is available, but is not used during binding on line 256 when binder.convertIfNecessary(value, paramTypes[i], methodParam) is called. When propertyEditorRegistry.findCustomEditor(requiredType, propertyName) is finally called in the TypeConverterDelegate, it is with a null value for the propertyName field, so any custom field editors will not be found.

Affects: 5.3.16


@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 4, 2022
@sbrannen
Copy link
Member

sbrannen commented Apr 5, 2022

Hi @seabamirum,

Thanks for creating your first issue for the Spring Framework. 👍

Is the behavior you are describing something that worked prior to 5.3.16?

Also, please provide a simple application or test case that reproduces the behavior you are describing (for example, as a GitHub project or ZIP file attached to this issue that we can download and run locally).

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Apr 5, 2022
@jhoeller
Copy link
Contributor

jhoeller commented Apr 5, 2022

From a quick glance, it looks like we never supported this, taking the constructor parameter names into account for value retrieval but not for type conversion purposes. So only setter-derived property names are being exposed for PropertyEditor resolution, not constructor parameter names. We can certainly try to redefine this for exposing constructor parameter names there as well.

@jhoeller jhoeller self-assigned this Apr 5, 2022
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 5, 2022
@jhoeller jhoeller added this to the 5.3.x milestone Apr 5, 2022
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Apr 12, 2022
@seabamirum
Copy link
Author

I'm sorry I could not make a test case, because there's no straightforward way to get an instance of WebDataBinder without a target object that is already instantiated.

However I think allowing custom property editors for constructor fields without setters would be a welcome improvement, if only for consistency in the framework. Right now it's not clear why it doesn't work, and I don't think it is documented.

In my case I wanted to use a custom PasswordEncodingPropertyEditor to immediately encode the password in Spring Security's User object on signup. The only workaround that I found was to make a dummy object for binding, and then call the User.builder() method in the controller with the bound fields of that object.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Apr 13, 2022
@jhoeller jhoeller modified the milestones: 5.3.x, 6.1.x Feb 1, 2023
@jhoeller jhoeller modified the milestones: 6.1.x, 6.1.0-M1 May 10, 2023
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
Supports name-bound PropertyEditor registrations on data classes.
Includes consistent support for field-aware method parameters.

Closes spring-projectsgh-28284
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants