SPR-6144 - @ResponseStatus annotation is ignored in an @Controller redirect (RedirectView)

SPR-5468 - Modify RedirectView to allow 301 Permanent Redirects

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/View.java

@@ -37,11 +37,20 @@ import javax.servlet.http.HttpServletResponse;
  * As this interface is stateless, view implementations should be thread-safe.
  *
  * @author Rod Johnson
+ * @author Arjen Poutsma
  * @see org.springframework.web.servlet.view.AbstractView
  * @see org.springframework.web.servlet.view.InternalResourceView
  */
 public interface View {
 
+	/**
+	 * Name of the {@link HttpServletRequest} attribute that contains the response status code.
+	 * <p>Note: This attribute is not required to be supported by all
+	 * View implementations.
+	 */
+	String RESPONSE_STATUS_ATTRIBUTE = View.class.getName() + ".responseStatus";
+
+
 	/**
 	 * Return the content type of the view, if predetermined.
 	 * <p>Can be used to check the content type upfront,

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

@@ -57,6 +57,7 @@ import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.http.HttpInputMessage;
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.MediaType;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.ByteArrayHttpMessageConverter;
 import org.springframework.http.converter.FormHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConverter;
@@ -708,10 +709,12 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen
 				ExtendedModelMap implicitModel,
 				ServletWebRequest webRequest) throws Exception {
 			
-			ResponseStatus responseStatus = AnnotationUtils.findAnnotation(handlerMethod, ResponseStatus.class);
+			ResponseStatus responseStatusAnn = AnnotationUtils.findAnnotation(handlerMethod, ResponseStatus.class);
-			if (responseStatus != null) {
+			if (responseStatusAnn != null) {
-				HttpServletResponse response = webRequest.getResponse();
+				HttpStatus responseStatus = responseStatusAnn.value();
-				response.setStatus(responseStatus.value().value());
+				// to be picked up by the RedirectView
+				webRequest.getRequest().setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, responseStatus);
+				webRequest.getResponse().setStatus(responseStatus.value());
 				responseArgumentUsed = true;
 			}
 

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java

@@ -32,6 +32,8 @@ import javax.servlet.http.HttpServletResponse;
 import org.springframework.beans.BeanUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.web.util.WebUtils;
+import org.springframework.web.servlet.View;
+import org.springframework.http.HttpStatus;
 
 /**
  * <p>View that redirects to an absolute, context relative, or current request
@@ -63,6 +65,7 @@ import org.springframework.web.util.WebUtils;
  * @author Juergen Hoeller
  * @author Colin Sampaleanu
  * @author Sam Brannen
+ * @author Arjen Poutsma
  * @see #setContextRelative
  * @see #setHttp10Compatible
  * @see #setExposeModelAttributes
@@ -78,6 +81,8 @@ public class RedirectView extends AbstractUrlBasedView {
 
 	private String encodingScheme;
 
+	private HttpStatus statusCode;
+
 
 	/**
 	 * Constructor for use as a bean.
@@ -183,6 +188,14 @@ public class RedirectView extends AbstractUrlBasedView {
 		this.encodingScheme = encodingScheme;
 	}
 
+	/**
+	 * Set the status code for this view.
+	 * <p>Default is to send 302/303, depending on the value of the
+	 * {@link #setHttp10Compatible(boolean) http10Compatible} flag.
+	 */
+	public void setStatusCode(HttpStatus statusCode) {
+		this.statusCode = statusCode;
+	}
 
 	/**
 	 * Convert model to request parameters and redirect to the given URL.
@@ -381,10 +394,29 @@ public class RedirectView extends AbstractUrlBasedView {
 			response.sendRedirect(response.encodeRedirectURL(targetUrl));
 		}
 		else {
-			// Correct HTTP status code is 303, in particular for POST requests.
+			HttpStatus statusCode = getHttp11StatusCode(request, response, targetUrl);
-			response.setStatus(303);
+			response.setStatus(statusCode.value());
 			response.setHeader("Location", response.encodeRedirectURL(targetUrl));
 		}
 	}
 
+	/**
+	 * Determines the status code to use for HTTP 1.1 compatible requests.
+	 * <p>The default implemenetation returns the {@link #setStatusCode(HttpStatus) statusCode}
+	 * property if set, or the value of the {@link #RESPONSE_STATUS_ATTRIBUTE} attribute. If neither are
+	 * set, it defaults to {@link HttpStatus#SEE_OTHER} (303).
+	 * @param request the request to inspect
+	 * @return the response
+	 */
+	protected HttpStatus getHttp11StatusCode(HttpServletRequest request, HttpServletResponse response, String targetUrl) {
+		if (statusCode != null) {
+			return statusCode;
+		}
+		HttpStatus attributeStatusCode = (HttpStatus) request.getAttribute(View.RESPONSE_STATUS_ATTRIBUTE);
+		if (attributeStatusCode != null) {
+			return attributeStatusCode;
+		}
+		return HttpStatus.SEE_OTHER;
+	}
+
 }

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

