MarshallingView should not close response OutputStream after copying to it

Also throws IllegalStateException instead of ServletException now, consistent with other Spring MVC classes.

Issue: SPR-11411

Author: jhoeller

spring-webmvc/src/main/java/org/springframework/web/servlet/view/xml/MarshallingView.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,26 +18,24 @@
 
 import java.io.ByteArrayOutputStream;
 import java.util.Map;
-
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.xml.transform.stream.StreamResult;
 
-import org.springframework.beans.BeansException;
 import org.springframework.oxm.Marshaller;
 import org.springframework.util.Assert;
-import org.springframework.util.FileCopyUtils;
+import org.springframework.util.StreamUtils;
 import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.view.AbstractView;
 
 /**
- * Spring-MVC {@link View} that allows for response context to be rendered as the result of marshalling by a {@link
- * Marshaller}.
+ * Spring-MVC {@link View} that allows for response context to be rendered as the result
+ * of marshalling by a {@link Marshaller}.
  *
- * <p>The Object to be marshalled is supplied as a parameter in the model and then {@linkplain
- * #locateToBeMarshalled(Map) detected} during response rendering. Users can either specify a specific entry in the
- * model via the {@link #setModelKey(String) sourceKey} property or have Spring locate the Source object.
+ * <p>The Object to be marshalled is supplied as a parameter in the model and then
+ * {@linkplain #locateToBeMarshalled(Map) detected} during response rendering. Users can
+ * either specify a specific entry in the model via the {@link #setModelKey(String) sourceKey}
+ * property or have Spring locate the Source object.
  *
  * @author Arjen Poutsma
  * @since 3.0
@@ -49,13 +47,15 @@ public class MarshallingView extends AbstractView {
 	 */
 	public static final String DEFAULT_CONTENT_TYPE = "application/xml";
 
+
 	private Marshaller marshaller;
 
 	private String modelKey;
 
