CR

palantir · May 29, 2020 · 6506ec4 · 6506ec4
1 parent 05822a1
commit 6506ec4
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 12 deletions.
diff --git a/...e-hc4-client/src/main/java/com/palantir/dialogue/hc4/ApacheHttpClientBlockingChannel.java b/...e-hc4-client/src/main/java/com/palantir/dialogue/hc4/ApacheHttpClientBlockingChannel.java
@@ -74,6 +74,8 @@ public Response execute(Endpoint endpoint, Request request) throws IOException {
                     endpoint.httpMethod() != HttpMethod.GET, "GET endpoints must not have a request body");
             Preconditions.checkArgument(
                     endpoint.httpMethod() != HttpMethod.HEAD, "HEAD endpoints must not have a request body");
+            Preconditions.checkArgument(
+                    endpoint.httpMethod() != HttpMethod.OPTIONS, "OPTIONS endpoints must not have a request body");
             RequestBody body = request.body().get();
             setBody(builder, body);
         }

diff --git a/...c/main/java/com/palantir/dialogue/httpurlconnection/HttpUrlConnectionBlockingChannel.java b/...c/main/java/com/palantir/dialogue/httpurlconnection/HttpUrlConnectionBlockingChannel.java
@@ -85,6 +85,8 @@ public Response execute(Endpoint endpoint, Request request) throws IOException {
                     endpoint.httpMethod() != HttpMethod.GET, "GET endpoints must not have a request body");
             Preconditions.checkArgument(
                     endpoint.httpMethod() != HttpMethod.HEAD, "HEAD endpoints must not have a request body");
+            Preconditions.checkArgument(
+                    endpoint.httpMethod() != HttpMethod.OPTIONS, "OPTIONS endpoints must not have a request body");
             RequestBody body = request.body().get();
             connection.setChunkedStreamingMode(1024 * 8);
             connection.setRequestProperty("content-type", body.contentType());

diff --git a/dialogue-java-client/src/main/java/com/palantir/dialogue/HttpChannel.java b/dialogue-java-client/src/main/java/com/palantir/dialogue/HttpChannel.java
@@ -68,6 +68,9 @@ public ListenableFuture<Response> execute(Endpoint endpoint, Request request) {
         Preconditions.checkArgument(
                 !(request.body().isPresent() && endpoint.httpMethod() == HttpMethod.HEAD),
                 "HEAD endpoints must not have a request body");
+        Preconditions.checkArgument(
+                !(request.body().isPresent() && endpoint.httpMethod() == HttpMethod.OPTIONS),
+                "OPTIONS endpoints must not have a request body");
         httpRequest.method(endpoint.httpMethod().name(), toBody(request));
 
         // Fill headers

diff --git a/dialogue-okhttp-client/src/main/java/com/palantir/dialogue/OkHttpChannel.java b/dialogue-okhttp-client/src/main/java/com/palantir/dialogue/OkHttpChannel.java
@@ -80,6 +80,11 @@ public ListenableFuture<Response> execute(Endpoint endpoint, Request request) {
                 Preconditions.checkArgument(!request.body().isPresent(), "HEAD endpoints must not have a request body");
                 okRequest = okRequest.head();
                 break;
+            case OPTIONS:
+                Preconditions.checkArgument(
+                        !request.body().isPresent(), "OPTIONS endpoints must not have a request body");
+                okRequest = okRequest.method("OPTIONS", null);
+                break;
             case POST:
                 okRequest = okRequest.post(toOkHttpBody(request.body()));
                 break;
@@ -93,11 +98,6 @@ public ListenableFuture<Response> execute(Endpoint endpoint, Request request) {
             case PATCH:
                 okRequest = okRequest.patch(toOkHttpBody(request.body()));
                 break;
-            case OPTIONS:
-                Preconditions.checkArgument(
-                        !request.body().isPresent(), "OPTIONS endpoints must not have a request body");
-                okRequest = okRequest.method("OPTIONS", null);
-                break;
         }
 
         // Fill headers

diff --git a/dialogue-target/src/main/java/com/palantir/dialogue/HttpMethod.java b/dialogue-target/src/main/java/com/palantir/dialogue/HttpMethod.java
@@ -18,11 +18,11 @@
 
 /** The HTTP methods supported by Dialogue. */
 public enum HttpMethod {
-    DELETE,
     GET,
     HEAD,
+    OPTIONS,
     PATCH,
     POST,
     PUT,
-    OPTIONS
+    DELETE,
 }
diff --git a/dialogue-test-common/src/main/java/com/palantir/dialogue/AbstractChannelTest.java b/dialogue-test-common/src/main/java/com/palantir/dialogue/AbstractChannelTest.java
@@ -227,6 +227,14 @@ public void head_failsWhenBodyIsGiven() {
                 .hasMessageContaining("HEAD endpoints must not have a request body");
     }
 
+    @Test
+    public void options_failsWhenBodyIsGiven() {
+        endpoint.method = HttpMethod.OPTIONS;
+        request = Request.builder().from(request).body(body).build();
+        assertThatThrownBy(() -> channel.execute(endpoint, request).get())
+                .hasMessageContaining("OPTIONS endpoints must not have a request body");
+    }
+
     @Test
     public void head_failsWhenBodyReturned() throws ExecutionException, InterruptedException {
         endpoint.method = HttpMethod.HEAD;

diff --git a/simulation/src/main/java/com/palantir/dialogue/core/SimulationServer.java b/simulation/src/main/java/com/palantir/dialogue/core/SimulationServer.java
@@ -25,7 +25,6 @@
 import com.google.common.util.concurrent.MoreExecutors;
 import com.palantir.dialogue.Channel;
 import com.palantir.dialogue.Endpoint;
-import com.palantir.dialogue.HttpMethod;
 import com.palantir.dialogue.Request;
 import com.palantir.dialogue.Response;
 import com.palantir.dialogue.TestResponse;
@@ -75,10 +74,6 @@ public static Builder builder() {
 
     @Override
     public ListenableFuture<Response> execute(Endpoint endpoint, Request request) {
-        if (endpoint.httpMethod() == HttpMethod.OPTIONS) {
-            return Futures.immediateFuture(new TestResponse().code(204));
-        }
-
         Meter perEndpointRequests = MetricNames.requestMeter(simulation.taggedMetrics(), serverName, endpoint);
 
         activeRequests.inc();