From d2fa88a973a9f0caa4a74aa8f2ef49144669960c Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 11:27:53 -0300 Subject: [PATCH 1/6] Add Util helpers for detecting JSON and XML response content types Signed-off-by: Marvin Froeder --- core/src/main/java/feign/Util.java | 78 ++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index 91cb7e5a1a..43a4031512 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -51,6 +51,7 @@ import java.util.TreeMap; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.regex.Pattern; import java.util.stream.Stream; /** Utilities, typically copied in from guava, so as to avoid dependency conflicts. */ @@ -59,6 +60,9 @@ public class Util { /** The HTTP Content-Length header field name. */ public static final String CONTENT_LENGTH = "Content-Length"; + /** The HTTP Content-Type header field name. */ + public static final String CONTENT_TYPE = "Content-Type"; + /** The HTTP Content-Encoding header field name. */ public static final String CONTENT_ENCODING = "Content-Encoding"; @@ -83,6 +87,15 @@ public class Util { private static final int BUF_SIZE = 0x800; // 2K chars (4K bytes) + // matches application/json, text/json, application/vnd.github+json, + // application/json;charset=utf-8 + private static final Pattern JSON_CONTENT_TYPE = + Pattern.compile("(?i)\\w+/(?:[\\w._-]+\\+)?json.*"); + + // matches application/xml, text/xml, application/soap+xml, application/xml;charset=utf-8 + private static final Pattern XML_CONTENT_TYPE = + Pattern.compile("(?i)\\w+/(?:[\\w._-]+\\+)?xml.*"); + /** Type literal for {@code Map}. */ public static final Type MAP_STRING_WILDCARD = new Types.ParameterizedTypeImpl( @@ -371,4 +384,69 @@ public static String getThreadIdentifier() { + "_" + currentThread.getId(); } + + /** + * Checks whether the {@code Content-Type} header of the given response denotes JSON. + * + *

Matches {@code application/json} as well as suffixed types such as {@code + * application/vnd.github+json}. The header name is matched case-insensitively. + * + * @param response the response to check + * @return {@code true} if the content type is JSON, {@code false} otherwise + */ + @Experimental + public static boolean isJsonContentType(Response response) { + return hasContentTypeMatching(response, JSON_CONTENT_TYPE); + } + + /** + * Checks whether the {@code Content-Type} header of the given response denotes XML. + * + *

Matches {@code application/xml} and {@code text/xml} as well as suffixed types such as + * {@code application/soap+xml}. The header name is matched case-insensitively. + * + * @param response the response to check + * @return {@code true} if the content type is XML, {@code false} otherwise + */ + @Experimental + public static boolean isXmlContentType(Response response) { + return hasContentTypeMatching(response, XML_CONTENT_TYPE); + } + + /** + * Checks whether the {@code Content-Type} header of the given response starts with the given + * media type, ignoring case and any parameters such as {@code ;charset=utf-8}. + * + * @param response the response to check + * @param mediaType the media type to look for, for example {@code text/csv} + * @return {@code true} if the content type matches, {@code false} otherwise + */ + @Experimental + public static boolean hasContentType(Response response, String mediaType) { + return contentTypes(response) + .anyMatch( + contentType -> { + String trimmed = contentType.trim(); + return trimmed.regionMatches(true, 0, mediaType, 0, mediaType.length()) + && (trimmed.length() == mediaType.length() + || trimmed.charAt(mediaType.length()) == ';'); + }); + } + + private static Stream contentTypes(Response response) { + if (response == null || response.headers() == null) { + return Stream.empty(); + } + return response.headers().entrySet().stream() + .filter(header -> CONTENT_TYPE.equalsIgnoreCase(header.getKey())) + .map(Map.Entry::getValue) + .filter(Objects::nonNull) + .flatMap(Collection::stream) + .filter(Objects::nonNull); + } + + private static boolean hasContentTypeMatching(Response response, Pattern pattern) { + return contentTypes(response) + .anyMatch(contentType -> pattern.matcher(contentType.trim()).matches()); + } } From 42b1573e08f379c8f07dc85b50a215315fc5765d Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 11:28:09 -0300 Subject: [PATCH 2/6] Add PredicatedDecoder and DecoderPredicate for conditional decoding Signed-off-by: Marvin Froeder --- .../java/feign/codec/DecoderPredicate.java | 105 ++++++++++++++ .../java/feign/codec/PredicatedDecoder.java | 65 +++++++++ .../feign/codec/DecoderPredicateTest.java | 130 ++++++++++++++++++ 3 files changed, 300 insertions(+) create mode 100644 core/src/main/java/feign/codec/DecoderPredicate.java create mode 100644 core/src/main/java/feign/codec/PredicatedDecoder.java create mode 100644 core/src/test/java/feign/codec/DecoderPredicateTest.java diff --git a/core/src/main/java/feign/codec/DecoderPredicate.java b/core/src/main/java/feign/codec/DecoderPredicate.java new file mode 100644 index 0000000000..0727c0ebed --- /dev/null +++ b/core/src/main/java/feign/codec/DecoderPredicate.java @@ -0,0 +1,105 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import feign.Experimental; +import feign.Response; +import feign.Util; +import java.lang.reflect.Type; +import java.util.Arrays; +import java.util.Objects; + +/** + * Decides whether a response can be handled by a {@link Decoder}. + * + *

Predicates receive the same two arguments as {@link Decoder#decode(Response, Type)}, so they + * can discriminate on the response status, on anything in its headers such as the {@code + * Content-Type}, or on the type the caller expects back. + * + *

Predicates must not read the response body. The body is a single-pass stream + * for most clients, so consuming it here would leave nothing for the decoder that is eventually + * chosen. + * + * @see PredicatedDecoder + * @see MultiDecoder + */ +@Experimental +@FunctionalInterface +public interface DecoderPredicate { + + /** + * Whether the decoder this predicate guards can handle the response. + * + * @param response the response that would be decoded. Its body must not be read. + * @param type the {@link java.lang.reflect.Method#getGenericReturnType() generic return type} the + * caller expects back + * @return {@code true} if the response can be decoded, {@code false} otherwise + */ + boolean canDecode(Response response, Type type); + + /** Matches responses whose {@code Content-Type} header denotes JSON. */ + static DecoderPredicate jsonContentType() { + return (response, type) -> Util.isJsonContentType(response); + } + + /** Matches responses whose {@code Content-Type} header denotes XML. */ + static DecoderPredicate xmlContentType() { + return (response, type) -> Util.isXmlContentType(response); + } + + /** + * Matches responses whose {@code Content-Type} header starts with the given media type, ignoring + * case and any parameters such as {@code ;charset=utf-8}. + */ + static DecoderPredicate contentType(String mediaType) { + Objects.requireNonNull(mediaType, "mediaType cannot be null"); + return (response, type) -> Util.hasContentType(response, mediaType); + } + + /** Matches responses carrying no body, such as a {@code 204 No Content}. */ + static DecoderPredicate emptyBody() { + return (response, type) -> + response.body() == null + || (response.body().length() != null && response.body().length() == 0); + } + + /** Matches responses whose status is one of the given codes. */ + static DecoderPredicate status(int... statuses) { + int[] accepted = Arrays.copyOf(statuses, statuses.length); + Arrays.sort(accepted); + return (response, type) -> Arrays.binarySearch(accepted, response.status()) >= 0; + } + + /** Matches responses the caller expects to come back as exactly the given type. */ + static DecoderPredicate returnType(Type expected) { + Objects.requireNonNull(expected, "expected cannot be null"); + return (response, type) -> expected.equals(type); + } + + default DecoderPredicate and(DecoderPredicate other) { + Objects.requireNonNull(other, "other cannot be null"); + return (response, type) -> canDecode(response, type) && other.canDecode(response, type); + } + + default DecoderPredicate or(DecoderPredicate other) { + Objects.requireNonNull(other, "other cannot be null"); + return (response, type) -> canDecode(response, type) || other.canDecode(response, type); + } + + default DecoderPredicate negate() { + return (response, type) -> !canDecode(response, type); + } +} diff --git a/core/src/main/java/feign/codec/PredicatedDecoder.java b/core/src/main/java/feign/codec/PredicatedDecoder.java new file mode 100644 index 0000000000..9d6bf5067c --- /dev/null +++ b/core/src/main/java/feign/codec/PredicatedDecoder.java @@ -0,0 +1,65 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import feign.Experimental; +import feign.Response; +import java.lang.reflect.Type; + +/** + * A {@link Decoder} that knows which responses it can handle. + * + *

Decoders implement this to declare their own applicability, so a {@link MultiDecoder} can + * route each response to the right one without the call site having to wrap anything: + * + * 

+ * public class JacksonDecoder implements Decoder, PredicatedDecoder {
+ *
+ *   @Override
+ *   public boolean canDecode(Response response, Type type) {
+ *     return Util.isJsonContentType(response);
+ *   }
+ * }
+ *
+ * + *

{@link Decoder#decode(Response, Type) decode} remains the only abstract method, so this stays + * a functional interface and a bare lambda is a decoder that accepts everything. + * + *

Decoders that wrap another decoder should forward {@code canDecode} to their delegate, so that + * wrapping does not discard the delegate's applicability. + * + * @see MultiDecoder + * @see DecoderPredicate + */ +@Experimental +@FunctionalInterface +public interface PredicatedDecoder extends Decoder { + + /** + * Whether this decoder can handle the response. Defaults to accepting everything. + * + *

The response body must not be read here: it is a single-pass stream for most clients, so + * consuming it would leave nothing for the decoder that is eventually chosen. + * + * @param response the response that would be decoded. Its body must not be read. + * @param type the {@link java.lang.reflect.Method#getGenericReturnType() generic return type} the + * caller expects back + * @return {@code true} if this decoder can decode the response, {@code false} otherwise + */ + default boolean canDecode(Response response, Type type) { + return true; + } +} diff --git a/core/src/test/java/feign/codec/DecoderPredicateTest.java b/core/src/test/java/feign/codec/DecoderPredicateTest.java new file mode 100644 index 0000000000..a99645a46d --- /dev/null +++ b/core/src/test/java/feign/codec/DecoderPredicateTest.java @@ -0,0 +1,130 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import static org.assertj.core.api.Assertions.assertThat; + +import feign.Request; +import feign.Request.HttpMethod; +import feign.Response; +import feign.Util; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class DecoderPredicateTest { + + private static Response response(String contentType) { + return response(contentType, 200, "body"); + } + + private static Response response(String contentType, int status, String body) { + Map> headers = new HashMap<>(); + if (contentType != null) { + headers.put("Content-Type", Collections.singletonList(contentType)); + } + Response.Builder builder = + Response.builder() + .status(status) + .reason("OK") + .headers(headers) + .request( + Request.create( + HttpMethod.GET, "/api", Collections.emptyMap(), null, Util.UTF_8, null)); + if (body != null) { + builder.body(body, Util.UTF_8); + } + return builder.build(); + } + + @Test + void jsonContentTypeMatchesPlainAndSuffixedTypes() { + DecoderPredicate predicate = DecoderPredicate.jsonContentType(); + + assertThat(predicate.canDecode(response("application/json"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/json;charset=utf-8"), String.class)) + .isTrue(); + assertThat(predicate.canDecode(response("APPLICATION/JSON"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("text/json"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/vnd.github+json"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/xml"), String.class)).isFalse(); + assertThat(predicate.canDecode(response("application/x-json-stream"), String.class)).isFalse(); + assertThat(predicate.canDecode(response(null), String.class)).isFalse(); + } + + @Test + void xmlContentTypeMatchesPlainAndSuffixedTypes() { + DecoderPredicate predicate = DecoderPredicate.xmlContentType(); + + assertThat(predicate.canDecode(response("application/xml"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("text/xml;charset=utf-8"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/soap+xml"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/json"), String.class)).isFalse(); + assertThat(predicate.canDecode(response(null), String.class)).isFalse(); + } + + @Test + void contentTypeIgnoresCaseAndParameters() { + DecoderPredicate predicate = DecoderPredicate.contentType("text/csv"); + + assertThat(predicate.canDecode(response("text/csv"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("TEXT/CSV;charset=utf-8"), String.class)).isTrue(); + assertThat(predicate.canDecode(response("text/csv-x"), String.class)).isFalse(); + assertThat(predicate.canDecode(response("text/plain"), String.class)).isFalse(); + } + + @Test + void emptyBodyMatchesResponsesWithoutContent() { + DecoderPredicate predicate = DecoderPredicate.emptyBody(); + + assertThat(predicate.canDecode(response("application/json", 204, null), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/json", 200, ""), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/json", 200, "body"), String.class)) + .isFalse(); + } + + @Test + void statusMatchesTheGivenCodes() { + DecoderPredicate predicate = DecoderPredicate.status(204, 404); + + assertThat(predicate.canDecode(response("application/json", 204, null), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/json", 404, null), String.class)).isTrue(); + assertThat(predicate.canDecode(response("application/json", 200, "body"), String.class)) + .isFalse(); + } + + @Test + void returnTypeMatchesTheExpectedType() { + DecoderPredicate predicate = DecoderPredicate.returnType(byte[].class); + + assertThat(predicate.canDecode(response("application/octet-stream"), byte[].class)).isTrue(); + assertThat(predicate.canDecode(response("application/octet-stream"), String.class)).isFalse(); + } + + @Test + void combinesPredicates() { + DecoderPredicate json = DecoderPredicate.jsonContentType(); + DecoderPredicate ok = DecoderPredicate.status(200); + + assertThat(json.and(ok).canDecode(response("application/json"), String.class)).isTrue(); + assertThat(json.and(ok).canDecode(response("application/json", 204, null), String.class)) + .isFalse(); + assertThat(json.or(ok).canDecode(response("text/plain"), String.class)).isTrue(); + assertThat(json.negate().canDecode(response("text/plain"), String.class)).isTrue(); + } +} From 4411979a6af66ad377a725ed7466d1ef62e9b10c Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 11:28:17 -0300 Subject: [PATCH 3/6] Add MultiDecoder to select a decoder per response Signed-off-by: Marvin Froeder --- .../main/java/feign/codec/MultiDecoder.java | 156 +++++++++++ .../java/feign/codec/MultiDecoderTest.java | 260 ++++++++++++++++++ 2 files changed, 416 insertions(+) create mode 100644 core/src/main/java/feign/codec/MultiDecoder.java create mode 100644 core/src/test/java/feign/codec/MultiDecoderTest.java diff --git a/core/src/main/java/feign/codec/MultiDecoder.java b/core/src/main/java/feign/codec/MultiDecoder.java new file mode 100644 index 0000000000..e895210238 --- /dev/null +++ b/core/src/main/java/feign/codec/MultiDecoder.java @@ -0,0 +1,156 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import feign.Experimental; +import feign.FeignException; +import feign.Response; +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * A {@link Decoder} that selects a delegate per response, falling back to a default decoder when no + * delegate accepts it. + * + *

