Introduce @ConditionalOnMissingServletFilter #7654
Conversation
| } | ||
| }); | ||
| } | ||
| catch (ClassNotFoundException ex) { |
There was a problem hiding this comment.
This should probably require a guard for NoClassDefFoundError as well.
| BeanTypeRegistry beanTypeRegistry = BeanTypeRegistry.get(context.getBeanFactory()); | ||
| try { | ||
| Map<String, FilterRegistrationBean> filterRegistrations = context | ||
| .getBeanFactory().getBeansOfType(FilterRegistrationBean.class); |
There was a problem hiding this comment.
Isn't there other ways to register a Filter?
| } | ||
| if (servletFilters.isEmpty() && | ||
| metadata instanceof MethodMetadata && metadata.isAnnotated(Bean.class.getName())) { | ||
| addDeducedBeanTypeForBeanMethod(context, metadata, servletFilters, (MethodMetadata) metadata); |
There was a problem hiding this comment.
There is only a single use where the value is omitted. Perhaps it's not a frequent use case? If it isn't, maybe it would be nicer to just not support it and remove all the code that deduces the type?
This commit introduces a new Conditional annotation `@ConditionalOnMissingServletFilter` which checks for the absence of Servlet filters as beans or `FilterRegistrationBean`s. This Conditional annotation can be used to guard Servlet Filter bean definitions like: ``` @bean @ConditionalOnMissingServletFilter(MyServletFilter.class) public MyServletFilter myServletFilter() { //... } ``` Fixes spring-projectsgh-7475
This commit registers all `ResourceUrlEncodingFilter` instances as `FilterRegistrationBean`s instead of regular beans, in order to register those for all Servlet dispatcher types (and make them available to error dispatches). To avoid duplicate registration of those filters, this commit is relying on the `@ConditionalOnMissingServletFilter` annotation that checks for both beans and filter registration beans. Fixes spring-projectsgh-7348
|
After a first round of review and discussion with @snicoll, here are a few remaining questions before we can introduce this new Condition:
@wilkinsona, @philwebb - I'd really like to get your opinion on those questions. Thanks! |
|
|
It's quite tricky. In hindsight, we should have probably made I wonder if we can fix this a different way by registering a variant of |
|
Perhaps we could avoid the problem by making the auto-configuration of both the filter and its registration bean conditional on their being no existing filter bean. That would allow the user to override our auto-configuration either by providing their own filter bean (and using the registration bean's defaults) or by providing their own filter bean and a registration bean. The generic enhancement in 2.0 would then allow you to override an auto-configured filter by just providing a registration bean, i.e. you wouldn't need to expose the filter as a bean as well. |
|
I've taken a step back and looked at the original issue #7348; I don't think this is worth making the current (auto-)configurations more complex than they are and worse, risking failing application initializations. I've moved the dedicate issues to 2.0.0.M1 and created #7666 to make |
|
It looks like the need for this has passed. We can always resurrect this if that turns out not to be the case. |
These commits introduce the
@ConditionalOnMissingServletFilterand make use of it to register
ResourceUrlEncodingFilterinstances as
FilterRegistrationBeans instead of beans.This new conditional annotation is borrows a bit from the bean
conditions but also checks for
FilterRegistrationBeanand thetype of filter they're about to register.
This requires getting the actual bean, which is obviously less
efficient than the checks involved in bean conditions. I've
configured that condition to have a low precedence, but I'm
wondering if more checks/guards should be added here.
The second commit applies that new Condition to existing filters
and turns them into
FilterRegistrationBeanto finally achievethe original goal: configure
ResourceUrlEncodingFilters forall dispatcher types and make them available during error
dispatches, so as to properly render URLs in error pages.