Reuse MockHttpServletRequest from ServletTestExecutionListener in Spring MVC Test framework [SPR-13211]

[spring-projects-issues]

Sam Brannen opened SPR-13211 and commented

Status Quo

As discussed in the referenced question on Stack Overflow, the combination of loading a WebApplicationContext with the Spring TestContext Framework and using the Spring MVC Test Framework results in two instances of MockHttpServletRequest being created.

1. ServletTestExecutionListener.setUpRequestContextIfNecessary() creates a MockHttpServletRequest and makes it available to the WebApplicationContext via the RequestAttributes stored as a thread-local variable in the RequestContextHolder.
   • The mocked request is then injected into any session-scoped or request-scoped bean in the WebApplicationContext (following standard Spring semantics for scoped beans) or into the test instance.
2. MockMvc.perform() creates another MockHttpServletRequest (via MockHttpServletRequestBuilder.createServletRequest()).
   • This mocked request is the one that is eventually supplied to the TestDispatcherServlet in MockMvc.perform() (via the MockFilterChain).

The end result is that there are two mocked requests. Consequently, components that have the request injected into them (following scoped-proxy semantics) will interact with a request object that has nothing to do with the request configured via RequestBuilders (e.g., get(), post(), etc.) in Spring MVC Test.

NB: the above assumptions regarding interactions between mocked requests and session/request-scoped beans have not been fully confirmed, but in any case it is possible to have a MockHttpServletRequest injected into a test that is then not used by Spring MVC Test, which leads to confusing results. For example, request parameters configured in the mock request created by the ServletTestExecutionListener are not visible to controller methods invoked via Spring MVC Test.

---

Deliverables

TBD

---

Proposal

Initial analysis shows that modifying the createServletRequest() method in MockHttpServletRequestBuilder as follows could work. In fact, no side effects have been uncovered within Spring's test suite as a result of this change.

/**
 * Create a {@link MockHttpServletRequest}.
 * <p>If an instance of {@code MockHttpServletRequest} is available via
 * the {@link RequestAttributes} bound to the current thread in
 * {@link RequestContextHolder}, this method simply returns that instance.
 * <p>Otherwise, this method creates a new {@code MockHttpServletRequest}
 * based on the supplied {@link ServletContext}.
 * <p>Can be overridden in subclasses.
 * @see RequestContextHolder#getRequestAttributes()
 * @see ServletRequestAttributes
 */
protected MockHttpServletRequest createServletRequest(ServletContext servletContext) {
	RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
	if (requestAttributes instanceof ServletRequestAttributes) {
		HttpServletRequest request = ((ServletRequestAttributes) requestAttributes).getRequest();
		if (request instanceof MockHttpServletRequest) {
			return (MockHttpServletRequest) request;
		}
	}

	return new MockHttpServletRequest(servletContext);
}

