From db378ebb232d936e648f31f06c1170de29cbdbae Mon Sep 17 00:00:00 2001 From: Donghyeon Kim Date: Tue, 20 Sep 2022 06:30:52 +0900 Subject: [PATCH] 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 --- README.md | 2 +- core/src/main/java/feign/AsyncFeign.java | 47 ++-- core/src/main/java/feign/Capability.java | 4 + .../main/java/feign/MethodInfoResolver.java | 24 ++ .../main/java/feign/ReflectiveAsyncFeign.java | 6 +- core/src/test/java/feign/AsyncFeignTest.java | 32 +-- core/src/test/java/feign/BaseBuilderTest.java | 4 +- .../test/java/feign/FeignUnderAsyncTest.java | 22 +- .../main/java/example/github/GitHubExample.kt | 2 +- .../feign/hc5/AsyncApacheHttp5ClientTest.java | 23 +- .../test/Http2ClientAsyncTest.java | 6 +- .../java/feign/kotlin/CoroutineFeign.java | 265 ++++-------------- .../kotlin/feign/kotlin/CoroutineFeignTest.kt | 2 +- .../micrometer/AbstractMetricsTestBase.java | 2 +- .../feign/okhttp/OkHttpClientAsyncTest.java | 6 +- 15 files changed, 164 insertions(+), 283 deletions(-) create mode 100644 core/src/main/java/feign/MethodInfoResolver.java diff --git a/README.md b/README.md index 7b9c104118..94dfdd2bec 100644 --- a/README.md +++ b/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"); diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index 17261d45df..ea10b060f2 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/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 extends Feign { +public abstract class AsyncFeign { + public static AsyncBuilder builder() { + return new AsyncBuilder<>(); + } + /** + * @deprecated use {@link #builder()} instead. + */ + @Deprecated() public static AsyncBuilder asyncBuilder() { - return new AsyncBuilder<>(); + return builder(); } private static class LazyInitializedExecutorService { @@ -66,6 +74,7 @@ public static class AsyncBuilder extends BaseBuilder> { private AsyncContextSupplier defaultContextSupplier = () -> null; private AsyncClient client = new AsyncClient.Default<>( new Client.Default(null, null), LazyInitializedExecutorService.instance); + private MethodInfoResolver methodInfoResolver = MethodInfo::new; @Deprecated public AsyncBuilder defaultContextSupplier(Supplier supplier) { @@ -78,6 +87,11 @@ public AsyncBuilder client(AsyncClient client) { return this; } + public AsyncBuilder methodInfoResolver(MethodInfoResolver methodInfoResolver) { + this.methodInfoResolver = methodInfoResolver; + return this; + } + @Override public AsyncBuilder mapAndDecode(ResponseMapper mapper, Decoder decoder) { return super.mapAndDecode(mapper, decoder); @@ -191,21 +205,19 @@ public AsyncFeign 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 defaultContextSupplier this.defaultContextSupplier = defaultContextSupplier; } - @Override public T newInstance(Target target) { return newInstance(target, defaultContextSupplier.newContext()); } diff --git a/core/src/main/java/feign/Capability.java b/core/src/main/java/feign/Capability.java index f27e9a8597..0b71a9f088 100644 --- a/core/src/main/java/feign/Capability.java +++ b/core/src/main/java/feign/Capability.java @@ -133,4 +133,8 @@ default AsyncResponseHandler enrich(AsyncResponseHandler asyncResponseHandler) { default AsyncContextSupplier enrich(AsyncContextSupplier asyncContextSupplier) { return asyncContextSupplier; } + + default MethodInfoResolver enrich(MethodInfoResolver methodInfoResolver) { + return methodInfoResolver; + } } diff --git a/core/src/main/java/feign/MethodInfoResolver.java b/core/src/main/java/feign/MethodInfoResolver.java new file mode 100644 index 0000000000..b1b4a54241 --- /dev/null +++ b/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); +} diff --git a/core/src/main/java/feign/ReflectiveAsyncFeign.java b/core/src/main/java/feign/ReflectiveAsyncFeign.java index 1fc4e1306c..9d71bb1a76 100644 --- a/core/src/main/java/feign/ReflectiveAsyncFeign.java +++ b/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> activeContextHolder; + private final MethodInfoResolver methodInfoResolver; public ReflectiveAsyncFeign(Feign feign, AsyncContextSupplier defaultContextSupplier, - ThreadLocal> contextHolder) { + ThreadLocal> contextHolder, MethodInfoResolver methodInfoResolver) { super(feign, defaultContextSupplier); this.activeContextHolder = contextHolder; + this.methodInfoResolver = methodInfoResolver; } protected void setInvocationContext(AsyncInvocation invocationContext) { diff --git a/core/src/test/java/feign/AsyncFeignTest.java b/core/src/test/java/feign/AsyncFeignTest.java index d89ca1cd76..524df18a22 100644 --- a/core/src/test/java/feign/AsyncFeignTest.java +++ b/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.asyncBuilder() + TestInterfaceAsync api = AsyncFeign.builder() .client(new AsyncClient.Default<>((request, options) -> response, execs)) .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); @@ -625,10 +625,10 @@ public void equalsHashCodeAndToStringWork() { Target 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 delegate = AsyncFeign.asyncBuilder() + private final AsyncFeign.AsyncBuilder delegate = AsyncFeign.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"); } diff --git a/core/src/test/java/feign/BaseBuilderTest.java b/core/src/test/java/feign/BaseBuilderTest.java index 1fc3fdf4f0..9599fd98f4 100644 --- a/core/src/test/java/feign/BaseBuilderTest.java +++ b/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) diff --git a/core/src/test/java/feign/FeignUnderAsyncTest.java b/core/src/test/java/feign/FeignUnderAsyncTest.java index b2332696c7..ef2e06eb5c 100644 --- a/core/src/test/java/feign/FeignUnderAsyncTest.java +++ b/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.asyncBuilder() + TestInterface api = AsyncFeign.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.class, "http://localhost:8888"); Target t3 = new HardCodedTarget(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 delegate = AsyncFeign.asyncBuilder() + private final AsyncFeign.AsyncBuilder delegate = AsyncFeign.builder() .decoder(new Decoder.Default()) .encoder(new Encoder() { @Override diff --git a/example-github-with-coroutine/src/main/java/example/github/GitHubExample.kt b/example-github-with-coroutine/src/main/java/example/github/GitHubExample.kt index 21538fca7c..03fae670e9 100644 --- a/example-github-with-coroutine/src/main/java/example/github/GitHubExample.kt +++ b/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() + return CoroutineFeign.builder() .encoder(encoder) .decoder(decoder) .errorDecoder(GitHubErrorDecoder(decoder)) diff --git a/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java b/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java index 4948accfbf..95404e3140 100644 --- a/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java +++ b/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java @@ -23,7 +23,6 @@ import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; import org.apache.hc.client5.http.protocol.HttpClientContext; -import org.apache.hc.core5.http.protocol.HttpContext; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -459,7 +458,7 @@ public void doesntRetryAfterResponseIsSent() throws Throwable { public void throwsFeignExceptionIncludingBody() throws Throwable { server.enqueue(new MockResponse().setBody("success!")); - final TestInterfaceAsync api = AsyncFeign.asyncBuilder().decoder((response, type) -> { + final TestInterfaceAsync api = AsyncFeign.builder().decoder((response, type) -> { throw new IOException("timeout"); }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); @@ -480,7 +479,7 @@ public void throwsFeignExceptionIncludingBody() throws Throwable { public void throwsFeignExceptionWithoutBody() { server.enqueue(new MockResponse().setBody("success!")); - final TestInterfaceAsync api = AsyncFeign.asyncBuilder().decoder((response, type) -> { + final TestInterfaceAsync api = AsyncFeign.builder().decoder((response, type) -> { throw new IOException("timeout"); }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); @@ -505,7 +504,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() throws Throwable { final ExecutorService execs = Executors.newSingleThreadExecutor(); // fake client as Client.Default follows redirects. - final TestInterfaceAsync api = AsyncFeign.asyncBuilder() + final TestInterfaceAsync api = AsyncFeign.builder() .client(new AsyncClient.Default<>((request, options) -> response, execs)) .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); @@ -581,10 +580,10 @@ public void equalsHashCodeAndToStringWork() { final Target t3 = new HardCodedTarget(OtherTestInterfaceAsync.class, "http://localhost:8080"); - final TestInterfaceAsync i1 = AsyncFeign.asyncBuilder().target(t1); - final TestInterfaceAsync i2 = AsyncFeign.asyncBuilder().target(t1); - final TestInterfaceAsync i3 = AsyncFeign.asyncBuilder().target(t2); - final OtherTestInterfaceAsync i4 = AsyncFeign.asyncBuilder().target(t3); + final TestInterfaceAsync i1 = AsyncFeign.builder().target(t1); + final TestInterfaceAsync i2 = AsyncFeign.builder().target(t1); + final TestInterfaceAsync i3 = AsyncFeign.builder().target(t2); + final OtherTestInterfaceAsync i4 = AsyncFeign.builder().target(t3); assertThat(i1).isEqualTo(i2).isNotEqualTo(i3).isNotEqualTo(i4); @@ -608,7 +607,7 @@ public void decodeLogicSupportsByteArray() throws Throwable { server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse))); final OtherTestInterfaceAsync api = - AsyncFeign.asyncBuilder().target(OtherTestInterfaceAsync.class, + AsyncFeign.builder().target(OtherTestInterfaceAsync.class, "http://localhost:" + server.getPort()); assertThat(unwrap(api.binaryResponseBody())).containsExactly(expectedResponse); @@ -620,7 +619,7 @@ public void encodeLogicSupportsByteArray() throws Exception { server.enqueue(new MockResponse()); final OtherTestInterfaceAsync api = - AsyncFeign.asyncBuilder().target(OtherTestInterfaceAsync.class, + AsyncFeign.builder().target(OtherTestInterfaceAsync.class, "http://localhost:" + server.getPort()); final CompletableFuture cf = api.binaryRequestBody(expectedRequest); @@ -691,7 +690,7 @@ public void mapAndDecodeExecutesMapFunction() throws Throwable { server.enqueue(new MockResponse().setBody("response!")); final 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())); @@ -921,7 +920,7 @@ public Exception decode(String methodKey, Response response) { static final class TestInterfaceAsyncBuilder { private final AsyncFeign.AsyncBuilder delegate = - AsyncFeign.asyncBuilder() + AsyncFeign.builder() .client(new AsyncApacheHttp5Client()) .decoder(new Decoder.Default()).encoder(new Encoder() { diff --git a/java11/src/test/java/feign/http2client/test/Http2ClientAsyncTest.java b/java11/src/test/java/feign/http2client/test/Http2ClientAsyncTest.java index e1f586983f..fbe872920e 100644 --- a/java11/src/test/java/feign/http2client/test/Http2ClientAsyncTest.java +++ b/java11/src/test/java/feign/http2client/test/Http2ClientAsyncTest.java @@ -554,7 +554,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() throws Throwable { final ExecutorService execs = Executors.newSingleThreadExecutor(); // fake client as Client.Default follows redirects. - final TestInterfaceAsync api = AsyncFeign.asyncBuilder() + final TestInterfaceAsync api = AsyncFeign.builder() .client(new AsyncClient.Default<>((request, options) -> response, execs)) .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); @@ -745,7 +745,7 @@ public void mapAndDecodeExecutesMapFunction() throws Throwable { server.enqueue(new MockResponse().setBody("response!")); final 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())); @@ -975,7 +975,7 @@ public Exception decode(String methodKey, Response response) { static final class TestInterfaceAsyncBuilder { private final AsyncFeign.AsyncBuilder delegate = - AsyncFeign.asyncBuilder() + AsyncFeign.builder() .client(new Http2Client()) .decoder(new Decoder.Default()).encoder(new Encoder() { diff --git a/kotlin/src/main/java/feign/kotlin/CoroutineFeign.java b/kotlin/src/main/java/feign/kotlin/CoroutineFeign.java index 8bba27a6dc..259dea67f2 100644 --- a/kotlin/src/main/java/feign/kotlin/CoroutineFeign.java +++ b/kotlin/src/main/java/feign/kotlin/CoroutineFeign.java @@ -16,47 +16,35 @@ import feign.AsyncClient; import feign.AsyncContextSupplier; import feign.AsyncFeign; -import feign.AsyncInvocation; -import feign.AsyncJoinException; -import feign.AsyncResponseHandler; import feign.BaseBuilder; -import feign.Capability; import feign.Client; import feign.Experimental; -import feign.Feign; -import feign.Logger; -import feign.Logger.Level; -import feign.MethodInfo; -import feign.Response; import feign.Target; import feign.Target.HardCodedTarget; -import feign.codec.Decoder; -import java.io.IOException; import java.lang.reflect.InvocationHandler; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.ParameterizedType; import java.lang.reflect.Proxy; -import java.lang.reflect.Type; -import java.lang.reflect.WildcardType; -import java.util.Map; -import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import kotlin.coroutines.Continuation; import kotlinx.coroutines.future.FutureKt; @Experimental -public class CoroutineFeign extends AsyncFeign { - public static CoroutineBuilder coBuilder() { +public class CoroutineFeign { + public static CoroutineBuilder builder() { return new CoroutineBuilder<>(); } + /** + * @deprecated use {@link #builder()} instead. + */ + @Deprecated() + public static CoroutineBuilder coBuilder() { + return builder(); + } + private static class LazyInitializedExecutorService { private static final ExecutorService instance = @@ -68,26 +56,22 @@ private static class LazyInitializedExecutorService { }); } - private class CoroutineFeignInvocationHandler implements InvocationHandler { - - private final Map methodInfoLookup = new ConcurrentHashMap<>(); + private static class CoroutineFeignInvocationHandler implements InvocationHandler { - private final Class type; private final T instance; - private final C context; - CoroutineFeignInvocationHandler(Class type, T instance, C context) { - this.type = type; + CoroutineFeignInvocationHandler(T instance) { this.instance = instance; - this.context = context; } @Override + @SuppressWarnings("unchecked") public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { if ("equals".equals(method.getName()) && method.getParameterCount() == 1) { try { - final Object otherHandler = - args.length > 0 && args[0] != null ? Proxy.getInvocationHandler(args[0]) : null; + final Object otherHandler = args.length > 0 && args[0] != null + ? Proxy.getInvocationHandler(args[0]) + : null; return equals(otherHandler); } catch (final IllegalArgumentException e) { return false; @@ -98,30 +82,15 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl return toString(); } - final MethodInfo methodInfo = - methodInfoLookup.computeIfAbsent(method, m -> KotlinMethodInfo.createInstance(type, m)); - - setInvocationContext(new AsyncInvocation<>(context, methodInfo)); - try { - if (MethodKt.isSuspend(method)) { - CompletableFuture result = (CompletableFuture) method.invoke(instance, args); - Continuation continuation = (Continuation) args[args.length - 1]; - return FutureKt.await(result, continuation); - } - - return method.invoke(instance, args); - } catch (final InvocationTargetException e) { - Throwable cause = e.getCause(); - if (cause instanceof AsyncJoinException) { - cause = cause.getCause(); - } - throw cause; - } finally { - clearInvocationContext(); + if (MethodKt.isSuspend(method)) { + CompletableFuture result = (CompletableFuture) method.invoke(instance, args); + Continuation continuation = (Continuation) args[args.length - 1]; + return FutureKt.await(result, continuation); } + + return method.invoke(instance, args); } - @SuppressWarnings("unchecked") @Override public boolean equals(Object obj) { if (obj instanceof CoroutineFeignInvocationHandler) { @@ -145,9 +114,8 @@ public String toString() { public static class CoroutineBuilder extends BaseBuilder> { private AsyncContextSupplier defaultContextSupplier = () -> null; - private AsyncClient client = - new AsyncClient.Default<>( - new Client.Default(null, null), LazyInitializedExecutorService.instance); + private AsyncClient client = new AsyncClient.Default<>( + new Client.Default(null, null), LazyInitializedExecutorService.instance); @Deprecated public CoroutineBuilder defaultContextSupplier(Supplier supplier) { @@ -181,178 +149,51 @@ public T target(Target target, C context) { return build().newInstance(target, context); } + @SuppressWarnings("unchecked") public CoroutineFeign build() { super.enrich(); - ThreadLocal> activeContextHolder = new ThreadLocal<>(); - - AsyncResponseHandler responseHandler = - (AsyncResponseHandler) Capability.enrich( - new AsyncResponseHandler( - logLevel, - logger, - decoder, - errorDecoder, - dismiss404, - closeAfterDecode, - responseInterceptor), - AsyncResponseHandler.class, - capabilities); - - return new CoroutineFeign<>( - 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); - } - - private Client stageExecution( - ThreadLocal> activeContext, - AsyncClient client) { - return (request, options) -> { - final Response result = Response.builder().status(200).request(request).build(); - - final AsyncInvocation invocationContext = activeContext.get(); - invocationContext.setResponseFuture( - client.execute(request, options, Optional.ofNullable(invocationContext.context()))); - - return result; - }; - } - - // from SynchronousMethodHandler - long elapsedTime(long start) { - return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); - } - - private Decoder stageDecode( - ThreadLocal> activeContext, - Logger logger, - Level logLevel, - AsyncResponseHandler responseHandler) { - return (response, type) -> { - final AsyncInvocation invocationContext = activeContext.get(); - - final CompletableFuture result = new CompletableFuture<>(); - - invocationContext - .responseFuture() - .whenComplete( - (r, t) -> { - final long elapsedTime = elapsedTime(invocationContext.startNanos()); - - if (t != null) { - if (logLevel != Logger.Level.NONE && t instanceof IOException) { - final IOException e = (IOException) t; - logger.logIOException( - invocationContext.configKey(), logLevel, e, elapsedTime); - } - result.completeExceptionally(t); - } else { - responseHandler.handleResponse( - result, - invocationContext.configKey(), - r, - invocationContext.underlyingType(), - elapsedTime); - } - }); - - result.whenComplete( - (r, t) -> { - if (result.isCancelled()) { - invocationContext.responseFuture().cancel(true); - } - }); - - if (invocationContext.isAsyncReturnType()) { - return result; - } - try { - return result.join(); - } catch (final CompletionException e) { - final Response r = invocationContext.responseFuture().join(); - Throwable cause = e.getCause(); - if (cause == null) { - cause = e; - } - throw new AsyncJoinException(r.status(), cause.getMessage(), r.request(), cause); - } - }; + AsyncFeign asyncFeign = (AsyncFeign) AsyncFeign.builder() + .logLevel(logLevel) + .client((AsyncClient) client) + .decoder(decoder) + .contract(contract) + .retryer(retryer) + .logger(logger) + .encoder(encoder) + .queryMapEncoder(queryMapEncoder) + .options(options) + .requestInterceptors(requestInterceptors) + .responseInterceptor(responseInterceptor) + .invocationHandlerFactory(invocationHandlerFactory) + .defaultContextSupplier((AsyncContextSupplier) defaultContextSupplier) + .methodInfoResolver(KotlinMethodInfo::createInstance) + .build(); + return new CoroutineFeign<>(asyncFeign); } } - protected ThreadLocal> activeContextHolder; - - protected CoroutineFeign( - Feign feign, - AsyncContextSupplier defaultContextSupplier, - ThreadLocal> contextHolder) { - super(feign, defaultContextSupplier); - this.activeContextHolder = contextHolder; - } + private final AsyncFeign feign; - protected void setInvocationContext(AsyncInvocation invocationContext) { - activeContextHolder.set(invocationContext); + protected CoroutineFeign(AsyncFeign feign) { + this.feign = feign; } - protected void clearInvocationContext() { - activeContextHolder.remove(); + public T newInstance(Target target) { + T instance = feign.newInstance(target); + return wrap(target.type(), instance); } - private String getFullMethodName(Class type, Type retType, Method m) { - return retType.getTypeName() + " " + type.toGenericString() + "." + m.getName(); + public T newInstance(Target target, C context) { + T instance = feign.newInstance(target, context); + return wrap(target.type(), instance); } - @Override - protected T wrap(Class type, T instance, C context) { - if (!type.isInterface()) { - throw new IllegalArgumentException("Type must be an interface: " + type); - } - - for (final Method m : type.getMethods()) { - final Class retType = m.getReturnType(); - - if (!CompletableFuture.class.isAssignableFrom(retType)) { - continue; // synchronous case - } - - if (retType != CompletableFuture.class) { - throw new IllegalArgumentException( - "Method return type is not CompleteableFuture: " + getFullMethodName(type, retType, m)); - } - - final Type genRetType = m.getGenericReturnType(); - - if (!ParameterizedType.class.isInstance(genRetType)) { - throw new IllegalArgumentException( - "Method return type is not parameterized: " + getFullMethodName(type, genRetType, m)); - } - - if (WildcardType.class.isInstance( - ParameterizedType.class.cast(genRetType).getActualTypeArguments()[0])) { - throw new IllegalArgumentException( - "Wildcards are not supported for return-type parameters: " - + getFullMethodName(type, genRetType, m)); - } - } - + private T wrap(Class type, T instance) { return type.cast( Proxy.newProxyInstance( type.getClassLoader(), new Class[] {type}, - new CoroutineFeignInvocationHandler<>(type, instance, context))); + new CoroutineFeignInvocationHandler<>(instance))); } } diff --git a/kotlin/src/test/kotlin/feign/kotlin/CoroutineFeignTest.kt b/kotlin/src/test/kotlin/feign/kotlin/CoroutineFeignTest.kt index 41943bc48a..65e1d78324 100644 --- a/kotlin/src/test/kotlin/feign/kotlin/CoroutineFeignTest.kt +++ b/kotlin/src/test/kotlin/feign/kotlin/CoroutineFeignTest.kt @@ -145,7 +145,7 @@ class CoroutineFeignTest { } internal class TestInterfaceAsyncBuilder { - private val delegate = CoroutineFeign.coBuilder() + private val delegate = CoroutineFeign.builder() .decoder(Decoder.Default()).encoder { `object`, bodyType, template -> if (`object` is Map<*, *>) { template.body(Gson().toJson(`object`)) diff --git a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java index df31f82a2a..a109c829c0 100644 --- a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java +++ b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java @@ -76,7 +76,7 @@ public final void addMetricsCapability() { @Test public final void addAsyncMetricsCapability() { final CompletableSource source = - AsyncFeign.asyncBuilder() + AsyncFeign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get", "1234567890abcde")) .addCapability(createMetricCapability()) .target(new MockTarget<>(CompletableSource.class)); diff --git a/okhttp/src/test/java/feign/okhttp/OkHttpClientAsyncTest.java b/okhttp/src/test/java/feign/okhttp/OkHttpClientAsyncTest.java index 95303213ed..6799572205 100644 --- a/okhttp/src/test/java/feign/okhttp/OkHttpClientAsyncTest.java +++ b/okhttp/src/test/java/feign/okhttp/OkHttpClientAsyncTest.java @@ -552,7 +552,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() throws Throwable { final ExecutorService execs = Executors.newSingleThreadExecutor(); // fake client as Client.Default follows redirects. - final TestInterfaceAsync api = AsyncFeign.asyncBuilder() + final TestInterfaceAsync api = AsyncFeign.builder() .client(new AsyncClient.Default<>((request, options) -> response, execs)) .target(TestInterfaceAsync.class, "http://localhost:" + server.getPort()); @@ -743,7 +743,7 @@ public void mapAndDecodeExecutesMapFunction() throws Throwable { server.enqueue(new MockResponse().setBody("response!")); final 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())); @@ -973,7 +973,7 @@ public Exception decode(String methodKey, Response response) { static final class TestInterfaceAsyncBuilder { private final AsyncFeign.AsyncBuilder delegate = - AsyncFeign.asyncBuilder() + AsyncFeign.builder() .client(new OkHttpClient()) .decoder(new Decoder.Default()).encoder((object, bodyType, template) -> { if (object instanceof Map) {