Improve handling of request parameter without values in MockMvc

[bennypi]

I want to test a login controller that is used together with spring security.

The controller is implemented like this:

@Controller
@RequestMapping("/login")
public class LoginController
{
  @GetMapping
  public ModelAndView loginPage(@RequestParam(value = "logout", required = false) String logout,
                                @RequestParam(value = "error", required = false) String error)
  {
    ModelAndView modelAndView = new ModelAndView("login");
    if (logout != null)
    {
      modelAndView.addObject("SUCCESS_MESSAGE", "Logout erfolgreich");
    }
    if (error != null)
    {
      modelAndView.addObject("ERROR_MESSAGE", "Login nicht erfolgreich");
    }
    return modelAndView;
  }

This works perfectly with spring-security's behavior for logouts and invalid login attempts, where the logout and error parameters are an empty string and not null.

For the test, I choose the following setup:

@WebMvcTest(controllers = {LoginController.class}, excludeAutoConfiguration = SecurityAutoConfiguration.class)
class LoginControllerTest
{

  @Autowired
  MockMvc mockMvc;

  @SneakyThrows
  @Test
  void testLogoutPage()
  {
    mockMvc.perform(get("/login?logout"))
           .andExpectAll(status().isOk(),
                         MockMvcResultMatchers.view().name("login"),
                         MockMvcResultMatchers.model().attributeExists("SUCCESS_MESSAGE"));
  }
}

This however leads to the query parameter logout being null in the LoginController, which fails the test. I also tried get("/login").queryParam("logout"), but queryParam(...) does not permit empty values. I could add a dummy value, but this would be inconsistent with the redirect behavior of spring-security. I would prefer to pass an empty query parameter through MockMvc, but this seems to be not possible?

[FBibonne]

@bennypi , slightly modifying the controller because I have not the view defined, these two fixes in the test work for me :

1. replace "/login?logout" with "/login?logout=" in the get
2. replace get("/login?logout") with get("/login).queryParam("logout", "")

Does it work for you ?

NB : the get method is from MockMvcRequestBuilders class

[bennypi]

Yeah, I can confirm that both fixes are working. However I would still argue that get("/login").queryParam("logout") should be expected to work, at least it should be noted in the JavaDoc.

[rstoyanchev]

In general the idea is that a query param may have a value, an empty string value, or be present but without a value. This is a distinction that UriComponentsBuilder supports through "?foo=bar", "?foo=", and "?foo". This is why "/login?logout=" works.

In MockMvc, queryParam(String name, String... values) raises an assertion exception in addToMultiValueMap if the values are empty. Could you confirm if you get the IllegalArgumentException and if not, could you try to debug what's different?

If the assertion is removed, and the parameter added with an empty list, the next issue is in build() where the query param should be added with all values, but currently is skipped.

We can improve MockMvc to ensure the absence of values is handled without an exception, similar to what UriComponentsBuilder does, and results in a query parameter without values.

Generally MockMvc is neutral and merely sets the MockHttpServletRequest with the provided values. So for an empty String you'll need query("foo").