From a291578ff630d46641241b521fb7e30d8f55873b Mon Sep 17 00:00:00 2001 From: Yevhen Vasyliev Date: Wed, 19 Aug 2026 09:27:19 -0300 Subject: [PATCH 1/7] Add Util helpers for detecting JSON and XML content types Co-authored-by: trumpetinc <6618744+trumpetinc@users.noreply.github.com> Signed-off-by: Marvin Froeder --- core/src/main/java/feign/Util.java | 49 ++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index 91cb7e5a1a..b91144242f 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. */ @@ -62,6 +63,9 @@ public class Util { /** The HTTP Content-Encoding header field name. */ public static final String CONTENT_ENCODING = "Content-Encoding"; + /** The HTTP Content-Type header field name. */ + public static final String CONTENT_TYPE = "Content-Type"; + /** The HTTP Accept-Encoding header field name. */ public static final String ACCEPT_ENCODING = "Accept-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,40 @@ public static String getThreadIdentifier() { + "_" + currentThread.getId(); } + + /** + * Checks whether the {@code Content-Type} header of the given template 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 template the request template to check + * @return {@code true} if the content type is JSON, {@code false} otherwise + */ + public static boolean isJsonContentType(RequestTemplate template) { + return hasContentTypeMatching(template, JSON_CONTENT_TYPE); + } + + /** + * Checks whether the {@code Content-Type} header of the given template 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 template the request template to check + * @return {@code true} if the content type is XML, {@code false} otherwise + */ + public static boolean isXmlContentType(RequestTemplate template) { + return hasContentTypeMatching(template, XML_CONTENT_TYPE); + } + + private static boolean hasContentTypeMatching(RequestTemplate template, Pattern pattern) { + return template.headers().entrySet().stream() + .filter(header -> CONTENT_TYPE.equalsIgnoreCase(header.getKey())) + .map(Map.Entry::getValue) + .filter(Objects::nonNull) + .flatMap(Collection::stream) + .anyMatch( + contentType -> contentType != null && pattern.matcher(contentType.trim()).matches()); + } } From a8f11c23dff36e6e2805912b7a47c16ec105af96 Mon Sep 17 00:00:00 2001 From: kevin Date: Wed, 19 Aug 2026 09:27:25 -0300 Subject: [PATCH 2/7] Add PredicatedEncoder and EncoderPredicate for conditional encoding Co-authored-by: Yevhen Vasyliev Signed-off-by: Marvin Froeder --- .../java/feign/codec/EncoderPredicate.java | 44 ++++++ .../java/feign/codec/PredicatedEncoder.java | 93 ++++++++++++ .../feign/codec/PredicatedEncoderTest.java | 138 ++++++++++++++++++ 3 files changed, 275 insertions(+) create mode 100644 core/src/main/java/feign/codec/EncoderPredicate.java create mode 100644 core/src/main/java/feign/codec/PredicatedEncoder.java create mode 100644 core/src/test/java/feign/codec/PredicatedEncoderTest.java diff --git a/core/src/main/java/feign/codec/EncoderPredicate.java b/core/src/main/java/feign/codec/EncoderPredicate.java new file mode 100644 index 0000000000..a00ceb2635 --- /dev/null +++ b/core/src/main/java/feign/codec/EncoderPredicate.java @@ -0,0 +1,44 @@ +/* + * 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.RequestTemplate; +import java.lang.reflect.Type; + +/** + * A predicate that decides whether a given request can be handled by an {@link Encoder}. + * + *

Predicates receive the same three arguments as {@link Encoder#encode(Object, Type, + * RequestTemplate)}, so they can discriminate on the body, on its declared type, or on anything + * already present in the template such as the {@code Content-Type} header. + * + * @see PredicatedEncoder + * @see MultiEncoder + */ +@FunctionalInterface +public interface EncoderPredicate { + + /** + * Tests whether the given request can be encoded. + * + * @param object what would be encoded as the request body + * @param bodyType the type the object would be encoded as. {@link Encoder#MAP_STRING_WILDCARD} + * indicates form encoding. + * @param template the request template that would be populated + * @return {@code true} if the request can be encoded, {@code false} otherwise + */ + boolean test(Object object, Type bodyType, RequestTemplate template); +} diff --git a/core/src/main/java/feign/codec/PredicatedEncoder.java b/core/src/main/java/feign/codec/PredicatedEncoder.java new file mode 100644 index 0000000000..f70b961332 --- /dev/null +++ b/core/src/main/java/feign/codec/PredicatedEncoder.java @@ -0,0 +1,93 @@ +/* + * 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.RequestTemplate; +import feign.Util; +import java.lang.reflect.Type; +import java.util.Objects; + +/** + * Pairs an {@link EncoderPredicate} with the {@link Encoder} it guards, so that a {@link + * MultiEncoder} can pick the right encoder per request. + * + *

Encoding through a {@code PredicatedEncoder} directly is allowed but strict: a request its + * predicate rejects raises {@link EncodeException} rather than silently doing nothing. Inside a + * {@link MultiEncoder} a rejected request simply moves on to the next candidate. + * + * 

+ * Feign.builder()
+ *     .encoder(
+ *         new DefaultEncoder(),
+ *         PredicatedEncoder.forJsonContentType(new JacksonEncoder()),
+ *         PredicatedEncoder.forXmlContentType(new JAXBEncoder()))
+ *
+ */ +public class PredicatedEncoder implements Encoder { + + private final EncoderPredicate predicate; + + private final Encoder delegate; + + public PredicatedEncoder(EncoderPredicate predicate, Encoder delegate) { + this.predicate = Objects.requireNonNull(predicate, "predicate cannot be null"); + this.delegate = Objects.requireNonNull(delegate, "delegate cannot be null"); + } + + /** Restricts the delegate to requests whose {@code Content-Type} header denotes JSON. */ + public static PredicatedEncoder forJsonContentType(Encoder delegate) { + return new PredicatedEncoder( + (object, bodyType, template) -> Util.isJsonContentType(template), delegate); + } + + /** Restricts the delegate to requests whose {@code Content-Type} header denotes XML. */ + public static PredicatedEncoder forXmlContentType(Encoder delegate) { + return new PredicatedEncoder( + (object, bodyType, template) -> Util.isXmlContentType(template), delegate); + } + + /** Restricts the delegate to requests carrying no body. */ + public static PredicatedEncoder forEmptyBody(Encoder delegate) { + return new PredicatedEncoder((object, bodyType, template) -> object == null, delegate); + } + + /** + * Whether the guarded encoder accepts this request. + * + * @param object what to encode as the request body + * @param bodyType the type the object should be encoded as + * @param template the request template to populate + * @return {@code true} if the delegate should handle this request + */ + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return predicate.test(object, bodyType, template); + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) + throws EncodeException { + if (!canEncode(object, bodyType, template)) { + throw new EncodeException( + "Predicate of " + this + " rejected the request, so " + delegate + " was not invoked"); + } + delegate.encode(object, bodyType, template); + } + + @Override + public String toString() { + return "PredicatedEncoder{predicate=" + predicate + ", delegate=" + delegate + '}'; + } +} diff --git a/core/src/test/java/feign/codec/PredicatedEncoderTest.java b/core/src/test/java/feign/codec/PredicatedEncoderTest.java new file mode 100644 index 0000000000..69a90b08d1 --- /dev/null +++ b/core/src/test/java/feign/codec/PredicatedEncoderTest.java @@ -0,0 +1,138 @@ +/* + * 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.RequestTemplate; +import java.lang.reflect.Type; +import org.junit.jupiter.api.Test; + +class PredicatedEncoderTest { + + private static class RecordingEncoder implements Encoder { + boolean invoked; + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + invoked = true; + template.body(Request.Body.create("encoded")); + } + } + + private static RequestTemplate templateWithContentType(String contentType) { + RequestTemplate template = new RequestTemplate(); + if (contentType != null) { + template.header("Content-Type", contentType); + } + return template; + } + + @Test + void delegatesWhenPredicateAccepts() { + RecordingEncoder delegate = new RecordingEncoder(); + PredicatedEncoder encoder = new PredicatedEncoder((o, t, tpl) -> true, delegate); + + RequestTemplate template = templateWithContentType(null); + encoder.encode("body", String.class, template); + + assertThat(delegate.invoked).isTrue(); + assertThat(template.requestBody().asString()).isEqualTo("encoded"); + } + + @Test + void throwsAndSkipsDelegateWhenPredicateRejects() { + RecordingEncoder delegate = new RecordingEncoder(); + PredicatedEncoder encoder = new PredicatedEncoder((o, t, tpl) -> false, delegate); + + assertThatThrownBy(() -> encoder.encode("body", String.class, templateWithContentType(null))) + .isInstanceOf(EncodeException.class); + + assertThat(delegate.invoked).isFalse(); + } + + @Test + void canEncodeReflectsThePredicate() { + PredicatedEncoder encoder = + new PredicatedEncoder((o, t, tpl) -> "yes".equals(o), new RecordingEncoder()); + + assertThat(encoder.canEncode("yes", String.class, templateWithContentType(null))).isTrue(); + assertThat(encoder.canEncode("no", String.class, templateWithContentType(null))).isFalse(); + } + + @Test + void forJsonContentTypeMatchesJsonOnly() { + PredicatedEncoder encoder = PredicatedEncoder.forJsonContentType(new RecordingEncoder()); + + assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/json"))) + .isTrue(); + assertThat( + encoder.canEncode( + null, String.class, templateWithContentType("application/json;charset=utf-8"))) + .isTrue(); + assertThat( + encoder.canEncode( + null, String.class, templateWithContentType("application/vnd.github+json"))) + .isTrue(); + assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/xml"))) + .isFalse(); + assertThat(encoder.canEncode(null, String.class, templateWithContentType(null))).isFalse(); + } + + @Test + void forXmlContentTypeMatchesXmlOnly() { + PredicatedEncoder encoder = PredicatedEncoder.forXmlContentType(new RecordingEncoder()); + + assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/xml"))) + .isTrue(); + assertThat(encoder.canEncode(null, String.class, templateWithContentType("text/xml"))).isTrue(); + assertThat( + encoder.canEncode(null, String.class, templateWithContentType("application/soap+xml"))) + .isTrue(); + assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/json"))) + .isFalse(); + } + + @Test + void contentTypeHeaderNameIsMatchedCaseInsensitively() { + PredicatedEncoder encoder = PredicatedEncoder.forJsonContentType(new RecordingEncoder()); + + RequestTemplate template = new RequestTemplate(); + template.header("content-type", "application/json"); + + assertThat(encoder.canEncode(null, String.class, template)).isTrue(); + } + + @Test + void forEmptyBodyMatchesNullBodyOnly() { + PredicatedEncoder encoder = PredicatedEncoder.forEmptyBody(new RecordingEncoder()); + + assertThat(encoder.canEncode(null, String.class, templateWithContentType(null))).isTrue(); + assertThat(encoder.canEncode("body", String.class, templateWithContentType(null))).isFalse(); + } + + @Test + void rejectsNullConstructorArguments() { + assertThatThrownBy(() -> new PredicatedEncoder(null, new RecordingEncoder())) + .isInstanceOf(NullPointerException.class) + .hasMessage("predicate cannot be null"); + assertThatThrownBy(() -> new PredicatedEncoder((o, t, tpl) -> true, null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("delegate cannot be null"); + } +} From df9ae654cae58eb0b5b4b7275ba11503708f9181 Mon Sep 17 00:00:00 2001 From: Yevhen Vasyliev Date: Wed, 19 Aug 2026 09:27:30 -0300 Subject: [PATCH 3/7] Add MultiEncoder to select an encoder per request Co-authored-by: trumpetinc <6618744+trumpetinc@users.noreply.github.com> Signed-off-by: Marvin Froeder --- .../main/java/feign/codec/MultiEncoder.java | 105 +++++++++++ .../java/feign/codec/MultiEncoderTest.java | 178 ++++++++++++++++++ 2 files changed, 283 insertions(+) create mode 100644 core/src/main/java/feign/codec/MultiEncoder.java create mode 100644 core/src/test/java/feign/codec/MultiEncoderTest.java diff --git a/core/src/main/java/feign/codec/MultiEncoder.java b/core/src/main/java/feign/codec/MultiEncoder.java new file mode 100644 index 0000000000..aa779930af --- /dev/null +++ b/core/src/main/java/feign/codec/MultiEncoder.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.RequestTemplate; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * An encoder that delegates to a list of {@link PredicatedEncoder}s, using the first one whose + * predicate accepts the request, and falling back to a default encoder when none do. + * + *

The default encoder is declared first so the predicated ones can be supplied as varargs, but + * it is consulted last — it is the fallback, not the first choice. + * + * 

+ * Encoder encoder =
+ *     MultiEncoder.of(
+ *         new DefaultEncoder(),
+ *         PredicatedEncoder.forJsonContentType(new JacksonEncoder()),
+ *         PredicatedEncoder.forXmlContentType(new JAXBEncoder()));
+ *
+ */ +public class MultiEncoder implements Encoder { + + private final Encoder defaultEncoder; + + private final List delegates; + + /** + * Creates an encoder that tries each predicated encoder in order and falls back to {@code + * defaultEncoder}. + * + * @param defaultEncoder the encoder used when no predicate accepts the request + * @param encoders the predicated encoders, consulted in the order given + * @return the multi-encoder + */ + public static Encoder of(Encoder defaultEncoder, PredicatedEncoder... encoders) { + return of(defaultEncoder, Arrays.asList(encoders)); + } + + /** + * Creates an encoder that tries each predicated encoder in order and falls back to {@code + * defaultEncoder}. + * + * @param defaultEncoder the encoder used when no predicate accepts the request + * @param encoders the predicated encoders, consulted in the order given + * @return the multi-encoder + */ + public static Encoder of(Encoder defaultEncoder, List encoders) { + return new MultiEncoder(defaultEncoder, encoders); + } + + private MultiEncoder(Encoder defaultEncoder, List delegates) { + this.defaultEncoder = Objects.requireNonNull(defaultEncoder, "defaultEncoder cannot be null"); + Objects.requireNonNull(delegates, "delegates cannot be null"); + for (PredicatedEncoder delegate : delegates) { + Objects.requireNonNull(delegate, "delegates cannot contain null"); + } + this.delegates = Collections.unmodifiableList(new ArrayList<>(delegates)); + } + + /** + * Encodes using the first delegate whose predicate accepts the request, or the default encoder if + * none do. + * + * @param object {@inheritDoc} + * @param bodyType {@inheritDoc} + * @param template {@inheritDoc} + * @throws EncodeException {@inheritDoc} + */ + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) + throws EncodeException { + for (PredicatedEncoder delegate : delegates) { + if (delegate.canEncode(object, bodyType, template)) { + delegate.encode(object, bodyType, template); + return; + } + } + defaultEncoder.encode(object, bodyType, template); + } + + @Override + public String toString() { + return "MultiEncoder{defaultEncoder=" + defaultEncoder + ", delegates=" + delegates + '}'; + } +} diff --git a/core/src/test/java/feign/codec/MultiEncoderTest.java b/core/src/test/java/feign/codec/MultiEncoderTest.java new file mode 100644 index 0000000000..a02f53eed0 --- /dev/null +++ b/core/src/test/java/feign/codec/MultiEncoderTest.java @@ -0,0 +1,178 @@ +/* + * 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.RequestTemplate; +import java.lang.reflect.Type; +import java.util.Arrays; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +class MultiEncoderTest { + + private static class RecordingEncoder implements Encoder { + private final String body; + boolean invoked; + + RecordingEncoder(String body) { + this.body = body; + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + invoked = true; + template.body(Request.Body.create(body)); + } + } + + private static RequestTemplate templateWithContentType(String contentType) { + RequestTemplate template = new RequestTemplate(); + if (contentType != null) { + template.header("Content-Type", contentType); + } + return template; + } + + @Test + void usesFirstDelegateWhosePredicateAccepts() { + RecordingEncoder json = new RecordingEncoder("json"); + RecordingEncoder xml = new RecordingEncoder("xml"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = + MultiEncoder.of( + fallback, + PredicatedEncoder.forJsonContentType(json), + PredicatedEncoder.forXmlContentType(xml)); + + RequestTemplate template = templateWithContentType("application/json"); + encoder.encode("body", String.class, template); + + assertThat(json.invoked).isTrue(); + assertThat(xml.invoked).isFalse(); + assertThat(fallback.invoked).isFalse(); + assertThat(template.requestBody().asString()).isEqualTo("json"); + } + + @Test + void matchesSuffixedContentTypes() { + RecordingEncoder json = new RecordingEncoder("json"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = MultiEncoder.of(fallback, PredicatedEncoder.forJsonContentType(json)); + + encoder.encode("body", String.class, templateWithContentType("application/vnd.github+json")); + + assertThat(json.invoked).isTrue(); + assertThat(fallback.invoked).isFalse(); + } + + @Test + void fallsBackToDefaultEncoderWhenNoPredicateAccepts() { + RecordingEncoder json = new RecordingEncoder("json"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = MultiEncoder.of(fallback, PredicatedEncoder.forJsonContentType(json)); + + RequestTemplate template = templateWithContentType("text/plain"); + encoder.encode("body", String.class, template); + + assertThat(json.invoked).isFalse(); + assertThat(fallback.invoked).isTrue(); + assertThat(template.requestBody().asString()).isEqualTo("fallback"); + } + + @Test + void fallsBackToDefaultEncoderWhenNoContentTypeIsSet() { + RecordingEncoder json = new RecordingEncoder("json"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = MultiEncoder.of(fallback, PredicatedEncoder.forJsonContentType(json)); + + encoder.encode("body", String.class, templateWithContentType(null)); + + assertThat(json.invoked).isFalse(); + assertThat(fallback.invoked).isTrue(); + } + + @Test + void withoutDelegatesEverythingGoesToTheDefaultEncoder() { + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = MultiEncoder.of(fallback); + + encoder.encode("body", String.class, templateWithContentType("application/json")); + + assertThat(fallback.invoked).isTrue(); + } + + @Test + void propagatesEncodeExceptionFromDelegate() { + Encoder failing = + (object, bodyType, template) -> { + throw new EncodeException("boom"); + }; + + Encoder encoder = + MultiEncoder.of(new DefaultEncoder(), PredicatedEncoder.forJsonContentType(failing)); + + assertThatThrownBy( + () -> encoder.encode("body", String.class, templateWithContentType("application/json"))) + .isInstanceOf(EncodeException.class) + .hasMessage("boom"); + } + + @Test + void rejectsNullDefaultEncoder() { + assertThatThrownBy(() -> MultiEncoder.of(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("defaultEncoder cannot be null"); + } + + @Test + void rejectsNullDelegate() { + assertThatThrownBy(() -> MultiEncoder.of(new DefaultEncoder(), Collections.singletonList(null))) + .isInstanceOf(NullPointerException.class) + .hasMessage("delegates cannot contain null"); + } + + @Test + void toStringDescribesDelegates() { + Encoder encoder = + MultiEncoder.of( + new DefaultEncoder(), PredicatedEncoder.forJsonContentType(new DefaultEncoder())); + + assertThat(encoder.toString()).startsWith("MultiEncoder{defaultEncoder="); + assertThat(encoder.toString()).contains("PredicatedEncoder{"); + } + + @Test + void listFactoryIsEquivalentToVarargs() { + RecordingEncoder json = new RecordingEncoder("json"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = + MultiEncoder.of(fallback, Arrays.asList(PredicatedEncoder.forJsonContentType(json))); + + encoder.encode("body", String.class, templateWithContentType("application/json")); + + assertThat(json.invoked).isTrue(); + } +} From 0ece2841f516c421784b769abdca2187de886af8 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 09:27:35 -0300 Subject: [PATCH 4/7] Expose and document multi-encoder configuration Co-authored-by: Yevhen Vasyliev Co-authored-by: trumpetinc <6618744+trumpetinc@users.noreply.github.com> Signed-off-by: Marvin Froeder --- CHANGELOG.md | 6 + README.md | 48 ++++++++ core/src/main/java/feign/BaseBuilder.java | 23 ++++ src/docs/overview-mindmap.iuml | 127 +++++++++++----------- 4 files changed, 141 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 513e923ab2..b3ff46675b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ ### Version 13.14 +* Add `MultiEncoder`, `PredicatedEncoder` and `EncoderPredicate`, letting a single client pick an + encoder per request. `Feign.builder().encoder(defaultEncoder, predicatedEncoders...)` builds one; + predicates are consulted in order and the default encoder is the fallback. `Util.isJsonContentType` + and `Util.isXmlContentType` back the `PredicatedEncoder.forJsonContentType`/`forXmlContentType` + factories. The `Encoder` interface is unchanged, so existing encoders keep working (#3485). + * `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/README.md b/README.md index cbaae10766..d03bf003ca 100644 --- a/README.md +++ b/README.md @@ -709,6 +709,54 @@ public class Example { } ``` +#### Multiple encoders + +A single client sometimes has to speak more than one format — JSON for most endpoints, XML for +a legacy one, plain bytes for an upload. `MultiEncoder` picks the encoder per request by asking each +candidate's predicate, falling back to a default encoder when none match. + +```java +interface MixedClient { + @RequestLine("POST /orders") + @Headers("Content-Type: application/json") + void createOrder(Order order); + + @RequestLine("POST /legacy/orders") + @Headers("Content-Type: application/xml") + void createLegacyOrder(Order order); +} + +public class Example { + public static void main(String[] args) { + MixedClient client = Feign.builder() + .encoder( + new DefaultEncoder(), + PredicatedEncoder.forJsonContentType(new GsonEncoder()), + PredicatedEncoder.forXmlContentType(new JAXBEncoder())) + .target(MixedClient.class, "https://foo.com"); + } +} +``` + +The first argument is the default encoder, used when no predicate accepts the request. The remaining +arguments are consulted in the order given, so the most specific encoder should come first. + +`PredicatedEncoder` ships with factories for the common cases — `forJsonContentType`, +`forXmlContentType` and `forEmptyBody`. `EncoderPredicate` is a functional interface, so any other +condition is a lambda over the same three arguments `Encoder#encode` receives: + +```java +Encoder encoder = + MultiEncoder.of( + new DefaultEncoder(), + new PredicatedEncoder( + (object, bodyType, template) -> bodyType == byte[].class, new BinaryEncoder()), + PredicatedEncoder.forJsonContentType(new GsonEncoder())); +``` + +A `PredicatedEncoder` used on its own, outside a `MultiEncoder`, is strict: a request its predicate +rejects raises `EncodeException` rather than silently encoding nothing. + ### @Body templates The `@Body` annotation indicates a template to expand using parameters annotated with `@Param`. 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..8d07b31dd9 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.MultiEncoder; +import feign.codec.PredicatedEncoder; import feign.interceptor.MethodInterceptor; import feign.interceptor.MethodInterceptors; import java.lang.reflect.Field; @@ -94,6 +96,27 @@ public B encoder(Encoder encoder) { return thisB(); } + /** + * Configures a {@link MultiEncoder} that picks an encoder per request. + * + *

Each {@link PredicatedEncoder} is consulted in the order given; {@code defaultEncoder} is + * the fallback used when no predicate accepts the request. + * + * 

+   * Feign.builder()
+   *     .encoder(
+   *         new DefaultEncoder(),
+   *         PredicatedEncoder.forJsonContentType(new JacksonEncoder()),
+   *         PredicatedEncoder.forXmlContentType(new JAXBEncoder()))
+   *
+ * + * @param defaultEncoder the encoder used when no predicate accepts the request + * @param encoders the predicated encoders, consulted in the order given + */ + public B encoder(Encoder defaultEncoder, PredicatedEncoder... encoders) { + return encoder(MultiEncoder.of(defaultEncoder, encoders)); + } + public B decoder(Decoder decoder) { this.decoder = decoder; return thisB(); diff --git a/src/docs/overview-mindmap.iuml b/src/docs/overview-mindmap.iuml index afd6aefbf3..805b77db3a 100644 --- a/src/docs/overview-mindmap.iuml +++ b/src/docs/overview-mindmap.iuml @@ -1,63 +1,64 @@ -@startmindmap -* Feign -** clients -*** java.net.URL -*** Apache HTTP -*** Apache HC5 -*** Google HTTP -*** Java 11 Http2 -*** OK Http -*** Ribbon -** async clients -*** java.net.URL -*** Apache HC5 -*** OkHttp -*** Vertx -*** Reactive Wrappers -** contracts -*** Feign -*** JAX-RS -*** JAX-RS 2 -*** JAX-RS 3 / Jakarta -*** JAX-RS 4 -*** Spring -*** SOAP -*** SOAP Jakarta -*** Spring boot (3rd party) -** language -*** Kotlin -*** GraphQL - -left side - -** encoders/decoders -*** GSON -*** JAXB -*** JAXB Jakarta -*** Jackson -*** Jackson 3 -*** Jackson JAXB -*** Jackson Jr -*** Sax -*** JSON-java -*** Moshi -*** Fastjson2 -*** Form -*** Form Spring -** metrics -*** Dropwizard Metrics 4 -*** Dropwizard Metrics 5 -*** Micrometer -** interceptors -*** RequestInterceptor -*** ResponseInterceptor -*** MethodInterceptor -**** Bean Validation (JSR-303) -**** Bean Validation (Jakarta) -**** HTTP Cache (ETag / Last-Modified) -** extras -*** Hystrix -*** SLF4J -*** Mock -*** Annotation Error Decoder -@endmindmap +@startmindmap +* Feign +** clients +*** java.net.URL +*** Apache HTTP +*** Apache HC5 +*** Google HTTP +*** Java 11 Http2 +*** OK Http +*** Ribbon +** async clients +*** java.net.URL +*** Apache HC5 +*** OkHttp +*** Vertx +*** Reactive Wrappers +** contracts +*** Feign +*** JAX-RS +*** JAX-RS 2 +*** JAX-RS 3 / Jakarta +*** JAX-RS 4 +*** Spring +*** SOAP +*** SOAP Jakarta +*** Spring boot (3rd party) +** language +*** Kotlin +*** GraphQL + +left side + +** encoders/decoders +*** Multi encoder (predicate based) +*** GSON +*** JAXB +*** JAXB Jakarta +*** Jackson +*** Jackson 3 +*** Jackson JAXB +*** Jackson Jr +*** Sax +*** JSON-java +*** Moshi +*** Fastjson2 +*** Form +*** Form Spring +** metrics +*** Dropwizard Metrics 4 +*** Dropwizard Metrics 5 +*** Micrometer +** interceptors +*** RequestInterceptor +*** ResponseInterceptor +*** MethodInterceptor +**** Bean Validation (JSR-303) +**** Bean Validation (Jakarta) +**** HTTP Cache (ETag / Last-Modified) +** extras +*** Hystrix +*** SLF4J +*** Mock +*** Annotation Error Decoder +@endmindmap From 78eab43b6282330eb7423d892cc0daf2a86806bb Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 09:57:11 -0300 Subject: [PATCH 5/7] Mark the multi-encoder API as experimental Signed-off-by: Marvin Froeder --- CHANGELOG.md | 2 +- README.md | 2 ++ core/src/main/java/feign/BaseBuilder.java | 1 + core/src/main/java/feign/Util.java | 2 ++ core/src/main/java/feign/codec/EncoderPredicate.java | 2 ++ core/src/main/java/feign/codec/MultiEncoder.java | 2 ++ core/src/main/java/feign/codec/PredicatedEncoder.java | 2 ++ src/docs/overview-mindmap.iuml | 2 +- 8 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3ff46675b..14e5619a39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ### Version 13.14 -* Add `MultiEncoder`, `PredicatedEncoder` and `EncoderPredicate`, letting a single client pick an +* Add `@Experimental` `MultiEncoder`, `PredicatedEncoder` and `EncoderPredicate`, letting a single client pick an encoder per request. `Feign.builder().encoder(defaultEncoder, predicatedEncoders...)` builds one; predicates are consulted in order and the default encoder is the fallback. `Util.isJsonContentType` and `Util.isXmlContentType` back the `PredicatedEncoder.forJsonContentType`/`forXmlContentType` diff --git a/README.md b/README.md index d03bf003ca..a66ad92e3f 100644 --- a/README.md +++ b/README.md @@ -711,6 +711,8 @@ public class Example { #### Multiple encoders +> This API is `@Experimental` and may change incompatibly, or be removed, in a future release. + A single client sometimes has to speak more than one format — JSON for most endpoints, XML for a legacy one, plain bytes for an upload. `MultiEncoder` picks the encoder per request by asking each candidate's predicate, falling back to a default encoder when none match. diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index 8d07b31dd9..abc246db3d 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -113,6 +113,7 @@ public B encoder(Encoder encoder) { * @param defaultEncoder the encoder used when no predicate accepts the request * @param encoders the predicated encoders, consulted in the order given */ + @Experimental public B encoder(Encoder defaultEncoder, PredicatedEncoder... encoders) { return encoder(MultiEncoder.of(defaultEncoder, encoders)); } diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index b91144242f..2b4b8d5bd5 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -394,6 +394,7 @@ public static String getThreadIdentifier() { * @param template the request template to check * @return {@code true} if the content type is JSON, {@code false} otherwise */ + @Experimental public static boolean isJsonContentType(RequestTemplate template) { return hasContentTypeMatching(template, JSON_CONTENT_TYPE); } @@ -407,6 +408,7 @@ public static boolean isJsonContentType(RequestTemplate template) { * @param template the request template to check * @return {@code true} if the content type is XML, {@code false} otherwise */ + @Experimental public static boolean isXmlContentType(RequestTemplate template) { return hasContentTypeMatching(template, XML_CONTENT_TYPE); } diff --git a/core/src/main/java/feign/codec/EncoderPredicate.java b/core/src/main/java/feign/codec/EncoderPredicate.java index a00ceb2635..16a9b25503 100644 --- a/core/src/main/java/feign/codec/EncoderPredicate.java +++ b/core/src/main/java/feign/codec/EncoderPredicate.java @@ -15,6 +15,7 @@ */ package feign.codec; +import feign.Experimental; import feign.RequestTemplate; import java.lang.reflect.Type; @@ -29,6 +30,7 @@ * @see MultiEncoder */ @FunctionalInterface +@Experimental public interface EncoderPredicate { /** diff --git a/core/src/main/java/feign/codec/MultiEncoder.java b/core/src/main/java/feign/codec/MultiEncoder.java index aa779930af..494fad00d3 100644 --- a/core/src/main/java/feign/codec/MultiEncoder.java +++ b/core/src/main/java/feign/codec/MultiEncoder.java @@ -15,6 +15,7 @@ */ package feign.codec; +import feign.Experimental; import feign.RequestTemplate; import java.lang.reflect.Type; import java.util.ArrayList; @@ -38,6 +39,7 @@ * PredicatedEncoder.forXmlContentType(new JAXBEncoder())); * */ +@Experimental public class MultiEncoder implements Encoder { private final Encoder defaultEncoder; diff --git a/core/src/main/java/feign/codec/PredicatedEncoder.java b/core/src/main/java/feign/codec/PredicatedEncoder.java index f70b961332..b8c130f725 100644 --- a/core/src/main/java/feign/codec/PredicatedEncoder.java +++ b/core/src/main/java/feign/codec/PredicatedEncoder.java @@ -15,6 +15,7 @@ */ package feign.codec; +import feign.Experimental; import feign.RequestTemplate; import feign.Util; import java.lang.reflect.Type; @@ -36,6 +37,7 @@ * PredicatedEncoder.forXmlContentType(new JAXBEncoder())) * */ +@Experimental public class PredicatedEncoder implements Encoder { private final EncoderPredicate predicate; diff --git a/src/docs/overview-mindmap.iuml b/src/docs/overview-mindmap.iuml index 805b77db3a..50dd352f26 100644 --- a/src/docs/overview-mindmap.iuml +++ b/src/docs/overview-mindmap.iuml @@ -31,7 +31,7 @@ left side ** encoders/decoders -*** Multi encoder (predicate based) +*** Multi encoder (predicate based, experimental) *** GSON *** JAXB *** JAXB Jakarta From fecafdb998ba876a0b14b274cd93f1922410d30e Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 19 Aug 2026 10:25:56 -0300 Subject: [PATCH 6/7] Rework multi-encoder around encoders that declare their own canEncode Signed-off-by: Marvin Froeder --- CHANGELOG.md | 17 +- README.md | 60 +++++-- core/src/main/java/feign/BaseBuilder.java | 20 ++- core/src/main/java/feign/Util.java | 31 +++- .../java/feign/codec/EncoderPredicate.java | 61 ++++++- .../main/java/feign/codec/MultiEncoder.java | 130 +++++++++----- .../java/feign/codec/PredicatedEncoder.java | 86 +++------ .../feign/codec/EncoderPredicateTest.java | 115 ++++++++++++ .../codec/MultiEncoderCapabilityTest.java | 170 ++++++++++++++++++ .../java/feign/codec/MultiEncoderTest.java | 161 ++++++++++++----- .../feign/codec/PredicatedEncoderTest.java | 138 -------------- .../java/feign/metrics4/MeteredEncoder.java | 9 +- .../java/feign/metrics5/MeteredEncoder.java | 9 +- .../feign/fastjson2/Fastjson2Encoder.java | 8 +- .../src/main/java/feign/gson/GsonEncoder.java | 9 +- .../jackson/jaxb/JacksonJaxbJsonEncoder.java | 9 +- .../feign/jackson/jr/JacksonJrEncoder.java | 9 +- .../java/feign/jackson/JacksonEncoder.java | 8 +- .../java/feign/jackson3/Jackson3Encoder.java | 8 +- .../src/main/java/feign/jaxb/JAXBEncoder.java | 9 +- .../src/main/java/feign/jaxb/JAXBEncoder.java | 9 +- .../src/main/java/feign/json/JsonEncoder.java | 9 +- .../java/feign/micrometer/MeteredEncoder.java | 9 +- .../main/java/feign/moshi/MoshiEncoder.java | 9 +- .../src/main/java/feign/soap/SOAPEncoder.java | 9 +- .../src/main/java/feign/soap/SOAPEncoder.java | 9 +- src/docs/overview-mindmap.iuml | 128 ++++++------- 27 files changed, 842 insertions(+), 407 deletions(-) create mode 100644 core/src/test/java/feign/codec/EncoderPredicateTest.java create mode 100644 core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java delete mode 100644 core/src/test/java/feign/codec/PredicatedEncoderTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 14e5619a39..804504da56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,14 @@ ### Version 13.14 -* Add `@Experimental` `MultiEncoder`, `PredicatedEncoder` and `EncoderPredicate`, letting a single client pick an - encoder per request. `Feign.builder().encoder(defaultEncoder, predicatedEncoders...)` builds one; - predicates are consulted in order and the default encoder is the fallback. `Util.isJsonContentType` - and `Util.isXmlContentType` back the `PredicatedEncoder.forJsonContentType`/`forXmlContentType` - factories. The `Encoder` interface is unchanged, so existing encoders keep working (#3485). - -* `JAXBContextFactory.withProperty` is now applied when creating Unmarshallers, not only - Marshallers. Marshaller-only properties are skipped on unmarshal (#3056). +* Add `@Experimental` `MultiEncoder`, `PredicatedEncoder` and `EncoderPredicate`, letting a single + client route each request to the right encoder. Encoders declare what they can handle by + implementing `PredicatedEncoder`; anything else is paired with a predicate via + `MultiEncoder.builder(defaultEncoder)`. The first-party JSON encoders (Gson, Jackson, Jackson 3, + Jackson Jr, Jackson JAXB, Moshi, Fastjson2, JSON-java) and XML encoders (JAXB, JAXB Jakarta, SOAP, + SOAP Jakarta) now declare themselves, and the metrics modules' `MeteredEncoder` forwards + `canEncode` to the encoder it wraps. The `Encoder` interface is unchanged, so existing encoders + keep working (#3485). + * Add support for the HTTP QUERY method (RFC 10008) — safe, idempotent, and cacheable with a request body. `HttpCacheInterceptor` includes QUERY in its default cacheable set and incorporates a body hash into the cache key to reduce cross-body collisions. diff --git a/README.md b/README.md index a66ad92e3f..7925caa30a 100644 --- a/README.md +++ b/README.md @@ -714,8 +714,10 @@ public class Example { > This API is `@Experimental` and may change incompatibly, or be removed, in a future release. A single client sometimes has to speak more than one format — JSON for most endpoints, XML for -a legacy one, plain bytes for an upload. `MultiEncoder` picks the encoder per request by asking each -candidate's predicate, falling back to a default encoder when none match. +a legacy one, plain bytes for an upload. `MultiEncoder` routes each request to the right encoder, +falling back to a default when none applies. + +Most first-party encoders already declare what they can handle, so they can simply be added: ```java interface MixedClient { @@ -731,33 +733,55 @@ interface MixedClient { public class Example { public static void main(String[] args) { MixedClient client = Feign.builder() - .encoder( - new DefaultEncoder(), - PredicatedEncoder.forJsonContentType(new GsonEncoder()), - PredicatedEncoder.forXmlContentType(new JAXBEncoder())) + .encoder(new DefaultEncoder(), new GsonEncoder(), new JAXBEncoder()) .target(MixedClient.class, "https://foo.com"); } } ``` -The first argument is the default encoder, used when no predicate accepts the request. The remaining -arguments are consulted in the order given, so the most specific encoder should come first. +The first argument is the default encoder, used when nothing else accepts the request. -`PredicatedEncoder` ships with factories for the common cases — `forJsonContentType`, -`forXmlContentType` and `forEmptyBody`. `EncoderPredicate` is a functional interface, so any other -condition is a lambda over the same three arguments `Encoder#encode` receives: +For an encoder that does not declare itself — including one you do not control — pair it +with an `EncoderPredicate` using the builder: ```java Encoder encoder = - MultiEncoder.of( - new DefaultEncoder(), - new PredicatedEncoder( - (object, bodyType, template) -> bodyType == byte[].class, new BinaryEncoder()), - PredicatedEncoder.forJsonContentType(new GsonEncoder())); + MultiEncoder.builder(new DefaultEncoder()) + .add(new GsonEncoder()) // declares itself + .add(EncoderPredicate.xmlContentType(), someXmlEncoder) // paired + .add((object, bodyType, template) -> bodyType == byte[].class, binaryEncoder) + .build(); ``` -A `PredicatedEncoder` used on its own, outside a `MultiEncoder`, is strict: a request its predicate -rejects raises `EncodeException` rather than silently encoding nothing. +Delegates are consulted in the order they were added, so put the narrowest predicate first. Note +that `Content-Type: application/json` with a null body is claimed by a JSON encoder before +`EncoderPredicate.emptyBody()` gets a chance — order accordingly. + +##### Declaring your own encoder + +Implement `PredicatedEncoder` alongside `Encoder` and override `canEncode`: + +```java +public class MyEncoder implements Encoder, PredicatedEncoder { + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + // ... + } +} +``` + +`EncoderPredicate` ships with `jsonContentType()`, `xmlContentType()`, `contentType(mediaType)`, +`emptyBody()`, `bodyType(type)` and `formEncoded()`, plus `and`/`or`/`negate` to combine them. + +**If you wrap an encoder, forward `canEncode` to your delegate.** A wrapper that does not will claim +every request, because the default `canEncode` accepts everything. The metrics modules' +`MeteredEncoder` forwards for exactly this reason. ### @Body templates The `@Body` annotation indicates a template to expand using parameters annotated with `@Param`. 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 abc246db3d..12cbf9c3cb 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -97,25 +97,29 @@ public B encoder(Encoder encoder) { } /** - * Configures a {@link MultiEncoder} that picks an encoder per request. + * Configures a {@link MultiEncoder} built from encoders that declare their own applicability. * *

Each {@link PredicatedEncoder} is consulted in the order given; {@code defaultEncoder} is - * the fallback used when no predicate accepts the request. + * the fallback used when none accepts the request. * * 

    * Feign.builder()
-   *     .encoder(
-   *         new DefaultEncoder(),
-   *         PredicatedEncoder.forJsonContentType(new JacksonEncoder()),
-   *         PredicatedEncoder.forXmlContentType(new JAXBEncoder()))
+   *     .encoder(new DefaultEncoder(), new JacksonEncoder(), new JAXBEncoder())
    *
* - * @param defaultEncoder the encoder used when no predicate accepts the request + *

To pair a predicate with an encoder that does not implement {@link PredicatedEncoder}, use + * {@link MultiEncoder#builder(Encoder)} instead. + * + * @param defaultEncoder the encoder used when no delegate accepts the request * @param encoders the predicated encoders, consulted in the order given */ @Experimental public B encoder(Encoder defaultEncoder, PredicatedEncoder... encoders) { - return encoder(MultiEncoder.of(defaultEncoder, encoders)); + MultiEncoder.Builder builder = MultiEncoder.builder(defaultEncoder); + for (PredicatedEncoder encoder : encoders) { + builder.add(encoder); + } + return encoder(builder.build()); } public B decoder(Decoder decoder) { diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index 2b4b8d5bd5..589ae406ca 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -413,13 +413,38 @@ public static boolean isXmlContentType(RequestTemplate template) { return hasContentTypeMatching(template, XML_CONTENT_TYPE); } - private static boolean hasContentTypeMatching(RequestTemplate template, Pattern pattern) { + /** + * Checks whether the {@code Content-Type} header of the given template starts with the given + * media type, ignoring case and any parameters such as {@code ;charset=utf-8}. + * + * @param template the request template to check + * @param mediaType the media type to look for, for example {@code + * application/x-www-form-urlencoded} + * @return {@code true} if the content type matches, {@code false} otherwise + */ + @Experimental + public static boolean hasContentType(RequestTemplate template, String mediaType) { + return contentTypes(template) + .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(RequestTemplate template) { return template.headers().entrySet().stream() .filter(header -> CONTENT_TYPE.equalsIgnoreCase(header.getKey())) .map(Map.Entry::getValue) .filter(Objects::nonNull) .flatMap(Collection::stream) - .anyMatch( - contentType -> contentType != null && pattern.matcher(contentType.trim()).matches()); + .filter(Objects::nonNull); + } + + private static boolean hasContentTypeMatching(RequestTemplate template, Pattern pattern) { + return contentTypes(template) + .anyMatch(contentType -> pattern.matcher(contentType.trim()).matches()); } } diff --git a/core/src/main/java/feign/codec/EncoderPredicate.java b/core/src/main/java/feign/codec/EncoderPredicate.java index 16a9b25503..1383dae47e 100644 --- a/core/src/main/java/feign/codec/EncoderPredicate.java +++ b/core/src/main/java/feign/codec/EncoderPredicate.java @@ -17,10 +17,12 @@ import feign.Experimental; import feign.RequestTemplate; +import feign.Util; import java.lang.reflect.Type; +import java.util.Objects; /** - * A predicate that decides whether a given request can be handled by an {@link Encoder}. + * Decides whether a request can be handled by an {@link Encoder}. * *

Predicates receive the same three arguments as {@link Encoder#encode(Object, Type, * RequestTemplate)}, so they can discriminate on the body, on its declared type, or on anything @@ -29,12 +31,12 @@ * @see PredicatedEncoder * @see MultiEncoder */ -@FunctionalInterface @Experimental +@FunctionalInterface public interface EncoderPredicate { /** - * Tests whether the given request can be encoded. + * Whether the encoder this predicate guards can handle the request. * * @param object what would be encoded as the request body * @param bodyType the type the object would be encoded as. {@link Encoder#MAP_STRING_WILDCARD} @@ -42,5 +44,56 @@ public interface EncoderPredicate { * @param template the request template that would be populated * @return {@code true} if the request can be encoded, {@code false} otherwise */ - boolean test(Object object, Type bodyType, RequestTemplate template); + boolean canEncode(Object object, Type bodyType, RequestTemplate template); + + /** Matches requests whose {@code Content-Type} header denotes JSON. */ + static EncoderPredicate jsonContentType() { + return (object, bodyType, template) -> Util.isJsonContentType(template); + } + + /** Matches requests whose {@code Content-Type} header denotes XML. */ + static EncoderPredicate xmlContentType() { + return (object, bodyType, template) -> Util.isXmlContentType(template); + } + + /** + * Matches requests whose {@code Content-Type} header starts with the given media type, ignoring + * case and any parameters such as {@code ;charset=utf-8}. + */ + static EncoderPredicate contentType(String mediaType) { + Objects.requireNonNull(mediaType, "mediaType cannot be null"); + return (object, bodyType, template) -> Util.hasContentType(template, mediaType); + } + + /** Matches requests carrying no body. */ + static EncoderPredicate emptyBody() { + return (object, bodyType, template) -> object == null; + } + + /** Matches requests whose declared body type is exactly the given type. */ + static EncoderPredicate bodyType(Type type) { + Objects.requireNonNull(type, "type cannot be null"); + return (object, bodyType, template) -> type.equals(bodyType); + } + + /** Matches form-encoded requests, as signalled by {@link Encoder#MAP_STRING_WILDCARD}. */ + static EncoderPredicate formEncoded() { + return (object, bodyType, template) -> Encoder.MAP_STRING_WILDCARD.equals(bodyType); + } + + default EncoderPredicate and(EncoderPredicate other) { + Objects.requireNonNull(other, "other cannot be null"); + return (object, bodyType, template) -> + canEncode(object, bodyType, template) && other.canEncode(object, bodyType, template); + } + + default EncoderPredicate or(EncoderPredicate other) { + Objects.requireNonNull(other, "other cannot be null"); + return (object, bodyType, template) -> + canEncode(object, bodyType, template) || other.canEncode(object, bodyType, template); + } + + default EncoderPredicate negate() { + return (object, bodyType, template) -> !canEncode(object, bodyType, template); + } } diff --git a/core/src/main/java/feign/codec/MultiEncoder.java b/core/src/main/java/feign/codec/MultiEncoder.java index 494fad00d3..c2ab15e8e5 100644 --- a/core/src/main/java/feign/codec/MultiEncoder.java +++ b/core/src/main/java/feign/codec/MultiEncoder.java @@ -19,69 +19,58 @@ import feign.RequestTemplate; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; /** - * An encoder that delegates to a list of {@link PredicatedEncoder}s, using the first one whose - * predicate accepts the request, and falling back to a default encoder when none do. + * An {@link Encoder} that selects a delegate per request, falling back to a default encoder when no + * delegate accepts it. * - *

The default encoder is declared first so the predicated ones can be supplied as varargs, but - * it is consulted last — it is the fallback, not the first choice. + *

Delegates come from two places. An encoder that implements {@link PredicatedEncoder} declares + * its own applicability and can simply be added; any other encoder is paired with an {@link + * EncoderPredicate} at the call site: * * 

- * Encoder encoder =
- *     MultiEncoder.of(
- *         new DefaultEncoder(),
- *         PredicatedEncoder.forJsonContentType(new JacksonEncoder()),
- *         PredicatedEncoder.forXmlContentType(new JAXBEncoder()));
+ * Feign.builder()
+ *     .encoder(
+ *         MultiEncoder.builder(new DefaultEncoder())
+ *             .add(new JacksonEncoder())
+ *             .add(EncoderPredicate.xmlContentType(), new JAXBEncoder())
+ *             .add((object, bodyType, template) -> bodyType == byte[].class, new BinaryEncoder())
+ *             .build());
  *
+ * + *

Delegates are consulted in the order they were added, so the narrowest predicate should come + * first. The default encoder is consulted last. + * + * @see PredicatedEncoder + * @see EncoderPredicate */ @Experimental public class MultiEncoder implements Encoder { private final Encoder defaultEncoder; - private final List delegates; + private final List delegates; - /** - * Creates an encoder that tries each predicated encoder in order and falls back to {@code - * defaultEncoder}. - * - * @param defaultEncoder the encoder used when no predicate accepts the request - * @param encoders the predicated encoders, consulted in the order given - * @return the multi-encoder - */ - public static Encoder of(Encoder defaultEncoder, PredicatedEncoder... encoders) { - return of(defaultEncoder, Arrays.asList(encoders)); + private MultiEncoder(Encoder defaultEncoder, List delegates) { + this.defaultEncoder = defaultEncoder; + this.delegates = Collections.unmodifiableList(new ArrayList<>(delegates)); } /** - * Creates an encoder that tries each predicated encoder in order and falls back to {@code - * defaultEncoder}. + * Starts building a multi-encoder. * - * @param defaultEncoder the encoder used when no predicate accepts the request - * @param encoders the predicated encoders, consulted in the order given - * @return the multi-encoder + * @param defaultEncoder the encoder used when no delegate accepts the request + * @return the builder */ - public static Encoder of(Encoder defaultEncoder, List encoders) { - return new MultiEncoder(defaultEncoder, encoders); - } - - private MultiEncoder(Encoder defaultEncoder, List delegates) { - this.defaultEncoder = Objects.requireNonNull(defaultEncoder, "defaultEncoder cannot be null"); - Objects.requireNonNull(delegates, "delegates cannot be null"); - for (PredicatedEncoder delegate : delegates) { - Objects.requireNonNull(delegate, "delegates cannot contain null"); - } - this.delegates = Collections.unmodifiableList(new ArrayList<>(delegates)); + public static Builder builder(Encoder defaultEncoder) { + return new Builder(defaultEncoder); } /** - * Encodes using the first delegate whose predicate accepts the request, or the default encoder if - * none do. + * Encodes using the first delegate that accepts the request, or the default encoder if none do. * * @param object {@inheritDoc} * @param bodyType {@inheritDoc} @@ -91,9 +80,9 @@ private MultiEncoder(Encoder defaultEncoder, List delegates) @Override public void encode(Object object, Type bodyType, RequestTemplate template) throws EncodeException { - for (PredicatedEncoder delegate : delegates) { - if (delegate.canEncode(object, bodyType, template)) { - delegate.encode(object, bodyType, template); + for (Delegate delegate : delegates) { + if (delegate.predicate.canEncode(object, bodyType, template)) { + delegate.encoder.encode(object, bodyType, template); return; } } @@ -104,4 +93,61 @@ public void encode(Object object, Type bodyType, RequestTemplate template) public String toString() { return "MultiEncoder{defaultEncoder=" + defaultEncoder + ", delegates=" + delegates + '}'; } + + private static final class Delegate { + private final EncoderPredicate predicate; + private final Encoder encoder; + + Delegate(EncoderPredicate predicate, Encoder encoder) { + this.predicate = predicate; + this.encoder = encoder; + } + + @Override + public String toString() { + return encoder.toString(); + } + } + + /** Collects the delegates of a {@link MultiEncoder}. */ + @Experimental + public static final class Builder { + + private final Encoder defaultEncoder; + + private final List delegates = new ArrayList<>(); + + private Builder(Encoder defaultEncoder) { + this.defaultEncoder = Objects.requireNonNull(defaultEncoder, "defaultEncoder cannot be null"); + } + + /** + * Adds an encoder that declares its own applicability. + * + * @param encoder the encoder, consulted via {@link PredicatedEncoder#canEncode} + */ + public Builder add(PredicatedEncoder encoder) { + Objects.requireNonNull(encoder, "encoder cannot be null"); + return add(encoder::canEncode, encoder); + } + + /** + * Adds any encoder, guarded by the given predicate. Use this for encoders that do not implement + * {@link PredicatedEncoder}, including ones you do not control. + * + * @param predicate decides whether the encoder handles a request + * @param encoder the encoder to delegate to + */ + public Builder add(EncoderPredicate predicate, Encoder encoder) { + Objects.requireNonNull(predicate, "predicate cannot be null"); + Objects.requireNonNull(encoder, "encoder cannot be null"); + delegates.add(new Delegate(predicate, encoder)); + return this; + } + + /** Builds the multi-encoder. */ + public MultiEncoder build() { + return new MultiEncoder(defaultEncoder, delegates); + } + } } diff --git a/core/src/main/java/feign/codec/PredicatedEncoder.java b/core/src/main/java/feign/codec/PredicatedEncoder.java index b8c130f725..c97cfd072f 100644 --- a/core/src/main/java/feign/codec/PredicatedEncoder.java +++ b/core/src/main/java/feign/codec/PredicatedEncoder.java @@ -17,79 +17,47 @@ import feign.Experimental; import feign.RequestTemplate; -import feign.Util; import java.lang.reflect.Type; -import java.util.Objects; /** - * Pairs an {@link EncoderPredicate} with the {@link Encoder} it guards, so that a {@link - * MultiEncoder} can pick the right encoder per request. + * An {@link Encoder} that knows which requests it can handle. * - *

Encoding through a {@code PredicatedEncoder} directly is allowed but strict: a request its - * predicate rejects raises {@link EncodeException} rather than silently doing nothing. Inside a - * {@link MultiEncoder} a rejected request simply moves on to the next candidate. + *

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

- * Feign.builder()
- *     .encoder(
- *         new DefaultEncoder(),
- *         PredicatedEncoder.forJsonContentType(new JacksonEncoder()),
- *         PredicatedEncoder.forXmlContentType(new JAXBEncoder()))
+ * public class JacksonEncoder implements Encoder, PredicatedEncoder {
+ *
+ *   @Override
+ *   public boolean canEncode(Object object, Type bodyType, RequestTemplate template) {
+ *     return EncoderPredicate.jsonContentType().canEncode(object, bodyType, template);
+ *   }
+ * }
  *
+ * + *

{@link Encoder#encode(Object, Type, RequestTemplate) encode} remains the only abstract method, + * so this stays a functional interface and a bare lambda is an encoder that accepts everything. + * + *

Encoders that wrap another encoder should forward {@code canEncode} to their delegate, so that + * wrapping does not discard the delegate's applicability. + * + * @see MultiEncoder + * @see EncoderPredicate */ @Experimental -public class PredicatedEncoder implements Encoder { - - private final EncoderPredicate predicate; - - private final Encoder delegate; - - public PredicatedEncoder(EncoderPredicate predicate, Encoder delegate) { - this.predicate = Objects.requireNonNull(predicate, "predicate cannot be null"); - this.delegate = Objects.requireNonNull(delegate, "delegate cannot be null"); - } - - /** Restricts the delegate to requests whose {@code Content-Type} header denotes JSON. */ - public static PredicatedEncoder forJsonContentType(Encoder delegate) { - return new PredicatedEncoder( - (object, bodyType, template) -> Util.isJsonContentType(template), delegate); - } - - /** Restricts the delegate to requests whose {@code Content-Type} header denotes XML. */ - public static PredicatedEncoder forXmlContentType(Encoder delegate) { - return new PredicatedEncoder( - (object, bodyType, template) -> Util.isXmlContentType(template), delegate); - } - - /** Restricts the delegate to requests carrying no body. */ - public static PredicatedEncoder forEmptyBody(Encoder delegate) { - return new PredicatedEncoder((object, bodyType, template) -> object == null, delegate); - } +@FunctionalInterface +public interface PredicatedEncoder extends Encoder { /** - * Whether the guarded encoder accepts this request. + * Whether this encoder can handle the request. Defaults to accepting everything. * * @param object what to encode as the request body - * @param bodyType the type the object should be encoded as + * @param bodyType the type the object should be encoded as. {@link Encoder#MAP_STRING_WILDCARD} + * indicates form encoding. * @param template the request template to populate - * @return {@code true} if the delegate should handle this request + * @return {@code true} if this encoder can encode the request, {@code false} otherwise */ - public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { - return predicate.test(object, bodyType, template); - } - - @Override - public void encode(Object object, Type bodyType, RequestTemplate template) - throws EncodeException { - if (!canEncode(object, bodyType, template)) { - throw new EncodeException( - "Predicate of " + this + " rejected the request, so " + delegate + " was not invoked"); - } - delegate.encode(object, bodyType, template); - } - - @Override - public String toString() { - return "PredicatedEncoder{predicate=" + predicate + ", delegate=" + delegate + '}'; + default boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return true; } } diff --git a/core/src/test/java/feign/codec/EncoderPredicateTest.java b/core/src/test/java/feign/codec/EncoderPredicateTest.java new file mode 100644 index 0000000000..899085d9fc --- /dev/null +++ b/core/src/test/java/feign/codec/EncoderPredicateTest.java @@ -0,0 +1,115 @@ +/* + * 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.RequestTemplate; +import org.junit.jupiter.api.Test; + +class EncoderPredicateTest { + + private static RequestTemplate template(String contentType) { + RequestTemplate template = new RequestTemplate(); + if (contentType != null) { + template.header("Content-Type", contentType); + } + return template; + } + + private static boolean test(EncoderPredicate predicate, String contentType) { + return predicate.canEncode("body", String.class, template(contentType)); + } + + @Test + void jsonContentTypeMatchesJsonOnly() { + EncoderPredicate json = EncoderPredicate.jsonContentType(); + + assertThat(test(json, "application/json")).isTrue(); + assertThat(test(json, "application/json;charset=utf-8")).isTrue(); + assertThat(test(json, "application/vnd.github+json")).isTrue(); + assertThat(test(json, "text/json")).isTrue(); + assertThat(test(json, "application/xml")).isFalse(); + assertThat(test(json, null)).isFalse(); + } + + @Test + void xmlContentTypeMatchesXmlOnly() { + EncoderPredicate xml = EncoderPredicate.xmlContentType(); + + assertThat(test(xml, "application/xml")).isTrue(); + assertThat(test(xml, "text/xml")).isTrue(); + assertThat(test(xml, "application/soap+xml")).isTrue(); + assertThat(test(xml, "application/json")).isFalse(); + assertThat(test(xml, null)).isFalse(); + } + + @Test + void contentTypeMatchesExactMediaTypeIgnoringParameters() { + EncoderPredicate form = EncoderPredicate.contentType("application/x-www-form-urlencoded"); + + assertThat(test(form, "application/x-www-form-urlencoded")).isTrue(); + assertThat(test(form, "APPLICATION/X-WWW-FORM-URLENCODED")).isTrue(); + assertThat(test(form, "application/x-www-form-urlencoded;charset=utf-8")).isTrue(); + assertThat(test(form, "application/x-www-form-urlencoded-extra")).isFalse(); + assertThat(test(form, "application/json")).isFalse(); + } + + @Test + void headerNameIsMatchedCaseInsensitively() { + RequestTemplate template = new RequestTemplate(); + template.header("content-type", "application/json"); + + assertThat(EncoderPredicate.jsonContentType().canEncode("body", String.class, template)) + .isTrue(); + } + + @Test + void emptyBodyMatchesNullBodyOnly() { + EncoderPredicate empty = EncoderPredicate.emptyBody(); + + assertThat(empty.canEncode(null, String.class, template(null))).isTrue(); + assertThat(empty.canEncode("body", String.class, template(null))).isFalse(); + } + + @Test + void bodyTypeMatchesExactType() { + EncoderPredicate bytes = EncoderPredicate.bodyType(byte[].class); + + assertThat(bytes.canEncode(new byte[0], byte[].class, template(null))).isTrue(); + assertThat(bytes.canEncode("body", String.class, template(null))).isFalse(); + } + + @Test + void formEncodedMatchesTheFormBodyTypeMarker() { + EncoderPredicate form = EncoderPredicate.formEncoded(); + + assertThat(form.canEncode(null, Encoder.MAP_STRING_WILDCARD, template(null))).isTrue(); + assertThat(form.canEncode("body", String.class, template(null))).isFalse(); + } + + @Test + void combinators() { + EncoderPredicate json = EncoderPredicate.jsonContentType(); + EncoderPredicate xml = EncoderPredicate.xmlContentType(); + + assertThat(test(json.or(xml), "application/xml")).isTrue(); + assertThat(test(json.or(xml), "text/plain")).isFalse(); + assertThat(test(json.and(xml), "application/json")).isFalse(); + assertThat(test(json.negate(), "application/xml")).isTrue(); + assertThat(test(json.negate(), "application/json")).isFalse(); + } +} diff --git a/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java b/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java new file mode 100644 index 0000000000..8432acaea2 --- /dev/null +++ b/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java @@ -0,0 +1,170 @@ +/* + * 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.Headers; +import feign.RequestLine; +import feign.RequestTemplate; +import feign.Response; +import feign.Util; +import java.lang.reflect.Type; +import java.util.Collections; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; + +/** How {@link MultiEncoder} behaves when a {@link Capability} wraps the configured encoder. */ +class MultiEncoderCapabilityTest { + + interface MixedApi { + @RequestLine("POST /json") + @Headers("Content-Type: application/json") + void json(String body); + + @RequestLine("POST /xml") + @Headers("Content-Type: application/xml") + void xml(String body); + } + + static class TaggingEncoder implements Encoder { + private final String tag; + + TaggingEncoder(String tag) { + this.tag = tag; + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + template.body(tag); + } + } + + /** A capability that wraps the encoder, the way the metrics modules do. */ + public static class CountingCapability implements Capability { + int wrapped; + int encodeCalls; + + @Override + public Encoder enrich(Encoder encoder) { + wrapped++; + return (object, bodyType, template) -> { + encodeCalls++; + encoder.encode(object, bodyType, template); + }; + } + } + + private static MixedApi target(Feign.Builder builder, AtomicReference captured) { + return builder + .client( + (request, options) -> { + captured.set(new String(request.body(), Util.UTF_8)); + return Response.builder() + .status(200) + .reason("OK") + .request(request) + .headers(Collections.emptyMap()) + .body("", Util.UTF_8) + .build(); + }) + .target(MixedApi.class, "http://localhost:1"); + } + + private static RequestTemplate template(String contentType) { + RequestTemplate template = new RequestTemplate(); + template.header("Content-Type", contentType); + return template; + } + + @Test + void capabilityWrapsTheCompositeAndRoutingStillWorks() { + CountingCapability capability = new CountingCapability(); + AtomicReference captured = new AtomicReference<>(); + + MixedApi api = + target( + Feign.builder() + .encoder( + MultiEncoder.builder(new TaggingEncoder("fallback")) + .add(EncoderPredicate.jsonContentType(), new TaggingEncoder("json")) + .add(EncoderPredicate.xmlContentType(), new TaggingEncoder("xml")) + .build()) + .addCapability(capability), + captured); + + api.json("{}"); + assertThat(captured.get()).isEqualTo("json"); + + api.xml(""); + assertThat(captured.get()).isEqualTo("xml"); + + // the capability sees the MultiEncoder as one encoder, not one per delegate + assertThat(capability.wrapped).isEqualTo(1); + assertThat(capability.encodeCalls).isEqualTo(2); + } + + /** + * A wrapper that does not forward {@code canEncode} claims every request, which is why the + * metrics modules' {@code MeteredEncoder} forwards it to its delegate. + */ + @Test + void wrappingWithoutForwardingCanEncodeErasesSelfDeclaration() { + PredicatedEncoder jsonOnly = + new PredicatedEncoder() { + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + template.body("json"); + } + }; + + PredicatedEncoder naive = jsonOnly::encode; + + PredicatedEncoder forwarding = + new PredicatedEncoder() { + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return jsonOnly.canEncode(object, bodyType, template); + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + jsonOnly.encode(object, bodyType, template); + } + }; + + RequestTemplate naiveTemplate = template("application/xml"); + MultiEncoder.builder(new TaggingEncoder("fallback")) + .add(naive) + .build() + .encode("body", String.class, naiveTemplate); + assertThat(naiveTemplate.requestBody().asString()).isEqualTo("json"); + + RequestTemplate forwardedTemplate = template("application/xml"); + MultiEncoder.builder(new TaggingEncoder("fallback")) + .add(forwarding) + .build() + .encode("body", String.class, forwardedTemplate); + assertThat(forwardedTemplate.requestBody().asString()).isEqualTo("fallback"); + } +} diff --git a/core/src/test/java/feign/codec/MultiEncoderTest.java b/core/src/test/java/feign/codec/MultiEncoderTest.java index a02f53eed0..c00021ad4c 100644 --- a/core/src/test/java/feign/codec/MultiEncoderTest.java +++ b/core/src/test/java/feign/codec/MultiEncoderTest.java @@ -20,13 +20,13 @@ import feign.Request; import feign.RequestTemplate; +import feign.Util; import java.lang.reflect.Type; -import java.util.Arrays; -import java.util.Collections; import org.junit.jupiter.api.Test; class MultiEncoderTest { + /** A plain encoder, with no opinion about what it can handle. */ private static class RecordingEncoder implements Encoder { private final String body; boolean invoked; @@ -42,6 +42,20 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { } } + /** An encoder that declares its own applicability, the way feign-gson and friends now do. */ + private static class SelfDeclaringJsonEncoder extends RecordingEncoder + implements PredicatedEncoder { + + SelfDeclaringJsonEncoder() { + super("json"); + } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } + } + private static RequestTemplate templateWithContentType(String contentType) { RequestTemplate template = new RequestTemplate(); if (contentType != null) { @@ -51,45 +65,75 @@ private static RequestTemplate templateWithContentType(String contentType) { } @Test - void usesFirstDelegateWhosePredicateAccepts() { - RecordingEncoder json = new RecordingEncoder("json"); - RecordingEncoder xml = new RecordingEncoder("xml"); + void routesToTheEncoderThatDeclaresItCanHandleTheRequest() { + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = - MultiEncoder.of( - fallback, - PredicatedEncoder.forJsonContentType(json), - PredicatedEncoder.forXmlContentType(xml)); + Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); RequestTemplate template = templateWithContentType("application/json"); encoder.encode("body", String.class, template); assertThat(json.invoked).isTrue(); - assertThat(xml.invoked).isFalse(); assertThat(fallback.invoked).isFalse(); assertThat(template.requestBody().asString()).isEqualTo("json"); } + @Test + void pairsAPredicateWithAnEncoderThatDoesNotDeclareItself() { + RecordingEncoder xml = new RecordingEncoder("xml"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = + MultiEncoder.builder(fallback).add(EncoderPredicate.xmlContentType(), xml).build(); + + encoder.encode("body", String.class, templateWithContentType("application/xml")); + + assertThat(xml.invoked).isTrue(); + assertThat(fallback.invoked).isFalse(); + } + + @Test + void mixesSelfDeclaringEncodersAndPairs() { + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); + RecordingEncoder xml = new RecordingEncoder("xml"); + RecordingEncoder binary = new RecordingEncoder("binary"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = + MultiEncoder.builder(fallback) + .add(json) + .add(EncoderPredicate.xmlContentType(), xml) + .add(EncoderPredicate.bodyType(byte[].class), binary) + .build(); + + encoder.encode( + new byte[] {1}, byte[].class, templateWithContentType("application/octet-stream")); + + assertThat(binary.invoked).isTrue(); + assertThat(json.invoked).isFalse(); + assertThat(xml.invoked).isFalse(); + assertThat(fallback.invoked).isFalse(); + } + @Test void matchesSuffixedContentTypes() { - RecordingEncoder json = new RecordingEncoder("json"); + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.of(fallback, PredicatedEncoder.forJsonContentType(json)); + Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); encoder.encode("body", String.class, templateWithContentType("application/vnd.github+json")); assertThat(json.invoked).isTrue(); - assertThat(fallback.invoked).isFalse(); } @Test - void fallsBackToDefaultEncoderWhenNoPredicateAccepts() { - RecordingEncoder json = new RecordingEncoder("json"); + void fallsBackWhenNoDelegateAccepts() { + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.of(fallback, PredicatedEncoder.forJsonContentType(json)); + Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); RequestTemplate template = templateWithContentType("text/plain"); encoder.encode("body", String.class, template); @@ -100,29 +144,61 @@ void fallsBackToDefaultEncoderWhenNoPredicateAccepts() { } @Test - void fallsBackToDefaultEncoderWhenNoContentTypeIsSet() { - RecordingEncoder json = new RecordingEncoder("json"); + void fallsBackWhenNoContentTypeIsSet() { + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.of(fallback, PredicatedEncoder.forJsonContentType(json)); + Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); encoder.encode("body", String.class, templateWithContentType(null)); - assertThat(json.invoked).isFalse(); assertThat(fallback.invoked).isTrue(); } @Test - void withoutDelegatesEverythingGoesToTheDefaultEncoder() { + void withNoDelegatesEverythingGoesToTheDefaultEncoder() { RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.of(fallback); + Encoder encoder = MultiEncoder.builder(fallback).build(); encoder.encode("body", String.class, templateWithContentType("application/json")); assertThat(fallback.invoked).isTrue(); } + @Test + void delegatesAreConsultedInOrder() { + RecordingEncoder first = new RecordingEncoder("first"); + RecordingEncoder second = new RecordingEncoder("second"); + RecordingEncoder fallback = new RecordingEncoder("fallback"); + + Encoder encoder = + MultiEncoder.builder(fallback) + .add(EncoderPredicate.jsonContentType(), first) + .add(EncoderPredicate.jsonContentType(), second) + .build(); + + encoder.encode("body", String.class, templateWithContentType("application/json")); + + assertThat(first.invoked).isTrue(); + assertThat(second.invoked).isFalse(); + } + + @Test + void anEncoderWithoutAPredicateAcceptsEverything() { + // a bare lambda is a PredicatedEncoder whose default canEncode returns true + RecordingEncoder fallback = new RecordingEncoder("fallback"); + PredicatedEncoder greedy = (object, bodyType, template) -> template.body("greedy"); + + Encoder encoder = MultiEncoder.builder(fallback).add(greedy).build(); + + RequestTemplate template = templateWithContentType("text/plain"); + encoder.encode("body", String.class, template); + + assertThat(fallback.invoked).isFalse(); + assertThat(template.requestBody().asString()).isEqualTo("greedy"); + } + @Test void propagatesEncodeExceptionFromDelegate() { Encoder failing = @@ -131,7 +207,9 @@ void propagatesEncodeExceptionFromDelegate() { }; Encoder encoder = - MultiEncoder.of(new DefaultEncoder(), PredicatedEncoder.forJsonContentType(failing)); + MultiEncoder.builder(new DefaultEncoder()) + .add(EncoderPredicate.jsonContentType(), failing) + .build(); assertThatThrownBy( () -> encoder.encode("body", String.class, templateWithContentType("application/json"))) @@ -140,39 +218,26 @@ void propagatesEncodeExceptionFromDelegate() { } @Test - void rejectsNullDefaultEncoder() { - assertThatThrownBy(() -> MultiEncoder.of(null)) + void rejectsNullArguments() { + assertThatThrownBy(() -> MultiEncoder.builder(null)) .isInstanceOf(NullPointerException.class) .hasMessage("defaultEncoder cannot be null"); - } - - @Test - void rejectsNullDelegate() { - assertThatThrownBy(() -> MultiEncoder.of(new DefaultEncoder(), Collections.singletonList(null))) + assertThatThrownBy(() -> MultiEncoder.builder(new DefaultEncoder()).add(null)) .isInstanceOf(NullPointerException.class) - .hasMessage("delegates cannot contain null"); + .hasMessage("encoder cannot be null"); + assertThatThrownBy( + () -> MultiEncoder.builder(new DefaultEncoder()).add(null, new DefaultEncoder())) + .isInstanceOf(NullPointerException.class) + .hasMessage("predicate cannot be null"); } @Test void toStringDescribesDelegates() { Encoder encoder = - MultiEncoder.of( - new DefaultEncoder(), PredicatedEncoder.forJsonContentType(new DefaultEncoder())); + MultiEncoder.builder(new DefaultEncoder()) + .add(EncoderPredicate.jsonContentType(), new RecordingEncoder("json")) + .build(); assertThat(encoder.toString()).startsWith("MultiEncoder{defaultEncoder="); - assertThat(encoder.toString()).contains("PredicatedEncoder{"); - } - - @Test - void listFactoryIsEquivalentToVarargs() { - RecordingEncoder json = new RecordingEncoder("json"); - RecordingEncoder fallback = new RecordingEncoder("fallback"); - - Encoder encoder = - MultiEncoder.of(fallback, Arrays.asList(PredicatedEncoder.forJsonContentType(json))); - - encoder.encode("body", String.class, templateWithContentType("application/json")); - - assertThat(json.invoked).isTrue(); } } diff --git a/core/src/test/java/feign/codec/PredicatedEncoderTest.java b/core/src/test/java/feign/codec/PredicatedEncoderTest.java deleted file mode 100644 index 69a90b08d1..0000000000 --- a/core/src/test/java/feign/codec/PredicatedEncoderTest.java +++ /dev/null @@ -1,138 +0,0 @@ -/* - * 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.RequestTemplate; -import java.lang.reflect.Type; -import org.junit.jupiter.api.Test; - -class PredicatedEncoderTest { - - private static class RecordingEncoder implements Encoder { - boolean invoked; - - @Override - public void encode(Object object, Type bodyType, RequestTemplate template) { - invoked = true; - template.body(Request.Body.create("encoded")); - } - } - - private static RequestTemplate templateWithContentType(String contentType) { - RequestTemplate template = new RequestTemplate(); - if (contentType != null) { - template.header("Content-Type", contentType); - } - return template; - } - - @Test - void delegatesWhenPredicateAccepts() { - RecordingEncoder delegate = new RecordingEncoder(); - PredicatedEncoder encoder = new PredicatedEncoder((o, t, tpl) -> true, delegate); - - RequestTemplate template = templateWithContentType(null); - encoder.encode("body", String.class, template); - - assertThat(delegate.invoked).isTrue(); - assertThat(template.requestBody().asString()).isEqualTo("encoded"); - } - - @Test - void throwsAndSkipsDelegateWhenPredicateRejects() { - RecordingEncoder delegate = new RecordingEncoder(); - PredicatedEncoder encoder = new PredicatedEncoder((o, t, tpl) -> false, delegate); - - assertThatThrownBy(() -> encoder.encode("body", String.class, templateWithContentType(null))) - .isInstanceOf(EncodeException.class); - - assertThat(delegate.invoked).isFalse(); - } - - @Test - void canEncodeReflectsThePredicate() { - PredicatedEncoder encoder = - new PredicatedEncoder((o, t, tpl) -> "yes".equals(o), new RecordingEncoder()); - - assertThat(encoder.canEncode("yes", String.class, templateWithContentType(null))).isTrue(); - assertThat(encoder.canEncode("no", String.class, templateWithContentType(null))).isFalse(); - } - - @Test - void forJsonContentTypeMatchesJsonOnly() { - PredicatedEncoder encoder = PredicatedEncoder.forJsonContentType(new RecordingEncoder()); - - assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/json"))) - .isTrue(); - assertThat( - encoder.canEncode( - null, String.class, templateWithContentType("application/json;charset=utf-8"))) - .isTrue(); - assertThat( - encoder.canEncode( - null, String.class, templateWithContentType("application/vnd.github+json"))) - .isTrue(); - assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/xml"))) - .isFalse(); - assertThat(encoder.canEncode(null, String.class, templateWithContentType(null))).isFalse(); - } - - @Test - void forXmlContentTypeMatchesXmlOnly() { - PredicatedEncoder encoder = PredicatedEncoder.forXmlContentType(new RecordingEncoder()); - - assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/xml"))) - .isTrue(); - assertThat(encoder.canEncode(null, String.class, templateWithContentType("text/xml"))).isTrue(); - assertThat( - encoder.canEncode(null, String.class, templateWithContentType("application/soap+xml"))) - .isTrue(); - assertThat(encoder.canEncode(null, String.class, templateWithContentType("application/json"))) - .isFalse(); - } - - @Test - void contentTypeHeaderNameIsMatchedCaseInsensitively() { - PredicatedEncoder encoder = PredicatedEncoder.forJsonContentType(new RecordingEncoder()); - - RequestTemplate template = new RequestTemplate(); - template.header("content-type", "application/json"); - - assertThat(encoder.canEncode(null, String.class, template)).isTrue(); - } - - @Test - void forEmptyBodyMatchesNullBodyOnly() { - PredicatedEncoder encoder = PredicatedEncoder.forEmptyBody(new RecordingEncoder()); - - assertThat(encoder.canEncode(null, String.class, templateWithContentType(null))).isTrue(); - assertThat(encoder.canEncode("body", String.class, templateWithContentType(null))).isFalse(); - } - - @Test - void rejectsNullConstructorArguments() { - assertThatThrownBy(() -> new PredicatedEncoder(null, new RecordingEncoder())) - .isInstanceOf(NullPointerException.class) - .hasMessage("predicate cannot be null"); - assertThatThrownBy(() -> new PredicatedEncoder((o, t, tpl) -> true, null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("delegate cannot be null"); - } -} diff --git a/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredEncoder.java b/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredEncoder.java index 2a2c644c5f..f5eb123c45 100644 --- a/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredEncoder.java +++ b/dropwizard-metrics4/src/main/java/feign/metrics4/MeteredEncoder.java @@ -20,10 +20,11 @@ import feign.RequestTemplate; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; /** Warp feign {@link Encoder} with metrics. */ -public class MeteredEncoder implements Encoder { +public class MeteredEncoder implements Encoder, PredicatedEncoder { private final Encoder encoder; private final MetricRegistry metricRegistry; @@ -59,4 +60,10 @@ public void encode(Object object, Type bodyType, RequestTemplate template) .update(template.body().length); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return !(encoder instanceof PredicatedEncoder) + || ((PredicatedEncoder) encoder).canEncode(object, bodyType, template); + } } diff --git a/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredEncoder.java b/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredEncoder.java index 77cc7b78cb..2cff8cd788 100644 --- a/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredEncoder.java +++ b/dropwizard-metrics5/src/main/java/feign/metrics5/MeteredEncoder.java @@ -18,13 +18,14 @@ import feign.RequestTemplate; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import io.dropwizard.metrics5.MetricRegistry; import io.dropwizard.metrics5.Timer.Context; import java.lang.reflect.Type; import java.util.Map; /** Warp feign {@link Encoder} with metrics. */ -public class MeteredEncoder implements Encoder { +public class MeteredEncoder implements Encoder, PredicatedEncoder { private final Encoder encoder; private final MetricRegistry metricRegistry; @@ -71,4 +72,10 @@ public void encode(Object object, Type bodyType, RequestTemplate template) .update(template.body().length); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return !(encoder instanceof PredicatedEncoder) + || ((PredicatedEncoder) encoder).canEncode(object, bodyType, template); + } } diff --git a/fastjson2/src/main/java/feign/fastjson2/Fastjson2Encoder.java b/fastjson2/src/main/java/feign/fastjson2/Fastjson2Encoder.java index 06a98e8dd2..35efae25b6 100644 --- a/fastjson2/src/main/java/feign/fastjson2/Fastjson2Encoder.java +++ b/fastjson2/src/main/java/feign/fastjson2/Fastjson2Encoder.java @@ -22,12 +22,13 @@ import feign.codec.EncodeException; import feign.codec.Encoder; import feign.codec.JsonEncoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; /** * @author changjin wei(魏昌进) */ -public class Fastjson2Encoder implements Encoder, JsonEncoder { +public class Fastjson2Encoder implements Encoder, PredicatedEncoder, JsonEncoder { private final JSONWriter.Feature[] features; @@ -44,4 +45,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) throws EncodeException { template.body(JSON.toJSONBytes(object, features), Util.UTF_8); } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/gson/src/main/java/feign/gson/GsonEncoder.java b/gson/src/main/java/feign/gson/GsonEncoder.java index c4484bc6eb..1056d5f9ce 100644 --- a/gson/src/main/java/feign/gson/GsonEncoder.java +++ b/gson/src/main/java/feign/gson/GsonEncoder.java @@ -18,12 +18,14 @@ import com.google.gson.Gson; import com.google.gson.TypeAdapter; import feign.RequestTemplate; +import feign.Util; import feign.codec.Encoder; import feign.codec.JsonEncoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; import java.util.Collections; -public class GsonEncoder implements Encoder, JsonEncoder { +public class GsonEncoder implements Encoder, PredicatedEncoder, JsonEncoder { private final Gson gson; @@ -43,4 +45,9 @@ public GsonEncoder(Gson gson) { public void encode(Object object, Type bodyType, RequestTemplate template) { template.body(gson.toJson(object, bodyType)); } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonEncoder.java b/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonEncoder.java index 3786edb36e..67f42eda57 100644 --- a/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonEncoder.java +++ b/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonEncoder.java @@ -21,14 +21,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.lang.reflect.Type; import java.nio.charset.Charset; -public final class JacksonJaxbJsonEncoder implements Encoder { +public final class JacksonJaxbJsonEncoder implements Encoder, PredicatedEncoder { private final JacksonJaxbJsonProvider jacksonJaxbJsonProvider; public JacksonJaxbJsonEncoder() { @@ -51,4 +53,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) throw new EncodeException(e.getMessage(), e); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrEncoder.java b/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrEncoder.java index 44118eed76..e1ee4cfd43 100644 --- a/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrEncoder.java +++ b/jackson-jr/src/main/java/feign/jackson/jr/JacksonJrEncoder.java @@ -18,13 +18,15 @@ import com.fasterxml.jackson.jr.ob.JSON; import com.fasterxml.jackson.jr.ob.JacksonJrExtension; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import java.io.IOException; import java.lang.reflect.Type; /** A {@link Encoder} that uses Jackson Jr to convert objects to String or byte representation. */ -public class JacksonJrEncoder extends JacksonJrMapper implements Encoder { +public class JacksonJrEncoder extends JacksonJrMapper implements Encoder, PredicatedEncoder { public JacksonJrEncoder() { super(); @@ -61,4 +63,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { throw new EncodeException(e.getMessage(), e); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/jackson/src/main/java/feign/jackson/JacksonEncoder.java b/jackson/src/main/java/feign/jackson/JacksonEncoder.java index 48b169a8d3..1c1a9d83e3 100644 --- a/jackson/src/main/java/feign/jackson/JacksonEncoder.java +++ b/jackson/src/main/java/feign/jackson/JacksonEncoder.java @@ -26,10 +26,11 @@ import feign.codec.EncodeException; import feign.codec.Encoder; import feign.codec.JsonEncoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; import java.util.Collections; -public class JacksonEncoder implements Encoder, JsonEncoder { +public class JacksonEncoder implements Encoder, PredicatedEncoder, JsonEncoder { private final ObjectMapper mapper; @@ -58,4 +59,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { throw new EncodeException(e.getMessage(), e); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/jackson3/src/main/java/feign/jackson3/Jackson3Encoder.java b/jackson3/src/main/java/feign/jackson3/Jackson3Encoder.java index 90342c0162..41a5ab5893 100644 --- a/jackson3/src/main/java/feign/jackson3/Jackson3Encoder.java +++ b/jackson3/src/main/java/feign/jackson3/Jackson3Encoder.java @@ -21,6 +21,7 @@ import feign.codec.EncodeException; import feign.codec.Encoder; import feign.codec.JsonEncoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; import java.util.Collections; import tools.jackson.core.JacksonException; @@ -29,7 +30,7 @@ import tools.jackson.databind.SerializationFeature; import tools.jackson.databind.json.JsonMapper; -public class Jackson3Encoder implements Encoder, JsonEncoder { +public class Jackson3Encoder implements Encoder, PredicatedEncoder, JsonEncoder { private final JsonMapper mapper; @@ -60,4 +61,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { throw new EncodeException(e.getMessage(), e); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/jaxb-jakarta/src/main/java/feign/jaxb/JAXBEncoder.java b/jaxb-jakarta/src/main/java/feign/jaxb/JAXBEncoder.java index 4ea3b7e998..b5eed6dc3d 100644 --- a/jaxb-jakarta/src/main/java/feign/jaxb/JAXBEncoder.java +++ b/jaxb-jakarta/src/main/java/feign/jaxb/JAXBEncoder.java @@ -16,8 +16,10 @@ package feign.jaxb; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import jakarta.xml.bind.JAXBException; import jakarta.xml.bind.Marshaller; import java.io.StringWriter; @@ -42,7 +44,7 @@ *

The JAXBContextFactory should be reused across requests as it caches the created JAXB * contexts. */ -public class JAXBEncoder implements Encoder { +public class JAXBEncoder implements Encoder, PredicatedEncoder { private final JAXBContextFactory jaxbContextFactory; @@ -65,4 +67,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { throw new EncodeException(e.toString(), e); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isXmlContentType(template); + } } diff --git a/jaxb/src/main/java/feign/jaxb/JAXBEncoder.java b/jaxb/src/main/java/feign/jaxb/JAXBEncoder.java index aae439cae6..ace9b148cd 100644 --- a/jaxb/src/main/java/feign/jaxb/JAXBEncoder.java +++ b/jaxb/src/main/java/feign/jaxb/JAXBEncoder.java @@ -16,8 +16,10 @@ package feign.jaxb; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import java.io.StringWriter; import java.lang.reflect.Type; import javax.xml.bind.JAXBException; @@ -42,7 +44,7 @@ *

The JAXBContextFactory should be reused across requests as it caches the created JAXB * contexts. */ -public class JAXBEncoder implements Encoder { +public class JAXBEncoder implements Encoder, PredicatedEncoder { private final JAXBContextFactory jaxbContextFactory; @@ -65,4 +67,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { throw new EncodeException(e.toString(), e); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isXmlContentType(template); + } } diff --git a/json/src/main/java/feign/json/JsonEncoder.java b/json/src/main/java/feign/json/JsonEncoder.java index 655bb7594a..9e0b3a078f 100644 --- a/json/src/main/java/feign/json/JsonEncoder.java +++ b/json/src/main/java/feign/json/JsonEncoder.java @@ -18,8 +18,10 @@ import static java.lang.String.format; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; import org.json.JSONArray; import org.json.JSONObject; @@ -51,7 +53,7 @@ * github.create("openfeign", "feign", contributor); * */ -public class JsonEncoder implements Encoder { +public class JsonEncoder implements Encoder, PredicatedEncoder { @Override public void encode(Object object, Type bodyType, RequestTemplate template) @@ -63,4 +65,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) throw new EncodeException(format("%s is not a type supported by this encoder.", bodyType)); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/micrometer/src/main/java/feign/micrometer/MeteredEncoder.java b/micrometer/src/main/java/feign/micrometer/MeteredEncoder.java index 2fb73d12f5..197c2721f6 100644 --- a/micrometer/src/main/java/feign/micrometer/MeteredEncoder.java +++ b/micrometer/src/main/java/feign/micrometer/MeteredEncoder.java @@ -20,11 +20,12 @@ import feign.RequestTemplate; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import io.micrometer.core.instrument.*; import java.lang.reflect.Type; /** Wrap feign {@link Encoder} with metrics. */ -public class MeteredEncoder implements Encoder { +public class MeteredEncoder implements Encoder, PredicatedEncoder { private final Encoder encoder; private final MeterRegistry meterRegistry; @@ -79,4 +80,10 @@ protected DistributionSummary createSummary( protected Tag[] extraTags(Object object, Type bodyType, RequestTemplate template) { return EMPTY_TAGS_ARRAY; } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return !(encoder instanceof PredicatedEncoder) + || ((PredicatedEncoder) encoder).canEncode(object, bodyType, template); + } } diff --git a/moshi/src/main/java/feign/moshi/MoshiEncoder.java b/moshi/src/main/java/feign/moshi/MoshiEncoder.java index b65f705e27..1e7283cef4 100644 --- a/moshi/src/main/java/feign/moshi/MoshiEncoder.java +++ b/moshi/src/main/java/feign/moshi/MoshiEncoder.java @@ -18,11 +18,13 @@ import com.squareup.moshi.JsonAdapter; import com.squareup.moshi.Moshi; import feign.RequestTemplate; +import feign.Util; import feign.codec.Encoder; import feign.codec.JsonEncoder; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; -public class MoshiEncoder implements Encoder, JsonEncoder { +public class MoshiEncoder implements Encoder, PredicatedEncoder, JsonEncoder { private final Moshi moshi; @@ -43,4 +45,9 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { JsonAdapter jsonAdapter = moshi.adapter(bodyType).indent(" "); template.body(jsonAdapter.toJson(object)); } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isJsonContentType(template); + } } diff --git a/soap-jakarta/src/main/java/feign/soap/SOAPEncoder.java b/soap-jakarta/src/main/java/feign/soap/SOAPEncoder.java index 2b4a59cabb..860ab67aa4 100644 --- a/soap-jakarta/src/main/java/feign/soap/SOAPEncoder.java +++ b/soap-jakarta/src/main/java/feign/soap/SOAPEncoder.java @@ -16,8 +16,10 @@ package feign.soap; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import feign.jaxb.JAXBContextFactory; import jakarta.xml.bind.JAXBException; import jakarta.xml.bind.Marshaller; @@ -78,7 +80,7 @@ *

The JAXBContextFactory should be reused across requests as it caches the created JAXB * contexts. */ -public class SOAPEncoder implements Encoder { +public class SOAPEncoder implements Encoder, PredicatedEncoder { private static final String DEFAULT_SOAP_PROTOCOL = SOAPConstants.SOAP_1_1_PROTOCOL; @@ -220,4 +222,9 @@ public SOAPEncoder build() { return new SOAPEncoder(this); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isXmlContentType(template); + } } diff --git a/soap/src/main/java/feign/soap/SOAPEncoder.java b/soap/src/main/java/feign/soap/SOAPEncoder.java index d22d97fefa..a9bbe81e03 100644 --- a/soap/src/main/java/feign/soap/SOAPEncoder.java +++ b/soap/src/main/java/feign/soap/SOAPEncoder.java @@ -16,8 +16,10 @@ package feign.soap; import feign.RequestTemplate; +import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import feign.jaxb.JAXBContextFactory; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -82,7 +84,7 @@ *

The JAXBContextFactory should be reused across requests as it caches the created JAXB * contexts. */ -public class SOAPEncoder implements Encoder { +public class SOAPEncoder implements Encoder, PredicatedEncoder { private static final String DEFAULT_SOAP_PROTOCOL = SOAPConstants.SOAP_1_1_PROTOCOL; @@ -224,4 +226,9 @@ public SOAPEncoder build() { return new SOAPEncoder(this); } } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return Util.isXmlContentType(template); + } } diff --git a/src/docs/overview-mindmap.iuml b/src/docs/overview-mindmap.iuml index 50dd352f26..c1396675a6 100644 --- a/src/docs/overview-mindmap.iuml +++ b/src/docs/overview-mindmap.iuml @@ -1,64 +1,64 @@ -@startmindmap -* Feign -** clients -*** java.net.URL -*** Apache HTTP -*** Apache HC5 -*** Google HTTP -*** Java 11 Http2 -*** OK Http -*** Ribbon -** async clients -*** java.net.URL -*** Apache HC5 -*** OkHttp -*** Vertx -*** Reactive Wrappers -** contracts -*** Feign -*** JAX-RS -*** JAX-RS 2 -*** JAX-RS 3 / Jakarta -*** JAX-RS 4 -*** Spring -*** SOAP -*** SOAP Jakarta -*** Spring boot (3rd party) -** language -*** Kotlin -*** GraphQL - -left side - -** encoders/decoders -*** Multi encoder (predicate based, experimental) -*** GSON -*** JAXB -*** JAXB Jakarta -*** Jackson -*** Jackson 3 -*** Jackson JAXB -*** Jackson Jr -*** Sax -*** JSON-java -*** Moshi -*** Fastjson2 -*** Form -*** Form Spring -** metrics -*** Dropwizard Metrics 4 -*** Dropwizard Metrics 5 -*** Micrometer -** interceptors -*** RequestInterceptor -*** ResponseInterceptor -*** MethodInterceptor -**** Bean Validation (JSR-303) -**** Bean Validation (Jakarta) -**** HTTP Cache (ETag / Last-Modified) -** extras -*** Hystrix -*** SLF4J -*** Mock -*** Annotation Error Decoder -@endmindmap +@startmindmap +* Feign +** clients +*** java.net.URL +*** Apache HTTP +*** Apache HC5 +*** Google HTTP +*** Java 11 Http2 +*** OK Http +*** Ribbon +** async clients +*** java.net.URL +*** Apache HC5 +*** OkHttp +*** Vertx +*** Reactive Wrappers +** contracts +*** Feign +*** JAX-RS +*** JAX-RS 2 +*** JAX-RS 3 / Jakarta +*** JAX-RS 4 +*** Spring +*** SOAP +*** SOAP Jakarta +*** Spring boot (3rd party) +** language +*** Kotlin +*** GraphQL + +left side + +** encoders/decoders +*** Multi encoder (predicate based, experimental) +*** GSON +*** JAXB +*** JAXB Jakarta +*** Jackson +*** Jackson 3 +*** Jackson JAXB +*** Jackson Jr +*** Sax +*** JSON-java +*** Moshi +*** Fastjson2 +*** Form +*** Form Spring +** metrics +*** Dropwizard Metrics 4 +*** Dropwizard Metrics 5 +*** Micrometer +** interceptors +*** RequestInterceptor +*** ResponseInterceptor +*** MethodInterceptor +**** Bean Validation (JSR-303) +**** Bean Validation (Jakarta) +**** HTTP Cache (ETag / Last-Modified) +** extras +*** Hystrix +*** SLF4J +*** Mock +*** Annotation Error Decoder +@endmindmap From e012d67589c204d965eb6bb04220942d920e6479 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 20 Aug 2026 12:12:06 -0300 Subject: [PATCH 7/7] Drop the multi-encoder default encoder in favour of an explicit any() predicate Signed-off-by: Marvin Froeder --- CHANGELOG.md | 14 +- README.md | 89 ++++++++-- core/src/main/java/feign/BaseBuilder.java | 20 ++- .../java/feign/codec/EncoderPredicate.java | 73 ++++++-- .../main/java/feign/codec/MultiEncoder.java | 120 +++++++------ .../main/java/feign/codec/PairedEncoder.java | 77 +++++++++ .../java/feign/codec/PredicatedEncoder.java | 65 ++++++- .../feign/codec/EncoderPredicateTest.java | 33 ++++ .../codec/MultiEncoderCapabilityTest.java | 65 ++++++- .../java/feign/codec/MultiEncoderTest.java | 163 +++++++++++++----- .../feign/form/spring/SpringFormEncoder.java | 15 +- .../src/main/java/feign/form/FormEncoder.java | 54 +++++- .../feign/form/PredicatedFormEncoderTest.java | 104 +++++++++++ 13 files changed, 730 insertions(+), 162 deletions(-) create mode 100644 core/src/main/java/feign/codec/PairedEncoder.java create mode 100644 form/src/test/java/feign/form/PredicatedFormEncoderTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 804504da56..3a2b483532 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,12 +3,14 @@ * Add `@Experimental` `MultiEncoder`, `PredicatedEncoder` and `EncoderPredicate`, letting a single client route each request to the right encoder. Encoders declare what they can handle by implementing `PredicatedEncoder`; anything else is paired with a predicate via - `MultiEncoder.builder(defaultEncoder)`. The first-party JSON encoders (Gson, Jackson, Jackson 3, - Jackson Jr, Jackson JAXB, Moshi, Fastjson2, JSON-java) and XML encoders (JAXB, JAXB Jakarta, SOAP, - SOAP Jakarta) now declare themselves, and the metrics modules' `MeteredEncoder` forwards - `canEncode` to the encoder it wraps. The `Encoder` interface is unchanged, so existing encoders - keep working (#3485). - + `PredicatedEncoder.of(predicate, encoder)` or `MultiEncoder.builder()`. Encoders are consulted in + the order given and a request nothing accepts fails with an `EncodeException` naming what was + tried, so a default is an encoder guarded by `EncoderPredicate.any()` listed last. `FormEncoder` + and `SpringFormEncoder` gain `createPredicatedFormEncoder()`, a delegate-free flavour that can + take part. The first-party JSON encoders (Gson, Jackson, Jackson 3, Jackson Jr, Jackson JAXB, + Moshi, Fastjson2, JSON-java) and XML encoders (JAXB, JAXB Jakarta, SOAP, SOAP Jakarta) now declare + themselves, and the metrics modules' `MeteredEncoder` forwards `canEncode` to the encoder it + wraps. The `Encoder` interface is unchanged, so existing encoders keep working (#3485). * Add support for the HTTP QUERY method (RFC 10008) — safe, idempotent, and cacheable with a request body. `HttpCacheInterceptor` includes QUERY in its default cacheable set and incorporates a body hash into the cache key to reduce cross-body collisions. diff --git a/README.md b/README.md index 7925caa30a..037cbb3188 100644 --- a/README.md +++ b/README.md @@ -714,10 +714,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 speak more than one format — JSON for most endpoints, XML for -a legacy one, plain bytes for an upload. `MultiEncoder` routes each request to the right encoder, -falling back to a default when none applies. +a legacy one, plain bytes for an upload. `MultiEncoder` hands each request to the first encoder that +accepts it. -Most first-party encoders already declare what they can handle, so they can simply be added: +Most first-party encoders already declare what they can handle, so they can simply be listed, in the +order they should be consulted: ```java interface MixedClient { @@ -733,36 +734,56 @@ interface MixedClient { public class Example { public static void main(String[] args) { MixedClient client = Feign.builder() - .encoder(new DefaultEncoder(), new GsonEncoder(), new JAXBEncoder()) + .encoders(new GsonEncoder(), new JAXBEncoder()) .target(MixedClient.class, "https://foo.com"); } } ``` -The first argument is the default encoder, used when nothing else accepts the request. +There is no implicit fallback. A request that no encoder accepts fails with an `EncodeException` +naming the encoders that were tried and what each one wants: -For an encoder that does not declare itself — including one you do not control — pair it -with an `EncoderPredicate` using the builder: +``` +Unable to encode java.lang.String (Content-Type: text/plain) for POST /orders. Encoders tried, in order: + - GsonEncoder + - JAXBEncoder +Add an encoder guarded by EncoderPredicate.any() last to act as a default. +``` + +To get a default, pair an encoder with the predicate that accepts everything and list it **last**: + +```java +Feign.builder() + .encoders( + new GsonEncoder(), + new JAXBEncoder(), + PredicatedEncoder.of(EncoderPredicate.any(), new DefaultEncoder())); +``` + +The same pairing works for any encoder that does not declare itself, including one you do not +control. `MultiEncoder.builder()` spells it out when a lambda reads better than a wrapper: ```java Encoder encoder = - MultiEncoder.builder(new DefaultEncoder()) - .add(new GsonEncoder()) // declares itself - .add(EncoderPredicate.xmlContentType(), someXmlEncoder) // paired + MultiEncoder.builder() + .add(new GsonEncoder()) // declares itself + .add(EncoderPredicate.xmlContentType(), someXmlEncoder) // paired .add((object, bodyType, template) -> bodyType == byte[].class, binaryEncoder) + .add(EncoderPredicate.any(), new DefaultEncoder()) // the default, last .build(); ``` -Delegates are consulted in the order they were added, so put the narrowest predicate first. Note -that `Content-Type: application/json` with a null body is claimed by a JSON encoder before +Encoders are consulted in the order they were added, so put the narrowest one first. Note that +`Content-Type: application/json` with a null body is claimed by a JSON encoder before `EncoderPredicate.emptyBody()` gets a chance — order accordingly. ##### Declaring your own encoder -Implement `PredicatedEncoder` alongside `Encoder` and override `canEncode`: +Implement `PredicatedEncoder` and say what you handle. `canEncode` has no default: an encoder that +declares nothing would claim every request, which is rarely what its author meant. ```java -public class MyEncoder implements Encoder, PredicatedEncoder { +public class MyEncoder implements PredicatedEncoder { @Override public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { @@ -776,12 +797,42 @@ public class MyEncoder implements Encoder, PredicatedEncoder { } ``` -`EncoderPredicate` ships with `jsonContentType()`, `xmlContentType()`, `contentType(mediaType)`, -`emptyBody()`, `bodyType(type)` and `formEncoded()`, plus `and`/`or`/`negate` to combine them. +`EncoderPredicate` is the `@FunctionalInterface` here, so predicates can be lambdas. It ships with +`any()`, `jsonContentType()`, `xmlContentType()`, `contentType(mediaType)`, `emptyBody()`, +`bodyType(type)` and `formEncoded()`, 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 +`EncoderPredicate.describedAs("it is Tuesday", ...)` to read as well. + +`PredicatedEncoder.of(predicate, encoder)` replaces whatever the encoder says about itself, so it +can widen an encoder as well as narrow it. To keep the encoder's own declaration and add to it, use +`narrowing`: + +```java +// only this vendor content type, and only what Gson would have taken anyway +PredicatedEncoder.narrowing( + EncoderPredicate.contentType("application/vnd.acme+json"), new GsonEncoder()); +``` + +**If you wrap an encoder, forward `canEncode` to your delegate**, otherwise wrapping silently +changes what the encoder handles. The metrics modules' `MeteredEncoder` forwards for exactly this +reason. + +##### Form encoders + +`FormEncoder` and `SpringFormEncoder` wrap a delegate encoder, so they cannot honestly declare what +they handle — the delegate's applicability is unknown to them. Instead, each offers a +delegate-free flavour that does: + +```java +Feign.builder() + .encoders( + FormEncoder.createPredicatedFormEncoder(), // form and multipart requests only + new JacksonEncoder()); +``` -**If you wrap an encoder, forward `canEncode` to your delegate.** A wrapper that does not will claim -every request, because the default `canEncode` accepts everything. The metrics modules' -`MeteredEncoder` forwards for exactly this reason. +It accepts form and multipart requests carrying a map or a user pojo, and leaves everything else to +the encoders registered alongside it. Constructing one directly with a `null` delegate does the same +thing: anything it cannot encode itself fails with an `EncodeException` instead of being passed on. ### @Body templates The `@Body` annotation indicates a template to expand using parameters annotated with `@Param`. 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 12cbf9c3cb..b598888621 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -26,6 +26,7 @@ import feign.codec.DefaultEncoder; import feign.codec.DefaultErrorDecoder; import feign.codec.Encoder; +import feign.codec.EncoderPredicate; import feign.codec.ErrorDecoder; import feign.codec.MultiEncoder; import feign.codec.PredicatedEncoder; @@ -99,23 +100,28 @@ public B encoder(Encoder encoder) { /** * Configures a {@link MultiEncoder} built from encoders that declare their own applicability. * - *

Each {@link PredicatedEncoder} is consulted in the order given; {@code defaultEncoder} is - * the fallback used when none accepts the request. + *

Encoders are consulted in the order given, and the first one that accepts the request + * encodes it. There is no implicit fallback: pair an encoder with {@link EncoderPredicate#any()} + * and list it last to act as a default, otherwise a request nothing accepts fails with an {@link + * feign.codec.EncodeException}. * * 

    * Feign.builder()
-   *     .encoder(new DefaultEncoder(), new JacksonEncoder(), new JAXBEncoder())
+   *     .encoders(
+   *         new JacksonEncoder(),
+   *         new JAXBEncoder(),
+   *         PredicatedEncoder.of(EncoderPredicate.any(), new DefaultEncoder()))
    *
* *

To pair a predicate with an encoder that does not implement {@link PredicatedEncoder}, use - * {@link MultiEncoder#builder(Encoder)} instead. + * {@link PredicatedEncoder#of(EncoderPredicate, Encoder)} as above, or {@link + * MultiEncoder#builder()} for the same thing spelled out. * - * @param defaultEncoder the encoder used when no delegate accepts the request * @param encoders the predicated encoders, consulted in the order given */ @Experimental - public B encoder(Encoder defaultEncoder, PredicatedEncoder... encoders) { - MultiEncoder.Builder builder = MultiEncoder.builder(defaultEncoder); + public B encoders(PredicatedEncoder... encoders) { + MultiEncoder.Builder builder = MultiEncoder.builder(); for (PredicatedEncoder encoder : encoders) { builder.add(encoder); } diff --git a/core/src/main/java/feign/codec/EncoderPredicate.java b/core/src/main/java/feign/codec/EncoderPredicate.java index 1383dae47e..150fd22ded 100644 --- a/core/src/main/java/feign/codec/EncoderPredicate.java +++ b/core/src/main/java/feign/codec/EncoderPredicate.java @@ -28,6 +28,10 @@ * RequestTemplate)}, so they can discriminate on the body, on its declared type, or on anything * already present in the template such as the {@code Content-Type} header. * + *

Every predicate built here describes itself, so a {@link MultiEncoder} that cannot route a + * request can say what it did consider. Wrap your own lambdas in {@link #describedAs(String, + * EncoderPredicate)} to get the same in error messages. + * * @see PredicatedEncoder * @see MultiEncoder */ @@ -46,14 +50,49 @@ public interface EncoderPredicate { */ boolean canEncode(Object object, Type bodyType, RequestTemplate template); + /** + * Wraps a predicate so that it describes itself, which is what a {@link MultiEncoder} reports + * when no encoder accepts a request. + * + * @param description how the predicate reads in an error message, for example {@code + * "Content-Type is JSON"} + * @param predicate the predicate to describe + */ + static EncoderPredicate describedAs(String description, EncoderPredicate predicate) { + Objects.requireNonNull(description, "description cannot be null"); + Objects.requireNonNull(predicate, "predicate cannot be null"); + return new EncoderPredicate() { + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return predicate.canEncode(object, bodyType, template); + } + + @Override + public String toString() { + return description; + } + }; + } + + /** + * Matches every request. Pair this with an encoder registered last to make it the default of a + * {@link MultiEncoder}. + */ + static EncoderPredicate any() { + return describedAs("any request", (object, bodyType, template) -> true); + } + /** Matches requests whose {@code Content-Type} header denotes JSON. */ static EncoderPredicate jsonContentType() { - return (object, bodyType, template) -> Util.isJsonContentType(template); + return describedAs( + "Content-Type is JSON", (object, bodyType, template) -> Util.isJsonContentType(template)); } /** Matches requests whose {@code Content-Type} header denotes XML. */ static EncoderPredicate xmlContentType() { - return (object, bodyType, template) -> Util.isXmlContentType(template); + return describedAs( + "Content-Type is XML", (object, bodyType, template) -> Util.isXmlContentType(template)); } /** @@ -62,38 +101,50 @@ static EncoderPredicate xmlContentType() { */ static EncoderPredicate contentType(String mediaType) { Objects.requireNonNull(mediaType, "mediaType cannot be null"); - return (object, bodyType, template) -> Util.hasContentType(template, mediaType); + return describedAs( + "Content-Type is " + mediaType, + (object, bodyType, template) -> Util.hasContentType(template, mediaType)); } /** Matches requests carrying no body. */ static EncoderPredicate emptyBody() { - return (object, bodyType, template) -> object == null; + return describedAs("body is empty", (object, bodyType, template) -> object == null); } /** Matches requests whose declared body type is exactly the given type. */ static EncoderPredicate bodyType(Type type) { Objects.requireNonNull(type, "type cannot be null"); - return (object, bodyType, template) -> type.equals(bodyType); + return describedAs( + "body type is " + type.getTypeName(), + (object, bodyType, template) -> type.equals(bodyType)); } /** Matches form-encoded requests, as signalled by {@link Encoder#MAP_STRING_WILDCARD}. */ static EncoderPredicate formEncoded() { - return (object, bodyType, template) -> Encoder.MAP_STRING_WILDCARD.equals(bodyType); + return describedAs( + "body is form encoded", + (object, bodyType, template) -> Encoder.MAP_STRING_WILDCARD.equals(bodyType)); } default EncoderPredicate and(EncoderPredicate other) { Objects.requireNonNull(other, "other cannot be null"); - return (object, bodyType, template) -> - canEncode(object, bodyType, template) && other.canEncode(object, bodyType, template); + return describedAs( + "(" + this + " and " + other + ")", + (object, bodyType, template) -> + canEncode(object, bodyType, template) && other.canEncode(object, bodyType, template)); } default EncoderPredicate or(EncoderPredicate other) { Objects.requireNonNull(other, "other cannot be null"); - return (object, bodyType, template) -> - canEncode(object, bodyType, template) || other.canEncode(object, bodyType, template); + return describedAs( + "(" + this + " or " + other + ")", + (object, bodyType, template) -> + canEncode(object, bodyType, template) || other.canEncode(object, bodyType, template)); } default EncoderPredicate negate() { - return (object, bodyType, template) -> !canEncode(object, bodyType, template); + return describedAs( + "not (" + this + ")", + (object, bodyType, template) -> !canEncode(object, bodyType, template)); } } diff --git a/core/src/main/java/feign/codec/MultiEncoder.java b/core/src/main/java/feign/codec/MultiEncoder.java index c2ab15e8e5..23feaf35ba 100644 --- a/core/src/main/java/feign/codec/MultiEncoder.java +++ b/core/src/main/java/feign/codec/MultiEncoder.java @@ -17,32 +17,38 @@ import feign.Experimental; import feign.RequestTemplate; +import feign.Util; 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; /** - * An {@link Encoder} that selects a delegate per request, falling back to a default encoder when no - * delegate accepts it. + * An {@link Encoder} that hands each request to the first encoder that accepts it. * - *

Delegates come from two places. An encoder that implements {@link PredicatedEncoder} declares + *

Encoders come from two places. An encoder that implements {@link PredicatedEncoder} declares * its own applicability and can simply be added; any other encoder is paired with an {@link * EncoderPredicate} at the call site: * * 

  * Feign.builder()
  *     .encoder(
- *         MultiEncoder.builder(new DefaultEncoder())
+ *         MultiEncoder.builder()
  *             .add(new JacksonEncoder())
  *             .add(EncoderPredicate.xmlContentType(), new JAXBEncoder())
  *             .add((object, bodyType, template) -> bodyType == byte[].class, new BinaryEncoder())
+ *             .add(EncoderPredicate.any(), new DefaultEncoder())
  *             .build());
  *
* - *

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

Encoders are consulted in the order they were added, so the narrowest one comes first. There + * is no implicit fallback: a request no encoder accepts fails with an {@link EncodeException} + * naming what was tried. Add an encoder guarded by {@link EncoderPredicate#any()} last to act as a + * default, as above. * * @see PredicatedEncoder * @see EncoderPredicate @@ -50,76 +56,83 @@ @Experimental public class MultiEncoder implements Encoder { - private final Encoder defaultEncoder; + private final List encoders; - private final List delegates; - - private MultiEncoder(Encoder defaultEncoder, List delegates) { - this.defaultEncoder = defaultEncoder; - this.delegates = Collections.unmodifiableList(new ArrayList<>(delegates)); + private MultiEncoder(List encoders) { + this.encoders = Collections.unmodifiableList(new ArrayList<>(encoders)); } - /** - * Starts building a multi-encoder. - * - * @param defaultEncoder the encoder used when no delegate accepts the request - * @return the builder - */ - public static Builder builder(Encoder defaultEncoder) { - return new Builder(defaultEncoder); + /** Starts building a multi-encoder. */ + public static Builder builder() { + return new Builder(); } /** - * Encodes using the first delegate that accepts the request, or the default encoder if none do. + * Encodes using the first encoder that accepts the request. * * @param object {@inheritDoc} * @param bodyType {@inheritDoc} * @param template {@inheritDoc} - * @throws EncodeException {@inheritDoc} + * @throws EncodeException when no encoder accepts the request, or the chosen one fails */ @Override public void encode(Object object, Type bodyType, RequestTemplate template) throws EncodeException { - for (Delegate delegate : delegates) { - if (delegate.predicate.canEncode(object, bodyType, template)) { - delegate.encoder.encode(object, bodyType, template); + for (PredicatedEncoder encoder : encoders) { + if (encoder.canEncode(object, bodyType, template)) { + encoder.encode(object, bodyType, template); return; } } - defaultEncoder.encode(object, bodyType, template); + throw new EncodeException(unableToEncode(bodyType, template)); } - @Override - public String toString() { - return "MultiEncoder{defaultEncoder=" + defaultEncoder + ", delegates=" + delegates + '}'; + private String unableToEncode(Type bodyType, RequestTemplate template) { + StringBuilder message = + new StringBuilder("Unable to encode ") + .append(bodyType == null ? "request body" : bodyType.getTypeName()) + .append(" (Content-Type: ") + .append(contentTypes(template)) + .append(')'); + if (template.method() != null) { + message.append(" for ").append(template.method()).append(' ').append(template.path()); + } + if (encoders.isEmpty()) { + return message.append(". No encoders were configured.").toString(); + } + message.append(". Encoders tried, in order:"); + for (PredicatedEncoder encoder : encoders) { + message.append("\n - ").append(PairedEncoder.describe(encoder)); + } + return message + .append("\nAdd an encoder guarded by EncoderPredicate.any() last to act as a default.") + .toString(); } - private static final class Delegate { - private final EncoderPredicate predicate; - private final Encoder encoder; - - Delegate(EncoderPredicate predicate, Encoder encoder) { - this.predicate = predicate; - this.encoder = encoder; - } + private static String contentTypes(RequestTemplate template) { + String contentTypes = + template.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 encoder.toString(); - } + @Override + public String toString() { + return "MultiEncoder" + + encoders.stream().map(PairedEncoder::describe).collect(Collectors.toList()); } - /** Collects the delegates of a {@link MultiEncoder}. */ + /** Collects the encoders of a {@link MultiEncoder}. */ @Experimental public static final class Builder { - private final Encoder defaultEncoder; + private final List encoders = new ArrayList<>(); - private final List delegates = new ArrayList<>(); - - private Builder(Encoder defaultEncoder) { - this.defaultEncoder = Objects.requireNonNull(defaultEncoder, "defaultEncoder cannot be null"); - } + private Builder() {} /** * Adds an encoder that declares its own applicability. @@ -127,8 +140,8 @@ private Builder(Encoder defaultEncoder) { * @param encoder the encoder, consulted via {@link PredicatedEncoder#canEncode} */ public Builder add(PredicatedEncoder encoder) { - Objects.requireNonNull(encoder, "encoder cannot be null"); - return add(encoder::canEncode, encoder); + encoders.add(Objects.requireNonNull(encoder, "encoder cannot be null")); + return this; } /** @@ -139,15 +152,12 @@ public Builder add(PredicatedEncoder encoder) { * @param encoder the encoder to delegate to */ public Builder add(EncoderPredicate predicate, Encoder encoder) { - Objects.requireNonNull(predicate, "predicate cannot be null"); - Objects.requireNonNull(encoder, "encoder cannot be null"); - delegates.add(new Delegate(predicate, encoder)); - return this; + return add(PredicatedEncoder.of(predicate, encoder)); } /** Builds the multi-encoder. */ public MultiEncoder build() { - return new MultiEncoder(defaultEncoder, delegates); + return new MultiEncoder(encoders); } } } diff --git a/core/src/main/java/feign/codec/PairedEncoder.java b/core/src/main/java/feign/codec/PairedEncoder.java new file mode 100644 index 0000000000..61a63f7636 --- /dev/null +++ b/core/src/main/java/feign/codec/PairedEncoder.java @@ -0,0 +1,77 @@ +/* + * 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.RequestTemplate; +import java.lang.reflect.Type; +import java.util.Objects; + +/** An encoder that does not declare itself, guarded by a predicate supplied at the call site. */ +final class PairedEncoder implements PredicatedEncoder { + + private final EncoderPredicate predicate; + + private final Encoder encoder; + + PairedEncoder(EncoderPredicate predicate, Encoder encoder) { + this.predicate = Objects.requireNonNull(predicate, "predicate cannot be null"); + this.encoder = Objects.requireNonNull(encoder, "encoder cannot be null"); + } + + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return predicate.canEncode(object, bodyType, template); + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) + throws EncodeException { + encoder.encode(object, bodyType, template); + } + + @Override + public String toString() { + return describe(encoder) + " when " + predicate; + } + + /** Requires both the predicate and, when the encoder declares one, its own applicability. */ + static EncoderPredicate narrow(EncoderPredicate predicate, Encoder encoder) { + Objects.requireNonNull(predicate, "predicate cannot be null"); + Objects.requireNonNull(encoder, "encoder cannot be null"); + if (!(encoder instanceof PredicatedEncoder)) { + return predicate; + } + if (encoder instanceof PairedEncoder) { + return predicate.and(((PairedEncoder) encoder).predicate); + } + PredicatedEncoder predicated = (PredicatedEncoder) encoder; + return predicate.and( + EncoderPredicate.describedAs(describe(encoder) + " accepts it", predicated::canEncode)); + } + + /** The encoder's own {@code toString} when it has one, its class name otherwise. */ + static String describe(Encoder encoder) { + Class type = encoder.getClass(); + try { + if (type.getMethod("toString").getDeclaringClass() != Object.class) { + return encoder.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/PredicatedEncoder.java b/core/src/main/java/feign/codec/PredicatedEncoder.java index c97cfd072f..f9cd135acf 100644 --- a/core/src/main/java/feign/codec/PredicatedEncoder.java +++ b/core/src/main/java/feign/codec/PredicatedEncoder.java @@ -26,17 +26,25 @@ * route each request to the right one without the call site having to wrap anything: * * 

- * public class JacksonEncoder implements Encoder, PredicatedEncoder {
+ * public class JacksonEncoder implements PredicatedEncoder {
  *
  *   @Override
  *   public boolean canEncode(Object object, Type bodyType, RequestTemplate template) {
- *     return EncoderPredicate.jsonContentType().canEncode(object, bodyType, template);
+ *     return Util.isJsonContentType(template);
+ *   }
+ *
+ *   @Override
+ *   public void encode(Object object, Type bodyType, RequestTemplate template) {
+ *     // ...
  *   }
  * }
  *
* - *

{@link Encoder#encode(Object, Type, RequestTemplate) encode} remains the only abstract method, - * so this stays a functional interface and a bare lambda is an encoder that accepts everything. + *

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

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

An encoder paired with {@link EncoderPredicate#any()} accepts everything, which is how a + * {@link MultiEncoder} is given a default: + * + * 

+   * Feign.builder()
+   *     .encoders(
+   *         new JacksonEncoder(),
+   *         PredicatedEncoder.of(EncoderPredicate.any(), new Encoder.Default()));
+   *
+ * + * @param predicate decides whether the encoder handles a request + * @param encoder the encoder to delegate to + */ + static PredicatedEncoder of(EncoderPredicate predicate, Encoder encoder) { + return new PairedEncoder(predicate, encoder); + } + + /** + * Narrows an encoder that already declares itself, by requiring both the given predicate and the + * encoder's own {@code canEncode} to accept the request: + * + * 
+   * PredicatedEncoder.narrowing(
+   *     EncoderPredicate.contentType("application/vnd.acme+json"), new GsonEncoder());
+   *
+ * + *

An encoder that does not implement {@link PredicatedEncoder} declares nothing to narrow, so + * this behaves like {@link #of(EncoderPredicate, Encoder)}. + * + * @param predicate narrows what the encoder handles + * @param encoder the encoder to delegate to + */ + static PredicatedEncoder narrowing(EncoderPredicate predicate, Encoder encoder) { + return new PairedEncoder(PairedEncoder.narrow(predicate, encoder), encoder); + } + + /** + * Whether this encoder can handle the request. * * @param object what to encode as the request body * @param bodyType the type the object should be encoded as. {@link Encoder#MAP_STRING_WILDCARD} @@ -57,7 +106,5 @@ public interface PredicatedEncoder extends Encoder { * @param template the request template to populate * @return {@code true} if this encoder can encode the request, {@code false} otherwise */ - default boolean canEncode(Object object, Type bodyType, RequestTemplate template) { - return true; - } + boolean canEncode(Object object, Type bodyType, RequestTemplate template); } diff --git a/core/src/test/java/feign/codec/EncoderPredicateTest.java b/core/src/test/java/feign/codec/EncoderPredicateTest.java index 899085d9fc..5028567d6a 100644 --- a/core/src/test/java/feign/codec/EncoderPredicateTest.java +++ b/core/src/test/java/feign/codec/EncoderPredicateTest.java @@ -34,6 +34,15 @@ private static boolean test(EncoderPredicate predicate, String contentType) { return predicate.canEncode("body", String.class, template(contentType)); } + @Test + void anyMatchesEverything() { + EncoderPredicate any = EncoderPredicate.any(); + + assertThat(test(any, "application/json")).isTrue(); + assertThat(test(any, null)).isTrue(); + assertThat(any.canEncode(null, null, template(null))).isTrue(); + } + @Test void jsonContentTypeMatchesJsonOnly() { EncoderPredicate json = EncoderPredicate.jsonContentType(); @@ -101,6 +110,30 @@ void formEncodedMatchesTheFormBodyTypeMarker() { assertThat(form.canEncode("body", String.class, template(null))).isFalse(); } + @Test + void predicatesDescribeThemselves() { + assertThat(EncoderPredicate.any()).hasToString("any request"); + assertThat(EncoderPredicate.jsonContentType()).hasToString("Content-Type is JSON"); + assertThat(EncoderPredicate.xmlContentType()).hasToString("Content-Type is XML"); + assertThat(EncoderPredicate.contentType("text/plain")) + .hasToString("Content-Type is text/plain"); + assertThat(EncoderPredicate.emptyBody()).hasToString("body is empty"); + assertThat(EncoderPredicate.bodyType(byte[].class)).hasToString("body type is byte[]"); + assertThat(EncoderPredicate.formEncoded()).hasToString("body is form encoded"); + assertThat(EncoderPredicate.describedAs("it is Tuesday", (o, b, t) -> true)) + .hasToString("it is Tuesday"); + } + + @Test + void combinedPredicatesDescribeThemselves() { + EncoderPredicate json = EncoderPredicate.jsonContentType(); + EncoderPredicate xml = EncoderPredicate.xmlContentType(); + + assertThat(json.or(xml)).hasToString("(Content-Type is JSON or Content-Type is XML)"); + assertThat(json.and(xml)).hasToString("(Content-Type is JSON and Content-Type is XML)"); + assertThat(json.negate()).hasToString("not (Content-Type is JSON)"); + } + @Test void combinators() { EncoderPredicate json = EncoderPredicate.jsonContentType(); diff --git a/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java b/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java index 8432acaea2..b72fe9cdc9 100644 --- a/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.java +++ b/core/src/test/java/feign/codec/MultiEncoderCapabilityTest.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; @@ -29,7 +30,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; -/** How {@link MultiEncoder} behaves when a {@link Capability} wraps the configured encoder. */ +/** How {@link MultiEncoder} behaves once configured on a {@link Feign} builder. */ class MultiEncoderCapabilityTest { interface MixedApi { @@ -92,6 +93,42 @@ private static RequestTemplate template(String contentType) { return template; } + @Test + void encodersOnTheBuilderRouteInTheOrderGiven() { + AtomicReference captured = new AtomicReference<>(); + + MixedApi api = + target( + Feign.builder() + .encoders( + PredicatedEncoder.of( + EncoderPredicate.jsonContentType(), new TaggingEncoder("json")), + PredicatedEncoder.of(EncoderPredicate.any(), new TaggingEncoder("fallback"))), + captured); + + api.json("{}"); + assertThat(captured.get()).isEqualTo("json"); + + api.xml(""); + assertThat(captured.get()).isEqualTo("fallback"); + } + + @Test + void encodersOnTheBuilderFailWhenNothingAccepts() { + MixedApi api = + target( + Feign.builder() + .encoders( + PredicatedEncoder.of( + EncoderPredicate.jsonContentType(), new TaggingEncoder("json"))), + new AtomicReference<>()); + + assertThatThrownBy(() -> api.xml("")) + .isInstanceOf(EncodeException.class) + .hasMessageContaining("Unable to encode java.lang.String (Content-Type: application/xml)") + .hasMessageContaining("TaggingEncoder when Content-Type is JSON"); + } + @Test void capabilityWrapsTheCompositeAndRoutingStillWorks() { CountingCapability capability = new CountingCapability(); @@ -101,9 +138,10 @@ void capabilityWrapsTheCompositeAndRoutingStillWorks() { target( Feign.builder() .encoder( - MultiEncoder.builder(new TaggingEncoder("fallback")) + MultiEncoder.builder() .add(EncoderPredicate.jsonContentType(), new TaggingEncoder("json")) .add(EncoderPredicate.xmlContentType(), new TaggingEncoder("xml")) + .add(EncoderPredicate.any(), new TaggingEncoder("fallback")) .build()) .addCapability(capability), captured); @@ -120,8 +158,8 @@ void capabilityWrapsTheCompositeAndRoutingStillWorks() { } /** - * A wrapper that does not forward {@code canEncode} claims every request, which is why the - * metrics modules' {@code MeteredEncoder} forwards it to its delegate. + * A wrapper that answers {@code canEncode} for itself instead of forwarding claims every request, + * which is why the metrics modules' {@code MeteredEncoder} forwards it to its delegate. */ @Test void wrappingWithoutForwardingCanEncodeErasesSelfDeclaration() { @@ -138,7 +176,18 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { } }; - PredicatedEncoder naive = jsonOnly::encode; + PredicatedEncoder naive = + new PredicatedEncoder() { + @Override + public boolean canEncode(Object object, Type bodyType, RequestTemplate template) { + return true; + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + jsonOnly.encode(object, bodyType, template); + } + }; PredicatedEncoder forwarding = new PredicatedEncoder() { @@ -154,15 +203,17 @@ public void encode(Object object, Type bodyType, RequestTemplate template) { }; RequestTemplate naiveTemplate = template("application/xml"); - MultiEncoder.builder(new TaggingEncoder("fallback")) + MultiEncoder.builder() .add(naive) + .add(EncoderPredicate.any(), new TaggingEncoder("fallback")) .build() .encode("body", String.class, naiveTemplate); assertThat(naiveTemplate.requestBody().asString()).isEqualTo("json"); RequestTemplate forwardedTemplate = template("application/xml"); - MultiEncoder.builder(new TaggingEncoder("fallback")) + MultiEncoder.builder() .add(forwarding) + .add(EncoderPredicate.any(), new TaggingEncoder("fallback")) .build() .encode("body", String.class, forwardedTemplate); assertThat(forwardedTemplate.requestBody().asString()).isEqualTo("fallback"); diff --git a/core/src/test/java/feign/codec/MultiEncoderTest.java b/core/src/test/java/feign/codec/MultiEncoderTest.java index c00021ad4c..88914e77ec 100644 --- a/core/src/test/java/feign/codec/MultiEncoderTest.java +++ b/core/src/test/java/feign/codec/MultiEncoderTest.java @@ -69,7 +69,8 @@ void routesToTheEncoderThatDeclaresItCanHandleTheRequest() { SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); + Encoder encoder = + MultiEncoder.builder().add(json).add(EncoderPredicate.any(), fallback).build(); RequestTemplate template = templateWithContentType("application/json"); encoder.encode("body", String.class, template); @@ -85,7 +86,10 @@ void pairsAPredicateWithAnEncoderThatDoesNotDeclareItself() { RecordingEncoder fallback = new RecordingEncoder("fallback"); Encoder encoder = - MultiEncoder.builder(fallback).add(EncoderPredicate.xmlContentType(), xml).build(); + MultiEncoder.builder() + .add(EncoderPredicate.xmlContentType(), xml) + .add(EncoderPredicate.any(), fallback) + .build(); encoder.encode("body", String.class, templateWithContentType("application/xml")); @@ -101,10 +105,11 @@ void mixesSelfDeclaringEncodersAndPairs() { RecordingEncoder fallback = new RecordingEncoder("fallback"); Encoder encoder = - MultiEncoder.builder(fallback) + MultiEncoder.builder() .add(json) .add(EncoderPredicate.xmlContentType(), xml) .add(EncoderPredicate.bodyType(byte[].class), binary) + .add(EncoderPredicate.any(), fallback) .build(); encoder.encode( @@ -119,9 +124,8 @@ void mixesSelfDeclaringEncodersAndPairs() { @Test void matchesSuffixedContentTypes() { SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); - RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); + Encoder encoder = MultiEncoder.builder().add(json).build(); encoder.encode("body", String.class, templateWithContentType("application/vnd.github+json")); @@ -129,11 +133,12 @@ void matchesSuffixedContentTypes() { } @Test - void fallsBackWhenNoDelegateAccepts() { + void fallsBackToTheEncoderThatAcceptsAnything() { SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); + Encoder encoder = + MultiEncoder.builder().add(json).add(EncoderPredicate.any(), fallback).build(); RequestTemplate template = templateWithContentType("text/plain"); encoder.encode("body", String.class, template); @@ -148,7 +153,8 @@ void fallsBackWhenNoContentTypeIsSet() { SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); RecordingEncoder fallback = new RecordingEncoder("fallback"); - Encoder encoder = MultiEncoder.builder(fallback).add(json).build(); + Encoder encoder = + MultiEncoder.builder().add(json).add(EncoderPredicate.any(), fallback).build(); encoder.encode("body", String.class, templateWithContentType(null)); @@ -156,24 +162,12 @@ void fallsBackWhenNoContentTypeIsSet() { } @Test - void withNoDelegatesEverythingGoesToTheDefaultEncoder() { - RecordingEncoder fallback = new RecordingEncoder("fallback"); - - Encoder encoder = MultiEncoder.builder(fallback).build(); - - encoder.encode("body", String.class, templateWithContentType("application/json")); - - assertThat(fallback.invoked).isTrue(); - } - - @Test - void delegatesAreConsultedInOrder() { + void encodersAreConsultedInOrder() { RecordingEncoder first = new RecordingEncoder("first"); RecordingEncoder second = new RecordingEncoder("second"); - RecordingEncoder fallback = new RecordingEncoder("fallback"); Encoder encoder = - MultiEncoder.builder(fallback) + MultiEncoder.builder() .add(EncoderPredicate.jsonContentType(), first) .add(EncoderPredicate.jsonContentType(), second) .build(); @@ -185,18 +179,47 @@ void delegatesAreConsultedInOrder() { } @Test - void anEncoderWithoutAPredicateAcceptsEverything() { - // a bare lambda is a PredicatedEncoder whose default canEncode returns true - RecordingEncoder fallback = new RecordingEncoder("fallback"); - PredicatedEncoder greedy = (object, bodyType, template) -> template.body("greedy"); + void pairingReplacesWhatTheEncoderDeclaresAboutItself() { + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); - Encoder encoder = MultiEncoder.builder(fallback).add(greedy).build(); + Encoder encoder = + MultiEncoder.builder().add(PredicatedEncoder.of(EncoderPredicate.any(), json)).build(); - RequestTemplate template = templateWithContentType("text/plain"); - encoder.encode("body", String.class, template); + encoder.encode("body", String.class, templateWithContentType("text/plain")); - assertThat(fallback.invoked).isFalse(); - assertThat(template.requestBody().asString()).isEqualTo("greedy"); + assertThat(json.invoked).isTrue(); + } + + @Test + void narrowingKeepsWhatTheEncoderDeclaresAboutItself() { + SelfDeclaringJsonEncoder json = new SelfDeclaringJsonEncoder(); + PredicatedEncoder narrowed = + PredicatedEncoder.narrowing( + EncoderPredicate.contentType("application/vnd.acme+json"), json); + + assertThat( + narrowed.canEncode("body", String.class, templateWithContentType("application/json"))) + .isFalse(); + assertThat( + narrowed.canEncode( + "body", String.class, templateWithContentType("application/vnd.acme+json"))) + .isTrue(); + assertThat(narrowed) + .hasToString( + "SelfDeclaringJsonEncoder when (Content-Type is application/vnd.acme+json" + + " and SelfDeclaringJsonEncoder accepts it)"); + } + + @Test + void narrowingAnEncoderThatDeclaresNothingIsJustThePredicate() { + RecordingEncoder plain = new RecordingEncoder("plain"); + PredicatedEncoder narrowed = + PredicatedEncoder.narrowing(EncoderPredicate.jsonContentType(), plain); + + assertThat(narrowed).hasToString("RecordingEncoder when Content-Type is JSON"); + assertThat( + narrowed.canEncode("body", String.class, templateWithContentType("application/json"))) + .isTrue(); } @Test @@ -207,9 +230,7 @@ void propagatesEncodeExceptionFromDelegate() { }; Encoder encoder = - MultiEncoder.builder(new DefaultEncoder()) - .add(EncoderPredicate.jsonContentType(), failing) - .build(); + MultiEncoder.builder().add(EncoderPredicate.jsonContentType(), failing).build(); assertThatThrownBy( () -> encoder.encode("body", String.class, templateWithContentType("application/json"))) @@ -217,27 +238,83 @@ void propagatesEncodeExceptionFromDelegate() { .hasMessage("boom"); } + @Test + void throwsWhenNoEncoderAcceptsTheRequest() { + Encoder encoder = + MultiEncoder.builder() + .add(new SelfDeclaringJsonEncoder()) + .add(EncoderPredicate.xmlContentType(), new RecordingEncoder("xml")) + .build(); + + assertThatThrownBy( + () -> encoder.encode("body", String.class, templateWithContentType("text/plain"))) + .isInstanceOf(EncodeException.class) + .hasMessage( + "Unable to encode java.lang.String (Content-Type: text/plain)." + + " Encoders tried, in order:" + + "\n - SelfDeclaringJsonEncoder" + + "\n - RecordingEncoder when Content-Type is XML" + + "\nAdd an encoder guarded by EncoderPredicate.any() last to act as a default."); + } + + @Test + void theFailureNamesTheRequestWhenTheTemplateHasOne() { + RequestTemplate template = templateWithContentType("text/plain"); + template.method(Request.HttpMethod.POST); + template.uri("/orders"); + + Encoder encoder = MultiEncoder.builder().add(new SelfDeclaringJsonEncoder()).build(); + + assertThatThrownBy(() -> encoder.encode("body", String.class, template)) + .isInstanceOf(EncodeException.class) + .hasMessageContaining( + "Unable to encode java.lang.String (Content-Type: text/plain) for POST /orders."); + } + + @Test + void theFailureReportsAMissingContentType() { + Encoder encoder = MultiEncoder.builder().add(new SelfDeclaringJsonEncoder()).build(); + + assertThatThrownBy(() -> encoder.encode("body", String.class, templateWithContentType(null))) + .isInstanceOf(EncodeException.class) + .hasMessageContaining("(Content-Type: not set)"); + } + + @Test + void throwsWhenNoEncodersAreConfigured() { + Encoder encoder = MultiEncoder.builder().build(); + + assertThatThrownBy( + () -> encoder.encode("body", String.class, templateWithContentType("application/json"))) + .isInstanceOf(EncodeException.class) + .hasMessage( + "Unable to encode java.lang.String (Content-Type: application/json)." + + " No encoders were configured."); + } + @Test void rejectsNullArguments() { - assertThatThrownBy(() -> MultiEncoder.builder(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("defaultEncoder cannot be null"); - assertThatThrownBy(() -> MultiEncoder.builder(new DefaultEncoder()).add(null)) + assertThatThrownBy(() -> MultiEncoder.builder().add(null)) .isInstanceOf(NullPointerException.class) .hasMessage("encoder cannot be null"); - assertThatThrownBy( - () -> MultiEncoder.builder(new DefaultEncoder()).add(null, new DefaultEncoder())) + assertThatThrownBy(() -> MultiEncoder.builder().add(null, new DefaultEncoder())) .isInstanceOf(NullPointerException.class) .hasMessage("predicate cannot be null"); + assertThatThrownBy(() -> MultiEncoder.builder().add(EncoderPredicate.any(), null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("encoder cannot be null"); } @Test - void toStringDescribesDelegates() { + void toStringDescribesEncoders() { Encoder encoder = - MultiEncoder.builder(new DefaultEncoder()) + MultiEncoder.builder() + .add(new SelfDeclaringJsonEncoder()) .add(EncoderPredicate.jsonContentType(), new RecordingEncoder("json")) .build(); - assertThat(encoder.toString()).startsWith("MultiEncoder{defaultEncoder="); + assertThat(encoder.toString()) + .isEqualTo( + "MultiEncoder[SelfDeclaringJsonEncoder, RecordingEncoder when Content-Type is JSON]"); } } diff --git a/form-spring/src/main/java/feign/form/spring/SpringFormEncoder.java b/form-spring/src/main/java/feign/form/spring/SpringFormEncoder.java index 67c9bd2177..a26600838a 100644 --- a/form-spring/src/main/java/feign/form/spring/SpringFormEncoder.java +++ b/form-spring/src/main/java/feign/form/spring/SpringFormEncoder.java @@ -22,6 +22,7 @@ import feign.codec.DefaultEncoder; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.PredicatedEncoder; import feign.form.FormEncoder; import feign.form.MultipartFormContentProcessor; import java.lang.reflect.Type; @@ -42,10 +43,22 @@ public SpringFormEncoder() { this(new DefaultEncoder()); } + /** + * Creates a Spring form encoder that declares what it can handle, for use with {@code + * MultiEncoder}. It has no delegate, so a request it does not accept is left for the other + * encoders registered alongside it. + * + * @return a Spring form encoder guarded by {@link FormEncoder#formRequests()} + */ + public static PredicatedEncoder createPredicatedFormEncoder() { + return PredicatedEncoder.of(FormEncoder.formRequests(), new SpringFormEncoder(null)); + } + /** * Constructor with specified delegate encoder. * - * @param delegate delegate encoder, if this encoder couldn't encode object. + * @param delegate delegate encoder, if this encoder couldn't encode object. {@code null} leaves + * this encoder without one, see {@link FormEncoder#FormEncoder(Encoder)}. */ public SpringFormEncoder(Encoder delegate) { super(delegate); diff --git a/form/src/main/java/feign/form/FormEncoder.java b/form/src/main/java/feign/form/FormEncoder.java index fb05cde7cd..3d2ad676a7 100644 --- a/form/src/main/java/feign/form/FormEncoder.java +++ b/form/src/main/java/feign/form/FormEncoder.java @@ -25,6 +25,8 @@ import feign.codec.DefaultEncoder; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.codec.EncoderPredicate; +import feign.codec.PredicatedEncoder; import java.lang.reflect.Type; import java.nio.charset.Charset; import java.nio.charset.IllegalCharsetNameException; @@ -48,6 +50,16 @@ public class FormEncoder implements Encoder { private static final Pattern CHARSET_PATTERN; + /** Stands in for a delegate that was never supplied, see {@link #FormEncoder(Encoder)}. */ + private static final Encoder NO_DELEGATE = + (object, bodyType, template) -> { + throw new EncodeException( + "This form encoder has no delegate encoder, so it can only encode form and multipart" + + " requests, and " + + bodyType + + " is neither. Register an encoder that handles it."); + }; + static { CONTENT_TYPE_HEADER = "Content-Type"; CHARSET_PATTERN = Pattern.compile("(?<=charset=)([\\w\\-]+)"); @@ -65,13 +77,16 @@ public FormEncoder() { /** * Constructor with specified delegate encoder. * - * @param delegate delegate encoder, if this encoder couldn't encode object. + * @param delegate delegate encoder, if this encoder couldn't encode object. {@code null} leaves + * this encoder without one, in which case anything it cannot encode itself fails with an + * {@link EncodeException} rather than being passed on. */ public FormEncoder(Encoder delegate) { - this.delegate = delegate; + this.delegate = delegate == null ? NO_DELEGATE : delegate; val list = - asList(new MultipartFormContentProcessor(delegate), new UrlencodedFormContentProcessor()); + asList( + new MultipartFormContentProcessor(this.delegate), new UrlencodedFormContentProcessor()); processors = new HashMap(list.size(), 1.F); for (ContentProcessor processor : list) { @@ -79,6 +94,37 @@ public FormEncoder(Encoder delegate) { } } + /** + * Creates a form encoder that declares what it can handle, for use with {@code MultiEncoder}. + * + *

It has no delegate: a request it does not accept is left for the other encoders registered + * alongside it, instead of being swallowed by a fallback of its own. + * + * 

+   * Feign.builder()
+   *     .encoders(FormEncoder.createPredicatedFormEncoder(), new JacksonEncoder());
+   *
+ * + * @return a form encoder guarded by {@link #formRequests()} + */ + public static PredicatedEncoder createPredicatedFormEncoder() { + return PredicatedEncoder.of(formRequests(), new FormEncoder(null)); + } + + /** + * The requests a delegate-less form encoder can handle: a form or multipart {@code Content-Type}, + * carrying a body this encoder knows how to turn into fields. + * + * @return the predicate + */ + public static EncoderPredicate formRequests() { + return EncoderPredicate.describedAs( + "Content-Type is a form type and the body is a map or a user pojo", + (object, bodyType, template) -> + ContentType.of(getContentTypeValue(template.headers())) != ContentType.UNDEFINED + && (object instanceof Map || (bodyType != null && isUserPojo(bodyType)))); + } + @Override @SuppressWarnings("unchecked") public void encode(Object object, Type bodyType, RequestTemplate template) @@ -115,7 +161,7 @@ public final ContentProcessor getContentProcessor(ContentType type) { } @SuppressWarnings("PMD.AvoidBranchingStatementAsLastInLoop") - private String getContentTypeValue(Map> headers) { + private static String getContentTypeValue(Map> headers) { for (val entry : headers.entrySet()) { if (!entry.getKey().equalsIgnoreCase(CONTENT_TYPE_HEADER)) { continue; diff --git a/form/src/test/java/feign/form/PredicatedFormEncoderTest.java b/form/src/test/java/feign/form/PredicatedFormEncoderTest.java new file mode 100644 index 0000000000..449202a8da --- /dev/null +++ b/form/src/test/java/feign/form/PredicatedFormEncoderTest.java @@ -0,0 +1,104 @@ +/* + * 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.form; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import feign.RequestTemplate; +import feign.codec.EncodeException; +import feign.codec.Encoder; +import feign.codec.EncoderPredicate; +import feign.codec.MultiEncoder; +import feign.codec.PredicatedEncoder; +import java.nio.charset.StandardCharsets; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class PredicatedFormEncoderTest { + + private static RequestTemplate template(String contentType) { + RequestTemplate template = new RequestTemplate(); + if (contentType != null) { + template.header("Content-Type", contentType); + } + return template; + } + + private static Map data() { + Map data = new LinkedHashMap<>(); + data.put("foo", "bar"); + return data; + } + + @Test + void acceptsFormRequests() { + PredicatedEncoder encoder = FormEncoder.createPredicatedFormEncoder(); + + assertThat( + encoder.canEncode( + data(), Map.class, template("application/x-www-form-urlencoded; charset=utf-8"))) + .isTrue(); + assertThat(encoder.canEncode(data(), Map.class, template("multipart/form-data"))).isTrue(); + } + + @Test + void leavesEverythingElseToTheOtherEncoders() { + PredicatedEncoder encoder = FormEncoder.createPredicatedFormEncoder(); + + assertThat(encoder.canEncode("body", String.class, template("application/json"))).isFalse(); + assertThat(encoder.canEncode(data(), Map.class, template(null))).isFalse(); + assertThat(encoder.canEncode("body", String.class, template("multipart/form-data"))).isFalse(); + } + + @Test + void encodesTheFormItAccepted() { + RequestTemplate template = template("application/x-www-form-urlencoded"); + + FormEncoder.createPredicatedFormEncoder().encode(data(), Map.class, template); + + assertThat(new String(template.body(), StandardCharsets.UTF_8)).isEqualTo("foo=bar"); + } + + @Test + void routesAlongsideOtherEncoders() { + Encoder json = (object, bodyType, template) -> template.body("json"); + + Encoder encoder = + MultiEncoder.builder() + .add(FormEncoder.createPredicatedFormEncoder()) + .add(EncoderPredicate.jsonContentType(), json) + .build(); + + RequestTemplate form = template("application/x-www-form-urlencoded"); + encoder.encode(data(), Map.class, form); + assertThat(new String(form.body(), StandardCharsets.UTF_8)).isEqualTo("foo=bar"); + + RequestTemplate other = template("application/json"); + encoder.encode("body", String.class, other); + assertThat(other.requestBody().asString()).isEqualTo("json"); + } + + @Test + void withoutADelegateAnythingItCannotEncodeFails() { + RequestTemplate template = template("application/x-www-form-urlencoded"); + + assertThatThrownBy(() -> new FormEncoder(null).encode("body", String.class, template)) + .isInstanceOf(EncodeException.class) + .hasMessageContaining("This form encoder has no delegate encoder"); + } +}