Accept empty filter dispatcher types in auto-configurations #22138
Conversation
Prior to this commit, the usage of EnumSet.copyOf resulted in exceptions when the underlying collection was empty. Closes spring-projectsgh-22128
spring-projects-issues
added
the
status: waiting-for-triage
There was a problem hiding this comment.
Thanks very much, @dreis2211. The approach looks good to me. I left a couple of comments about possibly using AssertJ’s extracting. Please let us know if you agree. If you do, we can tweak it when we merge or you could update the PR if you prefer.
| DelegatingFilterProxyRegistrationBean bean = context.getBean("securityFilterChainRegistration", | ||
| DelegatingFilterProxyRegistrationBean.class); | ||
| @SuppressWarnings("unchecked") | ||
| EnumSet<DispatcherType> dispatcherTypes = (EnumSet<DispatcherType>) ReflectionTestUtils |
There was a problem hiding this comment.
I think we could use AssertJ’s extracting here and we prefer to do so where possible.
| this.contextRunner.withUserConfiguration(SessionRepositoryConfiguration.class) | ||
| .withPropertyValues("spring.session.servlet.filter-dispatcher-types=").run((context) -> { | ||
| FilterRegistrationBean<?> registration = context.getBean(FilterRegistrationBean.class); | ||
| Object dispatcherTypes = ReflectionTestUtils.getField(registration, "dispatcherTypes"); |
There was a problem hiding this comment.
I think this could use AssertJ’s extracting too.
wilkinsona
added
for: merge-with-amendments
|
I was undecided on that one actually. I wanted to stay consistent inside the specific tests, but I can change it of course. |
wilkinsona
removed
the
for: merge-with-amendments
|
The dangers of reviewing a diff in isolation. Ok, let's keep it consistent within the test classes for now. We can do a separate pass that replaces the use of |
|
Yeah, I think doing this in a separate pass is probably better. On top of that I think that neither |
Prior to this commit, the usage of EnumSet.copyOf resulted in exceptions when the underlying collection was empty. See gh-22138
|
Thanks very much once again, @dreis2211. |
Hi,
this PR fixes #22128 by using
Collectors.toCollection(() -> EnumSet.noneOf(DispatcherType.class))rather thanCollectors.collectingAndThen(Collectors.toSet(), EnumSet::copyOf)to prevent exceptions when the underlying collection is empty.
Let me know what you think.
Cheers,
Christoph