Move QueryParam encoding to a separate class.

The patch implements a QueryValue class, which encodes the underlying
value based on whether the "encoded" flag is set. This class is used
by the RestAnnotationProcessor to propagate the @Encoded value set on
any parameters.

Since the encoding is now handled by the QueryValue instances, we
should no longer call encodeQueryLine() in the URI builder and instead
call buildQueryLine(). The caveat is that we need to make sure all of
the parameters that may need to be encoded are converted to QueryValue
objects. This is done by converting Object instances to QueryValue by
an instance of the TransformObjectToQueryValue when adding any query
parameters to the URI.
This commit is contained in:
Timur Alperovich 2015-10-26 11:26:00 -07:00 committed by Ignasi Barrera
parent 9df30c5a09
commit 1fc9b0e259
4 changed files with 135 additions and 49 deletions

View File

@ -20,7 +20,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Strings.emptyToNull;
import static com.google.common.collect.Multimaps.forMap;
import static org.jclouds.http.utils.Queries.buildQueryLine;
import static org.jclouds.http.utils.Queries.encodeQueryLine;
import static org.jclouds.http.utils.Queries.queryParser;
import static org.jclouds.util.Strings2.urlDecode;
import static org.jclouds.util.Strings2.urlEncode;
@ -30,14 +29,18 @@ import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.Map;
import org.jclouds.http.utils.QueryValue;
import org.jclouds.javax.annotation.Nullable;
import com.google.common.annotations.Beta;
import com.google.common.base.Function;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.collect.Multimaps;
/**
* Functions on {@code String}s and {@link URI}s. Strings can be level 1 <a
@ -93,6 +96,8 @@ public final class Uris {
*
*/
public static final class UriBuilder {
private static final TransformObjectToQueryValue QUERY_VALUE_TRANSFORMER = new TransformObjectToQueryValue();
// colon for urns, semicolon & equals for matrix params
private Iterable<Character> skipPathEncoding = Lists.charactersOf("/:;=");
private String scheme;
@ -160,14 +165,16 @@ public final class Uris {
}
public UriBuilder query(Multimap<String, ?> parameters) {
checkNotNull(parameters, "parameters");
Multimap<String, QueryValue> queryValueMultimap = Multimaps.transformValues(
checkNotNull(parameters, "parameters"), QUERY_VALUE_TRANSFORMER);
query.clear();
query.putAll(parameters);
query.putAll(queryValueMultimap);
return this;
}
public UriBuilder addQuery(String name, Iterable<?> values) {
query.putAll(checkNotNull(name, "name"), checkNotNull(values, "values of %s", name));
query.putAll(checkNotNull(name, "name"), Iterables.transform(checkNotNull(values, "values of %s", name),
QUERY_VALUE_TRANSFORMER));
return this;
}
@ -176,12 +183,16 @@ public final class Uris {
}
public UriBuilder addQuery(Multimap<String, ?> parameters) {
query.putAll(checkNotNull(parameters, "parameters"));
Multimap<String, QueryValue> queryValueMultimap = Multimaps.transformValues(
checkNotNull(parameters, "parameters"), QUERY_VALUE_TRANSFORMER);
query.putAll(queryValueMultimap);
return this;
}
public UriBuilder replaceQuery(String name, Iterable<?> values) {
query.replaceValues(checkNotNull(name, "name"), checkNotNull(values, "values of %s", name));
Iterable<QueryValue> queryValues = Iterables.transform(checkNotNull(values, "values of %s", name),
QUERY_VALUE_TRANSFORMER);
query.replaceValues(checkNotNull(name, "name"), queryValues);
return this;
}
@ -282,9 +293,9 @@ public final class Uris {
return build(ImmutableMap.<String, Object> of());
}
public URI build(Map<String, ?> variables, boolean encodePath, boolean encodeQuery) {
public URI build(Map<String, ?> variables, boolean encodePath) {
try {
return new URI(expand(variables, encodePath, encodeQuery));
return new URI(expand(variables, encodePath));
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
@ -296,13 +307,13 @@ public final class Uris {
*/
public URI build(Map<String, ?> variables) {
try {
return new URI(expand(variables, true, true));
return new URI(expand(variables, true));
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}
private String expand(Map<String, ?> variables, boolean encodePath, boolean encodeQuery) {
private String expand(Map<String, ?> variables, boolean encodePath) {
StringBuilder b = new StringBuilder();
if (scheme != null)
b.append(scheme).append("://");
@ -318,11 +329,7 @@ public final class Uris {
}
}
if (!query.isEmpty()) {
if (encodeQuery) {
b.append('?').append(encodeQueryLine(query));
} else {
b.append('?').append(buildQueryLine(query));
}
b.append('?').append(buildQueryLine(query));
}
return b.toString();
}
@ -388,4 +395,17 @@ public final class Uris {
return new StringBuilder().append('/').append(in).toString();
return in;
}
private static class TransformObjectToQueryValue implements Function<Object, QueryValue> {
@Override
public QueryValue apply(Object o) {
if (o == null) {
return null;
}
if (o instanceof QueryValue) {
return (QueryValue) o;
}
return new QueryValue(o.toString(), false);
}
}
}

View File

@ -0,0 +1,62 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jclouds.http.utils;
import org.jclouds.util.Strings2;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
public class QueryValue implements Comparable {
private final boolean encoded;
private final Object value;
private final Iterable<Character> skipChars;
public QueryValue(Object value, boolean encoded) {
this.value = value;
this.encoded = encoded;
this.skipChars = ImmutableList.of('/', ',');
}
@Override
public String toString() {
if (!encoded) {
return Strings2.urlEncode(value.toString(), skipChars);
}
return value.toString();
}
@Override
public boolean equals(Object other) {
if (other instanceof QueryValue) {
// Compare the resulting string, as opposed to whether the string is encoded or not
return this.toString().equals(other.toString());
} else {
return false;
}
}
@Override
public int hashCode() {
return Objects.hashCode(this.toString());
}
@Override
public int compareTo(Object o) {
return this.toString().compareTo(o.toString());
}
}

View File

@ -45,6 +45,7 @@ import static org.jclouds.util.Strings2.urlEncode;
import java.lang.annotation.Annotation;
import java.lang.reflect.Array;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.List;
@ -71,6 +72,7 @@ import org.jclouds.http.Uris.UriBuilder;
import org.jclouds.http.filters.ConnectionCloseHeader;
import org.jclouds.http.filters.StripExpectHeader;
import org.jclouds.http.options.HttpRequestOptions;
import org.jclouds.http.utils.QueryValue;
import org.jclouds.io.ContentMetadataCodec;
import org.jclouds.io.Payload;
import org.jclouds.io.PayloadEnclosing;
@ -271,7 +273,7 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
}
for (Entry<String, String> query : options.buildQueryParameters().entries()) {
queryParams.put(urlEncode(query.getKey(), '/', ','),
urlEncode(replaceTokens(query.getValue(), tokenValues), '/', ','));
new QueryValue(replaceTokens(query.getValue(), tokenValues), false));
}
for (Entry<String, String> form : options.buildFormParameters().entries()) {
formParams.put(form.getKey(), replaceTokens(form.getValue(), tokenValues));
@ -293,8 +295,7 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
requestBuilder.headers(filterOutContentHeaders(headers));
// Query parameter encoding is handled in the annotation processor
requestBuilder.endpoint(uriBuilder.build(convertUnsafe(tokenValues), /*encodePath=*/encodeFullPath,
/*encodeQuery=*/ false));
requestBuilder.endpoint(uriBuilder.build(convertUnsafe(tokenValues), /*encodePath=*/encodeFullPath));
if (payload == null) {
PayloadEnclosing payloadEnclosing = findOrNull(invocation.getArgs(), PayloadEnclosing.class);
@ -433,8 +434,8 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
addQuery(queryMap, query, tokenValues);
}
for (Entry<String, Object> query : getQueryParamKeyValues(invocation).entries()) {
queryMap.put(query.getKey(), replaceTokens(query.getValue().toString(), tokenValues));
for (Entry<String, Object> query : getQueryParamKeyValues(invocation, tokenValues).entries()) {
queryMap.put(query.getKey(), query.getValue());
}
return queryMap;
}
@ -457,7 +458,7 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
queryParams.removeAll(key);
queryParams.put(key, null);
} else {
queryParams.put(key, urlEncode(replaceTokens(query.values()[i], tokenValues), '/', ','));
queryParams.put(key, new QueryValue(replaceTokens(query.values()[i], tokenValues), false));
}
}
}
@ -818,32 +819,26 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
return formParamValues;
}
private Multimap<String, Object> getQueryParamKeyValues(Invocation invocation) {
private Multimap<String, Object> getQueryParamKeyValues(Invocation invocation, Multimap<String, ?> tokenValues) {
Multimap<String, Object> queryParamValues = LinkedHashMultimap.create();
for (Parameter param : parametersWithAnnotation(invocation.getInvokable(), QueryParam.class)) {
QueryParam queryParam = param.getAnnotation(QueryParam.class);
String paramKey = urlEncode(queryParam.value(), '/', ',');
Optional<?> paramValue = getParamValue(invocation, param.getAnnotation(ParamParser.class), param.hashCode(),
paramKey);
boolean encoded = param.isAnnotationPresent(Encoded.class);
if (paramValue.isPresent())
if (paramValue.get() instanceof Iterable) {
@SuppressWarnings("unchecked")
Iterable<String> iterableStrings = transform(Iterable.class.cast(paramValue.get()), toStringFunction());
if (!param.isAnnotationPresent(Encoded.class)) {
iterableStrings = transform(iterableStrings, new Function<String, String>() {
@Override
public String apply(@Nullable String s) {
return urlEncode(s, '/', ',');
}
});
List<QueryValue> values = new ArrayList<QueryValue>();
for (String stringValue : iterableStrings) {
values.add(new QueryValue(replaceTokens(stringValue, tokenValues), encoded));
}
queryParamValues.putAll(paramKey, iterableStrings);
queryParamValues.putAll(paramKey, values);
} else {
String value = paramValue.get().toString();
if (!param.isAnnotationPresent(Encoded.class)) {
value = urlEncode(value, '/', ',');
}
queryParamValues.put(paramKey, value);
queryParamValues.put(paramKey, new QueryValue(replaceTokens(value, tokenValues), encoded));
}
}
return queryParamValues;

View File

@ -16,6 +16,7 @@
*/
package org.jclouds.http;
import static org.assertj.core.api.Assertions.assertThat;
import static org.jclouds.http.Uris.uriBuilder;
import static org.jclouds.util.Strings2.urlEncode;
import static org.testng.Assert.assertEquals;
@ -67,18 +68,26 @@ public class UrisTest {
@Test(dataProvider = "strings")
public void testQuery(String val) {
assertEquals(uriBuilder("https://foo.com:8080").addQuery("moo", val).toString(), "https://foo.com:8080?moo=" + val);
assertEquals(uriBuilder("https://foo.com:8080").addQuery("moo", val).build().toString(), "https://foo.com:8080?moo="
+ urlEncode(val, '/', ','));
assertEquals(uriBuilder("https://api.github.com/repos/user?foo=bar&kung=fu").addQuery("moo", val).toString(),
"https://api.github.com/repos/user?foo=bar&kung=fu&moo=" + val);
assertEquals(uriBuilder("https://api.github.com/repos/user?foo=bar&kung=fu").addQuery("moo", val).build().toString(),
"https://api.github.com/repos/user?foo=bar&kung=fu&moo=" + urlEncode(val, '/', ','));
assertEquals(uriBuilder("https://api.github.com/repos/{user}").addQuery("moo", val).toString(),
"https://api.github.com/repos/{user}?moo=" + val);
assertEquals(
uriBuilder("https://api.github.com/repos/{user}").addQuery("moo", val).build(templateParams).toASCIIString(),
"https://api.github.com/repos/bob?moo=" + urlEncode(val, '/', ','));
assertThat(uriBuilder("https://foo.com:8080").addQuery("moo", val).build().getQuery())
.isEqualTo("moo=" + val);
assertThat(uriBuilder("https://foo.com:8080").addQuery("moo", val).build().getRawQuery())
.isEqualTo("moo=" + urlEncode(val, '/', ','));
assertThat(uriBuilder("https://api.github.com/repos/user?foo=bar&kung=fu").addQuery("moo", val).build()
.getQuery())
.isEqualTo("foo=bar&kung=fu&moo=" + val);
assertThat(uriBuilder("https://api.github.com/repos/user?foo=bar&kung=fu").addQuery("moo", val).build()
.getRawQuery())
.isEqualTo("foo=bar&kung=fu&moo=" + urlEncode(val, '/', ','));
assertThat(uriBuilder("https://api.github.com/repos/{user}").addQuery("moo", val).build().getQuery())
.isEqualTo("moo=" + val);
assertThat(uriBuilder("https://api.github.com/repos/{user}").addQuery("moo", val).toString())
.isEqualTo("https://api.github.com/repos/{user}?moo=" + urlEncode(val, '/', ','));
assertThat(uriBuilder("https://api.github.com/repos/{user}").addQuery("moo", val).build(templateParams)
.getRawQuery())
.isEqualTo("moo=" + urlEncode(val, '/', ','));
assertThat(uriBuilder("https://api.github.com/repos/{user}").addQuery("moo", val).build(templateParams)
.getPath())
.isEqualTo("/repos/bob");
}
@Test(dataProvider = "strings")
@ -125,9 +134,9 @@ public class UrisTest {
@Test(dataProvider = "strings")
public void testReplaceQueryIsEncoded(String key) {
assertEquals(uriBuilder("/redirect").addQuery("foo", key).toString(), "/redirect?foo=" + key);
assertEquals(uriBuilder("/redirect").addQuery("foo", key).build().toString(),
"/redirect?foo=" + urlEncode(key, '/', ','));
assertThat(uriBuilder("/redirect").addQuery("foo", key).build().getQuery()).isEqualTo("foo=" + key);
assertThat(uriBuilder("/redirect").addQuery("foo", key).build().getRawQuery())
.isEqualTo("foo=" + urlEncode(key, '/', ','));
}
public void testAddQuery() {