Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated Query Expressions to support empty and undefined values #910

Merged

Conversation

kdavisk6
Copy link
Member

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

public void test() {
   Map<String, Object> parameters = new LinkedHashMap<>();
   parameters.put("param", "");
   this.demoClient.test(parameters);
}

Result

http://localhost:8080/test?param=

Missing

public void test() {
   Map<String, Object> parameters = new LinkedHashMap<>();
   this.demoClient.test(parameters);
}

Result

http://localhost:8080/test

Undefined

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.

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.
@kdavisk6 kdavisk6 added the regression Bugs and issues related to unintended breaking changes label Feb 18, 2019
@velo velo merged commit 089a59f into OpenFeign:master Mar 4, 2019
@kdavisk6 kdavisk6 added this to the 10.2.1 milestone Apr 28, 2019
@kdavisk6 kdavisk6 deleted the gh-872-empty-parameters-should-remain branch May 9, 2019 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Bugs and issues related to unintended breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty query params shouldn't be filtered out in RequestTemplate
2 participants