Skip to content

Commit 04a85e6

Browse files
vitalijr2vitalijr2
authored
All decoders follow rule: if a status is 404 it returns empty or null value (OpenFeign#1597)
* All decoders follow rule: if status 404 it returns empty or null value * Replace decode404 with dismiss404
1 parent c4686e0 commit 04a85e6

33 files changed

Lines changed: 176 additions & 104 deletions

File tree

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public static class AsyncBuilder<C> {
6868

6969
private Decoder decoder = new Decoder.Default();
7070
private ErrorDecoder errorDecoder = new ErrorDecoder.Default();
71-
private boolean decode404;
71+
private boolean dismiss404;
7272
private boolean closeAfterDecode = true;
7373

7474
public AsyncBuilder() {
@@ -104,9 +104,18 @@ public AsyncBuilder<C> decoder(Decoder decoder) {
104104

105105
/**
106106
* @see Builder#decode404()
107+
* @deprecated
107108
*/
108109
public AsyncBuilder<C> decode404() {
109-
this.decode404 = true;
110+
this.dismiss404 = true;
111+
return this;
112+
}
113+
114+
/**
115+
* @see Builder#dismiss404()
116+
*/
117+
public AsyncBuilder<C> dismiss404() {
118+
this.dismiss404 = true;
110119
return this;
111120
}
112121

@@ -256,7 +265,7 @@ protected AsyncFeign(AsyncBuilder<C> asyncBuilder) {
256265
asyncBuilder.logger,
257266
asyncBuilder.decoder,
258267
asyncBuilder.errorDecoder,
259-
asyncBuilder.decode404,
268+
asyncBuilder.dismiss404,
260269
asyncBuilder.closeAfterDecode);
261270

262271
asyncBuilder.builder.client(this::stageExecution);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,17 @@ class AsyncResponseHandler {
3737

3838
private final Decoder decoder;
3939
private final ErrorDecoder errorDecoder;
40-
private final boolean decode404;
40+
private final boolean dismiss404;
4141
private final boolean closeAfterDecode;
4242

4343
AsyncResponseHandler(Level logLevel, Logger logger, Decoder decoder, ErrorDecoder errorDecoder,
44-
boolean decode404, boolean closeAfterDecode) {
44+
boolean dismiss404, boolean closeAfterDecode) {
4545
super();
4646
this.logLevel = logLevel;
4747
this.logger = logger;
4848
this.decoder = decoder;
4949
this.errorDecoder = errorDecoder;
50-
this.decode404 = decode404;
50+
this.dismiss404 = dismiss404;
5151
this.closeAfterDecode = closeAfterDecode;
5252
}
5353

@@ -88,7 +88,7 @@ void handleResponse(CompletableFuture<Object> resultFuture,
8888
shouldClose = closeAfterDecode;
8989
resultFuture.complete(result);
9090
}
91-
} else if (decode404 && response.status() == 404 && !isVoidType(returnType)) {
91+
} else if (dismiss404 && response.status() == 404 && !isVoidType(returnType)) {
9292
final Object result = decode(response, returnType);
9393
shouldClose = closeAfterDecode;
9494
resultFuture.complete(result);

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public static class Builder {
110110
private Options options = new Options();
111111
private InvocationHandlerFactory invocationHandlerFactory =
112112
new InvocationHandlerFactory.Default();
113-
private boolean decode404;
113+
private boolean dismiss404;
114114
private boolean closeAfterDecode = true;
115115
private ExceptionPropagationPolicy propagationPolicy = NONE;
116116
private boolean forceDecoding = false;
@@ -180,9 +180,32 @@ public Builder mapAndDecode(ResponseMapper mapper, Decoder decoder) {
180180
* custom {@link #client(Client) client}.
181181
*
182182
* @since 8.12
183+
* @deprecated
183184
*/
184185
public Builder decode404() {
185-
this.decode404 = true;
186+
this.dismiss404 = true;
187+
return this;
188+
}
189+
190+
/**
191+
* This flag indicates that the {@link #decoder(Decoder) decoder} should process responses with
192+
* 404 status, specifically returning null or empty instead of throwing {@link FeignException}.
193+
*
194+
* <p/>
195+
* All first-party (ex gson) decoders return well-known empty values defined by
196+
* {@link Util#emptyValueOf}. To customize further, wrap an existing {@link #decoder(Decoder)
197+
* decoder} or make your own.
198+
*
199+
* <p/>
200+
* This flag only works with 404, as opposed to all or arbitrary status codes. This was an
201+
* explicit decision: 404 -> empty is safe, common and doesn't complicate redirection, retry or
202+
* fallback policy. If your server returns a different status for not-found, correct via a
203+
* custom {@link #client(Client) client}.
204+
*
205+
* @since 11.9
206+
*/
207+
public Builder dismiss404() {
208+
this.dismiss404 = true;
186209
return this;
187210
}
188211

@@ -285,7 +308,7 @@ public Feign build() {
285308

286309
SynchronousMethodHandler.Factory synchronousMethodHandlerFactory =
287310
new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors, logger,
288-
logLevel, decode404, closeAfterDecode, propagationPolicy, forceDecoding);
311+
logLevel, dismiss404, closeAfterDecode, propagationPolicy, forceDecoding);
289312
ParseHandlersByName handlersByName =
290313
new ParseHandlersByName(contract, options, encoder, decoder, queryMapEncoder,
291314
errorDecoder, synchronousMethodHandlerFactory);

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ private SynchronousMethodHandler(Target<?> target, Client client, Retryer retrye
5151
List<RequestInterceptor> requestInterceptors, Logger logger,
5252
Logger.Level logLevel, MethodMetadata metadata,
5353
RequestTemplate.Factory buildTemplateFromArgs, Options options,
54-
Decoder decoder, ErrorDecoder errorDecoder, boolean decode404,
54+
Decoder decoder, ErrorDecoder errorDecoder, boolean dismiss404,
5555
boolean closeAfterDecode, ExceptionPropagationPolicy propagationPolicy,
5656
boolean forceDecoding) {
5757

@@ -75,7 +75,7 @@ private SynchronousMethodHandler(Target<?> target, Client client, Retryer retrye
7575
} else {
7676
this.decoder = null;
7777
this.asyncResponseHandler = new AsyncResponseHandler(logLevel, logger, decoder, errorDecoder,
78-
decode404, closeAfterDecode);
78+
dismiss404, closeAfterDecode);
7979
}
8080
}
8181

@@ -181,20 +181,20 @@ static class Factory {
181181
private final List<RequestInterceptor> requestInterceptors;
182182
private final Logger logger;
183183
private final Logger.Level logLevel;
184-
private final boolean decode404;
184+
private final boolean dismiss404;
185185
private final boolean closeAfterDecode;
186186
private final ExceptionPropagationPolicy propagationPolicy;
187187
private final boolean forceDecoding;
188188

189189
Factory(Client client, Retryer retryer, List<RequestInterceptor> requestInterceptors,
190-
Logger logger, Logger.Level logLevel, boolean decode404, boolean closeAfterDecode,
190+
Logger logger, Logger.Level logLevel, boolean dismiss404, boolean closeAfterDecode,
191191
ExceptionPropagationPolicy propagationPolicy, boolean forceDecoding) {
192192
this.client = checkNotNull(client, "client");
193193
this.retryer = checkNotNull(retryer, "retryer");
194194
this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors");
195195
this.logger = checkNotNull(logger, "logger");
196196
this.logLevel = checkNotNull(logLevel, "logLevel");
197-
this.decode404 = decode404;
197+
this.dismiss404 = dismiss404;
198198
this.closeAfterDecode = closeAfterDecode;
199199
this.propagationPolicy = propagationPolicy;
200200
this.forceDecoding = forceDecoding;
@@ -208,7 +208,7 @@ public MethodHandler create(Target<?> target,
208208
ErrorDecoder errorDecoder) {
209209
return new SynchronousMethodHandler(target, client, retryer, requestInterceptors, logger,
210210
logLevel, md, buildTemplateFromArgs, options, decoder,
211-
errorDecoder, decode404, closeAfterDecode, propagationPolicy, forceDecoding);
211+
errorDecoder, dismiss404, closeAfterDecode, propagationPolicy, forceDecoding);
212212
}
213213
}
214214
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ public static Type resolveLastTypeParameter(Type genericContext, Class<?> supert
241241
* </ul>
242242
*
243243
* <p/>
244-
* When {@link Feign.Builder#decode404() decoding HTTP 404 status}, you'll need to teach decoders
244+
* When {@link Feign.Builder#dismiss404() decoding HTTP 404 status}, you'll need to teach decoders
245245
* a default empty value for a type. This method cheaply supports typical types by only looking at
246246
* the raw type (vs type hierarchy). Decorate for sophistication.
247247
*/

‎core/src/main/java/feign/codec/Decoder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
* List<Foo>}. <br/>
5959
* <h3>Note on exception propagation</h3> Exceptions thrown by {@link Decoder}s get wrapped in a
6060
* {@link DecodeException} unless they are a subclass of {@link FeignException} already, and unless
61-
* the client was configured with {@link Feign.Builder#decode404()}.
61+
* the client was configured with {@link Feign.Builder#dismiss404()}.
6262
*/
6363
public interface Decoder {
6464

‎core/src/main/java/feign/codec/ErrorDecoder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
* <p/>
6565
* It is commonly the case that 404 (Not Found) status has semantic value in HTTP apis. While the
6666
* default behavior is to raise exeception, users can alternatively enable 404 processing via
67-
* {@link feign.Feign.Builder#decode404()}.
67+
* {@link feign.Feign.Builder#dismiss404()}.
6868
*/
6969
public interface ErrorDecoder {
7070

‎core/src/main/java/feign/codec/StringDecoder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class StringDecoder implements Decoder {
2424
@Override
2525
public Object decode(Response response, Type type) throws IOException {
2626
Response.Body body = response.body();
27-
if (body == null) {
27+
if (response.status() == 404 || response.status() == 204 || body == null) {
2828
return null;
2929
}
3030
if (String.class.equals(type)) {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -583,12 +583,12 @@ public Object decode(Response response, Type type) throws IOException {
583583
}
584584

585585
@Test
586-
public void decodingExceptionGetWrappedInDecode404Mode() throws Throwable {
586+
public void decodingExceptionGetWrappedInDismiss404Mode() throws Throwable {
587587
server.enqueue(new MockResponse().setResponseCode(404));
588588
thrown.expect(DecodeException.class);
589589
thrown.expectCause(isA(NoSuchElementException.class));;
590590

591-
TestInterfaceAsync api = new TestInterfaceAsyncBuilder().decode404().decoder(new Decoder() {
591+
TestInterfaceAsync api = new TestInterfaceAsyncBuilder().dismiss404().decoder(new Decoder() {
592592
@Override
593593
public Object decode(Response response, Type type) throws IOException {
594594
assertEquals(404, response.status());
@@ -600,11 +600,11 @@ public Object decode(Response response, Type type) throws IOException {
600600
}
601601

602602
@Test
603-
public void decodingDoesNotSwallow404ErrorsInDecode404Mode() throws Throwable {
603+
public void decodingDoesNotSwallow404ErrorsInDismiss404Mode() throws Throwable {
604604
server.enqueue(new MockResponse().setResponseCode(404));
605605
thrown.expect(IllegalArgumentException.class);
606606

607-
TestInterfaceAsync api = new TestInterfaceAsyncBuilder().decode404()
607+
TestInterfaceAsync api = new TestInterfaceAsyncBuilder().dismiss404()
608608
.errorDecoder(new IllegalArgumentExceptionOn404())
609609
.target("http://localhost:" + server.getPort());
610610

@@ -1010,8 +1010,8 @@ TestInterfaceAsyncBuilder errorDecoder(ErrorDecoder errorDecoder) {
10101010
return this;
10111011
}
10121012

1013-
TestInterfaceAsyncBuilder decode404() {
1014-
delegate.decode404();
1013+
TestInterfaceAsyncBuilder dismiss404() {
1014+
delegate.dismiss404();
10151015
return this;
10161016
}
10171017

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public void testDefaults() throws Exception {
6666

6767
/** Shows exception handling isn't required to coerce 404 to null or empty */
6868
@Test
69-
public void testDecode404() {
69+
public void testDismiss404() {
7070
server.enqueue(new MockResponse().setResponseCode(404));
7171
server.enqueue(new MockResponse().setResponseCode(404));
7272
server.enqueue(new MockResponse().setResponseCode(404));
@@ -75,7 +75,7 @@ public void testDecode404() {
7575
server.enqueue(new MockResponse().setResponseCode(400));
7676

7777
String url = "http://localhost:" + server.getPort();
78-
TestInterface api = Feign.builder().decode404().target(TestInterface.class, url);
78+
TestInterface api = Feign.builder().dismiss404().target(TestInterface.class, url);
7979

8080
assertThat(api.getQueues("/")).isEmpty(); // empty, not null!
8181
assertThat(api.decodedLazyPost().hasNext()).isFalse(); // empty, not null!

0 commit comments

Comments
 (0)