From 072dcf9436bc0d14e597c1ec225ac608d21813a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Carvalho?= Date: Tue, 20 Jul 2021 18:10:50 -0500 Subject: [PATCH] Modifying Contract to support passing all parameters to encoders #1448 (#1459) * Modifying Contract to support passing all parameters to encoders * Formatting license * Adding unit tests * Adding AlwaysEncodeBodyContract abstract class (#1) --- .../java/feign/AlwaysEncodeBodyContract.java | 31 +++++ core/src/main/java/feign/Contract.java | 5 +- .../main/java/feign/DeclarativeContract.java | 2 +- core/src/main/java/feign/MethodMetadata.java | 14 ++- core/src/main/java/feign/ReflectiveFeign.java | 22 +++- .../feign/AlwaysEncodeBodyContractTest.java | 117 ++++++++++++++++++ 6 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 core/src/main/java/feign/AlwaysEncodeBodyContract.java create mode 100644 core/src/test/java/feign/AlwaysEncodeBodyContractTest.java diff --git a/core/src/main/java/feign/AlwaysEncodeBodyContract.java b/core/src/main/java/feign/AlwaysEncodeBodyContract.java new file mode 100644 index 0000000000..ee4fa430dc --- /dev/null +++ b/core/src/main/java/feign/AlwaysEncodeBodyContract.java @@ -0,0 +1,31 @@ +/** + * Copyright 2012-2021 The Feign Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package feign; + +/** + * {@link DeclarativeContract} extension that allows user provided custom encoders to define the + * request message payload using only the request template and the method parameters, not requiring + * a specific and unique body object. + * + * This type of contract is useful when an application needs a Feign client whose request payload is + * defined entirely by a custom Feign encoder regardless of how many parameters are declared at the + * client method. In this case, even with no presence of body parameter the provided encoder will + * have to know how to define the request payload (for example, based on the method name, method + * return type, and other metadata provided by custom annotations, all available via the provided + * {@link RequestTemplate} object). + * + * @author fabiocarvalho777@gmail.com + */ +public abstract class AlwaysEncodeBodyContract extends DeclarativeContract { +} diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java index 360eea876a..3d653ca9ce 100644 --- a/core/src/main/java/feign/Contract.java +++ b/core/src/main/java/feign/Contract.java @@ -96,6 +96,9 @@ protected MethodMetadata parseAndValidateMetadata(Class targetType, Method me data.returnType( Types.resolve(targetType, targetType, method.getGenericReturnType())); data.configKey(Feign.configKey(targetType, method)); + if (AlwaysEncodeBodyContract.class.isAssignableFrom(this.getClass())) { + data.alwaysEncodeBody(true); + } if (targetType.getInterfaces().length == 1) { processAnnotationOnClass(data, targetType.getInterfaces()[0]); @@ -133,7 +136,7 @@ protected MethodMetadata parseAndValidateMetadata(Class targetType, Method me if (data.isAlreadyProcessed(i)) { checkState(data.formParams().isEmpty() || data.bodyIndex() == null, "Body parameters cannot be used with form parameters.%s", data.warnings()); - } else { + } else if (!data.alwaysEncodeBody()) { checkState(data.formParams().isEmpty(), "Body parameters cannot be used with form parameters.%s", data.warnings()); checkState(data.bodyIndex() == null, diff --git a/core/src/main/java/feign/DeclarativeContract.java b/core/src/main/java/feign/DeclarativeContract.java index 2eb55efb76..81743dafbb 100644 --- a/core/src/main/java/feign/DeclarativeContract.java +++ b/core/src/main/java/feign/DeclarativeContract.java @@ -43,7 +43,7 @@ public final List parseAndValidateMetadata(Class targetType) * (unless they are the same). * * @param data metadata collected so far relating to the current java method. - * @param clz the class to process + * @param targetType the class to process */ @Override protected final void processAnnotationOnClass(MethodMetadata data, Class targetType) { diff --git a/core/src/main/java/feign/MethodMetadata.java b/core/src/main/java/feign/MethodMetadata.java index 87de5e43f9..b4370764dc 100644 --- a/core/src/main/java/feign/MethodMetadata.java +++ b/core/src/main/java/feign/MethodMetadata.java @@ -1,5 +1,5 @@ /** - * Copyright 2012-2020 The Feign Authors + * Copyright 2012-2021 The Feign Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -30,6 +30,7 @@ public final class MethodMetadata implements Serializable { private Integer headerMapIndex; private Integer queryMapIndex; private boolean queryMapEncoded; + private boolean alwaysEncodeBody; private transient Type bodyType; private final RequestTemplate template = new RequestTemplate(); private final List formParams = new ArrayList(); @@ -118,6 +119,17 @@ public MethodMetadata queryMapEncoded(boolean queryMapEncoded) { return this; } + @Experimental + public boolean alwaysEncodeBody() { + return alwaysEncodeBody; + } + + @Experimental + MethodMetadata alwaysEncodeBody(boolean alwaysEncodeBody) { + this.alwaysEncodeBody = alwaysEncodeBody; + return this; + } + /** * Type corresponding to {@link #bodyIndex()}. */ diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java index c067c49094..426f7cf96f 100644 --- a/core/src/main/java/feign/ReflectiveFeign.java +++ b/core/src/main/java/feign/ReflectiveFeign.java @@ -1,5 +1,5 @@ /** - * Copyright 2012-2020 The Feign Authors + * Copyright 2012-2021 The Feign Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -155,7 +155,7 @@ public Map apply(Target target) { if (!md.formParams().isEmpty() && md.template().bodyTemplate() == null) { buildTemplate = new BuildFormEncodedTemplateFromArgs(md, encoder, queryMapEncoder, target); - } else if (md.bodyIndex() != null) { + } else if (md.bodyIndex() != null || md.alwaysEncodeBody()) { buildTemplate = new BuildEncodedTemplateFromArgs(md, encoder, queryMapEncoder, target); } else { buildTemplate = new BuildTemplateByResolvingArgs(md, queryMapEncoder, target); @@ -379,10 +379,22 @@ private BuildEncodedTemplateFromArgs(MethodMetadata metadata, Encoder encoder, protected RequestTemplate resolve(Object[] argv, RequestTemplate mutable, Map variables) { - Object body = argv[metadata.bodyIndex()]; - checkArgument(body != null, "Body parameter %s was null", metadata.bodyIndex()); + + boolean alwaysEncodeBody = mutable.methodMetadata().alwaysEncodeBody(); + + Object body = null; + if (!alwaysEncodeBody) { + body = argv[metadata.bodyIndex()]; + checkArgument(body != null, "Body parameter %s was null", metadata.bodyIndex()); + } + try { - encoder.encode(body, metadata.bodyType(), mutable); + if (alwaysEncodeBody) { + body = argv == null ? new Object[0] : argv; + encoder.encode(body, Object[].class, mutable); + } else { + encoder.encode(body, metadata.bodyType(), mutable); + } } catch (EncodeException e) { throw e; } catch (RuntimeException e) { diff --git a/core/src/test/java/feign/AlwaysEncodeBodyContractTest.java b/core/src/test/java/feign/AlwaysEncodeBodyContractTest.java new file mode 100644 index 0000000000..9d692996e3 --- /dev/null +++ b/core/src/test/java/feign/AlwaysEncodeBodyContractTest.java @@ -0,0 +1,117 @@ +/** + * Copyright 2012-2021 The Feign Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package feign; + +import feign.codec.EncodeException; +import feign.codec.Encoder; +import org.junit.Assert; +import org.junit.Test; +import java.io.IOException; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.lang.reflect.Type; +import java.util.Arrays; +import java.util.stream.Collectors; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +public class AlwaysEncodeBodyContractTest { + + @Retention(RUNTIME) + @Target(ElementType.METHOD) + private @interface SampleMethodAnnotation { + } + + private static class SampleContract extends AlwaysEncodeBodyContract { + SampleContract() { + AnnotationProcessor annotationProcessor = + (annotation, metadata) -> metadata.template().method(Request.HttpMethod.POST); + super.registerMethodAnnotation(SampleMethodAnnotation.class, annotationProcessor); + } + } + + private interface SampleTargetMultipleNonAnnotatedParameters { + @SampleMethodAnnotation + String concatenate(String word1, String word2, String word3); + } + + private interface SampleTargetNoParameters { + @SampleMethodAnnotation + String concatenate(); + } + + private interface SampleTargetOneParameter { + @SampleMethodAnnotation + String concatenate(String word1); + } + + private static class AllParametersSampleEncoder implements Encoder { + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) + throws EncodeException { + Object[] methodParameters = (Object[]) object; + String body = + Arrays.stream(methodParameters).map(String::valueOf).collect(Collectors.joining()); + template.body(body); + } + } + + private static class BodyParameterSampleEncoder implements Encoder { + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) + throws EncodeException { + template.body(String.valueOf(object)); + } + } + + private static class SampleClient implements Client { + @Override + public Response execute(Request request, Request.Options options) throws IOException { + return Response.builder() + .status(200) + .request(request) + .body(request.body()) + .build(); + } + } + + /** + * This test makes sure Feign calls the client provided encoder regardless of how many + * non-annotated parameters the client method has, as alwaysEncodeBody is set to true. + */ + @Test + public void alwaysEncodeBodyTrueTest() { + SampleTargetMultipleNonAnnotatedParameters sampleClient1 = Feign.builder() + .contract(new SampleContract()) + .encoder(new AllParametersSampleEncoder()) + .client(new SampleClient()) + .target(SampleTargetMultipleNonAnnotatedParameters.class, "http://localhost"); + Assert.assertEquals("foobarchar", sampleClient1.concatenate("foo", "bar", "char")); + + SampleTargetNoParameters sampleClient2 = Feign.builder() + .contract(new SampleContract()) + .encoder(new AllParametersSampleEncoder()) + .client(new SampleClient()) + .target(SampleTargetNoParameters.class, "http://localhost"); + Assert.assertEquals("", sampleClient2.concatenate()); + + SampleTargetOneParameter sampleClient3 = Feign.builder() + .contract(new SampleContract()) + .encoder(new AllParametersSampleEncoder()) + .client(new SampleClient()) + .target(SampleTargetOneParameter.class, "http://localhost"); + Assert.assertEquals("moo", sampleClient3.concatenate("moo")); + } + +}