Adds support for per request timeout options. Fixes OpenFeign#562 (OpenFeign#970)

meifans · kdavisk6 · commit 23ee09ef88a0 · 2019-05-31T10:38:49.000-04:00
* Add Options UT

* Ignore Options when set bodyIndex
diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java
@@ -13,9 +13,6 @@
  */
 package feign;
 
-import static feign.Util.checkState;
-import static feign.Util.emptyToNull;
-import feign.Request.HttpMethod;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -29,6 +26,9 @@
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import feign.Request.HttpMethod;
+import static feign.Util.checkState;
+import static feign.Util.emptyToNull;
 
 /**
  * Defines what annotations and values are valid on interfaces.
@@ -111,7 +111,7 @@ protected MethodMetadata parseAndValidateMetadata(Class<?> targetType, Method me
         }
         if (parameterTypes[i] == URI.class) {
           data.urlIndex(i);
-        } else if (!isHttpAnnotation) {
+        } else if (!isHttpAnnotation && parameterTypes[i] != Request.Options.class) {
           checkState(data.formParams().isEmpty(),
               "Body parameters cannot be used with form parameters.");
           checkState(data.bodyIndex() == null, "Method has too many Body parameters: %s", method);
diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java
@@ -16,6 +16,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
 import feign.InvocationHandlerFactory.MethodHandler;
 import feign.Request.Options;
 import feign.codec.DecodeException;
@@ -72,10 +73,11 @@ private SynchronousMethodHandler(Target<?> target, Client client, Retryer retrye
   @Override
   public Object invoke(Object[] argv) throws Throwable {
     RequestTemplate template = buildTemplateFromArgs.create(argv);
+    Options options = findOptions(argv);
     Retryer retryer = this.retryer.clone();
     while (true) {
       try {
-        return executeAndDecode(template);
+        return executeAndDecode(template, options);
       } catch (RetryableException e) {
         try {
           retryer.continueOrPropagate(e);
@@ -95,7 +97,7 @@ public Object invoke(Object[] argv) throws Throwable {
     }
   }
 
-  Object executeAndDecode(RequestTemplate template) throws Throwable {
+  Object executeAndDecode(RequestTemplate template, Options options) throws Throwable {
     Request request = targetRequest(template);
 
     if (logLevel != Logger.Level.NONE) {
@@ -181,6 +183,16 @@ Object decode(Response response) throws Throwable {
     }
   }
 
+  Options findOptions(Object[] argv) {
+    if (argv == null || argv.length == 0) {
+      return this.options;
+    }
+    return (Options) Stream.of(argv)
+        .filter(o -> o instanceof Options)
+        .findFirst()
+        .orElse(this.options);
+  }
+
   static class Factory {
 
     private final Client client;
diff --git a/core/src/test/java/feign/OptionsTest.java b/core/src/test/java/feign/OptionsTest.java
@@ -0,0 +1,68 @@
+/**
+ * Copyright 2012-2019 The Feign Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package feign;
+
+import org.hamcrest.CoreMatchers;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import java.net.SocketTimeoutException;
+import java.util.concurrent.TimeUnit;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author pengfei.zhao
+ */
+public class OptionsTest {
+
+  interface OptionsInterface {
+    @RequestLine("GET /")
+    String get(Request.Options options);
+
+    @RequestLine("GET /")
+    String get();
+  }
+
+  @Rule
+  public final ExpectedException thrown = ExpectedException.none();
+
+  @Test
+  public void socketTimeoutTest() {
+    final MockWebServer server = new MockWebServer();
+    server.enqueue(new MockResponse().setBody("foo").setBodyDelay(3, TimeUnit.SECONDS));
+
+    final OptionsInterface api = Feign.builder()
+        .options(new Request.Options(1000, 1000))
+        .target(OptionsInterface.class, server.url("/").toString());
+
+    thrown.expect(FeignException.class);
+    thrown.expectCause(CoreMatchers.isA(SocketTimeoutException.class));
+
+    api.get();
+  }
+
+  @Test
+  public void normalResponseTest() {
+    final MockWebServer server = new MockWebServer();
+    server.enqueue(new MockResponse().setBody("foo").setBodyDelay(3, TimeUnit.SECONDS));
+
+    final OptionsInterface api = Feign.builder()
+        .options(new Request.Options(1000, 1000))
+        .target(OptionsInterface.class, server.url("/").toString());
+
+    assertThat(api.get(new Request.Options(1000, 4 * 1000))).isEqualTo("foo");
+  }
+}
