Skip to content

Commit cda8fc4

Browse files
wgtmacwgtmac
authored
feat(rest): support namespace separator (#617)
1 parent 42b00d4 commit cda8fc4

File tree

7 files changed

+71
-35
lines changed

7 files changed

+71
-35
lines changed

‎src/iceberg/catalog/rest/catalog_properties.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ class ICEBERG_REST_EXPORT RestCatalogProperties
4949
inline static Entry<std::string> kWarehouse{"warehouse", ""};
5050
/// \brief The optional prefix for REST API paths.
5151
inline static Entry<std::string> kPrefix{"prefix", ""};
52+
/// \brief The encoded separator used to join namespace levels in REST paths.
53+
inline static Entry<std::string> kNamespaceSeparator{"namespace-separator", "%1F"};
5254
/// \brief The snapshot loading mode (ALL or REFS).
5355
inline static Entry<std::string> kSnapshotLoadingMode{"snapshot-loading-mode", "ALL"};
5456
/// \brief The prefix for HTTP headers.

‎src/iceberg/catalog/rest/resource_paths.cc‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,21 @@
2727

2828
namespace iceberg::rest {
2929

30-
Result<std::unique_ptr<ResourcePaths>> ResourcePaths::Make(std::string base_uri,
31-
const std::string& prefix) {
30+
Result<std::unique_ptr<ResourcePaths>> ResourcePaths::Make(
31+
std::string base_uri, const std::string& prefix,
32+
const std::string& namespace_separator) {
3233
if (base_uri.empty()) {
3334
return InvalidArgument("Base URI is empty");
3435
}
35-
return std::unique_ptr<ResourcePaths>(new ResourcePaths(std::move(base_uri), prefix));
36+
return std::unique_ptr<ResourcePaths>(
37+
new ResourcePaths(std::move(base_uri), prefix, namespace_separator));
3638
}
3739

38-
ResourcePaths::ResourcePaths(std::string base_uri, const std::string& prefix)
39-
: base_uri_(std::move(base_uri)), prefix_(prefix.empty() ? "" : (prefix + "/")) {}
40+
ResourcePaths::ResourcePaths(std::string base_uri, const std::string& prefix,
41+
std::string namespace_separator)
42+
: base_uri_(std::move(base_uri)),
43+
prefix_(prefix.empty() ? "" : (prefix + "/")),
44+
namespace_separator_(std::move(namespace_separator)) {}
4045

4146
Result<std::string> ResourcePaths::Config() const {
4247
return std::format("{}/v1/config", base_uri_);
@@ -51,31 +56,36 @@ Result<std::string> ResourcePaths::Namespaces() const {
5156
}
5257

5358
Result<std::string> ResourcePaths::Namespace_(const Namespace& ns) const {
54-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns));
59+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
60+
EncodeNamespace(ns, namespace_separator_));
5561
return std::format("{}/v1/{}namespaces/{}", base_uri_, prefix_, encoded_namespace);
5662
}
5763

5864
Result<std::string> ResourcePaths::NamespaceProperties(const Namespace& ns) const {
59-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns));
65+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
66+
EncodeNamespace(ns, namespace_separator_));
6067
return std::format("{}/v1/{}namespaces/{}/properties", base_uri_, prefix_,
6168
encoded_namespace);
6269
}
6370

6471
Result<std::string> ResourcePaths::Tables(const Namespace& ns) const {
65-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns));
72+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
73+
EncodeNamespace(ns, namespace_separator_));
6674
return std::format("{}/v1/{}namespaces/{}/tables", base_uri_, prefix_,
6775
encoded_namespace);
6876
}
6977

7078
Result<std::string> ResourcePaths::Table(const TableIdentifier& ident) const {
71-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ident.ns));
79+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
80+
EncodeNamespace(ident.ns, namespace_separator_));
7281
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name, EncodeString(ident.name));
7382
return std::format("{}/v1/{}namespaces/{}/tables/{}", base_uri_, prefix_,
7483
encoded_namespace, encoded_table_name);
7584
}
7685

7786
Result<std::string> ResourcePaths::Register(const Namespace& ns) const {
78-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns));
87+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
88+
EncodeNamespace(ns, namespace_separator_));
7989
return std::format("{}/v1/{}namespaces/{}/register", base_uri_, prefix_,
8090
encoded_namespace);
8191
}
@@ -85,14 +95,16 @@ Result<std::string> ResourcePaths::Rename() const {
8595
}
8696

8797
Result<std::string> ResourcePaths::Metrics(const TableIdentifier& ident) const {
88-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ident.ns));
98+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
99+
EncodeNamespace(ident.ns, namespace_separator_));
89100
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name, EncodeString(ident.name));
90101
return std::format("{}/v1/{}namespaces/{}/tables/{}/metrics", base_uri_, prefix_,
91102
encoded_namespace, encoded_table_name);
92103
}
93104