However, special care should be taken with regard to MockMultipartHttpServletRequestBuilder.createServletRequest() since it creates a MockMultipartHttpServletRequest instead of a MockHttpServletRequest. Furthermore, potential side effects for third-party add-ons (e.g., the upcoming support for HtmlUnit in Spring MVC Test (#17749)) must be taken into consideration as well.

---

Affects: 3.2 GA

Reference URL: http://stackoverflow.com/questions/30757044/autowired-httpservletrequest-in-spring-test-integration-tests

Issue Links:

• Populate RequestAttributes before invoking FilterChain in MockMvc [SPR-13217] #17808 Populate RequestAttributes before invoking FilterChain in MockMvc
• Add MockMvc HtmlUnit Integration [SPR-13158] #17749 Add MockMvc HtmlUnit Integration
• MockMvc.perform with the TestContext framework causes additional perform invocations to be "polluted" [SPR-13260] #17851 MockMvc.perform with the TestContext framework causes additional perform invocations to be "polluted"

Referenced from: commits 5e24ee9, 3c799e6, bf06bf3, 3c2efee

0 votes, 5 watchers

[spring-projects-issues]

Sam Brannen commented

I have pushed an initial proof of concept to my #17803 branch.

[spring-projects-issues]

Rossen Stoyanchev commented

It makes sense that since the TestContext framework support comes first and already sets up a mock request and response, that Spring MVC Test should try and re-use it. It already does that for the ServletContext. However, the TestContext framework has no chance of knowing that the user may choose to do MockMvc.perform(fileUpload("/foo")) which wold set up a MockMultipartHttpServletRequest instead. I see no alternative good solution for that.

Taking a step back when writing MockMvc tests there is hardly a reason I can see to inject the mock request or response into the test fixture. The only way this gets exposed is when a controller has the request injected (e.g. via @Autowired). Perhaps we can consider addressing that problem specifically vs a broader solution that ensures TestContext and MockMvc use the same request/response instances. The DispatcherServlet does its part to set up the RequestContextHolder with the request created in MockMvc. Shouldn't then request-scoped proxy beans have access to the "right" request instance?

[spring-projects-issues]

Sam Brannen commented

Pushed updated version of RequestContextHolderTests in GitHub commit 5e24ee9.

[spring-projects-issues]

Sam Brannen commented

Pushed an additional commit:

Investigate claims raised in #17803

This commit adds additional tests to RequestContextHolderTests that
verify proper support for the MockHttpServletRequest managed by Spring
MVC Test in the following scenarios:

• request-scoped service invoked by controller
• session-scoped service invoked by controller
• custom filter that sets request attributes

[spring-projects-issues]

Sam Brannen commented

In summary, RequestContextHolderTests now tests the following scenarios:

• singleton controller
• request-scoped controller
• request-scoped service invoked by controller
• session-scoped service invoked by controller

Each scenario interacts with the request attributes set in the following ways:

• custom filter configured in MockMvc
• as default request attribute in MockMvc
• as a per-request request attribute configured in MockMvc

For all of the aforementioned scenarios, I have not been able to reproduce the inappropriate behavior described on Stack Overflow.

The only thing that does not work is the following:

• Request attributes set in the MockHttpServletRequest managed by the ServletTestExecutionListener are not visible to MVC components (including request-scoped or session-scoped components) when MockMvc is in use.

[spring-projects-issues]

Rossen Stoyanchev commented

I think this might be the clue: "So, basically, the attribute i set in the filter in the request, can't be read in the ClientSessionServiceImpl, because different instance of MockHttpServletRequest is injected there."

If he is setting a request attribute from a Filter, at that point the DispatcherServlet has not yet set the RequestAttributes to the current request. He could add RequestContextFilter to his MockMvc setup. We could also consider a fix in MockMvc.perform that does this early, i.e. before the filter chain executes.

[spring-projects-issues]

Sam Brannen commented

Completed as described in GitHub commit bf06bf3:

Reuse mock request from the TCF in Spring MVC Test

Prior to this commit, the Spring MVC Test Framework always created a
new MockHttpServletRequest, disregarding any mock request already
present in Spring Web's RequestContextHolder -- for example, one
created by the ServletTestExecutionListener in the Spring TestContext
Framework (TCF).

This commit modifies MockHttpServletRequestBuilder so that it reuses a
mock request created by the TCF. However,
MockMultipartHttpServletRequestBuilder continues to always create a new
MockMultipartHttpServletRequest since a MockHttpServletRequest created
by the TCF is not directly compatible with a
MockMultipartHttpServletRequest. Furthermore, in order to avoid
unforeseen side effects, MockHttpServletRequestBuilder will always
create a new MockHttpServletRequest if a mock request is present in the
RequestContextHolder but not created by the TCF.

[spring-projects-issues]

Sam Brannen commented

Reopening this issue in light of the fact that reuse of the mock request precludes reliable reuse of MockMvc multiple times within a single test method. See #17851 and the (soon to be committed) MockMvcReuseTests class for details.

[spring-projects-issues]

Sam Brannen commented

Now resolving this issue as Invalid due to the regression it introduced as outlined in #17851.