From d578a3f32042bda18bb50aa3c50eeb0a9d642f30 Mon Sep 17 00:00:00 2001 From: David Goss Date: Wed, 29 Nov 2023 18:10:50 +0000 Subject: [PATCH] change url path encoding to match jersey decoding (#2693) Signed-off-by: David Goss --- .../java/marquez/client/MarquezPathV1.java | 10 ++------ .../java/marquez/client/MarquezHttpTest.java | 2 +- .../marquez/client/MarquezPathV1Test.java | 25 +++++++++++++------ 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/clients/java/src/main/java/marquez/client/MarquezPathV1.java b/clients/java/src/main/java/marquez/client/MarquezPathV1.java index 2ceba79f9e..7145a882f5 100644 --- a/clients/java/src/main/java/marquez/client/MarquezPathV1.java +++ b/clients/java/src/main/java/marquez/client/MarquezPathV1.java @@ -6,9 +6,7 @@ package marquez.client; import com.google.common.annotations.VisibleForTesting; -import java.io.UnsupportedEncodingException; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; +import com.google.common.net.UrlEscapers; import java.util.Arrays; import java.util.Iterator; import java.util.stream.Collectors; @@ -57,11 +55,7 @@ Converts path template (prepended with BASE_PATH), where path parts are separate } static String encode(String input) { - try { - return URLEncoder.encode(input, StandardCharsets.UTF_8.toString()); - } catch (UnsupportedEncodingException e) { - throw new MarquezClientException(e); - } + return UrlEscapers.urlPathSegmentEscaper().escape(input); } static String listNamespacesPath() { diff --git a/clients/java/src/test/java/marquez/client/MarquezHttpTest.java b/clients/java/src/test/java/marquez/client/MarquezHttpTest.java index 9eeef4ee4c..cded97388a 100644 --- a/clients/java/src/test/java/marquez/client/MarquezHttpTest.java +++ b/clients/java/src/test/java/marquez/client/MarquezHttpTest.java @@ -105,7 +105,7 @@ public void testClient_preservesBaseURL() throws Exception { public void testClient_properlyFormatsNamespaces() throws Exception { final String pathTemplate = "/namespaces/%s"; final String pathArg = "database://localhost:1234"; - final String path = "/namespaces/database%3A%2F%2Flocalhost%3A1234"; + final String path = "/namespaces/database:%2F%2Flocalhost:1234"; URL expected = new URL(BASE_URL + BASE_PATH + path); URL actual = marquezUrl.from(path(pathTemplate, pathArg)); diff --git a/clients/java/src/test/java/marquez/client/MarquezPathV1Test.java b/clients/java/src/test/java/marquez/client/MarquezPathV1Test.java index f7076ccc30..336882c956 100644 --- a/clients/java/src/test/java/marquez/client/MarquezPathV1Test.java +++ b/clients/java/src/test/java/marquez/client/MarquezPathV1Test.java @@ -23,17 +23,26 @@ void testPath_namespaceUrl(String expected, String namespaceName) { private static Stream testPath_namespaceUrl() { return Stream.of( - Arguments.of("/api/v1/namespaces/s3%3A%2F%2Fbucket", "s3://bucket"), - Arguments.of("/api/v1/namespaces/bigquery%3A", "bigquery:"), + Arguments.of("/api/v1/namespaces/s3:%2F%2Fbucket", "s3://bucket"), + Arguments.of("/api/v1/namespaces/bigquery:", "bigquery:"), Arguments.of("/api/v1/namespaces/usual-namespace-name", "usual-namespace-name"), - Arguments.of("/api/v1/namespaces/a%3A%5C%3Aa", "a:\\:a")); + Arguments.of("/api/v1/namespaces/a:%5C:a", "a:\\:a")); } - @Test - void testPath_datasetUrl() { - Assertions.assertEquals( - "/api/v1/namespaces/s3%3A%2F%2Fbuckets/datasets/source-file.json", - MarquezPathV1.datasetPath("s3://buckets", "source-file.json")); + @ParameterizedTest + @MethodSource + void testPath_datasetUrl(String expected, String namespaceName, String datasetName) { + Assertions.assertEquals(expected, MarquezPathV1.datasetPath(namespaceName, datasetName)); + } + + private static Stream testPath_datasetUrl() { + return Stream.of( + Arguments.of( + "/api/v1/namespaces/s3:%2F%2Fbucket/datasets/source-file.json", + "s3://bucket", "source-file.json"), + Arguments.of( + "/api/v1/namespaces/snowflake:%2F%2Faccount/datasets/DATABASE.SCHEMA.%22Exotic%20Table%20Name!%22", + "snowflake://account", "DATABASE.SCHEMA.\"Exotic Table Name!\"")); } @Test