diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java index 4692b92b51be..b903f13adc09 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java @@ -570,7 +570,7 @@ public List listNamespaces(SessionContext context, Namespace namespac Map queryParams = Maps.newHashMap(); if (!namespace.isEmpty()) { - queryParams.put("parent", RESTUtil.NAMESPACE_JOINER.join(namespace.levels())); + queryParams.put("parent", RESTUtil.namespaceToQueryParam(namespace)); } ImmutableList.Builder namespaces = ImmutableList.builder(); diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java b/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java index 01b57a415117..d31260a918fd 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java @@ -32,13 +32,23 @@ public class RESTUtil { private static final char NAMESPACE_SEPARATOR = '\u001f'; - public static final Joiner NAMESPACE_JOINER = Joiner.on(NAMESPACE_SEPARATOR); - public static final Splitter NAMESPACE_SPLITTER = Splitter.on(NAMESPACE_SEPARATOR); private static final String NAMESPACE_ESCAPED_SEPARATOR = "%1F"; private static final Joiner NAMESPACE_ESCAPED_JOINER = Joiner.on(NAMESPACE_ESCAPED_SEPARATOR); private static final Splitter NAMESPACE_ESCAPED_SPLITTER = Splitter.on(NAMESPACE_ESCAPED_SEPARATOR); + /** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * RESTUtil#namespaceToQueryParam(Namespace)}} instead. + */ + @Deprecated public static final Joiner NAMESPACE_JOINER = Joiner.on(NAMESPACE_SEPARATOR); + + /** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * RESTUtil#namespaceFromQueryParam(String)} instead. + */ + @Deprecated public static final Splitter NAMESPACE_SPLITTER = Splitter.on(NAMESPACE_SEPARATOR); + private RESTUtil() {} public static String stripTrailingSlash(String path) { @@ -160,6 +170,41 @@ public static String decodeString(String encoded) { return URLDecoder.decode(encoded, StandardCharsets.UTF_8); } + /** + * This converts the given namespace to a string and separates each part in a multipart namespace + * using the unicode character '\u001f'. Note that this method is different from {@link + * RESTUtil#encodeNamespace(Namespace)}, which uses the UTF-8 escaped version of '\u001f', which + * is '0x1F'. + * + *

{@link #namespaceFromQueryParam(String)} should be used to convert the namespace string back + * to a {@link Namespace} instance. + * + * @param namespace The namespace to convert + * @return The namespace converted to a string where each part in a multipart namespace is + * separated using the unicode character '\u001f' + */ + public static String namespaceToQueryParam(Namespace namespace) { + Preconditions.checkArgument(null != namespace, "Invalid namespace: null"); + return RESTUtil.NAMESPACE_JOINER.join(namespace.levels()); + } + + /** + * This converts a namespace where each part in a multipart namespace has been separated using + * '\u001f' to its original {@link Namespace} instance. + * + *

{@link #namespaceToQueryParam(Namespace)} should be used to convert the {@link Namespace} to + * a string. + * + * @param namespace The namespace to convert + * @return The namespace instance from the given namespace string, where each multipart separator + * ('\u001f') is converted to a separate namespace level + */ + public static Namespace namespaceFromQueryParam(String namespace) { + Preconditions.checkArgument(null != namespace, "Invalid namespace: null"); + return Namespace.of( + RESTUtil.NAMESPACE_SPLITTER.splitToStream(namespace).toArray(String[]::new)); + } + /** * Returns a String representation of a namespace that is suitable for use in a URL / URI. * diff --git a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java index 35ef6dac35c1..675aa8ab6f46 100644 --- a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java +++ b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java @@ -314,11 +314,7 @@ public T handleRequest( if (asNamespaceCatalog != null) { Namespace ns; if (vars.containsKey("parent")) { - ns = - Namespace.of( - RESTUtil.NAMESPACE_SPLITTER - .splitToStream(vars.get("parent")) - .toArray(String[]::new)); + ns = RESTUtil.namespaceFromQueryParam(vars.get("parent")); } else { ns = Namespace.empty(); } diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java index 6eb8f59bf6f8..fe022b351822 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Map; import org.apache.iceberg.catalog.Namespace; @@ -166,4 +167,49 @@ public void testRelativeEndpointPath() { RESTUtil.resolveEndpoint("http://catalog-uri/", "relative-endpoint/refresh-endpoint")) .isEqualTo("http://catalog-uri/relative-endpoint/refresh-endpoint"); } + + @Test + public void namespaceToQueryParam() { + assertThatThrownBy(() -> RESTUtil.namespaceToQueryParam(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid namespace: null"); + + assertThat(RESTUtil.namespaceToQueryParam(Namespace.empty())).isEqualTo(""); + assertThat(RESTUtil.namespaceToQueryParam(Namespace.of(""))).isEqualTo(""); + assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("ns"))).isEqualTo("ns"); + + // verify that the unicode character (\001f) and not its UTF-8 escaped version (%1F) is used + assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("one", "ns"))) + .isEqualTo("one\u001fns") + .isNotEqualTo("one%1Fns"); + assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("one", "two", "ns"))) + .isEqualTo("one\u001ftwo\u001fns") + .isNotEqualTo("one%1Ftwo%1Fns"); + assertThat(RESTUtil.namespaceToQueryParam(Namespace.of("one.two", "three.four", "ns"))) + .isEqualTo("one.two\u001fthree.four\u001fns") + .isNotEqualTo("one.two%1Fthree.four%1Fns"); + } + + @Test + public void namespaceFromQueryParam() { + assertThatThrownBy(() -> RESTUtil.namespaceFromQueryParam(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid namespace: null"); + + assertThat(RESTUtil.namespaceFromQueryParam("")).isEqualTo(Namespace.of("")); + assertThat(RESTUtil.namespaceFromQueryParam("ns")).isEqualTo(Namespace.of("ns")); + assertThat(RESTUtil.namespaceFromQueryParam("one\u001fns")) + .isEqualTo(Namespace.of("one", "ns")); + assertThat(RESTUtil.namespaceFromQueryParam("one\u001ftwo\u001fns")) + .isEqualTo(Namespace.of("one", "two", "ns")); + assertThat(RESTUtil.namespaceFromQueryParam("one.two\u001fthree.four\u001fns")) + .isEqualTo(Namespace.of("one.two", "three.four", "ns")); + + // using the UTF-8 escaped version will produce a wrong namespace instance + assertThat(RESTUtil.namespaceFromQueryParam("one%1Fns")).isEqualTo(Namespace.of("one%1Fns")); + assertThat(RESTUtil.namespaceFromQueryParam("one%1Ftwo%1Fns")) + .isEqualTo(Namespace.of("one%1Ftwo%1Fns")); + assertThat(RESTUtil.namespaceFromQueryParam("one%1Ftwo\u001fns")) + .isEqualTo(Namespace.of("one%1Ftwo", "ns")); + } }