DefaultMockMvcBuilder combined with @ContextHierarchy registers wrong context as ROOT [SPR-13075]

[spring-projects-issues]

Joris Kuipers opened SPR-13075 and commented

When you have test class that looks like this:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextHierarchy({
    @ContextConfiguration(classes = DemoApplicationTests.RootConfig.class),
    @ContextConfiguration(classes = DemoApplicationTests.ServletConfig.class)
})
@WebAppConfiguration
public class DemoApplicationTests {
    // ....

... and you bootstrap a MockMvc variable using MockMvcBuilders.webAppContextSetup(applicationContext).build(), you end up with the wrong WebApplicationContext getting registered as the ROOT context in the mocked ServletContext.

We found this when upgrading an app using the Thymeleaf Spring Security dialect: their code expects some security expression handler bean which we register in the root context, but since the wrong context is found the code fails.

The corresponding code change was introduced in Spring 4.1.4, probably as part of #17155.

There's a check missing to see if the given context is in fact the root context in DefaultMockMvcBuilder#initWebAppContext(). A similar check is done in other places of the test support (like the AbstractGenericWebContextLoader).

I've attached a project with (only) a test case to reproduce the issue. By commenting out the line of code which created the MockMvc or by lowering the Spring version to 4.1.3 the test case passes, but as given it fails. I think it should succeed, and the most likely solution seems to navigate the context hierarchy to find the real root context in DefaultMockMvcBuilder#initWebAppContext(): I'll leave that up to you guys to decide. ;)

---

Affects: 4.1.4, 4.1.5, 4.1.6

Attachments:

• spring-bug.zip (2.38 kB)

Referenced from: commits spring-attic/spring-framework-issues@321c4c3

[spring-projects-issues]

Sam Brannen commented

FYI: the code that performs the look-up of the root WebApplicationContext can be found in AbstractGenericWebContextLoader.configureWebResources().

We can consider extracting this algorithm into a common utility class.

[spring-projects-issues]

Rossen Stoyanchev commented

Sounds like that would probably be needed but I should have a closer look.

[spring-projects-issues]

Sam Brannen commented

See also: http://stackoverflow.com/questions/30613148/spring-security-defaultmethodsecurityexpressionhandler-bean-is-not-registered-fo/30633603#30633603

[spring-projects-issues]

Rossen Stoyanchev commented

Sam Brannen, looking at AbstractGenericWebContextLoader.configureWebResources it's clear initialization of the ServletContext ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE is supported in the TestContext framework and has been probably from the start, i.e. v 4.0? In which case I'm not sure how to reproduce the issue that was reported in #17155 using 4.1.3 (the version against which it was reported). See my comments under #17155 as well.

[spring-projects-issues]

Rossen Stoyanchev commented

Attached test project with one additional class (standaloneSetup) is now in https://github.com/spring-projects/spring-framework-issues/tree/master/SPR-13075.

[spring-projects-issues]

Sam Brannen commented

Hi Rossen,

Thanks for investigating this.

Your assumptions are correct: the TCF has supported this correctly since 3.2 and with context hierarchies since 3.2.2 (when @ContextHierarchy was introduced).

Also, you're right: the fix introduced in #17155 is broken: it only makes sense if ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE has not yet been set (which is not the case if you use the TCF to load your context). In other words, if you use @ContextConfiguration and the TCF to load your contexts, you can rest assured that ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE has been set properly.

And... the only way that ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE would not be set properly is if the user manually instantiates the WebApplicationContext (or multiple WACs in a hierarchy) and fails to set the ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE in the WebApplicationContext's ServletContext - which is a user error, not an error on the part of Spring's testing support.

In summary, #17155 should have addressed the scenario I described in the previous paragraph. DefaultMockMvcBuilder simply must not override ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE if it has already been set.

I'll undo the breaking changes introduced in #17155 and add a proper fix in conjunction with this issue.

Regards,

Sam

[spring-projects-issues]

Sam Brannen commented

Fixed as described in GitHub commit f6d2fe4:

Proper support for Root WAC in Spring MVC Test

The modifications to DefaultMockMvcBuilder performed in conjunction
with #17155 introduced a breaking change: the WebApplicationContext
supplied to DefaultMockMvcBuilder's constructor was always stored in
the ServletContext as the root WebApplicationContext, overwriting a
root WebApplicationContext that had been set by the user or by the
Spring TestContext Framework (TCF) -- for example, in
AbstractGenericWebContextLoader. Consequently, the changes in #17155
cause tests that use @ContextHierarchy to fail if web components rely
on the correct WebApplicationContext being stored under the
WebApplicationContext#ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE key.

This commit reverts the breaking changes introduced in #17155: if
the root WebApplicationContext has already been set in the
ServletContext of the WebApplicationContext supplied to
DefaultMockMvcBuilder, no action is taken.

Furthermore, this commit introduces new code to address the initial
intent of #17155. Specifically, if the root WebApplicationContext
has NOT been set in the ServletContext of the WebApplicationContext
supplied to DefaultMockMvcBuilder, the application context hierarchy
will be traversed in search of the root WebApplicationContext, and the
root WebApplicationContext will then be stored under the
ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE key.

[spring-projects-issues]

Sam Brannen commented

Backported to Spring Framework 4.1.7 in GitHub commit 33b9bd7.