Skip to content

Commit 089a59f

Browse files
kdavisk6velo
kdavisk6
authored and
velo
committed
Updated Query Expressions to support empty and undefined values (OpenFeign#910)
Fixes OpenFeign#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.
1 parent 32019a2 commit 089a59f

5 files changed

Lines changed: 107 additions & 15 deletions

File tree

‎README.md‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,52 @@ resolved values. *Example* `owner` must be alphabetic. `{owner:[a-zA-Z]*}`
107107
* Unresolved expressions are omitted.
108108
* All literals and variable values are pct-encoded, if not already encoded or marked `encoded` via a `@Param` annotation.
109109

110+
#### Undefined vs. Empty Values ####
111+
112+
Undefined expressions are expressions where the value for the expression is an explicit `null` or no value is provided.
113+
Per [URI Template - RFC 6570](https://tools.ietf.org/html/rfc6570), it is possible to provide an empty value
114+
for an expression. When Feign resolves an expression, it first determines if the value is defined, if it is then
115+
the query parameter will remain. If the expression is undefined, the query parameter is removed. See below
116+
for a complete breakdown.
117+
118+
*Empty String*
119+
```java
120+
public void test() {
121+
Map<String, Object> parameters = new LinkedHashMap<>();
122+
parameters.put("param", "");
123+
this.demoClient.test(parameters);
124+
}
125+
```
126+
Result
127+
```
128+
http://localhost:8080/test?param=
129+
```
130+
131+
*Missing*
132+
```java
133+
public void test() {
134+
Map<String, Object> parameters = new LinkedHashMap<>();
135+
this.demoClient.test(parameters);
136+
}
137+
```
138+
Result
139+
```
140+
http://localhost:8080/test
141+
```
142+
143+
*Undefined*
144+
```java
145+
public void test() {
146+
Map<String, Object> parameters = new LinkedHashMap<>();
147+
parameters.put("param", null);
148+
this.demoClient.test(parameters);
149+
}
150+
```
151+
Result
152+
```
153+
http://localhost:8080/test
154+
```
155+
110156
See [Advanced Usage](#advanced-usage) for more examples.
111157

112158
> **What about slashes? `/`**

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import java.util.Iterator;
2323
import java.util.List;
2424
import java.util.Map;
25+
import java.util.Objects;
26+
import java.util.function.Predicate;
2527
import java.util.stream.Collectors;
2628
import java.util.stream.StreamSupport;
2729

@@ -30,6 +32,7 @@
3032
*/
3133
public final class QueryTemplate extends Template {
3234

35+
public static final String UNDEF = "undef";
3336
/* cache a copy of the variables for lookup later */
3437
private List<String> values;
3538
private final Template name;
@@ -158,14 +161,29 @@ public String expand(Map<String, ?> variables) {
158161
return this.queryString(name, super.expand(variables));
159162
}
160163

164+
@Override
165+
protected String resolveExpression(Expression expression, Map<String, ?> variables) {
166+
if (variables.containsKey(expression.getName())) {
167+
if (variables.get(expression.getName()) == null) {
168+
/* explicit undefined */
169+
return UNDEF;
170+
}
171+
return super.resolveExpression(expression, variables);
172+
}
173+
174+
/* mark the variable as undefined */
175+
return UNDEF;
176+
}
177+
161178
private String queryString(String name, String values) {
162179
if (this.pure) {
163180
return name;
164181
}
165182

166183
/* covert the comma separated values into a value query string */
167184
List<String> resolved = Arrays.stream(values.split(","))
168-
.filter(Util::isNotBlank)
185+
.filter(Objects::nonNull)
186+
.filter(s -> !UNDEF.equalsIgnoreCase(s))
169187
.collect(Collectors.toList());
170188

171189
if (!resolved.isEmpty()) {

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

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
*/
1414
package feign.template;
1515

16+
import feign.Util;
1617
import feign.template.UriUtils.FragmentType;
1718
import java.nio.charset.Charset;
1819
import java.util.ArrayList;
@@ -77,20 +78,9 @@ public String expand(Map<String, ?> variables) {
7778
StringBuilder resolved = new StringBuilder();
7879
for (TemplateChunk chunk : this.templateChunks) {
7980
if (chunk instanceof Expression) {
80-
Expression expression = (Expression) chunk;
81-
Object value = variables.get(expression.getName());
82-
if (value != null) {
83-
String expanded = expression.expand(value, this.encode.isEncodingRequired());
84-
if (this.encodeSlash) {
85-
logger.fine("Explicit slash decoding specified, decoding all slashes in uri");
86-
expanded = expanded.replaceAll("/", "%2F");
87-
}
88-
resolved.append(expanded);
89-
} else {
90-
if (this.allowUnresolved) {
91-
/* unresolved variables are treated as literals */
92-
resolved.append(encode(expression.toString()));
93-
}
81+
String resolvedExpression = this.resolveExpression((Expression) chunk, variables);
82+
if (resolvedExpression != null) {
83+
resolved.append(resolvedExpression);
9484
}
9585
} else {
9686
/* chunk is a literal value */
@@ -100,6 +90,27 @@ public String expand(Map<String, ?> variables) {
10090
return resolved.toString();
10191
}
10292

93+
protected String resolveExpression(Expression expression, Map<String, ?> variables) {
94+
String resolved = null;
95+
Object value = variables.get(expression.getName());
96+
if (value != null) {
97+
String expanded = expression.expand(value, this.encode.isEncodingRequired());
98+
if (Util.isNotBlank(expanded)) {
99+
if (this.encodeSlash) {
100+
logger.fine("Explicit slash decoding specified, decoding all slashes in uri");
101+
expanded = expanded.replaceAll("/", "%2F");
102+
}
103+
resolved = expanded;
104+
}
105+
} else {
106+
if (this.allowUnresolved) {
107+
/* unresolved variables are treated as literals */
108+
resolved = encode(expression.toString());
109+
}
110+
}
111+
return resolved;
112+
}
113+
103114
/**
104115
* Uri Encode the value.
105116
*

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,22 @@ public void unresolvedMultiValueQueryTemplates() {
6262
assertThat(expanded).isNullOrEmpty();
6363
}
6464

65+
@Test
66+
public void explicitNullValuesAreRemoved() {
67+
QueryTemplate template =
68+
QueryTemplate.create("name", Collections.singletonList("{value}"), Util.UTF_8);
69+
String expanded = template.expand(Collections.singletonMap("value", null));
70+
assertThat(expanded).isNullOrEmpty();
71+
}
72+
73+
@Test
74+
public void emptyParameterRemains() {
75+
QueryTemplate template =
76+
QueryTemplate.create("name", Collections.singletonList("{value}"), Util.UTF_8);
77+
String expanded = template.expand(Collections.singletonMap("value", ""));
78+
assertThat(expanded).isEqualToIgnoringCase("name=");
79+
}
80+
6581
@Test
6682
public void collectionFormat() {
6783
QueryTemplate template =

‎jaxb/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747

4848
<profiles>
4949
<profile>
50+
<id>java11</id>
5051
<activation>
5152
<jdk>11</jdk>
5253
</activation>

0 commit comments

Comments
 (0)