Skip to content

Commit a4313e4

Browse files
nastranastra
committed
Core: Make namespace separator configurable
1 parent 5fc1413 commit a4313e4

File tree

6 files changed

+162
-24
lines changed

6 files changed

+162
-24
lines changed

‎core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
116116
private static final String DEFAULT_FILE_IO_IMPL = "org.apache.iceberg.io.ResolvingFileIO";
117117
private static final String REST_METRICS_REPORTING_ENABLED = "rest-metrics-reporting-enabled";
118118
private static final String REST_SNAPSHOT_LOADING_MODE = "snapshot-loading-mode";
119+
static final String REST_NAMESPACE_SEPARATOR = "rest-namespace-separator";
119120
public static final String REST_PAGE_SIZE = "rest-page-size";
120121
private static final List<String> TOKEN_PREFERENCE_ORDER =
121122
ImmutableList.of(
@@ -148,6 +149,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
148149
private boolean reportingViaRestEnabled;
149150
private Integer pageSize = null;
150151
private CloseableGroup closeables = null;
152+
private String namespaceSeparator = null;
151153

152154
// a lazy thread pool for token refresh
153155
private volatile ScheduledExecutorService refreshExecutor = null;
@@ -284,6 +286,9 @@ public void initialize(String name, Map<String, String> unresolved) {
284286

285287
this.reportingViaRestEnabled =
286288
PropertyUtil.propertyAsBoolean(mergedProps, REST_METRICS_REPORTING_ENABLED, true);
289+
this.namespaceSeparator =
290+
PropertyUtil.propertyAsString(
291+
mergedProps, REST_NAMESPACE_SEPARATOR, RESTUtil.NAMESPACE_ESCAPED_SEPARATOR);
287292
super.initialize(name, mergedProps);
288293
}
289294

@@ -547,7 +552,7 @@ public void createNamespace(
547552
public List<Namespace> listNamespaces(SessionContext context, Namespace namespace) {
548553
Map<String, String> queryParams = Maps.newHashMap();
549554
if (!namespace.isEmpty()) {
550-
queryParams.put("parent", RESTUtil.encodeNamespace(namespace));
555+
queryParams.put("parent", RESTUtil.encodeNamespace(namespace, namespaceSeparator));
551556
}
552557

553558
ImmutableList.Builder<Namespace> namespaces = ImmutableList.builder();

‎core/src/main/java/org/apache/iceberg/rest/RESTUtil.java

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,13 @@
2828
import org.apache.iceberg.relocated.com.google.common.base.Joiner;
2929
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
3030
import org.apache.iceberg.relocated.com.google.common.base.Splitter;
31+
import org.apache.iceberg.relocated.com.google.common.base.Strings;
3132
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
3233
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
3334
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
3435

3536
public class RESTUtil {
36-
private static final String NAMESPACE_ESCAPED_SEPARATOR = "%1F";
37-
private static final Joiner NAMESPACE_ESCAPED_JOINER = Joiner.on(NAMESPACE_ESCAPED_SEPARATOR);
38-
private static final Splitter NAMESPACE_ESCAPED_SPLITTER =
39-
Splitter.on(NAMESPACE_ESCAPED_SEPARATOR);
37+
static final String NAMESPACE_ESCAPED_SEPARATOR = "%1F";
4038

4139
/**
4240
* @deprecated since 1.7.0, will be made private in 1.8.0; use {@link
@@ -194,15 +192,34 @@ public static String decodeString(String encoded) {
194192
* @return UTF-8 encoded string representing the namespace, suitable for use as a URL parameter
195193
*/
196194
public static String encodeNamespace(Namespace ns) {
195+
return encodeNamespace(ns, NAMESPACE_ESCAPED_SEPARATOR);
196+
}
197+
198+
/**
199+
* Returns a String representation of a namespace that is suitable for use in a URL / URI.
200+
*
201+
* <p>This function needs to be called when a namespace is used as a path variable (or query
202+
* parameter etc.), to format the namespace per the spec.
203+
*
204+
* <p>{@link RESTUtil#decodeNamespace(String, String)} should be used to parse the namespace from
205+
* a URL parameter.
206+
*
207+
* @param ns namespace to encode
208+
* @param namespaceSeparator The namespace separator to use for encoding
209+
* @return UTF-8 encoded string representing the namespace, suitable for use as a URL parameter
210+
*/
211+
public static String encodeNamespace(Namespace ns, String namespaceSeparator) {
197212
Preconditions.checkArgument(ns != null, "Invalid namespace: null");
213+
Preconditions.checkArgument(
214+
!Strings.isNullOrEmpty(namespaceSeparator), "Invalid namespace separator: null or empty");
198215
String[] levels = ns.levels();
199216
String[] encodedLevels = new String[levels.length];
200217

201218
for (int i = 0; i < levels.length; i++) {
202219
encodedLevels[i] = encodeString(levels[i]);
203220
}
204221

205-
return NAMESPACE_ESCAPED_JOINER.join(encodedLevels);
222+
return Joiner.on(namespaceSeparator).join(encodedLevels);
206223
}
207224

208225
/**
@@ -215,8 +232,26 @@ public static String encodeNamespace(Namespace ns) {
215232
* @return a namespace
216233
*/
217234
public static Namespace decodeNamespace(String encodedNs) {
235+
return decodeNamespace(encodedNs, NAMESPACE_ESCAPED_SEPARATOR);
236+
}
237+
238+
/**
239+
* Takes in a string representation of a namespace as used for a URL parameter and returns the
240+
* corresponding namespace.
241+
*
242+
* <p>See also {@link #encodeNamespace} for generating correctly formatted URLs.
243+
*
244+
* @param encodedNs a namespace to decode
245+
* @param namespaceSeparator The namespace separator to use for decoding. This should be the same
246+
* separator that was used when calling {@link RESTUtil#encodeNamespace(Namespace, String)}
247+
* @return a namespace
248+
*/
249+
public static Namespace decodeNamespace(String encodedNs, String namespaceSeparator) {
218250
Preconditions.checkArgument(encodedNs != null, "Invalid namespace: null");
219-
String[] levels = Iterables.toArray(NAMESPACE_ESCAPED_SPLITTER.split(encodedNs), String.class);
251+
Preconditions.checkArgument(
252+
!Strings.isNullOrEmpty(namespaceSeparator), "Invalid namespace separator: null or empty");
253+
String[] levels =
254+
Iterables.toArray(Splitter.on(namespaceSeparator).split(encodedNs), String.class);
220255

221256
// Decode levels in place
222257
for (int i = 0; i < levels.length; i++) {

‎core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,19 @@
2222
import org.apache.iceberg.catalog.Namespace;
2323
import org.apache.iceberg.catalog.TableIdentifier;
2424
import org.apache.iceberg.relocated.com.google.common.base.Joiner;
25+
import org.apache.iceberg.util.PropertyUtil;
2526

2627
public class ResourcePaths {
2728
private static final Joiner SLASH = Joiner.on("/").skipNulls();
2829
private static final String PREFIX = "prefix";
2930

3031
public static ResourcePaths forCatalogProperties(Map<String, String> properties) {
31-
return new ResourcePaths(properties.get(PREFIX));
32+
return new ResourcePaths(
33+
properties.get(PREFIX),
34+
PropertyUtil.propertyAsString(
35+
properties,
36+
RESTSessionCatalog.REST_NAMESPACE_SEPARATOR,
37+
RESTUtil.NAMESPACE_ESCAPED_SEPARATOR));
3238
}
3339

3440
public static String config() {
@@ -40,39 +46,48 @@ public static String tokens() {
4046
}
4147

4248
private final String prefix;
49+
private final String namespaceSeparator;
4350

4451
public ResourcePaths(String prefix) {
52+
this(prefix, RESTUtil.NAMESPACE_ESCAPED_SEPARATOR);
53+
}
54+
55+
private ResourcePaths(String prefix, String namespaceSeparator) {
4556
this.prefix = prefix;
57+
this.namespaceSeparator = namespaceSeparator;
4658
}
4759

4860
public String namespaces() {
4961
return SLASH.join("v1", prefix, "namespaces");
5062
}
5163

5264
public String namespace(Namespace ns) {
53-
return SLASH.join("v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns));
65+
return SLASH.join("v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns, namespaceSeparator));
5466
}
5567

5668
public String namespaceProperties(Namespace ns) {
57-
return SLASH.join("v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns), "properties");
69+
return SLASH.join(
70+
"v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns, namespaceSeparator), "properties");
5871
}
5972

6073
public String tables(Namespace ns) {
61-
return SLASH.join("v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns), "tables");
74+
return SLASH.join(
75+
"v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns, namespaceSeparator), "tables");
6276
}
6377

6478
public String table(TableIdentifier ident) {
6579
return SLASH.join(
6680
"v1",
6781
prefix,
6882
"namespaces",
69-
RESTUtil.encodeNamespace(ident.namespace()),
83+
RESTUtil.encodeNamespace(ident.namespace(), namespaceSeparator),
7084
"tables",
7185
RESTUtil.encodeString(ident.name()));
7286
}
7387

7488
public String register(Namespace ns) {
75-
return SLASH.join("v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns), "register");
89+
return SLASH.join(
90+
"v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns, namespaceSeparator), "register");
7691
}
7792

7893
public String rename() {
@@ -84,7 +99,7 @@ public String metrics(TableIdentifier identifier) {
8499
"v1",
85100
prefix,
86101
"namespaces",
87-
RESTUtil.encodeNamespace(identifier.namespace()),
102+
RESTUtil.encodeNamespace(identifier.namespace(), namespaceSeparator),
88103
"tables",
89104
RESTUtil.encodeString(identifier.name()),
90105
"metrics");
@@ -95,15 +110,16 @@ public String commitTransaction() {
95110
}
96111

97112
public String views(Namespace ns) {
98-
return SLASH.join("v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns), "views");
113+
return SLASH.join(
114+
"v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns, namespaceSeparator), "views");
99115
}
100116

101117
public String view(TableIdentifier ident) {
102118
return SLASH.join(
103119
"v1",
104120
prefix,
105121
"namespaces",
106-
RESTUtil.encodeNamespace(ident.namespace()),
122+
RESTUtil.encodeNamespace(ident.namespace(), namespaceSeparator),
107123
"views",
108124
RESTUtil.encodeString(ident.name()));
109125
}

‎core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
/** Adaptor class to translate REST requests into {@link Catalog} API calls. */
7474
public class RESTCatalogAdapter implements RESTClient {
7575
private static final Splitter SLASH = Splitter.on('/');
76+
private static final String NAMESPACE_SEPARATOR = "%2E";
7677

7778
private static final Map<Class<? extends Exception>, Integer> EXCEPTION_ERROR_CODES =
7879
ImmutableMap.<Class<? extends Exception>, Integer>builder()
@@ -282,7 +283,11 @@ public <T extends RESTResponse> T handleRequest(
282283
return castResponse(responseType, handleOAuthRequest(body));
283284

284285
case CONFIG:
285-
return castResponse(responseType, ConfigResponse.builder().build());
286+
return castResponse(
287+
responseType,
288+
ConfigResponse.builder()
289+
.withDefault(RESTSessionCatalog.REST_NAMESPACE_SEPARATOR, NAMESPACE_SEPARATOR)
290+
.build());
286291

287292
case LIST_NAMESPACES:
288293
if (asNamespaceCatalog != null) {
@@ -665,7 +670,7 @@ public static void configureResponseFromException(
665670
}
666671

667672
private static Namespace namespaceFromPathVars(Map<String, String> pathVars) {
668-
return RESTUtil.decodeNamespace(pathVars.get("namespace"));
673+
return RESTUtil.decodeNamespace(pathVars.get("namespace"), NAMESPACE_SEPARATOR);
669674
}
670675

671676
private static TableIdentifier identFromPathVars(Map<String, String> pathVars) {

‎core/src/test/java/org/apache/iceberg/rest/TestRESTUtil.java

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@
2020

2121
import static org.assertj.core.api.Assertions.assertThat;
2222
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
23+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2324

2425
import java.util.Map;
2526
import org.apache.iceberg.catalog.Namespace;
2627
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
2728
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.params.ParameterizedTest;
30+
import org.junit.jupiter.params.provider.ValueSource;
2831

2932
public class TestRESTUtil {
3033

@@ -67,18 +70,24 @@ public void testStripTrailingSlash() {
6770
}
6871
}
6972

70-
@Test
71-
public void testRoundTripUrlEncodeDecodeNamespace() {
73+
@ParameterizedTest
74+
@ValueSource(strings = {"%1F", "%2D", "%2E"})
75+
public void testRoundTripUrlEncodeDecodeNamespace(String namespaceSeparator) {
7276
// Namespace levels and their expected url encoded form
7377
Object[][] testCases =
7478
new Object[][] {
7579
new Object[] {new String[] {"dogs"}, "dogs"},
7680
new Object[] {new String[] {"dogs.named.hank"}, "dogs.named.hank"},
7781
new Object[] {new String[] {"dogs/named/hank"}, "dogs%2Fnamed%2Fhank"},
78-
new Object[] {new String[] {"dogs", "named", "hank"}, "dogs%1Fnamed%1Fhank"},
82+
new Object[] {
83+
new String[] {"dogs", "named", "hank"},
84+
String.format("dogs%snamed%shank", namespaceSeparator, namespaceSeparator)
85+
},
7986
new Object[] {
8087
new String[] {"dogs.and.cats", "named", "hank.or.james-westfall"},
81-
"dogs.and.cats%1Fnamed%1Fhank.or.james-westfall"
88+
String.format(
89+
"dogs.and.cats%snamed%shank.or.james-westfall",
90+
namespaceSeparator, namespaceSeparator),
8291
}
8392
};
8493

@@ -89,10 +98,10 @@ public void testRoundTripUrlEncodeDecodeNamespace() {
8998
Namespace namespace = Namespace.of(levels);
9099

91100
// To be placed into a URL path as query parameter or path parameter
92-
assertThat(RESTUtil.encodeNamespace(namespace)).isEqualTo(encodedNs);
101+
assertThat(RESTUtil.encodeNamespace(namespace, namespaceSeparator)).isEqualTo(encodedNs);
93102

94103
// Decoded (after pulling as String) from URL
95-
Namespace asNamespace = RESTUtil.decodeNamespace(encodedNs);
104+
Namespace asNamespace = RESTUtil.decodeNamespace(encodedNs, namespaceSeparator);
96105
assertThat(asNamespace).isEqualTo(namespace);
97106
}
98107
}
@@ -139,4 +148,24 @@ public void testOAuth2FormDataDecoding() {
139148

140149
assertThat(RESTUtil.decodeFormData(formString)).isEqualTo(expected);
141150
}
151+
152+
@Test
153+
public void nullOrEmptyNamespaceSeparator() {
154+
String errorMsg = "Invalid namespace separator: null or empty";
155+
assertThatThrownBy(() -> RESTUtil.encodeNamespace(Namespace.empty(), null))
156+
.isInstanceOf(IllegalArgumentException.class)
157+
.hasMessage(errorMsg);
158+
159+
assertThatThrownBy(() -> RESTUtil.encodeNamespace(Namespace.empty(), ""))
160+
.isInstanceOf(IllegalArgumentException.class)
161+
.hasMessage(errorMsg);
162+
163+
assertThatThrownBy(() -> RESTUtil.decodeNamespace("namespace", null))
164+
.isInstanceOf(IllegalArgumentException.class)
165+
.hasMessage(errorMsg);
166+
167+
assertThatThrownBy(() -> RESTUtil.decodeNamespace("namespace", ""))
168+
.isInstanceOf(IllegalArgumentException.class)
169+
.hasMessage(errorMsg);
170+
}
142171
}

‎core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.apache.iceberg.catalog.TableIdentifier;
2525
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
2626
import org.junit.jupiter.api.Test;
27+
import org.junit.jupiter.params.ParameterizedTest;
28+
import org.junit.jupiter.params.provider.ValueSource;
2729

2830
public class TestResourcePaths {
2931
private final String prefix = "ws/catalog";
@@ -64,6 +66,52 @@ public void testNamespaceWithMultipartNamespace() {
6466
assertThat(withoutPrefix.namespace(ns)).isEqualTo("v1/namespaces/n%1Fs");
6567
}
6668

69+
@ParameterizedTest
70+
@ValueSource(strings = {"%1F", "%2D", "%2E"})
71+
public void testNamespaceWithMultipartNamespace(String namespaceSeparator) {
72+
Namespace ns = Namespace.of("n", "s");
73+
String namespace = String.format("n%ss", namespaceSeparator);
74+
assertThat(
75+
ResourcePaths.forCatalogProperties(
76+
ImmutableMap.of(
77+
"prefix",
78+
prefix,
79+
RESTSessionCatalog.REST_NAMESPACE_SEPARATOR,
80+
namespaceSeparator))
81+
.namespace(ns))
82+
.isEqualTo("v1/ws/catalog/namespaces/" + namespace);
83+
84+
assertThat(
85+
ResourcePaths.forCatalogProperties(
86+
ImmutableMap.of(
87+
RESTSessionCatalog.REST_NAMESPACE_SEPARATOR, namespaceSeparator))
88+
.namespace(ns))
89+
.isEqualTo("v1/namespaces/" + namespace);
90+
}
91+
92+
@ParameterizedTest
93+
@ValueSource(strings = {"%1F", "%2D", "%2E"})
94+
public void testNamespaceWithDot(String namespaceSeparator) {
95+
Namespace ns = Namespace.of("n.s", "a.b");
96+
String namespace = String.format("n.s%sa.b", namespaceSeparator);
97+
assertThat(
98+
ResourcePaths.forCatalogProperties(
99+
ImmutableMap.of(
100+
"prefix",
101+
prefix,
102+
RESTSessionCatalog.REST_NAMESPACE_SEPARATOR,
103+
namespaceSeparator))
104+
.namespace(ns))
105+
.isEqualTo("v1/ws/catalog/namespaces/" + namespace);
106+
107+
assertThat(
108+
ResourcePaths.forCatalogProperties(
109+
ImmutableMap.of(
110+
RESTSessionCatalog.REST_NAMESPACE_SEPARATOR, namespaceSeparator))
111+
.namespace(ns))
112+
.isEqualTo("v1/namespaces/" + namespace);
113+
}
114+
67115
@Test
68116
public void testNamespaceProperties() {
69117
Namespace ns = Namespace.of("ns");

0 commit comments

Comments
 (0)