94105
Result<std::string> ResourcePaths::Credentials(const TableIdentifier& ident) const {
95-
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ident.ns));
106+
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
107+
EncodeNamespace(ident.ns, namespace_separator_));
96108
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name, EncodeString(ident.name));
97109
return std::format("{}/v1/{}namespaces/{}/tables/{}/credentials", base_uri_, prefix_,
98110
encoded_namespace, encoded_table_name);

‎src/iceberg/catalog/rest/resource_paths.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ class ICEBERG_REST_EXPORT ResourcePaths {
3939
/// \brief Construct a ResourcePaths with base URI and optional prefix.
4040
/// \param base_uri The base URI of the REST catalog server (without trailing slash)
4141
/// \param prefix Optional prefix for REST API paths (default: empty)
42+
/// \param namespace_separator Encoded separator used between namespace levels.
4243
/// \return A unique_ptr to ResourcePaths instance
43-
static Result<std::unique_ptr<ResourcePaths>> Make(std::string base_uri,
44-
const std::string& prefix);
44+
static Result<std::unique_ptr<ResourcePaths>> Make(
45+
std::string base_uri, const std::string& prefix,
46+
const std::string& namespace_separator);
4547

4648
/// \brief Get the /v1/config endpoint path.
4749
Result<std::string> Config() const;
@@ -82,10 +84,12 @@ class ICEBERG_REST_EXPORT ResourcePaths {
8284
Result<std::string> CommitTransaction() const;
8385

8486
private:
85-
ResourcePaths(std::string base_uri, const std::string& prefix);
87+
ResourcePaths(std::string base_uri, const std::string& prefix,
88+
std::string namespace_separator);
8689

8790
std::string base_uri_; // required
8891
const std::string prefix_; // optional
92+
const std::string namespace_separator_;
8993
};
9094

9195
} // namespace iceberg::rest

‎src/iceberg/catalog/rest/rest_catalog.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,10 @@ Result<std::shared_ptr<RestCatalog>> RestCatalog::Make(
131131
ICEBERG_ASSIGN_OR_RAISE(auto auth_manager,
132132
auth::AuthManagers::Load(catalog_name, config.configs()));
133133
ICEBERG_ASSIGN_OR_RAISE(
134-
auto paths, ResourcePaths::Make(std::string(TrimTrailingSlash(uri)),
135-
config.Get(RestCatalogProperties::kPrefix)));
134+
auto paths,
135+
ResourcePaths::Make(std::string(TrimTrailingSlash(uri)),
136+
config.Get(RestCatalogProperties::kPrefix),
137+
config.Get(RestCatalogProperties::kNamespaceSeparator)));
136138

137139
// Create init session for fetching server configuration
138140
HttpClient init_client(config.ExtractHeaders());
@@ -158,8 +160,10 @@ Result<std::shared_ptr<RestCatalog>> RestCatalog::Make(
158160
// Update resource paths based on the final config
159161
ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config.Uri());
160162
ICEBERG_ASSIGN_OR_RAISE(
161-
paths, ResourcePaths::Make(std::string(TrimTrailingSlash(final_uri)),
162-
final_config.Get(RestCatalogProperties::kPrefix)));
163+
paths,
164+
ResourcePaths::Make(std::string(TrimTrailingSlash(final_uri)),
165+
final_config.Get(RestCatalogProperties::kPrefix),
166+
final_config.Get(RestCatalogProperties::kNamespaceSeparator)));
163167

