Skip to content

Commit 2295372

Browse files
committed
Backport fixes in HandlerMethod and sub-classes
Issue: SPR-9747, SPR-9748, SPR-9218, SPR-8946, SPR-9159 Backport Issue: SPR-9622
1 parent bec5463 commit 2295372

File tree

7 files changed

+339
-281
lines changed

7 files changed

+339
-281
lines changed

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java

Lines changed: 64 additions & 76 deletions
Large diffs are not rendered by default.

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java

Lines changed: 74 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,28 @@
2222
import org.springframework.http.HttpStatus;
2323
import org.springframework.util.StringUtils;
2424
import org.springframework.web.bind.annotation.ResponseStatus;
25-
import org.springframework.web.context.request.NativeWebRequest;
2625
import org.springframework.web.context.request.ServletWebRequest;
27-
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
26+
import org.springframework.web.method.HandlerMethod;
2827
import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
2928
import org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite;
3029
import org.springframework.web.method.support.InvocableHandlerMethod;
3130
import org.springframework.web.method.support.ModelAndViewContainer;
3231
import org.springframework.web.servlet.View;
3332

3433
/**
35-
* Extends {@link InvocableHandlerMethod} with the ability to handle the value returned from the method through
36-
* a registered {@link HandlerMethodArgumentResolver} that supports the given return value type.
37-
* Return value handling may include writing to the response or updating the {@link ModelAndViewContainer} structure.
34+
* Extends {@link InvocableHandlerMethod} with the ability to handle return
35+
* values through a registered {@link HandlerMethodReturnValueHandler} and
36+
* also supports setting the response status based on a method-level
37+
* {@code @ResponseStatus} annotation.
3838
*
39-
* <p>If the underlying method has a {@link ResponseStatus} instruction, the status on the response is set
40-
* accordingly after the method is invoked but before the return value is handled.
39+
* <p>A {@code null} return value (including void) may be interpreted as the
40+
* end of request processing in combination with a {@code @ResponseStatus}
41+
* annotation, a not-modified check condition
42+
* (see {@link ServletWebRequest#checkNotModified(long)}), or
43+
* a method argument that provides access to the response stream.
4144
*
4245
* @author Rossen Stoyanchev
4346
* @since 3.1
44-
* @see #invokeAndHandle(NativeWebRequest, ModelAndViewContainer, Object...)
4547
*/
4648
public class ServletInvocableHandlerMethod extends InvocableHandlerMethod {
4749

@@ -51,104 +53,104 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod {
5153

5254
private HandlerMethodReturnValueHandlerComposite returnValueHandlers;
5355

54-
public void setHandlerMethodReturnValueHandlers(HandlerMethodReturnValueHandlerComposite returnValueHandlers) {
55-
this.returnValueHandlers = returnValueHandlers;
56-
}
5756

5857
/**
59-
* Creates a {@link ServletInvocableHandlerMethod} instance with the given bean and method.
60-
* @param handler the object handler
61-
* @param method the method
58+
* Creates an instance from the given handler and method.
6259
*/
6360
public ServletInvocableHandlerMethod(Object handler, Method method) {
6461
super(handler, method);
62+
initResponseStatus();
63+
}
64+
65+
/**
66+
* Create an instance from a {@code HandlerMethod}.
67+
*/
68+
public ServletInvocableHandlerMethod(HandlerMethod handlerMethod) {
69+
super(handlerMethod);
70+
initResponseStatus();
71+
}
6572

66-
ResponseStatus annotation = getMethodAnnotation(ResponseStatus.class);
67-
if (annotation != null) {
68-
this.responseStatus = annotation.value();
69-
this.responseReason = annotation.reason();
73+
private void initResponseStatus() {
74+
ResponseStatus annot = getMethodAnnotation(ResponseStatus.class);
75+
if (annot != null) {
76+
this.responseStatus = annot.value();
77+
this.responseReason = annot.reason();
7078
}
7179
}
7280

7381
/**
74-
* Invokes the method and handles the return value through a registered {@link HandlerMethodReturnValueHandler}.
75-
* <p>Return value handling may be skipped entirely when the method returns {@code null} (also possibly due
76-
* to a {@code void} return type) and one of the following additional conditions is true:
77-
* <ul>
78-
* <li>A {@link HandlerMethodArgumentResolver} has set the {@link ModelAndViewContainer#setRequestHandled(boolean)}
79-
* flag to {@code false} -- e.g. method arguments providing access to the response.
80-
* <li>The request qualifies as "not modified" as defined in {@link ServletWebRequest#checkNotModified(long)}
81-
* and {@link ServletWebRequest#checkNotModified(String)}. In this case a response with "not modified" response
82-
* headers will be automatically generated without the need for return value handling.
83-
* <li>The status on the response is set due to a @{@link ResponseStatus} instruction.
84-
* </ul>
85-
* <p>After the return value is handled, callers of this method can use the {@link ModelAndViewContainer}
86-
* to gain access to model attributes, view selection choices, and to check if view resolution is even needed.
82+
* Register {@link HandlerMethodReturnValueHandler} instances to use to
83+
* handle return values.
84+
*/
85+
public void setHandlerMethodReturnValueHandlers(HandlerMethodReturnValueHandlerComposite returnValueHandlers) {
86+
this.returnValueHandlers = returnValueHandlers;
87+
}
88+
89+
/**
90+
* Invokes the method and handles the return value through a registered
91+
* {@link HandlerMethodReturnValueHandler}.
8792
*
88-
* @param request the current request
89-
* @param mavContainer the {@link ModelAndViewContainer} for the current request
90-
* @param providedArgs argument values to try to use without the need for view resolution
93+
* @param webRequest the current request
94+
* @param mavContainer the ModelAndViewContainer for this request
95+
* @param providedArgs "given" arguments matched by type, not resolved
9196
*/
92-
public final void invokeAndHandle(
93-
NativeWebRequest request, ModelAndViewContainer mavContainer,
94-
Object... providedArgs) throws Exception {
97+
public final void invokeAndHandle(ServletWebRequest webRequest,
98+
ModelAndViewContainer mavContainer, Object... providedArgs) throws Exception {
9599

96-
Object returnValue = invokeForRequest(request, mavContainer, providedArgs);
100+
Object returnValue = invokeForRequest(webRequest, mavContainer, providedArgs);
97101

98-
setResponseStatus((ServletWebRequest) request);
102+
setResponseStatus(webRequest);
99103

100104
if (returnValue == null) {
101-
if (isRequestNotModified(request) || hasResponseStatus() || mavContainer.isRequestHandled()) {
105+
if (isRequestNotModified(webRequest) || hasResponseStatus() || mavContainer.isRequestHandled()) {
102106
mavContainer.setRequestHandled(true);
103107
return;
104108
}
105109
}
110+
else if (StringUtils.hasText(this.responseReason)) {
111+
mavContainer.setRequestHandled(true);
112+
return;
113+
}
106114

107115
mavContainer.setRequestHandled(false);
108116

109117
try {
110-
returnValueHandlers.handleReturnValue(returnValue, getReturnType(), mavContainer, request);
111-
} catch (Exception ex) {
118+
this.returnValueHandlers.handleReturnValue(returnValue, getReturnValueType(returnValue), mavContainer, webRequest);
119+
}
120+
catch (Exception ex) {
112121
if (logger.isTraceEnabled()) {
113122
logger.trace(getReturnValueHandlingErrorMessage("Error handling return value", returnValue), ex);
114123
}
115124
throw ex;
116125
}
117126
}
118127

119-
private String getReturnValueHandlingErrorMessage(String message, Object returnValue) {
120-
StringBuilder sb = new StringBuilder(message);
121-
if (returnValue != null) {
122-
sb.append(" [type=" + returnValue.getClass().getName() + "] ");
123-
}
124-
sb.append("[value=" + returnValue + "]");
125-
return getDetailedErrorMessage(sb.toString());
126-
}
127-
128128
/**
129129
* Set the response status according to the {@link ResponseStatus} annotation.
130130
*/
131131
private void setResponseStatus(ServletWebRequest webRequest) throws IOException {
132-
if (this.responseStatus != null) {
133-
if (StringUtils.hasText(this.responseReason)) {
134-
webRequest.getResponse().sendError(this.responseStatus.value(), this.responseReason);
135-
}
136-
else {
137-
webRequest.getResponse().setStatus(this.responseStatus.value());
138-
}
132+
if (this.responseStatus == null) {
133+
return;
134+
}
139135

140-
// to be picked up by the RedirectView
141-
webRequest.getRequest().setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, this.responseStatus);
136+
if (StringUtils.hasText(this.responseReason)) {
137+
webRequest.getResponse().sendError(this.responseStatus.value(), this.responseReason);
142138
}
139+
else {
140+
webRequest.getResponse().setStatus(this.responseStatus.value());
141+
}
142+
143+
// to be picked up by the RedirectView
144+
webRequest.getRequest().setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, this.responseStatus);
143145
}
144146

145147
/**
146148
* Does the given request qualify as "not modified"?
147149
* @see ServletWebRequest#checkNotModified(long)
148150
* @see ServletWebRequest#checkNotModified(String)
149151
*/
150-
private boolean isRequestNotModified(NativeWebRequest request) {
151-
return ((ServletWebRequest) request).isNotModified();
152+
private boolean isRequestNotModified(ServletWebRequest webRequest) {
153+
return webRequest.isNotModified();
152154
}
153155

154156
/**
@@ -157,4 +159,14 @@ private boolean isRequestNotModified(NativeWebRequest request) {
157159
private boolean hasResponseStatus() {
158160
return responseStatus != null;
159161
}
162+
163+
private String getReturnValueHandlingErrorMessage(String message, Object returnValue) {
164+
StringBuilder sb = new StringBuilder(message);
165+
if (returnValue != null) {
166+
sb.append(" [type=" + returnValue.getClass().getName() + "] ");
167+
}
168+
sb.append("[value=" + returnValue + "]");
169+
return getDetailedErrorMessage(sb.toString());
170+
}
171+
160172
}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,9 +15,10 @@
1515
*/
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
18-
import static org.junit.Assert.*;
1918
import static org.junit.Assert.assertEquals;
20-
import static org.junit.Assert.assertFalse;
19+
import static org.junit.Assert.assertNotNull;
20+
import static org.junit.Assert.assertTrue;
21+
import static org.junit.Assert.fail;
2122

2223
import java.lang.reflect.Method;
2324

@@ -30,17 +31,20 @@
3031
import org.springframework.http.converter.HttpMessageNotWritableException;
3132
import org.springframework.mock.web.MockHttpServletRequest;
3233
import org.springframework.mock.web.MockHttpServletResponse;
34+
import org.springframework.web.bind.annotation.RequestParam;
3335
import org.springframework.web.bind.annotation.ResponseStatus;
3436
import org.springframework.web.context.request.NativeWebRequest;
3537
import org.springframework.web.context.request.ServletWebRequest;
38+
import org.springframework.web.method.annotation.RequestParamMethodArgumentResolver;
3639
import org.springframework.web.method.support.HandlerMethodArgumentResolverComposite;
3740
import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
3841
import org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite;
3942
import org.springframework.web.method.support.ModelAndViewContainer;
43+
import org.springframework.web.servlet.view.RedirectView;
4044

4145
/**
4246
* Test fixture with {@link ServletInvocableHandlerMethod}.
43-
*
47+
*
4448
* @author Rossen Stoyanchev
4549
*/
4650
public class ServletInvocableHandlerMethodTests {
@@ -53,31 +57,32 @@ public class ServletInvocableHandlerMethodTests {
5357

5458
private ServletWebRequest webRequest;
5559

60+
private MockHttpServletRequest request;
61+
5662
private MockHttpServletResponse response;
5763

5864
@Before
5965
public void setUp() throws Exception {
6066
returnValueHandlers = new HandlerMethodReturnValueHandlerComposite();
6167
argumentResolvers = new HandlerMethodArgumentResolverComposite();
6268
mavContainer = new ModelAndViewContainer();
69+
request = new MockHttpServletRequest();
6370
response = new MockHttpServletResponse();
64-
webRequest = new ServletWebRequest(new MockHttpServletRequest(), response);
71+
webRequest = new ServletWebRequest(request, response);
6572
}
6673

6774
@Test
68-
public void nullReturnValueResponseStatus() throws Exception {
75+
public void invokeAndHandle_VoidWithResponseStatus() throws Exception {
6976
ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("responseStatus");
7077
handlerMethod.invokeAndHandle(webRequest, mavContainer);
7178

7279
assertTrue("Null return value + @ResponseStatus should result in 'request handled'",
7380
mavContainer.isRequestHandled());
74-
7581
assertEquals(HttpStatus.BAD_REQUEST.value(), response.getStatus());
76-
assertEquals("400 Bad Request", response.getErrorMessage());
7782
}
7883

7984
@Test
80-
public void nullReturnValueHttpServletResponseArg() throws Exception {
85+
public void invokeAndHandle_VoidWithHttpServletResponseArgument() throws Exception {
8186
argumentResolvers.addResolver(new ServletResponseMethodArgumentResolver());
8287

8388
ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("httpServletResponse", HttpServletResponse.class);
@@ -88,33 +93,61 @@ public void nullReturnValueHttpServletResponseArg() throws Exception {
8893
}
8994

9095
@Test
91-
public void nullReturnValueRequestNotModified() throws Exception {
96+
public void invokeAndHandle_VoidRequestNotModified() throws Exception {
9297
webRequest.getNativeRequest(MockHttpServletRequest.class).addHeader("If-Modified-Since", 10 * 1000 * 1000);
9398
int lastModifiedTimestamp = 1000 * 1000;
9499
webRequest.checkNotModified(lastModifiedTimestamp);
95-
100+
96101
ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("notModified");
97102
handlerMethod.invokeAndHandle(webRequest, mavContainer);
98103

99104
assertTrue("Null return value + 'not modified' request should result in 'request handled'",
100105
mavContainer.isRequestHandled());
101106
}
102-
107+
108+
// SPR-9159
109+
103110
@Test
104-
public void exceptionWhileHandlingReturnValue() throws Exception {
111+
public void invokeAndHandle_NotVoidWithResponseStatusAndReason() throws Exception {
112+
ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("responseStatusWithReason");
113+
handlerMethod.invokeAndHandle(webRequest, mavContainer);
114+
115+
assertTrue("When a phrase is used, the response should not be used any more", mavContainer.isRequestHandled());
116+
assertEquals(HttpStatus.BAD_REQUEST.value(), response.getStatus());
117+
assertEquals("400 Bad Request", response.getErrorMessage());
118+
}
119+
120+
@Test(expected=HttpMessageNotWritableException.class)
121+
public void invokeAndHandle_Exception() throws Exception {
105122
returnValueHandlers.addHandler(new ExceptionRaisingReturnValueHandler());
106123

107124
ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("handle");
108-
try {
109-
handlerMethod.invokeAndHandle(webRequest, mavContainer);
110-
fail("Expected exception");
111-
} catch (HttpMessageNotWritableException ex) {
112-
// Expected..
113-
// Allow HandlerMethodArgumentResolver exceptions to propagate..
114-
}
125+
handlerMethod.invokeAndHandle(webRequest, mavContainer);
126+
fail("Expected exception");
115127
}
116128

117-
private ServletInvocableHandlerMethod getHandlerMethod(String methodName, Class<?>... argTypes)
129+
@Test
130+
public void invokeAndHandle_DynamicReturnValue() throws Exception {
131+
argumentResolvers.addResolver(new RequestParamMethodArgumentResolver(null, false));
132+
returnValueHandlers.addHandler(new ViewMethodReturnValueHandler());
133+
returnValueHandlers.addHandler(new ViewNameMethodReturnValueHandler());
134+
135+
// Invoke without a request parameter (String return value)
136+
ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("dynamicReturnValue", String.class);
137+
handlerMethod.invokeAndHandle(webRequest, mavContainer);
138+
139+
assertNotNull(mavContainer.getView());
140+
assertEquals(RedirectView.class, mavContainer.getView().getClass());
141+
142+
// Invoke with a request parameter (RedirectView return value)
143+
request.setParameter("param", "value");
144+
handlerMethod.invokeAndHandle(webRequest, mavContainer);
145+
146+
assertEquals("view", mavContainer.getViewName());
147+
}
148+
149+
150+
private ServletInvocableHandlerMethod getHandlerMethod(String methodName, Class<?>... argTypes)
118151
throws NoSuchMethodException {
119152
Method method = Handler.class.getDeclaredMethod(methodName, argTypes);
120153
ServletInvocableHandlerMethod handlerMethod = new ServletInvocableHandlerMethod(new Handler(), method);
@@ -130,16 +163,24 @@ public String handle() {
130163
return "view";
131164
}
132165

133-
@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request")
166+
@ResponseStatus(value = HttpStatus.BAD_REQUEST)
134167
public void responseStatus() {
135168
}
136-
169+
170+
@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request")
171+
public String responseStatusWithReason() {
172+
return "foo";
173+
}
174+
137175
public void httpServletResponse(HttpServletResponse response) {
138176
}
139-
177+
140178
public void notModified() {
141179
}
142-
180+
181+
public Object dynamicReturnValue(@RequestParam(required=false) String param) {
182+
return (param != null) ? "view" : new RedirectView("redirectView");
183+
}
143184
}
144185

145186
private static class ExceptionRaisingReturnValueHandler implements HandlerMethodReturnValueHandler {

0 commit comments

Comments
 (0)