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

RedirectAttributes parameter results in IllegalArgumentException if using DefaultAnnotationHandlerMapping or AnnotationMethodHandlerAdapter [SPR-8782] #13425

Closed
spring-issuemaster opened this Issue Oct 17, 2011 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Oct 17, 2011

Andrea opened SPR-8782 and commented

When a @RequestMapping method contains a RedirectAttributes parameter, an IllegalArgumentException is thrown.

For example, consider this HTML form:

<form action="/deleteaction" method="post">
<input type="checkbox" name="itemId" value="1" />
<input type="checkbox" name="itemId" value="2" />
<input type="submit" name="delete" value="Delete items" />
</form>

When submitted, the following method is executed properly:

@RequestMapping(value = "/deleteaction", method = RequestMethod.POST, params={"delete"})
public String deleteItems(@RequestParam(value="itemId", required=false) String itemId[], Model model) {
// DO BUSINESS LOGIC
return "redirect:/";
}

If instead of a Model, I put a RedirectAttributes parameter in the method, HandlerMethodInvoker throws an exception:

@RequestMapping(value = "/deleteaction", method = RequestMethod.POST, params={"delete"})
public String deleteItems(@RequestParam(value="itemId", required=false) String itemId[], RedirectAttributes redirectAttributes) {
// DO BUSINESS LOGIC
return "redirect:/";
}

The problem is at line 317 in class HandlerMethodInvoker, where a BindingAwareModelMap (which is fine for Model) is resolved as parameter for RedirectAttributes. When the BindingAwareModelMap parameter is passed as argument to the request mapping method, the exception occurs.


Affects: 3.1 RC1

Reference URL: http://forum.springsource.org/showthread.php?115976-Spring-MVC-FlashMap-and-RedirectAttributes-request-mapping

Referenced from: commits 1164f5a

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 17, 2011

Rossen Stoyanchev commented

Use of RedirectAttributes requires the new RequestMappingHandlerMapping and RequestMappingHandlerAdapter pair. Either the MVC namespace (or MVC Java config) must be used or if the application configuration includes DefaultAnnotationHandlerMapping or AnnotationMethodHandlerAdapter, they must be replaced with the above mentioned classes.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 18, 2011

Andrea commented

I completely agree the new RedirectAttributes only works with the new RequestMappingHandlerMapping and RequestMappingHandlerAdapter pair.

Anyway, if somebody manually(wrongly) configures the dispatcher-servlet.xml without
<mvc:annotation-driven /> and defines the old DefaultAnnotationHandlerMapping, Spring should not try to call a method having RedirectAttributes parameter passing a BindingAwareModelMap.

Lines 316-319 here:
https://src.springframework.org/svn/spring-framework/trunk/org.springframework.web/src/main/java/org/springframework/web/bind/annotation/support/HandlerMethodInvoker.java

Class paramType = methodParam.getParameterType();
if (Model.class.isAssignableFrom(paramType) || Map.class.isAssignableFrom(paramType)) {
args[i] = implicitModel;
}

I think HandlerMethodInvoker makes some confusions here. Model is assignable from RedirectAttributes, but RedirectAttributes is not (always) assignable from Model. So it does not make much sense to resolve a RedirectAttributes starting from a Model...

In the above described case, I think it is better for Spring to find no method at all to process a given request rather than throwing an IllegalArgumentException.

Probably the above code should be something like, inverting paramType with model:

Class paramType = methodParam.getParameterType();
if (paramType.isAssignableFrom(Model.class) || paramType.isAssignableFrom(Map.class)) {
args[i] = implicitModel;
}

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 19, 2011

Rossen Stoyanchev commented

I see what you're getting at although this will fail:

Class<?> paramType = BindingAwareModelMap.class;
assertTrue(paramType.isAssignableFrom(Model.class));

This may help to ensure that the actual type of the model is compatible with the argument type:

if (paramType.isAssignableFrom(implicitModel.getClass())) {
    // ...
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment