Skip to content

Commit 183a5a1

Browse files
author
Adrian Cole
committed
Makes body parameter type explicit
Feign has `MethodMetadata.bodyType()`, but never passed it to encoders. Encoders that register type adapters need to do so based on the interface desired as opposed to the implementation class. This change breaks api compatibility for < 8.x, by requiring an additional arg on `Encoder.encode`. see square/retrofit#713
1 parent 50ea3ea commit 183a5a1

23 files changed

Lines changed: 561 additions & 444 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
### Version 8.0
22
* Removes Dagger 1.x Dependency
33
* Removes support for parameters annotated with `javax.inject.@Named`. Use `feign.@Param` instead.
4+
* Makes body parameter type explicit.
45

56
### Version 7.1
67
* Introduces feign.@Param to annotate template parameters. Users must migrate from `javax.inject.@Named` to `feign.@Param` before updating to Feign 8.0.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ MethodMetadata bodyIndex(Integer bodyIndex) {
7979
return this;
8080
}
8181

82+
/** Type corresponding to {@link #bodyIndex()}. */
8283
public Type bodyType() {
8384
return bodyType;
8485
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ protected RequestTemplate resolve(Object[] argv, RequestTemplate mutable, Map<St
201201
formVariables.put(entry.getKey(), entry.getValue());
202202
}
203203
try {
204-
encoder.encode(formVariables, mutable);
204+
encoder.encode(formVariables, Types.MAP_STRING_WILDCARD, mutable);
205205
} catch (EncodeException e) {
206206
throw e;
207207
} catch (RuntimeException e) {
@@ -224,7 +224,7 @@ protected RequestTemplate resolve(Object[] argv, RequestTemplate mutable, Map<St
224224
Object body = argv[metadata.bodyIndex()];
225225
checkArgument(body != null, "Body parameter %s was null", metadata.bodyIndex());
226226
try {
227-
encoder.encode(body, mutable);
227+
encoder.encode(body, metadata.bodyType(), mutable);
228228
} catch (EncodeException e) {
229229
throw e;
230230
} catch (RuntimeException e) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.lang.reflect.TypeVariable;
2424
import java.lang.reflect.WildcardType;
2525
import java.util.Arrays;
26+
import java.util.Map;
2627
import java.util.NoSuchElementException;
2728

2829
/**
@@ -32,6 +33,10 @@
3233
* @author Jesse Wilson
3334
*/
3435
final class Types {
36+
/** Type literal for {@code Map<String, ?>}. */
37+
static final Type MAP_STRING_WILDCARD = new ParameterizedTypeImpl(null, Map.class, String.class,
38+
new WildcardTypeImpl(new Type[] { Object.class }, new Type[] { }));
39+
3540
private static final Type[] EMPTY_TYPE_ARRAY = new Type[0];
3641

3742
private Types() {

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package feign.codec;
1717

1818
import feign.RequestTemplate;
19+
import java.lang.reflect.Type;
1920

2021
import static java.lang.String.format;
2122

@@ -40,8 +41,8 @@
4041
* }
4142
*
4243
* &#064;Override
43-
* public void encode(Object object, RequestTemplate template) {
44-
* template.body(gson.toJson(object));
44+
* public void encode(Object object, Type bodyType, RequestTemplate template) {
45+
* template.body(gson.toJson(object, bodyType));
4546
* }
4647
* }
4748
* </pre>
@@ -59,24 +60,25 @@
5960
* </pre>
6061
*/
6162
public interface Encoder {
63+
6264
/**
6365
* Converts objects to an appropriate representation in the template.
6466
*
6567
* @param object what to encode as the request body.
68+
* @param bodyType the type the object should be encoded as. {@code Map<String, ?>}, if form encoding.
6669
* @param template the request template to populate.
6770
* @throws EncodeException when encoding failed due to a checked exception.
6871
*/
69-
void encode(Object object, RequestTemplate template) throws EncodeException;
72+
void encode(Object object, Type bodyType, RequestTemplate template) throws EncodeException;
7073

7174
/**
7275
* Default implementation of {@code Encoder}.
7376
*/
7477
class Default implements Encoder {
75-
@Override
76-
public void encode(Object object, RequestTemplate template) throws EncodeException {
77-
if (object instanceof String) {
78+
@Override public void encode(Object object, Type bodyType, RequestTemplate template) {
79+
if (bodyType == String.class) {
7880
template.body(object.toString());
79-
} else if (object instanceof byte[]) {
81+
} else if (bodyType == byte[].class) {
8082
template.body((byte[]) object, null);
8183
} else if (object != null) {
8284
throw new EncodeException(format("%s is not a type supported by this encoder.", object.getClass()));

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,14 @@ void login(
231231
);
232232
}
233233

234+
/** Body type is only for the body param. */
235+
@Test public void formParamsDoesNotSetBodyType() throws Exception {
236+
MethodMetadata md = contract.parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class,
237+
String.class, String.class));
238+
239+
assertThat(md.bodyType()).isNull();
240+
}
241+
234242
interface HeaderParams {
235243
@RequestLine("POST /")
236244
@Headers({"Auth-Token: {Auth-Token}", "Auth-Token: Foo"})

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,15 @@
1818
import com.squareup.okhttp.mockwebserver.MockResponse;
1919
import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
2020
import feign.codec.Decoder;
21-
import feign.codec.EncodeException;
2221
import feign.codec.Encoder;
23-
import org.junit.Rule;
24-
2522
import java.lang.reflect.InvocationHandler;
2623
import java.lang.reflect.Method;
2724
import java.lang.reflect.Type;
2825
import java.util.Arrays;
2926
import java.util.List;
3027
import java.util.Map;
3128
import java.util.concurrent.atomic.AtomicInteger;
29+
import org.junit.Rule;
3230
import org.junit.Test;
3331

3432
import static feign.assertj.MockWebServerAssertions.assertThat;
@@ -63,8 +61,7 @@ interface TestInterface {
6361

6462
String url = "http://localhost:" + server.getPort();
6563
Encoder encoder = new Encoder() {
66-
@Override
67-
public void encode(Object object, RequestTemplate template) throws EncodeException {
64+
@Override public void encode(Object object, Type bodyType, RequestTemplate template) {
6865
template.body(object.toString());
6966
}
7067
};

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616
package feign;
1717

1818
import com.google.gson.Gson;
19+
import com.google.gson.reflect.TypeToken;
1920
import com.squareup.okhttp.mockwebserver.MockResponse;
2021
import com.squareup.okhttp.mockwebserver.SocketPolicy;
2122
import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
2223
import feign.Target.HardCodedTarget;
2324
import feign.codec.Decoder;
25+
import feign.codec.EncodeException;
2426
import feign.codec.Encoder;
2527
import feign.codec.ErrorDecoder;
2628
import feign.codec.StringDecoder;
@@ -31,6 +33,7 @@
3133
import java.util.Date;
3234
import java.util.List;
3335
import java.util.Map;
36+
import java.util.concurrent.atomic.AtomicReference;
3437
import org.junit.Rule;
3538
import org.junit.Test;
3639
import org.junit.rules.ExpectedException;
@@ -137,6 +140,26 @@ interface OtherTestInterface {
137140
.hasBody("[netflix, denominator, password]");
138141
}
139142

143+
/** The type of a parameter value may not be the desired type to encode as. Prefer the interface type. */
144+
@Test public void bodyTypeCorrespondsWithParameterType() throws IOException, InterruptedException {
145+
server.enqueue(new MockResponse().setBody("foo"));
146+
147+
final AtomicReference<Type> encodedType = new AtomicReference<Type>();
148+
TestInterface api = new TestInterfaceBuilder()
149+
.encoder(new Encoder.Default() {
150+
@Override public void encode(Object object, Type bodyType, RequestTemplate template) throws EncodeException {
151+
encodedType.set(bodyType);
152+
}
153+
})
154+
.target("http://localhost:" + server.getPort());
155+
156+
api.body(Arrays.asList("netflix", "denominator", "password"));
157+
158+
server.takeRequest();
159+
160+
assertThat(encodedType.get()).isEqualTo(new TypeToken<List<String>>(){}.getType());
161+
}
162+
140163
@Test public void postGZIPEncodedBodyParam() throws IOException, InterruptedException {
141164
server.enqueue(new MockResponse().setBody("foo"));
142165

@@ -348,7 +371,7 @@ static final class TestInterfaceBuilder {
348371
private final Feign.Builder delegate = new Feign.Builder()
349372
.decoder(new Decoder.Default())
350373
.encoder(new Encoder() {
351-
@Override public void encode(Object object, RequestTemplate template) {
374+
@Override public void encode(Object object, Type bodyType, RequestTemplate template) {
352375
if (object instanceof Map) {
353376
template.body(new Gson().toJson(object));
354377
} else {
@@ -362,8 +385,8 @@ TestInterfaceBuilder requestInterceptor(RequestInterceptor requestInterceptor) {
362385
return this;
363386
}
364387

365-
TestInterfaceBuilder client(Client client) {
366-
delegate.client(client);
388+
TestInterfaceBuilder encoder(Encoder encoder) {
389+
delegate.encoder(encoder);
367390
return this;
368391
}
369392

‎core/src/test/java/feign/assertj/RequestTemplateAssert.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ public RequestTemplateAssert hasUrl(String expected) {
4646
}
4747

4848
public RequestTemplateAssert hasBody(String utf8Expected) {
49-
return hasBody(utf8Expected.getBytes(UTF_8));
49+
isNotNull();
50+
if (actual.bodyTemplate() != null) {
51+
failWithMessage("\nExpecting bodyTemplate to be null, but was:<%s>", actual.bodyTemplate());
52+
}
53+
objects.assertEqual(info, new String(actual.body(), UTF_8), utf8Expected);
54+
return this;
5055
}
5156

5257
public RequestTemplateAssert hasBody(byte[] expected) {

‎core/src/test/java/feign/codec/DefaultEncoderTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,21 @@ public class DefaultEncoderTest {
3434
@Test public void testEncodesStrings() throws Exception {
3535
String content = "This is my content";
3636
RequestTemplate template = new RequestTemplate();
37-
encoder.encode(content, template);
37+
encoder.encode(content, String.class, template);
3838
assertEquals(content, new String(template.body(), UTF_8));
3939
}
4040

4141
@Test public void testEncodesByteArray() throws Exception {
4242
byte[] content = {12, 34, 56};
4343
RequestTemplate template = new RequestTemplate();
44-
encoder.encode(content, template);
44+
encoder.encode(content, byte[].class, template);
4545
assertTrue(Arrays.equals(content, template.body()));
4646
}
4747

4848
@Test public void testRefusesToEncodeOtherTypes() throws Exception {
4949
thrown.expect(EncodeException.class);
5050
thrown.expectMessage("is not a type supported by this encoder.");
5151

52-
encoder.encode(new Date(), new RequestTemplate());
52+
encoder.encode(new Date(), Date.class, new RequestTemplate());
5353
}
5454
}

0 commit comments

Comments
 (0)