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

Provide a callback mechanism for customizing LocalValidatorFactoryBean's configuration #29429

Closed
wants to merge 5 commits into from

Conversation

dangzhicairang
Copy link
Contributor

Add a AddValueExtractorsLocalValidatorFactoryBean that expand LocalValidatorFactoryBean which based SpringFramework, it can support user add custom ValueExtractor(s). For example, user can define ValueExtractor like this in their application:

@Component
public class CustomValueExtractor implements ValueExtractor<CustomResult<@ExtractedValue ?>> {
 
     @Override
     public void extractValues(CustomResult<?> result, ValueReceiver valueReceiver) {
         valueReceiver.value(null, result.getData());
     }
}

And modify the class ValidationAutoConfiguration, auto-configure an instance of type AddValueExtractorsLocalValidatorFactoryBean instead of type LocalValidatorFactoryBean

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 16, 2022
@wilkinsona
Copy link
Member

Thanks for the proposal, @dangzhicairang. Do you think it's likely that you will want to inject dependencies into a customValueExtractor implementation? If so, there would be a benefit to them being beans, but if not it may be better to provide a more general callback for customizing the validator Configuration.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jan 18, 2022
@dangzhicairang
Copy link
Contributor Author

Thanks for the proposal, @dangzhicairang. Do you think it's likely that you will want to inject dependencies into a customValueExtractor implementation? If so, there would be a benefit to them being beans, but if not it may be better to provide a more general callback for customizing the validator Configuration.

Thank you @wilkinsona , it is a really nice suggestion, and i provide a callback class Customizer for the Configuration. And i retained the ability to automatically add the ValueExtractor(s) bean, so user can choose whether these ValueExtractor(s) are bean or not. The ValueExtractor which is bean will automatically add but added by Customizer or AddValueExtractorCustomizer if it not.

@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 labels Jan 18, 2022
@wilkinsona wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress and removed status: feedback-provided Feedback has been provided labels Jan 19, 2022
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 19, 2022
@wilkinsona
Copy link
Member

Thanks for the updates, @dangzhicairang. Our feeling is that the general-purpose callback mechanism possibly belongs in Spring Framework rather than in Spring Boot, particularly as Framework already has its own LocalValidatorFactoryBean sub-class. I've raised spring-projects/spring-framework#27956 so that the Framework team can consider things. I'll place this proposal on hold until we know what, if anything, the Framework team are going to do.

@wilkinsona wilkinsona added the status: on-hold We can't start working on this issue yet label Jan 19, 2022
@dangzhicairang
Copy link
Contributor Author

Thanks for the updates, @dangzhicairang. Our feeling is that the general-purpose callback mechanism possibly belongs in Spring Framework rather than in Spring Boot, particularly as Framework already has its own LocalValidatorFactoryBean sub-class. I've raised spring-projects/spring-framework#27956 so that the Framework team can consider things. I'll place this proposal on hold until we know what, if anything, the Framework team are going to do.

Thank you for your reply @wilkinsona . It is right.

@snicoll
Copy link
Member

snicoll commented Apr 14, 2022

spring-projects/spring-framework#27956 has been addressed.

@snicoll snicoll removed the status: on-hold We can't start working on this issue yet label Apr 14, 2022
@wilkinsona wilkinsona added this to the 3.0.x milestone May 4, 2022
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels May 4, 2022
@wilkinsona wilkinsona changed the title Expand LocalValidatorFactoryBean to support custom ValueExtractor(s) Provide a callback mechanism for customizing LocalValidatorFactoryBean's configuration May 4, 2022
@wilkinsona wilkinsona closed this in f12dcb5 May 4, 2022
@wilkinsona
Copy link
Member

Thanks very much for making your first contribution to Spring Boot, @dangzhicairang.

I have updated your proposal a little in 0e00faf to make use of the changes to Framework's LocalValidatorFactoryBean. In the interests of simplicity, I've removed support for ValueExtractor beans, preferring a more general callback mechanism that can be used to customise the configuration as needed, including adding value extractors.

@wilkinsona wilkinsona removed this from the 3.0.x milestone May 4, 2022
@wilkinsona wilkinsona added this to the 3.0.0-M3 milestone May 4, 2022
@dangzhicairang
Copy link
Contributor Author

Thanks very much for making your first contribution to Spring Boot, @dangzhicairang.

I have updated your proposal a little in 0e00faf to make use of the changes to Framework's LocalValidatorFactoryBean. In the interests of simplicity, I've removed support for ValueExtractor beans, preferring a more general callback mechanism that can be used to customise the configuration as needed, including adding value extractors.

Thanks a lot for your update @wilkinsona

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants