SPR-5536: RestTemplate does not do HTTP GET if it should

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@719 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.web/src/main/java/org/springframework/http/client/CommonsClientHttpRequest.java

@@ -28,8 +28,6 @@ import org.apache.commons.httpclient.methods.RequestEntity;
 
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
-import org.springframework.http.client.AbstractClientHttpRequest;
-import org.springframework.http.client.ClientHttpResponse;
 
 /**
  * {@link org.springframework.http.client.ClientHttpRequest} implementation that uses
@@ -48,7 +46,7 @@ final class CommonsClientHttpRequest extends AbstractClientHttpRequest {
 	private final HttpMethodBase httpMethod;
 
 
-	public CommonsClientHttpRequest(HttpClient httpClient, HttpMethodBase httpMethod) {
+	CommonsClientHttpRequest(HttpClient httpClient, HttpMethodBase httpMethod) {
 		this.httpClient = httpClient;
 		this.httpMethod = httpMethod;
 	}

org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java

@@ -21,9 +21,9 @@ import java.net.HttpURLConnection;
 import java.util.List;
 import java.util.Map;
 
-import org.springframework.util.FileCopyUtils;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
+import org.springframework.util.FileCopyUtils;
 
 /**
  * {@link ClientHttpRequest} implementation that uses standard J2SE facilities to execute requests.
@@ -38,7 +38,7 @@ final class SimpleClientHttpRequest extends AbstractClientHttpRequest {
 	private final HttpURLConnection connection;
 
 
-	public SimpleClientHttpRequest(HttpURLConnection connection) {
+	SimpleClientHttpRequest(HttpURLConnection connection) {
 		this.connection = connection;
 	}
 
@@ -56,7 +56,9 @@ final class SimpleClientHttpRequest extends AbstractClientHttpRequest {
 			}
 		}
 		this.connection.connect();
-		FileCopyUtils.copy(bufferedOutput, this.connection.getOutputStream());
+		if (bufferedOutput.length > 0) {
+			FileCopyUtils.copy(bufferedOutput, this.connection.getOutputStream());
+		}
 		return new SimpleClientHttpResponse(this.connection);
 	}
 

org.springframework.web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java

@@ -21,8 +21,8 @@ import java.net.HttpURLConnection;
 import java.net.URI;
 import java.net.URLConnection;
 
-import org.springframework.util.Assert;
 import org.springframework.http.HttpMethod;
+import org.springframework.util.Assert;
 
 /**
  * {@link ClientHttpRequestFactory} implementation that uses standard J2SE facilities.
@@ -51,7 +51,11 @@ public class SimpleClientHttpRequestFactory implements ClientHttpRequestFactory
 	 */
 	protected void prepareConnection(HttpURLConnection connection, String httpMethod) throws IOException {
 		connection.setDoInput(true);
-		connection.setDoOutput(true);
+		if ("PUT".equals(httpMethod) || "POST".equals(httpMethod)) {
+			connection.setDoOutput(true);
+		} else {
+			connection.setDoOutput(false);
+		}
 		connection.setRequestMethod(httpMethod);
 	}
 

org.springframework.web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTestCase.java

@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.net.URI;
 import java.util.Arrays;
 import java.util.Enumeration;
+import java.util.Locale;
 import javax.servlet.GenericServlet;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
@@ -37,9 +38,9 @@ import org.mortbay.jetty.Server;
 import org.mortbay.jetty.servlet.Context;
 import org.mortbay.jetty.servlet.ServletHolder;
 
-import org.springframework.util.FileCopyUtils;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
+import org.springframework.util.FileCopyUtils;
 
 public abstract class AbstractHttpRequestFactoryTestCase {
 
@@ -53,6 +54,12 @@ public abstract class AbstractHttpRequestFactoryTestCase {
 		Context jettyContext = new Context(jettyServer, "/");
 		jettyContext.addServlet(new ServletHolder(new EchoServlet()), "/echo");
 		jettyContext.addServlet(new ServletHolder(new ErrorServlet(404)), "/errors/notfound");
+		jettyContext.addServlet(new ServletHolder(new MethodServlet("DELETE")), "/methods/delete");
+		jettyContext.addServlet(new ServletHolder(new MethodServlet("GET")), "/methods/get");
+		jettyContext.addServlet(new ServletHolder(new MethodServlet("HEAD")), "/methods/head");
+		jettyContext.addServlet(new ServletHolder(new MethodServlet("OPTIONS")), "/methods/options");
+		jettyContext.addServlet(new ServletHolder(new MethodServlet("POST")), "/methods/post");
+		jettyContext.addServlet(new ServletHolder(new MethodServlet("PUT")), "/methods/put");
 		jettyServer.start();
 	}
 
@@ -104,8 +111,13 @@ public abstract class AbstractHttpRequestFactoryTestCase {
 		ClientHttpRequest request = factory.createRequest(new URI("http://localhost:8889/echo"), HttpMethod.POST);
 		byte[] body = "Hello World".getBytes("UTF-8");
 		FileCopyUtils.copy(body, request.getBody());
-		request.execute();
-		FileCopyUtils.copy(body, request.getBody());
+		ClientHttpResponse response = request.execute();
+		try {
+			FileCopyUtils.copy(body, request.getBody());
+		}
+		finally {
+			response.close();
+		}
 	}
 
 	@Test(expected = IllegalStateException.class)
@@ -114,13 +126,40 @@ public abstract class AbstractHttpRequestFactoryTestCase {
 		request.getHeaders().add("MyHeader", "value");
 		byte[] body = "Hello World".getBytes("UTF-8");
 		FileCopyUtils.copy(body, request.getBody());
-		request.execute();
-		request.getHeaders().add("MyHeader", "value");
+		ClientHttpResponse response = request.execute();
+		try {
+			request.getHeaders().add("MyHeader", "value");
+		}
+		finally {
+			response.close();
+		}
+	}
+
+	@Test
+	public void httpMethods() throws Exception {
+		assertHttpMethod("get", HttpMethod.GET);
+		assertHttpMethod("head", HttpMethod.HEAD);
+		assertHttpMethod("post", HttpMethod.POST);
+		assertHttpMethod("put", HttpMethod.PUT);
+		assertHttpMethod("options", HttpMethod.OPTIONS);
+		assertHttpMethod("delete", HttpMethod.DELETE);
 	}
 
-	/**
-	 * Servlet that returns and error message for a given status code.
-	 */
+	private void assertHttpMethod(String path, HttpMethod method) throws Exception {
+		ClientHttpResponse response = null;
+		try {
+			ClientHttpRequest request = factory.createRequest(new URI("http://localhost:8889/methods/" + path), method);
+			response = request.execute();
+			assertEquals("Invalid method", path.toUpperCase(Locale.ENGLISH), request.getMethod().name());
+		}
+		finally {
+			if (response != null) {
+				response.close();
+			}
+		}
+	}
+
+	/** Servlet that returns and error message for a given status code. */
 	private static class ErrorServlet extends GenericServlet {
 
 		private final int sc;
@@ -135,6 +174,21 @@ public abstract class AbstractHttpRequestFactoryTestCase {
 		}
 	}
 
+	private static class MethodServlet extends GenericServlet {
+
+		private final String method;
+
+		private MethodServlet(String method) {
+			this.method = method;
+		}
+
+		@Override
+		public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException {
+			HttpServletRequest httpReq = (HttpServletRequest) req;
+			assertEquals("Invalid HTTP method", method, httpReq.getMethod());
+		}
+	}
+
 	private static class EchoServlet extends HttpServlet {
 
 		@Override