nastra commented on code in PR #10753:
URL: https://github.com/apache/iceberg/pull/10753#discussion_r1876065276
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java:
##########
@@ -139,4 +148,102 @@ public void testOAuth2FormDataDecoding() {
assertThat(RESTUtil.decodeFormData(formString)).isEqualTo(expected);
}
+
+ @ParameterizedTest
+ @MethodSource("validRequestUris")
+ public void validRequestUris(HTTPRequest request, URI expected) {
+ assertThat(RESTUtil.buildRequestUri(request)).isEqualTo(expected);
+ }
+
+ public static Stream<Arguments> validRequestUris() {
+ return Stream.of(
+ Arguments.of(
+ ImmutableHTTPRequest.builder()
+ .baseUri(URI.create("http://localhost:8080/foo";))
+ .method(HTTPMethod.GET)
+ .path("v1/namespaces/ns/tables/") // trailing slash should be
removed
+ .putQueryParameter("pageToken", "1234")
+ .putQueryParameter("pageSize", "10")
+ .build(),
+ URI.create(
+
"http://localhost:8080/foo/v1/namespaces/ns/tables?pageToken=1234&pageSize=10";)),
+ Arguments.of(
+ ImmutableHTTPRequest.builder()
+ .baseUri(URI.create("http://localhost:8080/foo";))
+ .method(HTTPMethod.GET)
+ .path("https://authserver.com/token";) // absolute path HTTPS
+ .build(),
+ URI.create("https://authserver.com/token";)),
+ Arguments.of(
+ ImmutableHTTPRequest.builder()
+ .baseUri(URI.create("http://localhost:8080/foo";))
+ .method(HTTPMethod.GET)
+ .path("http://authserver.com/token";) // absolute path HTTP
+ .build(),
+ URI.create("http://authserver.com/token";)));
+ }
+
+ @Test
+ public void buildRequestUriFailures() {
+ HTTPRequest request =
+ ImmutableHTTPRequest.builder()
+ .baseUri(URI.create("http://localhost";))
+ .method(HTTPMethod.GET)
+ .path("/v1/namespaces") // wrong leading slash
+ .build();
+ assertThatThrownBy(() -> RESTUtil.buildRequestUri(request))
+ .isInstanceOf(RESTException.class)
+ .hasMessage(
+ "Received a malformed path for a REST request: /v1/namespaces.
Paths should not start with /");
+ HTTPRequest request2 =
+ ImmutableHTTPRequest.builder()
+ .baseUri(URI.create("http://localhost";))
+ .method(HTTPMethod.GET)
+ .path(" not a valid path") // wrong path
+ .build();
+ assertThatThrownBy(() -> RESTUtil.buildRequestUri(request2))
+ .isInstanceOf(RESTException.class)
+ .hasMessage(
+ "Failed to create request URI from base http://localhost/ not a
valid path, params {}");
+ }
+
+ @ParameterizedTest
+ @MethodSource
Review Comment:
this one should also explicitly point to the method being used
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
