feature 2034: SdkHttpFullRequest builder.uri keep query parameters of…

… provided URI
aws · Oct 17, 2020 · 30660f4 · 30660f4
1 parent 367d973
commit 30660f4
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 7 deletions.
diff --git a/.changes/next-release/feature-HTTPclientspi-25d1ed9.json b/.changes/next-release/feature-HTTPclientspi-25d1ed9.json
@@ -0,0 +1,5 @@
+{
+    "category": "HTTP Client SPI",
+    "type": "feature",
+    "description": "Calling the SdkHttpFullRequest uri() builder method, query parameters of the provided URI will be kept.\nThis can be useful in case you want to provide an already fully formed URI like a callback URI."
+}
diff --git a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java
@@ -57,18 +57,24 @@ static SdkHttpFullRequest.Builder builder() {
      */
     interface Builder extends SdkHttpRequest.Builder {
         /**
-         * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, and
-         * {@link #encodedPath()} from a {@link URI} object.
+         * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()},
+         * {@link #encodedPath()} and extracts query parameters from a {@link URI} object.
          *
          * @param uri URI containing protocol, host, port and path.
          * @return This builder for method chaining.
          */
         @Override
         default Builder uri(URI uri) {
-            return this.protocol(uri.getScheme())
+            Builder builder =  this.protocol(uri.getScheme())
                        .host(uri.getHost())
                        .port(uri.getPort())
                        .encodedPath(SdkHttpUtils.appendUri(uri.getRawPath(), encodedPath()));
+            if (uri.getRawQuery() != null) {
+                builder.clearQueryParameters();
+                SdkHttpUtils.uriParams(uri)
+                            .forEach(this::putRawQueryParameter);
+            }
+            return builder;
         }
 
         /**

diff --git a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java
@@ -135,17 +135,23 @@ default URI getUri() {
      */
     interface Builder extends CopyableBuilder<Builder, SdkHttpRequest> {
         /**
-         * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, and
-         * {@link #encodedPath()} from a {@link URI} object.
+         * Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()},
+         * {@link #encodedPath()} and extracts query parameters from a {@link URI} object.
          *
          * @param uri URI containing protocol, host, port and path.
          * @return This builder for method chaining.
          */
         default Builder uri(URI uri) {
-            return this.protocol(uri.getScheme())
+            Builder builder = this.protocol(uri.getScheme())
                        .host(uri.getHost())
                        .port(uri.getPort())
                        .encodedPath(SdkHttpUtils.appendUri(uri.getRawPath(), encodedPath()));
+            if (uri.getRawQuery() != null) {
+                builder.clearQueryParameters();
+                SdkHttpUtils.uriParams(uri)
+                            .forEach(this::putRawQueryParameter);
+            }
+            return builder;
         }
 
         /**

diff --git a/http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java b/http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java
@@ -22,9 +22,10 @@
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.AbstractMap;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -401,13 +402,74 @@ public Map<String, List<String>> getMap() {
         });
     }
 
+    @Test
+    public void testSdkHttpFullRequestBuilderNoQueryParams() {
+        URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034");
+        final SdkHttpFullRequest sdkHttpFullRequest = SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
+        assertThat(sdkHttpFullRequest.getUri().getQuery()).isNullOrEmpty();
+    }
+
+    @Test
+    public void testSdkHttpFullRequestBuilderUriWithQueryParams() {
+        URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678");
+        final SdkHttpFullRequest sdkHttpFullRequest =
+            SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
+        assertThat(sdkHttpFullRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678");
+    }
+
+    @Test
+    public void testSdkHttpFullRequestBuilderUriWithQueryParamWithoutValue() {
+        final String expected = "https://github.com/aws/aws-sdk-for-java-v2?foo";
+        URI myUri = URI.create(expected);
+        SdkHttpFullRequest actual = SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build();
+        assertThat(actual.getUri()).hasToString(expected);
+    }
+
+    @Test
+    public void testSdkHttpRequestBuilderNoQueryParams() {
+        URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034");
+        final SdkHttpRequest sdkHttpRequest = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
+        assertThat(sdkHttpRequest.getUri().getQuery()).isNullOrEmpty();
+    }
+
+    @Test
+    public void testSdkHttpRequestBuilderUriWithQueryParams() {
+        URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678");
+        final SdkHttpRequest sdkHttpRequest =
+            SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
+        assertThat(sdkHttpRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678");
+    }
+
+    @Test
+    public void testSdkHttpRequestBuilderUriWithQueryParamsIgnoreOtherQueryParams() {
+        URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678");
+        final SdkHttpRequest sdkHttpRequest =
+            SdkHttpRequest.builder().method(SdkHttpMethod.POST).appendRawQueryParameter("clean", "up").uri(uri).build();
+        assertThat(sdkHttpRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678")
+                                                      .doesNotContain("clean");
+    }
+
+    @Test
+    public void testSdkHttpRequestBuilderUriWithQueryParamWithoutValue() {
+        final String expected = "https://github.com/aws/aws-sdk-for-java-v2?foo";
+        URI myUri = URI.create(expected);
+        SdkHttpRequest actual = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build();
+        assertThat(actual.getUri()).hasToString(expected);
+    }
+
     private interface BuilderProxy {
         BuilderProxy setValue(String key, String value);
+
         BuilderProxy appendValue(String key, String value);
+
         BuilderProxy setValues(String key, List<String> values);
+
         BuilderProxy removeValue(String key);
+
         BuilderProxy clearValues();
+
         BuilderProxy setMap(Map<String, List<String>> map);
+
         Map<String, List<String>> getMap();
     }
 

diff --git a/utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java b/utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java
@@ -15,6 +15,9 @@
 
 package software.amazon.awssdk.utils.http;
 
+import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.mapping;
+import static java.util.stream.Collectors.toList;
 import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
 
 import java.io.UnsupportedEncodingException;
@@ -30,6 +33,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.UnaryOperator;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
@@ -52,6 +56,9 @@ public final class SdkHttpUtils {
     private static final String[] ENCODED_CHARACTERS_WITHOUT_SLASHES = new String[] {"+", "*", "%7E"};
     private static final String[] ENCODED_CHARACTERS_WITHOUT_SLASHES_REPLACEMENTS = new String[] {"%20", "%2A", "~"};
 
+    private static final String QUERY_PARAM_DELIMITER_REGEX = "\\s*&\\s*";
+    private static final Pattern QUERY_PARAM_DELIMITER_PATTERN = Pattern.compile(QUERY_PARAM_DELIMITER_REGEX);
+
     // List of headers that may appear only once in a request; i.e. is not a list of values.
     // Taken from https://github.com/apache/httpcomponents-client/blob/81c1bc4dc3ca5a3134c5c60e8beff08be2fd8792/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java#L69-L85 with modifications:
     // removed: accept-ranges, if-match, if-none-match, vary since it looks like they're defined as lists
@@ -62,6 +69,7 @@ public final class SdkHttpUtils {
             "proxy-authorization", "range", "referer", "retry-after", "server", "user-agent")
             .collect(Collectors.toSet());
 
+
     private SdkHttpUtils() {
     }
 
@@ -322,4 +330,15 @@ public static Optional<String> firstMatchingHeaderFromCollection(Map<String, Lis
     public static boolean isSingleHeader(String h) {
         return SINGLE_HEADERS.contains(StringUtils.lowerCase(h));
     }
+
+    /**
+     * Extracts query parameters from the given URI
+     */
+    public static Map<String, List<String>> uriParams(URI uri) {
+        return QUERY_PARAM_DELIMITER_PATTERN
+                      .splitAsStream(uri.getRawQuery().trim())
+                      .map(s -> s.contains("=") ? s.split("=", 2) : new String[] {s, null})
+                      .collect(groupingBy(a -> urlDecode(a[0]), mapping(a -> urlDecode(a[1]), toList())));
+    }
+
 }
diff --git a/utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java b/utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java
@@ -19,7 +19,12 @@
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.entry;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -173,4 +178,46 @@ public void headersFromCollectionWorksCorrectly() {
         assertThat(SdkHttpUtils.firstMatchingHeaderFromCollection(headers, asList("foo", "nothing"))).hasValue("bar");
         assertThat(SdkHttpUtils.firstMatchingHeaderFromCollection(headers, asList("foo", "other"))).hasValue("foo");
     }
+
+    @Test
+    public void isSingleHeader() {
+        assertThat(SdkHttpUtils.isSingleHeader("age")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("authorization")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("content-length")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("content-location")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("content-md5")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("content-range")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("content-type")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("date")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("etag")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("expires")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("from")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("host")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("if-modified-since")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("if-range")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("if-unmodified-since")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("last-modified")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("location")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("max-forwards")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("proxy-authorization")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("range")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("referer")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("retry-after")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("server")).isTrue();
+        assertThat(SdkHttpUtils.isSingleHeader("user-agent")).isTrue();
+
+        assertThat(SdkHttpUtils.isSingleHeader("custom")).isFalse();
+    }
+
+    @Test
+    public void uriParams() throws URISyntaxException {
+        URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456&reqParam=5678&noval"
+                          + "&decoded%26Part=equals%3Dval");
+        Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);
+        assertThat(uriParams).contains(entry("reqParam", Arrays.asList("1234", "5678")),
+                                       entry("oParam", Collections.singletonList("3456")),
+                                       entry("noval", Arrays.asList((String)null)),
+                                       entry("decoded&Part", Arrays.asList("equals=val")));
+    }
+
 }