@@ -117,6 +117,7 @@ import org.springframework.web.servlet.mvc.AbstractController;
 import org.springframework.web.servlet.mvc.multiaction.InternalPathMethodNameResolver;
 import org.springframework.web.servlet.mvc.support.ControllerClassNameHandlerMapping;
 import org.springframework.web.servlet.view.InternalResourceViewResolver;
+import org.springframework.web.servlet.view.RedirectView;
 import org.springframework.web.util.NestedServletException;
 
 /**
@@ -1028,6 +1029,16 @@ public class ServletAnnotationControllerTests {
 		assertEquals(201, response.getStatus());
 	}
 
+	@Test
+	public void responseStatusRedirect() throws ServletException, IOException {
+		initServlet(ResponseStatusRedirectController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/something");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals(201, response.getStatus());
+	}
+
 	@Test
 	public void mavResolver() throws ServletException, IOException {
 		@SuppressWarnings("serial") DispatcherServlet servlet = new DispatcherServlet() {
@@ -1758,6 +1769,16 @@ public class ServletAnnotationControllerTests {
 		}
 	}
 
+	@Controller
+	public static class ResponseStatusRedirectController {
+
+		@RequestMapping("/something")
+		@ResponseStatus(HttpStatus.CREATED)
+		public RedirectView handle(Writer writer) throws IOException {
+			return new RedirectView("somelocation.html", false, false);
+		}
+	}
+
 
 	@Controller
 	public static class ModelAndViewResolverController {

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java

@@ -28,10 +28,15 @@ import javax.servlet.http.HttpServletResponse;
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 import org.easymock.MockControl;
+import static org.easymock.EasyMock.*;
+import org.junit.Test;
+import static org.junit.Assert.*;
 
 import org.springframework.beans.TestBean;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.http.HttpStatus;
+import org.springframework.web.servlet.View;
 
 /**
  * Tests for redirect view, and query string construction.
@@ -40,22 +45,19 @@ import org.springframework.mock.web.MockHttpServletResponse;
  * @author Rod Johnson
  * @author Juergen Hoeller
  * @author Sam Brannen
+ * @author Arjen Poutsma
  * @since 27.05.2003
  */
