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

Deprecate QueryMap.encode, remove processing of the "encode" parameter #1551

Merged
merged 10 commits into from
Mar 20, 2022
1 change: 0 additions & 1 deletion core/src/main/java/feign/Contract.java
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ public Default() {
checkState(data.queryMapIndex() == null,
"QueryMap annotation was present on multiple parameters.");
data.queryMapIndex(paramIndex);
data.queryMapEncoded(queryMap.encoded());
});
super.registerParameterAnnotation(HeaderMap.class, (queryMap, data, paramIndex) -> {
checkState(data.headerMapIndex() == null,
Expand Down
10 changes: 0 additions & 10 deletions core/src/main/java/feign/MethodMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public final class MethodMetadata implements Serializable {
private Integer bodyIndex;
private Integer headerMapIndex;
private Integer queryMapIndex;
private boolean queryMapEncoded;
private boolean alwaysEncodeBody;
private transient Type bodyType;
private final RequestTemplate template = new RequestTemplate();
Expand Down Expand Up @@ -110,15 +109,6 @@ public MethodMetadata queryMapIndex(Integer queryMapIndex) {
return this;
}

public boolean queryMapEncoded() {
return queryMapEncoded;
}

public MethodMetadata queryMapEncoded(boolean queryMapEncoded) {
this.queryMapEncoded = queryMapEncoded;
return this;
}

@Experimental
public boolean alwaysEncodeBody() {
return alwaysEncodeBody;
Expand Down
8 changes: 7 additions & 1 deletion core/src/main/java/feign/QueryMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,21 @@
* Once this conversion is applied, the query keys and resulting String values follow the same
* contract as if they were set using {@link RequestTemplate#query(String, String...)}.
*/
@SuppressWarnings("deprecation")
@Retention(RUNTIME)
@java.lang.annotation.Target(PARAMETER)
public @interface QueryMap {

/**
* Specifies whether parameter names and values are already encoded.
* <p>
* Deprecation: there are two options
* <ul>
* <li>if name or value are already encoded we do nothing;</li>
* <li>if name or value are not encoded we encode them.</li>
* </ul>
*
* @see Param#encoded
* @deprecated
*/
boolean encoded() default false;
}
13 changes: 4 additions & 9 deletions core/src/main/java/feign/ReflectiveFeign.java
Original file line number Diff line number Diff line change
Expand Up @@ -302,27 +302,22 @@ private RequestTemplate addQueryMapQueryParameters(Map<String, Object> queryMap,
for (Entry<String, Object> currEntry : queryMap.entrySet()) {
Collection<String> values = new ArrayList<String>();

boolean encoded = metadata.queryMapEncoded();
Object currValue = currEntry.getValue();
if (currValue instanceof Iterable<?>) {
Iterator<?> iter = ((Iterable<?>) currValue).iterator();
while (iter.hasNext()) {
Object nextObject = iter.next();
values.add(nextObject == null ? null
: encoded ? nextObject.toString()
: UriUtils.encode(nextObject.toString()));
values.add(nextObject == null ? null : UriUtils.encode(nextObject.toString()));
}
} else if (currValue instanceof Object[]) {
for (Object value : (Object[]) currValue) {
values.add(value == null ? null
: encoded ? value.toString() : UriUtils.encode(value.toString()));
values.add(value == null ? null : UriUtils.encode(value.toString()));
}
} else {
values.add(currValue == null ? null
: encoded ? currValue.toString() : UriUtils.encode(currValue.toString()));
values.add(currValue == null ? null : UriUtils.encode(currValue.toString()));
}

mutable.query(encoded ? currEntry.getKey() : UriUtils.encode(currEntry.getKey()), values);
mutable.query(UriUtils.encode(currEntry.getKey()), values);
}
return mutable;
}
Expand Down
54 changes: 0 additions & 54 deletions core/src/test/java/feign/DefaultContractTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -326,30 +326,6 @@ public void queryMap() throws Exception {
assertThat(md.queryMapIndex()).isEqualTo(0);
}

@Test
public void queryMapEncodedDefault() throws Exception {
final MethodMetadata md =
parseAndValidateMetadata(QueryMapTestInterface.class, "queryMap", Map.class);

assertThat(md.queryMapEncoded()).isFalse();
}

@Test
public void queryMapEncodedTrue() throws Exception {
final MethodMetadata md =
parseAndValidateMetadata(QueryMapTestInterface.class, "queryMapEncoded", Map.class);

assertThat(md.queryMapEncoded()).isTrue();
}

@Test
public void queryMapEncodedFalse() throws Exception {
final MethodMetadata md =
parseAndValidateMetadata(QueryMapTestInterface.class, "queryMapNotEncoded", Map.class);

assertThat(md.queryMapEncoded()).isFalse();
}

@Test
public void queryMapMapSubclass() throws Exception {
final MethodMetadata md =
Expand Down Expand Up @@ -388,24 +364,6 @@ public void queryMapPojoObject() throws Exception {
assertThat(md.queryMapIndex()).isEqualTo(0);
}

@Test
public void queryMapPojoObjectEncoded() throws Exception {
final MethodMetadata md =
parseAndValidateMetadata(QueryMapTestInterface.class, "pojoObjectEncoded", Object.class);

assertThat(md.queryMapIndex()).isEqualTo(0);
assertThat(md.queryMapEncoded()).isTrue();
}

@Test
public void queryMapPojoObjectNotEncoded() throws Exception {
final MethodMetadata md =
parseAndValidateMetadata(QueryMapTestInterface.class, "pojoObjectNotEncoded", Object.class);

assertThat(md.queryMapIndex()).isEqualTo(0);
assertThat(md.queryMapEncoded()).isFalse();
}

@Test
public void slashAreEncodedWhenNeeded() throws Exception {
MethodMetadata md = parseAndValidateMetadata(SlashNeedToBeEncoded.class,
Expand Down Expand Up @@ -609,21 +567,9 @@ interface QueryMapTestInterface {
@RequestLine("POST /")
void queryMapMapSubclass(@QueryMap SortedMap<String, String> queryMap);

@RequestLine("POST /")
void queryMapEncoded(@QueryMap(encoded = true) Map<String, String> queryMap);

@RequestLine("POST /")
void queryMapNotEncoded(@QueryMap(encoded = false) Map<String, String> queryMap);

@RequestLine("POST /")
void pojoObject(@QueryMap Object object);

@RequestLine("POST /")
void pojoObjectEncoded(@QueryMap(encoded = true) Object object);

@RequestLine("POST /")
void pojoObjectNotEncoded(@QueryMap(encoded = false) Object object);

// invalid
@RequestLine("POST /")
void multipleQueryMap(@QueryMap Map<String, String> mapOne,
Expand Down
7 changes: 2 additions & 5 deletions core/src/test/java/feign/FeignTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -365,14 +365,14 @@ public void queryMapValueStartingWithBrace() throws Exception {
server.enqueue(new MockResponse());
queryMap = new LinkedHashMap<String, Object>();
queryMap.put("name", "%7Balice");
api.queryMapEncoded(queryMap);
api.queryMap(queryMap);
assertThat(server.takeRequest())
.hasPath("/?name=%7Balice");

server.enqueue(new MockResponse());
queryMap = new LinkedHashMap<String, Object>();
queryMap.put("%7Bname", "%7Balice");
api.queryMapEncoded(queryMap);
api.queryMap(queryMap);
assertThat(server.takeRequest())
.hasPath("/?%7Bname=%7Balice");
}
Expand Down Expand Up @@ -971,9 +971,6 @@ void form(
@RequestLine("GET /")
void queryMap(@QueryMap Map<String, Object> queryMap);

@RequestLine("GET /")
void queryMapEncoded(@QueryMap(encoded = true) Map<String, Object> queryMap);

@RequestLine("GET /?name={name}")
void queryMapWithQueryParams(@Param("name") String name,
@QueryMap Map<String, Object> queryMap);
Expand Down