Skip to content

Commit

Permalink
Updated Expression Patterns to allow brackets
Browse files Browse the repository at this point in the history
Fixes OpenFeign#928

Relaxed the regular expression that is used to determine if a given
value is an Expression per the URI Template Spec RFC 6570.  We already
deviated by allowing dashes to exist without pct-encoding, this change
adds braces `[]` to this list.

Also included is the ability to set Collection Format per Query, overriding
the Template default.  This allows for mixed Collection formats in the
same template and provides a way for Contract extensions to determine
which expansion type they want when parsing a contract.
  • Loading branch information
kdavisk6 committed Apr 8, 2019
1 parent 7ba16df commit 040aeb1
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 6 deletions.
26 changes: 21 additions & 5 deletions core/src/main/java/feign/RequestTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ public RequestTemplate query(String name, String... values) {
return query(name, Arrays.asList(values));
}


/**
* Specify a Query String parameter, with the specified values. Values can be literals or template
* expressions.
Expand All @@ -552,17 +553,33 @@ public RequestTemplate query(String name, String... values) {
* @return a RequestTemplate for chaining.
*/
public RequestTemplate query(String name, Iterable<String> values) {
return appendQuery(name, values);
return appendQuery(name, values, this.collectionFormat);
}

/**
* Specify a Query String parameter, with the specified values. Values can be literals or
* template expressions.
*
* @param name of the parameter.
* @param values for this parameter.
* @param collectionFormat to use when resolving collection based expressions.
* @return a Request Template for chaining.
*/
public RequestTemplate query(String name, Iterable<String> values,
CollectionFormat collectionFormat) {
return appendQuery(name, values, collectionFormat);
}

/**
* Appends the query name and values.
*
* @param name of the parameter.
* @param values for the parameter, may be expressions.
* @param collectionFormat to use when resolving collection based query variables.
* @return a RequestTemplate for chaining.
*/
private RequestTemplate appendQuery(String name, Iterable<String> values) {
private RequestTemplate appendQuery(String name, Iterable<String> values,
CollectionFormat collectionFormat) {
if (!values.iterator().hasNext()) {
/* empty value, clear the existing values */
this.queries.remove(name);
Expand All @@ -572,12 +589,11 @@ private RequestTemplate appendQuery(String name, Iterable<String> values) {
/* create a new query template out of the information here */
this.queries.compute(name, (key, queryTemplate) -> {
if (queryTemplate == null) {
return QueryTemplate.create(name, values, this.charset, this.collectionFormat);
return QueryTemplate.create(name, values, this.charset, collectionFormat);
} else {
return QueryTemplate.append(queryTemplate, values, this.collectionFormat);
return QueryTemplate.append(queryTemplate, values, collectionFormat);
}
});
// this.queries.put(name, QueryTemplate.create(name, values));
return this;
}

Expand Down
12 changes: 11 additions & 1 deletion core/src/main/java/feign/template/Expressions.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,17 @@ public final class Expressions {

static {
expressions = new LinkedHashMap<>();
expressions.put(Pattern.compile("(\\w[-\\w.]*[ ]*)(:(.+))?"), SimpleExpression.class);

/*
* basic pattern for variable names. this is compliant with RFC 6570 Simple Expressions ONLY
* with the following additional values allowed without required pct-encoding:
*
* - brackets - dashes
*
* see https://tools.ietf.org/html/rfc6570#section-2.3 for more information.
*/
expressions.put(Pattern.compile("(\\w[-\\w.\\[\\]]*[ ]*)(:(.+))?"),
SimpleExpression.class);
}

public static Expression create(final String value, final FragmentType type) {
Expand Down
16 changes: 16 additions & 0 deletions core/src/test/java/feign/RequestTemplateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,22 @@ public void resolveTemplateWithBaseAndParameterizedIterableQuery() {
entry("Queries", asList("us-east-1", "eu-west-1")));
}

@Test
public void resolveTemplateWithMixedCollectionFormatsByQuery() {
RequestTemplate template = new RequestTemplate()
.method(HttpMethod.GET)
.collectionFormat(CollectionFormat.EXPLODED)
.uri("/api/collections")
.query("keys", "{keys}") // default collection format
.query("values[]", Collections.singletonList("{values[]}"), CollectionFormat.CSV);

template = template.resolve(mapOf("keys", Arrays.asList("one", "two"),
"values[]", Arrays.asList("1", "2")));

assertThat(template.url())
.isEqualToIgnoringCase("/api/collections?keys=one&keys=two&values%5B%5D=1,2");
}

@Test
public void resolveTemplateWithHeaderSubstitutions() {
RequestTemplate template = new RequestTemplate().method(HttpMethod.GET)
Expand Down
12 changes: 12 additions & 0 deletions core/src/test/java/feign/template/QueryTemplateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,16 @@ public void expandSingleValueWithJson() {
assertThat(expanded).isEqualToIgnoringCase(
"json=%7B%22name%22:%22feign%22,%22version%22:%20%2210%22%7D");
}


@Test
public void expandCollectionValueWithBrackets() {
QueryTemplate template =
QueryTemplate.create("collection[]", Collections.singletonList("{collection[]}"),
Util.UTF_8, CollectionFormat.CSV);
String expanded = template.expand(Collections.singletonMap("collection[]",
Arrays.asList("1", "2")));
/* brackets will be pct-encoded */
assertThat(expanded).isEqualToIgnoringCase("collection%5B%5D=1,2");
}
}

0 comments on commit 040aeb1

Please sign in to comment.