Repository: jclouds Updated Branches: refs/heads/master e0c959c21 -> 5bbcb8342
JCLOUDS-1008: Add @Encoded annotation. Certain providers (e.g. Google Cloud Storage) place tokens that should be encoded in the request path (e.g. GET http://<host>/b/<bucket>/o/<object>) and expect them to be percent-encoded. In the above example a GET request for "foo/bar" should be translated to http://<host>/b/<bucket>/o/foo%2Fbar. Currently, there is no way to express this in jclouds, as the entire request path is encoded exactly once and there is no control over whether a request parameter should be handled specially. In the example above, "/" are not encoded in the path and the URL is submitted as "http://<host>/b/<bucket>/o/foo/bar", which may be wrong. This patch extends the annotation processor to support @Encoded for the individual parameters of the request. However, this means that the entire path is _NOT_ URL encoded. The caller *must* make sure that the appropriate parameters are encoded -- ones that are marked with the @Encoded annotation. Parameters not marked with the @Encoded annotation are URI encoded prior to being added to the path. This means that "/" characters will also be URI encoded in this case (i.e. "foo/bar" is turned into "foo%2Fbar"). For the Google Storage provider, we will annotate the parameters that are going to be pre-encoded (object names) and ensure the provider encodes them prior to calling the API (separate patch in jclouds-labs-google). Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/94b3cba6 Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/94b3cba6 Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/94b3cba6 Branch: refs/heads/master Commit: 94b3cba6c254aaf8d67e633d2fe46b3c4c30a2c2 Parents: e0c959c Author: Timur Alperovich <[email protected]> Authored: Mon Sep 28 16:32:15 2015 -0700 Committer: Ignasi Barrera <[email protected]> Committed: Wed Oct 21 00:06:15 2015 +0200 ---------------------------------------------------------------------- core/src/main/java/org/jclouds/http/Uris.java | 21 ++++++++++--- .../rest/internal/RestAnnotationProcessor.java | 32 +++++++++++++++----- .../internal/RestAnnotationProcessorTest.java | 27 +++++++++++++++++ 3 files changed, 68 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/94b3cba6/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 facc181..bf8449b 100644 --- a/core/src/main/java/org/jclouds/http/Uris.java +++ b/core/src/main/java/org/jclouds/http/Uris.java @@ -282,19 +282,27 @@ public final class Uris { return build(ImmutableMap.<String, Object> of()); } + public URI buildNoEncoding(Map<String, ?> variables) { + try { + return new URI(expand(variables, false)); + } catch (URISyntaxException e) { + throw new IllegalArgumentException(e); + } + } + /** * @throws IllegalArgumentException * if there's a problem parsing the URI */ public URI build(Map<String, ?> variables) { try { - return new URI(expand(variables)); + return new URI(expand(variables, true)); } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } } - private String expand(Map<String, ?> variables) { + private String expand(Map<String, ?> variables, boolean shouldEncode) { StringBuilder b = new StringBuilder(); if (scheme != null) b.append(scheme).append("://"); @@ -302,8 +310,13 @@ public final class Uris { b.append(UriTemplates.expand(host, variables)); if (port != null) b.append(':').append(port); - if (path != null) - b.append(urlEncode(UriTemplates.expand(path, variables), skipPathEncoding)); + if (path != null) { + if (shouldEncode) { + b.append(urlEncode(UriTemplates.expand(path, variables), skipPathEncoding)); + } else { + b.append(UriTemplates.expand(path, variables)); + } + } if (!query.isEmpty()) b.append('?').append(encodeQueryLine(query)); return b.toString(); http://git-wip-us.apache.org/repos/asf/jclouds/blob/94b3cba6/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 f6ebc8c..e1c7468 100644 --- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java +++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java @@ -54,6 +54,7 @@ import java.util.Set; import javax.annotation.Resource; import javax.inject.Named; +import javax.ws.rs.Encoded; import javax.ws.rs.FormParam; import javax.ws.rs.HeaderParam; import javax.ws.rs.Path; @@ -101,6 +102,7 @@ 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; @@ -229,9 +231,10 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest overridePathEncoding(uriBuilder, invocation); + boolean encodedParams = isEncodedUsed(invocation); if (caller != null) - tokenValues.putAll(addPathAndGetTokens(caller, uriBuilder)); - tokenValues.putAll(addPathAndGetTokens(invocation, uriBuilder)); + tokenValues.putAll(addPathAndGetTokens(caller, uriBuilder, encodedParams)); + tokenValues.putAll(addPathAndGetTokens(invocation, uriBuilder, encodedParams)); Multimap<String, Object> formParams; if (caller != null) { formParams = addFormParams(tokenValues, caller); @@ -288,7 +291,11 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest requestBuilder.headers(filterOutContentHeaders(headers)); - requestBuilder.endpoint(uriBuilder.build(convertUnsafe(tokenValues))); + if (encodedParams) { + requestBuilder.endpoint(uriBuilder.buildNoEncoding(convertUnsafe(tokenValues))); + } else { + requestBuilder.endpoint(uriBuilder.build(convertUnsafe(tokenValues))); + } if (payload == null) { PayloadEnclosing payloadEnclosing = findOrNull(invocation.getArgs(), PayloadEnclosing.class); @@ -388,12 +395,12 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest return endpoint; } - private Multimap<String, Object> addPathAndGetTokens(Invocation invocation, UriBuilder uriBuilder) { + private Multimap<String, Object> addPathAndGetTokens(Invocation invocation, UriBuilder uriBuilder, boolean encoded) { 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); + return getPathParamKeyValues(invocation, encoded); } private Multimap<String, Object> addFormParams(Multimap<String, ?> tokenValues, Invocation invocation) { @@ -757,15 +764,24 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest return parts.build(); } - private Multimap<String, Object> getPathParamKeyValues(Invocation invocation) { + private boolean isEncodedUsed(Invocation invocation) { + return !parametersWithAnnotation(invocation.getInvokable(), Encoded.class).isEmpty(); + } + + private Multimap<String, Object> getPathParamKeyValues(Invocation invocation, boolean encodedParams) { Multimap<String, Object> pathParamValues = LinkedHashMultimap.create(); for (Parameter param : parametersWithAnnotation(invocation.getInvokable(), PathParam.class)) { PathParam pathParam = param.getAnnotation(PathParam.class); String paramKey = pathParam.value(); Optional<?> paramValue = getParamValue(invocation, param.getAnnotation(ParamParser.class), param.hashCode(), paramKey); - if (paramValue.isPresent()) - pathParamValues.put(paramKey, paramValue.get().toString()); + if (paramValue.isPresent()) { + if (encodedParams && !param.isAnnotationPresent(Encoded.class)) { + pathParamValues.put(paramKey, Strings2.urlEncode(paramValue.get().toString())); + } else { + pathParamValues.put(paramKey, paramValue.get().toString()); + } + } } return pathParamValues; } http://git-wip-us.apache.org/repos/asf/jclouds/blob/94b3cba6/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 7e43551..95bed02 100644 --- a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java +++ b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java @@ -55,6 +55,7 @@ import javax.inject.Named; import javax.inject.Qualifier; import javax.inject.Singleton; import javax.ws.rs.Consumes; +import javax.ws.rs.Encoded; import javax.ws.rs.FormParam; import javax.ws.rs.GET; import javax.ws.rs.HeaderParam; @@ -1525,6 +1526,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { } @GET + @Path("/{paramA}/{paramB}/{paramC}") + public void encodedParams(@PathParam("paramA") @Encoded String a, @PathParam("paramB") String b, + @PathParam("paramC") @Encoded String c) { + } + + @GET @Path("/{path}") public void onePathNullable(@Nullable @PathParam("path") String path) { } @@ -1572,6 +1579,26 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { } @Test + public void testPathParamEncoding() throws Exception { + Invokable<?, ?> method = method(TestPath.class, "onePath", String.class); + // By default, "/" should not be encoded + GeneratedHttpRequest request = processor.apply(Invocation.create(method, + ImmutableList.<Object> of("foo/bar"))); + assertRequestLineEquals(request, "GET http://localhost:9999/foo/bar HTTP/1.1"); + + // If we pass an encoded string, it should be encoded twice + request = processor.apply(Invocation.create(method, ImmutableList.<Object> of("foo%2Fbar"))); + assertRequestLineEquals(request, "GET http://localhost:9999/foo%252Fbar HTTP/1.1"); + + // If we pass in a pre-encoded param, it should not be double encoded + method = method(TestPath.class, "encodedParams", String.class, String.class, String.class); + request = processor.apply(Invocation.create(method, ImmutableList.<Object>of("encoded%2Fparam", "encode%2Fdouble", + "foo%20bar"))); + assertRequestLineEquals(request, + "GET http://localhost:9999/encoded%2Fparam/encode%252Fdouble/foo%20bar HTTP/1.1"); + } + + @Test public void testQueryParamExtractor() throws Exception { Invokable<?, ?> method = method(TestPath.class, "oneQueryParamExtractor", String.class); GeneratedHttpRequest request = processor.apply(Invocation.create(method,
