Skip to content

Commit c91e770

Browse files
roexberroexber
committed
feature 2034: SdkHttpFullRequest builder.uri keep query parameters of provided URI
1 parent 14a4017 commit c91e770

File tree

6 files changed

+152
-7
lines changed

6 files changed

+152
-7
lines changed

‎.changes/next-release/feature-HTTPclientspi-25d1ed9.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "HTTP Client SPI",
3+
"type": "feature",
4+
"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."
5+
}

‎http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpFullRequest.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,24 @@ static SdkHttpFullRequest.Builder builder() {
5757
*/
5858
interface Builder extends SdkHttpRequest.Builder {
5959
/**
60-
* Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, and
61-
* {@link #encodedPath()} from a {@link URI} object.
60+
* Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()},
61+
* {@link #encodedPath()} and extracts query parameters from a {@link URI} object.
6262
*
6363
* @param uri URI containing protocol, host, port and path.
6464
* @return This builder for method chaining.
6565
*/
6666
@Override
6767
default Builder uri(URI uri) {
68-
return this.protocol(uri.getScheme())
68+
Builder builder = this.protocol(uri.getScheme())
6969
.host(uri.getHost())
7070
.port(uri.getPort())
7171
.encodedPath(SdkHttpUtils.appendUri(uri.getRawPath(), encodedPath()));
72+
if (uri.getRawQuery() != null) {
73+
builder.clearQueryParameters();
74+
SdkHttpUtils.uriParams(uri)
75+
.forEach(this::putRawQueryParameter);
76+
}
77+
return builder;
7278
}
7379

7480
/**

‎http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,23 @@ default URI getUri() {
135135
*/
136136
interface Builder extends CopyableBuilder<Builder, SdkHttpRequest> {
137137
/**
138-
* Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()}, and
139-
* {@link #encodedPath()} from a {@link URI} object.
138+
* Convenience method to set the {@link #protocol()}, {@link #host()}, {@link #port()},
139+
* {@link #encodedPath()} and extracts query parameters from a {@link URI} object.
140140
*
141141
* @param uri URI containing protocol, host, port and path.
142142
* @return This builder for method chaining.
143143
*/
144144
default Builder uri(URI uri) {
145-
return this.protocol(uri.getScheme())
145+
Builder builder = this.protocol(uri.getScheme())
146146
.host(uri.getHost())
147147
.port(uri.getPort())
148148
.encodedPath(SdkHttpUtils.appendUri(uri.getRawPath(), encodedPath()));
149+
if (uri.getRawQuery() != null) {
150+
builder.clearQueryParameters();
151+
SdkHttpUtils.uriParams(uri)
152+
.forEach(this::putRawQueryParameter);
153+
}
154+
return builder;
149155
}
150156

151157
/**

‎http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@
2222
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2323
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2424

25+
import java.net.URI;
26+
import java.net.URISyntaxException;
2527
import java.util.AbstractMap;
2628
import java.util.Arrays;
27-
import java.util.Collections;
2829
import java.util.LinkedHashMap;
2930
import java.util.List;
3031
import java.util.Map;
@@ -401,13 +402,74 @@ public Map<String, List<String>> getMap() {
401402
});
402403
}
403404

405+
@Test
406+
public void testSdkHttpFullRequestBuilderNoQueryParams() {
407+
URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034");
408+
final SdkHttpFullRequest sdkHttpFullRequest = SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
409+
assertThat(sdkHttpFullRequest.getUri().getQuery()).isNullOrEmpty();
410+
}
411+
412+
@Test
413+
public void testSdkHttpFullRequestBuilderUriWithQueryParams() {
414+
URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678");
415+
final SdkHttpFullRequest sdkHttpFullRequest =
416+
SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
417+
assertThat(sdkHttpFullRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678");
418+
}
419+
420+
@Test
421+
public void testSdkHttpFullRequestBuilderUriWithQueryParamWithoutValue() {
422+
final String expected = "https://github.com/aws/aws-sdk-for-java-v2?foo";
423+
URI myUri = URI.create(expected);
424+
SdkHttpFullRequest actual = SdkHttpFullRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build();
425+
assertThat(actual.getUri()).hasToString(expected);
426+
}
427+
428+
@Test
429+
public void testSdkHttpRequestBuilderNoQueryParams() {
430+
URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034");
431+
final SdkHttpRequest sdkHttpRequest = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
432+
assertThat(sdkHttpRequest.getUri().getQuery()).isNullOrEmpty();
433+
}
434+
435+
@Test
436+
public void testSdkHttpRequestBuilderUriWithQueryParams() {
437+
URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678");
438+
final SdkHttpRequest sdkHttpRequest =
439+
SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(uri).build();
440+
assertThat(sdkHttpRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678");
441+
}
442+
443+
@Test
444+
public void testSdkHttpRequestBuilderUriWithQueryParamsIgnoreOtherQueryParams() {
445+
URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456%26reqParam%3D5678");
446+
final SdkHttpRequest sdkHttpRequest =
447+
SdkHttpRequest.builder().method(SdkHttpMethod.POST).appendRawQueryParameter("clean", "up").uri(uri).build();
448+
assertThat(sdkHttpRequest.getUri().getQuery()).contains("reqParam=1234", "oParam=3456&reqParam=5678")
449+
.doesNotContain("clean");
450+
}
451+
452+
@Test
453+
public void testSdkHttpRequestBuilderUriWithQueryParamWithoutValue() {
454+
final String expected = "https://github.com/aws/aws-sdk-for-java-v2?foo";
455+
URI myUri = URI.create(expected);
456+
SdkHttpRequest actual = SdkHttpRequest.builder().method(SdkHttpMethod.POST).uri(myUri).build();
457+
assertThat(actual.getUri()).hasToString(expected);
458+
}
459+
404460
private interface BuilderProxy {
405461
BuilderProxy setValue(String key, String value);
462+
406463
BuilderProxy appendValue(String key, String value);
464+
407465
BuilderProxy setValues(String key, List<String> values);
466+
408467
BuilderProxy removeValue(String key);
468+
409469
BuilderProxy clearValues();
470+
410471
BuilderProxy setMap(Map<String, List<String>> map);
472+
411473
Map<String, List<String>> getMap();
412474
}
413475

‎utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
package software.amazon.awssdk.utils.http;
1717

18+
import static java.util.stream.Collectors.groupingBy;
19+
import static java.util.stream.Collectors.mapping;
20+
import static java.util.stream.Collectors.toList;
1821
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
1922

2023
import java.io.UnsupportedEncodingException;
@@ -30,6 +33,7 @@
3033
import java.util.Optional;
3134
import java.util.Set;
3235
import java.util.function.UnaryOperator;
36+
import java.util.regex.Pattern;
3337
import java.util.stream.Collectors;
3438
import java.util.stream.Stream;
3539
import software.amazon.awssdk.annotations.SdkProtectedApi;
@@ -52,6 +56,9 @@ public final class SdkHttpUtils {
5256
private static final String[] ENCODED_CHARACTERS_WITHOUT_SLASHES = new String[] {"+", "*", "%7E"};
5357
private static final String[] ENCODED_CHARACTERS_WITHOUT_SLASHES_REPLACEMENTS = new String[] {"%20", "%2A", "~"};
5458

59+
private static final String QUERY_PARAM_DELIMITER_REGEX = "\\s*&\\s*";
60+
private static final Pattern QUERY_PARAM_DELIMITER_PATTERN = Pattern.compile(QUERY_PARAM_DELIMITER_REGEX);
61+
5562
// List of headers that may appear only once in a request; i.e. is not a list of values.
5663
// 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:
5764
// 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 {
6269
"proxy-authorization", "range", "referer", "retry-after", "server", "user-agent")
6370
.collect(Collectors.toSet());
6471

72+
6573
private SdkHttpUtils() {
6674
}
6775

@@ -322,4 +330,15 @@ public static Optional<String> firstMatchingHeaderFromCollection(Map<String, Lis
322330
public static boolean isSingleHeader(String h) {
323331
return SINGLE_HEADERS.contains(StringUtils.lowerCase(h));
324332
}
333+
334+
/**
335+
* Extracts query parameters from the given URI
336+
*/
337+
public static Map<String, List<String>> uriParams(URI uri) {
338+
return QUERY_PARAM_DELIMITER_PATTERN
339+
.splitAsStream(uri.getRawQuery().trim())
340+
.map(s -> s.contains("=") ? s.split("=", 2) : new String[] {s, null})
341+
.collect(groupingBy(a -> urlDecode(a[0]), mapping(a -> urlDecode(a[1]), toList())));
342+
}
343+
325344
}

‎utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@
1919
import static java.util.Collections.singletonList;
2020
import static org.assertj.core.api.Assertions.assertThat;
2121
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
22+
import static org.assertj.core.api.Assertions.entry;
2223

24+
import java.net.URI;
25+
import java.net.URISyntaxException;
26+
import java.util.ArrayList;
27+
import java.util.Arrays;
2328
import java.util.Collections;
2429
import java.util.HashMap;
2530
import java.util.LinkedHashMap;
@@ -173,4 +178,46 @@ public void headersFromCollectionWorksCorrectly() {
173178
assertThat(SdkHttpUtils.firstMatchingHeaderFromCollection(headers, asList("foo", "nothing"))).hasValue("bar");
174179
assertThat(SdkHttpUtils.firstMatchingHeaderFromCollection(headers, asList("foo", "other"))).hasValue("foo");
175180
}
181+
182+
@Test
183+
public void isSingleHeader() {
184+
assertThat(SdkHttpUtils.isSingleHeader("age")).isTrue();
185+
assertThat(SdkHttpUtils.isSingleHeader("authorization")).isTrue();
186+
assertThat(SdkHttpUtils.isSingleHeader("content-length")).isTrue();
187+
assertThat(SdkHttpUtils.isSingleHeader("content-location")).isTrue();
188+
assertThat(SdkHttpUtils.isSingleHeader("content-md5")).isTrue();
189+
assertThat(SdkHttpUtils.isSingleHeader("content-range")).isTrue();
190+
assertThat(SdkHttpUtils.isSingleHeader("content-type")).isTrue();
191+
assertThat(SdkHttpUtils.isSingleHeader("date")).isTrue();
192+
assertThat(SdkHttpUtils.isSingleHeader("etag")).isTrue();
193+
assertThat(SdkHttpUtils.isSingleHeader("expires")).isTrue();
194+
assertThat(SdkHttpUtils.isSingleHeader("from")).isTrue();
195+
assertThat(SdkHttpUtils.isSingleHeader("host")).isTrue();
196+
assertThat(SdkHttpUtils.isSingleHeader("if-modified-since")).isTrue();
197+
assertThat(SdkHttpUtils.isSingleHeader("if-range")).isTrue();
198+
assertThat(SdkHttpUtils.isSingleHeader("if-unmodified-since")).isTrue();
199+
assertThat(SdkHttpUtils.isSingleHeader("last-modified")).isTrue();
200+
assertThat(SdkHttpUtils.isSingleHeader("location")).isTrue();
201+
assertThat(SdkHttpUtils.isSingleHeader("max-forwards")).isTrue();
202+
assertThat(SdkHttpUtils.isSingleHeader("proxy-authorization")).isTrue();
203+
assertThat(SdkHttpUtils.isSingleHeader("range")).isTrue();
204+
assertThat(SdkHttpUtils.isSingleHeader("referer")).isTrue();
205+
assertThat(SdkHttpUtils.isSingleHeader("retry-after")).isTrue();
206+
assertThat(SdkHttpUtils.isSingleHeader("server")).isTrue();
207+
assertThat(SdkHttpUtils.isSingleHeader("user-agent")).isTrue();
208+
209+
assertThat(SdkHttpUtils.isSingleHeader("custom")).isFalse();
210+
}
211+
212+
@Test
213+
public void uriParams() throws URISyntaxException {
214+
URI uri = URI.create("https://github.com/aws/aws-sdk-java-v2/issues/2034?reqParam=1234&oParam=3456&reqParam=5678&noval"
215+
+ "&decoded%26Part=equals%3Dval");
216+
Map<String, List<String>> uriParams = SdkHttpUtils.uriParams(uri);
217+
assertThat(uriParams).contains(entry("reqParam", Arrays.asList("1234", "5678")),
218+
entry("oParam", Collections.singletonList("3456")),
219+
entry("noval", Arrays.asList((String)null)),
220+
entry("decoded&Part", Arrays.asList("equals=val")));
221+
}
222+
176223
}

0 commit comments

Comments
 (0)