-public class RedirectViewTests extends TestCase {
+public class RedirectViewTests {
 
-	public void testNoUrlSet() throws Exception {
+	@Test(expected = IllegalArgumentException.class)
+	public void noUrlSet() throws Exception {
 		RedirectView rv = new RedirectView();
-		try {
+		rv.afterPropertiesSet();
-			rv.afterPropertiesSet();
-			fail("Should have thrown IllegalArgumentException");
-		}
-		catch (IllegalArgumentException ex) {
-			// expected
-		}
 	}
 
-	public void testHttp11() throws Exception {
+	@Test
+	public void http11() throws Exception {
 		RedirectView rv = new RedirectView();
 		rv.setUrl("http://url.somewhere.com");
 		rv.setHttp10Compatible(false);
@@ -66,17 +68,46 @@ public class RedirectViewTests extends TestCase {
 		assertEquals("http://url.somewhere.com", response.getHeader("Location"));
 	}
 
-	public void testEmptyMap() throws Exception {
+	@Test
+	public void explicitStatusCode() throws Exception {
+		RedirectView rv = new RedirectView();
+		rv.setUrl("http://url.somewhere.com");
+		rv.setHttp10Compatible(false);
+		rv.setStatusCode(HttpStatus.CREATED);
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		rv.render(new HashMap(), request, response);
+		assertEquals(201, response.getStatus());
+		assertEquals("http://url.somewhere.com", response.getHeader("Location"));
+	}
+
+	@Test
+	public void attributeStatusCode() throws Exception {
+		RedirectView rv = new RedirectView();
+		rv.setUrl("http://url.somewhere.com");
+		rv.setHttp10Compatible(false);
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, HttpStatus.CREATED);
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		rv.render(new HashMap(), request, response);
+		assertEquals(201, response.getStatus());
+		assertEquals("http://url.somewhere.com", response.getHeader("Location"));
+	}
+
+	@Test
+	public void emptyMap() throws Exception {
 		String url = "/myUrl";
 		doTest(new HashMap(), url, false, url);
 	}
 
-	public void testEmptyMapWithContextRelative() throws Exception {
+	@Test
+	public void emptyMapWithContextRelative() throws Exception {
 		String url = "/myUrl";
 		doTest(new HashMap(), url, true, url);
 	}
 
-	public void testSingleParam() throws Exception {
+	@Test
+	public void singleParam() throws Exception {
 		String url = "http://url.somewhere.com";
 		String key = "foo";
 		String val = "bar";
@@ -86,7 +117,8 @@ public class RedirectViewTests extends TestCase {
 		doTest(model, url, false, expectedUrlForEncoding);
 	}
 
-	public void testSingleParamWithoutExposingModelAttributes() throws Exception {
+	@Test
+	public void singleParamWithoutExposingModelAttributes() throws Exception {
 		String url = "http://url.somewhere.com";
 		String key = "foo";
 		String val = "bar";
@@ -96,7 +128,8 @@ public class RedirectViewTests extends TestCase {
 		doTest(model, url, false, false, expectedUrlForEncoding);
 	}
 
-	public void testParamWithAnchor() throws Exception {
+	@Test
+	public void paramWithAnchor() throws Exception {
 		String url = "http://url.somewhere.com/test.htm#myAnchor";
 		String key = "foo";
 		String val = "bar";
@@ -106,7 +139,8 @@ public class RedirectViewTests extends TestCase {
 		doTest(model, url, false, expectedUrlForEncoding);
 	}
 
-	public void testTwoParams() throws Exception {
+	@Test
+	public void twoParams() throws Exception {
 		String url = "http://url.somewhere.com";
 		String key = "foo";
 		String val = "bar";
@@ -126,7 +160,8 @@ public class RedirectViewTests extends TestCase {
 		}
 	}
 
-	public void testArrayParam() throws Exception {
+	@Test
+	public void arrayParam() throws Exception {
 		String url = "http://url.somewhere.com";
 		String key = "foo";
 		String[] val = new String[] {"bar", "baz"};
@@ -143,7 +178,8 @@ public class RedirectViewTests extends TestCase {
 		}
 	}
 
-	public void testCollectionParam() throws Exception {
+	@Test
+	public void collectionParam() throws Exception {
 		String url = "http://url.somewhere.com";
 		String key = "foo";
 		List val = new ArrayList();
@@ -162,7 +198,8 @@ public class RedirectViewTests extends TestCase {
 		}
 	}
 
-	public void testObjectConversion() throws Exception {
+	@Test
+	public void objectConversion() throws Exception {
 		String url = "http://url.somewhere.com";
 		String key = "foo";
 		String val = "bar";
@@ -183,7 +220,7 @@ public class RedirectViewTests extends TestCase {
 		doTest(map, url, contextRelative, true, expectedUrlForEncoding);
 	}
 
-	private void doTest(final Map map, final String url, final boolean contextRelative,
+	private void doTest(final Map<String, ?> map, final String url, final boolean contextRelative,
 			final boolean exposeModelAttributes, String expectedUrlForEncoding) throws Exception {
 
 		class TestRedirectView extends RedirectView {