Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ public List<Namespace> listNamespaces(SessionContext context, Namespace namespac

Map<String, String> queryParams = Maps.newHashMap();
if (!namespace.isEmpty()) {
queryParams.put("parent", RESTUtil.NAMESPACE_JOINER.join(namespace.levels()));
queryParams.put("parent", RESTUtil.namespaceToQueryParam(namespace));
}

ImmutableList.Builder<Namespace> namespaces = ImmutableList.builder();
Expand Down
49 changes: 47 additions & 2 deletions core/src/main/java/org/apache/iceberg/rest/RESTUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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'.
*
* <p>{@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.
*
* <p>{@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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,7 @@ public <T extends RESTResponse> 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();
}
Expand Down
46 changes: 46 additions & 0 deletions core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
}
}