Skip to content

Commit 72cc060

Browse files
jhoellerjhoeller
committed
request handler methods with @ModelAttribute annotation always return a model attribute (SPR-4867)
1 parent 50f49ff commit 72cc060

File tree

3 files changed

+41
-38
lines changed

3 files changed

+41
-38
lines changed

‎org.springframework.web.servlet/src/main/java/org/springframework/web/bind/annotation/support/HandlerMethodInvoker.java

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ public final Object invokeHandlerMethod(Method handlerMethod, Object handler,
136136
Object attrValue = doInvokeMethod(attributeMethodToInvoke, handler, args);
137137
String attrName = AnnotationUtils.findAnnotation(attributeMethodToInvoke, ModelAttribute.class).value();
138138
if ("".equals(attrName)) {
139-
Class resolvedType =
140-
GenericTypeResolver.resolveReturnType(attributeMethodToInvoke, handler.getClass());
141-
attrName =
142-
Conventions.getVariableNameForReturnType(attributeMethodToInvoke, resolvedType, attrValue);
139+
Class resolvedType = GenericTypeResolver.resolveReturnType(
140+
attributeMethodToInvoke, handler.getClass());
141+
attrName = Conventions.getVariableNameForReturnType(
142+
attributeMethodToInvoke, resolvedType, attrValue);
143143
}
144144
implicitModel.addAttribute(attrName, attrValue);
145145
}
@@ -156,10 +156,8 @@ public final Object invokeHandlerMethod(Method handlerMethod, Object handler,
156156
}
157157

