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/11/02 17:50:11 UTC
jclouds git commit: Move QueryParam encoding to a separate class.
Repository: jclouds
Updated Branches:
refs/heads/master 9df30c5a0 -> 1fc9b0e25
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.
Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo
Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/1fc9b0e2
Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/1fc9b0e2
Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/1fc9b0e2
Branch: refs/heads/master
Commit: 1fc9b0e2597ee0e11d8ae09db478c2dbf516912b
Parents: 9df30c5
Author: Timur Alperovich <ti...@gmail.com>
Authored: Mon Oct 26 11:26:00 2015 -0700
Committer: Ignasi Barrera <na...@apache.org>
Committed: Mon Nov 2 17:11:42 2015 +0100
----------------------------------------------------------------------
core/src/main/java/org/jclouds/http/Uris.java | 50 +++++++++++-----
.../java/org/jclouds/http/utils/QueryValue.java | 62 ++++++++++++++++++++
.../rest/internal/RestAnnotationProcessor.java | 33 +++++------
.../test/java/org/jclouds/http/UrisTest.java | 39 +++++++-----
4 files changed, 135 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/jclouds/blob/1fc9b0e2/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 441db62..1beeec2 100644
--- a/core/src/main/java/org/jclouds/http/Uris.java
+++ b/core/src/main/java/org/jclouds/http/Uris.java
@@ -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);
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/jclouds/blob/1fc9b0e2/core/src/main/java/org/jclouds/http/utils/QueryValue.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/jclouds/http/utils/QueryValue.java b/core/src/main/java/org/jclouds/http/utils/QueryValue.java
new file mode 100644
index 0000000..969126b
--- /dev/null
+++ b/core/src/main/java/org/jclouds/http/utils/QueryValue.java
@@ -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());
+ }
+}
http://git-wip-us.apache.org/repos/asf/jclouds/blob/1fc9b0e2/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 113e9dc..6e0f047 100644
--- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
+++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java
@@ -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;
http://git-wip-us.apache.org/repos/asf/jclouds/blob/1fc9b0e2/core/src/test/java/org/jclouds/http/UrisTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/jclouds/http/UrisTest.java b/core/src/test/java/org/jclouds/http/UrisTest.java
index 2dbef5e..c129313 100644
--- a/core/src/test/java/org/jclouds/http/UrisTest.java
+++ b/core/src/test/java/org/jclouds/http/UrisTest.java
@@ -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() {