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

Allow filters to be registered with MockMvc for particular dispatcher types #27717

Closed
wilkinsona opened this issue Nov 22, 2021 · 6 comments
Closed
Assignees
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

In Spring Boot 2.6, we've introduced a new filter that's only registered for ERROR dispatches. This works fine at runtime and in integration tests over HTTP, but doesn't work as intended when testing with MockMvc. I overlooked the fact that MockMvc's filter chain includes every known filter, irrespective of the request's dispatcher type. When adding a filter, it would be useful to be able to specify the filter's dispatcher types, as you can do with the Servlet API. In the meantime, we can fix Boot by updating the filter to ignore non-ERROR requests.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 22, 2021
@jhoeller jhoeller added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 22, 2021
@jhoeller jhoeller added this to the 6.0.x milestone Nov 22, 2021
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Nov 23, 2021

On AbstractMockMvcBuilder we have:

  • addFilters(Filter... filters)
  • addFilter(Filter filter, String... urlPatterns)

We could also have:

  • addFilter(Filter filter, DispatcherType dispatcherType, String... urlPatterns)

Is this what you had in mind as well?

@wilkinsona
Copy link
Member Author

To align with the Servlet API on FilterRegistration, I think EnumSet<DispatcherType> would be better than DispatcherType.

@viktorardelean
Copy link

viktorardelean commented Dec 10, 2021

@wilkinsona I would like to implement this enhancement.

From what I understood, I need to add addFilter(Filter filter, DispatcherType dispatcherType, String... urlPatterns) to AbstractMockMvcBuilder and also declare it in the ConfigurableMockMvcBuilder interface.

Inside the new addFilter, I will check if there is any DispatcherType.ERROR in the set. If there is, I will not add the filter to the filters list.

Here is how it will look like:

@Override
public final  <T extends B> T addFilter(Filter filter, EnumSet<DispatcherType> dispatcherType, String... urlPatterns { 
    Assert.notNull(filter, "filter cannot be null");
    Assert.notNull(urlPatterns, "urlPatterns cannot be null");
    Assert.notNull(dispatcherType, "dispatcherType cannot be null");
    if (dispatcherType.contains(DispatcherType.ERROR)) {
        return self();
    }
    if (urlPatterns.length > 0) {
        filter = new PatternMappingFilterProxy(filter, urlPatterns);
    }
    this.filters.add(filter);
    return self();
}

@wilkinsona
Copy link
Member Author

Thanks, but I am not a member of the Spring Framework team. If you are interested in contributing, @rstoyanchev would be a better person to discuss things with.

@viktorardelean
Copy link

@rstoyanchev Could you kindly give your feedback on my comment? Then I can implement it and raise a PR.
Thanks!

@viktorardelean
Copy link

@rstoyanchev Could you please give an update on this? I want to make sure that my approach on this is correct. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants