Skip to content

Commit 9ed47d5

Browse files
bbdouglasvelo
bbdouglas
authored and
velo
committed
Allows different collection encodings (OpenFeign#543)
* Allows different collection encodings In the case where a parameter represents a collection of values, there are conflicting ways of encoding that collection. Common ways are repeating the parameter name (foo=bar&foo=baz) and using comma separated values (foo=bar,baz). The current behavior repeats the parameter name. This change introduces an additional RequestLine parameter that explicitly specifies the encoding type, one of CSV, TSV, space-delimited, pipe-delimited, and repeating the parameter name. The default value for this option is repeating the parameter name, so backwards compatibility is maintained. * Replace switch statement with enum method for joining values
1 parent ea51e70 commit 9ed47d5

6 files changed

Lines changed: 151 additions & 13 deletions

File tree

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Copyright 2012-2018 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign;
15+
16+
import java.util.Collection;
17+
18+
/**
19+
* Various ways to encode collections in URL parameters.
20+
*
21+
* <p>These specific cases are inspired by the
22+
* <a href="http://swagger.io/specification/">OpenAPI specification</a>.</p>
23+
*/
24+
public enum CollectionFormat {
25+
/** Comma separated values, eg foo=bar,baz */
26+
CSV(","),
27+
/** Space separated values, eg foo=bar baz */
28+
SSV(" "),
29+
/** Tab separated values, eg foo=bar[tab]baz */
30+
TSV("\t"),
31+
/** Values separated with the pipe (|) character, eg foo=bar|baz */
32+
PIPES("|"),
33+
/** Parameter name repeated for each value, eg foo=bar&foo=baz */
34+
// Using null as a special case since there is no single separator character
35+
EXPLODED(null);
36+
37+
private final String separator;
38+
39+
CollectionFormat(String separator) {
40+
this.separator = separator;
41+
}
42+
43+
/**
44+
* Joins the field and possibly multiple values with the given separator.
45+
*
46+
* <p>Calling EXPLODED.join("foo", ["bar"]) will return "foo=bar".</p>
47+
*
48+
* <p>Calling CSV.join("foo", ["bar", "baz"]) will return "foo=bar,baz". </p>
49+
*
50+
* <p>Null values are treated somewhat specially. With EXPLODED, the field
51+
* is repeated without any "=" for backwards compatibility. With all other
52+
* formats, null values are not included in the joined value list.</p>
53+
*
54+
* @param field The field name corresponding to these values.
55+
* @param values A collection of value strings for the given field.
56+
* @return The formatted char sequence of the field and joined values. If the
57+
* value collection is empty, an empty char sequence will be returned.
58+
*/
59+
CharSequence join(String field, Collection<String> values) {
60+
StringBuilder builder = new StringBuilder();
61+
int valueCount = 0;
62+
for (String value : values) {
63+
if (separator == null) {
64+
// exploded
65+
builder.append(valueCount++ == 0 ? "" : "&");
66+
builder.append(field);
67+
if (value != null) {
68+
builder.append('=');
69+
builder.append(value);
70+
}
71+
} else {
72+
// delimited with a separator character
73+
if (builder.length() == 0) {
74+
builder.append(field);
75+
}
76+
if (value == null) {
77+
continue;
78+
}
79+
builder.append(valueCount++ == 0 ? "=" : separator);
80+
builder.append(value);
81+
}
82+
}
83+
return builder;
84+
}
85+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA
232232
}
233233

234234
data.template().decodeSlash(RequestLine.class.cast(methodAnnotation).decodeSlash());
235+
data.template().collectionFormat(RequestLine.class.cast(methodAnnotation).collectionFormat());
235236

236237
} else if (annotationType == Body.class) {
237238
String body = Body.class.cast(methodAnnotation).value();

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,5 @@
6161

6262
String value();
6363
boolean decodeSlash() default true;
64+
CollectionFormat collectionFormat() default CollectionFormat.EXPLODED;
6465
}

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public final class RequestTemplate implements Serializable {
5656
private byte[] body;
5757
private String bodyTemplate;
5858
private boolean decodeSlash = true;
59+
private CollectionFormat collectionFormat = CollectionFormat.EXPLODED;
5960

6061
public RequestTemplate() {
6162
}
@@ -71,6 +72,7 @@ public RequestTemplate(RequestTemplate toCopy) {
7172
this.body = toCopy.body;
7273
this.bodyTemplate = toCopy.bodyTemplate;
7374
this.decodeSlash = toCopy.decodeSlash;
75+
this.collectionFormat = toCopy.collectionFormat;
7476
}
7577

7678
private static String urlDecode(String arg) {
@@ -282,6 +284,15 @@ public boolean decodeSlash() {
282284
return decodeSlash;
283285
}
284286

287+
public RequestTemplate collectionFormat(CollectionFormat collectionFormat) {
288+
this.collectionFormat = collectionFormat;
289+
return this;
290+
}
291+
292+
public CollectionFormat collectionFormat() {
293+
return collectionFormat;
294+
}
295+
285296
/* @see #url() */
286297
public RequestTemplate append(CharSequence value) {
287298
url.append(value);
@@ -652,21 +663,14 @@ public String queryLine() {
652663
if (queries.isEmpty()) {
653664
return "";
654665
}
655-
StringBuilder queryBuilder = new StringBuilder();
666+
StringBuilder queryBuilder = new StringBuilder("?");
656667
for (String field : queries.keySet()) {
657-
for (String value : valuesOrEmpty(queries, field)) {
658-
queryBuilder.append('&');
659-
queryBuilder.append(field);
660-
if (value != null) {
661-
queryBuilder.append('=');
662-
if (!value.isEmpty()) {
663-
queryBuilder.append(value);
664-
}
665-
}
666-
}
668+
Collection<String> values = valuesOrEmpty(queries, field);
669+
CharSequence fieldAndValues = collectionFormat.join(field, values);
670+
queryBuilder.append(queryBuilder.length() == 1 || fieldAndValues.length() == 0 ? "" : "&");
671+
queryBuilder.append(fieldAndValues);
667672
}
668-
queryBuilder.deleteCharAt(0);
669-
return queryBuilder.insert(0, '?').toString();
673+
return queryBuilder.toString();
670674
}
671675

672676
interface Factory {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ public RecordedRequestAssert hasPath(String expected) {
6262
return this;
6363
}
6464

65+
public RecordedRequestAssert hasOneOfPath(String... expected) {
66+
isNotNull();
67+
objects.assertIsIn(info, actual.getPath(), expected);
68+
return this;
69+
}
70+
6571
public RecordedRequestAssert hasBody(String utf8Expected) {
6672
isNotNull();
6773
objects.assertEqual(info, actual.getBody().readUtf8(), utf8Expected);

‎core/src/test/java/feign/client/AbstractClientTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515

1616
import java.io.ByteArrayInputStream;
1717
import java.io.IOException;
18+
import java.util.Arrays;
19+
import java.util.List;
1820

1921
import org.junit.Rule;
2022
import org.junit.Test;
2123
import org.junit.rules.ExpectedException;
2224

2325
import feign.Client;
26+
import feign.CollectionFormat;
2427
import feign.Feign.Builder;
2528
import feign.FeignException;
2629
import feign.Headers;
@@ -269,6 +272,38 @@ public void testContentTypeDefaultsToRequestCharset() throws Exception {
269272
.hasBody("àáâãäåèéêë");
270273
}
271274

275+
@Test
276+
public void testDefaultCollectionFormat() throws Exception {
277+
server.enqueue(new MockResponse().setBody("body"));
278+
279+
TestInterface api = newBuilder()
280+
.target(TestInterface.class, "http://localhost:" + server.getPort());
281+
282+
Response response = api.get(Arrays.asList(new String[] {"bar","baz"}));
283+
284+
assertThat(response.status()).isEqualTo(200);
285+
assertThat(response.reason()).isEqualTo("OK");
286+
287+
MockWebServerAssertions.assertThat(server.takeRequest()).hasMethod("GET")
288+
.hasPath("/?foo=bar&foo=baz");
289+
}
290+
@Test
291+
public void testAlternativeCollectionFormat() throws Exception {
292+
server.enqueue(new MockResponse().setBody("body"));
293+
294+
TestInterface api = newBuilder()
295+
.target(TestInterface.class, "http://localhost:" + server.getPort());
296+
297+
Response response = api.getCSV(Arrays.asList(new String[] {"bar","baz"}));
298+
299+
assertThat(response.status()).isEqualTo(200);
300+
assertThat(response.reason()).isEqualTo("OK");
301+
302+
// Some HTTP libraries percent-encode commas in query parameters and others don't.
303+
MockWebServerAssertions.assertThat(server.takeRequest()).hasMethod("GET")
304+
.hasOneOfPath("/?foo=bar,baz", "/?foo=bar%2Cbaz");
305+
}
306+
272307
public interface TestInterface {
273308

274309
@RequestLine("POST /?foo=bar&foo=baz&qux=")
@@ -283,6 +318,12 @@ public interface TestInterface {
283318
@Headers("Accept: text/plain")
284319
String get();
285320

321+
@RequestLine("GET /?foo={multiFoo}")
322+
Response get(@Param("multiFoo") List<String> multiFoo);
323+
324+
@RequestLine(value = "GET /?foo={multiFoo}", collectionFormat = CollectionFormat.CSV)
325+
Response getCSV(@Param("multiFoo") List<String> multiFoo);
326+
286327
@RequestLine("PATCH /")
287328
@Headers("Accept: text/plain")
288329
String patch(String body);

0 commit comments

Comments
 (0)