Skip to content

Commit 2122b55

Browse files
wplong11velo
wplong11
and
velo
authored
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>
1 parent 0f4e328 commit 2122b55

15 files changed

Lines changed: 169 additions & 277 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,7 @@ interface GitHub {
10771077
10781078
public class MyApp {
10791079
public static void main(String... args) {
1080-
GitHub github = AsyncFeign.asyncBuilder()
1080+
GitHub github = AsyncFeign.builder()
10811081
.decoder(new GsonDecoder())
10821082
.target(GitHub.class, "https://api.github.com");
10831083

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

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package feign;
1515

1616
import feign.Logger.Level;
17+
import feign.ReflectiveFeign.ParseHandlersByName;
1718
import feign.Request.Options;
1819
import feign.Target.HardCodedTarget;
1920
import feign.codec.Decoder;
@@ -43,10 +44,17 @@
4344
* be done (for example, creating and submitting a task to an {@link ExecutorService}).
4445
*/
4546
@Experimental
46-
public abstract class AsyncFeign<C> extends Feign {
47+
public abstract class AsyncFeign<C> {
48+
public static <C> AsyncBuilder<C> builder() {
49+
return new AsyncBuilder<>();
50+
}
4751

52+
/**
53+
* @deprecated use {@link #builder()} instead.
54+
*/
55+
@Deprecated()
4856
public static <C> AsyncBuilder<C> asyncBuilder() {
49-
return new AsyncBuilder<>();
57+
return builder();
5058
}
5159

5260
private static class LazyInitializedExecutorService {
@@ -66,6 +74,7 @@ public static class AsyncBuilder<C> extends BaseBuilder<AsyncBuilder<C>> {
6674
private AsyncClient<C> client =
6775
new AsyncClient.Default<>(
6876
new Client.Default(null, null), LazyInitializedExecutorService.instance);
77+
private MethodInfoResolver methodInfoResolver = MethodInfo::new;
6978

7079
@Deprecated
7180
public AsyncBuilder<C> defaultContextSupplier(Supplier<C> supplier) {
@@ -78,6 +87,11 @@ public AsyncBuilder<C> client(AsyncClient<C> client) {
7887
return this;
7988
}
8089

90+
public AsyncBuilder<C> methodInfoResolver(MethodInfoResolver methodInfoResolver) {
91+
this.methodInfoResolver = methodInfoResolver;
92+
return this;
93+
}
94+
8195
@Override
8296
public AsyncBuilder<C> mapAndDecode(ResponseMapper mapper, Decoder decoder) {
8397
return super.mapAndDecode(mapper, decoder);
@@ -194,23 +208,31 @@ public AsyncFeign<C> build() {
194208
AsyncResponseHandler.class,
195209
capabilities);
196210

211+
final SynchronousMethodHandler.Factory synchronousMethodHandlerFactory =
212+
new SynchronousMethodHandler.Factory(
213+
stageExecution(activeContextHolder, client),
214+
retryer,
215+
requestInterceptors,
216+
responseInterceptor,
217+
logger,
218+
logLevel,
219+
dismiss404,
220+
closeAfterDecode,
221+
propagationPolicy,
222+
true);
223+
final ParseHandlersByName handlersByName =
224+
new ParseHandlersByName(
225+
contract,
226+
options,
227+
encoder,
228+
stageDecode(activeContextHolder, logger, logLevel, responseHandler),
229+
queryMapEncoder,
230+
errorDecoder,
231+
synchronousMethodHandlerFactory);
232+
final ReflectiveFeign feign =
233+
new ReflectiveFeign(handlersByName, invocationHandlerFactory, queryMapEncoder);
197234
return new ReflectiveAsyncFeign<>(
198-
Feign.builder()
199-
.logLevel(logLevel)
200-
.client(stageExecution(activeContextHolder, client))
201-
.decoder(stageDecode(activeContextHolder, logger, logLevel, responseHandler))
202-
.forceDecoding() // force all handling through stageDecode
203-
.contract(contract)
204-
.logger(logger)
205-
.encoder(encoder)
206-
.queryMapEncoder(queryMapEncoder)
207-
.options(options)
208-
.requestInterceptors(requestInterceptors)
209-
.responseInterceptor(responseInterceptor)
210-
.invocationHandlerFactory(invocationHandlerFactory)
211-
.build(),
212-
defaultContextSupplier,
213-
activeContextHolder);
235+
feign, defaultContextSupplier, activeContextHolder, methodInfoResolver);
214236
}
215237

216238
private Client stageExecution(
@@ -297,7 +319,6 @@ protected AsyncFeign(Feign feign, AsyncContextSupplier<C> defaultContextSupplier
297319
this.defaultContextSupplier = defaultContextSupplier;
298320
}
299321

300-
@Override
301322
public <T> T newInstance(Target<T> target) {
302323
return newInstance(target, defaultContextSupplier.newContext());
303324
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,8 @@ default AsyncResponseHandler enrich(AsyncResponseHandler asyncResponseHandler) {
134134
default <C> AsyncContextSupplier<C> enrich(AsyncContextSupplier<C> asyncContextSupplier) {
135135
return asyncContextSupplier;
136136
}
137+
138+
default MethodInfoResolver enrich(MethodInfoResolver methodInfoResolver) {
139+
return methodInfoResolver;
140+
}
137141
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Copyright 2012-2022 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign;
15+
16+
import java.lang.reflect.Method;
17+
18+
@Experimental
19+
public interface MethodInfoResolver {
20+
public MethodInfo resolve(Class<?> targetType, Method method);
21+
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
5858
}
5959

6060
final MethodInfo methodInfo =
61-
methodInfoLookup.computeIfAbsent(method, m -> new MethodInfo(type, m));
61+
methodInfoLookup.computeIfAbsent(method, m -> methodInfoResolver.resolve(type, m));
6262

6363
setInvocationContext(new AsyncInvocation<>(context, methodInfo));
6464
try {
@@ -96,13 +96,16 @@ public String toString() {
9696
}
9797

9898
private ThreadLocal<AsyncInvocation<C>> activeContextHolder;
99+
private final MethodInfoResolver methodInfoResolver;
99100

100101
public ReflectiveAsyncFeign(
101102
Feign feign,
102103
AsyncContextSupplier<C> defaultContextSupplier,
103-
ThreadLocal<AsyncInvocation<C>> contextHolder) {
104+
ThreadLocal<AsyncInvocation<C>> contextHolder,
105+
MethodInfoResolver methodInfoResolver) {
104106
super(feign, defaultContextSupplier);
105107
this.activeContextHolder = contextHolder;
108+
this.methodInfoResolver = methodInfoResolver;
106109
}
107110

108111
protected void setInvocationContext(AsyncInvocation<C> invocationContext) {

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ public void throwsFeignExceptionIncludingBody() throws Throwable {
530530
server.enqueue(new MockResponse().setBody("success!"));
531531

532532
TestInterfaceAsync api =
533-
AsyncFeign.asyncBuilder()
533+
AsyncFeign.builder()
534534
.decoder(
535535
(response, type) -> {
536536
throw new IOException("timeout");
@@ -555,7 +555,7 @@ public void throwsFeignExceptionWithoutBody() {
555555
server.enqueue(new MockResponse().setBody("success!"));
556556

557557
TestInterfaceAsync api =
558-
AsyncFeign.asyncBuilder()
558+
AsyncFeign.builder()
559559
.decoder(
560560
(response, type) -> {
561561
throw new IOException("timeout");
@@ -589,7 +589,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() throws Throwable {
589589

590590
// fake client as Client.Default follows redirects.
591591
TestInterfaceAsync api =
592-
AsyncFeign.<Void>asyncBuilder()
592+
AsyncFeign.<Void>builder()
593593
.client(new AsyncClient.Default<>((request, options) -> response, execs))
594594
.target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
595595

@@ -679,10 +679,10 @@ public void equalsHashCodeAndToStringWork() {
679679
new HardCodedTarget<>(TestInterfaceAsync.class, "http://localhost:8888");
680680
Target<OtherTestInterfaceAsync> t3 =
681681
new HardCodedTarget<>(OtherTestInterfaceAsync.class, "http://localhost:8080");
682-
TestInterfaceAsync i1 = AsyncFeign.asyncBuilder().target(t1);
683-
TestInterfaceAsync i2 = AsyncFeign.asyncBuilder().target(t1);
684-
TestInterfaceAsync i3 = AsyncFeign.asyncBuilder().target(t2);
685-
OtherTestInterfaceAsync i4 = AsyncFeign.asyncBuilder().target(t3);
682+
TestInterfaceAsync i1 = AsyncFeign.builder().target(t1);
683+
TestInterfaceAsync i2 = AsyncFeign.builder().target(t1);
684+
TestInterfaceAsync i3 = AsyncFeign.builder().target(t2);
685+
OtherTestInterfaceAsync i4 = AsyncFeign.builder().target(t3);
686686

687687
assertThat(i1).isEqualTo(i2).isNotEqualTo(i3).isNotEqualTo(i4);
688688

@@ -710,7 +710,7 @@ public void decodeLogicSupportsByteArray() throws Throwable {
710710
server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse)));
711711

712712
OtherTestInterfaceAsync api =
713-
AsyncFeign.asyncBuilder()
713+
AsyncFeign.builder()
714714
.target(OtherTestInterfaceAsync.class, "http://localhost:" + server.getPort());
715715

716716
assertThat(unwrap(api.binaryResponseBody())).containsExactly(expectedResponse);
@@ -722,7 +722,7 @@ public void encodeLogicSupportsByteArray() throws Exception {
722722
server.enqueue(new MockResponse());
723723

724724
OtherTestInterfaceAsync api =
725-
AsyncFeign.asyncBuilder()
725+
AsyncFeign.builder()
726726
.target(OtherTestInterfaceAsync.class, "http://localhost:" + server.getPort());
727727

728728
CompletableFuture<?> cf = api.binaryRequestBody(expectedRequest);
@@ -796,7 +796,7 @@ public void mapAndDecodeExecutesMapFunction() throws Throwable {
796796
server.enqueue(new MockResponse().setBody("response!"));
797797

798798
TestInterfaceAsync api =
799-
AsyncFeign.asyncBuilder()
799+
AsyncFeign.builder()
800800
.mapAndDecode(upperCaseResponseMapper(), new StringDecoder())
801801
.target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
802802

@@ -1037,7 +1037,7 @@ public Exception decode(String methodKey, Response response) {
10371037
static final class TestInterfaceAsyncBuilder {
10381038

10391039
private final AsyncFeign.AsyncBuilder<Void> delegate =
1040-
AsyncFeign.<Void>asyncBuilder()
1040+
AsyncFeign.<Void>builder()
10411041
.decoder(new Decoder.Default())
10421042
.encoder(
10431043
new Encoder() {
@@ -1095,31 +1095,31 @@ TestInterfaceAsync target(String url) {
10951095
@Test
10961096
public void testNonInterface() {
10971097
thrown.expect(IllegalArgumentException.class);
1098-
AsyncFeign.asyncBuilder().target(NonInterface.class, "http://localhost");
1098+
AsyncFeign.builder().target(NonInterface.class, "http://localhost");
10991099
}
11001100

11011101
@Test
11021102
public void testExtendedCFReturnType() {
11031103
thrown.expect(IllegalArgumentException.class);
1104-
AsyncFeign.asyncBuilder().target(ExtendedCFApi.class, "http://localhost");
1104+
AsyncFeign.builder().target(ExtendedCFApi.class, "http://localhost");
11051105
}
11061106

11071107
@Test
11081108
public void testLowerWildReturnType() {
11091109
thrown.expect(IllegalArgumentException.class);
1110-
AsyncFeign.asyncBuilder().target(LowerWildApi.class, "http://localhost");
1110+
AsyncFeign.builder().target(LowerWildApi.class, "http://localhost");
11111111
}
11121112

11131113
@Test
11141114
public void testUpperWildReturnType() {
11151115
thrown.expect(IllegalArgumentException.class);
1116-
AsyncFeign.asyncBuilder().target(UpperWildApi.class, "http://localhost");
1116+
AsyncFeign.builder().target(UpperWildApi.class, "http://localhost");
11171117
}
11181118

11191119
@Test
11201120
public void testrWildReturnType() {
11211121
thrown.expect(IllegalArgumentException.class);
1122-
AsyncFeign.asyncBuilder().target(WildApi.class, "http://localhost");
1122+
AsyncFeign.builder().target(WildApi.class, "http://localhost");
11231123
}
11241124

11251125
static final class ExtendedCF<T> extends CompletableFuture<T> {}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public class BaseBuilderTest {
2727
@Test
2828
public void checkEnrichTouchesAllAsyncBuilderFields()
2929
throws IllegalArgumentException, IllegalAccessException {
30-
test(AsyncFeign.asyncBuilder().requestInterceptor(template -> {}), 13);
30+
test(AsyncFeign.builder().requestInterceptor(template -> {}), 14);
3131
}
3232

3333
private void test(BaseBuilder<?> builder, int expectedFieldsCount)

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ public void throwsFeignExceptionIncludingBody() {
488488
server.enqueue(new MockResponse().setBody("success!"));
489489

490490
TestInterface api =
491-
AsyncFeign.asyncBuilder()
491+
AsyncFeign.builder()
492492
.decoder(
493493
(response, type) -> {
494494
throw new IOException("timeout");
@@ -509,7 +509,7 @@ public void throwsFeignExceptionWithoutBody() {
509509
server.enqueue(new MockResponse().setBody("success!"));
510510

511511
TestInterface api =
512-
AsyncFeign.asyncBuilder()
512+
AsyncFeign.builder()
513513
.decoder(
514514
(response, type) -> {
515515
throw new IOException("timeout");
@@ -542,7 +542,7 @@ public void whenReturnTypeIsResponseNoErrorHandling() {
542542

543543
// fake client as Client.Default follows redirects.
544544
TestInterface api =
545-
AsyncFeign.<Void>asyncBuilder()
545+
AsyncFeign.<Void>builder()
546546
.client(new AsyncClient.Default<>((request, options) -> response, execs))
547547
.target(TestInterface.class, "http://localhost:" + server.getPort());
548548

@@ -655,10 +655,10 @@ public void equalsHashCodeAndToStringWork() {
655655
new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8888");
656656
Target<OtherTestInterface> t3 =
657657
new HardCodedTarget<OtherTestInterface>(OtherTestInterface.class, "http://localhost:8080");
658-
TestInterface i1 = AsyncFeign.asyncBuilder().target(t1);
659-
TestInterface i2 = AsyncFeign.asyncBuilder().target(t1);
660-
TestInterface i3 = AsyncFeign.asyncBuilder().target(t2);
661-
OtherTestInterface i4 = AsyncFeign.asyncBuilder().target(t3);
658+
TestInterface i1 = AsyncFeign.builder().target(t1);
659+
TestInterface i2 = AsyncFeign.builder().target(t1);
660+
TestInterface i3 = AsyncFeign.builder().target(t2);
661+
OtherTestInterface i4 = AsyncFeign.builder().target(t3);
662662

663663
assertThat(i1).isEqualTo(i2).isNotEqualTo(i3).isNotEqualTo(i4);
664664

@@ -685,7 +685,7 @@ public void decodeLogicSupportsByteArray() throws Exception {
685685
server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse)));
686686

687687
OtherTestInterface api =
688-
AsyncFeign.asyncBuilder()
688+
AsyncFeign.builder()
689689
.target(OtherTestInterface.class, "http://localhost:" + server.getPort());
690690

691691
assertThat(api.binaryResponseBody()).containsExactly(expectedResponse);
@@ -697,7 +697,7 @@ public void encodeLogicSupportsByteArray() throws Exception {
697697
server.enqueue(new MockResponse());
698698

699699
OtherTestInterface api =
700-
AsyncFeign.asyncBuilder()
700+
AsyncFeign.builder()
701701
.target(OtherTestInterface.class, "http://localhost:" + server.getPort());
702702

703703
api.binaryRequestBody(expectedRequest);
@@ -754,7 +754,7 @@ public void mapAndDecodeExecutesMapFunction() throws Exception {
754754
server.enqueue(new MockResponse().setBody("response!"));
755755

756756
TestInterface api =
757-
AsyncFeign.asyncBuilder()
757+
AsyncFeign.builder()
758758
.mapAndDecode(upperCaseResponseMapper(), new StringDecoder())
759759
.target(TestInterface.class, "http://localhost:" + server.getPort());
760760

@@ -981,7 +981,7 @@ public Exception decode(String methodKey, Response response) {
981981
static final class TestInterfaceBuilder {
982982

983983
private final AsyncFeign.AsyncBuilder<Void> delegate =
984-
AsyncFeign.<Void>asyncBuilder()
984+
AsyncFeign.<Void>builder()
985985
.decoder(new Decoder.Default())
986986
.encoder(
987987
new Encoder() {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ interface GitHub {
8686
fun connect(): GitHub {
8787
val decoder: Decoder = feign.gson.GsonDecoder()
8888
val encoder: Encoder = GsonEncoder()
89-
return CoroutineFeign.coBuilder<Unit>()
89+
return CoroutineFeign.builder<Unit>()
9090
.encoder(encoder)
9191
.decoder(decoder)
9292
.errorDecoder(GitHubErrorDecoder(decoder))

0 commit comments

Comments
 (0)