You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jclouds.apache.org by na...@apache.org on 2015/10/21 00:25:26 UTC
[2/2] jclouds git commit: JCLOUDS-1008: Add @Encoded to @QueryParam.
JCLOUDS-1008: Add @Encoded to @QueryParam.
Adds support for the @Encoded option for the @QueryParam annotation.
The @Encoded params are not encoded, while all parameters that don't
have it are encoded. The change applies to the @QueryParam annotation
on a single parameter. There is no way to express @Encoded on the list
of parameters and their values in @QueryParams.
The big change is that query parameter encoding is now handled within
the annotation processor, as opposed to relying on the UriBuilder to
perform the encoding. This is required since the UriBuilder does not
have any information about additional annotations associated with each
of the query parameters.
Also, adds unit tests for making sure keys and values are properly
encoded when using the @QueryParams option.
Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo
Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/5bbcb834
Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/5bbcb834
Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/5bbcb834
Branch: refs/heads/master
Commit: 5bbcb8342f9eb5aadcb99885931891e5a8362255
Parents: 94b3cba
Author: Timur Alperovich <ti...@gmail.com>
Authored: Fri Oct 16 01:48:58 2015 -0700
Committer: Ignasi Barrera <na...@apache.org>
Committed: Wed Oct 21 00:06:23 2015 +0200
----------------------------------------------------------------------
core/src/main/java/org/jclouds/http/Uris.java | 19 ++--
.../rest/internal/RestAnnotationProcessor.java | 53 +++++++-----
.../internal/RestAnnotationProcessorTest.java | 91 ++++++++++++++++++++
3 files changed, 136 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jclouds/blob/5bbcb834/core/src/main/java/org/jclouds/http/Uris.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/http/Uris.java b/core/src/main/java/org/jclouds/http/Uris.java
index bf8449b..441db62 100644
--- a/core/src/main/java/org/jclouds/http/Uris.java
+++ b/core/src/main/java/org/jclouds/http/Uris.java
@@ -282,9 +282,9 @@ public final class Uris {
return build(ImmutableMap.<String, Object> of());
}
- public URI buildNoEncoding(Map<String, ?> variables) {
+ public URI build(Map<String, ?> variables, boolean encodePath, boolean encodeQuery) {
try {
- return new URI(expand(variables, false));
+ return new URI(expand(variables, encodePath, encodeQuery));
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
@@ -296,13 +296,13 @@ public final class Uris {
*/
public URI build(Map<String, ?> variables) {
try {
- return new URI(expand(variables, true));
+ return new URI(expand(variables, true, true));
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}
- private String expand(Map<String, ?> variables, boolean shouldEncode) {
+ private String expand(Map<String, ?> variables, boolean encodePath, boolean encodeQuery) {
StringBuilder b = new StringBuilder();
if (scheme != null)
b.append(scheme).append("://");
@@ -311,14 +311,19 @@ public final class Uris {
if (port != null)
b.append(':').append(port);
if (path != null) {
- if (shouldEncode) {
+ if (encodePath) {
b.append(urlEncode(UriTemplates.expand(path, variables), skipPathEncoding));
} else {
b.append(UriTemplates.expand(path, variables));
}
}
- if (!query.isEmpty())
- b.append('?').append(encodeQueryLine(query));
+ if (!query.isEmpty()) {
+ if (encodeQuery) {
+ b.append('?').append(encodeQueryLine(query));
+ } else {
+ b.append('?').append(buildQueryLine(query));
+ }
+ }
return b.toString();
}
http://git-wip-us.apache.org/repos/asf/jclouds/blob/5bbcb834/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
index e1c7468..113e9dc 100644
--- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
+++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
@@ -40,6 +40,7 @@ import static org.jclouds.http.Uris.uriBuilder;
import static org.jclouds.io.Payloads.newPayload;
import static org.jclouds.reflect.Reflection2.getInvokableParameters;
import static org.jclouds.util.Strings2.replaceTokens;
+import static org.jclouds.util.Strings2.urlEncode;
import java.lang.annotation.Annotation;
import java.lang.reflect.Array;
@@ -102,7 +103,6 @@ import org.jclouds.rest.annotations.VirtualHost;
import org.jclouds.rest.annotations.WrapWith;
import org.jclouds.rest.binders.BindMapToStringPayload;
import org.jclouds.rest.binders.BindToJsonPayloadWrappedWith;
-import org.jclouds.util.Strings2;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
@@ -231,10 +231,10 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
overridePathEncoding(uriBuilder, invocation);
- boolean encodedParams = isEncodedUsed(invocation);
+ boolean encodeFullPath = !isEncodedUsed(invocation);
if (caller != null)
- tokenValues.putAll(addPathAndGetTokens(caller, uriBuilder, encodedParams));
- tokenValues.putAll(addPathAndGetTokens(invocation, uriBuilder, encodedParams));
+ tokenValues.putAll(addPathAndGetTokens(caller, uriBuilder, encodeFullPath));
+ tokenValues.putAll(addPathAndGetTokens(invocation, uriBuilder, encodeFullPath));
Multimap<String, Object> formParams;
if (caller != null) {
formParams = addFormParams(tokenValues, caller);
@@ -270,7 +270,8 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
headers.put(header.getKey(), replaceTokens(header.getValue(), tokenValues));
}
for (Entry<String, String> query : options.buildQueryParameters().entries()) {
- queryParams.put(query.getKey(), replaceTokens(query.getValue(), tokenValues));
+ queryParams.put(urlEncode(query.getKey(), '/', ','),
+ urlEncode(replaceTokens(query.getValue(), tokenValues), '/', ','));
}
for (Entry<String, String> form : options.buildFormParameters().entries()) {
formParams.put(form.getKey(), replaceTokens(form.getValue(), tokenValues));
@@ -291,11 +292,9 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
requestBuilder.headers(filterOutContentHeaders(headers));
- if (encodedParams) {
- requestBuilder.endpoint(uriBuilder.buildNoEncoding(convertUnsafe(tokenValues)));
- } else {
- requestBuilder.endpoint(uriBuilder.build(convertUnsafe(tokenValues)));
- }
+ // Query parameter encoding is handled in the annotation processor
+ requestBuilder.endpoint(uriBuilder.build(convertUnsafe(tokenValues), /*encodePath=*/encodeFullPath,
+ /*encodeQuery=*/ false));
if (payload == null) {
PayloadEnclosing payloadEnclosing = findOrNull(invocation.getArgs(), PayloadEnclosing.class);
@@ -395,12 +394,13 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
return endpoint;
}
- private Multimap<String, Object> addPathAndGetTokens(Invocation invocation, UriBuilder uriBuilder, boolean encoded) {
+ private Multimap<String, Object> addPathAndGetTokens(Invocation invocation, UriBuilder uriBuilder,
+ boolean encodeFullPath) {
if (invocation.getInvokable().getOwnerType().getRawType().isAnnotationPresent(Path.class))
uriBuilder.appendPath(invocation.getInvokable().getOwnerType().getRawType().getAnnotation(Path.class).value());
if (invocation.getInvokable().isAnnotationPresent(Path.class))
uriBuilder.appendPath(invocation.getInvokable().getAnnotation(Path.class).value());
- return getPathParamKeyValues(invocation, encoded);
+ return getPathParamKeyValues(invocation, encodeFullPath);
}
private Multimap<String, Object> addFormParams(Multimap<String, ?> tokenValues, Invocation invocation) {
@@ -452,11 +452,12 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
private void addQuery(Multimap<String, Object> queryParams, QueryParams query, Multimap<String, ?> tokenValues) {
for (int i = 0; i < query.keys().length; i++) {
+ String key = urlEncode(query.keys()[i], '/', ',');
if (query.values()[i].equals(QueryParams.NULL)) {
- queryParams.removeAll(query.keys()[i]);
- queryParams.put(query.keys()[i], null);
+ queryParams.removeAll(key);
+ queryParams.put(key, null);
} else {
- queryParams.put(query.keys()[i], replaceTokens(query.values()[i], tokenValues));
+ queryParams.put(key, urlEncode(replaceTokens(query.values()[i], tokenValues), '/', ','));
}
}
}
@@ -768,7 +769,7 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
return !parametersWithAnnotation(invocation.getInvokable(), Encoded.class).isEmpty();
}
- private Multimap<String, Object> getPathParamKeyValues(Invocation invocation, boolean encodedParams) {
+ private Multimap<String, Object> getPathParamKeyValues(Invocation invocation, boolean encodeFullPath) {
Multimap<String, Object> pathParamValues = LinkedHashMultimap.create();
for (Parameter param : parametersWithAnnotation(invocation.getInvokable(), PathParam.class)) {
PathParam pathParam = param.getAnnotation(PathParam.class);
@@ -776,8 +777,8 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
Optional<?> paramValue = getParamValue(invocation, param.getAnnotation(ParamParser.class), param.hashCode(),
paramKey);
if (paramValue.isPresent()) {
- if (encodedParams && !param.isAnnotationPresent(Encoded.class)) {
- pathParamValues.put(paramKey, Strings2.urlEncode(paramValue.get().toString()));
+ if (!encodeFullPath && !param.isAnnotationPresent(Encoded.class)) {
+ pathParamValues.put(paramKey, urlEncode(paramValue.get().toString()));
} else {
pathParamValues.put(paramKey, paramValue.get().toString());
}
@@ -821,16 +822,28 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
Multimap<String, Object> queryParamValues = LinkedHashMultimap.create();
for (Parameter param : parametersWithAnnotation(invocation.getInvokable(), QueryParam.class)) {
QueryParam queryParam = param.getAnnotation(QueryParam.class);
- String paramKey = queryParam.value();
+ String paramKey = urlEncode(queryParam.value(), '/', ',');
Optional<?> paramValue = getParamValue(invocation, param.getAnnotation(ParamParser.class), param.hashCode(),
paramKey);
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, '/', ',');
+ }
+ });
+ }
queryParamValues.putAll(paramKey, iterableStrings);
} else {
- queryParamValues.put(paramKey, paramValue.get().toString());
+ String value = paramValue.get().toString();
+ if (!param.isAnnotationPresent(Encoded.class)) {
+ value = urlEncode(value, '/', ',');
+ }
+ queryParamValues.put(paramKey, value);
}
}
return queryParamValues;
http://git-wip-us.apache.org/repos/asf/jclouds/blob/5bbcb834/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
index 95bed02..6a9dd73 100644
--- a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
+++ b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java
@@ -24,6 +24,7 @@ import static org.jclouds.io.Payloads.newInputStreamPayload;
import static org.jclouds.io.Payloads.newStringPayload;
import static org.jclouds.providers.AnonymousProviderMetadata.forApiOnEndpoint;
import static org.jclouds.reflect.Reflection2.method;
+import static org.jclouds.util.Strings2.urlEncode;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
@@ -483,6 +484,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
@Path("/")
public void queryParamIterable(@Nullable @QueryParam("foo") Iterable<String> bars) {
}
+
+ @FOO
+ @Path("/")
+ @QueryParams(keys = { "test param"}, values = { "foo bar" })
+ public void queryKeyEncoded() {
+ }
}
public void testQuery() throws SecurityException, NoSuchMethodException {
@@ -575,6 +582,30 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
assertEquals(request.getMethod(), "FOO");
}
+ public void testQueryEncodedKey() throws SecurityException, NoSuchMethodException {
+ GeneratedHttpRequest request = processor.apply(Invocation.create(method(TestQuery.class, "queryKeyEncoded")));
+ assertEquals(request.getEndpoint().getHost(), "localhost");
+ assertEquals(request.getEndpoint().getPath(), "/");
+ assertEquals(request.getEndpoint().getRawQuery(), "x-ms-version=2009-07-17&test%20param=foo%20bar");
+ assertEquals(request.getMethod(), "FOO");
+ }
+
+ @QueryParams(keys = "test%param", values = "percent%")
+ public class TestInterfaceQueryParam {
+ @FOO
+ @Path("/")
+ public void query() {
+ }
+ }
+
+ public void testInterfaceEncodedKey() throws SecurityException, NoSuchMethodException {
+ GeneratedHttpRequest request = processor.apply(Invocation.create(method(TestInterfaceQueryParam.class, "query")));
+ assertEquals(request.getEndpoint().getHost(), "localhost");
+ assertEquals(request.getEndpoint().getPath(), "/");
+ assertEquals(request.getEndpoint().getRawQuery(), "test%25param=percent%25");
+ assertEquals(request.getMethod(), "FOO");
+ }
+
interface TestPayloadParamVarargs {
@POST
void varargs(HttpRequestOptions... options);
@@ -663,6 +694,16 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
assertPayloadEquals(request, "fooya", "application/unknown", false);
}
+ public void testQueryVarargsEncoding() throws Exception {
+ Invokable<?, ?> method = method(TestPayloadParamVarargs.class, "varargsWithReq", String.class,
+ HttpRequestOptions[].class);
+ GeneratedHttpRequest request = processor.apply(
+ Invocation.create(method,
+ ImmutableList.<Object> of("required param",
+ new TestHttpRequestOptions().queryParams(ImmutableMultimap.of("key", "foo bar")))));
+ assertRequestLineEquals(request, "POST http://localhost:9999?key=foo%20bar HTTP/1.1");
+ }
+
public void testDuplicateHeaderAndQueryVarargs() throws Exception {
Invokable<?, ?> method = method(TestPayloadParamVarargs.class, "varargs",
HttpRequestOptions[].class);
@@ -1556,6 +1597,21 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
public void oneQueryParamExtractor(@QueryParam("one") @ParamParser(FirstCharacter.class) String one) {
}
+ @GET
+ @Path("/")
+ public void oneQueryParam(@QueryParam("one") String one) {
+ }
+
+ @GET
+ @Path("/")
+ public void encodedQueryParam(@QueryParam("encoded") @Encoded String encoded) {
+ }
+
+ @GET
+ @Path("/")
+ public void encodedQueryListParam(@QueryParam("encoded") @Encoded List<String> encodedStrings) {
+ }
+
@POST
@Path("/")
public void oneFormParamExtractor(@FormParam("one") @ParamParser(FirstCharacter.class) String one) {
@@ -1609,6 +1665,41 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
}
@Test
+ public void testEncodedQueryParam() throws Exception {
+ Invokable<?, ?> method = method(TestPath.class, "encodedQueryParam", String.class);
+ GeneratedHttpRequest request = processor.apply(Invocation.create(method,
+ ImmutableList.<Object> of("foo%20bar")));
+ assertRequestLineEquals(request, "GET http://localhost:9999/?encoded=foo%20bar HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "");
+ assertPayloadEquals(request, null, null, false);
+
+ method = method(TestPath.class, "encodedQueryListParam", List.class);
+ String [] args = {"foo%20bar", "foo/bar"};
+ request = processor.apply(Invocation.create(method,
+ ImmutableList.<Object> of(ImmutableList.of("foo%20bar", "foo/bar"))));
+ assertRequestLineEquals(request, "GET http://localhost:9999/?encoded=foo%20bar&encoded=foo/bar HTTP/1.1");
+ assertNonPayloadHeadersEqual(request, "");
+ assertPayloadEquals(request, null, null, false);
+ }
+
+ @DataProvider(name = "queryStrings")
+ public Object[][] createQueryData() {
+ return new Object[][] { { "normal" }, { "sp ace" }, { "qu?stion" }, { "unic₪de" }, { "path/foo" }, { "colon:" },
+ { "asteri*k" }, { "quote\"" }, { "great<r" }, { "lesst>en" }, { "p|pe" } };
+ }
+
+ @Test(dataProvider = "queryStrings")
+ public void testQueryParam(String val) {
+ Invokable<?, ?> method = method(TestPath.class, "oneQueryParam", String.class);
+ GeneratedHttpRequest request = processor.apply(Invocation.create(method,
+ ImmutableList.<Object> of(val)));
+ assertRequestLineEquals(request, String.format("GET http://localhost:9999/?one=%s HTTP/1.1",
+ urlEncode(val, '/', ',')));
+ assertNonPayloadHeadersEqual(request, "");
+ assertPayloadEquals(request, null, null, false);
+ }
+
+ @Test
public void testFormParamExtractor() throws Exception {
Invokable<?, ?> method = method(TestPath.class, "oneFormParamExtractor", String.class);
GeneratedHttpRequest request = processor.apply(Invocation.create(method,