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 support for post-processing a LocalValidatorFactoryBean's validator Configuration without requiring sub-classing #27956

Closed
wilkinsona opened this issue Jan 19, 2022 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

Affects: 5.3

LocalValidatorFactoryBean currently allows the validator Configuration to be post-processed but this requires a custom sub-class and an override of postProcessConfiguration. Given that Framework itself already provides a sub-class (OptionalValidatorFactoryBean) this is a little awkward as you have to know which class to sub-class or maybe sub-class both depending on circumstances.

A Spring Boot user has proposed an enhancement to make it easier to register ValueExtractors with the Configuration. We'd like to do this via composition with some sort of ConfigurationPostProcessor or ConfigurationCustomizer callback. We could create our own LocalValidatorFactoryBean sub-class that enables this customization but that functionality would then be lost by anyone who's using OptionalValidatorFactoryBean instead.

We wondered if Framework could add a callback-based mechanism for post-processing the Configuration instead. We could then auto-configure the LocalValidatorFactoryBean with any post-processors/customizers found in the context and users could implement one to register their ValueExtractor without resorting to inheritance.

@mdeinum
Copy link
Contributor

mdeinum commented Jan 20, 2022

I've seen more and more different customizers pop up in Spring and Spring Boot code. Things like the CacheManagerCustomizer, CodecCustomizer, and EntityManagerFactoryBuilderCustomizer. Each serving the purpose of customizing a specific type of bean. Each of them is also invoked in similar ways (some ordered, some not, and there are some slight differences).

But isn't this also the task of a BeanPostProcessor, and isn't this a specialized version of a BeanPostProcessor with an implementation of the BeanPostProcessor.postProcessBeforeInitialization for a specific type of bean? Or maybe Spring is in need of an additional way of customizing beans. Now it looks like that for each type of bean that requires customization the wheel is (somewhat) reinvented or at least copy/pasted with sometimes some minor differences (or those differences aren't known).

I do get that the BeanPostProcessor doesn't apply to this specific case as it is an internally created instance instead of a managed bean, but it does apply to certain other cases.

@sbrannen sbrannen added in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) and removed in: data Issues in data modules (jdbc, orm, oxm, tx) labels Jan 20, 2022
@quaff
Copy link
Contributor

quaff commented Jan 21, 2022

BeanPostProcessor cannot change state of immutable bean, Customizer is applied to their Builder.

@mdeinum
Copy link
Contributor

mdeinum commented Jan 21, 2022

Not in all cases, the EntityManagerFactoryBuilder has a customizer which could also be done by a BeanPostProcessor, the same applies to the CacheManagerCustomizer. As I stated in my comment it won't work for the internal beans here, but for those beans that are already exposed as beans.

I pointed out that there seems to be an emerging pattern for customizing beans outside of the regular BeanPostProcessor which might be better suited for a general solution instead of reinventing/implementing the wheel each time. It has been duplicated in Spring Boot several times, and in other projects as well.

@jhoeller jhoeller removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 21, 2022
@jhoeller jhoeller added this to the 5.3.18 milestone Mar 21, 2022
@jhoeller jhoeller self-assigned this Mar 21, 2022
@snicoll snicoll modified the milestones: 5.3.18, 5.3.19 Mar 31, 2022
@jhoeller
Copy link
Contributor

jhoeller commented Apr 4, 2022

@wilkinsona would it be sufficient to let postProcessConfiguration delegate to a setter-specified Consumer<Configuration> instance - or possibly a custom ConfigurationCustomizer type, or a vararg of such a type? I assume you would build your own detection mechanism on top and then just pass such customizers, or an adapter around Boot's customizer bean type, to that new LocalValidatorFactoryBean setter method?

The main question for me is the use of a special customizer type. Would you want to reuse it in Boot? If you'll be doing a custom customizer type in any case, we could also simply go with Consumer<Configuration> at the core level so that there's no conflict of customizer types on the common Boot classpath.

@wilkinsona
Copy link
Member Author

would it be sufficient to let postProcessConfiguration delegate to a setter-specified Consumer instance - or possibly a custom ConfigurationCustomizer type, or a vararg of such a type?

It would.

The main question for me is the use of a special customizer type. Would you want to reuse it in Boot? If you'll be doing a custom customizer type in any case, we could also simply go with Consumer at the core level so that there's no conflict of customizer types on the common Boot classpath

If Framework provided a ConfigurationCustomizer interface, I can't see any reason not to reuse it in Boot. We'd look for ConfigurationCustomizer beans in the context and set them (or a composite of them if only single customizer is supported) on the LocalValidatorFactoryBean that we auto-configure.

we could also simply go with Consumer at the core level

If it were a plain Consumer (after type erasure), we'd define our own interface and adapt it to Consumer<Configuration>.

In short, both would work pretty much equally well for Boot. I should also say that I don't consider this change to be urgent. It would be nice to deliver something in Boot 2.x, but it's far from essential. We could live with a change in Framework 6 and Boot 3 if that aligns better with Framework's goals/priorities.

@jhoeller
Copy link
Contributor

jhoeller commented Apr 8, 2022

I went with a setConfigurationInitializer(Consumer<Configuration<?>>) method, along the lines of setEntityManagerInitializer that we got in AbstractEntityManagerFactoryBean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants