From 9b550f7293f6e5f078437ba4f0d5ac65d6bc59f5 Mon Sep 17 00:00:00 2001
From: Spencer Gibb <spencer@gibb.us>
Date: Wed, 31 Aug 2016 11:43:05 -0600
Subject: [PATCH] Support delete with body in zuul where possible.

The apache http client and ok http client support delete with body, ribbon rest client does not.

fixes gh-949
---
 .../route/RestClientRibbonCommand.java        | 10 ++++-
 .../filters/route/RibbonRoutingFilter.java    |  4 --
 .../route/RestClientRibbonCommandTests.java   | 21 ++++++++---
 ...tpClientRibbonCommandIntegrationTests.java |  5 ---
 .../OkHttpRibbonCommandIntegrationTests.java  |  5 ---
 ...stClientRibbonCommandIntegrationTests.java |  5 +++
 .../route/support/ZuulProxyTestBase.java      | 37 +++++++++++++++++--
 7 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java
index b5423ce7..d8fb01ed 100644
--- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java
+++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java
@@ -55,9 +55,17 @@ public class RestClientRibbonCommand extends AbstractRibbonCommand<RestClient, H
 
 	@Override
 	protected HttpRequest createRequest() throws Exception {
+		final InputStream requestEntity;
+		// ApacheHttpClient4Handler does not support body in delete requests
+		if (getContext().getMethod().equalsIgnoreCase("DELETE")) {
+			requestEntity = null;
+		} else {
+			requestEntity = this.context.getRequestEntity();
+		}
+
 		HttpRequest.Builder builder = HttpRequest.newBuilder()
 				.verb(getVerb(this.context.getMethod())).uri(this.context.uri())
-				.entity(this.context.getRequestEntity());
+				.entity(requestEntity);
 
 		if (this.context.getRetryable() != null) {
 			builder.setRetriable(this.context.getRetryable());
diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java
index 2df7c7bd..86d9b940 100644
--- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java
+++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java
@@ -176,10 +176,6 @@ public class RibbonRoutingFilter extends ZuulFilter {
 
 	protected InputStream getRequestBody(HttpServletRequest request) {
 		InputStream requestEntity = null;
-		// ApacheHttpClient4Handler does not support body in delete requests
-		if (request.getMethod().equals("DELETE")) {
-			return null;
-		}
 		try {
 			requestEntity = (InputStream) RequestContext.getCurrentContext()
 					.get("requestEntity");
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java
index 7b0b4614..2fc81a33 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java
@@ -21,6 +21,7 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.startsWith;
+import static org.hamcrest.core.IsNull.nullValue;
 import static org.junit.Assert.assertThat;
 
 import java.io.ByteArrayInputStream;
@@ -116,6 +117,12 @@ public class RestClientRibbonCommandTests {
 		testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, "POST");
 	}
 
+	@Test
+	public void testNonEmptyEntityDelete() throws Exception {
+		String entityValue = "abcd";
+		testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, "DELETE");
+	}
+
 	void testEntity(String entityValue, ByteArrayInputStream requestEntity, boolean addContentLengthHeader, String method) throws Exception {
 		String lengthString = String.valueOf(entityValue.length());
 		Long length = null;
@@ -154,10 +161,14 @@ public class RestClientRibbonCommandTests {
 				is(equalTo("foo")));
 
 
-		assertThat("entity is missing", request.getEntity(), is(notNullValue()));
-		assertThat("entity is wrong type", InputStream.class.isAssignableFrom(request.getEntity().getClass()), is(true));
-		InputStream entity = (InputStream) request.getEntity();
-		String string = StreamUtils.copyToString(entity, Charset.forName("UTF-8"));
-		assertThat("content is wrong", string, is(equalTo(entityValue)));
+		if (method.equalsIgnoreCase("DELETE")) {
+			assertThat("entity is was non-null", request.getEntity(), is(nullValue()));
+		} else {
+			assertThat("entity is missing", request.getEntity(), is(notNullValue()));
+			assertThat("entity is wrong type", InputStream.class.isAssignableFrom(request.getEntity().getClass()), is(true));
+			InputStream entity = (InputStream) request.getEntity();
+			String string = StreamUtils.copyToString(entity, Charset.forName("UTF-8"));
+			assertThat("content is wrong", string, is(equalTo(entityValue)));
+		}
 	}
 }
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java
index e6765192..f8ff51f5 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java
@@ -77,11 +77,6 @@ import com.netflix.loadbalancer.ServerList;
 @DirtiesContext
 public class HttpClientRibbonCommandIntegrationTests extends ZuulProxyTestBase {
 
-	@Override
-	protected boolean supportsPatch() {
-		return true;
-	}
-
 	@Before
 	public void init() {
 		super.setTestRequestcontext();
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java
index 023e7abe..80f06806 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java
@@ -52,11 +52,6 @@ import org.springframework.web.bind.annotation.RestController;
 @DirtiesContext
 public class OkHttpRibbonCommandIntegrationTests extends ZuulProxyTestBase {
 
-	@Override
-	protected boolean supportsPatch() {
-		return true;
-	}
-
 	@Before
 	public void init() {
 		super.setTestRequestcontext();
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java
index 3a88e5c5..c70e49fc 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java
@@ -100,6 +100,11 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase {
 		return false;
 	}
 
+	@Override
+	protected boolean supportsDeleteWithBody() {
+		return false;
+	}
+
 	@Test
 	public void simpleHostRouteWithTrailingSlash() {
 		this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/");
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java
index d4651e2f..9119c4a3 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java
@@ -103,6 +103,20 @@ public abstract class ZuulProxyTestBase {
 		RequestContext.getCurrentContext().unset();
 	}
 
+	/**
+	 * used to disable patch tests if client doesn't support it
+	 */
+	protected boolean supportsPatch() {
+		return true;
+	}
+
+	/**
+	 * used to switch delete tests with a boyd if client doesn't support it
+	 */
+	protected boolean supportsDeleteWithBody() {
+		return true;
+	}
+
 	protected String getRoute(String path) {
 		for (Route route : this.routes.getRoutes()) {
 			if (path.equals(route.getFullPath())) {
@@ -192,6 +206,20 @@ public abstract class ZuulProxyTestBase {
 		assertFalse(myErrorController.wasControllerUsed());
 	}
 
+	@Test
+	public void ribbonDeleteWithBody() {
+		this.endpoint.reset();
+		ResponseEntity<String> result = new TestRestTemplate().exchange(
+				"http://localhost:" + this.port + "/simple/deletewithbody", HttpMethod.DELETE,
+				new HttpEntity<>("deleterequestbody"), String.class);
+		assertEquals(HttpStatus.OK, result.getStatusCode());
+		if (supportsDeleteWithBody()) {
+			assertEquals("Deleted deleterequestbody", result.getBody());
+		} else {
+			assertEquals("Deleted null", result.getBody());
+		}
+	}
+
 	@Test
 	public void ribbonRouteWithNonExistentUri() {
 		String uri = "/simple/nonExistent";
@@ -272,8 +300,6 @@ public abstract class ZuulProxyTestBase {
 		assertEquals("Posted [(bar)] and Content-Length was: 13!", result.getBody());
 	}
 
-	protected abstract boolean supportsPatch();
-
 	public static abstract class AbstractZuulProxyApplication
 			extends DelegatingWebMvcConfiguration {
 
@@ -297,7 +323,12 @@ public abstract class ZuulProxyTestBase {
 		public String postWithFormParam(HttpServletRequest request, 
 				@RequestBody MultiValueMap<String, String> body) {
 			return "Posted " + body.get("foo") + " and Content-Length was: " + request.getContentLength() + "!";
-		}		
+		}
+
+		@RequestMapping(value = "/deletewithbody", method = RequestMethod.DELETE)
+		public String deleteWithBody(@RequestBody(required = false) String body) {
+			return "Deleted " + body;
+		}
 
 		@RequestMapping(value = "/local/{id}", method = RequestMethod.DELETE)
 		public String delete(@PathVariable String id) {