164168
// Get snapshot loading mode
165169
ICEBERG_ASSIGN_OR_RAISE(auto snapshot_mode, final_config.SnapshotLoadingMode());
@@ -203,7 +207,9 @@ Result<std::vector<Namespace>> RestCatalog::ListNamespaces(const Namespace& ns)
203207
while (true) {
204208
std::unordered_map<std::string, std::string> params;
205209
if (!ns.levels.empty()) {
206-
ICEBERG_ASSIGN_OR_RAISE(params[kQueryParamParent], EncodeNamespace(ns));
210+
ICEBERG_ASSIGN_OR_RAISE(
211+
params[kQueryParamParent],
212+
EncodeNamespace(ns, config_.Get(RestCatalogProperties::kNamespaceSeparator)));
207213
}
208214
if (!next_token.empty()) {
209215
params[kQueryParamPageToken] = next_token;

‎src/iceberg/catalog/rest/rest_util.cc‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@
3030

3131
namespace iceberg::rest {
3232

33-
namespace {
34-
const std::string kNamespaceEscapeSeparator = "%1F";
35-
}
36-
3733
std::string_view TrimTrailingSlash(std::string_view str) {
3834
while (!str.empty() && str.back() == '/') {
3935
str.remove_suffix(1);
@@ -69,7 +65,8 @@ Result<std::string> DecodeString(std::string_view str_to_decode) {
6965
return std::string{decoded.data(), decoded.size()};
7066
}
7167

72-
Result<std::string> EncodeNamespace(const Namespace& ns_to_encode) {
68+
Result<std::string> EncodeNamespace(const Namespace& ns_to_encode,
69+
std::string_view separator) {
7370
if (ns_to_encode.levels.empty()) {
7471
return "";
7572
}
@@ -79,28 +76,29 @@ Result<std::string> EncodeNamespace(const Namespace& ns_to_encode) {
7976
for (size_t i = 1; i < ns_to_encode.levels.size(); ++i) {
8077
ICEBERG_ASSIGN_OR_RAISE(std::string encoded_level,
8178
EncodeString(ns_to_encode.levels[i]));
82-
result.append(kNamespaceEscapeSeparator);
79+
result.append(separator);
8380
result.append(std::move(encoded_level));
8481
}
8582

8683
return result;
8784
}
8885

89-
Result<Namespace> DecodeNamespace(std::string_view str_to_decode) {
86+
Result<Namespace> DecodeNamespace(std::string_view str_to_decode,
87+
std::string_view separator) {
9088
if (str_to_decode.empty()) {
9189
return Namespace{.levels = {}};
9290
}
9391

9492
Namespace ns{};
9593
std::string::size_type start = 0;
96-
std::string::size_type end = str_to_decode.find(kNamespaceEscapeSeparator);
94+
std::string::size_type end = str_to_decode.find(separator);
9795

9896
while (end != std::string::npos) {
9997
ICEBERG_ASSIGN_OR_RAISE(std::string decoded_level,
10098
DecodeString(str_to_decode.substr(start, end - start)));
10199
ns.levels.push_back(std::move(decoded_level));
102-
start = end + kNamespaceEscapeSeparator.size();
103-
end = str_to_decode.find(kNamespaceEscapeSeparator, start);
100+
start = end + separator.size();
101+
end = str_to_decode.find(separator, start);
104102
}
105103

106104
ICEBERG_ASSIGN_OR_RAISE(std::string decoded_level,

‎src/iceberg/catalog/rest/rest_util.h‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,23 @@ ICEBERG_REST_EXPORT Result<std::string> DecodeString(std::string_view str_to_dec
5858

5959
/// \brief Encode a Namespace into a URL-safe component.
6060
///
61-
/// \details Encodes each level separately using EncodeString, then joins them with "%1F".
61+
/// \details Encodes each level separately using EncodeString, then joins them with the
62+
/// provided separator. The default matches the REST spec's historical "%1F".
6263
/// \param ns_to_encode The namespace to encode.
64+
/// \param separator The encoded separator to place between namespace levels.
6365
/// \return The percent-encoded namespace string suitable for URLs.
64-
ICEBERG_REST_EXPORT Result<std::string> EncodeNamespace(const Namespace& ns_to_encode);
66+
ICEBERG_REST_EXPORT Result<std::string> EncodeNamespace(
67+
const Namespace& ns_to_encode, std::string_view separator = "%1F");
6568

6669
/// \brief Decode a URL-encoded namespace string back to a Namespace.
6770
///
68-
/// \details Splits by "%1F" (the URL-encoded form of ASCII Unit Separator), then decodes
69-
/// each level separately using DecodeString.
71+
/// \details Splits by the provided separator, then decodes each level separately using
72+
/// DecodeString.
7073
/// \param str_to_decode The percent-encoded namespace string.
74+
/// \param separator The encoded separator used between namespace levels.
7175
/// \return The decoded Namespace.
72-
ICEBERG_REST_EXPORT Result<Namespace> DecodeNamespace(std::string_view str_to_decode);
76+
ICEBERG_REST_EXPORT Result<Namespace> DecodeNamespace(std::string_view str_to_decode,
77+
std::string_view separator = "%1F");
7378

7479
/// \brief Merge catalog configuration properties.
7580
///

‎src/iceberg/test/rest_util_test.cc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,15 @@ TEST(RestUtilTest, RoundTripUrlEncodeDecodeNamespace) {
7474
EXPECT_THAT(DecodeNamespace(""), HasValue(::testing::Eq(Namespace{.levels = {}})));
7575
}
7676

77+
TEST(RestUtilTest, RoundTripNamespaceWithCustomSeparator) {
78+
Namespace ns{.levels = {"dogs.and.cats", "named", "hank.or.james-westfall"}};
79+
80+
EXPECT_THAT(EncodeNamespace(ns, "%2E"),
81+
HasValue(::testing::Eq("dogs.and.cats%2Enamed%2Ehank.or.james-westfall")));
82+
EXPECT_THAT(DecodeNamespace("dogs.and.cats%2Enamed%2Ehank.or.james-westfall", "%2E"),
83+
HasValue(::testing::Eq(ns)));
84+
}
85+
7786
TEST(RestUtilTest, EncodeString) {
7887
// RFC 3986 unreserved characters should not be encoded
7988
EXPECT_THAT(EncodeString("abc123XYZ"), HasValue(::testing::Eq("abc123XYZ")));

0 commit comments

Comments
 (0)