+
 	/**
-	 * Constructs a new {@code MarshallingView} with no {@link Marshaller} set. The marshaller must be set after
-	 * construction by invoking {@link #setMarshaller(Marshaller)}.
+	 * Constructs a new {@code MarshallingView} with no {@link Marshaller} set.
+	 * The marshaller must be set after construction by invoking {@link #setMarshaller}.
 	 */
 	public MarshallingView() {
 		setContentType(DEFAULT_CONTENT_TYPE);
@@ -66,80 +66,79 @@ public MarshallingView() {
 	 * Constructs a new {@code MarshallingView} with the given {@link Marshaller} set.
 	 */
 	public MarshallingView(Marshaller marshaller) {
-		Assert.notNull(marshaller, "'marshaller' must not be null");
-		setContentType(DEFAULT_CONTENT_TYPE);
-		this.marshaller = marshaller;
-		setExposePathVariables(false);
+		this();
+		setMarshaller(marshaller);
 	}
 
+
 	/**
 	 * Sets the {@link Marshaller} to be used by this view.
 	 */
 	public void setMarshaller(Marshaller marshaller) {
-		Assert.notNull(marshaller, "'marshaller' must not be null");
+		Assert.notNull(marshaller, "Marshaller must not be null");
 		this.marshaller = marshaller;
 	}
 
 	/**
-	 * Set the name of the model key that represents the object to be marshalled. If not specified, the model map will be
-	 * searched for a supported value type.
-	 *
+	 * Set the name of the model key that represents the object to be marshalled.
+	 * If not specified, the model map will be searched for a supported value type.
 	 * @see Marshaller#supports(Class)
 	 */
 	public void setModelKey(String modelKey) {
 		this.modelKey = modelKey;
 	}
 
 	@Override
-	protected void initApplicationContext() throws BeansException {
-		Assert.notNull(marshaller, "Property 'marshaller' is required");
+	protected void initApplicationContext() {
+		Assert.notNull(this.marshaller, "Property 'marshaller' is required");
 	}
 
+
 	@Override
-	protected void renderMergedOutputModel(Map<String, Object> model,
-										   HttpServletRequest request,
-										   HttpServletResponse response) throws Exception {
+	protected void renderMergedOutputModel(Map<String, Object> model, HttpServletRequest request,
+			HttpServletResponse response) throws Exception {
+
 		Object toBeMarshalled = locateToBeMarshalled(model);
 		if (toBeMarshalled == null) {
-			throw new ServletException("Unable to locate object to be marshalled in model: " + model);
+			throw new IllegalStateException("Unable to locate object to be marshalled in model: " + model);
 		}
 		ByteArrayOutputStream bos = new ByteArrayOutputStream(2048);
-		marshaller.marshal(toBeMarshalled, new StreamResult(bos));
+		this.marshaller.marshal(toBeMarshalled, new StreamResult(bos));
 
 		setResponseContentType(request, response);
 		response.setContentLength(bos.size());
 
-		FileCopyUtils.copy(bos.toByteArray(), response.getOutputStream());
+		StreamUtils.copy(bos.toByteArray(), response.getOutputStream());
 	}
 
 	/**
-	 * Locates the object to be marshalled. The default implementation first attempts to look under the configured
-	 * {@linkplain #setModelKey(String) model key}, if any, before attempting to locate an object of {@linkplain
-	 * Marshaller#supports(Class) supported type}.
-	 *
+	 * Locates the object to be marshalled. The default implementation first attempts to look
+	 * under the configured {@linkplain #setModelKey(String) model key}, if any, before attempting
+	 * to locate an object of {@linkplain Marshaller#supports(Class) supported type}.
 	 * @param model the model Map
 	 * @return the Object to be marshalled (or {@code null} if none found)
-	 * @throws ServletException if the model object specified by the {@linkplain #setModelKey(String) model key} is not
-	 *                          supported by the marshaller
+	 * @throws IllegalStateException if the model object specified by the
+	 * {@linkplain #setModelKey(String) model key} is not supported by the marshaller
 	 * @see #setModelKey(String)
 	 */
-	protected Object locateToBeMarshalled(Map<String, Object> model) throws ServletException {
+	protected Object locateToBeMarshalled(Map<String, Object> model) throws IllegalStateException {
 		if (this.modelKey != null) {
-			Object o = model.get(this.modelKey);
-			if (o == null) {
-				throw new ServletException("Model contains no object with key [" + modelKey + "]");
+			Object obj = model.get(this.modelKey);
+			if (obj == null) {
+				throw new IllegalStateException("Model contains no object with key [" + this.modelKey + "]");
 			}
-			if (!this.marshaller.supports(o.getClass())) {
-				throw new ServletException("Model object [" + o + "] retrieved via key [" + modelKey +
-						"] is not supported by the Marshaller");
+			if (!this.marshaller.supports(obj.getClass())) {
+				throw new IllegalStateException("Model object [" + obj + "] retrieved via key [" +
+						this.modelKey + "] is not supported by the Marshaller");
 			}
-			return o;
+			return obj;
 		}
-		for (Object o : model.values()) {
-			if (o != null && this.marshaller.supports(o.getClass())) {
-				return o;
+		for (Object obj : model.values()) {
+			if (obj != null && this.marshaller.supports(obj.getClass())) {
+				return obj;
 			}
 		}
 		return null;
 	}
+
 }

spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MarshallingViewTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,12 +18,11 @@
 
 import java.util.HashMap;
 import java.util.Map;
-
-import javax.servlet.ServletException;
 import javax.xml.transform.stream.StreamResult;
 
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.mock.web.test.MockHttpServletResponse;
 import org.springframework.oxm.Marshaller;
@@ -93,9 +92,9 @@ public void renderInvalidModelKey() throws Exception {
 
 		try {
 			view.render(model, request, response);
-			fail("ServletException expected");
+			fail("IllegalStateException expected");
 		}
-		catch (ServletException ex) {
+		catch (IllegalStateException ex) {
 			// expected
 		}
 		assertEquals("Invalid content length", 0, response.getContentLength());
@@ -112,9 +111,9 @@ public void renderNullModelValue() throws Exception {
 
 		try {
 			view.render(model, request, response);
-			fail("ServletException expected");
+			fail("IllegalStateException expected");
 		}
-		catch (ServletException ex) {
+		catch (IllegalStateException ex) {
 			// expected
 		}
 		assertEquals("Invalid content length", 0, response.getContentLength());
@@ -135,9 +134,9 @@ public void renderModelKeyUnsupported() throws Exception {
 
 		try {
 			view.render(model, request, response);
-			fail("ServletException expected");
+			fail("IllegalStateException expected");
 		}
-		catch (ServletException ex) {
+		catch (IllegalStateException ex) {
 			// expected
 		}
 	}
@@ -174,9 +173,9 @@ public void testRenderUnsupportedModel() throws Exception {
 
 		try {
 			view.render(model, request, response);
-			fail("ServletException expected");
+			fail("IllegalStateException expected");
 		}
-		catch (ServletException ex) {
+		catch (IllegalStateException ex) {
 			// expected
 		}
 	}