Browse Source

Updated Query Expressions to support empty and undefined values (#910)

Fixes #872

Previously, all unresolved query template expressions resolved
to empty strings, which then indcate that the entire query parameter
should be removed.  This violates RFC 6570 in that only undefined
values should be removed.  This change updates Query Template to
check the provided `variables` map for an entry expression.  If
no value is provided, the entry is explicitly marked `UNDEF` and
removed.

This brings us in line with the specification.  The following is
now how parameters are resolved:

*Empty String*
```java
public void test() {
   Map<String, Object> parameters = new LinkedHashMap<>();
   parameters.put("param", "");
   this.demoClient.test(parameters);
}
```
Result
```
http://localhost:8080/test?param=
```

*Missing*
```java
public void test() {
   Map<String, Object> parameters = new LinkedHashMap<>();
   this.demoClient.test(parameters);
}
```
Result
```
http://localhost:8080/test
```

*Undefined*
```java
public void test() {
   Map<String, Object> parameters = new LinkedHashMap<>();
   parameters.put("param", null);
   this.demoClient.test(parameters);
}
```
Result
```
http://localhost:8080/test
```

* Adding additional test case for explicit null parameter value

* Additional Test case for the explict `null` case.  Updates to the
documentation.
pull/918/head
Kevin Davis 6 years ago committed by Marvin Froeder
parent
commit
089a59f980
  1. 46
      README.md
  2. 20
      core/src/main/java/feign/template/QueryTemplate.java
  3. 39
      core/src/main/java/feign/template/Template.java
  4. 16
      core/src/test/java/feign/template/QueryTemplateTest.java
  5. 1
      jaxb/pom.xml

46
README.md

@ -107,6 +107,52 @@ resolved values. *Example* `owner` must be alphabetic. `{owner:[a-zA-Z]*}` @@ -107,6 +107,52 @@ resolved values. *Example* `owner` must be alphabetic. `{owner:[a-zA-Z]*}`
* Unresolved expressions are omitted.
* All literals and variable values are pct-encoded, if not already encoded or marked `encoded` via a `@Param` annotation.
#### Undefined vs. Empty Values ####
Undefined expressions are expressions where the value for the expression is an explicit `null` or no value is provided.
Per [URI Template - RFC 6570](https://tools.ietf.org/html/rfc6570), it is possible to provide an empty value
for an expression. When Feign resolves an expression, it first determines if the value is defined, if it is then
the query parameter will remain. If the expression is undefined, the query parameter is removed. See below
for a complete breakdown.
*Empty String*
```java
public void test() {
Map<String, Object> parameters = new LinkedHashMap<>();
parameters.put("param", "");
this.demoClient.test(parameters);
}
```
Result
```
http://localhost:8080/test?param=
```
*Missing*
```java
public void test() {
Map<String, Object> parameters = new LinkedHashMap<>();
this.demoClient.test(parameters);
}
```
Result
```
http://localhost:8080/test
```
*Undefined*
```java
public void test() {
Map<String, Object> parameters = new LinkedHashMap<>();
parameters.put("param", null);
this.demoClient.test(parameters);
}
```
Result
```
http://localhost:8080/test
```
See [Advanced Usage](#advanced-usage) for more examples.
> **What about slashes? `/`**

20
core/src/main/java/feign/template/QueryTemplate.java

@ -22,6 +22,8 @@ import java.util.Collection; @@ -22,6 +22,8 @@ import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
@ -30,6 +32,7 @@ import java.util.stream.StreamSupport; @@ -30,6 +32,7 @@ import java.util.stream.StreamSupport;
*/
public final class QueryTemplate extends Template {
public static final String UNDEF = "undef";
/* cache a copy of the variables for lookup later */
private List<String> values;
private final Template name;
@ -158,6 +161,20 @@ public final class QueryTemplate extends Template { @@ -158,6 +161,20 @@ public final class QueryTemplate extends Template {
return this.queryString(name, super.expand(variables));
}
@Override
protected String resolveExpression(Expression expression, Map<String, ?> variables) {
if (variables.containsKey(expression.getName())) {
if (variables.get(expression.getName()) == null) {
/* explicit undefined */
return UNDEF;
}
return super.resolveExpression(expression, variables);
}
/* mark the variable as undefined */
return UNDEF;
}
private String queryString(String name, String values) {
if (this.pure) {
return name;
@ -165,7 +182,8 @@ public final class QueryTemplate extends Template { @@ -165,7 +182,8 @@ public final class QueryTemplate extends Template {
/* covert the comma separated values into a value query string */
List<String> resolved = Arrays.stream(values.split(","))
.filter(Util::isNotBlank)
.filter(Objects::nonNull)
.filter(s -> !UNDEF.equalsIgnoreCase(s))
.collect(Collectors.toList());
if (!resolved.isEmpty()) {

39
core/src/main/java/feign/template/Template.java

@ -13,6 +13,7 @@ @@ -13,6 +13,7 @@
*/
package feign.template;
import feign.Util;
import feign.template.UriUtils.FragmentType;
import java.nio.charset.Charset;
import java.util.ArrayList;
@ -77,20 +78,9 @@ public class Template { @@ -77,20 +78,9 @@ public class Template {
StringBuilder resolved = new StringBuilder();
for (TemplateChunk chunk : this.templateChunks) {
if (chunk instanceof Expression) {
Expression expression = (Expression) chunk;
Object value = variables.get(expression.getName());
if (value != null) {
String expanded = expression.expand(value, this.encode.isEncodingRequired());
if (this.encodeSlash) {
logger.fine("Explicit slash decoding specified, decoding all slashes in uri");
expanded = expanded.replaceAll("/", "%2F");
}
resolved.append(expanded);
} else {
if (this.allowUnresolved) {
/* unresolved variables are treated as literals */
resolved.append(encode(expression.toString()));
}
String resolvedExpression = this.resolveExpression((Expression) chunk, variables);
if (resolvedExpression != null) {
resolved.append(resolvedExpression);
}
} else {
/* chunk is a literal value */
@ -100,6 +90,27 @@ public class Template { @@ -100,6 +90,27 @@ public class Template {
return resolved.toString();
}
protected String resolveExpression(Expression expression, Map<String, ?> variables) {
String resolved = null;
Object value = variables.get(expression.getName());
if (value != null) {
String expanded = expression.expand(value, this.encode.isEncodingRequired());
if (Util.isNotBlank(expanded)) {
if (this.encodeSlash) {
logger.fine("Explicit slash decoding specified, decoding all slashes in uri");
expanded = expanded.replaceAll("/", "%2F");
}
resolved = expanded;
}
} else {
if (this.allowUnresolved) {
/* unresolved variables are treated as literals */
resolved = encode(expression.toString());
}
}
return resolved;
}
/**
* Uri Encode the value.
*

16
core/src/test/java/feign/template/QueryTemplateTest.java

@ -62,6 +62,22 @@ public class QueryTemplateTest { @@ -62,6 +62,22 @@ public class QueryTemplateTest {
assertThat(expanded).isNullOrEmpty();
}
@Test
public void explicitNullValuesAreRemoved() {
QueryTemplate template =
QueryTemplate.create("name", Collections.singletonList("{value}"), Util.UTF_8);
String expanded = template.expand(Collections.singletonMap("value", null));
assertThat(expanded).isNullOrEmpty();
}
@Test
public void emptyParameterRemains() {
QueryTemplate template =
QueryTemplate.create("name", Collections.singletonList("{value}"), Util.UTF_8);
String expanded = template.expand(Collections.singletonMap("value", ""));
assertThat(expanded).isEqualToIgnoringCase("name=");
}
@Test
public void collectionFormat() {
QueryTemplate template =

1
jaxb/pom.xml

@ -47,6 +47,7 @@ @@ -47,6 +47,7 @@
<profiles>
<profile>
<id>java11</id>
<activation>
<jdk>11</jdk>
</activation>

Loading…
Cancel
Save