158158
@SuppressWarnings("unchecked")
159-
private Object[] resolveHandlerArguments(Method handlerMethod,
160-
Object handler,
161-
NativeWebRequest webRequest,
162-
ExtendedModelMap implicitModel) throws Exception {
159+
private Object[] resolveHandlerArguments(Method handlerMethod, Object handler,
160+
NativeWebRequest webRequest, ExtendedModelMap implicitModel) throws Exception {
163161

164162
Class[] paramTypes = handlerMethod.getParameterTypes();
165163
Object[] args = new Object[paramTypes.length];
@@ -446,7 +444,6 @@ protected Object resolveRequestBody(MethodParameter methodParam, NativeWebReques
446444
throws Exception {
447445

448446
HttpInputMessage inputMessage = createHttpInputMessage(webRequest);
449-
450447
Class paramType = methodParam.getParameterType();
451448
MediaType contentType = inputMessage.getHeaders().getContentType();
452449
if (contentType == null) {
@@ -463,16 +460,14 @@ protected Object resolveRequestBody(MethodParameter methodParam, NativeWebReques
463460
}
464461
}
465462
}
466-
467463
throw new HttpMediaTypeNotSupportedException(contentType, allSupportedMediaTypes);
468464
}
469465

470466
/**
471-
* Returns a {@link HttpInputMessage} for the given {@link NativeWebRequest}.
467+
* Return a {@link HttpInputMessage} for the given {@link NativeWebRequest}.
472468
* Throws an UnsupportedOperationException by default.
473469
*/
474470
protected HttpInputMessage createHttpInputMessage(NativeWebRequest webRequest) throws Exception {
475-
476471
throw new UnsupportedOperationException("@RequestBody not supported");
477472
}
478473

@@ -585,10 +580,8 @@ else if (this.methodResolver.isSessionAttribute(name, paramType)) {
585580
}
586581

587582
@SuppressWarnings("unchecked")
588-
public final void updateModelAttributes(Object handler,
589-
Map mavModel,
590-
ExtendedModelMap implicitModel,
591-
NativeWebRequest webRequest) throws Exception {
583+
public final void updateModelAttributes(Object handler, Map<String, Object> mavModel,
584+
ExtendedModelMap implicitModel, NativeWebRequest webRequest) throws Exception {
592585

593586
if (this.methodResolver.hasSessionAttributes() && this.sessionStatus.isComplete()) {
594587
for (String attrName : this.methodResolver.getActualSessionAttributeNames()) {
@@ -692,11 +685,22 @@ protected Object resolveCommonArgument(MethodParameter methodParameter, NativeWe
692685
}
693686

694687
protected Object resolveStandardArgument(Class parameterType, NativeWebRequest webRequest) throws Exception {
695-
696688
if (WebRequest.class.isAssignableFrom(parameterType)) {
697689
return webRequest;
698690
}
699691
return WebArgumentResolver.UNRESOLVED;
700692
}
701693

694+
protected final void addReturnValueAsModelAttribute(
695+
Method handlerMethod, Class handlerType, Object returnValue, ExtendedModelMap implicitModel) {
696+
697+
ModelAttribute attr = AnnotationUtils.findAnnotation(handlerMethod, ModelAttribute.class);
698+
String attrName = (attr != null ? attr.value() : "");
699+
if ("".equals(attrName)) {
700+
Class resolvedType = GenericTypeResolver.resolveReturnType(handlerMethod, handlerType);
701+
attrName = Conventions.getVariableNameForReturnType(handlerMethod, resolvedType, returnValue);
702+
}
703+
implicitModel.addAttribute(attrName, returnValue);
704+
}
705+
702706
}

‎org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@
4545
import org.apache.commons.logging.LogFactory;
4646

4747
import org.springframework.beans.BeanUtils;
48-
import org.springframework.core.Conventions;
49-
import org.springframework.core.GenericTypeResolver;
5048
import org.springframework.core.LocalVariableTableParameterNameDiscoverer;
5149
import org.springframework.core.MethodParameter;
5250
import org.springframework.core.ParameterNameDiscoverer;
@@ -662,12 +660,16 @@ public ModelAndView getModelAndView(Method handlerMethod,
662660
else if (returnValue instanceof Model) {
663661
return new ModelAndView().addAllObjects(implicitModel).addAllObjects(((Model) returnValue).asMap());
664662
}
665-
else if (returnValue instanceof Map) {
666-
return new ModelAndView().addAllObjects(implicitModel).addAllObjects((Map) returnValue);
667-
}
668663
else if (returnValue instanceof View) {
669664
return new ModelAndView((View) returnValue).addAllObjects(implicitModel);
670665
}
666+
else if (handlerMethod.isAnnotationPresent(ModelAttribute.class)) {
667+
addReturnValueAsModelAttribute(handlerMethod, handlerType, returnValue, implicitModel);
668+
return new ModelAndView().addAllObjects(implicitModel);
669+
}
670+
else if (returnValue instanceof Map) {
671+
return new ModelAndView().addAllObjects(implicitModel).addAllObjects((Map) returnValue);
672+
}
671673
else if (returnValue instanceof String) {
672674
return new ModelAndView((String) returnValue).addAllObjects(implicitModel);
673675
}
@@ -683,14 +685,8 @@ else if (returnValue == null) {
683685
}
684686
else if (!BeanUtils.isSimpleProperty(returnValue.getClass())) {
685687
// Assume a single model attribute...
686-
ModelAttribute attr = AnnotationUtils.findAnnotation(handlerMethod, ModelAttribute.class);
687-
String attrName = (attr != null ? attr.value() : "");
688-
ModelAndView mav = new ModelAndView().addAllObjects(implicitModel);
689-
if ("".equals(attrName)) {
690-
Class resolvedType = GenericTypeResolver.resolveReturnType(handlerMethod, handlerType);
691-
attrName = Conventions.getVariableNameForReturnType(handlerMethod, resolvedType, returnValue);
692-
}
693-
return mav.addObject(attrName, returnValue);
688+
addReturnValueAsModelAttribute(handlerMethod, handlerType, returnValue, implicitModel);
689+
return new ModelAndView().addAllObjects(implicitModel);
694690
}
695691
else {
696692
throw new IllegalArgumentException("Invalid handler method return value: " + returnValue);
@@ -726,6 +722,7 @@ public int hashCode() {
726722
}
727723
}
728724

725+
729726
/**
730727
* Comparator capable of sorting {@link RequestMappingInfo}s (RHIs) so that sorting a list with this comparator will
731728
* result in: <ul> <li>RHIs with {@linkplain RequestMappingInfo#matchedPaths better matched paths} take prescedence

‎org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ protected WebApplicationContext createWebApplicationContext(WebApplicationContex
346346
request.addParameter("age", "value2");
347347
MockHttpServletResponse response = new MockHttpServletResponse();
348348
servlet.service(request, response);
349-
assertEquals("myView-name1-typeMismatch-tb1-myValue", response.getContentAsString());
349+
assertEquals("myPath-name1-typeMismatch-tb1-myValue-yourValue", response.getContentAsString());
350350
}
351351

352352
@Test
@@ -360,8 +360,7 @@ protected WebApplicationContext createWebApplicationContext(WebApplicationContex
360360
DefaultAdvisorAutoProxyCreator autoProxyCreator = new DefaultAdvisorAutoProxyCreator();
361361
autoProxyCreator.setBeanFactory(wac.getBeanFactory());
362362
wac.getBeanFactory().addBeanPostProcessor(autoProxyCreator);
363-
wac.getBeanFactory()
364-
.registerSingleton("advisor", new DefaultPointcutAdvisor(new SimpleTraceInterceptor()));
363+
wac.getBeanFactory().registerSingleton("advisor", new DefaultPointcutAdvisor(new SimpleTraceInterceptor()));
365364
wac.refresh();
366365
return wac;
367366
}
@@ -1108,11 +1107,12 @@ public List<TestBean> getTestBeans() {
11081107
}
11091108

11101109
@RequestMapping("/myPath.do")
1110+
@ModelAttribute("yourKey")
11111111
public String myHandle(@ModelAttribute("myCommand") TestBean tb, BindingResult errors, Model model) {
11121112
if (!model.containsAttribute("myKey")) {
11131113
model.addAttribute("myKey", "myValue");
11141114
}
1115-
return "myView";
1115+
return "yourValue";
11161116
}
11171117
}
11181118

@@ -1379,9 +1379,9 @@ public void render(Map model, HttpServletRequest request, HttpServletResponse re
13791379
}
13801380
List<TestBean> testBeans = (List<TestBean>) model.get("testBeanList");
13811381
if (errors.hasFieldErrors("age")) {
1382-
response.getWriter()
1383-
.write(viewName + "-" + tb.getName() + "-" + errors.getFieldError("age").getCode() +
1384-
"-" + testBeans.get(0).getName() + "-" + model.get("myKey"));
1382+
response.getWriter().write(viewName + "-" + tb.getName() + "-" +
1383+
errors.getFieldError("age").getCode() + "-" + testBeans.get(0).getName() + "-" +
1384+
model.get("myKey") + (model.containsKey("yourKey") ? "-" + model.get("yourKey") : ""));
13851385
}
13861386
else {
13871387
response.getWriter().write(viewName + "-" + tb.getName() + "-" + tb.getAge() + "-" +
@@ -1472,6 +1472,7 @@ public void get() {
14721472
}
14731473
}
14741474

1475+
14751476
@Controller
14761477
public static class PathOrderingController {
14771478

@@ -1486,6 +1487,7 @@ public void method2(Writer writer) throws IOException {
14861487
}
14871488
}
14881489

1490+
14891491
@Controller
14901492
public static class RequestBodyController {
14911493

@@ -1495,6 +1497,7 @@ public void handle(@RequestBody String body, Writer writer) throws IOException {
14951497
}
14961498
}
14971499

1500+
14981501
public static class MyMessageConverter implements HttpMessageConverter {
14991502

15001503
public boolean supports(Class clazz) {
@@ -1516,5 +1519,4 @@ public void write(Object o, HttpOutputMessage outputMessage)
15161519
}
15171520
}
15181521

1519-
15201522
}

0 commit comments

Comments
 (0)