Delegates come from two places. A decoder that implements {@link PredicatedDecoder} declares + * its own applicability and can simply be added; any other decoder is paired with a {@link + * DecoderPredicate} at the call site: + * + * 

+ * Feign.builder()
+ *     .decoder(
+ *         MultiDecoder.builder(new DefaultDecoder())
+ *             .add(new JacksonDecoder())
+ *             .add(DecoderPredicate.xmlContentType(), new JAXBDecoder())
+ *             .add((response, type) -> type == byte[].class, new BinaryDecoder())
+ *             .build());
+ *
+ * + *

Delegates are consulted in the order they were added, so the narrowest predicate should come + * first. The default decoder is consulted last. + * + * @see PredicatedDecoder + * @see DecoderPredicate + */ +@Experimental +public class MultiDecoder implements Decoder { + + private final Decoder defaultDecoder; + + private final List delegates; + + private MultiDecoder(Decoder defaultDecoder, List delegates) { + this.defaultDecoder = defaultDecoder; + this.delegates = Collections.unmodifiableList(new ArrayList<>(delegates)); + } + + /** + * Starts building a multi-decoder. + * + * @param defaultDecoder the decoder used when no delegate accepts the response + * @return the builder + */ + public static Builder builder(Decoder defaultDecoder) { + return new Builder(defaultDecoder); + } + + /** + * Decodes using the first delegate that accepts the response, or the default decoder if none do. + * + * @param response {@inheritDoc} + * @param type {@inheritDoc} + * @return {@inheritDoc} + * @throws IOException {@inheritDoc} + * @throws DecodeException {@inheritDoc} + * @throws FeignException {@inheritDoc} + */ + @Override + public Object decode(Response response, Type type) + throws IOException, DecodeException, FeignException { + for (Delegate delegate : delegates) { + if (delegate.predicate.canDecode(response, type)) { + return delegate.decoder.decode(response, type); + } + } + return defaultDecoder.decode(response, type); + } + + @Override + public String toString() { + return "MultiDecoder{defaultDecoder=" + defaultDecoder + ", delegates=" + delegates + '}'; + } + + private static final class Delegate { + private final DecoderPredicate predicate; + private final Decoder decoder; + + Delegate(DecoderPredicate predicate, Decoder decoder) { + this.predicate = predicate; + this.decoder = decoder; + } + + @Override + public String toString() { + return decoder.toString(); + } + } + + /** Collects the delegates of a {@link MultiDecoder}. */ + @Experimental + public static final class Builder { + + private final Decoder defaultDecoder; + + private final List delegates = new ArrayList<>(); + + private Builder(Decoder defaultDecoder) { + this.defaultDecoder = Objects.requireNonNull(defaultDecoder, "defaultDecoder cannot be null"); + } + + /** + * Adds a decoder that declares its own applicability. + * + * @param decoder the decoder, consulted via {@link PredicatedDecoder#canDecode} + */ + public Builder add(PredicatedDecoder decoder) { + Objects.requireNonNull(decoder, "decoder cannot be null"); + return add(decoder::canDecode, decoder); + } + + /** + * Adds any decoder, guarded by the given predicate. Use this for decoders that do not implement + * {@link PredicatedDecoder}, including ones you do not control. + * + * @param predicate decides whether the decoder handles a response + * @param decoder the decoder to delegate to + */ + public Builder add(DecoderPredicate predicate, Decoder decoder) { + Objects.requireNonNull(predicate, "predicate cannot be null"); + Objects.requireNonNull(decoder, "decoder cannot be null"); + delegates.add(new Delegate(predicate, decoder)); + return this; + } + + /** Builds the multi-decoder. */ + public MultiDecoder build() { + return new MultiDecoder(defaultDecoder, delegates); + } + } +} diff --git a/core/src/test/java/feign/codec/MultiDecoderTest.java b/core/src/test/java/feign/codec/MultiDecoderTest.java new file mode 100644 index 0000000000..b1babf6c42 --- /dev/null +++ b/core/src/test/java/feign/codec/MultiDecoderTest.java @@ -0,0 +1,260 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import feign.Request; +import feign.Request.HttpMethod; +import feign.Response; +import feign.Util; +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class MultiDecoderTest { + + /** A plain decoder, with no opinion about what it can handle. */ + private static class RecordingDecoder implements Decoder { + private final String result; + boolean invoked; + + RecordingDecoder(String result) { + this.result = result; + } + + @Override + public Object decode(Response response, Type type) { + invoked = true; + return result; + } + } + + /** A decoder that declares its own applicability, the way feign-gson and friends now do. */ + private static class SelfDeclaringJsonDecoder extends RecordingDecoder + implements PredicatedDecoder { + + SelfDeclaringJsonDecoder() { + super("json"); + } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } + } + + private static Response responseWithContentType(String contentType) { + return responseWithContentType(contentType, 200, "body"); + } + + private static Response responseWithContentType(String contentType, int status, String body) { + Map> headers = new HashMap<>(); + if (contentType != null) { + headers.put("Content-Type", Collections.singletonList(contentType)); + } + Response.Builder builder = + Response.builder() + .status(status) + .reason("OK") + .headers(headers) + .request( + Request.create( + HttpMethod.GET, "/api", Collections.emptyMap(), null, Util.UTF_8, null)); + if (body != null) { + builder.body(body, Util.UTF_8); + } + return builder.build(); + } + + @Test + void routesToTheDecoderThatDeclaresItCanHandleTheResponse() throws IOException { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + + assertThat(decoder.decode(responseWithContentType("application/json"), String.class)) + .isEqualTo("json"); + assertThat(json.invoked).isTrue(); + assertThat(fallback.invoked).isFalse(); + } + + @Test + void pairsAPredicateWithADecoderThatDoesNotDeclareItself() throws IOException { + RecordingDecoder xml = new RecordingDecoder("xml"); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = + MultiDecoder.builder(fallback).add(DecoderPredicate.xmlContentType(), xml).build(); + + assertThat(decoder.decode(responseWithContentType("application/xml"), String.class)) + .isEqualTo("xml"); + assertThat(fallback.invoked).isFalse(); + } + + @Test + void mixesSelfDeclaringDecodersAndPairs() throws IOException { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); + RecordingDecoder xml = new RecordingDecoder("xml"); + RecordingDecoder csv = new RecordingDecoder("csv"); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = + MultiDecoder.builder(fallback) + .add(json) + .add(DecoderPredicate.xmlContentType(), xml) + .add(DecoderPredicate.contentType("text/csv"), csv) + .build(); + + assertThat(decoder.decode(responseWithContentType("text/csv;charset=utf-8"), String.class)) + .isEqualTo("csv"); + assertThat(json.invoked).isFalse(); + assertThat(xml.invoked).isFalse(); + assertThat(fallback.invoked).isFalse(); + } + + @Test + void matchesSuffixedContentTypes() throws IOException { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + + assertThat(decoder.decode(responseWithContentType("application/vnd.github+json"), String.class)) + .isEqualTo("json"); + } + + @Test + void fallsBackToTheDefaultDecoderWhenNoDelegateAccepts() throws IOException { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + + assertThat(decoder.decode(responseWithContentType("text/plain"), String.class)) + .isEqualTo("fallback"); + assertThat(json.invoked).isFalse(); + } + + @Test + void fallsBackWhenTheResponseCarriesNoContentType() throws IOException { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + + assertThat(decoder.decode(responseWithContentType(null), String.class)).isEqualTo("fallback"); + } + + @Test + void consultsDelegatesInTheOrderTheyWereAdded() throws IOException { + RecordingDecoder first = new RecordingDecoder("first"); + RecordingDecoder second = new RecordingDecoder("second"); + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = + MultiDecoder.builder(fallback) + .add(DecoderPredicate.jsonContentType(), first) + .add(DecoderPredicate.jsonContentType(), second) + .build(); + + assertThat(decoder.decode(responseWithContentType("application/json"), String.class)) + .isEqualTo("first"); + assertThat(second.invoked).isFalse(); + } + + @Test + void aBareLambdaIsADecoderThatAcceptsEverything() throws IOException { + PredicatedDecoder anything = (response, type) -> "anything"; + RecordingDecoder fallback = new RecordingDecoder("fallback"); + + Decoder decoder = MultiDecoder.builder(fallback).add(anything).build(); + + assertThat(decoder.decode(responseWithContentType("text/plain"), String.class)) + .isEqualTo("anything"); + assertThat(fallback.invoked).isFalse(); + } + + @Test + void propagatesIoExceptionsFromTheSelectedDecoder() { + Decoder failing = + (response, type) -> { + throw new IOException("boom"); + }; + + Decoder decoder = + MultiDecoder.builder(new RecordingDecoder("fallback")) + .add(DecoderPredicate.jsonContentType(), failing) + .build(); + + assertThatThrownBy( + () -> decoder.decode(responseWithContentType("application/json"), String.class)) + .isInstanceOf(IOException.class) + .hasMessage("boom"); + } + + @Test + void rejectsANullDefaultDecoder() { + assertThatThrownBy(() -> MultiDecoder.builder(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("defaultDecoder cannot be null"); + } + + @Test + void rejectsNullDelegates() { + MultiDecoder.Builder builder = MultiDecoder.builder(new RecordingDecoder("fallback")); + + assertThatThrownBy(() -> builder.add((PredicatedDecoder) null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("decoder cannot be null"); + assertThatThrownBy(() -> builder.add(null, new RecordingDecoder("x"))) + .isInstanceOf(NullPointerException.class) + .hasMessage("predicate cannot be null"); + assertThatThrownBy(() -> builder.add(DecoderPredicate.jsonContentType(), null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("decoder cannot be null"); + } + + @Test + void describesItsDelegates() { + Decoder decoder = + MultiDecoder.builder( + new RecordingDecoder("fallback") { + @Override + public String toString() { + return "fallback"; + } + }) + .add( + DecoderPredicate.jsonContentType(), + new RecordingDecoder("json") { + @Override + public String toString() { + return "json"; + } + }) + .build(); + + assertThat(decoder.toString()) + .isEqualTo("MultiDecoder{defaultDecoder=fallback, delegates=[json]}"); + } +} From 02078d3821253f73faa15953a9cd0bf84d2a89e9 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 11:28:26 -0300 Subject: [PATCH 4/6] Expose and document multi-decoder configuration Signed-off-by: Marvin Froeder --- README.md | 76 +++++++ core/src/main/java/feign/BaseBuilder.java | 28 +++ .../codec/MultiDecoderCapabilityTest.java | 207 ++++++++++++++++++ src/docs/overview-mindmap.iuml | 1 + 4 files changed, 312 insertions(+) create mode 100644 core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java diff --git a/README.md b/README.md index cbaae10766..12f1b7601b 100644 --- a/README.md +++ b/README.md @@ -663,6 +663,82 @@ public class Example { } ``` +#### Multiple decoders + +> This API is `@Experimental` and may change incompatibly, or be removed, in a future release. + +A single client sometimes has to read more than one format — JSON for most endpoints, XML for +a legacy one, plain text for a health check. `MultiDecoder` routes each response to the right +decoder, falling back to a default when none applies. + +Most first-party decoders already declare what they can handle, so they can simply be added: + +```java +interface MixedClient { + @RequestLine("GET /orders/{id}") + Order order(@Param("id") String id); + + @RequestLine("GET /legacy/orders/{id}") + Order legacyOrder(@Param("id") String id); +} + +public class Example { + public static void main(String[] args) { + MixedClient client = Feign.builder() + .decoder(new DefaultDecoder(), new GsonDecoder(), new JAXBDecoder()) + .target(MixedClient.class, "https://foo.com"); + } +} +``` + +The first argument is the default decoder, used when nothing else accepts the response. Routing is +driven by what the server actually sent back, so a client that talks to endpoints answering +`application/json` and `application/xml` no longer needs one Feign instance per format. + +For a decoder that does not declare itself — including one you do not control — pair it +with a `DecoderPredicate` using the builder: + +```java +Decoder decoder = + MultiDecoder.builder(new DefaultDecoder()) + .add(new GsonDecoder()) // declares itself + .add(DecoderPredicate.xmlContentType(), someXmlDecoder) // paired + .add((response, type) -> type == byte[].class, binaryDecoder) + .build(); +``` + +Delegates are consulted in the order they were added, so put the narrowest predicate first. + +##### Declaring your own decoder + +Implement `PredicatedDecoder` alongside `Decoder` and override `canDecode`: + +```java +public class MyDecoder implements Decoder, PredicatedDecoder { + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } + + @Override + public Object decode(Response response, Type type) throws IOException { + // ... + } +} +``` + +`DecoderPredicate` ships with `jsonContentType()`, `xmlContentType()`, `contentType(mediaType)`, +`emptyBody()`, `status(codes...)` and `returnType(type)`, plus `and`/`or`/`negate` to combine them. + +**Predicates must not read the response body.** For most clients it is a single-pass stream, so +consuming it in `canDecode` would leave nothing for the decoder that is eventually chosen. Decide +on the status, the headers and the expected type instead. + +**If you wrap a decoder, forward `canDecode` to your delegate.** A wrapper that does not will claim +every response, because the default `canDecode` accepts everything. `OptionalDecoder` and the +metrics modules' `MeteredDecoder` forward for exactly this reason. + ### Encoders The simplest way to send a request body to a server is to define a `POST` method that has a `String` or `byte[]` parameter without any annotations on it. You will likely need to add a `Content-Type` header. diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index 754fcd3067..59526ace06 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -27,6 +27,8 @@ import feign.codec.DefaultErrorDecoder; import feign.codec.Encoder; import feign.codec.ErrorDecoder; +import feign.codec.MultiDecoder; +import feign.codec.PredicatedDecoder; import feign.interceptor.MethodInterceptor; import feign.interceptor.MethodInterceptors; import java.lang.reflect.Field; @@ -99,6 +101,32 @@ public B decoder(Decoder decoder) { return thisB(); } + /** + * Configures a {@link MultiDecoder} built from decoders that declare their own applicability. + * + *

Each {@link PredicatedDecoder} is consulted in the order given; {@code defaultDecoder} is + * the fallback used when none accepts the response. + * + * 

+   * Feign.builder()
+   *     .decoder(new DefaultDecoder(), new JacksonDecoder(), new JAXBDecoder())
+   *
+ * + *

To pair a predicate with a decoder that does not implement {@link PredicatedDecoder}, use + * {@link MultiDecoder#builder(Decoder)} instead. + * + * @param defaultDecoder the decoder used when no delegate accepts the response + * @param decoders the predicated decoders, consulted in the order given + */ + @Experimental + public B decoder(Decoder defaultDecoder, PredicatedDecoder... decoders) { + MultiDecoder.Builder builder = MultiDecoder.builder(defaultDecoder); + for (PredicatedDecoder decoder : decoders) { + builder.add(decoder); + } + return decoder(builder.build()); + } + public B codec(Codec codec) { this.encoder = codec.encoder(); this.decoder = codec.decoder(); diff --git a/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java b/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java new file mode 100644 index 0000000000..024a3c94ad --- /dev/null +++ b/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java @@ -0,0 +1,207 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import static org.assertj.core.api.Assertions.assertThat; + +import feign.Capability; +import feign.Feign; +import feign.Param; +import feign.Request; +import feign.Request.HttpMethod; +import feign.RequestLine; +import feign.Response; +import feign.Util; +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** How {@link MultiDecoder} behaves end to end and when a {@link Capability} wraps the decoder. */ +class MultiDecoderCapabilityTest { + + interface MixedApi { + @RequestLine("GET /{path}") + String get(@Param("path") String path); + } + + static class TaggingDecoder implements Decoder { + private final String tag; + + TaggingDecoder(String tag) { + this.tag = tag; + } + + @Override + public Object decode(Response response, Type type) { + return tag; + } + } + + /** A capability that wraps the decoder, the way the metrics modules do. */ + public static class CountingCapability implements Capability { + int wrapped; + int decodeCalls; + + @Override + public Decoder enrich(Decoder decoder) { + wrapped++; + return (response, type) -> { + decodeCalls++; + return decoder.decode(response, type); + }; + } + } + + private static Response response(String contentType, String body) { + return response( + contentType, + body, + Request.create( + HttpMethod.GET, "http://localhost:1/", Collections.emptyMap(), null, Util.UTF_8, null)); + } + + private static Response response(String contentType, String body, Request request) { + Map> headers = new HashMap<>(); + headers.put("Content-Type", Collections.singletonList(contentType)); + return Response.builder() + .status(200) + .reason("OK") + .headers(headers) + .body(body, Util.UTF_8) + .request(request) + .build(); + } + + private static MixedApi target(Feign.Builder builder, Map contentTypes) { + return builder + .client( + (request, options) -> { + String path = request.url().substring(request.url().lastIndexOf('/') + 1); + return response(contentTypes.get(path), "payload", request); + }) + .target(MixedApi.class, "http://localhost:1"); + } + + @Test + void capabilityWrapsTheCompositeAndRoutingStillWorks() { + CountingCapability capability = new CountingCapability(); + Map contentTypes = new HashMap<>(); + contentTypes.put("json", "application/json"); + contentTypes.put("xml", "application/xml"); + + MixedApi api = + target( + Feign.builder() + .decoder( + MultiDecoder.builder(new TaggingDecoder("fallback")) + .add(DecoderPredicate.jsonContentType(), new TaggingDecoder("json")) + .add(DecoderPredicate.xmlContentType(), new TaggingDecoder("xml")) + .build()) + .addCapability(capability), + contentTypes); + + assertThat(api.get("json")).isEqualTo("json"); + assertThat(api.get("xml")).isEqualTo("xml"); + + // the capability sees the MultiDecoder as one decoder, not one per delegate + assertThat(capability.wrapped).isEqualTo(1); + assertThat(capability.decodeCalls).isEqualTo(2); + } + + @Test + void builderShorthandRoutesToSelfDeclaringDecoders() { + Map contentTypes = new HashMap<>(); + contentTypes.put("json", "application/json"); + contentTypes.put("csv", "text/csv"); + + MixedApi api = + target( + Feign.builder().decoder(new TaggingDecoder("fallback"), new SelfDeclaringJsonDecoder()), + contentTypes); + + assertThat(api.get("json")).isEqualTo("json"); + assertThat(api.get("csv")).isEqualTo("fallback"); + } + + /** The selected decoder still receives an unread body: predicates must not consume it. */ + @Test + void predicatesLeaveTheBodyForTheSelectedDecoder() throws IOException { + Decoder decoder = + MultiDecoder.builder(new TaggingDecoder("fallback")) + .add( + DecoderPredicate.jsonContentType(), + (response, type) -> Util.toString(response.body().asReader(Util.UTF_8))) + .build(); + + assertThat(decoder.decode(response("application/json", "payload"), String.class)) + .isEqualTo("payload"); + } + + static class SelfDeclaringJsonDecoder implements Decoder, PredicatedDecoder { + + @Override + public Object decode(Response response, Type type) { + return "json"; + } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } + } + + /** + * A wrapper that does not forward {@code canDecode} claims every response, which is why the + * metrics modules' {@code MeteredDecoder} forwards it to its delegate. + */ + @Test + void wrappingWithoutForwardingCanDecodeErasesSelfDeclaration() throws IOException { + PredicatedDecoder jsonOnly = new SelfDeclaringJsonDecoder(); + + PredicatedDecoder naive = jsonOnly::decode; + + PredicatedDecoder forwarding = + new PredicatedDecoder() { + @Override + public boolean canDecode(Response response, Type type) { + return jsonOnly.canDecode(response, type); + } + + @Override + public Object decode(Response response, Type type) throws IOException { + return jsonOnly.decode(response, type); + } + }; + + assertThat( + MultiDecoder.builder(new TaggingDecoder("fallback")) + .add(naive) + .build() + .decode(response("application/xml", "payload"), String.class)) + .isEqualTo("json"); + + assertThat( + MultiDecoder.builder(new TaggingDecoder("fallback")) + .add(forwarding) + .build() + .decode(response("application/xml", "payload"), String.class)) + .isEqualTo("fallback"); + } +} diff --git a/src/docs/overview-mindmap.iuml b/src/docs/overview-mindmap.iuml index afd6aefbf3..b567ede3e8 100644 --- a/src/docs/overview-mindmap.iuml +++ b/src/docs/overview-mindmap.iuml @@ -31,6 +31,7 @@ left side ** encoders/decoders +*** Multi decoder (predicate based, experimental) *** GSON *** JAXB *** JAXB Jakarta From 93d52d647d2bcbdd1b9fd672384cb1990ee7108b Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 11:28:35 -0300 Subject: [PATCH 5/6] Declare applicability on the first-party decoders Signed-off-by: Marvin Froeder --- CHANGELOG.md | 9 +++++++++ .../main/java/feign/optionals/OptionalDecoder.java | 13 ++++++++++++- .../main/java/feign/metrics4/MeteredDecoder.java | 9 ++++++++- .../main/java/feign/metrics5/MeteredDecoder.java | 9 ++++++++- .../main/java/feign/fastjson2/Fastjson2Decoder.java | 8 +++++++- gson/src/main/java/feign/gson/GsonDecoder.java | 8 +++++++- .../feign/jackson/jaxb/JacksonJaxbJsonDecoder.java | 8 +++++++- .../java/feign/jackson/jr/JacksonJrDecoder.java | 9 ++++++++- .../src/main/java/feign/jackson/JacksonDecoder.java | 8 +++++++- .../main/java/feign/jackson3/Jackson3Decoder.java | 8 +++++++- .../src/main/java/feign/jaxb/JAXBDecoder.java | 8 +++++++- jaxb/src/main/java/feign/jaxb/JAXBDecoder.java | 8 +++++++- json/src/main/java/feign/json/JsonDecoder.java | 8 +++++++- .../main/java/feign/micrometer/MeteredDecoder.java | 9 ++++++++- moshi/src/main/java/feign/moshi/MoshiDecoder.java | 8 +++++++- sax/src/main/java/feign/sax/SAXDecoder.java | 8 +++++++- .../src/main/java/feign/soap/SOAPDecoder.java | 8 +++++++- soap/src/main/java/feign/soap/SOAPDecoder.java | 8 +++++++- 18 files changed, 137 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 513e923ab2..eac92e6416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ ### Version 13.14 +* Add `@Experimental` `MultiDecoder`, `PredicatedDecoder` and `DecoderPredicate`, letting a single + client route each response to the right decoder. Decoders declare what they can handle by + implementing `PredicatedDecoder`; anything else is paired with a predicate via + `MultiDecoder.builder(defaultDecoder)`. The first-party JSON decoders (Gson, Jackson, Jackson 3, + Jackson Jr, Jackson JAXB, Moshi, Fastjson2, JSON-java) and XML decoders (JAXB, JAXB Jakarta, SAX, + SOAP, SOAP Jakarta) now declare themselves, and `OptionalDecoder` and the metrics modules' + `MeteredDecoder` forward `canDecode` to the decoder they wrap. The `Decoder` interface is + unchanged, so existing decoders keep working. + * `JAXBContextFactory.withProperty` is now applied when creating Unmarshallers, not only Marshallers. Marshaller-only properties are skipped on unmarshal (#3056). * Add support for the HTTP QUERY method (RFC 10008) — safe, idempotent, and cacheable with a diff --git a/core/src/main/java/feign/optionals/OptionalDecoder.java b/core/src/main/java/feign/optionals/OptionalDecoder.java index 475ee74b95..0edb3bedeb 100644 --- a/core/src/main/java/feign/optionals/OptionalDecoder.java +++ b/core/src/main/java/feign/optionals/OptionalDecoder.java @@ -18,13 +18,14 @@ import feign.Response; import feign.Util; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.Objects; import java.util.Optional; -public final class OptionalDecoder implements Decoder { +public final class OptionalDecoder implements Decoder, PredicatedDecoder { final Decoder delegate; public OptionalDecoder(Decoder delegate) { @@ -44,6 +45,16 @@ public Object decode(Response response, Type type) throws IOException { return Optional.ofNullable(delegate.decode(response, enclosedType)); } + @Override + public boolean canDecode(Response response, Type type) { + if (!(delegate instanceof PredicatedDecoder)) { + return true; + } + Type enclosedType = + isOptional(type) ? Util.resolveLastTypeParameter(type, Optional.class) : type; + return ((PredicatedDecoder) delegate).canDecode(response, enclosedType); + } + static boolean isOptional(Type type) { if (!(type instanceof ParameterizedType)) { return false; diff --git a/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredDecoder.java b/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredDecoder.java index e47fbe852d..b51d154b27 100644 --- a/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredDecoder.java +++ b/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredDecoder.java @@ -22,11 +22,12 @@ import feign.Response; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.lang.reflect.Type; /** Warp feign {@link Decoder} with metrics. */ -public class MeteredDecoder implements Decoder { +public class MeteredDecoder implements Decoder, PredicatedDecoder { private final Decoder decoder; private final MetricRegistry metricRegistry; @@ -73,4 +74,10 @@ public Object decode(Response response, Type type) return decoded; } + + @Override + public boolean canDecode(Response response, Type type) { + return !(decoder instanceof PredicatedDecoder) + || ((PredicatedDecoder) decoder).canDecode(response, type); + } } diff --git a/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredDecoder.java b/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredDecoder.java index 653d29e62e..5f56b57701 100644 --- a/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredDecoder.java +++ b/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredDecoder.java @@ -20,6 +20,7 @@ import feign.Response; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import feign.utils.ExceptionUtils; import io.dropwizard.metrics5.MetricRegistry; import io.dropwizard.metrics5.Timer.Context; @@ -28,7 +29,7 @@ import java.util.Map; /** Warp feign {@link Decoder} with metrics. */ -public class MeteredDecoder implements Decoder { +public class MeteredDecoder implements Decoder, PredicatedDecoder { private final Decoder decoder; private final MetricRegistry metricRegistry; @@ -110,4 +111,10 @@ public Object decode(Response response, Type type) return decoded; } + + @Override + public boolean canDecode(Response response, Type type) { + return !(decoder instanceof PredicatedDecoder) + || ((PredicatedDecoder) decoder).canDecode(response, type); + } } diff --git a/fastjson2/src/main/java/feign/fastjson2/Fastjson2Decoder.java b/fastjson2/src/main/java/feign/fastjson2/Fastjson2Decoder.java index 80b1ada8d7..e8631ae0c5 100644 --- a/fastjson2/src/main/java/feign/fastjson2/Fastjson2Decoder.java +++ b/fastjson2/src/main/java/feign/fastjson2/Fastjson2Decoder.java @@ -26,6 +26,7 @@ import feign.Util; import feign.codec.Decoder; import feign.codec.JsonDecoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.io.Reader; import java.lang.reflect.Type; @@ -33,7 +34,7 @@ /** * @author changjin wei(魏昌进) */ -public class Fastjson2Decoder implements Decoder, JsonDecoder { +public class Fastjson2Decoder implements Decoder, PredicatedDecoder, JsonDecoder { private final JSONReader.Feature[] features; @@ -69,4 +70,9 @@ public Object convert(Object object, Type type) { } return JSON.parseObject(JSON.toJSONString(object), type); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/gson/src/main/java/feign/gson/GsonDecoder.java b/gson/src/main/java/feign/gson/GsonDecoder.java index 5fa6ee0369..8a908087c1 100644 --- a/gson/src/main/java/feign/gson/GsonDecoder.java +++ b/gson/src/main/java/feign/gson/GsonDecoder.java @@ -24,12 +24,13 @@ import feign.Util; import feign.codec.Decoder; import feign.codec.JsonDecoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.io.Reader; import java.lang.reflect.Type; import java.util.Collections; -public class GsonDecoder implements Decoder, JsonDecoder { +public class GsonDecoder implements Decoder, PredicatedDecoder, JsonDecoder { private final Gson gson; @@ -66,4 +67,9 @@ public Object decode(Response response, Type type) throws IOException { public Object convert(Object object, Type type) { return gson.fromJson(gson.toJsonTree(object), type); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java b/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java index ed1cb12a2c..98a3e0e833 100644 --- a/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java +++ b/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java @@ -24,10 +24,11 @@ import feign.Response; import feign.Util; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.lang.reflect.Type; -public final class JacksonJaxbJsonDecoder implements Decoder { +public final class JacksonJaxbJsonDecoder implements Decoder, PredicatedDecoder { private final JacksonJaxbJsonProvider jacksonJaxbJsonProvider; public JacksonJaxbJsonDecoder() { @@ -45,4 +46,9 @@ public Object decode(Response response, Type type) throws IOException, FeignExce return jacksonJaxbJsonProvider.readFrom( Object.class, type, null, APPLICATION_JSON_TYPE, null, response.body().asInputStream()); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrDecoder.java b/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrDecoder.java index 3edb1f8dda..7d4581f5f2 100644 --- a/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrDecoder.java +++ b/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrDecoder.java @@ -23,6 +23,7 @@ import feign.codec.DecodeException; import feign.codec.Decoder; import feign.codec.JsonDecoder; +import feign.codec.PredicatedDecoder; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; @@ -34,7 +35,8 @@ /** * A {@link JsonDecoder} that uses Jackson Jr to convert objects to String or byte representation. */ -public class JacksonJrDecoder extends JacksonJrMapper implements Decoder, JsonDecoder { +public class JacksonJrDecoder extends JacksonJrMapper + implements Decoder, PredicatedDecoder, JsonDecoder { @FunctionalInterface protected interface Transformer { @@ -134,4 +136,9 @@ public Object convert(Object object, Type type) throws IOException { } throw new IOException("Cannot convert to type: " + type.getTypeName()); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/jackson/src/main/java/feign/jackson/JacksonDecoder.java b/jackson/src/main/java/feign/jackson/JacksonDecoder.java index 370f745dc7..db5a0dbe58 100644 --- a/jackson/src/main/java/feign/jackson/JacksonDecoder.java +++ b/jackson/src/main/java/feign/jackson/JacksonDecoder.java @@ -23,13 +23,14 @@ import feign.Util; import feign.codec.Decoder; import feign.codec.JsonDecoder; +import feign.codec.PredicatedDecoder; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; import java.lang.reflect.Type; import java.util.Collections; -public class JacksonDecoder implements Decoder, JsonDecoder { +public class JacksonDecoder implements Decoder, PredicatedDecoder, JsonDecoder { private final ObjectMapper mapper; @@ -76,4 +77,9 @@ public Object decode(Response response, Type type) throws IOException { public Object convert(Object object, Type type) { return mapper.convertValue(object, mapper.constructType(type)); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/jackson3/src/main/java/feign/jackson3/Jackson3Decoder.java b/jackson3/src/main/java/feign/jackson3/Jackson3Decoder.java index 5726d582bd..363b17287e 100644 --- a/jackson3/src/main/java/feign/jackson3/Jackson3Decoder.java +++ b/jackson3/src/main/java/feign/jackson3/Jackson3Decoder.java @@ -19,6 +19,7 @@ import feign.Util; import feign.codec.Decoder; import feign.codec.JsonDecoder; +import feign.codec.PredicatedDecoder; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; @@ -29,7 +30,7 @@ import tools.jackson.databind.JacksonModule; import tools.jackson.databind.json.JsonMapper; -public class Jackson3Decoder implements Decoder, JsonDecoder { +public class Jackson3Decoder implements Decoder, PredicatedDecoder, JsonDecoder { private final JsonMapper mapper; @@ -77,4 +78,9 @@ public Object decode(Response response, Type type) throws IOException { public Object convert(Object object, Type type) { return mapper.convertValue(object, mapper.constructType(type)); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/jaxb-jakarta/src/main/java/feign/jaxb/JAXBDecoder.java b/jaxb-jakarta/src/main/java/feign/jaxb/JAXBDecoder.java index 6a40861cd1..feaf8237b5 100644 --- a/jaxb-jakarta/src/main/java/feign/jaxb/JAXBDecoder.java +++ b/jaxb-jakarta/src/main/java/feign/jaxb/JAXBDecoder.java @@ -19,6 +19,7 @@ import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import jakarta.xml.bind.JAXBException; import java.io.IOException; import java.lang.reflect.ParameterizedType; @@ -48,7 +49,7 @@ *

The JAXBContextFactory should be reused across requests as it caches the created JAXB * contexts. */ -public class JAXBDecoder implements Decoder { +public class JAXBDecoder implements Decoder, PredicatedDecoder { private final JAXBContextFactory jaxbContextFactory; private final boolean namespaceAware; @@ -123,4 +124,9 @@ public JAXBDecoder build() { return new JAXBDecoder(this); } } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isXmlContentType(response); + } } diff --git a/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java b/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java index 9d998d26dc..a7132472ec 100644 --- a/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java +++ b/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java @@ -19,6 +19,7 @@ import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; @@ -48,7 +49,7 @@ *

The JAXBContextFactory should be reused across requests as it caches the created JAXB * contexts. */ -public class JAXBDecoder implements Decoder { +public class JAXBDecoder implements Decoder, PredicatedDecoder { private final JAXBContextFactory jaxbContextFactory; private final boolean namespaceAware; @@ -123,4 +124,9 @@ public JAXBDecoder build() { return new JAXBDecoder(this); } } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isXmlContentType(response); + } } diff --git a/json/src/main/java/feign/json/JsonDecoder.java b/json/src/main/java/feign/json/JsonDecoder.java index edf7fd80f0..5f0a264869 100644 --- a/json/src/main/java/feign/json/JsonDecoder.java +++ b/json/src/main/java/feign/json/JsonDecoder.java @@ -21,6 +21,7 @@ import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; @@ -53,7 +54,7 @@ * System.out.println(contributors.getJSONObject(0).getString("login")); * */ -public class JsonDecoder implements Decoder, feign.codec.JsonDecoder { +public class JsonDecoder implements Decoder, PredicatedDecoder, feign.codec.JsonDecoder { @Override public Object decode(Response response, Type type) throws IOException, DecodeException { @@ -114,4 +115,9 @@ public Object convert(Object object, Type type) throws IOException { } throw new IOException(type.getTypeName() + " is not a type supported by this decoder."); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/micrometer/src/main/java/feign/micrometer/MeteredDecoder.java b/micrometer/src/main/java/feign/micrometer/MeteredDecoder.java index 65b8067ebc..926452254e 100644 --- a/micrometer/src/main/java/feign/micrometer/MeteredDecoder.java +++ b/micrometer/src/main/java/feign/micrometer/MeteredDecoder.java @@ -19,6 +19,7 @@ import feign.RequestTemplate; import feign.Response; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import feign.utils.ExceptionUtils; import io.micrometer.core.instrument.*; import java.io.IOException; @@ -26,7 +27,7 @@ import java.util.Optional; /** Wrap feign {@link Decoder} with metrics. */ -public class MeteredDecoder implements Decoder { +public class MeteredDecoder implements Decoder, PredicatedDecoder { private final Decoder decoder; private final MeterRegistry meterRegistry; @@ -117,4 +118,10 @@ protected Tag[] extraTags(Response response, Type type, Exception e) { RequestTemplate template = response.request().requestTemplate(); return new Tag[] {Tag.of("uri", template.methodMetadata().template().path())}; } + + @Override + public boolean canDecode(Response response, Type type) { + return !(decoder instanceof PredicatedDecoder) + || ((PredicatedDecoder) decoder).canDecode(response, type); + } } diff --git a/moshi/src/main/java/feign/moshi/MoshiDecoder.java b/moshi/src/main/java/feign/moshi/MoshiDecoder.java index ac08ee96a4..9f4e006f42 100644 --- a/moshi/src/main/java/feign/moshi/MoshiDecoder.java +++ b/moshi/src/main/java/feign/moshi/MoshiDecoder.java @@ -22,12 +22,13 @@ import feign.Util; import feign.codec.Decoder; import feign.codec.JsonDecoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.lang.reflect.Type; import okio.BufferedSource; import okio.Okio; -public class MoshiDecoder implements Decoder, JsonDecoder { +public class MoshiDecoder implements Decoder, PredicatedDecoder, JsonDecoder { private final Moshi moshi; public MoshiDecoder(Moshi moshi) { @@ -67,4 +68,9 @@ public Object convert(Object object, Type type) throws IOException { JsonAdapter adapter = moshi.adapter(type); return adapter.fromJsonValue(object); } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isJsonContentType(response); + } } diff --git a/sax/src/main/java/feign/sax/SAXDecoder.java b/sax/src/main/java/feign/sax/SAXDecoder.java index 6aa799d0a2..20da34bea1 100644 --- a/sax/src/main/java/feign/sax/SAXDecoder.java +++ b/sax/src/main/java/feign/sax/SAXDecoder.java @@ -24,6 +24,7 @@ import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Constructor; @@ -53,7 +54,7 @@ * .target(MyApi.class, "http://api"); * */ -public class SAXDecoder implements Decoder { +public class SAXDecoder implements Decoder, PredicatedDecoder { private final Map> handlerFactories; @@ -176,4 +177,9 @@ public ContentHandlerWithResult create() { } } } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isXmlContentType(response); + } } diff --git a/soap-jakarta/src/main/java/feign/soap/SOAPDecoder.java b/soap-jakarta/src/main/java/feign/soap/SOAPDecoder.java index 37386f0268..619ce01d7f 100644 --- a/soap-jakarta/src/main/java/feign/soap/SOAPDecoder.java +++ b/soap-jakarta/src/main/java/feign/soap/SOAPDecoder.java @@ -19,6 +19,7 @@ import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import feign.jaxb.JAXBContextFactory; import jakarta.xml.bind.JAXBException; import jakarta.xml.bind.Unmarshaller; @@ -75,7 +76,7 @@ * @see SOAPErrorDecoder * @see SOAPFaultException */ -public class SOAPDecoder implements Decoder { +public class SOAPDecoder implements Decoder, PredicatedDecoder { private final JAXBContextFactory jaxbContextFactory; private final String soapProtocol; @@ -175,4 +176,9 @@ public SOAPDecoder build() { return new SOAPDecoder(this); } } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isXmlContentType(response); + } } diff --git a/soap/src/main/java/feign/soap/SOAPDecoder.java b/soap/src/main/java/feign/soap/SOAPDecoder.java index 8079a622a4..bf63ba4c27 100644 --- a/soap/src/main/java/feign/soap/SOAPDecoder.java +++ b/soap/src/main/java/feign/soap/SOAPDecoder.java @@ -19,6 +19,7 @@ import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; +import feign.codec.PredicatedDecoder; import feign.jaxb.JAXBContextFactory; import java.io.IOException; import java.lang.reflect.ParameterizedType; @@ -79,7 +80,7 @@ * @see SOAPErrorDecoder * @see SOAPFaultException */ -public class SOAPDecoder implements Decoder { +public class SOAPDecoder implements Decoder, PredicatedDecoder { private final JAXBContextFactory jaxbContextFactory; private final String soapProtocol; @@ -179,4 +180,9 @@ public SOAPDecoder build() { return new SOAPDecoder(this); } } + + @Override + public boolean canDecode(Response response, Type type) { + return Util.isXmlContentType(response); + } } From cfdd1935518226c53e81d0355649069e4cfba9ed Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 20 Aug 2026 12:12:44 -0300 Subject: [PATCH 6/6] Drop the multi-decoder default decoder in favour of an explicit any() predicate Signed-off-by: Marvin Froeder --- CHANGELOG.md | 5 +- README.md | 74 ++++++--- core/src/main/java/feign/BaseBuilder.java | 20 ++- .../java/feign/codec/DecoderPredicate.java | 71 +++++++-- .../main/java/feign/codec/MultiDecoder.java | 119 ++++++++------- .../main/java/feign/codec/PairedDecoder.java | 79 ++++++++++ .../java/feign/codec/PredicatedDecoder.java | 63 +++++++- .../codec/MultiDecoderCapabilityTest.java | 49 ++++-- .../java/feign/codec/MultiDecoderTest.java | 144 ++++++++++++------ 9 files changed, 467 insertions(+), 157 deletions(-) create mode 100644 core/src/main/java/feign/codec/PairedDecoder.java diff --git a/CHANGELOG.md b/CHANGELOG.md index eac92e6416..49b183fbfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,10 @@ * Add `@Experimental` `MultiDecoder`, `PredicatedDecoder` and `DecoderPredicate`, letting a single client route each response to the right decoder. Decoders declare what they can handle by implementing `PredicatedDecoder`; anything else is paired with a predicate via - `MultiDecoder.builder(defaultDecoder)`. The first-party JSON decoders (Gson, Jackson, Jackson 3, + `PredicatedDecoder.of(predicate, decoder)` or `MultiDecoder.builder()`. Decoders are consulted in + the order given and a response nothing accepts fails with a `DecodeException` naming what was + tried, so a default is a decoder guarded by `DecoderPredicate.any()` listed last. The first-party + JSON decoders (Gson, Jackson, Jackson 3, Jackson Jr, Jackson JAXB, Moshi, Fastjson2, JSON-java) and XML decoders (JAXB, JAXB Jakarta, SAX, SOAP, SOAP Jakarta) now declare themselves, and `OptionalDecoder` and the metrics modules' `MeteredDecoder` forward `canDecode` to the decoder they wrap. The `Decoder` interface is diff --git a/README.md b/README.md index 12f1b7601b..c7d0e0390b 100644 --- a/README.md +++ b/README.md @@ -668,10 +668,11 @@ public class Example { > This API is `@Experimental` and may change incompatibly, or be removed, in a future release. A single client sometimes has to read more than one format — JSON for most endpoints, XML for -a legacy one, plain text for a health check. `MultiDecoder` routes each response to the right -decoder, falling back to a default when none applies. +a legacy one, plain text for a health check. `MultiDecoder` hands each response to the first decoder +that accepts it. -Most first-party decoders already declare what they can handle, so they can simply be added: +Most first-party decoders already declare what they can handle, so they can simply be listed, in the +order they should be consulted: ```java interface MixedClient { @@ -685,36 +686,57 @@ interface MixedClient { public class Example { public static void main(String[] args) { MixedClient client = Feign.builder() - .decoder(new DefaultDecoder(), new GsonDecoder(), new JAXBDecoder()) + .decoders(new GsonDecoder(), new JAXBDecoder()) .target(MixedClient.class, "https://foo.com"); } } ``` -The first argument is the default decoder, used when nothing else accepts the response. Routing is -driven by what the server actually sent back, so a client that talks to endpoints answering -`application/json` and `application/xml` no longer needs one Feign instance per format. +Routing is driven by what the server actually sent back, so a client that talks to endpoints +answering `application/json` and `application/xml` no longer needs one Feign instance per format. -For a decoder that does not declare itself — including one you do not control — pair it -with a `DecoderPredicate` using the builder: +There is no implicit fallback. A response that no decoder accepts fails with a `DecodeException` +naming the decoders that were tried and what each one wants: + +``` +Unable to decode 200 response (Content-Type: text/plain) as com.example.Order. Decoders tried, in order: + - GsonDecoder + - JAXBDecoder +Add a decoder guarded by DecoderPredicate.any() last to act as a default. +``` + +To get a default, pair a decoder with the predicate that accepts everything and list it **last**: + +```java +Feign.builder() + .decoders( + new GsonDecoder(), + new JAXBDecoder(), + PredicatedDecoder.of(DecoderPredicate.any(), new DefaultDecoder())); +``` + +The same pairing works for any decoder that does not declare itself, including one you do not +control. `MultiDecoder.builder()` spells it out when a lambda reads better than a wrapper: ```java Decoder decoder = - MultiDecoder.builder(new DefaultDecoder()) - .add(new GsonDecoder()) // declares itself - .add(DecoderPredicate.xmlContentType(), someXmlDecoder) // paired + MultiDecoder.builder() + .add(new GsonDecoder()) // declares itself + .add(DecoderPredicate.xmlContentType(), someXmlDecoder) // paired .add((response, type) -> type == byte[].class, binaryDecoder) + .add(DecoderPredicate.any(), new DefaultDecoder()) // the default, last .build(); ``` -Delegates are consulted in the order they were added, so put the narrowest predicate first. +Decoders are consulted in the order they were added, so put the narrowest one first. ##### Declaring your own decoder -Implement `PredicatedDecoder` alongside `Decoder` and override `canDecode`: +Implement `PredicatedDecoder` and say what you handle. `canDecode` has no default: a decoder that +declares nothing would claim every response, which is rarely what its author meant. ```java -public class MyDecoder implements Decoder, PredicatedDecoder { +public class MyDecoder implements PredicatedDecoder { @Override public boolean canDecode(Response response, Type type) { @@ -728,16 +750,28 @@ public class MyDecoder implements Decoder, PredicatedDecoder { } ``` -`DecoderPredicate` ships with `jsonContentType()`, `xmlContentType()`, `contentType(mediaType)`, -`emptyBody()`, `status(codes...)` and `returnType(type)`, plus `and`/`or`/`negate` to combine them. +`DecoderPredicate` is the `@FunctionalInterface` here, so predicates can be lambdas. It ships with +`any()`, `jsonContentType()`, `xmlContentType()`, `contentType(mediaType)`, `emptyBody()`, +`status(codes...)` and `returnType(type)`, plus `and`/`or`/`negate` to combine them. Each one +describes itself, which is what shows up in the error message above; wrap your own lambdas in +`DecoderPredicate.describedAs("it is Tuesday", ...)` to read as well. + +`PredicatedDecoder.of(predicate, decoder)` replaces whatever the decoder says about itself, so it +can widen a decoder as well as narrow it. To keep the decoder's own declaration and add to it, use +`narrowing`: + +```java +// JSON responses as usual, but only when the call actually succeeded +PredicatedDecoder.narrowing(DecoderPredicate.status(200, 201), new GsonDecoder()); +``` **Predicates must not read the response body.** For most clients it is a single-pass stream, so consuming it in `canDecode` would leave nothing for the decoder that is eventually chosen. Decide on the status, the headers and the expected type instead. -**If you wrap a decoder, forward `canDecode` to your delegate.** A wrapper that does not will claim -every response, because the default `canDecode` accepts everything. `OptionalDecoder` and the -metrics modules' `MeteredDecoder` forward for exactly this reason. +**If you wrap a decoder, forward `canDecode` to your delegate**, otherwise wrapping silently changes +what the decoder handles. `OptionalDecoder` and the metrics modules' `MeteredDecoder` forward for +exactly this reason. ### Encoders The simplest way to send a request body to a server is to define a `POST` method that has a `String` or `byte[]` parameter without any annotations on it. You will likely need to add a `Content-Type` header. diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index 59526ace06..4e625bff6d 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -22,6 +22,7 @@ import feign.Request.Options; import feign.codec.Codec; import feign.codec.Decoder; +import feign.codec.DecoderPredicate; import feign.codec.DefaultDecoder; import feign.codec.DefaultEncoder; import feign.codec.DefaultErrorDecoder; @@ -104,23 +105,28 @@ public B decoder(Decoder decoder) { /** * Configures a {@link MultiDecoder} built from decoders that declare their own applicability. * - *

Each {@link PredicatedDecoder} is consulted in the order given; {@code defaultDecoder} is - * the fallback used when none accepts the response. + *

Decoders are consulted in the order given, and the first one that accepts the response + * decodes it. There is no implicit fallback: pair a decoder with {@link DecoderPredicate#any()} + * and list it last to act as a default, otherwise a response nothing accepts fails with a {@link + * feign.codec.DecodeException}. * * 

    * Feign.builder()
-   *     .decoder(new DefaultDecoder(), new JacksonDecoder(), new JAXBDecoder())
+   *     .decoders(
+   *         new JacksonDecoder(),
+   *         new JAXBDecoder(),
+   *         PredicatedDecoder.of(DecoderPredicate.any(), new DefaultDecoder()))
    *
* *

To pair a predicate with a decoder that does not implement {@link PredicatedDecoder}, use - * {@link MultiDecoder#builder(Decoder)} instead. + * {@link PredicatedDecoder#of(DecoderPredicate, Decoder)} as above, or {@link + * MultiDecoder#builder()} for the same thing spelled out. * - * @param defaultDecoder the decoder used when no delegate accepts the response * @param decoders the predicated decoders, consulted in the order given */ @Experimental - public B decoder(Decoder defaultDecoder, PredicatedDecoder... decoders) { - MultiDecoder.Builder builder = MultiDecoder.builder(defaultDecoder); + public B decoders(PredicatedDecoder... decoders) { + MultiDecoder.Builder builder = MultiDecoder.builder(); for (PredicatedDecoder decoder : decoders) { builder.add(decoder); } diff --git a/core/src/main/java/feign/codec/DecoderPredicate.java b/core/src/main/java/feign/codec/DecoderPredicate.java index 0727c0ebed..941d9cbab4 100644 --- a/core/src/main/java/feign/codec/DecoderPredicate.java +++ b/core/src/main/java/feign/codec/DecoderPredicate.java @@ -33,6 +33,10 @@ * for most clients, so consuming it here would leave nothing for the decoder that is eventually * chosen. * + *

Every predicate built here describes itself, so a {@link MultiDecoder} that cannot route a + * response can say what it did consider. Wrap your own lambdas in {@link #describedAs(String, + * DecoderPredicate)} to get the same in error messages. + * * @see PredicatedDecoder * @see MultiDecoder */ @@ -50,14 +54,48 @@ public interface DecoderPredicate { */ boolean canDecode(Response response, Type type); + /** + * Wraps a predicate so that it describes itself, which is what a {@link MultiDecoder} reports + * when no decoder accepts a response. + * + * @param description how the predicate reads in an error message, for example {@code + * "Content-Type is JSON"} + * @param predicate the predicate to describe + */ + static DecoderPredicate describedAs(String description, DecoderPredicate predicate) { + Objects.requireNonNull(description, "description cannot be null"); + Objects.requireNonNull(predicate, "predicate cannot be null"); + return new DecoderPredicate() { + + @Override + public boolean canDecode(Response response, Type type) { + return predicate.canDecode(response, type); + } + + @Override + public String toString() { + return description; + } + }; + } + + /** + * Matches every response. Pair this with a decoder registered last to make it the default of a + * {@link MultiDecoder}. + */ + static DecoderPredicate any() { + return describedAs("any response", (response, type) -> true); + } + /** Matches responses whose {@code Content-Type} header denotes JSON. */ static DecoderPredicate jsonContentType() { - return (response, type) -> Util.isJsonContentType(response); + return describedAs( + "Content-Type is JSON", (response, type) -> Util.isJsonContentType(response)); } /** Matches responses whose {@code Content-Type} header denotes XML. */ static DecoderPredicate xmlContentType() { - return (response, type) -> Util.isXmlContentType(response); + return describedAs("Content-Type is XML", (response, type) -> Util.isXmlContentType(response)); } /** @@ -66,40 +104,51 @@ static DecoderPredicate xmlContentType() { */ static DecoderPredicate contentType(String mediaType) { Objects.requireNonNull(mediaType, "mediaType cannot be null"); - return (response, type) -> Util.hasContentType(response, mediaType); + return describedAs( + "Content-Type is " + mediaType, + (response, type) -> Util.hasContentType(response, mediaType)); } /** Matches responses carrying no body, such as a {@code 204 No Content}. */ static DecoderPredicate emptyBody() { - return (response, type) -> - response.body() == null - || (response.body().length() != null && response.body().length() == 0); + return describedAs( + "body is empty", + (response, type) -> + response.body() == null + || (response.body().length() != null && response.body().length() == 0)); } /** Matches responses whose status is one of the given codes. */ static DecoderPredicate status(int... statuses) { int[] accepted = Arrays.copyOf(statuses, statuses.length); Arrays.sort(accepted); - return (response, type) -> Arrays.binarySearch(accepted, response.status()) >= 0; + return describedAs( + "status is one of " + Arrays.toString(accepted), + (response, type) -> Arrays.binarySearch(accepted, response.status()) >= 0); } /** Matches responses the caller expects to come back as exactly the given type. */ static DecoderPredicate returnType(Type expected) { Objects.requireNonNull(expected, "expected cannot be null"); - return (response, type) -> expected.equals(type); + return describedAs( + "return type is " + expected.getTypeName(), (response, type) -> expected.equals(type)); } default DecoderPredicate and(DecoderPredicate other) { Objects.requireNonNull(other, "other cannot be null"); - return (response, type) -> canDecode(response, type) && other.canDecode(response, type); + return describedAs( + "(" + this + " and " + other + ")", + (response, type) -> canDecode(response, type) && other.canDecode(response, type)); } default DecoderPredicate or(DecoderPredicate other) { Objects.requireNonNull(other, "other cannot be null"); - return (response, type) -> canDecode(response, type) || other.canDecode(response, type); + return describedAs( + "(" + this + " or " + other + ")", + (response, type) -> canDecode(response, type) || other.canDecode(response, type)); } default DecoderPredicate negate() { - return (response, type) -> !canDecode(response, type); + return describedAs("not (" + this + ")", (response, type) -> !canDecode(response, type)); } } diff --git a/core/src/main/java/feign/codec/MultiDecoder.java b/core/src/main/java/feign/codec/MultiDecoder.java index e895210238..f48ee1975e 100644 --- a/core/src/main/java/feign/codec/MultiDecoder.java +++ b/core/src/main/java/feign/codec/MultiDecoder.java @@ -18,33 +18,39 @@ import feign.Experimental; import feign.FeignException; import feign.Response; +import feign.Util; import java.io.IOException; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; /** - * A {@link Decoder} that selects a delegate per response, falling back to a default decoder when no - * delegate accepts it. + * A {@link Decoder} that hands each response to the first decoder that accepts it. * - *

Delegates come from two places. A decoder that implements {@link PredicatedDecoder} declares + *

Decoders come from two places. A decoder that implements {@link PredicatedDecoder} declares * its own applicability and can simply be added; any other decoder is paired with a {@link * DecoderPredicate} at the call site: * * 

  * Feign.builder()
  *     .decoder(
- *         MultiDecoder.builder(new DefaultDecoder())
+ *         MultiDecoder.builder()
  *             .add(new JacksonDecoder())
  *             .add(DecoderPredicate.xmlContentType(), new JAXBDecoder())
  *             .add((response, type) -> type == byte[].class, new BinaryDecoder())
+ *             .add(DecoderPredicate.any(), new DefaultDecoder())
  *             .build());
  *
* - *

Delegates are consulted in the order they were added, so the narrowest predicate should come - * first. The default decoder is consulted last. + *

Decoders are consulted in the order they were added, so the narrowest one comes first. There + * is no implicit fallback: a response no decoder accepts fails with a {@link DecodeException} + * naming what was tried. Add a decoder guarded by {@link DecoderPredicate#any()} last to act as a + * default, as above. * * @see PredicatedDecoder * @see DecoderPredicate @@ -52,77 +58,83 @@ @Experimental public class MultiDecoder implements Decoder { - private final Decoder defaultDecoder; + private final List decoders; - private final List delegates; - - private MultiDecoder(Decoder defaultDecoder, List delegates) { - this.defaultDecoder = defaultDecoder; - this.delegates = Collections.unmodifiableList(new ArrayList<>(delegates)); + private MultiDecoder(List decoders) { + this.decoders = Collections.unmodifiableList(new ArrayList<>(decoders)); } - /** - * Starts building a multi-decoder. - * - * @param defaultDecoder the decoder used when no delegate accepts the response - * @return the builder - */ - public static Builder builder(Decoder defaultDecoder) { - return new Builder(defaultDecoder); + /** Starts building a multi-decoder. */ + public static Builder builder() { + return new Builder(); } /** - * Decodes using the first delegate that accepts the response, or the default decoder if none do. + * Decodes using the first decoder that accepts the response. * * @param response {@inheritDoc} * @param type {@inheritDoc} * @return {@inheritDoc} * @throws IOException {@inheritDoc} - * @throws DecodeException {@inheritDoc} + * @throws DecodeException when no decoder accepts the response, or the chosen one fails * @throws FeignException {@inheritDoc} */ @Override public Object decode(Response response, Type type) throws IOException, DecodeException, FeignException { - for (Delegate delegate : delegates) { - if (delegate.predicate.canDecode(response, type)) { - return delegate.decoder.decode(response, type); + for (PredicatedDecoder decoder : decoders) { + if (decoder.canDecode(response, type)) { + return decoder.decode(response, type); } } - return defaultDecoder.decode(response, type); + throw new DecodeException( + response.status(), unableToDecode(response, type), response.request()); } - @Override - public String toString() { - return "MultiDecoder{defaultDecoder=" + defaultDecoder + ", delegates=" + delegates + '}'; + private String unableToDecode(Response response, Type type) { + StringBuilder message = + new StringBuilder("Unable to decode ") + .append(response.status()) + .append(" response (Content-Type: ") + .append(contentTypes(response)) + .append(") as ") + .append(type == null ? "the expected type" : type.getTypeName()); + if (decoders.isEmpty()) { + return message.append(". No decoders were configured.").toString(); + } + message.append(". Decoders tried, in order:"); + for (PredicatedDecoder decoder : decoders) { + message.append("\n - ").append(PairedDecoder.describe(decoder)); + } + return message + .append("\nAdd a decoder guarded by DecoderPredicate.any() last to act as a default.") + .toString(); } - private static final class Delegate { - private final DecoderPredicate predicate; - private final Decoder decoder; - - Delegate(DecoderPredicate predicate, Decoder decoder) { - this.predicate = predicate; - this.decoder = decoder; - } + private static String contentTypes(Response response) { + String contentTypes = + response.headers().entrySet().stream() + .filter(header -> Util.CONTENT_TYPE.equalsIgnoreCase(header.getKey())) + .map(Map.Entry::getValue) + .filter(Objects::nonNull) + .flatMap(Collection::stream) + .collect(Collectors.joining(", ")); + return contentTypes.isEmpty() ? "not set" : contentTypes; + } - @Override - public String toString() { - return decoder.toString(); - } + @Override + public String toString() { + return "MultiDecoder" + + decoders.stream().map(PairedDecoder::describe).collect(Collectors.toList()); } - /** Collects the delegates of a {@link MultiDecoder}. */ + /** Collects the decoders of a {@link MultiDecoder}. */ @Experimental public static final class Builder { - private final Decoder defaultDecoder; + private final List decoders = new ArrayList<>(); - private final List delegates = new ArrayList<>(); - - private Builder(Decoder defaultDecoder) { - this.defaultDecoder = Objects.requireNonNull(defaultDecoder, "defaultDecoder cannot be null"); - } + private Builder() {} /** * Adds a decoder that declares its own applicability. @@ -130,8 +142,8 @@ private Builder(Decoder defaultDecoder) { * @param decoder the decoder, consulted via {@link PredicatedDecoder#canDecode} */ public Builder add(PredicatedDecoder decoder) { - Objects.requireNonNull(decoder, "decoder cannot be null"); - return add(decoder::canDecode, decoder); + decoders.add(Objects.requireNonNull(decoder, "decoder cannot be null")); + return this; } /** @@ -142,15 +154,12 @@ public Builder add(PredicatedDecoder decoder) { * @param decoder the decoder to delegate to */ public Builder add(DecoderPredicate predicate, Decoder decoder) { - Objects.requireNonNull(predicate, "predicate cannot be null"); - Objects.requireNonNull(decoder, "decoder cannot be null"); - delegates.add(new Delegate(predicate, decoder)); - return this; + return add(PredicatedDecoder.of(predicate, decoder)); } /** Builds the multi-decoder. */ public MultiDecoder build() { - return new MultiDecoder(defaultDecoder, delegates); + return new MultiDecoder(decoders); } } } diff --git a/core/src/main/java/feign/codec/PairedDecoder.java b/core/src/main/java/feign/codec/PairedDecoder.java new file mode 100644 index 0000000000..f2922259bc --- /dev/null +++ b/core/src/main/java/feign/codec/PairedDecoder.java @@ -0,0 +1,79 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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.codec; + +import feign.FeignException; +import feign.Response; +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.Objects; + +/** A decoder that does not declare itself, guarded by a predicate supplied at the call site. */ +final class PairedDecoder implements PredicatedDecoder { + + private final DecoderPredicate predicate; + + private final Decoder decoder; + + PairedDecoder(DecoderPredicate predicate, Decoder decoder) { + this.predicate = Objects.requireNonNull(predicate, "predicate cannot be null"); + this.decoder = Objects.requireNonNull(decoder, "decoder cannot be null"); + } + + @Override + public boolean canDecode(Response response, Type type) { + return predicate.canDecode(response, type); + } + + @Override + public Object decode(Response response, Type type) + throws IOException, DecodeException, FeignException { + return decoder.decode(response, type); + } + + @Override + public String toString() { + return describe(decoder) + " when " + predicate; + } + + /** Requires both the predicate and, when the decoder declares one, its own applicability. */ + static DecoderPredicate narrow(DecoderPredicate predicate, Decoder decoder) { + Objects.requireNonNull(predicate, "predicate cannot be null"); + Objects.requireNonNull(decoder, "decoder cannot be null"); + if (!(decoder instanceof PredicatedDecoder)) { + return predicate; + } + if (decoder instanceof PairedDecoder) { + return predicate.and(((PairedDecoder) decoder).predicate); + } + PredicatedDecoder predicated = (PredicatedDecoder) decoder; + return predicate.and( + DecoderPredicate.describedAs(describe(decoder) + " accepts it", predicated::canDecode)); + } + + /** The decoder's own {@code toString} when it has one, its class name otherwise. */ + static String describe(Decoder decoder) { + Class type = decoder.getClass(); + try { + if (type.getMethod("toString").getDeclaringClass() != Object.class) { + return decoder.toString(); + } + } catch (NoSuchMethodException ignored) { + // cannot happen, every class has toString + } + return type.getSimpleName().isEmpty() ? type.getName() : type.getSimpleName(); + } +} diff --git a/core/src/main/java/feign/codec/PredicatedDecoder.java b/core/src/main/java/feign/codec/PredicatedDecoder.java index 9d6bf5067c..d8d48467a5 100644 --- a/core/src/main/java/feign/codec/PredicatedDecoder.java +++ b/core/src/main/java/feign/codec/PredicatedDecoder.java @@ -26,17 +26,25 @@ * route each response to the right one without the call site having to wrap anything: * * 

- * public class JacksonDecoder implements Decoder, PredicatedDecoder {
+ * public class JacksonDecoder implements PredicatedDecoder {
  *
  *   @Override
  *   public boolean canDecode(Response response, Type type) {
  *     return Util.isJsonContentType(response);
  *   }
+ *
+ *   @Override
+ *   public Object decode(Response response, Type type) throws IOException {
+ *     // ...
+ *   }
  * }
  *
* - *

{@link Decoder#decode(Response, Type) decode} remains the only abstract method, so this stays - * a functional interface and a bare lambda is a decoder that accepts everything. + *

{@code canDecode} is deliberately abstract: a decoder that says nothing about what it handles + * would claim every response, which is almost never what its author meant. Use {@link + * #of(DecoderPredicate, Decoder)} to give an existing decoder a predicate instead of implementing + * this on it, and {@link DecoderPredicate} — which is a {@code @FunctionalInterface} — + * to write that predicate as a lambda. * *

Decoders that wrap another decoder should forward {@code canDecode} to their delegate, so that * wrapping does not discard the delegate's applicability. @@ -45,11 +53,52 @@ * @see DecoderPredicate */ @Experimental -@FunctionalInterface public interface PredicatedDecoder extends Decoder { /** - * Whether this decoder can handle the response. Defaults to accepting everything. + * Pairs any decoder with a predicate, for decoders that do not declare themselves, including ones + * you do not control. The predicate is the whole answer: whatever the decoder may declare about + * itself is replaced, so this can widen a decoder as well as narrow it. Use {@link + * #narrowing(DecoderPredicate, Decoder)} to keep the decoder's own declaration. + * + *

A decoder paired with {@link DecoderPredicate#any()} accepts everything, which is how a + * {@link MultiDecoder} is given a default: + * + * 

+   * Feign.builder()
+   *     .decoders(
+   *         new JacksonDecoder(),
+   *         PredicatedDecoder.of(DecoderPredicate.any(), new DefaultDecoder()));
+   *
+ * + * @param predicate decides whether the decoder handles a response + * @param decoder the decoder to delegate to + */ + static PredicatedDecoder of(DecoderPredicate predicate, Decoder decoder) { + return new PairedDecoder(predicate, decoder); + } + + /** + * Narrows a decoder that already declares itself, by requiring both the given predicate and the + * decoder's own {@code canDecode} to accept the response: + * + * 
+   * PredicatedDecoder.narrowing(
+   *     DecoderPredicate.status(200), new JacksonDecoder());
+   *
+ * + *

A decoder that does not implement {@link PredicatedDecoder} declares nothing to narrow, so + * this behaves like {@link #of(DecoderPredicate, Decoder)}. + * + * @param predicate narrows what the decoder handles + * @param decoder the decoder to delegate to + */ + static PredicatedDecoder narrowing(DecoderPredicate predicate, Decoder decoder) { + return new PairedDecoder(PairedDecoder.narrow(predicate, decoder), decoder); + } + + /** + * Whether this decoder can handle the response. * *

The response body must not be read here: it is a single-pass stream for most clients, so * consuming it would leave nothing for the decoder that is eventually chosen. @@ -59,7 +108,5 @@ public interface PredicatedDecoder extends Decoder { * caller expects back * @return {@code true} if this decoder can decode the response, {@code false} otherwise */ - default boolean canDecode(Response response, Type type) { - return true; - } + boolean canDecode(Response response, Type type); } diff --git a/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java b/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java index 024a3c94ad..d286cc03a0 100644 --- a/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java +++ b/core/src/test/java/feign/codec/MultiDecoderCapabilityTest.java @@ -16,6 +16,7 @@ package feign.codec; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import feign.Capability; import feign.Feign; @@ -110,9 +111,10 @@ void capabilityWrapsTheCompositeAndRoutingStillWorks() { target( Feign.builder() .decoder( - MultiDecoder.builder(new TaggingDecoder("fallback")) + MultiDecoder.builder() .add(DecoderPredicate.jsonContentType(), new TaggingDecoder("json")) .add(DecoderPredicate.xmlContentType(), new TaggingDecoder("xml")) + .add(DecoderPredicate.any(), new TaggingDecoder("fallback")) .build()) .addCapability(capability), contentTypes); @@ -126,25 +128,41 @@ void capabilityWrapsTheCompositeAndRoutingStillWorks() { } @Test - void builderShorthandRoutesToSelfDeclaringDecoders() { + void decodersOnTheBuilderRouteInTheOrderGiven() { Map contentTypes = new HashMap<>(); contentTypes.put("json", "application/json"); contentTypes.put("csv", "text/csv"); MixedApi api = target( - Feign.builder().decoder(new TaggingDecoder("fallback"), new SelfDeclaringJsonDecoder()), + Feign.builder() + .decoders( + new SelfDeclaringJsonDecoder(), + PredicatedDecoder.of(DecoderPredicate.any(), new TaggingDecoder("fallback"))), contentTypes); assertThat(api.get("json")).isEqualTo("json"); assertThat(api.get("csv")).isEqualTo("fallback"); } + @Test + void decodersOnTheBuilderFailWhenNothingAccepts() { + Map contentTypes = new HashMap<>(); + contentTypes.put("csv", "text/csv"); + + MixedApi api = target(Feign.builder().decoders(new SelfDeclaringJsonDecoder()), contentTypes); + + assertThatThrownBy(() -> api.get("csv")) + .isInstanceOf(DecodeException.class) + .hasMessageContaining("Unable to decode 200 response (Content-Type: text/csv)") + .hasMessageContaining("SelfDeclaringJsonDecoder"); + } + /** The selected decoder still receives an unread body: predicates must not consume it. */ @Test void predicatesLeaveTheBodyForTheSelectedDecoder() throws IOException { Decoder decoder = - MultiDecoder.builder(new TaggingDecoder("fallback")) + MultiDecoder.builder() .add( DecoderPredicate.jsonContentType(), (response, type) -> Util.toString(response.body().asReader(Util.UTF_8))) @@ -168,14 +186,25 @@ public boolean canDecode(Response response, Type type) { } /** - * A wrapper that does not forward {@code canDecode} claims every response, which is why the - * metrics modules' {@code MeteredDecoder} forwards it to its delegate. + * A wrapper that answers {@code canDecode} for itself instead of forwarding claims every + * response, which is why the metrics modules' {@code MeteredDecoder} forwards it to its delegate. */ @Test void wrappingWithoutForwardingCanDecodeErasesSelfDeclaration() throws IOException { PredicatedDecoder jsonOnly = new SelfDeclaringJsonDecoder(); - PredicatedDecoder naive = jsonOnly::decode; + PredicatedDecoder naive = + new PredicatedDecoder() { + @Override + public boolean canDecode(Response response, Type type) { + return true; + } + + @Override + public Object decode(Response response, Type type) throws IOException { + return jsonOnly.decode(response, type); + } + }; PredicatedDecoder forwarding = new PredicatedDecoder() { @@ -191,15 +220,17 @@ public Object decode(Response response, Type type) throws IOException { }; assertThat( - MultiDecoder.builder(new TaggingDecoder("fallback")) + MultiDecoder.builder() .add(naive) + .add(DecoderPredicate.any(), new TaggingDecoder("fallback")) .build() .decode(response("application/xml", "payload"), String.class)) .isEqualTo("json"); assertThat( - MultiDecoder.builder(new TaggingDecoder("fallback")) + MultiDecoder.builder() .add(forwarding) + .add(DecoderPredicate.any(), new TaggingDecoder("fallback")) .build() .decode(response("application/xml", "payload"), String.class)) .isEqualTo("fallback"); diff --git a/core/src/test/java/feign/codec/MultiDecoderTest.java b/core/src/test/java/feign/codec/MultiDecoderTest.java index b1babf6c42..5ca7502b1f 100644 --- a/core/src/test/java/feign/codec/MultiDecoderTest.java +++ b/core/src/test/java/feign/codec/MultiDecoderTest.java @@ -90,7 +90,8 @@ void routesToTheDecoderThatDeclaresItCanHandleTheResponse() throws IOException { SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); RecordingDecoder fallback = new RecordingDecoder("fallback"); - Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + Decoder decoder = + MultiDecoder.builder().add(json).add(DecoderPredicate.any(), fallback).build(); assertThat(decoder.decode(responseWithContentType("application/json"), String.class)) .isEqualTo("json"); @@ -104,7 +105,10 @@ void pairsAPredicateWithADecoderThatDoesNotDeclareItself() throws IOException { RecordingDecoder fallback = new RecordingDecoder("fallback"); Decoder decoder = - MultiDecoder.builder(fallback).add(DecoderPredicate.xmlContentType(), xml).build(); + MultiDecoder.builder() + .add(DecoderPredicate.xmlContentType(), xml) + .add(DecoderPredicate.any(), fallback) + .build(); assertThat(decoder.decode(responseWithContentType("application/xml"), String.class)) .isEqualTo("xml"); @@ -119,10 +123,11 @@ void mixesSelfDeclaringDecodersAndPairs() throws IOException { RecordingDecoder fallback = new RecordingDecoder("fallback"); Decoder decoder = - MultiDecoder.builder(fallback) + MultiDecoder.builder() .add(json) .add(DecoderPredicate.xmlContentType(), xml) .add(DecoderPredicate.contentType("text/csv"), csv) + .add(DecoderPredicate.any(), fallback) .build(); assertThat(decoder.decode(responseWithContentType("text/csv;charset=utf-8"), String.class)) @@ -135,20 +140,20 @@ void mixesSelfDeclaringDecodersAndPairs() throws IOException { @Test void matchesSuffixedContentTypes() throws IOException { SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); - RecordingDecoder fallback = new RecordingDecoder("fallback"); - Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + Decoder decoder = MultiDecoder.builder().add(json).build(); assertThat(decoder.decode(responseWithContentType("application/vnd.github+json"), String.class)) .isEqualTo("json"); } @Test - void fallsBackToTheDefaultDecoderWhenNoDelegateAccepts() throws IOException { + void fallsBackToTheDecoderThatAcceptsAnything() throws IOException { SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); RecordingDecoder fallback = new RecordingDecoder("fallback"); - Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + Decoder decoder = + MultiDecoder.builder().add(json).add(DecoderPredicate.any(), fallback).build(); assertThat(decoder.decode(responseWithContentType("text/plain"), String.class)) .isEqualTo("fallback"); @@ -160,19 +165,19 @@ void fallsBackWhenTheResponseCarriesNoContentType() throws IOException { SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); RecordingDecoder fallback = new RecordingDecoder("fallback"); - Decoder decoder = MultiDecoder.builder(fallback).add(json).build(); + Decoder decoder = + MultiDecoder.builder().add(json).add(DecoderPredicate.any(), fallback).build(); assertThat(decoder.decode(responseWithContentType(null), String.class)).isEqualTo("fallback"); } @Test - void consultsDelegatesInTheOrderTheyWereAdded() throws IOException { + void consultsDecodersInTheOrderTheyWereAdded() throws IOException { RecordingDecoder first = new RecordingDecoder("first"); RecordingDecoder second = new RecordingDecoder("second"); - RecordingDecoder fallback = new RecordingDecoder("fallback"); Decoder decoder = - MultiDecoder.builder(fallback) + MultiDecoder.builder() .add(DecoderPredicate.jsonContentType(), first) .add(DecoderPredicate.jsonContentType(), second) .build(); @@ -183,15 +188,82 @@ void consultsDelegatesInTheOrderTheyWereAdded() throws IOException { } @Test - void aBareLambdaIsADecoderThatAcceptsEverything() throws IOException { - PredicatedDecoder anything = (response, type) -> "anything"; - RecordingDecoder fallback = new RecordingDecoder("fallback"); + void pairingReplacesWhatTheDecoderDeclaresAboutItself() throws IOException { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); - Decoder decoder = MultiDecoder.builder(fallback).add(anything).build(); + Decoder decoder = + MultiDecoder.builder().add(PredicatedDecoder.of(DecoderPredicate.any(), json)).build(); assertThat(decoder.decode(responseWithContentType("text/plain"), String.class)) - .isEqualTo("anything"); - assertThat(fallback.invoked).isFalse(); + .isEqualTo("json"); + } + + @Test + void narrowingKeepsWhatTheDecoderDeclaresAboutItself() { + SelfDeclaringJsonDecoder json = new SelfDeclaringJsonDecoder(); + PredicatedDecoder narrowed = PredicatedDecoder.narrowing(DecoderPredicate.status(200), json); + + assertThat(narrowed.canDecode(responseWithContentType("application/json"), String.class)) + .isTrue(); + assertThat( + narrowed.canDecode( + responseWithContentType("application/json", 204, null), String.class)) + .isFalse(); + assertThat(narrowed.canDecode(responseWithContentType("text/plain"), String.class)).isFalse(); + assertThat(narrowed) + .hasToString( + "SelfDeclaringJsonDecoder when (status is one of [200]" + + " and SelfDeclaringJsonDecoder accepts it)"); + } + + @Test + void narrowingADecoderThatDeclaresNothingIsJustThePredicate() { + RecordingDecoder plain = new RecordingDecoder("plain"); + PredicatedDecoder narrowed = + PredicatedDecoder.narrowing(DecoderPredicate.jsonContentType(), plain); + + assertThat(narrowed).hasToString("RecordingDecoder when Content-Type is JSON"); + assertThat(narrowed.canDecode(responseWithContentType("application/json"), String.class)) + .isTrue(); + } + + @Test + void throwsWhenNoDecoderAcceptsTheResponse() { + Decoder decoder = + MultiDecoder.builder() + .add(new SelfDeclaringJsonDecoder()) + .add(DecoderPredicate.xmlContentType(), new RecordingDecoder("xml")) + .build(); + + assertThatThrownBy(() -> decoder.decode(responseWithContentType("text/plain"), String.class)) + .isInstanceOf(DecodeException.class) + .hasMessage( + "Unable to decode 200 response (Content-Type: text/plain) as java.lang.String." + + " Decoders tried, in order:" + + "\n - SelfDeclaringJsonDecoder" + + "\n - RecordingDecoder when Content-Type is XML" + + "\nAdd a decoder guarded by DecoderPredicate.any() last to act as a default."); + } + + @Test + void theFailureReportsAMissingContentType() { + Decoder decoder = MultiDecoder.builder().add(new SelfDeclaringJsonDecoder()).build(); + + assertThatThrownBy(() -> decoder.decode(responseWithContentType(null), String.class)) + .isInstanceOf(DecodeException.class) + .hasMessageContaining("(Content-Type: not set)"); + } + + @Test + void throwsWhenNoDecodersAreConfigured() { + Decoder decoder = MultiDecoder.builder().build(); + + assertThatThrownBy( + () -> decoder.decode(responseWithContentType("application/json"), String.class)) + .isInstanceOf(DecodeException.class) + .hasMessage( + "Unable to decode 200 response (Content-Type: application/json) as java.lang.String." + + " No decoders were configured."); } @Test @@ -202,9 +274,7 @@ void propagatesIoExceptionsFromTheSelectedDecoder() { }; Decoder decoder = - MultiDecoder.builder(new RecordingDecoder("fallback")) - .add(DecoderPredicate.jsonContentType(), failing) - .build(); + MultiDecoder.builder().add(DecoderPredicate.jsonContentType(), failing).build(); assertThatThrownBy( () -> decoder.decode(responseWithContentType("application/json"), String.class)) @@ -213,15 +283,8 @@ void propagatesIoExceptionsFromTheSelectedDecoder() { } @Test - void rejectsANullDefaultDecoder() { - assertThatThrownBy(() -> MultiDecoder.builder(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("defaultDecoder cannot be null"); - } - - @Test - void rejectsNullDelegates() { - MultiDecoder.Builder builder = MultiDecoder.builder(new RecordingDecoder("fallback")); + void rejectsNullDecoders() { + MultiDecoder.Builder builder = MultiDecoder.builder(); assertThatThrownBy(() -> builder.add((PredicatedDecoder) null)) .isInstanceOf(NullPointerException.class) @@ -235,26 +298,15 @@ void rejectsNullDelegates() { } @Test - void describesItsDelegates() { + void describesItsDecoders() { Decoder decoder = - MultiDecoder.builder( - new RecordingDecoder("fallback") { - @Override - public String toString() { - return "fallback"; - } - }) - .add( - DecoderPredicate.jsonContentType(), - new RecordingDecoder("json") { - @Override - public String toString() { - return "json"; - } - }) + MultiDecoder.builder() + .add(new SelfDeclaringJsonDecoder()) + .add(DecoderPredicate.jsonContentType(), new RecordingDecoder("json")) .build(); assertThat(decoder.toString()) - .isEqualTo("MultiDecoder{defaultDecoder=fallback, delegates=[json]}"); + .isEqualTo( + "MultiDecoder[SelfDeclaringJsonDecoder, RecordingDecoder when Content-Type is JSON]"); } }