Refactor async feign (#1755)

* Add MethodInfoResolver to customize MethodInfo creation logic

* Add methodInfoResolver setter to AsyncBuilder

* Refactor CoroutineFeign to use AsyncFeignBuilder instead of FeignBuilder

* Deprecate CoroutineFeign.coBuilder

* Change AsyncFeign to not inherit Feign

* Deprecate AsyncFeign.asyncBuilder

* Refactor AsyncBuilder to build Feign directly

Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>

Author: wplong11

README.md

@@ -1077,7 +1077,7 @@ interface GitHub {
 
 public class MyApp {
   public static void main(String... args) {
-    GitHub github = AsyncFeign.asyncBuilder()
+    GitHub github = AsyncFeign.builder()
                          .decoder(new GsonDecoder())
                          .target(GitHub.class, "https://api.github.com");
 

core/src/main/java/feign/AsyncFeign.java

@@ -13,6 +13,7 @@
  */
 package feign;
 
+import feign.ReflectiveFeign.ParseHandlersByName;
 import feign.Logger.Level;
 import feign.Request.Options;
 import feign.Target.HardCodedTarget;
@@ -44,10 +45,17 @@
  * be done (for example, creating and submitting a task to an {@link ExecutorService}).
  */
 @Experimental
-public abstract class AsyncFeign<C> extends Feign {
+public abstract class AsyncFeign<C> {
+  public static <C> AsyncBuilder<C> builder() {
+    return new AsyncBuilder<>();
+  }
 
+  /**
+   * @deprecated use {@link #builder()} instead.
+   */
+  @Deprecated()
   public static <C> AsyncBuilder<C> asyncBuilder() {
-    return new AsyncBuilder<>();
+    return builder();
   }
 
   private static class LazyInitializedExecutorService {
@@ -66,6 +74,7 @@ public static class AsyncBuilder<C> extends BaseBuilder<AsyncBuilder<C>> {
     private AsyncContextSupplier<C> defaultContextSupplier = () -> null;
     private AsyncClient<C> client = new AsyncClient.Default<>(
         new Client.Default(null, null), LazyInitializedExecutorService.instance);
+    private MethodInfoResolver methodInfoResolver = MethodInfo::new;
 
     @Deprecated
     public AsyncBuilder<C> defaultContextSupplier(Supplier<C> supplier) {
@@ -78,6 +87,11 @@ public AsyncBuilder<C> client(AsyncClient<C> client) {
       return this;
     }
 
+    public AsyncBuilder<C> methodInfoResolver(MethodInfoResolver methodInfoResolver) {
+      this.methodInfoResolver = methodInfoResolver;
+      return this;
+    }
+
     @Override
     public AsyncBuilder<C> mapAndDecode(ResponseMapper mapper, Decoder decoder) {
       return super.mapAndDecode(mapper, decoder);
@@ -191,21 +205,19 @@ public AsyncFeign<C> build() {
               AsyncResponseHandler.class,
               capabilities);
 
-
-      return new ReflectiveAsyncFeign<>(Feign.builder()
-          .logLevel(logLevel)
-          .client(stageExecution(activeContextHolder, client))
-          .decoder(stageDecode(activeContextHolder, logger, logLevel, responseHandler))
-          .forceDecoding() // force all handling through stageDecode
-          .contract(contract)
-          .logger(logger)
-          .encoder(encoder)
-          .queryMapEncoder(queryMapEncoder)
-          .options(options)
-          .requestInterceptors(requestInterceptors)
-          .responseInterceptor(responseInterceptor)
-          .invocationHandlerFactory(invocationHandlerFactory)
-          .build(), defaultContextSupplier, activeContextHolder);
+      final SynchronousMethodHandler.Factory synchronousMethodHandlerFactory =
+          new SynchronousMethodHandler.Factory(stageExecution(activeContextHolder, client), retryer,
+              requestInterceptors,
+              responseInterceptor, logger, logLevel, dismiss404, closeAfterDecode,
+              propagationPolicy, true);
+      final ParseHandlersByName handlersByName =
+          new ParseHandlersByName(contract, options, encoder,
+              stageDecode(activeContextHolder, logger, logLevel, responseHandler), queryMapEncoder,
+              errorDecoder, synchronousMethodHandlerFactory);
+      final ReflectiveFeign feign =
+          new ReflectiveFeign(handlersByName, invocationHandlerFactory, queryMapEncoder);
+      return new ReflectiveAsyncFeign<>(feign, defaultContextSupplier, activeContextHolder,
+          methodInfoResolver);
     }
 
     private Client stageExecution(
@@ -293,7 +305,6 @@ protected AsyncFeign(Feign feign, AsyncContextSupplier<C> defaultContextSupplier
     this.defaultContextSupplier = defaultContextSupplier;
   }
 
-  @Override
   public <T> T newInstance(Target<T> target) {
     return newInstance(target, defaultContextSupplier.newContext());
   }

core/src/main/java/feign/Capability.java

@@ -133,4 +133,8 @@ default AsyncResponseHandler enrich(AsyncResponseHandler asyncResponseHandler) {
   default <C> AsyncContextSupplier<C> enrich(AsyncContextSupplier<C> asyncContextSupplier) {
     return asyncContextSupplier;
   }
+
+  default MethodInfoResolver enrich(MethodInfoResolver methodInfoResolver) {
+    return methodInfoResolver;
+  }
 }

core/src/main/java/feign/MethodInfoResolver.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2012-2022 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 java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.concurrent.CompletableFuture;
+
+@Experimental
+public interface MethodInfoResolver {
+  public MethodInfo resolve(Class<?> targetType, Method method);
+}

core/src/main/java/feign/ReflectiveAsyncFeign.java

@@ -59,7 +59,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
       }
 
       final MethodInfo methodInfo =
-          methodInfoLookup.computeIfAbsent(method, m -> new MethodInfo(type, m));
+          methodInfoLookup.computeIfAbsent(method, m -> methodInfoResolver.resolve(type, m));
 
       setInvocationContext(new AsyncInvocation<>(context, methodInfo));
       try {
@@ -97,11 +97,13 @@ public String toString() {
   }
 
   private ThreadLocal<AsyncInvocation<C>> activeContextHolder;
+  private final MethodInfoResolver methodInfoResolver;
 
   public ReflectiveAsyncFeign(Feign feign, AsyncContextSupplier<C> defaultContextSupplier,
-      ThreadLocal<AsyncInvocation<C>> contextHolder) {
+      ThreadLocal<AsyncInvocation<C>> contextHolder, MethodInfoResolver methodInfoResolver) {
     super(feign, defaultContextSupplier);
     this.activeContextHolder = contextHolder;
+    this.methodInfoResolver = methodInfoResolver;
   }
 
   protected void setInvocationContext(AsyncInvocation<C> invocationContext) {

core/src/test/java/feign/AsyncFeignTest.java

@@ -503,7 +503,7 @@ public void doesntRetryAfterResponseIsSent() throws Throwable {
   public void throwsFeignExceptionIncludingBody() throws Throwable {
     server.enqueue(new MockResponse().setBody("success!"));
 
-    TestInterfaceAsync api = AsyncFeign.asyncBuilder().decoder((response, type) -> {
+    TestInterfaceAsync api = AsyncFeign.builder().decoder((response, type) -> {
       throw new IOException("timeout");
     }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
@@ -524,7 +524,7 @@ public void throwsFeignExceptionIncludingBody() throws Throwable {
   public void throwsFeignExceptionWithoutBody() {
     server.enqueue(new MockResponse().setBody("success!"));
 
-    TestInterfaceAsync api = AsyncFeign.asyncBuilder().decoder((response, type) -> {
+    TestInterfaceAsync api = AsyncFeign.builder().decoder((response, type) -> {
       throw new IOException("timeout");
     }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
@@ -549,7 +549,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() throws Throwable {
     ExecutorService execs = Executors.newSingleThreadExecutor();
 
     // fake client as Client.Default follows redirects.
-    TestInterfaceAsync api = AsyncFeign.<Void>asyncBuilder()
+    TestInterfaceAsync api = AsyncFeign.<Void>builder()
         .client(new AsyncClient.Default<>((request, options) -> response, execs))
         .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
@@ -625,10 +625,10 @@ public void equalsHashCodeAndToStringWork() {
     Target<OtherTestInterfaceAsync> t3 =
         new HardCodedTarget<>(OtherTestInterfaceAsync.class,
             "http://localhost:8080");
-    TestInterfaceAsync i1 = AsyncFeign.asyncBuilder().target(t1);
-    TestInterfaceAsync i2 = AsyncFeign.asyncBuilder().target(t1);
-    TestInterfaceAsync i3 = AsyncFeign.asyncBuilder().target(t2);
-    OtherTestInterfaceAsync i4 = AsyncFeign.asyncBuilder().target(t3);
+    TestInterfaceAsync i1 = AsyncFeign.builder().target(t1);
+    TestInterfaceAsync i2 = AsyncFeign.builder().target(t1);
+    TestInterfaceAsync i3 = AsyncFeign.builder().target(t2);
+    OtherTestInterfaceAsync i4 = AsyncFeign.builder().target(t3);
 
     assertThat(i1).isEqualTo(i2).isNotEqualTo(i3).isNotEqualTo(i4);
 
@@ -651,7 +651,7 @@ public void decodeLogicSupportsByteArray() throws Throwable {
     byte[] expectedResponse = {12, 34, 56};
     server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse)));
 
-    OtherTestInterfaceAsync api = AsyncFeign.asyncBuilder().target(OtherTestInterfaceAsync.class,
+    OtherTestInterfaceAsync api = AsyncFeign.builder().target(OtherTestInterfaceAsync.class,
         "http://localhost:" + server.getPort());
 
     assertThat(unwrap(api.binaryResponseBody())).containsExactly(expectedResponse);
@@ -662,7 +662,7 @@ public void encodeLogicSupportsByteArray() throws Exception {
     byte[] expectedRequest = {12, 34, 56};
     server.enqueue(new MockResponse());
 
-    OtherTestInterfaceAsync api = AsyncFeign.asyncBuilder().target(OtherTestInterfaceAsync.class,
+    OtherTestInterfaceAsync api = AsyncFeign.builder().target(OtherTestInterfaceAsync.class,
         "http://localhost:" + server.getPort());
 
     CompletableFuture<?> cf = api.binaryRequestBody(expectedRequest);
@@ -733,7 +733,7 @@ public void mapAndDecodeExecutesMapFunction() throws Throwable {
     server.enqueue(new MockResponse().setBody("response!"));
 
     TestInterfaceAsync api =
-        AsyncFeign.asyncBuilder().mapAndDecode(upperCaseResponseMapper(), new StringDecoder())
+        AsyncFeign.builder().mapAndDecode(upperCaseResponseMapper(), new StringDecoder())
             .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
     assertEquals("RESPONSE!", unwrap(api.post()));
@@ -962,7 +962,7 @@ public Exception decode(String methodKey, Response response) {
 
   static final class TestInterfaceAsyncBuilder {
 
-    private final AsyncFeign.AsyncBuilder<Void> delegate = AsyncFeign.<Void>asyncBuilder()
+    private final AsyncFeign.AsyncBuilder<Void> delegate = AsyncFeign.<Void>builder()
         .decoder(new Decoder.Default()).encoder(new Encoder() {
 
           @SuppressWarnings("deprecation")
@@ -1018,31 +1018,31 @@ TestInterfaceAsync target(String url) {
   @Test
   public void testNonInterface() {
     thrown.expect(IllegalArgumentException.class);
-    AsyncFeign.asyncBuilder().target(NonInterface.class, "http://localhost");
+    AsyncFeign.builder().target(NonInterface.class, "http://localhost");
   }
 
   @Test
   public void testExtendedCFReturnType() {
     thrown.expect(IllegalArgumentException.class);
-    AsyncFeign.asyncBuilder().target(ExtendedCFApi.class, "http://localhost");
+    AsyncFeign.builder().target(ExtendedCFApi.class, "http://localhost");
   }
 
   @Test
   public void testLowerWildReturnType() {
     thrown.expect(IllegalArgumentException.class);
-    AsyncFeign.asyncBuilder().target(LowerWildApi.class, "http://localhost");
+    AsyncFeign.builder().target(LowerWildApi.class, "http://localhost");
   }
 
   @Test
   public void testUpperWildReturnType() {
     thrown.expect(IllegalArgumentException.class);
-    AsyncFeign.asyncBuilder().target(UpperWildApi.class, "http://localhost");
+    AsyncFeign.builder().target(UpperWildApi.class, "http://localhost");
   }
 
   @Test
   public void testrWildReturnType() {
     thrown.expect(IllegalArgumentException.class);
-    AsyncFeign.asyncBuilder().target(WildApi.class, "http://localhost");
+    AsyncFeign.builder().target(WildApi.class, "http://localhost");
   }
 
 

core/src/test/java/feign/BaseBuilderTest.java

@@ -26,8 +26,8 @@ public class BaseBuilderTest {
   @Test
   public void checkEnrichTouchesAllAsyncBuilderFields()
       throws IllegalArgumentException, IllegalAccessException {
-    test(AsyncFeign.asyncBuilder().requestInterceptor(template -> {
-    }), 13);
+    test(AsyncFeign.builder().requestInterceptor(template -> {
+    }), 14);
   }
 
   private void test(BaseBuilder<?> builder, int expectedFieldsCount)

core/src/test/java/feign/FeignUnderAsyncTest.java

@@ -487,7 +487,7 @@ public Object decode(Response response, Type type) throws IOException {
   public void throwsFeignExceptionIncludingBody() {
     server.enqueue(new MockResponse().setBody("success!"));
 
-    TestInterface api = AsyncFeign.asyncBuilder()
+    TestInterface api = AsyncFeign.builder()
         .decoder((response, type) -> {
           throw new IOException("timeout");
         })
@@ -506,7 +506,7 @@ public void throwsFeignExceptionIncludingBody() {
   public void throwsFeignExceptionWithoutBody() {
     server.enqueue(new MockResponse().setBody("success!"));
 
-    TestInterface api = AsyncFeign.asyncBuilder()
+    TestInterface api = AsyncFeign.builder()
         .decoder((response, type) -> {
           throw new IOException("timeout");
         })
@@ -536,7 +536,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() {
     ExecutorService execs = Executors.newSingleThreadExecutor();
 
     // fake client as Client.Default follows redirects.
-    TestInterface api = AsyncFeign.<Void>asyncBuilder()
+    TestInterface api = AsyncFeign.<Void>builder()
         .client(new AsyncClient.Default<>((request, options) -> response, execs))
         .target(TestInterface.class, "http://localhost:" + server.getPort());
 
@@ -635,10 +635,10 @@ public void equalsHashCodeAndToStringWork() {
         new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8888");
     Target<OtherTestInterface> t3 =
         new HardCodedTarget<OtherTestInterface>(OtherTestInterface.class, "http://localhost:8080");
-    TestInterface i1 = AsyncFeign.asyncBuilder().target(t1);
-    TestInterface i2 = AsyncFeign.asyncBuilder().target(t1);
-    TestInterface i3 = AsyncFeign.asyncBuilder().target(t2);
-    OtherTestInterface i4 = AsyncFeign.asyncBuilder().target(t3);
+    TestInterface i1 = AsyncFeign.builder().target(t1);
+    TestInterface i2 = AsyncFeign.builder().target(t1);
+    TestInterface i3 = AsyncFeign.builder().target(t2);
+    OtherTestInterface i4 = AsyncFeign.builder().target(t3);
 
     assertThat(i1)
         .isEqualTo(i2)
@@ -671,7 +671,7 @@ public void decodeLogicSupportsByteArray() throws Exception {
     server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse)));
 
     OtherTestInterface api =
-        AsyncFeign.asyncBuilder().target(OtherTestInterface.class,
+        AsyncFeign.builder().target(OtherTestInterface.class,
             "http://localhost:" + server.getPort());
 
     assertThat(api.binaryResponseBody())
@@ -684,7 +684,7 @@ public void encodeLogicSupportsByteArray() throws Exception {
     server.enqueue(new MockResponse());
 
     OtherTestInterface api =
-        AsyncFeign.asyncBuilder().target(OtherTestInterface.class,
+        AsyncFeign.builder().target(OtherTestInterface.class,
             "http://localhost:" + server.getPort());
 
     api.binaryRequestBody(expectedRequest);
@@ -743,7 +743,7 @@ private Response responseWithText(String text) {
   public void mapAndDecodeExecutesMapFunction() throws Exception {
     server.enqueue(new MockResponse().setBody("response!"));
 
-    TestInterface api = AsyncFeign.asyncBuilder()
+    TestInterface api = AsyncFeign.builder()
         .mapAndDecode(upperCaseResponseMapper(), new StringDecoder())
         .target(TestInterface.class, "http://localhost:" + server.getPort());
 
@@ -967,7 +967,7 @@ public Exception decode(String methodKey, Response response) {
 
   static final class TestInterfaceBuilder {
 
-    private final AsyncFeign.AsyncBuilder<Void> delegate = AsyncFeign.<Void>asyncBuilder()
+    private final AsyncFeign.AsyncBuilder<Void> delegate = AsyncFeign.<Void>builder()
         .decoder(new Decoder.Default())
         .encoder(new Encoder() {
           @Override

example-github-with-coroutine/src/main/java/example/github/GitHubExample.kt

@@ -86,7 +86,7 @@ interface GitHub {
         fun connect(): GitHub {
             val decoder: Decoder = feign.gson.GsonDecoder()
             val encoder: Encoder = GsonEncoder()
-            return CoroutineFeign.coBuilder<Unit>()
+            return CoroutineFeign.builder<Unit>()
                 .encoder(encoder)
                 .decoder(decoder)
                 .errorDecoder(GitHubErrorDecoder(decoder))