diff --git a/CHANGELOG.md b/CHANGELOG.md index a8b555b9..4d6a7dcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,17 @@ ## [Unreleased] +### Changed + +- Changed [LookupQueryInfo](src/main/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfo.java) + Any custom implementation of this interface that aims to provide path-based requests is able to provide + the lookup query url with parameters surrounded by curly brackets. For example the supplied + URL `http://service/{customerId}`, will result in the lookup parameter `customerId` value being used + in the url. + ### Fixed - - Moved junit support to junit 5, allowing junits to be run against flink 1.17 and 1.18. + +- Moved junit support to junit 5, allowing junits to be run against flink 1.17 and 1.18. ## [0.12.0] - 2024-03-22 diff --git a/src/main/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactory.java b/src/main/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactory.java index 3dd7c2d7..b005931d 100644 --- a/src/main/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactory.java +++ b/src/main/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactory.java @@ -16,7 +16,7 @@ /** * Implementation of {@link HttpRequestFactory} for REST calls that sends their parameters using - * request body. + * request body or in the path. */ @Slf4j public class BodyBasedRequestFactory extends RequestFactoryBase { @@ -43,7 +43,7 @@ public BodyBasedRequestFactory( @Override protected Builder setUpRequestMethod(LookupQueryInfo lookupQueryInfo) { return HttpRequest.newBuilder() - .uri(constructBodyBasedUri(lookupQueryInfo)) + .uri(constructUri(lookupQueryInfo)) .method(methodName, BodyPublishers.ofString(lookupQueryInfo.getLookupQuery())) .timeout(Duration.ofSeconds(this.httpRequestTimeOutSeconds)); } @@ -53,12 +53,13 @@ protected Logger getLogger() { return log; } - URI constructBodyBasedUri(LookupQueryInfo lookupQueryInfo) { + URI constructUri(LookupQueryInfo lookupQueryInfo) { StringBuilder resolvedUrl = new StringBuilder(baseUrl); if (lookupQueryInfo.hasBodyBasedUrlQueryParameters()) { resolvedUrl.append(baseUrl.contains("?") ? "&" : "?") .append(lookupQueryInfo.getBodyBasedUrlQueryParameters()); } + resolvedUrl = resolvePathParameters(lookupQueryInfo, resolvedUrl); try { return new URIBuilder(resolvedUrl.toString()).build(); @@ -66,4 +67,5 @@ URI constructBodyBasedUri(LookupQueryInfo lookupQueryInfo) { throw new RuntimeException(e); } } + } diff --git a/src/main/java/com/getindata/connectors/http/internal/table/lookup/GetRequestFactory.java b/src/main/java/com/getindata/connectors/http/internal/table/lookup/GetRequestFactory.java index 3b649ecc..d5f2811c 100644 --- a/src/main/java/com/getindata/connectors/http/internal/table/lookup/GetRequestFactory.java +++ b/src/main/java/com/getindata/connectors/http/internal/table/lookup/GetRequestFactory.java @@ -60,7 +60,7 @@ URI constructGetUri(LookupQueryInfo lookupQueryInfo) { resolvedUrl.append(baseUrl.contains("?") ? "&" : "?") .append(lookupQueryInfo.getLookupQuery()); } - + resolvedUrl = resolvePathParameters(lookupQueryInfo, resolvedUrl); try { return new URIBuilder(resolvedUrl.toString()).build(); } catch (URISyntaxException e) { diff --git a/src/main/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfo.java b/src/main/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfo.java index 9c63b2d5..f2c759d9 100644 --- a/src/main/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfo.java +++ b/src/main/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfo.java @@ -12,6 +12,7 @@ import com.getindata.connectors.http.internal.utils.uri.NameValuePair; import com.getindata.connectors.http.internal.utils.uri.URLEncodedUtils; + /** * Holds the lookup query for an HTTP request. * The {@code lookupQuery} either contain the query parameters for a GET operation @@ -26,15 +27,20 @@ public class LookupQueryInfo implements Serializable { private final Map bodyBasedUrlQueryParams; + private final Map pathBasedUrlParams; + public LookupQueryInfo(String lookupQuery) { - this(lookupQuery, null); + this(lookupQuery, null, null); } - public LookupQueryInfo(String lookupQuery, Map bodyBasedUrlQueryParams) { + public LookupQueryInfo(String lookupQuery, Map bodyBasedUrlQueryParams, + Map pathBasedUrlParams) { this.lookupQuery = lookupQuery == null ? "" : lookupQuery; this.bodyBasedUrlQueryParams = bodyBasedUrlQueryParams == null ? Collections.emptyMap() : bodyBasedUrlQueryParams; + this.pathBasedUrlParams = + pathBasedUrlParams == null ? Collections.emptyMap() : pathBasedUrlParams; } public String getBodyBasedUrlQueryParameters() { @@ -42,16 +48,26 @@ public String getBodyBasedUrlQueryParameters() { bodyBasedUrlQueryParams .entrySet() .stream() + // sort the map by key to ensure there is a reliable order for unit tests + .sorted(Map.Entry.comparingByKey()) .map(entry -> new NameValuePair(entry.getKey(), entry.getValue())) .collect(Collectors.toList()), StandardCharsets.UTF_8); } + public Map getPathBasedUrlParameters() { + return pathBasedUrlParams; + } + public boolean hasLookupQuery() { return !lookupQuery.isBlank(); } + public boolean hasBodyBasedUrlQueryParameters() { return !bodyBasedUrlQueryParams.isEmpty(); } + public boolean hasPathBasedUrlParameters() { + return !pathBasedUrlParams.isEmpty(); + } } diff --git a/src/main/java/com/getindata/connectors/http/internal/table/lookup/RequestFactoryBase.java b/src/main/java/com/getindata/connectors/http/internal/table/lookup/RequestFactoryBase.java index 54efe115..f6c19f62 100644 --- a/src/main/java/com/getindata/connectors/http/internal/table/lookup/RequestFactoryBase.java +++ b/src/main/java/com/getindata/connectors/http/internal/table/lookup/RequestFactoryBase.java @@ -3,9 +3,11 @@ import java.net.http.HttpRequest; import java.net.http.HttpRequest.Builder; import java.util.Arrays; +import java.util.Map; import org.apache.flink.annotation.VisibleForTesting; import org.apache.flink.table.data.RowData; +import org.apache.flink.util.FlinkRuntimeException; import org.slf4j.Logger; import com.getindata.connectors.http.LookupQueryCreator; @@ -82,6 +84,24 @@ public HttpLookupSourceRequestEntry buildLookupRequest(RowData lookupRow) { */ protected abstract Builder setUpRequestMethod(LookupQueryInfo lookupQuery); + protected static StringBuilder resolvePathParameters(LookupQueryInfo lookupQueryInfo, + StringBuilder resolvedUrl) { + if (lookupQueryInfo.hasPathBasedUrlParameters()) { + for (Map.Entry entry : + lookupQueryInfo.getPathBasedUrlParameters().entrySet()) { + String pathParam = "{" + entry.getKey() + "}"; + int startIndex = resolvedUrl.indexOf(pathParam); + if (startIndex == -1) { + throw new FlinkRuntimeException( + "Unexpected error while parsing the URL for path parameters."); + } + int endIndex = startIndex + pathParam.length(); + resolvedUrl = resolvedUrl.replace(startIndex, endIndex, entry.getValue()); + } + } + return resolvedUrl; + } + @VisibleForTesting String[] getHeadersAndValues() { return Arrays.copyOf(headersAndValues, headersAndValues.length); diff --git a/src/test/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactoryTest.java b/src/test/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactoryTest.java new file mode 100644 index 00000000..756af45f --- /dev/null +++ b/src/test/java/com/getindata/connectors/http/internal/table/lookup/BodyBasedRequestFactoryTest.java @@ -0,0 +1,124 @@ +package com.getindata.connectors.http.internal.table.lookup; + +import java.net.URI; +import java.util.Collection; +import java.util.Map; + +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.testcontainers.shaded.com.google.common.collect.ImmutableList; +import static org.assertj.core.api.Assertions.assertThat; + +public class BodyBasedRequestFactoryTest { + + @ParameterizedTest + @MethodSource("configProvider") + void testconstructUri(TestSpec testSpec) throws Exception { + LookupQueryInfo lookupQueryInfo = new LookupQueryInfo(testSpec.url, + testSpec.bodyBasedUrlQueryParams, + testSpec.pathBasedUrlParams); + HttpLookupConfig httpLookupConfig = HttpLookupConfig.builder() + .lookupMethod(testSpec.lookupMethod) + .url(testSpec.url) + .useAsync(false) + .build(); + BodyBasedRequestFactory bodyBasedRequestFactory = + new BodyBasedRequestFactory("test", null, null, httpLookupConfig); + + URI uri = bodyBasedRequestFactory.constructUri(lookupQueryInfo); + assertThat(uri.toString()).isEqualTo(testSpec.expected); + } + + private static class TestSpec { + + Map bodyBasedUrlQueryParams; + Map pathBasedUrlParams; + String url; + String lookupMethod; + String expected; + + private TestSpec(Map bodyBasedUrlQueryParams, + Map pathBasedUrlParams, + String url, + String lookupMethod, + String expected) { + this.bodyBasedUrlQueryParams = bodyBasedUrlQueryParams; + this.pathBasedUrlParams = pathBasedUrlParams; + this.url = url; + this.lookupMethod = lookupMethod; + this.expected = expected; + } + + @Override + public String toString() { + return "TestSpec{" + + "bodyBasedUrlQueryParams=" + + bodyBasedUrlQueryParams + + ", pathBasedUrlParams=" + + pathBasedUrlParams + + ", url=" + + url + + ", lookupMethod=" + + lookupMethod + + ", expected=" + + expected + + '}'; + } + } + + static Collection configProvider() { + return ImmutableList.builder() + .addAll(getTestSpecs("GET")) + .addAll(getTestSpecs("POST")) + .build(); + } + + @NotNull + private static ImmutableList getTestSpecs(String lookupMethod) { + return ImmutableList.of( + // 1 path param + new TestSpec( + null, + Map. of("param1", "value1"), + "http://service/{param1}", + lookupMethod, + "http://service/value1"), + // 2 path param + new TestSpec( + null, + Map. of("param1", "value1", "param2", "value2"), + "http://service/{param1}/param2/{param2}", + lookupMethod, + "http://service/value1/param2/value2"), + // 1 query param + new TestSpec( + Map. of("param3", "value3"), + null, + "http://service", + lookupMethod, + "http://service?param3=value3"), + // 1 query param with a parameter on base url + new TestSpec( + Map. of("param3", "value3"), + null, + "http://service?extrakey=extravalue", + lookupMethod, + "http://service?extrakey=extravalue¶m3=value3"), + // 2 query params + new TestSpec( + Map. of("param3", "value3", "param4", "value4"), + null, + "http://service", + lookupMethod, + "http://service?param3=value3¶m4=value4"), + // 2 query params and 2 path params + new TestSpec( + Map. of("param3", "value3", "param4", "value4"), + Map. of("param1", "value1", "param2", "value2"), + "http://service/{param1}/param2/{param2}", + lookupMethod, + "http://service/value1/param2/value2?param3=value3¶m4=value4") + ); + } +} diff --git a/src/test/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientTest.java b/src/test/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientTest.java index 4eb03244..6d6feac1 100644 --- a/src/test/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientTest.java +++ b/src/test/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientTest.java @@ -165,7 +165,8 @@ public void shouldBuildBodyBasedClientUri() { urlBodyBasedQueryParameters.put("key1", "value1"); urlBodyBasedQueryParameters.put("key2", "value2"); - LookupQueryInfo lookupQueryInfo = new LookupQueryInfo("{}", urlBodyBasedQueryParameters); + LookupQueryInfo lookupQueryInfo = new LookupQueryInfo("{}", + urlBodyBasedQueryParameters, null); // WHEN HttpRequest httpRequest = requestFactory.setUpRequestMethod(lookupQueryInfo).build(); diff --git a/src/test/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfoTest.java b/src/test/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfoTest.java index 148598f9..b5fa2d49 100644 --- a/src/test/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfoTest.java +++ b/src/test/java/com/getindata/connectors/http/internal/table/lookup/LookupQueryInfoTest.java @@ -14,7 +14,7 @@ public void testConfiguredLookupQuery() { String lookupQuery = "{\"param1\": \"value1\"}"; Map bodyBasedUrlQueryParameters = Map.of("key1", "value1"); - lookupQueryInfo = new LookupQueryInfo(lookupQuery, bodyBasedUrlQueryParameters); + lookupQueryInfo = new LookupQueryInfo(lookupQuery, bodyBasedUrlQueryParameters, null); assertThat(lookupQueryInfo.hasLookupQuery()).isTrue(); assertThat(lookupQueryInfo.getLookupQuery()).isEqualTo(lookupQuery); @@ -23,7 +23,7 @@ public void testConfiguredLookupQuery() { } @Test public void testEmptyLookupQueryInfo() { - lookupQueryInfo = new LookupQueryInfo(null, null); + lookupQueryInfo = new LookupQueryInfo(null, null, null); assertThat(lookupQueryInfo.hasLookupQuery()).isFalse(); assertThat(lookupQueryInfo.hasBodyBasedUrlQueryParameters()).isFalse(); @@ -32,13 +32,57 @@ public void testEmptyLookupQueryInfo() { } @Test - public void testBodyBasedUrlQueryParams() { + public void test1BodyParam() { Map bodyBasedUrlQueryParameters = Map.of("key1", "value1"); - lookupQueryInfo = new LookupQueryInfo(null, bodyBasedUrlQueryParameters); + lookupQueryInfo = new LookupQueryInfo(null, bodyBasedUrlQueryParameters, null); assertThat(lookupQueryInfo.hasLookupQuery()).isFalse(); assertThat(lookupQueryInfo.hasBodyBasedUrlQueryParameters()).isTrue(); assertThat(lookupQueryInfo.getBodyBasedUrlQueryParameters()).isEqualTo("key1=value1"); } + + @Test + public void test1PathParam() { + Map pathBasedUrlPathParameters = Map.of("key1", "value1"); + + lookupQueryInfo = new LookupQueryInfo("http://service/{key1}", + null, pathBasedUrlPathParameters); + + assertThat(lookupQueryInfo.hasLookupQuery()).isTrue(); + assertThat(lookupQueryInfo.hasPathBasedUrlParameters()).isTrue(); + assertThat(lookupQueryInfo.getPathBasedUrlParameters()) + .isEqualTo(pathBasedUrlPathParameters); + } + @Test + public void test2Path2BodyParams() { + Map pathBasedUrlPathParameters = + Map.of("key1", "value1", "key2", "value2"); + Map bodyBasedQueryParameters = + Map.of("key3", "value3", "key4", "value4"); + + lookupQueryInfo = new LookupQueryInfo(null, + bodyBasedQueryParameters, pathBasedUrlPathParameters); + + assertThat(lookupQueryInfo.hasLookupQuery()).isFalse(); + assertThat(lookupQueryInfo.hasPathBasedUrlParameters()).isTrue(); + assertThat(lookupQueryInfo.getPathBasedUrlParameters()) + .isEqualTo(pathBasedUrlPathParameters); + assertThat(lookupQueryInfo.hasBodyBasedUrlQueryParameters()) + .isTrue(); + assertThat(lookupQueryInfo.getBodyBasedUrlQueryParameters()) + .isEqualTo("key3=value3&key4=value4"); + } + + @Test + public void test2PathParams() { + Map pathBasedUrlPathParameters = Map.of("key1", "value1", "key2", "value2"); + + lookupQueryInfo = new LookupQueryInfo(null, null, pathBasedUrlPathParameters); + + assertThat(lookupQueryInfo.hasLookupQuery()).isFalse(); + assertThat(lookupQueryInfo.hasPathBasedUrlParameters()).isTrue(); + assertThat(lookupQueryInfo.getPathBasedUrlParameters()) + .isEqualTo(pathBasedUrlPathParameters); + } }