From c99784e0b689b6bd1064afa488ebe6985b9a96a0 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Sat, 15 Nov 2025 19:19:43 +0800 Subject: [PATCH 01/12] feat: first version of rest catalog --- src/iceberg/catalog/rest/CMakeLists.txt | 7 +- src/iceberg/catalog/rest/catalog.cc | 200 ++++++++++++++++++ src/iceberg/catalog/rest/catalog.h | 110 ++++++++++ src/iceberg/catalog/rest/config.cc | 62 ++++++ src/iceberg/catalog/rest/config.h | 69 ++++++ src/iceberg/catalog/rest/constant.h | 44 ++++ src/iceberg/catalog/rest/endpoint_util.h | 81 +++++++ .../catalog/rest/http_client_interal.h | 93 ++++++++ .../catalog/rest/http_client_internal.cc | 84 ++++++++ src/iceberg/catalog/rest/meson.build | 22 +- src/iceberg/catalog/rest/resource_paths.cc | 135 ++++++++++++ src/iceberg/catalog/rest/resource_paths.h | 132 ++++++++++++ src/iceberg/catalog/rest/rest_catalog.cc | 44 ---- src/iceberg/catalog/rest/rest_catalog.h | 41 ---- src/iceberg/test/rest_catalog_test.cc | 200 ++++++++++++------ 15 files changed, 1167 insertions(+), 157 deletions(-) create mode 100644 src/iceberg/catalog/rest/catalog.cc create mode 100644 src/iceberg/catalog/rest/catalog.h create mode 100644 src/iceberg/catalog/rest/config.cc create mode 100644 src/iceberg/catalog/rest/config.h create mode 100644 src/iceberg/catalog/rest/constant.h create mode 100644 src/iceberg/catalog/rest/endpoint_util.h create mode 100644 src/iceberg/catalog/rest/http_client_interal.h create mode 100644 src/iceberg/catalog/rest/http_client_internal.cc create mode 100644 src/iceberg/catalog/rest/resource_paths.cc create mode 100644 src/iceberg/catalog/rest/resource_paths.h delete mode 100644 src/iceberg/catalog/rest/rest_catalog.cc delete mode 100644 src/iceberg/catalog/rest/rest_catalog.h diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index 38d897270..e230646b1 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -15,7 +15,12 @@ # specific language governing permissions and limitations # under the License. -set(ICEBERG_REST_SOURCES rest_catalog.cc json_internal.cc) +set(ICEBERG_REST_SOURCES + catalog.cc + json_internal.cc + config.cc + http_client_internal.cc + resource_paths.cc) set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/rest/catalog.cc b/src/iceberg/catalog/rest/catalog.cc new file mode 100644 index 000000000..c4f0a88c6 --- /dev/null +++ b/src/iceberg/catalog/rest/catalog.cc @@ -0,0 +1,200 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/catalog.h" + +#include +#include + +#include + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result> RestCatalog::Make(const RestCatalogConfig& config) { + // Create ResourcePaths and validate URI + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + + ICEBERG_ASSIGN_OR_RAISE(auto tmp_client, HttpClient::Make(config)); + + const std::string endpoint = paths->V1Config(); + cpr::Parameters params; + ICEBERG_ASSIGN_OR_RAISE(const auto& response, tmp_client->Get(endpoint, params)); + switch (response.status_code) { + case cpr::status::HTTP_OK: { + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); + ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); + // Merge server config into client config, server config overrides > client config + // properties > server config defaults + auto final_props = std::move(server_config.defaults); + for (const auto& kv : config.configs()) { + final_props.insert_or_assign(kv.first, kv.second); + } + + for (const auto& kv : server_config.overrides) { + final_props.insert_or_assign(kv.first, kv.second); + } + auto final_config = RestCatalogConfig::FromMap(final_props); + ICEBERG_ASSIGN_OR_RAISE(auto client, HttpClient::Make(*final_config)); + ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config)); + return std::unique_ptr(new RestCatalog( + std::move(final_config), std::move(client), std::move(*final_paths))); + }; + default: { + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); + ICEBERG_ASSIGN_OR_RAISE(auto list_response, ErrorResponseFromJson(json)); + return UnknownError("Error listing namespaces: {}", list_response.error.message); + } + } +} + +RestCatalog::RestCatalog(std::unique_ptr config, + std::unique_ptr client, ResourcePaths paths) + : config_(std::move(config)), client_(std::move(client)), paths_(std::move(paths)) {} + +std::string_view RestCatalog::name() const { + auto it = config_->configs().find(std::string(RestCatalogConfig::kName)); + if (it == config_->configs().end() || it->second.empty()) { + return {""}; + } + return std::string_view(it->second); +} + +Result> RestCatalog::ListNamespaces(const Namespace& ns) const { + const std::string endpoint = paths_.V1Namespaces(); + std::vector result; + std::string next_token; + while (true) { + cpr::Parameters params; + if (!ns.levels.empty()) { + params.Add({std::string(kQueryParamParent), EncodeNamespaceForUrl(ns)}); + } + if (!next_token.empty()) { + params.Add({std::string(kQueryParamPageToken), next_token}); + } + ICEBERG_ASSIGN_OR_RAISE(const auto& response, client_->Get(endpoint, params)); + switch (response.status_code) { + case cpr::status::HTTP_OK: { + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); + ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json)); + result.insert(result.end(), list_response.namespaces.begin(), + list_response.namespaces.end()); + if (list_response.next_page_token.empty()) { + return result; + } + next_token = list_response.next_page_token; + continue; + } + case cpr::status::HTTP_NOT_FOUND: { + return NoSuchNamespace("Namespace not found"); + } + default: + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); + ICEBERG_ASSIGN_OR_RAISE(auto list_response, ErrorResponseFromJson(json)); + return UnknownError("Error listing namespaces: {}", list_response.error.message); + } + } +} + +Status RestCatalog::CreateNamespace( + const Namespace& ns, const std::unordered_map& properties) { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::GetNamespaceProperties( + const Namespace& ns) const { + return NotImplemented("Not implemented"); +} + +Status RestCatalog::DropNamespace(const Namespace& ns) { + return NotImplemented("Not implemented"); +} + +Result RestCatalog::NamespaceExists(const Namespace& ns) const { + return NotImplemented("Not implemented"); +} + +Status RestCatalog::UpdateNamespaceProperties( + const Namespace& ns, const std::unordered_map& updates, + const std::unordered_set& removals) { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::ListTables(const Namespace& ns) const { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::CreateTable( + const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, + const std::string& location, + const std::unordered_map& properties) { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::UpdateTable( + const TableIdentifier& identifier, + const std::vector>& requirements, + const std::vector>& updates) { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::StageCreateTable( + const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, + const std::string& location, + const std::unordered_map& properties) { + return NotImplemented("Not implemented"); +} + +Status RestCatalog::DropTable(const TableIdentifier& identifier, bool purge) { + return NotImplemented("Not implemented"); +} + +Result RestCatalog::TableExists(const TableIdentifier& identifier) const { + return NotImplemented("Not implemented"); +} + +Status RestCatalog::RenameTable(const TableIdentifier& from, const TableIdentifier& to) { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::LoadTable(const TableIdentifier& identifier) { + return NotImplemented("Not implemented"); +} + +Result> RestCatalog::RegisterTable( + const TableIdentifier& identifier, const std::string& metadata_file_location) { + return NotImplemented("Not implemented"); +} + +std::unique_ptr RestCatalog::BuildTable( + const TableIdentifier& identifier, const Schema& schema) const { + return nullptr; +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/catalog.h b/src/iceberg/catalog/rest/catalog.h new file mode 100644 index 000000000..66d7ceccb --- /dev/null +++ b/src/iceberg/catalog/rest/catalog.h @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include + +#include "iceberg/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/resource_paths.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/catalog.h +/// RestCatalog implementation for Iceberg REST API. + +namespace iceberg::rest { + +class ICEBERG_REST_EXPORT RestCatalog : public Catalog { + public: + RestCatalog(const RestCatalog&) = delete; + RestCatalog& operator=(const RestCatalog&) = delete; + RestCatalog(RestCatalog&&) = delete; + RestCatalog& operator=(RestCatalog&&) = delete; + + /// \brief Create a RestCatalog instance + /// + /// \param config the configuration for the RestCatalog + /// \return a unique_ptr to RestCatalog instance + static Result> Make(const RestCatalogConfig& config); + + std::string_view name() const override; + + Result> ListNamespaces(const Namespace& ns) const override; + + Status CreateNamespace( + const Namespace& ns, + const std::unordered_map& properties) override; + + Result> GetNamespaceProperties( + const Namespace& ns) const override; + + Status DropNamespace(const Namespace& ns) override; + + Result NamespaceExists(const Namespace& ns) const override; + + Status UpdateNamespaceProperties( + const Namespace& ns, const std::unordered_map& updates, + const std::unordered_set& removals) override; + + Result> ListTables(const Namespace& ns) const override; + + Result> CreateTable( + const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, + const std::string& location, + const std::unordered_map& properties) override; + + Result> UpdateTable( + const TableIdentifier& identifier, + const std::vector>& requirements, + const std::vector>& updates) override; + + Result> StageCreateTable( + const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, + const std::string& location, + const std::unordered_map& properties) override; + + Result TableExists(const TableIdentifier& identifier) const override; + + Status RenameTable(const TableIdentifier& from, const TableIdentifier& to) override; + + Status DropTable(const TableIdentifier& identifier, bool purge) override; + + Result> LoadTable(const TableIdentifier& identifier) override; + + Result> RegisterTable( + const TableIdentifier& identifier, + const std::string& metadata_file_location) override; + + std::unique_ptr BuildTable( + const TableIdentifier& identifier, const Schema& schema) const override; + + private: + RestCatalog(std::unique_ptr config, + std::unique_ptr client, ResourcePaths paths); + + std::unique_ptr config_; + std::unique_ptr client_; + ResourcePaths paths_; +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/config.cc b/src/iceberg/catalog/rest/config.cc new file mode 100644 index 000000000..6220a301c --- /dev/null +++ b/src/iceberg/catalog/rest/config.cc @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/config.h" + +#include "iceberg/catalog/rest/constant.h" + +namespace iceberg::rest { + +std::unique_ptr RestCatalogConfig::default_properties() { + return std::make_unique(); +} + +std::unique_ptr RestCatalogConfig::FromMap( + const std::unordered_map& properties) { + auto rest_catalog_config = std::make_unique(); + rest_catalog_config->configs_ = properties; + return rest_catalog_config; +} + +Result RestCatalogConfig::GetExtraHeaders() const { + cpr::Header headers; + + headers[std::string(kHeaderContentType)] = std::string(kMimeTypeApplicationJson); + headers[std::string(kHeaderUserAgent)] = std::string(kUserAgent); + headers[std::string(kHeaderXClientVersion)] = std::string(kClientVersion); + + constexpr std::string_view prefix = "header."; + for (const auto& [key, value] : configs_) { + if (key.starts_with(prefix)) { + std::string_view header_name = std::string_view(key).substr(prefix.length()); + + if (header_name.empty()) { + return InvalidArgument("Header name cannot be empty after '{}' prefix", prefix); + } + + if (value.empty()) { + return InvalidArgument("Header value for '{}' cannot be empty", header_name); + } + headers[std::string(header_name)] = value; + } + } + return headers; +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/config.h b/src/iceberg/catalog/rest/config.h new file mode 100644 index 000000000..b9d015af6 --- /dev/null +++ b/src/iceberg/catalog/rest/config.h @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include + +#include + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/util/config.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration class for a REST Catalog. +class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase { + public: + template + using Entry = const ConfigBase::Entry; + + /// \brief The URI of the REST catalog server. + inline static std::string_view kUri{"uri"}; + + /// \brief The name of the catalog. + inline static std::string_view kName{"name"}; + + /// \brief The warehouse path. + inline static std::string_view kWarehouse{"warehouse"}; + + /// \brief Create a default RestCatalogConfig instance. + static std::unique_ptr default_properties(); + + /// \brief Create a RestCatalogConfig instance from a map of key-value pairs. + static std::unique_ptr FromMap( + const std::unordered_map& properties); + + /// \brief Generates extra HTTP headers to be added to every request from the + /// configuration. + /// + /// This includes default headers like Content-Type, User-Agent, X-Client-Version and + /// any custom headers prefixed with "header." in the properties. + /// \return A Result containing cpr::Header object, or an error if names/values are + /// invalid. + Result GetExtraHeaders() const; +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/constant.h b/src/iceberg/catalog/rest/constant.h new file mode 100644 index 000000000..8f1e9fbf5 --- /dev/null +++ b/src/iceberg/catalog/rest/constant.h @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include + +#include "iceberg/version.h" + +/// \file iceberg/catalog/rest/constant.h +/// Constant values for Iceberg REST API. + +namespace iceberg::rest { + +constexpr std::string_view kHeaderContentType = "Content-Type"; +constexpr std::string_view kHeaderAccept = "Accept"; +constexpr std::string_view kHeaderXClientVersion = "X-Client-Version"; +constexpr std::string_view kHeaderUserAgent = "User-Agent"; + +constexpr std::string_view kMimeTypeApplicationJson = "application/json"; +constexpr std::string_view kClientVersion = "0.14.1"; +constexpr std::string_view kUserAgentPrefix = "iceberg-cpp/"; +constexpr std::string_view kUserAgent = "iceberg-cpp/" ICEBERG_VERSION_STRING; + +constexpr std::string_view kQueryParamParent = "parent"; +constexpr std::string_view kQueryParamPageToken = "page_token"; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/endpoint_util.h b/src/iceberg/catalog/rest/endpoint_util.h new file mode 100644 index 000000000..8aa4a69c9 --- /dev/null +++ b/src/iceberg/catalog/rest/endpoint_util.h @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include +#include + +#include "iceberg/table_identifier.h" + +namespace iceberg::rest { + +/// \brief Trim a single trailing slash from a URI string_view. +/// \details If \p uri_sv ends with '/', remove that last character; otherwise the input +/// is returned unchanged. +/// \param uri_sv The URI string view to trim. +/// \return The (possibly) trimmed URI string view. +inline std::string_view TrimTrailingSlash(std::string_view uri_sv) { + if (uri_sv.ends_with('/')) { + uri_sv.remove_suffix(1); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). +/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are +/// percent-encoded using uppercase hexadecimal (e.g., space -> "%20"). +/// \param value The string to encode. +/// \return The encoded string. +inline std::string EncodeUriComponent(std::string_view value) { + std::string escaped; + escaped.reserve(value.length()); + for (const unsigned char c : value) { + if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { + escaped += c; + } else { + std::format_to(std::back_inserter(escaped), "%{:02X}", c); + } + } + return escaped; +} + +/// \brief Encode a Namespace into a URL-safe component. +/// \details Joins \p ns.levels with the ASCII Unit Separator (0x1F), then percent-encodes +/// the result via EncodeUriComponent. Returns an empty string if there are no levels. +/// \param ns The namespace (sequence of path-like levels) to encode. +/// \return The percent-encoded namespace string suitable for URLs. +inline std::string EncodeNamespaceForUrl(const Namespace& ns) { + if (ns.levels.empty()) { + return ""; + } + + std::string joined_string; + joined_string.append(ns.levels.front()); + for (size_t i = 1; i < ns.levels.size(); ++i) { + joined_string.append("\x1F"); + joined_string.append(ns.levels[i]); + } + + return EncodeUriComponent(joined_string); +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client_interal.h b/src/iceberg/catalog/rest/http_client_interal.h new file mode 100644 index 000000000..ea89eda68 --- /dev/null +++ b/src/iceberg/catalog/rest/http_client_interal.h @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include + +#include +#include + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/http_client.h +/// \brief Http client for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief HTTP client for making requests to Iceberg REST Catalog API. +/// +/// This class wraps the CPR library and provides a type-safe interface for making +/// HTTP requests. It handles authentication, headers, and response parsing. +class ICEBERG_REST_EXPORT HttpClient { + public: + /// \brief Factory function to create and initialize an HttpClient. + /// This is the preferred way to construct an HttpClient, as it can handle + /// potential errors during configuration parsing (e.g., invalid headers). + /// \param config The catalog configuration. + /// \return A Result containing a unique_ptr to the HttpClient, or an Error. + static Result> Make(const RestCatalogConfig& config); + + HttpClient(const HttpClient&) = delete; + HttpClient& operator=(const HttpClient&) = delete; + HttpClient(HttpClient&&) = default; + HttpClient& operator=(HttpClient&&) = default; + + /// \brief Sends a GET request. + /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). + Result Get(const std::string& target, const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); + + /// \brief Sends a POST request. + /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). + Result Post(const std::string& target, const cpr::Body& body, + const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); + + /// \brief Sends a HEAD request. + /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). + Result Head(const std::string& target, + const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); + + /// \brief Sends a DELETE request. + /// \param target The target path relative to the base URL (e.g., "/v + Result Delete(const std::string& target, + const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); + + private: + /// \brief Private constructor. Use the static Create() factory function instead. + explicit HttpClient(cpr::Header session_headers); + + /// \brief Internal helper to execute a request. + template + Result Execute(const std::string& target, const cpr::Parameters& params, + const cpr::Header& request_headers, + Func&& perform_request); + + cpr::Header default_headers_; + std::unique_ptr session_; +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client_internal.cc b/src/iceberg/catalog/rest/http_client_internal.cc new file mode 100644 index 000000000..737e07066 --- /dev/null +++ b/src/iceberg/catalog/rest/http_client_internal.cc @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include + +#include "cpr/body.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result> HttpClient::Make(const RestCatalogConfig& config) { + ICEBERG_ASSIGN_OR_RAISE(auto session_headers, config.GetExtraHeaders()); + return std::unique_ptr(new HttpClient(std::move(session_headers))); +} + +HttpClient::HttpClient(cpr::Header session_headers) + : default_headers_(std::move(session_headers)), + session_(std::make_unique()) {} + +Result HttpClient::Get(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& headers) { + return Execute(target, params, headers, + [&](cpr::Session& session) { return session.Get(); }); +} + +Result HttpClient::Post(const std::string& target, const cpr::Body& body, + const cpr::Parameters& params, + const cpr::Header& headers) { + return Execute(target, params, headers, [&](cpr::Session& session) { + session.SetBody(body); + return session.Post(); + }); +} + +Result HttpClient::Head(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& headers) { + return Execute(target, params, headers, + [&](cpr::Session& session) { return session.Head(); }); +} + +Result HttpClient::Delete(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& headers) { + return Execute(target, params, headers, + [&](cpr::Session& session) { return session.Delete(); }); +} + +template +Result HttpClient::Execute(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& request_headers, + Func&& perform_request) { + cpr::Header combined_headers = default_headers_; + combined_headers.insert(request_headers.begin(), request_headers.end()); + + session_->SetUrl(cpr::Url{target}); + session_->SetParameters(params); + session_->SetHeader(combined_headers); + + cpr::Response response = perform_request(*session_); + return response; +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index 5f1f635ab..30d86b96a 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -15,7 +15,13 @@ # specific language governing permissions and limitations # under the License. -iceberg_rest_sources = files('json_internal.cc', 'rest_catalog.cc') +iceberg_rest_sources = files( + 'catalog.cc', + 'config.cc', + 'http_client_internal.cc', + 'json_internal.cc', + 'resource_path.cc', +) # cpr does not export symbols, so on Windows it must # be used as a static lib cpr_needs_static = ( @@ -46,4 +52,16 @@ iceberg_rest_dep = declare_dependency( meson.override_dependency('iceberg-rest', iceberg_rest_dep) pkg.generate(iceberg_rest_lib) -install_headers(['rest_catalog.h', 'types.h'], subdir: 'iceberg/catalog/rest') +install_headers( + [ + 'catalog.h', + 'config.h', + 'constant.h', + 'http_client_interal.h', + 'json_internal.h', + 'types.h', + 'util.h', + 'resource_path.h', + ], + subdir: 'iceberg/catalog/rest', +) diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc new file mode 100644 index 000000000..24e750c9d --- /dev/null +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/resource_paths.h" + +#include + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/result.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result> ResourcePaths::Make( + const RestCatalogConfig& config) { + // Validate and extract URI + auto it = config.configs().find(std::string(RestCatalogConfig::kUri)); + if (it == config.configs().end() || it->second.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + + std::string base_uri = std::string(TrimTrailingSlash(it->second)); + std::string prefix; + auto prefix_it = config.configs().find("prefix"); + if (prefix_it != config.configs().end() && !prefix_it->second.empty()) { + prefix = prefix_it->second; + } + + return std::unique_ptr( + new ResourcePaths(std::move(base_uri), std::move(prefix))); +} + +ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) + : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} + +std::string ResourcePaths::BuildPath(std::string_view path) const { + if (prefix_.empty()) { + return std::format("{}/v1/{}", base_uri_, path); + } + return std::format("{}/v1/{}/{}", base_uri_, prefix_, path); +} + +std::string ResourcePaths::V1Config() const { + return std::format("{}/v1/config", base_uri_); +} + +std::string ResourcePaths::V1OAuth2Tokens() const { + return std::format("{}/v1/oauth/tokens", base_uri_); +} + +std::string ResourcePaths::V1Namespaces() const { return BuildPath("namespaces"); } + +std::string ResourcePaths::V1Namespace(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1NamespaceProperties(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/properties", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1Tables(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/tables", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1Table(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}", EncodeNamespaceForUrl(table.ns), + table.name)); +} + +std::string ResourcePaths::V1RegisterTable(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/register", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1RenameTable() const { return BuildPath("tables/rename"); } + +std::string ResourcePaths::V1TableMetrics(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/metrics", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TableCredentials(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/credentials", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TableScanPlan(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/plan", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TableScanPlanResult(const TableIdentifier& table, + const std::string& plan_id) const { + return BuildPath(std::format("namespaces/{}/tables/{}/plan/{}", + EncodeNamespaceForUrl(table.ns), table.name, plan_id)); +} + +std::string ResourcePaths::V1TableTasks(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/tasks", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TransactionCommit() const { + return BuildPath("transactions/commit"); +} + +std::string ResourcePaths::V1Views(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/views", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1View(const TableIdentifier& view) const { + return BuildPath( + std::format("namespaces/{}/views/{}", EncodeNamespaceForUrl(view.ns), view.name)); +} + +std::string ResourcePaths::V1RenameView() const { return BuildPath("views/rename"); } + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/resource_paths.h b/src/iceberg/catalog/rest/resource_paths.h new file mode 100644 index 000000000..19c0ec64b --- /dev/null +++ b/src/iceberg/catalog/rest/resource_paths.h @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + /// \return The endpoint URL + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + /// \return The endpoint URL + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + /// \return The endpoint URL + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + /// \return The endpoint URL + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + /// \return The endpoint URL + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + /// \return The endpoint URL + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + /// \return The endpoint URL + std::string V1Table(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. + /// \return The endpoint URL + std::string V1RegisterTable(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/tables/rename endpoint path. + /// \return The endpoint URL + std::string V1RenameTable() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint + /// path. + /// \return The endpoint URL + std::string V1TableMetrics(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials + /// endpoint path. + /// \return The endpoint URL + std::string V1TableCredentials(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint + /// path. + /// \return The endpoint URL + std::string V1TableScanPlan(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{planId} + /// endpoint path. + /// \return The endpoint URL + std::string V1TableScanPlanResult(const TableIdentifier& table, + const std::string& plan_id) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint + /// path. + /// \return The endpoint URL + std::string V1TableTasks(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/transactions/commit endpoint path. + /// \return The endpoint URL + std::string V1TransactionCommit() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views endpoint path. + /// \return The endpoint URL + std::string V1Views(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views/{view} endpoint path. + /// \return The endpoint URL + std::string V1View(const TableIdentifier& view) const; + + /// \brief Get the /v1/{prefix}/views/rename endpoint path. + /// \return The endpoint URL + std::string V1RenameView() const; + + private: + explicit ResourcePaths(std::string base_uri, std::string prefix); + + // Helper to build path with optional prefix: {base_uri_}/{prefix_?}/{path} + std::string BuildPath(std::string_view path) const; + + std::string base_uri_; // URI with /v1, e.g., "http://localhost:8181/v1" + std::string prefix_; // Optional prefix from config +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc deleted file mode 100644 index cd008e9b2..000000000 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#include "iceberg/catalog/rest/rest_catalog.h" - -#include - -#include - -#include "iceberg/catalog/rest/types.h" - -namespace iceberg::catalog::rest { - -RestCatalog::RestCatalog(const std::string& base_url) : base_url_(std::move(base_url)) {} - -cpr::Response RestCatalog::GetConfig() { - cpr::Url url = cpr::Url{base_url_ + "/v1/config"}; - cpr::Response r = cpr::Get(url); - return r; -} - -cpr::Response RestCatalog::ListNamespaces() { - cpr::Url url = cpr::Url{base_url_ + "/v1/namespaces"}; - cpr::Response r = cpr::Get(url); - return r; -} - -} // namespace iceberg::catalog::rest diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h deleted file mode 100644 index 7b3e205c1..000000000 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ /dev/null @@ -1,41 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#pragma once - -#include - -#include - -#include "iceberg/catalog/rest/iceberg_rest_export.h" - -namespace iceberg::catalog::rest { - -class ICEBERG_REST_EXPORT RestCatalog { - public: - explicit RestCatalog(const std::string& base_url); - ~RestCatalog() = default; - - cpr::Response GetConfig(); - - cpr::Response ListNamespaces(); - - private: - std::string base_url_; -}; - -} // namespace iceberg::catalog::rest diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index fda9ef6de..7b30a36f7 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -17,95 +17,157 @@ * under the License. */ -#include "iceberg/catalog/rest/rest_catalog.h" - -#include - -#include -#include +#include +#include #include #include -#include -namespace iceberg::catalog::rest { +#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/table_identifier.h" +#include "iceberg/test/matchers.h" -class RestCatalogIntegrationTest : public ::testing::Test { +namespace iceberg::rest { + +// Test fixture for REST catalog tests, This assumes you have a local REST catalog service +// running Default configuration: http://localhost:8181 +class RestCatalogTest : public ::testing::Test { protected: void SetUp() override { - server_ = std::make_unique(); - port_ = server_->bind_to_any_port("127.0.0.1"); - - server_thread_ = std::thread([this]() { server_->listen_after_bind(); }); + // Default configuration for local testing + // You can override this with environment variables if needed + const char* uri_env = std::getenv("ICEBERG_REST_URI"); + const char* warehouse_env = std::getenv("ICEBERG_REST_WAREHOUSE"); + + std::string uri = uri_env ? uri_env : "http://localhost:8181"; + std::string warehouse = warehouse_env ? warehouse_env : "default"; + + config_ + .Set(RestCatalogConfig::Entry{std::string(RestCatalogConfig::kUri), + ""}, + uri) + .Set(RestCatalogConfig::Entry{std::string(RestCatalogConfig::kName), + ""}, + std::string("test_catalog")) + .Set( + RestCatalogConfig::Entry{ + std::string(RestCatalogConfig::kWarehouse), ""}, + warehouse); } - void TearDown() override { - server_->stop(); - if (server_thread_.joinable()) { - server_thread_.join(); - } - } + void TearDown() override {} - std::unique_ptr server_; - int port_ = -1; - std::thread server_thread_; + RestCatalogConfig config_; }; -TEST_F(RestCatalogIntegrationTest, DISABLED_GetConfigSuccessfully) { - server_->Get("/v1/config", [](const httplib::Request&, httplib::Response& res) { - res.status = 200; - res.set_content(R"({"warehouse": "s3://test-bucket"})", "application/json"); - }); +TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) { + auto catalog_result = RestCatalog::Make(config_); + EXPECT_THAT(catalog_result, IsOk()); - std::string base_uri = "http://127.0.0.1:" + std::to_string(port_); - RestCatalog catalog(base_uri); - cpr::Response response = catalog.GetConfig(); + if (catalog_result.has_value()) { + auto& catalog = catalog_result.value(); + EXPECT_EQ(catalog->name(), "test_catalog"); + } +} - ASSERT_EQ(response.error.code, cpr::ErrorCode::OK); - ASSERT_EQ(response.status_code, 200); +TEST_F(RestCatalogTest, DISABLED_MakeCatalogEmptyUri) { + RestCatalogConfig invalid_config = config_; + invalid_config.Set( + RestCatalogConfig::Entry{std::string(RestCatalogConfig::kUri), ""}, + std::string("")); - auto json_body = nlohmann::json::parse(response.text); - EXPECT_EQ(json_body["warehouse"], "s3://test-bucket"); + auto catalog_result = RestCatalog::Make(invalid_config); + EXPECT_THAT(catalog_result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(catalog_result, HasErrorMessage("uri")); } -TEST_F(RestCatalogIntegrationTest, DISABLED_ListNamespacesReturnsMultipleResults) { - server_->Get("/v1/namespaces", [](const httplib::Request&, httplib::Response& res) { - res.status = 200; - res.set_content(R"({ - "namespaces": [ - ["accounting", "db"], - ["production", "db"] - ] - })", - "application/json"); - }); - - std::string base_uri = "http://127.0.0.1:" + std::to_string(port_); - RestCatalog catalog(base_uri); - cpr::Response response = catalog.ListNamespaces(); - - ASSERT_EQ(response.error.code, cpr::ErrorCode::OK); - ASSERT_EQ(response.status_code, 200); - - auto json_body = nlohmann::json::parse(response.text); - ASSERT_TRUE(json_body.contains("namespaces")); - EXPECT_EQ(json_body["namespaces"].size(), 2); - EXPECT_THAT(json_body["namespaces"][0][0], "accounting"); +TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { + RestCatalogConfig custom_config = config_; + custom_config + .Set(RestCatalogConfig::Entry{"custom_prop", ""}, + std::string("custom_value")) + .Set(RestCatalogConfig::Entry{"timeout", ""}, std::string("30000")); + + auto catalog_result = RestCatalog::Make(custom_config); + EXPECT_THAT(catalog_result, IsOk()); +} + +TEST_F(RestCatalogTest, DISABLED_ListNamespaces) { + auto catalog_result = RestCatalog::Make(config_); + ASSERT_THAT(catalog_result, IsOk()); + auto& catalog = catalog_result.value(); + + Namespace ns{.levels = {}}; + auto result = catalog->ListNamespaces(ns); + EXPECT_THAT(result, IsOk()); + EXPECT_FALSE(result->empty()); + EXPECT_EQ(result->front().levels, (std::vector{"my_namespace_test2"})); } -TEST_F(RestCatalogIntegrationTest, DISABLED_HandlesServerError) { - server_->Get("/v1/config", [](const httplib::Request&, httplib::Response& res) { - res.status = 500; - res.set_content("Internal Server Error", "text/plain"); - }); +TEST_F(RestCatalogTest, DISABLED_CreateNamespaceNotImplemented) { + auto catalog_result = RestCatalog::Make(config_); + ASSERT_THAT(catalog_result, IsOk()); + auto catalog = std::move(catalog_result.value()); + + Namespace ns{.levels = {"test_namespace"}}; + std::unordered_map props = {{"owner", "test"}}; - std::string base_uri = "http://127.0.0.1:" + std::to_string(port_); - RestCatalog catalog(base_uri); - cpr::Response response = catalog.GetConfig(); + auto result = catalog->CreateNamespace(ns, props); + EXPECT_THAT(result, IsError(ErrorKind::kNotImplemented)); +} - ASSERT_EQ(response.error.code, cpr::ErrorCode::OK); - ASSERT_EQ(response.status_code, 500); - ASSERT_EQ(response.text, "Internal Server Error"); +TEST_F(RestCatalogTest, DISABLED_IntegrationTestFullNamespaceWorkflow) { + auto catalog_result = RestCatalog::Make(config_); + ASSERT_THAT(catalog_result, IsOk()); + auto catalog = std::move(catalog_result.value()); + + // 1. List initial namespaces + Namespace root{.levels = {}}; + auto list_result1 = catalog->ListNamespaces(root); + ASSERT_THAT(list_result1, IsOk()); + size_t initial_count = list_result1->size(); + + // 2. Create a new namespace + Namespace test_ns{.levels = {"integration_test_ns"}}; + std::unordered_map props = { + {"owner", "test"}, {"created_by", "rest_catalog_test"}}; + auto create_result = catalog->CreateNamespace(test_ns, props); + EXPECT_THAT(create_result, IsOk()); + + // 3. Verify namespace exists + auto exists_result = catalog->NamespaceExists(test_ns); + EXPECT_THAT(exists_result, HasValue(::testing::Eq(true))); + + // 4. List namespaces again (should have one more) + auto list_result2 = catalog->ListNamespaces(root); + ASSERT_THAT(list_result2, IsOk()); + EXPECT_EQ(list_result2->size(), initial_count + 1); + + // 5. Get namespace properties + auto props_result = catalog->GetNamespaceProperties(test_ns); + ASSERT_THAT(props_result, IsOk()); + EXPECT_EQ((*props_result)["owner"], "test"); + + // 6. Update properties + std::unordered_map updates = { + {"description", "test namespace"}}; + std::unordered_set removals = {}; + auto update_result = catalog->UpdateNamespaceProperties(test_ns, updates, removals); + EXPECT_THAT(update_result, IsOk()); + + // 7. Verify updated properties + auto props_result2 = catalog->GetNamespaceProperties(test_ns); + ASSERT_THAT(props_result2, IsOk()); + EXPECT_EQ((*props_result2)["description"], "test namespace"); + + // 8. Drop the namespace (cleanup) + auto drop_result = catalog->DropNamespace(test_ns); + EXPECT_THAT(drop_result, IsOk()); + + // 9. Verify namespace no longer exists + auto exists_result2 = catalog->NamespaceExists(test_ns); + EXPECT_THAT(exists_result2, HasValue(::testing::Eq(false))); } -} // namespace iceberg::catalog::rest +} // namespace iceberg::rest From a37fec796b236bf2a76ef52dd7e4654614a18a3b Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Tue, 18 Nov 2025 15:41:41 +0800 Subject: [PATCH 02/12] add HttpResponse --- src/iceberg/catalog/rest/config.cc | 23 +++----- src/iceberg/catalog/rest/config.h | 7 ++- src/iceberg/catalog/rest/constant.h | 21 ++++---- .../catalog/rest/http_client_interal.h | 51 +++++++++--------- .../catalog/rest/http_client_internal.cc | 53 ++++++------------- src/iceberg/catalog/rest/http_response.h | 46 ++++++++++++++++ src/iceberg/catalog/rest/meson.build | 2 +- 7 files changed, 109 insertions(+), 94 deletions(-) create mode 100644 src/iceberg/catalog/rest/http_response.h diff --git a/src/iceberg/catalog/rest/config.cc b/src/iceberg/catalog/rest/config.cc index 6220a301c..bf5a988e2 100644 --- a/src/iceberg/catalog/rest/config.cc +++ b/src/iceberg/catalog/rest/config.cc @@ -34,26 +34,19 @@ std::unique_ptr RestCatalogConfig::FromMap( return rest_catalog_config; } -Result RestCatalogConfig::GetExtraHeaders() const { - cpr::Header headers; - - headers[std::string(kHeaderContentType)] = std::string(kMimeTypeApplicationJson); - headers[std::string(kHeaderUserAgent)] = std::string(kUserAgent); - headers[std::string(kHeaderXClientVersion)] = std::string(kClientVersion); +std::unordered_map RestCatalogConfig::GetExtraHeaders() const { + std::unordered_map headers; + headers[kHeaderContentType] = kMimeTypeApplicationJson; + headers[kHeaderUserAgent] = kUserAgent; constexpr std::string_view prefix = "header."; for (const auto& [key, value] : configs_) { if (key.starts_with(prefix)) { - std::string_view header_name = std::string_view(key).substr(prefix.length()); - - if (header_name.empty()) { - return InvalidArgument("Header name cannot be empty after '{}' prefix", prefix); - } - - if (value.empty()) { - return InvalidArgument("Header value for '{}' cannot be empty", header_name); + std::string header_name = key.substr(prefix.length()); + if (header_name.empty() || value.empty()) { + continue; } - headers[std::string(header_name)] = value; + headers[header_name] = value; } } return headers; diff --git a/src/iceberg/catalog/rest/config.h b/src/iceberg/catalog/rest/config.h index b9d015af6..2c65b52de 100644 --- a/src/iceberg/catalog/rest/config.h +++ b/src/iceberg/catalog/rest/config.h @@ -21,12 +21,12 @@ #include #include +#include #include #include #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/result.h" #include "iceberg/util/config.h" /// \file iceberg/catalog/rest/config.h @@ -61,9 +61,8 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase GetExtraHeaders() const; + /// \return A map of header names to values. + std::unordered_map GetExtraHeaders() const; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/constant.h b/src/iceberg/catalog/rest/constant.h index 8f1e9fbf5..09ef33f37 100644 --- a/src/iceberg/catalog/rest/constant.h +++ b/src/iceberg/catalog/rest/constant.h @@ -19,7 +19,7 @@ #pragma once -#include +#include #include "iceberg/version.h" @@ -28,17 +28,16 @@ namespace iceberg::rest { -constexpr std::string_view kHeaderContentType = "Content-Type"; -constexpr std::string_view kHeaderAccept = "Accept"; -constexpr std::string_view kHeaderXClientVersion = "X-Client-Version"; -constexpr std::string_view kHeaderUserAgent = "User-Agent"; +inline const std::string kHeaderContentType = "Content-Type"; +inline const std::string kHeaderAccept = "Accept"; +inline const std::string kHeaderXClientVersion = "X-Client-Version"; +inline const std::string kHeaderUserAgent = "User-Agent"; -constexpr std::string_view kMimeTypeApplicationJson = "application/json"; -constexpr std::string_view kClientVersion = "0.14.1"; -constexpr std::string_view kUserAgentPrefix = "iceberg-cpp/"; -constexpr std::string_view kUserAgent = "iceberg-cpp/" ICEBERG_VERSION_STRING; +inline const std::string kMimeTypeApplicationJson = "application/json"; +inline const std::string kUserAgentPrefix = "iceberg-cpp/"; +inline const std::string kUserAgent = "iceberg-cpp/" ICEBERG_VERSION_STRING; -constexpr std::string_view kQueryParamParent = "parent"; -constexpr std::string_view kQueryParamPageToken = "page_token"; +inline const std::string kQueryParamParent = "parent"; +inline const std::string kQueryParamPageToken = "page_token"; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client_interal.h b/src/iceberg/catalog/rest/http_client_interal.h index ea89eda68..0dc71a042 100644 --- a/src/iceberg/catalog/rest/http_client_interal.h +++ b/src/iceberg/catalog/rest/http_client_interal.h @@ -27,6 +27,7 @@ #include #include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_response.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/result.h" @@ -36,17 +37,9 @@ namespace iceberg::rest { /// \brief HTTP client for making requests to Iceberg REST Catalog API. -/// -/// This class wraps the CPR library and provides a type-safe interface for making -/// HTTP requests. It handles authentication, headers, and response parsing. class ICEBERG_REST_EXPORT HttpClient { public: - /// \brief Factory function to create and initialize an HttpClient. - /// This is the preferred way to construct an HttpClient, as it can handle - /// potential errors during configuration parsing (e.g., invalid headers). - /// \param config The catalog configuration. - /// \return A Result containing a unique_ptr to the HttpClient, or an Error. - static Result> Make(const RestCatalogConfig& config); + explicit HttpClient(const RestCatalogConfig&); HttpClient(const HttpClient&) = delete; HttpClient& operator=(const HttpClient&) = delete; @@ -55,36 +48,42 @@ class ICEBERG_REST_EXPORT HttpClient { /// \brief Sends a GET request. /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). - Result Get(const std::string& target, const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); + Result Get(const std::string& target, const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); /// \brief Sends a POST request. /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). - Result Post(const std::string& target, const cpr::Body& body, - const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); + Result Post(const std::string& target, const cpr::Body& body, + const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); /// \brief Sends a HEAD request. /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). - Result Head(const std::string& target, - const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); + Result Head(const std::string& target, const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); /// \brief Sends a DELETE request. /// \param target The target path relative to the base URL (e.g., "/v - Result Delete(const std::string& target, - const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); + Result Delete(const std::string& target, + const cpr::Parameters& params = {}, + const cpr::Header& headers = {}); private: - /// \brief Private constructor. Use the static Create() factory function instead. - explicit HttpClient(cpr::Header session_headers); - /// \brief Internal helper to execute a request. template - Result Execute(const std::string& target, const cpr::Parameters& params, - const cpr::Header& request_headers, - Func&& perform_request); + Result Execute(const std::string& target, const cpr::Parameters& params, + const cpr::Header& request_headers, + Func&& perform_request) { + cpr::Header combined_headers = default_headers_; + combined_headers.insert(request_headers.begin(), request_headers.end()); + + session_->SetUrl(cpr::Url{target}); + session_->SetParameters(params); + session_->SetHeader(combined_headers); + + cpr::Response response = perform_request(*session_); + return HttpResponse{std::move(response)}; + } cpr::Header default_headers_; std::unique_ptr session_; diff --git a/src/iceberg/catalog/rest/http_client_internal.cc b/src/iceberg/catalog/rest/http_client_internal.cc index 737e07066..dc0429e30 100644 --- a/src/iceberg/catalog/rest/http_client_internal.cc +++ b/src/iceberg/catalog/rest/http_client_internal.cc @@ -20,65 +20,44 @@ #include #include "cpr/body.h" +#include "cpr/cprtypes.h" #include "iceberg/catalog/rest/config.h" #include "iceberg/catalog/rest/http_client_interal.h" -#include "iceberg/util/macros.h" namespace iceberg::rest { -Result> HttpClient::Make(const RestCatalogConfig& config) { - ICEBERG_ASSIGN_OR_RAISE(auto session_headers, config.GetExtraHeaders()); - return std::unique_ptr(new HttpClient(std::move(session_headers))); -} - -HttpClient::HttpClient(cpr::Header session_headers) - : default_headers_(std::move(session_headers)), - session_(std::make_unique()) {} +HttpClient::HttpClient(const RestCatalogConfig& config) + : default_headers_{config.GetExtraHeaders().begin(), config.GetExtraHeaders().end()}, + session_{std::make_unique()} {} -Result HttpClient::Get(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& headers) { +Result HttpClient::Get(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& headers) { return Execute(target, params, headers, [&](cpr::Session& session) { return session.Get(); }); } -Result HttpClient::Post(const std::string& target, const cpr::Body& body, - const cpr::Parameters& params, - const cpr::Header& headers) { +Result HttpClient::Post(const std::string& target, const cpr::Body& body, + const cpr::Parameters& params, + const cpr::Header& headers) { return Execute(target, params, headers, [&](cpr::Session& session) { session.SetBody(body); return session.Post(); }); } -Result HttpClient::Head(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& headers) { +Result HttpClient::Head(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& headers) { return Execute(target, params, headers, [&](cpr::Session& session) { return session.Head(); }); } -Result HttpClient::Delete(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& headers) { +Result HttpClient::Delete(const std::string& target, + const cpr::Parameters& params, + const cpr::Header& headers) { return Execute(target, params, headers, [&](cpr::Session& session) { return session.Delete(); }); } -template -Result HttpClient::Execute(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& request_headers, - Func&& perform_request) { - cpr::Header combined_headers = default_headers_; - combined_headers.insert(request_headers.begin(), request_headers.end()); - - session_->SetUrl(cpr::Url{target}); - session_->SetParameters(params); - session_->SetHeader(combined_headers); - - cpr::Response response = perform_request(*session_); - return response; -} - } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_response.h b/src/iceberg/catalog/rest/http_response.h new file mode 100644 index 000000000..980075ad1 --- /dev/null +++ b/src/iceberg/catalog/rest/http_response.h @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include + +#include "cpr/response.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" + +/// \file iceberg/catalog/rest/http_response.h +/// \brief A simple wrapper for cpr::Response. This class encapsulates the details of the +/// underlying cpr library's response, providing a consistent interface that is +/// independent of the specific network library used. + +class ICEBERG_REST_EXPORT HttpResponse { + public: + explicit HttpResponse(cpr::Response response) : response_(std::move(response)) {} + + /// \brief Get the HTTP status code of the response. + /// \return The HTTP status code. + int32_t status_code() const { return response_.status_code; } + + /// \brief Get the body of the response as a string. + /// \return The response body. + const std::string& body() const { return response_.text; } + + private: + cpr::Response response_; +}; diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index 30d86b96a..fd38dcf80 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -61,7 +61,7 @@ install_headers( 'json_internal.h', 'types.h', 'util.h', - 'resource_path.h', + 'resource_paths.h', ], subdir: 'iceberg/catalog/rest', ) From ceedea39302c42486247acfd74821dcaeace5659 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Wed, 19 Nov 2025 18:05:41 +0800 Subject: [PATCH 03/12] update --- src/iceberg/catalog/rest/CMakeLists.txt | 2 +- src/iceberg/catalog/rest/catalog.cc | 91 ++++---- src/iceberg/catalog/rest/catalog.h | 2 +- src/iceberg/catalog/rest/config.cc | 2 +- src/iceberg/catalog/rest/config.h | 2 +- src/iceberg/catalog/rest/endpoint_util.h | 10 +- src/iceberg/catalog/rest/error_handlers.h | 204 ++++++++++++++++++ src/iceberg/catalog/rest/http_client.cc | 160 ++++++++++++++ src/iceberg/catalog/rest/http_client.h | 94 ++++++++ .../catalog/rest/http_client_interal.h | 92 -------- .../catalog/rest/http_client_internal.cc | 63 ------ src/iceberg/catalog/rest/http_response.h | 8 +- src/iceberg/catalog/rest/meson.build | 11 +- src/iceberg/catalog/rest/resource_paths.cc | 2 - src/iceberg/catalog/rest/resource_paths.h | 22 +- src/iceberg/catalog/rest/types.h | 1 - src/iceberg/result.h | 16 ++ src/iceberg/test/rest_catalog_test.cc | 6 +- 18 files changed, 539 insertions(+), 249 deletions(-) create mode 100644 src/iceberg/catalog/rest/error_handlers.h create mode 100644 src/iceberg/catalog/rest/http_client.cc create mode 100644 src/iceberg/catalog/rest/http_client.h delete mode 100644 src/iceberg/catalog/rest/http_client_interal.h delete mode 100644 src/iceberg/catalog/rest/http_client_internal.cc diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index e230646b1..e2850abb0 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -19,7 +19,7 @@ set(ICEBERG_REST_SOURCES catalog.cc json_internal.cc config.cc - http_client_internal.cc + http_client.cc resource_paths.cc) set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/rest/catalog.cc b/src/iceberg/catalog/rest/catalog.cc index c4f0a88c6..bf1ea0a58 100644 --- a/src/iceberg/catalog/rest/catalog.cc +++ b/src/iceberg/catalog/rest/catalog.cc @@ -20,14 +20,18 @@ #include "iceberg/catalog/rest/catalog.h" #include +#include #include #include +#include "iceberg/catalog/rest/catalog.h" #include "iceberg/catalog/rest/config.h" #include "iceberg/catalog/rest/constant.h" #include "iceberg/catalog/rest/endpoint_util.h" -#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/http_response.h" #include "iceberg/catalog/rest/json_internal.h" #include "iceberg/catalog/rest/types.h" #include "iceberg/json_internal.h" @@ -41,37 +45,27 @@ Result> RestCatalog::Make(const RestCatalogConfig& // Create ResourcePaths and validate URI ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); - ICEBERG_ASSIGN_OR_RAISE(auto tmp_client, HttpClient::Make(config)); - + auto tmp_client = std::make_unique(config); const std::string endpoint = paths->V1Config(); - cpr::Parameters params; - ICEBERG_ASSIGN_OR_RAISE(const auto& response, tmp_client->Get(endpoint, params)); - switch (response.status_code) { - case cpr::status::HTTP_OK: { - ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); - ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); - // Merge server config into client config, server config overrides > client config - // properties > server config defaults - auto final_props = std::move(server_config.defaults); - for (const auto& kv : config.configs()) { - final_props.insert_or_assign(kv.first, kv.second); - } - - for (const auto& kv : server_config.overrides) { - final_props.insert_or_assign(kv.first, kv.second); - } - auto final_config = RestCatalogConfig::FromMap(final_props); - ICEBERG_ASSIGN_OR_RAISE(auto client, HttpClient::Make(*final_config)); - ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config)); - return std::unique_ptr(new RestCatalog( - std::move(final_config), std::move(client), std::move(*final_paths))); - }; - default: { - ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); - ICEBERG_ASSIGN_OR_RAISE(auto list_response, ErrorResponseFromJson(json)); - return UnknownError("Error listing namespaces: {}", list_response.error.message); - } + ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, + tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); + ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); + // Merge server config into client config, server config overrides > client config + // properties > server config defaults + auto final_props = std::move(server_config.defaults); + for (const auto& kv : config.configs()) { + final_props.insert_or_assign(kv.first, kv.second); + } + + for (const auto& kv : server_config.overrides) { + final_props.insert_or_assign(kv.first, kv.second); } + auto final_config = RestCatalogConfig::FromMap(final_props); + auto client = std::make_unique(*final_config); + ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config)); + return std::unique_ptr(new RestCatalog( + std::move(final_config), std::move(client), std::move(*final_paths))); } RestCatalog::RestCatalog(std::unique_ptr config, @@ -91,35 +85,26 @@ Result> RestCatalog::ListNamespaces(const Namespace& ns) std::vector result; std::string next_token; while (true) { - cpr::Parameters params; + std::unordered_map params; if (!ns.levels.empty()) { - params.Add({std::string(kQueryParamParent), EncodeNamespaceForUrl(ns)}); + params[kQueryParamParent] = EncodeNamespaceForUrl(ns); } if (!next_token.empty()) { - params.Add({std::string(kQueryParamPageToken), next_token}); + params[kQueryParamPageToken] = next_token; } - ICEBERG_ASSIGN_OR_RAISE(const auto& response, client_->Get(endpoint, params)); - switch (response.status_code) { - case cpr::status::HTTP_OK: { - ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); - ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json)); - result.insert(result.end(), list_response.namespaces.begin(), - list_response.namespaces.end()); - if (list_response.next_page_token.empty()) { - return result; - } - next_token = list_response.next_page_token; - continue; - } - case cpr::status::HTTP_NOT_FOUND: { - return NoSuchNamespace("Namespace not found"); - } - default: - ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); - ICEBERG_ASSIGN_OR_RAISE(auto list_response, ErrorResponseFromJson(json)); - return UnknownError("Error listing namespaces: {}", list_response.error.message); + ICEBERG_ASSIGN_OR_RAISE(const auto& response, + client_->Get(endpoint, params, {}, NamespaceErrorHandler())); + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); + ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json)); + result.insert(result.end(), list_response.namespaces.begin(), + list_response.namespaces.end()); + if (list_response.next_page_token.empty()) { + return result; } + next_token = list_response.next_page_token; + continue; } + return result; } Status RestCatalog::CreateNamespace( diff --git a/src/iceberg/catalog/rest/catalog.h b/src/iceberg/catalog/rest/catalog.h index 66d7ceccb..78581de6f 100644 --- a/src/iceberg/catalog/rest/catalog.h +++ b/src/iceberg/catalog/rest/catalog.h @@ -24,7 +24,7 @@ #include "iceberg/catalog.h" #include "iceberg/catalog/rest/config.h" -#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/catalog/rest/http_client.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/catalog/rest/resource_paths.h" #include "iceberg/result.h" diff --git a/src/iceberg/catalog/rest/config.cc b/src/iceberg/catalog/rest/config.cc index bf5a988e2..aa4b31a90 100644 --- a/src/iceberg/catalog/rest/config.cc +++ b/src/iceberg/catalog/rest/config.cc @@ -23,7 +23,7 @@ namespace iceberg::rest { -std::unique_ptr RestCatalogConfig::default_properties() { +std::unique_ptr RestCatalogConfig::DefaultProperties() { return std::make_unique(); } diff --git a/src/iceberg/catalog/rest/config.h b/src/iceberg/catalog/rest/config.h index 2c65b52de..02fd1f72a 100644 --- a/src/iceberg/catalog/rest/config.h +++ b/src/iceberg/catalog/rest/config.h @@ -50,7 +50,7 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase default_properties(); + static std::unique_ptr DefaultProperties(); /// \brief Create a RestCatalogConfig instance from a map of key-value pairs. static std::unique_ptr FromMap( diff --git a/src/iceberg/catalog/rest/endpoint_util.h b/src/iceberg/catalog/rest/endpoint_util.h index 8aa4a69c9..0fa0124f0 100644 --- a/src/iceberg/catalog/rest/endpoint_util.h +++ b/src/iceberg/catalog/rest/endpoint_util.h @@ -31,11 +31,11 @@ namespace iceberg::rest { /// \brief Trim a single trailing slash from a URI string_view. /// \details If \p uri_sv ends with '/', remove that last character; otherwise the input /// is returned unchanged. -/// \param uri_sv The URI string view to trim. -/// \return The (possibly) trimmed URI string view. -inline std::string_view TrimTrailingSlash(std::string_view uri_sv) { - if (uri_sv.ends_with('/')) { - uri_sv.remove_suffix(1); +/// \param uri_sv The URI string to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); } return uri_sv; } diff --git a/src/iceberg/catalog/rest/error_handlers.h b/src/iceberg/catalog/rest/error_handlers.h new file mode 100644 index 000000000..000082269 --- /dev/null +++ b/src/iceberg/catalog/rest/error_handlers.h @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/error_handlers.h +/// Error handlers for different HTTP error types in Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Error handler interface for processing REST API error responses. Maps HTTP +/// status codes to appropriate ErrorKind values following the Iceberg REST specification. +class ICEBERG_REST_EXPORT ErrorHandler { + public: + virtual ~ErrorHandler() = default; + + /// \brief Process an error response and return an appropriate Error. + /// + /// \param error The error model parsed from the HTTP response body + /// \return An Error object with appropriate ErrorKind and message + virtual Status Accept(const ErrorModel& error) const = 0; +}; + +/// \brief Default error handler for REST API responses. +class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 400: + if (error.type == "IllegalArgumentException") { + return InvalidArgument("{}", error.message); + } + return BadRequest("Malformed request: {}", error.message); + + case 401: + return NotAuthorized("Not authorized: {}", error.message); + + case 403: + return Forbidden("Forbidden: {}", error.message); + + case 405: + case 406: + break; + + case 500: + return ServiceFailure("Server error: {}: {}", error.type, error.message); + + case 501: + return NotSupported("{}", error.message); + + case 503: + return ServiceUnavailable("Service unavailable: {}", error.message); + } + return RESTError("Unable to process: {}", error.message); + } +}; + +/// \brief Namespace-specific error handler for create/read/update operations. +class ICEBERG_REST_EXPORT NamespaceErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 400: + if (error.type == "NamespaceNotEmptyException") { + return NamespaceNotEmpty("{}", error.message); + } + return BadRequest("Malformed request: {}", error.message); + + case 404: + return NoSuchNamespace("{}", error.message); + + case 409: + return AlreadyExists("{}", error.message); + + case 422: + return RESTError("Unable to process: {}", error.message); + } + + return DefaultErrorHandler::Accept(error); + } +}; + +/// \brief Error handler for drop namespace operations. +class ICEBERG_REST_EXPORT DropNamespaceErrorHandler : public NamespaceErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + if (error.code == 409) { + return NamespaceNotEmpty("{}", error.message); + } + + // Delegate to parent handler + return NamespaceErrorHandler::Accept(error); + } +}; + +/// \brief Table-level error handler. +class ICEBERG_REST_EXPORT TableErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 404: + if (error.type == "NoSuchNamespaceException") { + return NoSuchNamespace("{}", error.message); + } + return NoSuchTable("{}", error.message); + + case 409: + return AlreadyExists("{}", error.message); + } + + return DefaultErrorHandler::Accept(error); + } +}; + +/// \brief View-level error handler. +/// +/// Handles view-specific errors including NoSuchView, NoSuchNamespace, +/// and AlreadyExists scenarios. +class ICEBERG_REST_EXPORT ViewErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 404: + if (error.type == "NoSuchNamespaceException") { + return NoSuchNamespace("{}", error.message); + } + return NoSuchView("{}", error.message); + + case 409: + return AlreadyExists("{}", error.message); + } + + return DefaultErrorHandler::Accept(error); + } +}; + +/// \brief Table commit operation error handler. +class ICEBERG_REST_EXPORT TableCommitErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 404: + return NoSuchTable("{}", error.message); + + case 409: + return CommitFailed("Commit failed: {}", error.message); + + case 500: + case 502: + case 503: + case 504: + return CommitStateUnknown("Service failed: {}: {}", error.code, error.message); + } + + return DefaultErrorHandler::Accept(error); + } +}; + +/// \brief View commit operation error handler. +class ICEBERG_REST_EXPORT ViewCommitErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 404: + return NoSuchView("{}", error.message); + + case 409: + return CommitFailed("Commit failed: {}", error.message); + + case 500: + case 502: + case 503: + case 504: + return CommitStateUnknown("Service failed: {}: {}", error.code, error.message); + } + + return DefaultErrorHandler::Accept(error); + } +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc new file mode 100644 index 000000000..16ca4010b --- /dev/null +++ b/src/iceberg/catalog/rest/http_client.cc @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/http_client.h" + +#include + +#include "cpr/body.h" +#include "cpr/cprtypes.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/json_internal.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +namespace { + +/// \brief Merges global default headers with request-specific headers. +/// +/// Combines the global headers derived from RestCatalogConfig with the headers +/// passed in the specific request. Request-specific headers have higher priority +/// and will override global defaults if the keys conflict (e.g., overriding +/// the default "Content-Type"). +cpr::Header MergeHeaders(const std::unordered_map& defaults, + const std::unordered_map& overrides) { + cpr::Header combined_headers = {defaults.begin(), defaults.end()}; + for (const auto& [key, val] : overrides) { + combined_headers.insert_or_assign(key, val); + } + return combined_headers; +} + +/// \brief Converts a map of string key-value pairs to cpr::Parameters. +cpr::Parameters GetParameters( + const std::unordered_map& params) { + cpr::Parameters cpr_params; + for (const auto& [key, val] : params) { + cpr_params.Add({key, val}); + } + return cpr_params; +} + +bool IsSuccessful(int32_t status_code) { + return status_code == 200 // OK + || status_code == 202 // Accepted + || status_code == 204 // No Content + || status_code == 304; // Not Modified +} + +Status HandleFailureResponse(const cpr::Response& response, + const ErrorHandler& error_handler) { + if (!IsSuccessful(response.status_code)) { + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); + ICEBERG_ASSIGN_OR_RAISE(auto error_response, ErrorResponseFromJson(json)); + return error_handler.Accept(error_response.error); + } + return {}; +} + +} // namespace + +void HttpClient::PrepareSession( + const std::string& path, + const std::unordered_map& request_headers, + const std::unordered_map& params) { + session_->SetUrl(cpr::Url{path}); + session_->SetParameters(GetParameters(params)); + session_->RemoveContent(); + auto final_headers = MergeHeaders(default_headers_, request_headers); + session_->SetHeader(final_headers); +} + +HttpClient::HttpClient(const RestCatalogConfig& config) + : default_headers_{config.GetExtraHeaders()}, + session_{std::make_unique()} {} + +Result HttpClient::Get( + const std::string& path, const std::unordered_map& params, + const std::unordered_map& headers, + const ErrorHandler& error_handler) { + std::lock_guard lock(session_mutex_); + + PrepareSession(path, headers, params); + cpr::Response response = session_->Get(); + ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); + return HttpResponse(std::move(response)); +} + +Result HttpClient::Post( + const std::string& path, const std::string& body, + const std::unordered_map& headers, + const ErrorHandler& error_handler) { + std::lock_guard lock(session_mutex_); + + PrepareSession(path, headers); + session_->SetBody(cpr::Body{body}); + cpr::Response response = session_->Post(); + ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); + return HttpResponse(std::move(response)); +} + +Result HttpClient::PostForm( + const std::string& path, + const std::unordered_map& form_data, + const std::unordered_map& headers, + const ErrorHandler& error_handler) { + std::lock_guard lock(session_mutex_); + + PrepareSession(path, headers); + std::vector pair_list; + pair_list.reserve(form_data.size()); + for (const auto& [key, val] : form_data) { + pair_list.emplace_back(key, val); + } + session_->SetPayload(cpr::Payload(pair_list.begin(), pair_list.end())); + cpr::Response response = session_->Post(); + ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); + return HttpResponse(std::move(response)); +} + +Result HttpClient::Head( + const std::string& path, const std::unordered_map& headers, + const ErrorHandler& error_handler) { + std::lock_guard lock(session_mutex_); + + PrepareSession(path, headers); + cpr::Response response = session_->Head(); + ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); + return HttpResponse(std::move(response)); +} + +Result HttpClient::Delete( + const std::string& path, const std::unordered_map& headers, + const ErrorHandler& error_handler) { + std::lock_guard lock(session_mutex_); + + PrepareSession(path, headers); + cpr::Response response = session_->Delete(); + ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); + return HttpResponse(std::move(response)); +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client.h b/src/iceberg/catalog/rest/http_client.h new file mode 100644 index 000000000..9e5528b2e --- /dev/null +++ b/src/iceberg/catalog/rest/http_client.h @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include +#include +#include +#include + +#include +#include + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_response.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/http_client.h +/// \brief Http client for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief HTTP client for making requests to Iceberg REST Catalog API. +class ICEBERG_REST_EXPORT HttpClient { + public: + explicit HttpClient(const RestCatalogConfig&); + + HttpClient(const HttpClient&) = delete; + HttpClient& operator=(const HttpClient&) = delete; + HttpClient(HttpClient&&) = delete; + HttpClient& operator=(HttpClient&&) = delete; + + /// \brief Sends a GET request. + Result Get(const std::string& path, + const std::unordered_map& params, + const std::unordered_map& headers, + const ErrorHandler& error_handler); + + /// \brief Sends a POST request. + Result Post(const std::string& path, const std::string& body, + const std::unordered_map& headers, + const ErrorHandler& error_handler); + + /// \brief Sends a POST request with form data. + Result PostForm( + const std::string& path, + const std::unordered_map& form_data, + const std::unordered_map& headers, + const ErrorHandler& error_handler); + + /// \brief Sends a HEAD request. + Result Head(const std::string& path, + const std::unordered_map& headers, + const ErrorHandler& error_handler); + + /// \brief Sends a DELETE request. + Result Delete(const std::string& path, + const std::unordered_map& headers, + const ErrorHandler& error_handler); + + private: + void PrepareSession(const std::string& path, + const std::unordered_map& request_headers, + const std::unordered_map& params = {}); + + std::unordered_map default_headers_; + + // Mutex to protect the non-thread-safe cpr::Session. + mutable std::mutex session_mutex_; + + // TODO(Li Feiyang): use connection pool to support external multi-threaded concurrent + // calls + std::unique_ptr session_; +}; + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client_interal.h b/src/iceberg/catalog/rest/http_client_interal.h deleted file mode 100644 index 0dc71a042..000000000 --- a/src/iceberg/catalog/rest/http_client_interal.h +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#pragma once - -#include -#include -#include - -#include -#include - -#include "iceberg/catalog/rest/config.h" -#include "iceberg/catalog/rest/http_response.h" -#include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/result.h" - -/// \file iceberg/catalog/rest/http_client.h -/// \brief Http client for Iceberg REST API. - -namespace iceberg::rest { - -/// \brief HTTP client for making requests to Iceberg REST Catalog API. -class ICEBERG_REST_EXPORT HttpClient { - public: - explicit HttpClient(const RestCatalogConfig&); - - HttpClient(const HttpClient&) = delete; - HttpClient& operator=(const HttpClient&) = delete; - HttpClient(HttpClient&&) = default; - HttpClient& operator=(HttpClient&&) = default; - - /// \brief Sends a GET request. - /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). - Result Get(const std::string& target, const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); - - /// \brief Sends a POST request. - /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). - Result Post(const std::string& target, const cpr::Body& body, - const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); - - /// \brief Sends a HEAD request. - /// \param target The target path relative to the base URL (e.g., "/v1/namespaces"). - Result Head(const std::string& target, const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); - - /// \brief Sends a DELETE request. - /// \param target The target path relative to the base URL (e.g., "/v - Result Delete(const std::string& target, - const cpr::Parameters& params = {}, - const cpr::Header& headers = {}); - - private: - /// \brief Internal helper to execute a request. - template - Result Execute(const std::string& target, const cpr::Parameters& params, - const cpr::Header& request_headers, - Func&& perform_request) { - cpr::Header combined_headers = default_headers_; - combined_headers.insert(request_headers.begin(), request_headers.end()); - - session_->SetUrl(cpr::Url{target}); - session_->SetParameters(params); - session_->SetHeader(combined_headers); - - cpr::Response response = perform_request(*session_); - return HttpResponse{std::move(response)}; - } - - cpr::Header default_headers_; - std::unique_ptr session_; -}; - -} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client_internal.cc b/src/iceberg/catalog/rest/http_client_internal.cc deleted file mode 100644 index dc0429e30..000000000 --- a/src/iceberg/catalog/rest/http_client_internal.cc +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#include - -#include "cpr/body.h" -#include "cpr/cprtypes.h" -#include "iceberg/catalog/rest/config.h" -#include "iceberg/catalog/rest/http_client_interal.h" - -namespace iceberg::rest { - -HttpClient::HttpClient(const RestCatalogConfig& config) - : default_headers_{config.GetExtraHeaders().begin(), config.GetExtraHeaders().end()}, - session_{std::make_unique()} {} - -Result HttpClient::Get(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& headers) { - return Execute(target, params, headers, - [&](cpr::Session& session) { return session.Get(); }); -} - -Result HttpClient::Post(const std::string& target, const cpr::Body& body, - const cpr::Parameters& params, - const cpr::Header& headers) { - return Execute(target, params, headers, [&](cpr::Session& session) { - session.SetBody(body); - return session.Post(); - }); -} - -Result HttpClient::Head(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& headers) { - return Execute(target, params, headers, - [&](cpr::Session& session) { return session.Head(); }); -} - -Result HttpClient::Delete(const std::string& target, - const cpr::Parameters& params, - const cpr::Header& headers) { - return Execute(target, params, headers, - [&](cpr::Session& session) { return session.Delete(); }); -} - -} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_response.h b/src/iceberg/catalog/rest/http_response.h index 980075ad1..1e21c22cc 100644 --- a/src/iceberg/catalog/rest/http_response.h +++ b/src/iceberg/catalog/rest/http_response.h @@ -20,6 +20,7 @@ #pragma once #include +#include #include "cpr/response.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" @@ -34,13 +35,16 @@ class ICEBERG_REST_EXPORT HttpResponse { explicit HttpResponse(cpr::Response response) : response_(std::move(response)) {} /// \brief Get the HTTP status code of the response. - /// \return The HTTP status code. int32_t status_code() const { return response_.status_code; } /// \brief Get the body of the response as a string. - /// \return The response body. const std::string& body() const { return response_.text; } + /// \brief Get the headers of the response as a map. + const std::unordered_map headers() const { + return {response_.header.begin(), response_.header.end()}; + } + private: cpr::Response response_; }; diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index fd38dcf80..a528eee7f 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -18,9 +18,9 @@ iceberg_rest_sources = files( 'catalog.cc', 'config.cc', - 'http_client_internal.cc', + 'http_client.cc', 'json_internal.cc', - 'resource_path.cc', + 'resource_paths.cc', ) # cpr does not export symbols, so on Windows it must # be used as a static lib @@ -57,11 +57,14 @@ install_headers( 'catalog.h', 'config.h', 'constant.h', - 'http_client_interal.h', + 'http_client.h', + 'http_response.h', 'json_internal.h', + 'iceberg_rest_export.h', 'types.h', - 'util.h', 'resource_paths.h', + 'endpoint_util.h', + 'error_handlers.h', ], subdir: 'iceberg/catalog/rest', ) diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc index 24e750c9d..4ca15dd69 100644 --- a/src/iceberg/catalog/rest/resource_paths.cc +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -22,10 +22,8 @@ #include #include "iceberg/catalog/rest/config.h" -#include "iceberg/catalog/rest/constant.h" #include "iceberg/catalog/rest/endpoint_util.h" #include "iceberg/result.h" -#include "iceberg/util/macros.h" namespace iceberg::rest { diff --git a/src/iceberg/catalog/rest/resource_paths.h b/src/iceberg/catalog/rest/resource_paths.h index 19c0ec64b..e5b4095d8 100644 --- a/src/iceberg/catalog/rest/resource_paths.h +++ b/src/iceberg/catalog/rest/resource_paths.h @@ -42,81 +42,63 @@ class ICEBERG_REST_EXPORT ResourcePaths { static Result> Make(const RestCatalogConfig& config); /// \brief Get the /v1/{prefix}/config endpoint path. - /// \return The endpoint URL std::string V1Config() const; /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. - /// \return The endpoint URL std::string V1OAuth2Tokens() const; /// \brief Get the /v1/{prefix}/namespaces endpoint path. - /// \return The endpoint URL std::string V1Namespaces() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. - /// \return The endpoint URL std::string V1Namespace(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. - /// \return The endpoint URL std::string V1NamespaceProperties(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. - /// \return The endpoint URL std::string V1Tables(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. - /// \return The endpoint URL std::string V1Table(const TableIdentifier& table) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. - /// \return The endpoint URL std::string V1RegisterTable(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/tables/rename endpoint path. - /// \return The endpoint URL std::string V1RenameTable() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint /// path. - /// \return The endpoint URL std::string V1TableMetrics(const TableIdentifier& table) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials /// endpoint path. - /// \return The endpoint URL std::string V1TableCredentials(const TableIdentifier& table) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint /// path. - /// \return The endpoint URL std::string V1TableScanPlan(const TableIdentifier& table) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{planId} /// endpoint path. - /// \return The endpoint URL std::string V1TableScanPlanResult(const TableIdentifier& table, const std::string& plan_id) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint /// path. - /// \return The endpoint URL std::string V1TableTasks(const TableIdentifier& table) const; /// \brief Get the /v1/{prefix}/transactions/commit endpoint path. - /// \return The endpoint URL std::string V1TransactionCommit() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views endpoint path. - /// \return The endpoint URL std::string V1Views(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views/{view} endpoint path. - /// \return The endpoint URL std::string V1View(const TableIdentifier& view) const; /// \brief Get the /v1/{prefix}/views/rename endpoint path. - /// \return The endpoint URL std::string V1RenameView() const; private: @@ -125,8 +107,8 @@ class ICEBERG_REST_EXPORT ResourcePaths { // Helper to build path with optional prefix: {base_uri_}/{prefix_?}/{path} std::string BuildPath(std::string_view path) const; - std::string base_uri_; // URI with /v1, e.g., "http://localhost:8181/v1" - std::string prefix_; // Optional prefix from config + std::string base_uri_; + std::string prefix_; // Optional prefix from config }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/types.h b/src/iceberg/catalog/rest/types.h index 2e32f967f..50cdcc01a 100644 --- a/src/iceberg/catalog/rest/types.h +++ b/src/iceberg/catalog/rest/types.h @@ -19,7 +19,6 @@ #pragma once -#include #include #include #include diff --git a/src/iceberg/result.h b/src/iceberg/result.h index 99df37247..7861d44c7 100644 --- a/src/iceberg/result.h +++ b/src/iceberg/result.h @@ -30,9 +30,11 @@ namespace iceberg { /// \brief Error types for iceberg. enum class ErrorKind { kAlreadyExists, + kBadRequest, kCommitFailed, kCommitStateUnknown, kDecompressError, + kForbidden, kInvalid, // For general invalid errors kInvalidArgument, kInvalidArrowData, @@ -42,13 +44,19 @@ enum class ErrorKind { kInvalidManifestList, kIOError, kJsonParseError, + kNamespaceNotEmpty, kNoSuchNamespace, kNoSuchTable, + kNoSuchView, kNotAllowed, + kNotAuthorized, kNotFound, kNotImplemented, kNotSupported, + kServiceFailure, + kServiceUnavailable, kUnknownError, + kRESTError, }; /// \brief Error with a kind and a message. @@ -79,9 +87,11 @@ using Status = Result; } DEFINE_ERROR_FUNCTION(AlreadyExists) +DEFINE_ERROR_FUNCTION(BadRequest) DEFINE_ERROR_FUNCTION(CommitFailed) DEFINE_ERROR_FUNCTION(CommitStateUnknown) DEFINE_ERROR_FUNCTION(DecompressError) +DEFINE_ERROR_FUNCTION(Forbidden) DEFINE_ERROR_FUNCTION(Invalid) DEFINE_ERROR_FUNCTION(InvalidArgument) DEFINE_ERROR_FUNCTION(InvalidArrowData) @@ -91,13 +101,19 @@ DEFINE_ERROR_FUNCTION(InvalidManifest) DEFINE_ERROR_FUNCTION(InvalidManifestList) DEFINE_ERROR_FUNCTION(IOError) DEFINE_ERROR_FUNCTION(JsonParseError) +DEFINE_ERROR_FUNCTION(NamespaceNotEmpty) DEFINE_ERROR_FUNCTION(NoSuchNamespace) DEFINE_ERROR_FUNCTION(NoSuchTable) +DEFINE_ERROR_FUNCTION(NoSuchView) DEFINE_ERROR_FUNCTION(NotAllowed) +DEFINE_ERROR_FUNCTION(NotAuthorized) DEFINE_ERROR_FUNCTION(NotFound) DEFINE_ERROR_FUNCTION(NotImplemented) DEFINE_ERROR_FUNCTION(NotSupported) +DEFINE_ERROR_FUNCTION(ServiceFailure) +DEFINE_ERROR_FUNCTION(ServiceUnavailable) DEFINE_ERROR_FUNCTION(UnknownError) +DEFINE_ERROR_FUNCTION(RESTError) #undef DEFINE_ERROR_FUNCTION diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index 7b30a36f7..9ba35d370 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -61,7 +61,7 @@ class RestCatalogTest : public ::testing::Test { RestCatalogConfig config_; }; -TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) { +TEST_F(RestCatalogTest, MakeCatalogSuccess) { auto catalog_result = RestCatalog::Make(config_); EXPECT_THAT(catalog_result, IsOk()); @@ -82,7 +82,7 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogEmptyUri) { EXPECT_THAT(catalog_result, HasErrorMessage("uri")); } -TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { +TEST_F(RestCatalogTest, MakeCatalogWithCustomProperties) { RestCatalogConfig custom_config = config_; custom_config .Set(RestCatalogConfig::Entry{"custom_prop", ""}, @@ -93,7 +93,7 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { EXPECT_THAT(catalog_result, IsOk()); } -TEST_F(RestCatalogTest, DISABLED_ListNamespaces) { +TEST_F(RestCatalogTest, ListNamespaces) { auto catalog_result = RestCatalog::Make(config_); ASSERT_THAT(catalog_result, IsOk()); auto& catalog = catalog_result.value(); From 56ebfb2671c3e68aca4cce448e4e7b87f54910f6 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Wed, 19 Nov 2025 18:16:16 +0800 Subject: [PATCH 04/12] disable test --- src/iceberg/test/rest_catalog_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index 9ba35d370..7b30a36f7 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -61,7 +61,7 @@ class RestCatalogTest : public ::testing::Test { RestCatalogConfig config_; }; -TEST_F(RestCatalogTest, MakeCatalogSuccess) { +TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) { auto catalog_result = RestCatalog::Make(config_); EXPECT_THAT(catalog_result, IsOk()); @@ -82,7 +82,7 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogEmptyUri) { EXPECT_THAT(catalog_result, HasErrorMessage("uri")); } -TEST_F(RestCatalogTest, MakeCatalogWithCustomProperties) { +TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { RestCatalogConfig custom_config = config_; custom_config .Set(RestCatalogConfig::Entry{"custom_prop", ""}, @@ -93,7 +93,7 @@ TEST_F(RestCatalogTest, MakeCatalogWithCustomProperties) { EXPECT_THAT(catalog_result, IsOk()); } -TEST_F(RestCatalogTest, ListNamespaces) { +TEST_F(RestCatalogTest, DISABLED_ListNamespaces) { auto catalog_result = RestCatalog::Make(config_); ASSERT_THAT(catalog_result, IsOk()); auto& catalog = catalog_result.value(); From ad511371bcc9e352fca208011d5bd172a9162279 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Thu, 20 Nov 2025 11:12:12 +0800 Subject: [PATCH 05/12] update --- src/iceberg/catalog/rest/http_client.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index 16ca4010b..c06619081 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -95,7 +95,7 @@ Result HttpClient::Get( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::lock_guard lock(session_mutex_); + std::scoped_lock lock(session_mutex_); PrepareSession(path, headers, params); cpr::Response response = session_->Get(); @@ -107,7 +107,7 @@ Result HttpClient::Post( const std::string& path, const std::string& body, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::lock_guard lock(session_mutex_); + std::scoped_lock lock(session_mutex_); PrepareSession(path, headers); session_->SetBody(cpr::Body{body}); @@ -121,7 +121,7 @@ Result HttpClient::PostForm( const std::unordered_map& form_data, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::lock_guard lock(session_mutex_); + std::scoped_lock lock(session_mutex_); PrepareSession(path, headers); std::vector pair_list; @@ -138,7 +138,7 @@ Result HttpClient::PostForm( Result HttpClient::Head( const std::string& path, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::lock_guard lock(session_mutex_); + std::scoped_lock lock(session_mutex_); PrepareSession(path, headers); cpr::Response response = session_->Head(); @@ -149,7 +149,7 @@ Result HttpClient::Head( Result HttpClient::Delete( const std::string& path, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::lock_guard lock(session_mutex_); + std::scoped_lock lock(session_mutex_); PrepareSession(path, headers); cpr::Response response = session_->Delete(); From fa9c60fa177bb1213b00c01938706d3faa328acd Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Thu, 20 Nov 2025 17:25:01 +0800 Subject: [PATCH 06/12] fix1 --- src/iceberg/catalog/rest/CMakeLists.txt | 6 +- .../rest/{config.cc => catalog_properties.cc} | 4 +- .../rest/{config.h => catalog_properties.h} | 4 +- src/iceberg/catalog/rest/error_handlers.h | 26 ++------ src/iceberg/catalog/rest/http_client.cc | 2 +- src/iceberg/catalog/rest/http_client.h | 2 +- src/iceberg/catalog/rest/meson.build | 10 +-- src/iceberg/catalog/rest/resource_paths.cc | 64 +++++++++---------- src/iceberg/catalog/rest/resource_paths.h | 40 ++++++------ .../rest/{catalog.cc => rest_catalog.cc} | 12 ++-- .../rest/{catalog.h => rest_catalog.h} | 4 +- .../rest/{endpoint_util.h => rest_util.h} | 0 src/iceberg/result.h | 8 +-- src/iceberg/test/rest_catalog_test.cc | 5 +- 14 files changed, 84 insertions(+), 103 deletions(-) rename src/iceberg/catalog/rest/{config.cc => catalog_properties.cc} (93%) rename src/iceberg/catalog/rest/{config.h => catalog_properties.h} (95%) rename src/iceberg/catalog/rest/{catalog.cc => rest_catalog.cc} (95%) rename src/iceberg/catalog/rest/{catalog.h => rest_catalog.h} (97%) rename src/iceberg/catalog/rest/{endpoint_util.h => rest_util.h} (100%) diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index e2850abb0..714a12367 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -16,10 +16,10 @@ # under the License. set(ICEBERG_REST_SOURCES - catalog.cc - json_internal.cc - config.cc + rest_catalog.cc + catalog_properties.cc http_client.cc + json_internal.cc resource_paths.cc) set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/rest/config.cc b/src/iceberg/catalog/rest/catalog_properties.cc similarity index 93% rename from src/iceberg/catalog/rest/config.cc rename to src/iceberg/catalog/rest/catalog_properties.cc index aa4b31a90..00bc72a96 100644 --- a/src/iceberg/catalog/rest/config.cc +++ b/src/iceberg/catalog/rest/catalog_properties.cc @@ -17,13 +17,13 @@ * under the License. */ -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/constant.h" namespace iceberg::rest { -std::unique_ptr RestCatalogConfig::DefaultProperties() { +std::unique_ptr RestCatalogConfig::default_properties() { return std::make_unique(); } diff --git a/src/iceberg/catalog/rest/config.h b/src/iceberg/catalog/rest/catalog_properties.h similarity index 95% rename from src/iceberg/catalog/rest/config.h rename to src/iceberg/catalog/rest/catalog_properties.h index 02fd1f72a..7b058089d 100644 --- a/src/iceberg/catalog/rest/config.h +++ b/src/iceberg/catalog/rest/catalog_properties.h @@ -29,7 +29,7 @@ #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/util/config.h" -/// \file iceberg/catalog/rest/config.h +/// \file iceberg/catalog/rest/catalog_properties.h /// \brief RestCatalogConfig implementation for Iceberg REST API. namespace iceberg::rest { @@ -50,7 +50,7 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase DefaultProperties(); + static std::unique_ptr default_properties(); /// \brief Create a RestCatalogConfig instance from a map of key-value pairs. static std::unique_ptr FromMap( diff --git a/src/iceberg/catalog/rest/error_handlers.h b/src/iceberg/catalog/rest/error_handlers.h index 000082269..ab6b2f5af 100644 --- a/src/iceberg/catalog/rest/error_handlers.h +++ b/src/iceberg/catalog/rest/error_handlers.h @@ -54,27 +54,22 @@ class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler { return InvalidArgument("{}", error.message); } return BadRequest("Malformed request: {}", error.message); - case 401: return NotAuthorized("Not authorized: {}", error.message); - case 403: return Forbidden("Forbidden: {}", error.message); - case 405: case 406: break; - case 500: - return ServiceFailure("Server error: {}: {}", error.type, error.message); - + return InternalServerError("Server error: {}: {}", error.type, error.message); case 501: return NotSupported("{}", error.message); - case 503: return ServiceUnavailable("Service unavailable: {}", error.message); } - return RESTError("Unable to process: {}", error.message); + + return RestError("Unable to process: {}", error.message); } }; @@ -88,15 +83,12 @@ class ICEBERG_REST_EXPORT NamespaceErrorHandler : public DefaultErrorHandler { return NamespaceNotEmpty("{}", error.message); } return BadRequest("Malformed request: {}", error.message); - case 404: return NoSuchNamespace("{}", error.message); - case 409: return AlreadyExists("{}", error.message); - case 422: - return RESTError("Unable to process: {}", error.message); + return RestError("Unable to process: {}", error.message); } return DefaultErrorHandler::Accept(error); @@ -111,7 +103,6 @@ class ICEBERG_REST_EXPORT DropNamespaceErrorHandler : public NamespaceErrorHandl return NamespaceNotEmpty("{}", error.message); } - // Delegate to parent handler return NamespaceErrorHandler::Accept(error); } }; @@ -126,7 +117,6 @@ class ICEBERG_REST_EXPORT TableErrorHandler : public DefaultErrorHandler { return NoSuchNamespace("{}", error.message); } return NoSuchTable("{}", error.message); - case 409: return AlreadyExists("{}", error.message); } @@ -136,9 +126,6 @@ class ICEBERG_REST_EXPORT TableErrorHandler : public DefaultErrorHandler { }; /// \brief View-level error handler. -/// -/// Handles view-specific errors including NoSuchView, NoSuchNamespace, -/// and AlreadyExists scenarios. class ICEBERG_REST_EXPORT ViewErrorHandler : public DefaultErrorHandler { public: Status Accept(const ErrorModel& error) const override { @@ -148,7 +135,6 @@ class ICEBERG_REST_EXPORT ViewErrorHandler : public DefaultErrorHandler { return NoSuchNamespace("{}", error.message); } return NoSuchView("{}", error.message); - case 409: return AlreadyExists("{}", error.message); } @@ -164,10 +150,8 @@ class ICEBERG_REST_EXPORT TableCommitErrorHandler : public DefaultErrorHandler { switch (error.code) { case 404: return NoSuchTable("{}", error.message); - case 409: return CommitFailed("Commit failed: {}", error.message); - case 500: case 502: case 503: @@ -186,10 +170,8 @@ class ICEBERG_REST_EXPORT ViewCommitErrorHandler : public DefaultErrorHandler { switch (error.code) { case 404: return NoSuchView("{}", error.message); - case 409: return CommitFailed("Commit failed: {}", error.message); - case 500: case 502: case 503: diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index c06619081..109047215 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -23,7 +23,7 @@ #include "cpr/body.h" #include "cpr/cprtypes.h" -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/json_internal.h" #include "iceberg/json_internal.h" #include "iceberg/util/macros.h" diff --git a/src/iceberg/catalog/rest/http_client.h b/src/iceberg/catalog/rest/http_client.h index 9e5528b2e..8978f0588 100644 --- a/src/iceberg/catalog/rest/http_client.h +++ b/src/iceberg/catalog/rest/http_client.h @@ -27,7 +27,7 @@ #include #include -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/error_handlers.h" #include "iceberg/catalog/rest/http_response.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index a528eee7f..936183d00 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -16,11 +16,11 @@ # under the License. iceberg_rest_sources = files( - 'catalog.cc', - 'config.cc', + 'catalog_properties.cc', 'http_client.cc', 'json_internal.cc', 'resource_paths.cc', + 'rest_catalog.cc', ) # cpr does not export symbols, so on Windows it must # be used as a static lib @@ -54,8 +54,8 @@ pkg.generate(iceberg_rest_lib) install_headers( [ - 'catalog.h', - 'config.h', + 'rest_catalog.h', + 'catalog_properties.h', 'constant.h', 'http_client.h', 'http_response.h', @@ -63,7 +63,7 @@ install_headers( 'iceberg_rest_export.h', 'types.h', 'resource_paths.h', - 'endpoint_util.h', + 'rest_util.h', 'error_handlers.h', ], subdir: 'iceberg/catalog/rest', diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc index 4ca15dd69..0fe3d76b6 100644 --- a/src/iceberg/catalog/rest/resource_paths.cc +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -21,8 +21,8 @@ #include -#include "iceberg/catalog/rest/config.h" -#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/catalog_properties.h" +#include "iceberg/catalog/rest/rest_util.h" #include "iceberg/result.h" namespace iceberg::rest { @@ -50,84 +50,82 @@ ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} std::string ResourcePaths::BuildPath(std::string_view path) const { - if (prefix_.empty()) { - return std::format("{}/v1/{}", base_uri_, path); - } - return std::format("{}/v1/{}/{}", base_uri_, prefix_, path); + return std::format("{}/v1/{}{}", base_uri_, (prefix_.empty() ? "" : (prefix_ + "/")), + path); } -std::string ResourcePaths::V1Config() const { +std::string ResourcePaths::Config() const { return std::format("{}/v1/config", base_uri_); } -std::string ResourcePaths::V1OAuth2Tokens() const { +std::string ResourcePaths::OAuth2Tokens() const { return std::format("{}/v1/oauth/tokens", base_uri_); } -std::string ResourcePaths::V1Namespaces() const { return BuildPath("namespaces"); } +std::string ResourcePaths::Namespaces() const { return BuildPath("namespaces"); } -std::string ResourcePaths::V1Namespace(const Namespace& ns) const { +std::string ResourcePaths::Namespace_(const Namespace& ns) const { return BuildPath(std::format("namespaces/{}", EncodeNamespaceForUrl(ns))); } -std::string ResourcePaths::V1NamespaceProperties(const Namespace& ns) const { +std::string ResourcePaths::NamespaceProperties(const Namespace& ns) const { return BuildPath(std::format("namespaces/{}/properties", EncodeNamespaceForUrl(ns))); } -std::string ResourcePaths::V1Tables(const Namespace& ns) const { +std::string ResourcePaths::Tables(const Namespace& ns) const { return BuildPath(std::format("namespaces/{}/tables", EncodeNamespaceForUrl(ns))); } -std::string ResourcePaths::V1Table(const TableIdentifier& table) const { - return BuildPath(std::format("namespaces/{}/tables/{}", EncodeNamespaceForUrl(table.ns), - table.name)); +std::string ResourcePaths::Table(const TableIdentifier& ident) const { + return BuildPath(std::format("namespaces/{}/tables/{}", EncodeNamespaceForUrl(ident.ns), + ident.name)); } -std::string ResourcePaths::V1RegisterTable(const Namespace& ns) const { +std::string ResourcePaths::Register(const Namespace& ns) const { return BuildPath(std::format("namespaces/{}/register", EncodeNamespaceForUrl(ns))); } -std::string ResourcePaths::V1RenameTable() const { return BuildPath("tables/rename"); } +std::string ResourcePaths::Rename() const { return BuildPath("tables/rename"); } -std::string ResourcePaths::V1TableMetrics(const TableIdentifier& table) const { +std::string ResourcePaths::Metrics(const TableIdentifier& ident) const { return BuildPath(std::format("namespaces/{}/tables/{}/metrics", - EncodeNamespaceForUrl(table.ns), table.name)); + EncodeNamespaceForUrl(ident.ns), ident.name)); } -std::string ResourcePaths::V1TableCredentials(const TableIdentifier& table) const { +std::string ResourcePaths::Credentials(const TableIdentifier& ident) const { return BuildPath(std::format("namespaces/{}/tables/{}/credentials", - EncodeNamespaceForUrl(table.ns), table.name)); + EncodeNamespaceForUrl(ident.ns), ident.name)); } -std::string ResourcePaths::V1TableScanPlan(const TableIdentifier& table) const { +std::string ResourcePaths::ScanPlan(const TableIdentifier& ident) const { return BuildPath(std::format("namespaces/{}/tables/{}/plan", - EncodeNamespaceForUrl(table.ns), table.name)); + EncodeNamespaceForUrl(ident.ns), ident.name)); } -std::string ResourcePaths::V1TableScanPlanResult(const TableIdentifier& table, - const std::string& plan_id) const { +std::string ResourcePaths::ScanPlanResult(const TableIdentifier& ident, + const std::string& plan_id) const { return BuildPath(std::format("namespaces/{}/tables/{}/plan/{}", - EncodeNamespaceForUrl(table.ns), table.name, plan_id)); + EncodeNamespaceForUrl(ident.ns), ident.name, plan_id)); } -std::string ResourcePaths::V1TableTasks(const TableIdentifier& table) const { +std::string ResourcePaths::Tasks(const TableIdentifier& ident) const { return BuildPath(std::format("namespaces/{}/tables/{}/tasks", - EncodeNamespaceForUrl(table.ns), table.name)); + EncodeNamespaceForUrl(ident.ns), ident.name)); } -std::string ResourcePaths::V1TransactionCommit() const { +std::string ResourcePaths::CommitTransaction() const { return BuildPath("transactions/commit"); } -std::string ResourcePaths::V1Views(const Namespace& ns) const { +std::string ResourcePaths::Views(const Namespace& ns) const { return BuildPath(std::format("namespaces/{}/views", EncodeNamespaceForUrl(ns))); } -std::string ResourcePaths::V1View(const TableIdentifier& view) const { +std::string ResourcePaths::View(const TableIdentifier& ident) const { return BuildPath( - std::format("namespaces/{}/views/{}", EncodeNamespaceForUrl(view.ns), view.name)); + std::format("namespaces/{}/views/{}", EncodeNamespaceForUrl(ident.ns), ident.name)); } -std::string ResourcePaths::V1RenameView() const { return BuildPath("views/rename"); } +std::string ResourcePaths::RenameView() const { return BuildPath("views/rename"); } } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/resource_paths.h b/src/iceberg/catalog/rest/resource_paths.h index e5b4095d8..9b87346df 100644 --- a/src/iceberg/catalog/rest/resource_paths.h +++ b/src/iceberg/catalog/rest/resource_paths.h @@ -22,7 +22,7 @@ #include #include -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/result.h" #include "iceberg/table_identifier.h" @@ -42,64 +42,64 @@ class ICEBERG_REST_EXPORT ResourcePaths { static Result> Make(const RestCatalogConfig& config); /// \brief Get the /v1/{prefix}/config endpoint path. - std::string V1Config() const; + std::string Config() const; /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. - std::string V1OAuth2Tokens() const; + std::string OAuth2Tokens() const; /// \brief Get the /v1/{prefix}/namespaces endpoint path. - std::string V1Namespaces() const; + std::string Namespaces() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. - std::string V1Namespace(const Namespace& ns) const; + std::string Namespace_(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. - std::string V1NamespaceProperties(const Namespace& ns) const; + std::string NamespaceProperties(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. - std::string V1Tables(const Namespace& ns) const; + std::string Tables(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. - std::string V1Table(const TableIdentifier& table) const; + std::string Table(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. - std::string V1RegisterTable(const Namespace& ns) const; + std::string Register(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/tables/rename endpoint path. - std::string V1RenameTable() const; + std::string Rename() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint /// path. - std::string V1TableMetrics(const TableIdentifier& table) const; + std::string Metrics(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials /// endpoint path. - std::string V1TableCredentials(const TableIdentifier& table) const; + std::string Credentials(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint /// path. - std::string V1TableScanPlan(const TableIdentifier& table) const; + std::string ScanPlan(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{planId} /// endpoint path. - std::string V1TableScanPlanResult(const TableIdentifier& table, - const std::string& plan_id) const; + std::string ScanPlanResult(const TableIdentifier& ident, + const std::string& plan_id) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint /// path. - std::string V1TableTasks(const TableIdentifier& table) const; + std::string Tasks(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/transactions/commit endpoint path. - std::string V1TransactionCommit() const; + std::string CommitTransaction() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views endpoint path. - std::string V1Views(const Namespace& ns) const; + std::string Views(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views/{view} endpoint path. - std::string V1View(const TableIdentifier& view) const; + std::string View(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/views/rename endpoint path. - std::string V1RenameView() const; + std::string RenameView() const; private: explicit ResourcePaths(std::string base_uri, std::string prefix); diff --git a/src/iceberg/catalog/rest/catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc similarity index 95% rename from src/iceberg/catalog/rest/catalog.cc rename to src/iceberg/catalog/rest/rest_catalog.cc index bf1ea0a58..0653ef746 100644 --- a/src/iceberg/catalog/rest/catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -17,7 +17,7 @@ * under the License. */ -#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/rest_catalog.h" #include #include @@ -25,14 +25,14 @@ #include -#include "iceberg/catalog/rest/catalog.h" -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/constant.h" -#include "iceberg/catalog/rest/endpoint_util.h" #include "iceberg/catalog/rest/error_handlers.h" #include "iceberg/catalog/rest/http_client.h" #include "iceberg/catalog/rest/http_response.h" #include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/rest_catalog.h" +#include "iceberg/catalog/rest/rest_util.h" #include "iceberg/catalog/rest/types.h" #include "iceberg/json_internal.h" #include "iceberg/result.h" @@ -46,7 +46,7 @@ Result> RestCatalog::Make(const RestCatalogConfig& ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); auto tmp_client = std::make_unique(config); - const std::string endpoint = paths->V1Config(); + const std::string endpoint = paths->Config(); ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); @@ -81,7 +81,7 @@ std::string_view RestCatalog::name() const { } Result> RestCatalog::ListNamespaces(const Namespace& ns) const { - const std::string endpoint = paths_.V1Namespaces(); + const std::string endpoint = paths_.Namespaces(); std::vector result; std::string next_token; while (true) { diff --git a/src/iceberg/catalog/rest/catalog.h b/src/iceberg/catalog/rest/rest_catalog.h similarity index 97% rename from src/iceberg/catalog/rest/catalog.h rename to src/iceberg/catalog/rest/rest_catalog.h index 78581de6f..021b907be 100644 --- a/src/iceberg/catalog/rest/catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -23,13 +23,13 @@ #include #include "iceberg/catalog.h" -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/http_client.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/catalog/rest/resource_paths.h" #include "iceberg/result.h" -/// \file iceberg/catalog/rest/catalog.h +/// \file iceberg/catalog/rest/rest_catalog.h /// RestCatalog implementation for Iceberg REST API. namespace iceberg::rest { diff --git a/src/iceberg/catalog/rest/endpoint_util.h b/src/iceberg/catalog/rest/rest_util.h similarity index 100% rename from src/iceberg/catalog/rest/endpoint_util.h rename to src/iceberg/catalog/rest/rest_util.h diff --git a/src/iceberg/result.h b/src/iceberg/result.h index 7861d44c7..f77327365 100644 --- a/src/iceberg/result.h +++ b/src/iceberg/result.h @@ -53,10 +53,10 @@ enum class ErrorKind { kNotFound, kNotImplemented, kNotSupported, - kServiceFailure, + kInternalServerError, kServiceUnavailable, kUnknownError, - kRESTError, + kRestError, }; /// \brief Error with a kind and a message. @@ -110,10 +110,10 @@ DEFINE_ERROR_FUNCTION(NotAuthorized) DEFINE_ERROR_FUNCTION(NotFound) DEFINE_ERROR_FUNCTION(NotImplemented) DEFINE_ERROR_FUNCTION(NotSupported) -DEFINE_ERROR_FUNCTION(ServiceFailure) +DEFINE_ERROR_FUNCTION(InternalServerError) DEFINE_ERROR_FUNCTION(ServiceUnavailable) DEFINE_ERROR_FUNCTION(UnknownError) -DEFINE_ERROR_FUNCTION(RESTError) +DEFINE_ERROR_FUNCTION(RestError) #undef DEFINE_ERROR_FUNCTION diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index 7b30a36f7..f66cd4c8d 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -17,14 +17,15 @@ * under the License. */ +#include "iceberg/catalog/rest/rest_catalog.h" + #include #include #include #include -#include "iceberg/catalog/rest/catalog.h" -#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/table_identifier.h" #include "iceberg/test/matchers.h" From c3a992cc5b1b00c842fb9ee912905f445da7034f Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Fri, 21 Nov 2025 12:03:00 +0800 Subject: [PATCH 07/12] fix2 --- .../catalog/rest/catalog_properties.cc | 7 +-- src/iceberg/catalog/rest/catalog_properties.h | 16 +++--- src/iceberg/catalog/rest/http_client.cc | 8 ++- src/iceberg/catalog/rest/resource_paths.cc | 13 +---- src/iceberg/catalog/rest/resource_paths.h | 9 ---- src/iceberg/catalog/rest/rest_catalog.cc | 54 ++++++++++--------- src/iceberg/catalog/rest/rest_catalog.h | 11 +++- src/iceberg/catalog/rest/rest_util.h | 22 ++++++++ src/iceberg/test/rest_catalog_test.cc | 24 +++------ 9 files changed, 82 insertions(+), 82 deletions(-) diff --git a/src/iceberg/catalog/rest/catalog_properties.cc b/src/iceberg/catalog/rest/catalog_properties.cc index 00bc72a96..c20fd0acc 100644 --- a/src/iceberg/catalog/rest/catalog_properties.cc +++ b/src/iceberg/catalog/rest/catalog_properties.cc @@ -19,8 +19,6 @@ #include "iceberg/catalog/rest/catalog_properties.h" -#include "iceberg/catalog/rest/constant.h" - namespace iceberg::rest { std::unique_ptr RestCatalogConfig::default_properties() { @@ -34,11 +32,8 @@ std::unique_ptr RestCatalogConfig::FromMap( return rest_catalog_config; } -std::unordered_map RestCatalogConfig::GetExtraHeaders() const { +std::unordered_map RestCatalogConfig::ExtractHeaders() const { std::unordered_map headers; - headers[kHeaderContentType] = kMimeTypeApplicationJson; - headers[kHeaderUserAgent] = kUserAgent; - constexpr std::string_view prefix = "header."; for (const auto& [key, value] : configs_) { if (key.starts_with(prefix)) { diff --git a/src/iceberg/catalog/rest/catalog_properties.h b/src/iceberg/catalog/rest/catalog_properties.h index 7b058089d..859add069 100644 --- a/src/iceberg/catalog/rest/catalog_properties.h +++ b/src/iceberg/catalog/rest/catalog_properties.h @@ -41,13 +41,13 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase::Entry; /// \brief The URI of the REST catalog server. - inline static std::string_view kUri{"uri"}; + inline static Entry kUri{"uri", ""}; /// \brief The name of the catalog. - inline static std::string_view kName{"name"}; + inline static Entry kName{"name", ""}; /// \brief The warehouse path. - inline static std::string_view kWarehouse{"warehouse"}; + inline static Entry kWarehouse{"warehouse", ""}; /// \brief Create a default RestCatalogConfig instance. static std::unique_ptr default_properties(); @@ -56,13 +56,11 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase FromMap( const std::unordered_map& properties); - /// \brief Generates extra HTTP headers to be added to every request from the - /// configuration. + /// \brief Returns HTTP headers to be added to every request. /// - /// This includes default headers like Content-Type, User-Agent, X-Client-Version and - /// any custom headers prefixed with "header." in the properties. - /// \return A map of header names to values. - std::unordered_map GetExtraHeaders() const; + /// This includes any key prefixed with "header." in the properties. + /// \return A map of headers with the prefix removed from the keys. + std::unordered_map ExtractHeaders() const; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index 109047215..efc97d96a 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -24,6 +24,7 @@ #include "cpr/body.h" #include "cpr/cprtypes.h" #include "iceberg/catalog/rest/catalog_properties.h" +#include "iceberg/catalog/rest/constant.h" #include "iceberg/catalog/rest/json_internal.h" #include "iceberg/json_internal.h" #include "iceberg/util/macros.h" @@ -88,8 +89,11 @@ void HttpClient::PrepareSession( } HttpClient::HttpClient(const RestCatalogConfig& config) - : default_headers_{config.GetExtraHeaders()}, - session_{std::make_unique()} {} + : default_headers_{config.ExtractHeaders()}, + session_{std::make_unique()} { + default_headers_[kHeaderContentType] = kMimeTypeApplicationJson; + default_headers_[kHeaderUserAgent] = kUserAgent; +} Result HttpClient::Get( const std::string& path, const std::unordered_map& params, diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc index 0fe3d76b6..221e72938 100644 --- a/src/iceberg/catalog/rest/resource_paths.cc +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -30,7 +30,7 @@ namespace iceberg::rest { Result> ResourcePaths::Make( const RestCatalogConfig& config) { // Validate and extract URI - auto it = config.configs().find(std::string(RestCatalogConfig::kUri)); + auto it = config.configs().find(RestCatalogConfig::kUri.key()); if (it == config.configs().end() || it->second.empty()) { return InvalidArgument("Rest catalog configuration property 'uri' is required."); } @@ -117,15 +117,4 @@ std::string ResourcePaths::CommitTransaction() const { return BuildPath("transactions/commit"); } -std::string ResourcePaths::Views(const Namespace& ns) const { - return BuildPath(std::format("namespaces/{}/views", EncodeNamespaceForUrl(ns))); -} - -std::string ResourcePaths::View(const TableIdentifier& ident) const { - return BuildPath( - std::format("namespaces/{}/views/{}", EncodeNamespaceForUrl(ident.ns), ident.name)); -} - -std::string ResourcePaths::RenameView() const { return BuildPath("views/rename"); } - } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/resource_paths.h b/src/iceberg/catalog/rest/resource_paths.h index 9b87346df..e5cc85525 100644 --- a/src/iceberg/catalog/rest/resource_paths.h +++ b/src/iceberg/catalog/rest/resource_paths.h @@ -92,15 +92,6 @@ class ICEBERG_REST_EXPORT ResourcePaths { /// \brief Get the /v1/{prefix}/transactions/commit endpoint path. std::string CommitTransaction() const; - /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views endpoint path. - std::string Views(const Namespace& ns) const; - - /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views/{view} endpoint path. - std::string View(const TableIdentifier& ident) const; - - /// \brief Get the /v1/{prefix}/views/rename endpoint path. - std::string RenameView() const; - private: explicit ResourcePaths(std::string base_uri, std::string prefix); diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 0653ef746..7a2d07de7 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -41,44 +41,46 @@ namespace iceberg::rest { -Result> RestCatalog::Make(const RestCatalogConfig& config) { - // Create ResourcePaths and validate URI - ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); - +Result> RestCatalog::FetchAndMergeConfig( + const RestCatalogConfig& config, const ResourcePaths& paths) { + // Fetch server configuration auto tmp_client = std::make_unique(config); - const std::string endpoint = paths->Config(); - ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, + const std::string endpoint = paths.Config(); + ICEBERG_ASSIGN_OR_RAISE(const auto response, tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); + // Merge server config into client config, server config overrides > client config // properties > server config defaults - auto final_props = std::move(server_config.defaults); - for (const auto& kv : config.configs()) { - final_props.insert_or_assign(kv.first, kv.second); - } + auto final_props = + MergeConfigs(server_config.defaults, config.configs(), server_config.overrides); + return RestCatalogConfig::FromMap(final_props); +} + +Result> RestCatalog::Make(const RestCatalogConfig& config) { + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + // Fetch and merge server configuration + ICEBERG_ASSIGN_OR_RAISE(auto final_config, FetchAndMergeConfig(config, *paths)); - for (const auto& kv : server_config.overrides) { - final_props.insert_or_assign(kv.first, kv.second); - } - auto final_config = RestCatalogConfig::FromMap(final_props); auto client = std::make_unique(*final_config); ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config)); - return std::unique_ptr(new RestCatalog( - std::move(final_config), std::move(client), std::move(*final_paths))); + + std::string catalog_name = final_config->Get(RestCatalogConfig::kName); + return std::unique_ptr( + new RestCatalog(std::move(final_config), std::move(client), std::move(*final_paths), + std::move(catalog_name))); } RestCatalog::RestCatalog(std::unique_ptr config, - std::unique_ptr client, ResourcePaths paths) - : config_(std::move(config)), client_(std::move(client)), paths_(std::move(paths)) {} - -std::string_view RestCatalog::name() const { - auto it = config_->configs().find(std::string(RestCatalogConfig::kName)); - if (it == config_->configs().end() || it->second.empty()) { - return {""}; - } - return std::string_view(it->second); -} + std::unique_ptr client, ResourcePaths paths, + std::string name) + : config_(std::move(config)), + client_(std::move(client)), + paths_(std::move(paths)), + name_(std::move(name)) {} + +std::string_view RestCatalog::name() const { return name_; } Result> RestCatalog::ListNamespaces(const Namespace& ns) const { const std::string endpoint = paths_.Namespaces(); diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h index 021b907be..042428c00 100644 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -100,11 +100,20 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog { private: RestCatalog(std::unique_ptr config, - std::unique_ptr client, ResourcePaths paths); + std::unique_ptr client, ResourcePaths paths, std::string name); + + /// \brief Fetch server configuration and merge with client config + /// + /// \param config the initial client configuration + /// \param paths the resource paths for REST endpoints + /// \return the final merged configuration + static Result> FetchAndMergeConfig( + const RestCatalogConfig& config, const ResourcePaths& paths); std::unique_ptr config_; std::unique_ptr client_; ResourcePaths paths_; + std::string name_; // Cached catalog name }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_util.h b/src/iceberg/catalog/rest/rest_util.h index 0fa0124f0..65d0265fb 100644 --- a/src/iceberg/catalog/rest/rest_util.h +++ b/src/iceberg/catalog/rest/rest_util.h @@ -23,6 +23,7 @@ #include #include #include +#include #include "iceberg/table_identifier.h" @@ -78,4 +79,25 @@ inline std::string EncodeNamespaceForUrl(const Namespace& ns) { return EncodeUriComponent(joined_string); } +/// \brief Merge catalog configuration properties. +/// \details Merges three sets of configuration properties following the precedence order: +/// server overrides > client configs > server defaults. +/// \param server_defaults Default properties provided by the server. +/// \param client_configs Configuration properties from the client. +/// \param server_overrides Override properties enforced by the server. +/// \return A merged map containing all properties with correct precedence. +inline std::unordered_map MergeConfigs( + const std::unordered_map& server_defaults, + const std::unordered_map& client_configs, + const std::unordered_map& server_overrides) { + auto merged = server_defaults; + for (const auto& kv : client_configs) { + merged.insert_or_assign(kv.first, kv.second); + } + for (const auto& kv : server_overrides) { + merged.insert_or_assign(kv.first, kv.second); + } + return merged; +} + } // namespace iceberg::rest diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index f66cd4c8d..c17f350a1 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -44,17 +44,9 @@ class RestCatalogTest : public ::testing::Test { std::string uri = uri_env ? uri_env : "http://localhost:8181"; std::string warehouse = warehouse_env ? warehouse_env : "default"; - config_ - .Set(RestCatalogConfig::Entry{std::string(RestCatalogConfig::kUri), - ""}, - uri) - .Set(RestCatalogConfig::Entry{std::string(RestCatalogConfig::kName), - ""}, - std::string("test_catalog")) - .Set( - RestCatalogConfig::Entry{ - std::string(RestCatalogConfig::kWarehouse), ""}, - warehouse); + config_.Set(RestCatalogConfig::kUri, uri) + .Set(RestCatalogConfig::kName, std::string("test_catalog")) + .Set(RestCatalogConfig::kWarehouse, warehouse); } void TearDown() override {} @@ -62,7 +54,7 @@ class RestCatalogTest : public ::testing::Test { RestCatalogConfig config_; }; -TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) { +TEST_F(RestCatalogTest, MakeCatalogSuccess) { auto catalog_result = RestCatalog::Make(config_); EXPECT_THAT(catalog_result, IsOk()); @@ -74,16 +66,14 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) { TEST_F(RestCatalogTest, DISABLED_MakeCatalogEmptyUri) { RestCatalogConfig invalid_config = config_; - invalid_config.Set( - RestCatalogConfig::Entry{std::string(RestCatalogConfig::kUri), ""}, - std::string("")); + invalid_config.Set(RestCatalogConfig::kUri, std::string("")); auto catalog_result = RestCatalog::Make(invalid_config); EXPECT_THAT(catalog_result, IsError(ErrorKind::kInvalidArgument)); EXPECT_THAT(catalog_result, HasErrorMessage("uri")); } -TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { +TEST_F(RestCatalogTest, MakeCatalogWithCustomProperties) { RestCatalogConfig custom_config = config_; custom_config .Set(RestCatalogConfig::Entry{"custom_prop", ""}, @@ -94,7 +84,7 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { EXPECT_THAT(catalog_result, IsOk()); } -TEST_F(RestCatalogTest, DISABLED_ListNamespaces) { +TEST_F(RestCatalogTest, ListNamespaces) { auto catalog_result = RestCatalog::Make(config_); ASSERT_THAT(catalog_result, IsOk()); auto& catalog = catalog_result.value(); From 1b17be05d3a0a79717272b2ae99b219caee5e05c Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Tue, 25 Nov 2025 11:44:21 +0800 Subject: [PATCH 08/12] fix ci --- src/iceberg/catalog/rest/CMakeLists.txt | 4 +- .../catalog/rest/catalog_properties.cc | 22 ++- src/iceberg/catalog/rest/catalog_properties.h | 29 ++- src/iceberg/catalog/rest/constant.h | 1 + src/iceberg/catalog/rest/error_handlers.cc | 184 ++++++++++++++++++ src/iceberg/catalog/rest/error_handlers.h | 157 +++++---------- src/iceberg/catalog/rest/http_client.cc | 81 ++++++-- src/iceberg/catalog/rest/http_client.h | 43 +++- src/iceberg/catalog/rest/http_response.h | 50 ----- src/iceberg/catalog/rest/meson.build | 11 +- src/iceberg/catalog/rest/resource_paths.cc | 81 ++++---- src/iceberg/catalog/rest/resource_paths.h | 20 +- src/iceberg/catalog/rest/rest_catalog.cc | 109 ++++++----- src/iceberg/catalog/rest/rest_catalog.h | 16 +- src/iceberg/catalog/rest/rest_util.cc | 99 ++++++++++ src/iceberg/catalog/rest/rest_util.h | 91 ++++----- src/iceberg/result.h | 11 +- src/iceberg/test/CMakeLists.txt | 7 +- src/iceberg/test/meson.build | 1 + src/iceberg/test/rest_catalog_test.cc | 44 +++-- src/iceberg/test/rest_util_test.cc | 149 ++++++++++++++ 21 files changed, 822 insertions(+), 388 deletions(-) create mode 100644 src/iceberg/catalog/rest/error_handlers.cc delete mode 100644 src/iceberg/catalog/rest/http_response.h create mode 100644 src/iceberg/catalog/rest/rest_util.cc create mode 100644 src/iceberg/test/rest_util_test.cc diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index 714a12367..881b3d39a 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -18,9 +18,11 @@ set(ICEBERG_REST_SOURCES rest_catalog.cc catalog_properties.cc + error_handlers.cc http_client.cc json_internal.cc - resource_paths.cc) + resource_paths.cc + rest_util.cc) set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/rest/catalog_properties.cc b/src/iceberg/catalog/rest/catalog_properties.cc index c20fd0acc..c61cf35cf 100644 --- a/src/iceberg/catalog/rest/catalog_properties.cc +++ b/src/iceberg/catalog/rest/catalog_properties.cc @@ -19,20 +19,24 @@ #include "iceberg/catalog/rest/catalog_properties.h" +#include + namespace iceberg::rest { -std::unique_ptr RestCatalogConfig::default_properties() { - return std::make_unique(); +std::unique_ptr RestCatalogProperties::default_properties() { + return std::unique_ptr(new RestCatalogProperties()); } -std::unique_ptr RestCatalogConfig::FromMap( +std::unique_ptr RestCatalogProperties::FromMap( const std::unordered_map& properties) { - auto rest_catalog_config = std::make_unique(); + auto rest_catalog_config = + std::unique_ptr(new RestCatalogProperties()); rest_catalog_config->configs_ = properties; return rest_catalog_config; } -std::unordered_map RestCatalogConfig::ExtractHeaders() const { +std::unordered_map RestCatalogProperties::ExtractHeaders() + const { std::unordered_map headers; constexpr std::string_view prefix = "header."; for (const auto& [key, value] : configs_) { @@ -47,4 +51,12 @@ std::unordered_map RestCatalogConfig::ExtractHeaders() return headers; } +Result RestCatalogProperties::Uri() const { + auto it = configs_.find(kUri.key()); + if (it == configs_.end() || it->second.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + return it->second; +} + } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/catalog_properties.h b/src/iceberg/catalog/rest/catalog_properties.h index 859add069..39e6a7a9d 100644 --- a/src/iceberg/catalog/rest/catalog_properties.h +++ b/src/iceberg/catalog/rest/catalog_properties.h @@ -21,24 +21,23 @@ #include #include -#include #include -#include - #include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" #include "iceberg/util/config.h" /// \file iceberg/catalog/rest/catalog_properties.h -/// \brief RestCatalogConfig implementation for Iceberg REST API. +/// \brief RestCatalogProperties implementation for Iceberg REST API. namespace iceberg::rest { /// \brief Configuration class for a REST Catalog. -class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase { +class ICEBERG_REST_EXPORT RestCatalogProperties + : public ConfigBase { public: template - using Entry = const ConfigBase::Entry; + using Entry = const ConfigBase::Entry; /// \brief The URI of the REST catalog server. inline static Entry kUri{"uri", ""}; @@ -49,11 +48,14 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase kWarehouse{"warehouse", ""}; - /// \brief Create a default RestCatalogConfig instance. - static std::unique_ptr default_properties(); + /// \brief The optional prefix for REST API paths. + inline static Entry kPrefix{"prefix", ""}; + + /// \brief Create a default RestCatalogProperties instance. + static std::unique_ptr default_properties(); - /// \brief Create a RestCatalogConfig instance from a map of key-value pairs. - static std::unique_ptr FromMap( + /// \brief Create a RestCatalogProperties instance from a map of key-value pairs. + static std::unique_ptr FromMap( const std::unordered_map& properties); /// \brief Returns HTTP headers to be added to every request. @@ -61,6 +63,13 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase ExtractHeaders() const; + + /// \brief Get the URI of the REST catalog server. + /// \return The URI if configured, or an error if not set or empty. + Result Uri() const; + + private: + RestCatalogProperties() = default; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/constant.h b/src/iceberg/catalog/rest/constant.h index 09ef33f37..0a6e8d0c0 100644 --- a/src/iceberg/catalog/rest/constant.h +++ b/src/iceberg/catalog/rest/constant.h @@ -34,6 +34,7 @@ inline const std::string kHeaderXClientVersion = "X-Client-Version"; inline const std::string kHeaderUserAgent = "User-Agent"; inline const std::string kMimeTypeApplicationJson = "application/json"; +inline const std::string kMimeTypeFormUrlEncoded = "application/x-www-form-urlencoded"; inline const std::string kUserAgentPrefix = "iceberg-cpp/"; inline const std::string kUserAgent = "iceberg-cpp/" ICEBERG_VERSION_STRING; diff --git a/src/iceberg/catalog/rest/error_handlers.cc b/src/iceberg/catalog/rest/error_handlers.cc new file mode 100644 index 000000000..243e9e8a3 --- /dev/null +++ b/src/iceberg/catalog/rest/error_handlers.cc @@ -0,0 +1,184 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/error_handlers.h" + +#include + +namespace iceberg::rest { + +namespace { + +constexpr std::string_view kIllegalArgumentException = "IllegalArgumentException"; +constexpr std::string_view kNoSuchNamespaceException = "NoSuchNamespaceException"; +constexpr std::string_view kNamespaceNotEmptyException = "NamespaceNotEmptyException"; + +} // namespace + +const std::shared_ptr& DefaultErrorHandler::Instance() { + static const std::shared_ptr instance{new DefaultErrorHandler()}; + return instance; +} + +Status DefaultErrorHandler::Accept(const ErrorModel& error) const { + switch (error.code) { + case 400: + if (error.type == kIllegalArgumentException) { + return InvalidArgument(error.message); + } + return BadRequest("Malformed request: {}", error.message); + case 401: + return NotAuthorized("Not authorized: {}", error.message); + case 403: + return Forbidden("Forbidden: {}", error.message); + case 405: + case 406: + break; + case 500: + return InternalServerError("Server error: {}: {}", error.type, error.message); + case 501: + return NotSupported(error.message); + case 503: + return ServiceUnavailable("Service unavailable: {}", error.message); + } + + return RestError("Unable to process: {}", error.message); +} + +const std::shared_ptr& NamespaceErrorHandler::Instance() { + static const std::shared_ptr instance{ + new NamespaceErrorHandler()}; + return instance; +} + +Status NamespaceErrorHandler::Accept(const ErrorModel& error) const { + switch (error.code) { + case 400: + if (error.type == kNamespaceNotEmptyException) { + return NamespaceNotEmpty(error.message); + } + return BadRequest("Malformed request: {}", error.message); + case 404: + return NoSuchNamespace(error.message); + case 409: + return AlreadyExists(error.message); + case 422: + return RestError("Unable to process: {}", error.message); + } + + return DefaultErrorHandler::Accept(error); +} + +const std::shared_ptr& DropNamespaceErrorHandler::Instance() { + static const std::shared_ptr instance{ + new DropNamespaceErrorHandler()}; + return instance; +} + +Status DropNamespaceErrorHandler::Accept(const ErrorModel& error) const { + if (error.code == 409) { + return NamespaceNotEmpty(error.message); + } + + return NamespaceErrorHandler::Accept(error); +} + +const std::shared_ptr& TableErrorHandler::Instance() { + static const std::shared_ptr instance{new TableErrorHandler()}; + return instance; +} + +Status TableErrorHandler::Accept(const ErrorModel& error) const { + switch (error.code) { + case 404: + if (error.type == kNoSuchNamespaceException) { + return NoSuchNamespace(error.message); + } + return NoSuchTable(error.message); + case 409: + return AlreadyExists(error.message); + } + + return DefaultErrorHandler::Accept(error); +} + +const std::shared_ptr& ViewErrorHandler::Instance() { + static const std::shared_ptr instance{new ViewErrorHandler()}; + return instance; +} + +Status ViewErrorHandler::Accept(const ErrorModel& error) const { + switch (error.code) { + case 404: + if (error.type == kNoSuchNamespaceException) { + return NoSuchNamespace(error.message); + } + return NoSuchView(error.message); + case 409: + return AlreadyExists(error.message); + } + + return DefaultErrorHandler::Accept(error); +} + +const std::shared_ptr& TableCommitErrorHandler::Instance() { + static const std::shared_ptr instance{ + new TableCommitErrorHandler()}; + return instance; +} + +Status TableCommitErrorHandler::Accept(const ErrorModel& error) const { + switch (error.code) { + case 404: + return NoSuchTable(error.message); + case 409: + return CommitFailed("Commit failed: {}", error.message); + case 500: + case 502: + case 503: + case 504: + return CommitStateUnknown("Service failed: {}: {}", error.code, error.message); + } + + return DefaultErrorHandler::Accept(error); +} + +const std::shared_ptr& ViewCommitErrorHandler::Instance() { + static const std::shared_ptr instance{ + new ViewCommitErrorHandler()}; + return instance; +} + +Status ViewCommitErrorHandler::Accept(const ErrorModel& error) const { + switch (error.code) { + case 404: + return NoSuchView(error.message); + case 409: + return CommitFailed("Commit failed: {}", error.message); + case 500: + case 502: + case 503: + case 504: + return CommitStateUnknown("Service failed: {}: {}", error.code, error.message); + } + + return DefaultErrorHandler::Accept(error); +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/error_handlers.h b/src/iceberg/catalog/rest/error_handlers.h index ab6b2f5af..5dd5abb8e 100644 --- a/src/iceberg/catalog/rest/error_handlers.h +++ b/src/iceberg/catalog/rest/error_handlers.h @@ -19,8 +19,7 @@ #pragma once -#include -#include +#include #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/catalog/rest/types.h" @@ -37,6 +36,9 @@ class ICEBERG_REST_EXPORT ErrorHandler { public: virtual ~ErrorHandler() = default; + // TODO(Li Feiyang):removing ErrorModel as the inner layer and directly using + // ErrorResponse + /// \brief Process an error response and return an appropriate Error. /// /// \param error The error model parsed from the HTTP response body @@ -47,140 +49,85 @@ class ICEBERG_REST_EXPORT ErrorHandler { /// \brief Default error handler for REST API responses. class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - switch (error.code) { - case 400: - if (error.type == "IllegalArgumentException") { - return InvalidArgument("{}", error.message); - } - return BadRequest("Malformed request: {}", error.message); - case 401: - return NotAuthorized("Not authorized: {}", error.message); - case 403: - return Forbidden("Forbidden: {}", error.message); - case 405: - case 406: - break; - case 500: - return InternalServerError("Server error: {}: {}", error.type, error.message); - case 501: - return NotSupported("{}", error.message); - case 503: - return ServiceUnavailable("Service unavailable: {}", error.message); - } - - return RestError("Unable to process: {}", error.message); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; + + protected: + constexpr DefaultErrorHandler() = default; }; /// \brief Namespace-specific error handler for create/read/update operations. class ICEBERG_REST_EXPORT NamespaceErrorHandler : public DefaultErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - switch (error.code) { - case 400: - if (error.type == "NamespaceNotEmptyException") { - return NamespaceNotEmpty("{}", error.message); - } - return BadRequest("Malformed request: {}", error.message); - case 404: - return NoSuchNamespace("{}", error.message); - case 409: - return AlreadyExists("{}", error.message); - case 422: - return RestError("Unable to process: {}", error.message); - } - - return DefaultErrorHandler::Accept(error); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; + + protected: + constexpr NamespaceErrorHandler() = default; }; /// \brief Error handler for drop namespace operations. class ICEBERG_REST_EXPORT DropNamespaceErrorHandler : public NamespaceErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - if (error.code == 409) { - return NamespaceNotEmpty("{}", error.message); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; - return NamespaceErrorHandler::Accept(error); - } + private: + constexpr DropNamespaceErrorHandler() = default; }; /// \brief Table-level error handler. class ICEBERG_REST_EXPORT TableErrorHandler : public DefaultErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - switch (error.code) { - case 404: - if (error.type == "NoSuchNamespaceException") { - return NoSuchNamespace("{}", error.message); - } - return NoSuchTable("{}", error.message); - case 409: - return AlreadyExists("{}", error.message); - } - - return DefaultErrorHandler::Accept(error); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; + + private: + constexpr TableErrorHandler() = default; }; /// \brief View-level error handler. class ICEBERG_REST_EXPORT ViewErrorHandler : public DefaultErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - switch (error.code) { - case 404: - if (error.type == "NoSuchNamespaceException") { - return NoSuchNamespace("{}", error.message); - } - return NoSuchView("{}", error.message); - case 409: - return AlreadyExists("{}", error.message); - } - - return DefaultErrorHandler::Accept(error); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; + + private: + constexpr ViewErrorHandler() = default; }; /// \brief Table commit operation error handler. class ICEBERG_REST_EXPORT TableCommitErrorHandler : public DefaultErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - switch (error.code) { - case 404: - return NoSuchTable("{}", error.message); - case 409: - return CommitFailed("Commit failed: {}", error.message); - case 500: - case 502: - case 503: - case 504: - return CommitStateUnknown("Service failed: {}: {}", error.code, error.message); - } - - return DefaultErrorHandler::Accept(error); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; + + private: + constexpr TableCommitErrorHandler() = default; }; /// \brief View commit operation error handler. class ICEBERG_REST_EXPORT ViewCommitErrorHandler : public DefaultErrorHandler { public: - Status Accept(const ErrorModel& error) const override { - switch (error.code) { - case 404: - return NoSuchView("{}", error.message); - case 409: - return CommitFailed("Commit failed: {}", error.message); - case 500: - case 502: - case 503: - case 504: - return CommitStateUnknown("Service failed: {}: {}", error.code, error.message); - } - - return DefaultErrorHandler::Accept(error); - } + /// \brief Returns the singleton instance + static const std::shared_ptr& Instance(); + + Status Accept(const ErrorModel& error) const override; + + private: + constexpr ViewCommitErrorHandler() = default; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index efc97d96a..40a1266fd 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -19,11 +19,9 @@ #include "iceberg/catalog/rest/http_client.h" +#include #include -#include "cpr/body.h" -#include "cpr/cprtypes.h" -#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/constant.h" #include "iceberg/catalog/rest/json_internal.h" #include "iceberg/json_internal.h" @@ -31,11 +29,44 @@ namespace iceberg::rest { +class HttpResponse::Impl { + public: + explicit Impl(cpr::Response&& response) : response_(std::move(response)) {} + ~Impl() = default; + + int32_t status_code() const { return static_cast(response_.status_code); } + + std::string body() const { return response_.text; } + + std::unordered_map headers() const { + return {response_.header.begin(), response_.header.end()}; + } + + private: + cpr::Response response_; +}; + +HttpResponse::HttpResponse(HttpResponse&&) noexcept = default; + +HttpResponse& HttpResponse::operator=(HttpResponse&&) noexcept = default; + +HttpResponse::HttpResponse() = default; + +HttpResponse::~HttpResponse() = default; + +int32_t HttpResponse::status_code() const { return impl_->status_code(); } + +std::string HttpResponse::body() const { return impl_->body(); } + +std::unordered_map HttpResponse::headers() const { + return impl_->headers(); +} + namespace { /// \brief Merges global default headers with request-specific headers. /// -/// Combines the global headers derived from RestCatalogConfig with the headers +/// Combines the global headers derived from RestCatalogProperties with the headers /// passed in the specific request. Request-specific headers have higher priority /// and will override global defaults if the keys conflict (e.g., overriding /// the default "Content-Type"). @@ -58,6 +89,7 @@ cpr::Parameters GetParameters( return cpr_params; } +/// \brief Checks if the HTTP status code indicates a successful response. bool IsSuccessful(int32_t status_code) { return status_code == 200 // OK || status_code == 202 // Accepted @@ -65,6 +97,7 @@ bool IsSuccessful(int32_t status_code) { || status_code == 304; // Not Modified } +/// \brief Handles failure responses by invoking the provided error handler. Status HandleFailureResponse(const cpr::Response& response, const ErrorHandler& error_handler) { if (!IsSuccessful(response.status_code)) { @@ -88,13 +121,18 @@ void HttpClient::PrepareSession( session_->SetHeader(final_headers); } -HttpClient::HttpClient(const RestCatalogConfig& config) - : default_headers_{config.ExtractHeaders()}, +HttpClient::HttpClient(std::unordered_map default_headers) + : default_headers_{std::move(default_headers)}, session_{std::make_unique()} { + // Set default Content-Type for all requests (including GET/HEAD/DELETE). + // Many systems require that content type is set regardless and will fail, + // even on an empty bodied request. default_headers_[kHeaderContentType] = kMimeTypeApplicationJson; default_headers_[kHeaderUserAgent] = kUserAgent; } +HttpClient::~HttpClient() = default; + Result HttpClient::Get( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, @@ -104,7 +142,10 @@ Result HttpClient::Get( PrepareSession(path, headers, params); cpr::Response response = session_->Get(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - return HttpResponse(std::move(response)); + auto impl = std::make_unique(std::move(response)); + HttpResponse http_response; + http_response.impl_ = std::move(impl); + return http_response; } Result HttpClient::Post( @@ -117,7 +158,10 @@ Result HttpClient::Post( session_->SetBody(cpr::Body{body}); cpr::Response response = session_->Post(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - return HttpResponse(std::move(response)); + auto impl = std::make_unique(std::move(response)); + HttpResponse http_response; + http_response.impl_ = std::move(impl); + return http_response; } Result HttpClient::PostForm( @@ -127,7 +171,11 @@ Result HttpClient::PostForm( const ErrorHandler& error_handler) { std::scoped_lock lock(session_mutex_); - PrepareSession(path, headers); + // Override default Content-Type (application/json) with form-urlencoded + auto form_headers = headers; + form_headers[kHeaderContentType] = kMimeTypeFormUrlEncoded; + + PrepareSession(path, form_headers); std::vector pair_list; pair_list.reserve(form_data.size()); for (const auto& [key, val] : form_data) { @@ -136,7 +184,10 @@ Result HttpClient::PostForm( session_->SetPayload(cpr::Payload(pair_list.begin(), pair_list.end())); cpr::Response response = session_->Post(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - return HttpResponse(std::move(response)); + auto impl = std::make_unique(std::move(response)); + HttpResponse http_response; + http_response.impl_ = std::move(impl); + return http_response; } Result HttpClient::Head( @@ -147,7 +198,10 @@ Result HttpClient::Head( PrepareSession(path, headers); cpr::Response response = session_->Head(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - return HttpResponse(std::move(response)); + auto impl = std::make_unique(std::move(response)); + HttpResponse http_response; + http_response.impl_ = std::move(impl); + return http_response; } Result HttpClient::Delete( @@ -158,7 +212,10 @@ Result HttpClient::Delete( PrepareSession(path, headers); cpr::Response response = session_->Delete(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - return HttpResponse(std::move(response)); + auto impl = std::make_unique(std::move(response)); + HttpResponse http_response; + http_response.impl_ = std::move(impl); + return http_response; } } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/http_client.h b/src/iceberg/catalog/rest/http_client.h index 8978f0588..10d677697 100644 --- a/src/iceberg/catalog/rest/http_client.h +++ b/src/iceberg/catalog/rest/http_client.h @@ -19,29 +19,60 @@ #pragma once +#include #include #include #include #include -#include -#include - -#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/error_handlers.h" -#include "iceberg/catalog/rest/http_response.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/result.h" /// \file iceberg/catalog/rest/http_client.h /// \brief Http client for Iceberg REST API. +namespace cpr { +class Session; +} // namespace cpr + namespace iceberg::rest { +/// \brief A simple wrapper for cpr::Response. +/// +/// This class encapsulates the details of the underlying cpr library's response, +/// providing a consistent interface that is independent of the specific network +/// library used. +class ICEBERG_REST_EXPORT HttpResponse { + public: + HttpResponse(); + ~HttpResponse(); + + HttpResponse(const HttpResponse&) = delete; + HttpResponse& operator=(const HttpResponse&) = delete; + HttpResponse(HttpResponse&&) noexcept; + HttpResponse& operator=(HttpResponse&&) noexcept; + + /// \brief Get the HTTP status code of the response. + int32_t status_code() const; + + /// \brief Get the body of the response as a string. + std::string body() const; + + /// \brief Get the headers of the response as a map. + std::unordered_map headers() const; + + private: + friend class HttpClient; + class Impl; + std::unique_ptr impl_; +}; + /// \brief HTTP client for making requests to Iceberg REST Catalog API. class ICEBERG_REST_EXPORT HttpClient { public: - explicit HttpClient(const RestCatalogConfig&); + explicit HttpClient(std::unordered_map default_headers = {}); + ~HttpClient(); HttpClient(const HttpClient&) = delete; HttpClient& operator=(const HttpClient&) = delete; diff --git a/src/iceberg/catalog/rest/http_response.h b/src/iceberg/catalog/rest/http_response.h deleted file mode 100644 index 1e21c22cc..000000000 --- a/src/iceberg/catalog/rest/http_response.h +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#pragma once - -#include -#include - -#include "cpr/response.h" -#include "iceberg/catalog/rest/iceberg_rest_export.h" - -/// \file iceberg/catalog/rest/http_response.h -/// \brief A simple wrapper for cpr::Response. This class encapsulates the details of the -/// underlying cpr library's response, providing a consistent interface that is -/// independent of the specific network library used. - -class ICEBERG_REST_EXPORT HttpResponse { - public: - explicit HttpResponse(cpr::Response response) : response_(std::move(response)) {} - - /// \brief Get the HTTP status code of the response. - int32_t status_code() const { return response_.status_code; } - - /// \brief Get the body of the response as a string. - const std::string& body() const { return response_.text; } - - /// \brief Get the headers of the response as a map. - const std::unordered_map headers() const { - return {response_.header.begin(), response_.header.end()}; - } - - private: - cpr::Response response_; -}; diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index 936183d00..b858a9f1c 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -17,10 +17,12 @@ iceberg_rest_sources = files( 'catalog_properties.cc', + 'error_handlers.cc', 'http_client.cc', 'json_internal.cc', 'resource_paths.cc', 'rest_catalog.cc', + 'rest_util.cc', ) # cpr does not export symbols, so on Windows it must # be used as a static lib @@ -54,17 +56,16 @@ pkg.generate(iceberg_rest_lib) install_headers( [ - 'rest_catalog.h', 'catalog_properties.h', 'constant.h', + 'error_handlers.h', 'http_client.h', - 'http_response.h', - 'json_internal.h', 'iceberg_rest_export.h', - 'types.h', + 'json_internal.h', 'resource_paths.h', + 'rest_catalog.h', 'rest_util.h', - 'error_handlers.h', + 'types.h', ], subdir: 'iceberg/catalog/rest', ) diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc index 221e72938..e7b89860b 100644 --- a/src/iceberg/catalog/rest/resource_paths.cc +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -21,39 +21,13 @@ #include -#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/rest_util.h" -#include "iceberg/result.h" namespace iceberg::rest { -Result> ResourcePaths::Make( - const RestCatalogConfig& config) { - // Validate and extract URI - auto it = config.configs().find(RestCatalogConfig::kUri.key()); - if (it == config.configs().end() || it->second.empty()) { - return InvalidArgument("Rest catalog configuration property 'uri' is required."); - } - - std::string base_uri = std::string(TrimTrailingSlash(it->second)); - std::string prefix; - auto prefix_it = config.configs().find("prefix"); - if (prefix_it != config.configs().end() && !prefix_it->second.empty()) { - prefix = prefix_it->second; - } - - return std::unique_ptr( - new ResourcePaths(std::move(base_uri), std::move(prefix))); -} - ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} -std::string ResourcePaths::BuildPath(std::string_view path) const { - return std::format("{}/v1/{}{}", base_uri_, (prefix_.empty() ? "" : (prefix_ + "/")), - path); -} - std::string ResourcePaths::Config() const { return std::format("{}/v1/config", base_uri_); } @@ -62,59 +36,76 @@ std::string ResourcePaths::OAuth2Tokens() const { return std::format("{}/v1/oauth/tokens", base_uri_); } -std::string ResourcePaths::Namespaces() const { return BuildPath("namespaces"); } +std::string ResourcePaths::Namespaces() const { + return std::format("{}/v1/{}namespaces", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/"))); +} std::string ResourcePaths::Namespace_(const Namespace& ns) const { - return BuildPath(std::format("namespaces/{}", EncodeNamespaceForUrl(ns))); + return std::format("{}/v1/{}namespaces/{}", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); } std::string ResourcePaths::NamespaceProperties(const Namespace& ns) const { - return BuildPath(std::format("namespaces/{}/properties", EncodeNamespaceForUrl(ns))); + return std::format("{}/v1/{}namespaces/{}/properties", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); } std::string ResourcePaths::Tables(const Namespace& ns) const { - return BuildPath(std::format("namespaces/{}/tables", EncodeNamespaceForUrl(ns))); + return std::format("{}/v1/{}namespaces/{}/tables", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); } std::string ResourcePaths::Table(const TableIdentifier& ident) const { - return BuildPath(std::format("namespaces/{}/tables/{}", EncodeNamespaceForUrl(ident.ns), - ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), + EncodeNamespaceForUrl(ident.ns), ident.name); } std::string ResourcePaths::Register(const Namespace& ns) const { - return BuildPath(std::format("namespaces/{}/register", EncodeNamespaceForUrl(ns))); + return std::format("{}/v1/{}namespaces/{}/register", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); } -std::string ResourcePaths::Rename() const { return BuildPath("tables/rename"); } +std::string ResourcePaths::Rename() const { + return std::format("{}/v1/{}tables/rename", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/"))); +} std::string ResourcePaths::Metrics(const TableIdentifier& ident) const { - return BuildPath(std::format("namespaces/{}/tables/{}/metrics", - EncodeNamespaceForUrl(ident.ns), ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/metrics", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), + EncodeNamespaceForUrl(ident.ns), ident.name); } std::string ResourcePaths::Credentials(const TableIdentifier& ident) const { - return BuildPath(std::format("namespaces/{}/tables/{}/credentials", - EncodeNamespaceForUrl(ident.ns), ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/credentials", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), + EncodeNamespaceForUrl(ident.ns), ident.name); } std::string ResourcePaths::ScanPlan(const TableIdentifier& ident) const { - return BuildPath(std::format("namespaces/{}/tables/{}/plan", - EncodeNamespaceForUrl(ident.ns), ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/plan", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), + EncodeNamespaceForUrl(ident.ns), ident.name); } std::string ResourcePaths::ScanPlanResult(const TableIdentifier& ident, const std::string& plan_id) const { - return BuildPath(std::format("namespaces/{}/tables/{}/plan/{}", - EncodeNamespaceForUrl(ident.ns), ident.name, plan_id)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/plan/{}", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), + EncodeNamespaceForUrl(ident.ns), ident.name, plan_id); } std::string ResourcePaths::Tasks(const TableIdentifier& ident) const { - return BuildPath(std::format("namespaces/{}/tables/{}/tasks", - EncodeNamespaceForUrl(ident.ns), ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/tasks", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/")), + EncodeNamespaceForUrl(ident.ns), ident.name); } std::string ResourcePaths::CommitTransaction() const { - return BuildPath("transactions/commit"); + return std::format("{}/v1/{}transactions/commit", base_uri_, + (prefix_.empty() ? "" : (prefix_ + "/"))); } } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/resource_paths.h b/src/iceberg/catalog/rest/resource_paths.h index e5cc85525..14ecef89a 100644 --- a/src/iceberg/catalog/rest/resource_paths.h +++ b/src/iceberg/catalog/rest/resource_paths.h @@ -20,11 +20,8 @@ #pragma once #include -#include -#include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/result.h" #include "iceberg/table_identifier.h" /// \file iceberg/catalog/rest/resource_paths.h @@ -37,9 +34,10 @@ namespace iceberg::rest { /// This class constructs REST API endpoint URLs for various catalog operations. class ICEBERG_REST_EXPORT ResourcePaths { public: - /// \brief Construct a ResourcePaths with REST catalog configuration. - /// \param config The REST catalog configuration containing the base URI - static Result> Make(const RestCatalogConfig& config); + /// \brief Construct a ResourcePaths with base URI and optional prefix. + /// \param base_uri The base URI of the REST catalog server (without trailing slash) + /// \param prefix Optional prefix for REST API paths (default: empty) + explicit ResourcePaths(std::string base_uri, std::string prefix = ""); /// \brief Get the /v1/{prefix}/config endpoint path. std::string Config() const; @@ -93,13 +91,11 @@ class ICEBERG_REST_EXPORT ResourcePaths { std::string CommitTransaction() const; private: - explicit ResourcePaths(std::string base_uri, std::string prefix); - - // Helper to build path with optional prefix: {base_uri_}/{prefix_?}/{path} - std::string BuildPath(std::string_view path) const; - + /// \brief Base URI of the REST catalog server. std::string base_uri_; - std::string prefix_; // Optional prefix from config + + /// \brief Optional prefix for REST API paths from configuration. + std::string prefix_; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 7a2d07de7..ec0df1f7e 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -23,13 +23,12 @@ #include #include -#include +#include #include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/constant.h" #include "iceberg/catalog/rest/error_handlers.h" #include "iceberg/catalog/rest/http_client.h" -#include "iceberg/catalog/rest/http_response.h" #include "iceberg/catalog/rest/json_internal.h" #include "iceberg/catalog/rest/rest_catalog.h" #include "iceberg/catalog/rest/rest_util.h" @@ -41,13 +40,14 @@ namespace iceberg::rest { -Result> RestCatalog::FetchAndMergeConfig( - const RestCatalogConfig& config, const ResourcePaths& paths) { +Result> RestCatalog::FetchAndMergeConfig( + const RestCatalogProperties& config, const ResourcePaths& paths) { // Fetch server configuration - auto tmp_client = std::make_unique(config); + auto tmp_client = std::make_unique(config.ExtractHeaders()); const std::string endpoint = paths.Config(); - ICEBERG_ASSIGN_OR_RAISE(const auto response, - tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); + ICEBERG_ASSIGN_OR_RAISE( + const auto response, + tmp_client->Get(endpoint, {}, {}, *DefaultErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); @@ -55,30 +55,32 @@ Result> RestCatalog::FetchAndMergeConfig( // properties > server config defaults auto final_props = MergeConfigs(server_config.defaults, config.configs(), server_config.overrides); - return RestCatalogConfig::FromMap(final_props); + return RestCatalogProperties::FromMap(final_props); } -Result> RestCatalog::Make(const RestCatalogConfig& config) { - ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); - // Fetch and merge server configuration - ICEBERG_ASSIGN_OR_RAISE(auto final_config, FetchAndMergeConfig(config, *paths)); +Result> RestCatalog::Make( + const RestCatalogProperties& config) { + // Get URI and prefix from config + ICEBERG_ASSIGN_OR_RAISE(auto uri, config.Uri()); + std::string base_uri{TrimTrailingSlash(uri)}; + std::string prefix = config.Get(RestCatalogProperties::kPrefix); + ResourcePaths paths(base_uri, prefix); - auto client = std::make_unique(*final_config); - ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config)); + // Fetch and merge server configuration + ICEBERG_ASSIGN_OR_RAISE(auto final_config, FetchAndMergeConfig(config, paths)); - std::string catalog_name = final_config->Get(RestCatalogConfig::kName); + ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config->Uri()); + std::string final_base_uri{TrimTrailingSlash(final_uri)}; return std::unique_ptr( - new RestCatalog(std::move(final_config), std::move(client), std::move(*final_paths), - std::move(catalog_name))); + new RestCatalog(std::move(final_config), std::move(final_base_uri))); } -RestCatalog::RestCatalog(std::unique_ptr config, - std::unique_ptr client, ResourcePaths paths, - std::string name) +RestCatalog::RestCatalog(std::unique_ptr config, + std::string base_uri) : config_(std::move(config)), - client_(std::move(client)), - paths_(std::move(paths)), - name_(std::move(name)) {} + client_(std::make_shared(config_->ExtractHeaders())), + paths_(std::move(base_uri), config_->Get(RestCatalogProperties::kPrefix)), + name_(config_->Get(RestCatalogProperties::kName)) {} std::string_view RestCatalog::name() const { return name_; } @@ -94,8 +96,9 @@ Result> RestCatalog::ListNamespaces(const Namespace& ns) if (!next_token.empty()) { params[kQueryParamPageToken] = next_token; } - ICEBERG_ASSIGN_OR_RAISE(const auto& response, - client_->Get(endpoint, params, {}, NamespaceErrorHandler())); + ICEBERG_ASSIGN_OR_RAISE( + const auto& response, + client_->Get(endpoint, params, {}, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json)); result.insert(result.end(), list_response.namespaces.begin(), @@ -104,83 +107,93 @@ Result> RestCatalog::ListNamespaces(const Namespace& ns) return result; } next_token = list_response.next_page_token; - continue; } return result; } Status RestCatalog::CreateNamespace( - const Namespace& ns, const std::unordered_map& properties) { + [[maybe_unused]] const Namespace& ns, + [[maybe_unused]] const std::unordered_map& properties) { return NotImplemented("Not implemented"); } Result> RestCatalog::GetNamespaceProperties( - const Namespace& ns) const { + [[maybe_unused]] const Namespace& ns) const { return NotImplemented("Not implemented"); } -Status RestCatalog::DropNamespace(const Namespace& ns) { +Status RestCatalog::DropNamespace([[maybe_unused]] const Namespace& ns) { return NotImplemented("Not implemented"); } -Result RestCatalog::NamespaceExists(const Namespace& ns) const { +Result RestCatalog::NamespaceExists([[maybe_unused]] const Namespace& ns) const { return NotImplemented("Not implemented"); } Status RestCatalog::UpdateNamespaceProperties( - const Namespace& ns, const std::unordered_map& updates, - const std::unordered_set& removals) { + [[maybe_unused]] const Namespace& ns, + [[maybe_unused]] const std::unordered_map& updates, + [[maybe_unused]] const std::unordered_set& removals) { return NotImplemented("Not implemented"); } -Result> RestCatalog::ListTables(const Namespace& ns) const { +Result> RestCatalog::ListTables( + [[maybe_unused]] const Namespace& ns) const { return NotImplemented("Not implemented"); } Result> RestCatalog::CreateTable( - const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, - const std::string& location, - const std::unordered_map& properties) { + [[maybe_unused]] const TableIdentifier& identifier, + [[maybe_unused]] const Schema& schema, [[maybe_unused]] const PartitionSpec& spec, + [[maybe_unused]] const std::string& location, + [[maybe_unused]] const std::unordered_map& properties) { return NotImplemented("Not implemented"); } Result> RestCatalog::UpdateTable( - const TableIdentifier& identifier, - const std::vector>& requirements, - const std::vector>& updates) { + [[maybe_unused]] const TableIdentifier& identifier, + [[maybe_unused]] const std::vector>& requirements, + [[maybe_unused]] const std::vector>& updates) { return NotImplemented("Not implemented"); } Result> RestCatalog::StageCreateTable( - const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, - const std::string& location, - const std::unordered_map& properties) { + [[maybe_unused]] const TableIdentifier& identifier, + [[maybe_unused]] const Schema& schema, [[maybe_unused]] const PartitionSpec& spec, + [[maybe_unused]] const std::string& location, + [[maybe_unused]] const std::unordered_map& properties) { return NotImplemented("Not implemented"); } -Status RestCatalog::DropTable(const TableIdentifier& identifier, bool purge) { +Status RestCatalog::DropTable([[maybe_unused]] const TableIdentifier& identifier, + [[maybe_unused]] bool purge) { return NotImplemented("Not implemented"); } -Result RestCatalog::TableExists(const TableIdentifier& identifier) const { +Result RestCatalog::TableExists( + [[maybe_unused]] const TableIdentifier& identifier) const { return NotImplemented("Not implemented"); } -Status RestCatalog::RenameTable(const TableIdentifier& from, const TableIdentifier& to) { +Status RestCatalog::RenameTable([[maybe_unused]] const TableIdentifier& from, + [[maybe_unused]] const TableIdentifier& to) { return NotImplemented("Not implemented"); } -Result> RestCatalog::LoadTable(const TableIdentifier& identifier) { +Result> RestCatalog::LoadTable( + [[maybe_unused]] const TableIdentifier& identifier) { return NotImplemented("Not implemented"); } Result> RestCatalog::RegisterTable( - const TableIdentifier& identifier, const std::string& metadata_file_location) { + [[maybe_unused]] const TableIdentifier& identifier, + [[maybe_unused]] const std::string& metadata_file_location) { return NotImplemented("Not implemented"); } std::unique_ptr RestCatalog::BuildTable( - const TableIdentifier& identifier, const Schema& schema) const { + [[maybe_unused]] const TableIdentifier& identifier, + [[maybe_unused]] const Schema& schema) const { return nullptr; } diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h index 042428c00..29f62b220 100644 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -34,6 +34,7 @@ namespace iceberg::rest { +/// \brief Rest catalog implementation. class ICEBERG_REST_EXPORT RestCatalog : public Catalog { public: RestCatalog(const RestCatalog&) = delete; @@ -45,7 +46,7 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog { /// /// \param config the configuration for the RestCatalog /// \return a unique_ptr to RestCatalog instance - static Result> Make(const RestCatalogConfig& config); + static Result> Make(const RestCatalogProperties& config); std::string_view name() const override; @@ -99,21 +100,20 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog { const TableIdentifier& identifier, const Schema& schema) const override; private: - RestCatalog(std::unique_ptr config, - std::unique_ptr client, ResourcePaths paths, std::string name); + RestCatalog(std::unique_ptr config, std::string base_uri); /// \brief Fetch server configuration and merge with client config /// /// \param config the initial client configuration /// \param paths the resource paths for REST endpoints /// \return the final merged configuration - static Result> FetchAndMergeConfig( - const RestCatalogConfig& config, const ResourcePaths& paths); + static Result> FetchAndMergeConfig( + const RestCatalogProperties& config, const ResourcePaths& paths); - std::unique_ptr config_; - std::unique_ptr client_; + std::unique_ptr config_; + std::shared_ptr client_; ResourcePaths paths_; - std::string name_; // Cached catalog name + std::string name_; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_util.cc b/src/iceberg/catalog/rest/rest_util.cc new file mode 100644 index 000000000..9029410ce --- /dev/null +++ b/src/iceberg/catalog/rest/rest_util.cc @@ -0,0 +1,99 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/rest_util.h" + +#include +#include +#include + +#include + +namespace iceberg::rest { + +std::string_view TrimTrailingSlash(std::string_view str) { + while (!str.empty() && str.back() == '/') { + str.remove_suffix(1); + } + return str; +} + +std::string EncodeString(std::string_view toEncode) { + // Use CPR's urlEncode which internally calls libcurl's curl_easy_escape() + cpr::util::SecureString encoded = cpr::util::urlEncode(toEncode); + return {encoded.data(), encoded.size()}; +} + +std::string DecodeString(std::string_view encoded) { + // Use CPR's urlDecode which internally calls libcurl's curl_easy_unescape() + cpr::util::SecureString decoded = cpr::util::urlDecode(encoded); + return {decoded.data(), decoded.size()}; +} + +std::string EncodeNamespaceForUrl(const Namespace& ns) { + if (ns.levels.empty()) { + return ""; + } + std::string result = EncodeString(ns.levels.front()); + + // Join encoded levels with "%1F" + for (size_t i = 1; i < ns.levels.size(); ++i) { + result.append("%1F"); + result.append(EncodeString(ns.levels[i])); + } + + return result; +} + +Namespace DecodeNamespaceFromUrl(std::string_view encoded) { + if (encoded.empty()) { + return Namespace{.levels = {}}; + } + + // Split by "%1F" first + Namespace ns; + std::string::size_type start = 0; + std::string::size_type end = encoded.find("%1F"); + + while (end != std::string::npos) { + ns.levels.push_back(DecodeString(encoded.substr(start, end - start))); + // Skip the 3-character "%1F" separator + start = end + 3; + end = encoded.find("%1F", start); + } + ns.levels.push_back(DecodeString(encoded.substr(start))); + return ns; +} + +std::unordered_map MergeConfigs( + const std::unordered_map& server_defaults, + const std::unordered_map& client_configs, + const std::unordered_map& server_overrides) { + // Merge with precedence: server_overrides > client_configs > server_defaults + auto merged = server_defaults; + for (const auto& [key, value] : client_configs) { + merged.insert_or_assign(key, value); + } + for (const auto& [key, value] : server_overrides) { + merged.insert_or_assign(key, value); + } + return merged; +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_util.h b/src/iceberg/catalog/rest/rest_util.h index 65d0265fb..4b010d6b6 100644 --- a/src/iceberg/catalog/rest/rest_util.h +++ b/src/iceberg/catalog/rest/rest_util.h @@ -19,65 +19,53 @@ #pragma once -#include -#include #include #include #include +#include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/table_identifier.h" namespace iceberg::rest { -/// \brief Trim a single trailing slash from a URI string_view. -/// \details If \p uri_sv ends with '/', remove that last character; otherwise the input -/// is returned unchanged. -/// \param uri_sv The URI string to trim. -/// \return The (possibly) trimmed URI string. -inline std::string TrimTrailingSlash(std::string uri_sv) { - if (!uri_sv.empty() && uri_sv.back() == '/') { - uri_sv.pop_back(); - } - return uri_sv; -} +/// \brief Trim trailing slashes from a URI string. +/// \details Removes all trailing '/' characters from the URI. +/// \param str A string to trim. +/// \return The trimmed URI string with all trailing slashes removed. +ICEBERG_REST_EXPORT std::string_view TrimTrailingSlash(std::string_view str); -/// \brief Percent-encode a string as a URI component (RFC 3986). -/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are -/// percent-encoded using uppercase hexadecimal (e.g., space -> "%20"). -/// \param value The string to encode. -/// \return The encoded string. -inline std::string EncodeUriComponent(std::string_view value) { - std::string escaped; - escaped.reserve(value.length()); - for (const unsigned char c : value) { - if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { - escaped += c; - } else { - std::format_to(std::back_inserter(escaped), "%{:02X}", c); - } - } - return escaped; -} +/// \brief URL-encode a string (RFC 3986). +/// \details This implementation uses libcurl (via CPR), which follows RFC 3986 strictly: +/// - Unreserved characters: [A-Z], [a-z], [0-9], "-", "_", ".", "~" +/// - Space is encoded as "%20" (unlike Java's URLEncoder which uses "+"). +/// - All other characters are percent-encoded (%XX). +/// \param toEncode The string to encode. +/// \return The URL-encoded string. +/// \note Iceberg's Java client uses `java.net.URLEncoder`, which follows the +/// `application/x-www-form-urlencoded` standard (HTML Forms). We strictly adhere to RFC +/// 3986 to ensure correct URI path semantics (encoding spaces as %20 rather than +) for +/// maximum cross-platform interoperability. +ICEBERG_REST_EXPORT std::string EncodeString(std::string_view toEncode); + +/// \brief URL-decode a string. +/// \details Decodes percent-encoded characters (e.g., "%20" -> space). Uses libcurl's URL +/// decoding via the CPR library. +/// \param encoded The encoded string to decode. +/// \return The decoded string. +ICEBERG_REST_EXPORT std::string DecodeString(std::string_view encoded); /// \brief Encode a Namespace into a URL-safe component. -/// \details Joins \p ns.levels with the ASCII Unit Separator (0x1F), then percent-encodes -/// the result via EncodeUriComponent. Returns an empty string if there are no levels. +/// \details Encodes each level separately using EncodeString, then joins them with "%1F". /// \param ns The namespace (sequence of path-like levels) to encode. /// \return The percent-encoded namespace string suitable for URLs. -inline std::string EncodeNamespaceForUrl(const Namespace& ns) { - if (ns.levels.empty()) { - return ""; - } - - std::string joined_string; - joined_string.append(ns.levels.front()); - for (size_t i = 1; i < ns.levels.size(); ++i) { - joined_string.append("\x1F"); - joined_string.append(ns.levels[i]); - } +ICEBERG_REST_EXPORT std::string EncodeNamespaceForUrl(const Namespace& ns); - return EncodeUriComponent(joined_string); -} +/// \brief Decode a URL-encoded namespace string back to a Namespace. +/// \details Splits by "%1F" (the URL-encoded form of ASCII Unit Separator), then decodes +/// each level separately using DecodeString. +/// \param encoded The percent-encoded namespace string. +/// \return The decoded Namespace. +ICEBERG_REST_EXPORT Namespace DecodeNamespaceFromUrl(std::string_view encoded); /// \brief Merge catalog configuration properties. /// \details Merges three sets of configuration properties following the precedence order: @@ -86,18 +74,9 @@ inline std::string EncodeNamespaceForUrl(const Namespace& ns) { /// \param client_configs Configuration properties from the client. /// \param server_overrides Override properties enforced by the server. /// \return A merged map containing all properties with correct precedence. -inline std::unordered_map MergeConfigs( +ICEBERG_REST_EXPORT std::unordered_map MergeConfigs( const std::unordered_map& server_defaults, const std::unordered_map& client_configs, - const std::unordered_map& server_overrides) { - auto merged = server_defaults; - for (const auto& kv : client_configs) { - merged.insert_or_assign(kv.first, kv.second); - } - for (const auto& kv : server_overrides) { - merged.insert_or_assign(kv.first, kv.second); - } - return merged; -} + const std::unordered_map& server_overrides); } // namespace iceberg::rest diff --git a/src/iceberg/result.h b/src/iceberg/result.h index f77327365..8c90d07a8 100644 --- a/src/iceberg/result.h +++ b/src/iceberg/result.h @@ -35,6 +35,7 @@ enum class ErrorKind { kCommitStateUnknown, kDecompressError, kForbidden, + kInternalServerError, kInvalid, // For general invalid errors kInvalidArgument, kInvalidArrowData, @@ -53,10 +54,9 @@ enum class ErrorKind { kNotFound, kNotImplemented, kNotSupported, - kInternalServerError, + kRestError, kServiceUnavailable, kUnknownError, - kRestError, }; /// \brief Error with a kind and a message. @@ -84,6 +84,9 @@ using Status = Result; -> std::unexpected { \ return std::unexpected( \ {ErrorKind::k##name, std::format(fmt, std::forward(args)...)}); \ + } \ + inline auto name(const std::string& message) -> std::unexpected { \ + return std::unexpected({ErrorKind::k##name, message}); \ } DEFINE_ERROR_FUNCTION(AlreadyExists) @@ -92,6 +95,7 @@ DEFINE_ERROR_FUNCTION(CommitFailed) DEFINE_ERROR_FUNCTION(CommitStateUnknown) DEFINE_ERROR_FUNCTION(DecompressError) DEFINE_ERROR_FUNCTION(Forbidden) +DEFINE_ERROR_FUNCTION(InternalServerError) DEFINE_ERROR_FUNCTION(Invalid) DEFINE_ERROR_FUNCTION(InvalidArgument) DEFINE_ERROR_FUNCTION(InvalidArrowData) @@ -110,10 +114,9 @@ DEFINE_ERROR_FUNCTION(NotAuthorized) DEFINE_ERROR_FUNCTION(NotFound) DEFINE_ERROR_FUNCTION(NotImplemented) DEFINE_ERROR_FUNCTION(NotSupported) -DEFINE_ERROR_FUNCTION(InternalServerError) +DEFINE_ERROR_FUNCTION(RestError) DEFINE_ERROR_FUNCTION(ServiceUnavailable) DEFINE_ERROR_FUNCTION(UnknownError) -DEFINE_ERROR_FUNCTION(RestError) #undef DEFINE_ERROR_FUNCTION diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index d82fe17b8..fd3cae8b6 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -159,8 +159,11 @@ if(ICEBERG_BUILD_BUNDLE) endif() if(ICEBERG_BUILD_REST) - add_iceberg_test(rest_catalog_test SOURCES rest_catalog_test.cc - rest_json_internal_test.cc) + add_iceberg_test(rest_catalog_test + SOURCES + rest_catalog_test.cc + rest_json_internal_test.cc + rest_util_test.cc) target_link_libraries(rest_catalog_test PRIVATE iceberg_rest_static) target_include_directories(rest_catalog_test PRIVATE ${cpp-httplib_SOURCE_DIR}) endif() diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index 6fbe82dfd..17b8ec93d 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -91,6 +91,7 @@ if get_option('rest').enabled() 'sources': files( 'rest_catalog_test.cc', 'rest_json_internal_test.cc', + 'rest_util_test.cc', ), 'dependencies': [iceberg_rest_dep, cpp_httplib_dep], }, diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index c17f350a1..40befeedf 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -32,7 +32,7 @@ namespace iceberg::rest { // Test fixture for REST catalog tests, This assumes you have a local REST catalog service -// running Default configuration: http://localhost:8181 +// running Default configuration: http://localhost:8181. class RestCatalogTest : public ::testing::Test { protected: void SetUp() override { @@ -44,18 +44,19 @@ class RestCatalogTest : public ::testing::Test { std::string uri = uri_env ? uri_env : "http://localhost:8181"; std::string warehouse = warehouse_env ? warehouse_env : "default"; - config_.Set(RestCatalogConfig::kUri, uri) - .Set(RestCatalogConfig::kName, std::string("test_catalog")) - .Set(RestCatalogConfig::kWarehouse, warehouse); + config_ = RestCatalogProperties::default_properties(); + config_->Set(RestCatalogProperties::kUri, uri) + .Set(RestCatalogProperties::kName, std::string("test_catalog")) + .Set(RestCatalogProperties::kWarehouse, warehouse); } void TearDown() override {} - RestCatalogConfig config_; + std::unique_ptr config_; }; -TEST_F(RestCatalogTest, MakeCatalogSuccess) { - auto catalog_result = RestCatalog::Make(config_); +TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) { + auto catalog_result = RestCatalog::Make(*config_); EXPECT_THAT(catalog_result, IsOk()); if (catalog_result.has_value()) { @@ -65,27 +66,32 @@ TEST_F(RestCatalogTest, MakeCatalogSuccess) { } TEST_F(RestCatalogTest, DISABLED_MakeCatalogEmptyUri) { - RestCatalogConfig invalid_config = config_; - invalid_config.Set(RestCatalogConfig::kUri, std::string("")); + auto invalid_config = RestCatalogProperties::default_properties(); + invalid_config->Set(RestCatalogProperties::kUri, std::string("")); - auto catalog_result = RestCatalog::Make(invalid_config); + auto catalog_result = RestCatalog::Make(*invalid_config); EXPECT_THAT(catalog_result, IsError(ErrorKind::kInvalidArgument)); EXPECT_THAT(catalog_result, HasErrorMessage("uri")); } -TEST_F(RestCatalogTest, MakeCatalogWithCustomProperties) { - RestCatalogConfig custom_config = config_; +TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) { + auto custom_config = RestCatalogProperties::default_properties(); custom_config - .Set(RestCatalogConfig::Entry{"custom_prop", ""}, + ->Set(RestCatalogProperties::kUri, config_->Get(RestCatalogProperties::kUri)) + .Set(RestCatalogProperties::kName, config_->Get(RestCatalogProperties::kName)) + .Set(RestCatalogProperties::kWarehouse, + config_->Get(RestCatalogProperties::kWarehouse)) + .Set(RestCatalogProperties::Entry{"custom_prop", ""}, std::string("custom_value")) - .Set(RestCatalogConfig::Entry{"timeout", ""}, std::string("30000")); + .Set(RestCatalogProperties::Entry{"timeout", ""}, + std::string("30000")); - auto catalog_result = RestCatalog::Make(custom_config); + auto catalog_result = RestCatalog::Make(*custom_config); EXPECT_THAT(catalog_result, IsOk()); } -TEST_F(RestCatalogTest, ListNamespaces) { - auto catalog_result = RestCatalog::Make(config_); +TEST_F(RestCatalogTest, DISABLED_ListNamespaces) { + auto catalog_result = RestCatalog::Make(*config_); ASSERT_THAT(catalog_result, IsOk()); auto& catalog = catalog_result.value(); @@ -97,7 +103,7 @@ TEST_F(RestCatalogTest, ListNamespaces) { } TEST_F(RestCatalogTest, DISABLED_CreateNamespaceNotImplemented) { - auto catalog_result = RestCatalog::Make(config_); + auto catalog_result = RestCatalog::Make(*config_); ASSERT_THAT(catalog_result, IsOk()); auto catalog = std::move(catalog_result.value()); @@ -109,7 +115,7 @@ TEST_F(RestCatalogTest, DISABLED_CreateNamespaceNotImplemented) { } TEST_F(RestCatalogTest, DISABLED_IntegrationTestFullNamespaceWorkflow) { - auto catalog_result = RestCatalog::Make(config_); + auto catalog_result = RestCatalog::Make(*config_); ASSERT_THAT(catalog_result, IsOk()); auto catalog = std::move(catalog_result.value()); diff --git a/src/iceberg/test/rest_util_test.cc b/src/iceberg/test/rest_util_test.cc new file mode 100644 index 000000000..b0a876373 --- /dev/null +++ b/src/iceberg/test/rest_util_test.cc @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/rest_util.h" + +#include + +#include "iceberg/table_identifier.h" + +namespace iceberg::rest { + +TEST(RestUtilTest, TrimTrailingSlash) { + EXPECT_EQ(TrimTrailingSlash("https://foo"), "https://foo"); + EXPECT_EQ(TrimTrailingSlash("https://foo/"), "https://foo"); + EXPECT_EQ(TrimTrailingSlash("https://foo////"), "https://foo"); +} + +// Ported from Java TestRESTUtil.testRoundTripUrlEncodeDecodeNamespace +TEST(RestUtilTest, RoundTripUrlEncodeDecodeNamespace) { + // {"dogs"} + EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs"}}), "dogs"); + EXPECT_EQ(DecodeNamespaceFromUrl("dogs").levels, std::vector{"dogs"}); + + // {"dogs.named.hank"} + EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs.named.hank"}}), + "dogs.named.hank"); + EXPECT_EQ(DecodeNamespaceFromUrl("dogs.named.hank").levels, + std::vector{"dogs.named.hank"}); + + // {"dogs/named/hank"} + EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs/named/hank"}}), + "dogs%2Fnamed%2Fhank"); + EXPECT_EQ(DecodeNamespaceFromUrl("dogs%2Fnamed%2Fhank").levels, + std::vector{"dogs/named/hank"}); + + // {"dogs", "named", "hank"} + EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs", "named", "hank"}}), + "dogs%1Fnamed%1Fhank"); + EXPECT_EQ(DecodeNamespaceFromUrl("dogs%1Fnamed%1Fhank").levels, + (std::vector{"dogs", "named", "hank"})); + + // {"dogs.and.cats", "named", "hank.or.james-westfall"} + EXPECT_EQ(EncodeNamespaceForUrl(Namespace{ + .levels = {"dogs.and.cats", "named", "hank.or.james-westfall"}}), + "dogs.and.cats%1Fnamed%1Fhank.or.james-westfall"); + EXPECT_EQ( + DecodeNamespaceFromUrl("dogs.and.cats%1Fnamed%1Fhank.or.james-westfall").levels, + (std::vector{"dogs.and.cats", "named", "hank.or.james-westfall"})); + + // empty namespace + EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {}}), ""); + EXPECT_EQ(DecodeNamespaceFromUrl("").levels, std::vector{}); +} + +TEST(RestUtilTest, EncodeString) { + // RFC 3986 unreserved characters should not be encoded + EXPECT_EQ(EncodeString("abc123XYZ"), "abc123XYZ"); + EXPECT_EQ(EncodeString("test-file_name.txt~backup"), "test-file_name.txt~backup"); + + // Spaces and special characters should be encoded + EXPECT_EQ(EncodeString("hello world"), "hello%20world"); + EXPECT_EQ(EncodeString("test@example.com"), "test%40example.com"); + EXPECT_EQ(EncodeString("path/to/file"), "path%2Fto%2Ffile"); + EXPECT_EQ(EncodeString("key=value&foo=bar"), "key%3Dvalue%26foo%3Dbar"); + EXPECT_EQ(EncodeString("100%"), "100%25"); + + // ASCII Unit Separator (0x1F) - important for Iceberg namespaces + EXPECT_EQ(EncodeString("hello\x1Fworld"), "hello%1Fworld"); + EXPECT_EQ(EncodeString(""), ""); +} + +TEST(RestUtilTest, DecodeString) { + // Decode percent-encoded strings + EXPECT_EQ(DecodeString("hello%20world"), "hello world"); + EXPECT_EQ(DecodeString("test%40example.com"), "test@example.com"); + EXPECT_EQ(DecodeString("path%2Fto%2Ffile"), "path/to/file"); + EXPECT_EQ(DecodeString("key%3Dvalue%26foo%3Dbar"), "key=value&foo=bar"); + EXPECT_EQ(DecodeString("100%25"), "100%"); + + // ASCII Unit Separator (0x1F) + EXPECT_EQ(DecodeString("hello%1Fworld"), "hello\x1Fworld"); + + // Unreserved characters remain unchanged + EXPECT_EQ(DecodeString("test-file_name.txt~backup"), "test-file_name.txt~backup"); + EXPECT_EQ(DecodeString(""), ""); +} + +TEST(RestUtilTest, EncodeDecodeStringRoundTrip) { + std::vector test_cases = {"hello world", + "test@example.com", + "path/to/file", + "key=value&foo=bar", + "100%", + "hello\x1Fworld", + "special!@#$%^&*()chars", + "mixed-123_test.file~ok", + ""}; + + for (const auto& test : test_cases) { + std::string encoded = EncodeString(test); + std::string decoded = DecodeString(encoded); + EXPECT_EQ(decoded, test) << "Round-trip failed for: " << test; + } +} + +TEST(RestUtilTest, MergeConfigs) { + std::unordered_map server_defaults = { + {"default1", "value1"}, {"default2", "value2"}, {"common", "default_value"}}; + + std::unordered_map client_configs = { + {"client1", "value1"}, {"common", "client_value"}}; + + std::unordered_map server_overrides = { + {"override1", "value1"}, {"common", "override_value"}}; + + auto merged = MergeConfigs(server_defaults, client_configs, server_overrides); + + EXPECT_EQ(merged.size(), 5); + + // Check precedence: server_overrides > client_configs > server_defaults + EXPECT_EQ(merged["default1"], "value1"); + EXPECT_EQ(merged["default2"], "value2"); + EXPECT_EQ(merged["client1"], "value1"); + EXPECT_EQ(merged["override1"], "value1"); + EXPECT_EQ(merged["common"], "override_value"); + + // Test with empty maps + auto merged_empty = MergeConfigs({}, {{"key", "value"}}, {}); + EXPECT_EQ(merged_empty.size(), 1); + EXPECT_EQ(merged_empty["key"], "value"); +} + +} // namespace iceberg::rest From f1685bed745d597f78ea03157a15e511edc4d0b0 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Tue, 25 Nov 2025 15:44:12 +0800 Subject: [PATCH 09/12] fresh --- src/iceberg/test/rest_util_test.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/iceberg/test/rest_util_test.cc b/src/iceberg/test/rest_util_test.cc index b0a876373..5dfed419a 100644 --- a/src/iceberg/test/rest_util_test.cc +++ b/src/iceberg/test/rest_util_test.cc @@ -31,7 +31,6 @@ TEST(RestUtilTest, TrimTrailingSlash) { EXPECT_EQ(TrimTrailingSlash("https://foo////"), "https://foo"); } -// Ported from Java TestRESTUtil.testRoundTripUrlEncodeDecodeNamespace TEST(RestUtilTest, RoundTripUrlEncodeDecodeNamespace) { // {"dogs"} EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs"}}), "dogs"); @@ -79,8 +78,6 @@ TEST(RestUtilTest, EncodeString) { EXPECT_EQ(EncodeString("path/to/file"), "path%2Fto%2Ffile"); EXPECT_EQ(EncodeString("key=value&foo=bar"), "key%3Dvalue%26foo%3Dbar"); EXPECT_EQ(EncodeString("100%"), "100%25"); - - // ASCII Unit Separator (0x1F) - important for Iceberg namespaces EXPECT_EQ(EncodeString("hello\x1Fworld"), "hello%1Fworld"); EXPECT_EQ(EncodeString(""), ""); } From 7267af5f6159466c8fabffa92c2b3de90e5f37c3 Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Wed, 26 Nov 2025 14:01:14 +0800 Subject: [PATCH 10/12] polish impls --- .../catalog/rest/catalog_properties.cc | 13 +-- src/iceberg/catalog/rest/catalog_properties.h | 8 +- src/iceberg/catalog/rest/error_handlers.cc | 4 +- src/iceberg/catalog/rest/error_handlers.h | 2 +- src/iceberg/catalog/rest/http_client.cc | 87 ++++++++------ src/iceberg/catalog/rest/http_client.h | 6 +- src/iceberg/catalog/rest/meson.build | 1 + src/iceberg/catalog/rest/resource_paths.cc | 110 +++++++++--------- src/iceberg/catalog/rest/resource_paths.h | 56 ++++----- src/iceberg/catalog/rest/rest_catalog.cc | 65 ++++++----- src/iceberg/catalog/rest/rest_catalog.h | 21 ++-- src/iceberg/catalog/rest/rest_util.cc | 77 +++++++----- src/iceberg/catalog/rest/rest_util.h | 39 ++++--- src/iceberg/catalog/rest/type_fwd.h | 35 ++++++ src/iceberg/table_identifier.h | 6 + src/iceberg/test/rest_util_test.cc | 96 ++++++++------- src/iceberg/util/config.h | 13 +++ 17 files changed, 360 insertions(+), 279 deletions(-) create mode 100644 src/iceberg/catalog/rest/type_fwd.h diff --git a/src/iceberg/catalog/rest/catalog_properties.cc b/src/iceberg/catalog/rest/catalog_properties.cc index c61cf35cf..4d956837c 100644 --- a/src/iceberg/catalog/rest/catalog_properties.cc +++ b/src/iceberg/catalog/rest/catalog_properties.cc @@ -37,18 +37,7 @@ std::unique_ptr RestCatalogProperties::FromMap( std::unordered_map RestCatalogProperties::ExtractHeaders() const { - std::unordered_map headers; - constexpr std::string_view prefix = "header."; - for (const auto& [key, value] : configs_) { - if (key.starts_with(prefix)) { - std::string header_name = key.substr(prefix.length()); - if (header_name.empty() || value.empty()) { - continue; - } - headers[header_name] = value; - } - } - return headers; + return Extract(kHeaderPrefix); } Result RestCatalogProperties::Uri() const { diff --git a/src/iceberg/catalog/rest/catalog_properties.h b/src/iceberg/catalog/rest/catalog_properties.h index 39e6a7a9d..d351b50fc 100644 --- a/src/iceberg/catalog/rest/catalog_properties.h +++ b/src/iceberg/catalog/rest/catalog_properties.h @@ -41,15 +41,14 @@ class ICEBERG_REST_EXPORT RestCatalogProperties /// \brief The URI of the REST catalog server. inline static Entry kUri{"uri", ""}; - /// \brief The name of the catalog. inline static Entry kName{"name", ""}; - /// \brief The warehouse path. inline static Entry kWarehouse{"warehouse", ""}; - /// \brief The optional prefix for REST API paths. inline static Entry kPrefix{"prefix", ""}; + /// \brief The prefix for HTTP headers. + inline static constexpr std::string_view kHeaderPrefix = "header."; /// \brief Create a default RestCatalogProperties instance. static std::unique_ptr default_properties(); @@ -59,9 +58,6 @@ class ICEBERG_REST_EXPORT RestCatalogProperties const std::unordered_map& properties); /// \brief Returns HTTP headers to be added to every request. - /// - /// This includes any key prefixed with "header." in the properties. - /// \return A map of headers with the prefix removed from the keys. std::unordered_map ExtractHeaders() const; /// \brief Get the URI of the REST catalog server. diff --git a/src/iceberg/catalog/rest/error_handlers.cc b/src/iceberg/catalog/rest/error_handlers.cc index 243e9e8a3..8465c0018 100644 --- a/src/iceberg/catalog/rest/error_handlers.cc +++ b/src/iceberg/catalog/rest/error_handlers.cc @@ -21,6 +21,8 @@ #include +#include "iceberg/catalog/rest/types.h" + namespace iceberg::rest { namespace { @@ -58,7 +60,7 @@ Status DefaultErrorHandler::Accept(const ErrorModel& error) const { return ServiceUnavailable("Service unavailable: {}", error.message); } - return RestError("Unable to process: {}", error.message); + return RestError("Code: {}, message: {}", error.code, error.message); } const std::shared_ptr& NamespaceErrorHandler::Instance() { diff --git a/src/iceberg/catalog/rest/error_handlers.h b/src/iceberg/catalog/rest/error_handlers.h index 5dd5abb8e..072d70442 100644 --- a/src/iceberg/catalog/rest/error_handlers.h +++ b/src/iceberg/catalog/rest/error_handlers.h @@ -22,7 +22,7 @@ #include #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/catalog/rest/types.h" +#include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" /// \file iceberg/catalog/rest/error_handlers.h diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index 40a1266fd..60a8eb142 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -23,8 +23,10 @@ #include #include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/error_handlers.h" #include "iceberg/catalog/rest/json_internal.h" #include "iceberg/json_internal.h" +#include "iceberg/result.h" #include "iceberg/util/macros.h" namespace iceberg::rest { @@ -47,11 +49,8 @@ class HttpResponse::Impl { }; HttpResponse::HttpResponse(HttpResponse&&) noexcept = default; - HttpResponse& HttpResponse::operator=(HttpResponse&&) noexcept = default; - HttpResponse::HttpResponse() = default; - HttpResponse::~HttpResponse() = default; int32_t HttpResponse::status_code() const { return impl_->status_code(); } @@ -101,6 +100,7 @@ bool IsSuccessful(int32_t status_code) { Status HandleFailureResponse(const cpr::Response& response, const ErrorHandler& error_handler) { if (!IsSuccessful(response.status_code)) { + // TODO(gangwu): response status code is lost, wrap it with RestError. ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); ICEBERG_ASSIGN_OR_RAISE(auto error_response, ErrorResponseFromJson(json)); return error_handler.Accept(error_response.error); @@ -137,14 +137,16 @@ Result HttpClient::Get( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::scoped_lock lock(session_mutex_); + cpr::Response response; + { + std::scoped_lock lock(session_mutex_); + PrepareSession(path, headers, params); + response = session_->Get(); + } - PrepareSession(path, headers, params); - cpr::Response response = session_->Get(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - auto impl = std::make_unique(std::move(response)); HttpResponse http_response; - http_response.impl_ = std::move(impl); + http_response.impl_ = std::make_unique(std::move(response)); return http_response; } @@ -152,15 +154,17 @@ Result HttpClient::Post( const std::string& path, const std::string& body, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::scoped_lock lock(session_mutex_); + cpr::Response response; + { + std::scoped_lock lock(session_mutex_); + PrepareSession(path, headers); + session_->SetBody(cpr::Body{body}); + response = session_->Post(); + } - PrepareSession(path, headers); - session_->SetBody(cpr::Body{body}); - cpr::Response response = session_->Post(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - auto impl = std::make_unique(std::move(response)); HttpResponse http_response; - http_response.impl_ = std::move(impl); + http_response.impl_ = std::make_unique(std::move(response)); return http_response; } @@ -169,52 +173,61 @@ Result HttpClient::PostForm( const std::unordered_map& form_data, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::scoped_lock lock(session_mutex_); + cpr::Response response; - // Override default Content-Type (application/json) with form-urlencoded - auto form_headers = headers; - form_headers[kHeaderContentType] = kMimeTypeFormUrlEncoded; + { + std::scoped_lock lock(session_mutex_); - PrepareSession(path, form_headers); - std::vector pair_list; - pair_list.reserve(form_data.size()); - for (const auto& [key, val] : form_data) { - pair_list.emplace_back(key, val); + // Override default Content-Type (application/json) with form-urlencoded + auto form_headers = headers; + form_headers[kHeaderContentType] = kMimeTypeFormUrlEncoded; + + PrepareSession(path, form_headers); + std::vector pair_list; + pair_list.reserve(form_data.size()); + for (const auto& [key, val] : form_data) { + pair_list.emplace_back(key, val); + } + session_->SetPayload(cpr::Payload(pair_list.begin(), pair_list.end())); + + response = session_->Post(); } - session_->SetPayload(cpr::Payload(pair_list.begin(), pair_list.end())); - cpr::Response response = session_->Post(); + ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - auto impl = std::make_unique(std::move(response)); HttpResponse http_response; - http_response.impl_ = std::move(impl); + http_response.impl_ = std::make_unique(std::move(response)); return http_response; } Result HttpClient::Head( const std::string& path, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::scoped_lock lock(session_mutex_); + cpr::Response response; + { + std::scoped_lock lock(session_mutex_); + PrepareSession(path, headers); + response = session_->Head(); + } - PrepareSession(path, headers); - cpr::Response response = session_->Head(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - auto impl = std::make_unique(std::move(response)); HttpResponse http_response; - http_response.impl_ = std::move(impl); + http_response.impl_ = std::make_unique(std::move(response)); return http_response; } Result HttpClient::Delete( const std::string& path, const std::unordered_map& headers, const ErrorHandler& error_handler) { - std::scoped_lock lock(session_mutex_); + cpr::Response response; + { + std::scoped_lock lock(session_mutex_); + PrepareSession(path, headers); + response = session_->Delete(); + } - PrepareSession(path, headers); - cpr::Response response = session_->Delete(); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); - auto impl = std::make_unique(std::move(response)); HttpResponse http_response; - http_response.impl_ = std::move(impl); + http_response.impl_ = std::make_unique(std::move(response)); return http_response; } diff --git a/src/iceberg/catalog/rest/http_client.h b/src/iceberg/catalog/rest/http_client.h index 10d677697..56c9f2902 100644 --- a/src/iceberg/catalog/rest/http_client.h +++ b/src/iceberg/catalog/rest/http_client.h @@ -25,8 +25,8 @@ #include #include -#include "iceberg/catalog/rest/error_handlers.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" /// \file iceberg/catalog/rest/http_client.h @@ -114,12 +114,10 @@ class ICEBERG_REST_EXPORT HttpClient { std::unordered_map default_headers_; - // Mutex to protect the non-thread-safe cpr::Session. - mutable std::mutex session_mutex_; - // TODO(Li Feiyang): use connection pool to support external multi-threaded concurrent // calls std::unique_ptr session_; + mutable std::mutex session_mutex_; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index b858a9f1c..89a68850e 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -65,6 +65,7 @@ install_headers( 'resource_paths.h', 'rest_catalog.h', 'rest_util.h', + 'type_fwd.h', 'types.h', ], subdir: 'iceberg/catalog/rest', diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc index e7b89860b..c81467c4e 100644 --- a/src/iceberg/catalog/rest/resource_paths.cc +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -22,90 +22,92 @@ #include #include "iceberg/catalog/rest/rest_util.h" +#include "iceberg/table_identifier.h" +#include "iceberg/util/macros.h" namespace iceberg::rest { -ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) - : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} - -std::string ResourcePaths::Config() const { - return std::format("{}/v1/config", base_uri_); +Result> ResourcePaths::Make(std::string base_uri, + const std::string& prefix) { + if (base_uri.empty()) { + return InvalidArgument("Base URI is empty"); + } + return std::unique_ptr(new ResourcePaths(std::move(base_uri), prefix)); } -std::string ResourcePaths::OAuth2Tokens() const { - return std::format("{}/v1/oauth/tokens", base_uri_); -} +ResourcePaths::ResourcePaths(std::string base_uri, const std::string& prefix) + : base_uri_(std::move(base_uri)), prefix_(prefix.empty() ? "" : (prefix + "/")) {} -std::string ResourcePaths::Namespaces() const { - return std::format("{}/v1/{}namespaces", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/"))); +Status ResourcePaths::SetBaseUri(const std::string& base_uri) { + if (base_uri.empty()) { + return InvalidArgument("Base URI is empty"); + } + base_uri_ = base_uri; + return {}; } -std::string ResourcePaths::Namespace_(const Namespace& ns) const { - return std::format("{}/v1/{}namespaces/{}", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); +Result ResourcePaths::Config() const { + return std::format("{}/v1/{}config", base_uri_, prefix_); } -std::string ResourcePaths::NamespaceProperties(const Namespace& ns) const { - return std::format("{}/v1/{}namespaces/{}/properties", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); +Result ResourcePaths::OAuth2Tokens() const { + return std::format("{}/v1/{}oauth/tokens", base_uri_, prefix_); } -std::string ResourcePaths::Tables(const Namespace& ns) const { - return std::format("{}/v1/{}namespaces/{}/tables", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); +Result ResourcePaths::Namespaces() const { + return std::format("{}/v1/{}namespaces", base_uri_, prefix_); } -std::string ResourcePaths::Table(const TableIdentifier& ident) const { - return std::format("{}/v1/{}namespaces/{}/tables/{}", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), - EncodeNamespaceForUrl(ident.ns), ident.name); +Result ResourcePaths::Namespace_(const Namespace& ns) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns)); + return std::format("{}/v1/{}namespaces/{}", base_uri_, prefix_, encoded_namespace); } -std::string ResourcePaths::Register(const Namespace& ns) const { - return std::format("{}/v1/{}namespaces/{}/register", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), EncodeNamespaceForUrl(ns)); +Result ResourcePaths::NamespaceProperties(const Namespace& ns) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns)); + return std::format("{}/v1/{}namespaces/{}/properties", base_uri_, prefix_, + encoded_namespace); } -std::string ResourcePaths::Rename() const { - return std::format("{}/v1/{}tables/rename", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/"))); +Result ResourcePaths::Tables(const Namespace& ns) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns)); + return std::format("{}/v1/{}namespaces/{}/tables", base_uri_, prefix_, + encoded_namespace); } -std::string ResourcePaths::Metrics(const TableIdentifier& ident) const { - return std::format("{}/v1/{}namespaces/{}/tables/{}/metrics", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), - EncodeNamespaceForUrl(ident.ns), ident.name); +Result ResourcePaths::Table(const TableIdentifier& ident) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ident.ns)); + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name, EncodeString(ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}", base_uri_, prefix_, + encoded_namespace, encoded_table_name); } -std::string ResourcePaths::Credentials(const TableIdentifier& ident) const { - return std::format("{}/v1/{}namespaces/{}/tables/{}/credentials", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), - EncodeNamespaceForUrl(ident.ns), ident.name); +Result ResourcePaths::Register(const Namespace& ns) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ns)); + return std::format("{}/v1/{}namespaces/{}/register", base_uri_, prefix_, + encoded_namespace); } -std::string ResourcePaths::ScanPlan(const TableIdentifier& ident) const { - return std::format("{}/v1/{}namespaces/{}/tables/{}/plan", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), - EncodeNamespaceForUrl(ident.ns), ident.name); +Result ResourcePaths::Rename() const { + return std::format("{}/v1/{}tables/rename", base_uri_, prefix_); } -std::string ResourcePaths::ScanPlanResult(const TableIdentifier& ident, - const std::string& plan_id) const { - return std::format("{}/v1/{}namespaces/{}/tables/{}/plan/{}", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), - EncodeNamespaceForUrl(ident.ns), ident.name, plan_id); +Result ResourcePaths::Metrics(const TableIdentifier& ident) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ident.ns)); + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name, EncodeString(ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/metrics", base_uri_, prefix_, + encoded_namespace, encoded_table_name); } -std::string ResourcePaths::Tasks(const TableIdentifier& ident) const { - return std::format("{}/v1/{}namespaces/{}/tables/{}/tasks", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/")), - EncodeNamespaceForUrl(ident.ns), ident.name); +Result ResourcePaths::Credentials(const TableIdentifier& ident) const { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace, EncodeNamespace(ident.ns)); + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name, EncodeString(ident.name)); + return std::format("{}/v1/{}namespaces/{}/tables/{}/credentials", base_uri_, prefix_, + encoded_namespace, encoded_table_name); } -std::string ResourcePaths::CommitTransaction() const { - return std::format("{}/v1/{}transactions/commit", base_uri_, - (prefix_.empty() ? "" : (prefix_ + "/"))); +Result ResourcePaths::CommitTransaction() const { + return std::format("{}/v1/{}transactions/commit", base_uri_, prefix_); } } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/resource_paths.h b/src/iceberg/catalog/rest/resource_paths.h index 14ecef89a..9d0bdda63 100644 --- a/src/iceberg/catalog/rest/resource_paths.h +++ b/src/iceberg/catalog/rest/resource_paths.h @@ -19,10 +19,13 @@ #pragma once +#include #include #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/table_identifier.h" +#include "iceberg/catalog/rest/type_fwd.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" /// \file iceberg/catalog/rest/resource_paths.h /// \brief Resource path construction for Iceberg REST API endpoints. @@ -37,65 +40,56 @@ class ICEBERG_REST_EXPORT ResourcePaths { /// \brief Construct a ResourcePaths with base URI and optional prefix. /// \param base_uri The base URI of the REST catalog server (without trailing slash) /// \param prefix Optional prefix for REST API paths (default: empty) - explicit ResourcePaths(std::string base_uri, std::string prefix = ""); + /// \return A unique_ptr to ResourcePaths instance + static Result> Make(std::string base_uri, + const std::string& prefix); + + /// \brief Set the base URI of the REST catalog server. + Status SetBaseUri(const std::string& base_uri); /// \brief Get the /v1/{prefix}/config endpoint path. - std::string Config() const; + Result Config() const; /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. - std::string OAuth2Tokens() const; + Result OAuth2Tokens() const; /// \brief Get the /v1/{prefix}/namespaces endpoint path. - std::string Namespaces() const; + Result Namespaces() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. - std::string Namespace_(const Namespace& ns) const; + Result Namespace_(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. - std::string NamespaceProperties(const Namespace& ns) const; + Result NamespaceProperties(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. - std::string Tables(const Namespace& ns) const; + Result Tables(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. - std::string Table(const TableIdentifier& ident) const; + Result Table(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. - std::string Register(const Namespace& ns) const; + Result Register(const Namespace& ns) const; /// \brief Get the /v1/{prefix}/tables/rename endpoint path. - std::string Rename() const; + Result Rename() const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint /// path. - std::string Metrics(const TableIdentifier& ident) const; + Result Metrics(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials /// endpoint path. - std::string Credentials(const TableIdentifier& ident) const; - - /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint - /// path. - std::string ScanPlan(const TableIdentifier& ident) const; - - /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{planId} - /// endpoint path. - std::string ScanPlanResult(const TableIdentifier& ident, - const std::string& plan_id) const; - - /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint - /// path. - std::string Tasks(const TableIdentifier& ident) const; + Result Credentials(const TableIdentifier& ident) const; /// \brief Get the /v1/{prefix}/transactions/commit endpoint path. - std::string CommitTransaction() const; + Result CommitTransaction() const; private: - /// \brief Base URI of the REST catalog server. - std::string base_uri_; + ResourcePaths(std::string base_uri, const std::string& prefix); - /// \brief Optional prefix for REST API paths from configuration. - std::string prefix_; + std::string base_uri_; // required + const std::string prefix_; // optional }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index ec0df1f7e..5afa91f54 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -30,9 +30,9 @@ #include "iceberg/catalog/rest/error_handlers.h" #include "iceberg/catalog/rest/http_client.h" #include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/resource_paths.h" #include "iceberg/catalog/rest/rest_catalog.h" #include "iceberg/catalog/rest/rest_util.h" -#include "iceberg/catalog/rest/types.h" #include "iceberg/json_internal.h" #include "iceberg/result.h" #include "iceberg/table.h" @@ -40,65 +40,68 @@ namespace iceberg::rest { -Result> RestCatalog::FetchAndMergeConfig( - const RestCatalogProperties& config, const ResourcePaths& paths) { - // Fetch server configuration - auto tmp_client = std::make_unique(config.ExtractHeaders()); - const std::string endpoint = paths.Config(); - ICEBERG_ASSIGN_OR_RAISE( - const auto response, - tmp_client->Get(endpoint, {}, {}, *DefaultErrorHandler::Instance())); +namespace { + +// Fetch server config and merge it with client config +Result> FetchConfig( + const ResourcePaths& paths, const RestCatalogProperties& config) { + ICEBERG_ASSIGN_OR_RAISE(auto config_endpoint, paths.Config()); + HttpClient client(config.ExtractHeaders()); + ICEBERG_ASSIGN_OR_RAISE(const auto response, + client.Get(config_endpoint, /*params=*/{}, /*headers=*/{}, + *DefaultErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); - // Merge server config into client config, server config overrides > client config - // properties > server config defaults - auto final_props = - MergeConfigs(server_config.defaults, config.configs(), server_config.overrides); - return RestCatalogProperties::FromMap(final_props); + // Merge priorities: server overrides > client config > server defaults + return RestCatalogProperties::FromMap( + MergeConfigs(server_config.overrides, config.configs(), server_config.defaults)); } +} // namespace + +RestCatalog::~RestCatalog() = default; + Result> RestCatalog::Make( const RestCatalogProperties& config) { - // Get URI and prefix from config ICEBERG_ASSIGN_OR_RAISE(auto uri, config.Uri()); - std::string base_uri{TrimTrailingSlash(uri)}; - std::string prefix = config.Get(RestCatalogProperties::kPrefix); - ResourcePaths paths(base_uri, prefix); - - // Fetch and merge server configuration - ICEBERG_ASSIGN_OR_RAISE(auto final_config, FetchAndMergeConfig(config, paths)); + ICEBERG_ASSIGN_OR_RAISE( + auto paths, ResourcePaths::Make(std::string(TrimTrailingSlash(uri)), + config.Get(RestCatalogProperties::kPrefix))); + ICEBERG_ASSIGN_OR_RAISE(auto final_config, FetchConfig(*paths, config)); + // Update resource paths based on the final config ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config->Uri()); - std::string final_base_uri{TrimTrailingSlash(final_uri)}; + paths->SetBaseUri(std::string(TrimTrailingSlash(final_uri))); + return std::unique_ptr( - new RestCatalog(std::move(final_config), std::move(final_base_uri))); + new RestCatalog(std::move(final_config), std::move(paths))); } RestCatalog::RestCatalog(std::unique_ptr config, - std::string base_uri) + std::unique_ptr paths) : config_(std::move(config)), - client_(std::make_shared(config_->ExtractHeaders())), - paths_(std::move(base_uri), config_->Get(RestCatalogProperties::kPrefix)), + client_(std::make_unique(config_->ExtractHeaders())), + paths_(std::move(paths)), name_(config_->Get(RestCatalogProperties::kName)) {} std::string_view RestCatalog::name() const { return name_; } Result> RestCatalog::ListNamespaces(const Namespace& ns) const { - const std::string endpoint = paths_.Namespaces(); + ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->Namespaces()); std::vector result; std::string next_token; while (true) { std::unordered_map params; if (!ns.levels.empty()) { - params[kQueryParamParent] = EncodeNamespaceForUrl(ns); + ICEBERG_ASSIGN_OR_RAISE(params[kQueryParamParent], EncodeNamespace(ns)); } if (!next_token.empty()) { params[kQueryParamPageToken] = next_token; } - ICEBERG_ASSIGN_OR_RAISE( - const auto& response, - client_->Get(endpoint, params, {}, *NamespaceErrorHandler::Instance())); + ICEBERG_ASSIGN_OR_RAISE(const auto& response, + client_->Get(endpoint, params, /*headers=*/{}, + *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json)); result.insert(result.end(), list_response.namespaces.begin(), diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h index 29f62b220..84ab2b9c8 100644 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -23,10 +23,8 @@ #include #include "iceberg/catalog.h" -#include "iceberg/catalog/rest/catalog_properties.h" -#include "iceberg/catalog/rest/http_client.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/catalog/rest/resource_paths.h" +#include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" /// \file iceberg/catalog/rest/rest_catalog.h @@ -37,6 +35,8 @@ namespace iceberg::rest { /// \brief Rest catalog implementation. class ICEBERG_REST_EXPORT RestCatalog : public Catalog { public: + ~RestCatalog() override; + RestCatalog(const RestCatalog&) = delete; RestCatalog& operator=(const RestCatalog&) = delete; RestCatalog(RestCatalog&&) = delete; @@ -100,19 +100,12 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog { const TableIdentifier& identifier, const Schema& schema) const override; private: - RestCatalog(std::unique_ptr config, std::string base_uri); - - /// \brief Fetch server configuration and merge with client config - /// - /// \param config the initial client configuration - /// \param paths the resource paths for REST endpoints - /// \return the final merged configuration - static Result> FetchAndMergeConfig( - const RestCatalogProperties& config, const ResourcePaths& paths); + RestCatalog(std::unique_ptr config, + std::unique_ptr paths); std::unique_ptr config_; - std::shared_ptr client_; - ResourcePaths paths_; + std::unique_ptr client_; + std::unique_ptr paths_; std::string name_; }; diff --git a/src/iceberg/catalog/rest/rest_util.cc b/src/iceberg/catalog/rest/rest_util.cc index 9029410ce..51447ad8a 100644 --- a/src/iceberg/catalog/rest/rest_util.cc +++ b/src/iceberg/catalog/rest/rest_util.cc @@ -19,14 +19,16 @@ #include "iceberg/catalog/rest/rest_util.h" -#include -#include -#include - #include +#include "iceberg/table_identifier.h" +#include "iceberg/util/macros.h" namespace iceberg::rest { +namespace { +const std::string kNamespaceEscapeSeparator = "%1F"; +} + std::string_view TrimTrailingSlash(std::string_view str) { while (!str.empty() && str.back() == '/') { str.remove_suffix(1); @@ -34,50 +36,71 @@ std::string_view TrimTrailingSlash(std::string_view str) { return str; } -std::string EncodeString(std::string_view toEncode) { +Result EncodeString(std::string_view str_to_encode) { + if (str_to_encode.empty()) { + return ""; + } + // Use CPR's urlEncode which internally calls libcurl's curl_easy_escape() - cpr::util::SecureString encoded = cpr::util::urlEncode(toEncode); - return {encoded.data(), encoded.size()}; + cpr::util::SecureString encoded = cpr::util::urlEncode(str_to_encode); + if (encoded.empty()) { + return InvalidArgument("Failed to encode string '{}'", str_to_encode); + } + + return std::string{encoded.data(), encoded.size()}; } -std::string DecodeString(std::string_view encoded) { +Result DecodeString(std::string_view str_to_decode) { + if (str_to_decode.empty()) { + return ""; + } + // Use CPR's urlDecode which internally calls libcurl's curl_easy_unescape() - cpr::util::SecureString decoded = cpr::util::urlDecode(encoded); - return {decoded.data(), decoded.size()}; + cpr::util::SecureString decoded = cpr::util::urlDecode(str_to_decode); + if (decoded.empty()) { + return InvalidArgument("Failed to decode string '{}'", str_to_decode); + } + + return std::string{decoded.data(), decoded.size()}; } -std::string EncodeNamespaceForUrl(const Namespace& ns) { - if (ns.levels.empty()) { +Result EncodeNamespace(const Namespace& ns_to_encode) { + if (ns_to_encode.levels.empty()) { return ""; } - std::string result = EncodeString(ns.levels.front()); - // Join encoded levels with "%1F" - for (size_t i = 1; i < ns.levels.size(); ++i) { - result.append("%1F"); - result.append(EncodeString(ns.levels[i])); + ICEBERG_ASSIGN_OR_RAISE(std::string result, EncodeString(ns_to_encode.levels.front())); + + for (size_t i = 1; i < ns_to_encode.levels.size(); ++i) { + ICEBERG_ASSIGN_OR_RAISE(std::string encoded_level, + EncodeString(ns_to_encode.levels[i])); + result.append(kNamespaceEscapeSeparator); + result.append(std::move(encoded_level)); } return result; } -Namespace DecodeNamespaceFromUrl(std::string_view encoded) { - if (encoded.empty()) { +Result DecodeNamespace(std::string_view str_to_decode) { + if (str_to_decode.empty()) { return Namespace{.levels = {}}; } - // Split by "%1F" first - Namespace ns; + Namespace ns{}; std::string::size_type start = 0; - std::string::size_type end = encoded.find("%1F"); + std::string::size_type end = str_to_decode.find(kNamespaceEscapeSeparator); while (end != std::string::npos) { - ns.levels.push_back(DecodeString(encoded.substr(start, end - start))); - // Skip the 3-character "%1F" separator - start = end + 3; - end = encoded.find("%1F", start); + ICEBERG_ASSIGN_OR_RAISE(std::string decoded_level, + DecodeString(str_to_decode.substr(start, end - start))); + ns.levels.push_back(std::move(decoded_level)); + start = end + kNamespaceEscapeSeparator.size(); + end = str_to_decode.find(kNamespaceEscapeSeparator, start); } - ns.levels.push_back(DecodeString(encoded.substr(start))); + + ICEBERG_ASSIGN_OR_RAISE(std::string decoded_level, + DecodeString(str_to_decode.substr(start))); + ns.levels.push_back(std::move(decoded_level)); return ns; } diff --git a/src/iceberg/catalog/rest/rest_util.h b/src/iceberg/catalog/rest/rest_util.h index 4b010d6b6..71e81a30d 100644 --- a/src/iceberg/catalog/rest/rest_util.h +++ b/src/iceberg/catalog/rest/rest_util.h @@ -24,50 +24,53 @@ #include #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/table_identifier.h" +#include "iceberg/catalog/rest/type_fwd.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" namespace iceberg::rest { -/// \brief Trim trailing slashes from a URI string. -/// \details Removes all trailing '/' characters from the URI. +/// \brief Trim trailing slashes from a string. +/// /// \param str A string to trim. -/// \return The trimmed URI string with all trailing slashes removed. +/// \return The trimmed string with all trailing slashes removed. ICEBERG_REST_EXPORT std::string_view TrimTrailingSlash(std::string_view str); /// \brief URL-encode a string (RFC 3986). +/// /// \details This implementation uses libcurl (via CPR), which follows RFC 3986 strictly: /// - Unreserved characters: [A-Z], [a-z], [0-9], "-", "_", ".", "~" /// - Space is encoded as "%20" (unlike Java's URLEncoder which uses "+"). /// - All other characters are percent-encoded (%XX). -/// \param toEncode The string to encode. -/// \return The URL-encoded string. -/// \note Iceberg's Java client uses `java.net.URLEncoder`, which follows the -/// `application/x-www-form-urlencoded` standard (HTML Forms). We strictly adhere to RFC -/// 3986 to ensure correct URI path semantics (encoding spaces as %20 rather than +) for -/// maximum cross-platform interoperability. -ICEBERG_REST_EXPORT std::string EncodeString(std::string_view toEncode); +/// \param str_to_encode The string to encode. +/// \return The URL-encoded string or InvalidArgument if the string is invalid. +ICEBERG_REST_EXPORT Result EncodeString(std::string_view str_to_encode); /// \brief URL-decode a string. +/// /// \details Decodes percent-encoded characters (e.g., "%20" -> space). Uses libcurl's URL /// decoding via the CPR library. -/// \param encoded The encoded string to decode. -/// \return The decoded string. -ICEBERG_REST_EXPORT std::string DecodeString(std::string_view encoded); +/// \param str_to_decode The encoded string to decode. +/// \return The decoded string or InvalidArgument if the string is invalid. +ICEBERG_REST_EXPORT Result DecodeString(std::string_view str_to_decode); /// \brief Encode a Namespace into a URL-safe component. +/// /// \details Encodes each level separately using EncodeString, then joins them with "%1F". -/// \param ns The namespace (sequence of path-like levels) to encode. +/// \param ns_to_encode The namespace to encode. /// \return The percent-encoded namespace string suitable for URLs. -ICEBERG_REST_EXPORT std::string EncodeNamespaceForUrl(const Namespace& ns); +ICEBERG_REST_EXPORT Result EncodeNamespace(const Namespace& ns_to_encode); /// \brief Decode a URL-encoded namespace string back to a Namespace. +/// /// \details Splits by "%1F" (the URL-encoded form of ASCII Unit Separator), then decodes /// each level separately using DecodeString. -/// \param encoded The percent-encoded namespace string. +/// \param str_to_decode The percent-encoded namespace string. /// \return The decoded Namespace. -ICEBERG_REST_EXPORT Namespace DecodeNamespaceFromUrl(std::string_view encoded); +ICEBERG_REST_EXPORT Result DecodeNamespace(std::string_view str_to_decode); /// \brief Merge catalog configuration properties. +/// /// \details Merges three sets of configuration properties following the precedence order: /// server overrides > client configs > server defaults. /// \param server_defaults Default properties provided by the server. diff --git a/src/iceberg/catalog/rest/type_fwd.h b/src/iceberg/catalog/rest/type_fwd.h new file mode 100644 index 000000000..082630f2d --- /dev/null +++ b/src/iceberg/catalog/rest/type_fwd.h @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/catalog/rest/type_fwd.h +/// Forward declarations and enum definitions for Iceberg REST API types. + +namespace iceberg::rest { + +struct ErrorModel; + +class ErrorHandler; +class HttpClient; +class ResourcePaths; +class RestCatalog; +class RestCatalogProperties; + +} // namespace iceberg::rest diff --git a/src/iceberg/table_identifier.h b/src/iceberg/table_identifier.h index b145e75f3..bef9b81dd 100644 --- a/src/iceberg/table_identifier.h +++ b/src/iceberg/table_identifier.h @@ -33,6 +33,8 @@ namespace iceberg { /// \brief A namespace in a catalog. struct ICEBERG_EXPORT Namespace { std::vector levels; + + bool operator==(const Namespace& other) const { return levels == other.levels; } }; /// \brief Identifies a table in iceberg catalog. @@ -40,6 +42,10 @@ struct ICEBERG_EXPORT TableIdentifier { Namespace ns; std::string name; + bool operator==(const TableIdentifier& other) const { + return ns == other.ns && name == other.name; + } + /// \brief Validates the TableIdentifier. Status Validate() const { if (name.empty()) { diff --git a/src/iceberg/test/rest_util_test.cc b/src/iceberg/test/rest_util_test.cc index 5dfed419a..b95a220bb 100644 --- a/src/iceberg/test/rest_util_test.cc +++ b/src/iceberg/test/rest_util_test.cc @@ -22,6 +22,7 @@ #include #include "iceberg/table_identifier.h" +#include "iceberg/test/matchers.h" namespace iceberg::rest { @@ -33,69 +34,77 @@ TEST(RestUtilTest, TrimTrailingSlash) { TEST(RestUtilTest, RoundTripUrlEncodeDecodeNamespace) { // {"dogs"} - EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs"}}), "dogs"); - EXPECT_EQ(DecodeNamespaceFromUrl("dogs").levels, std::vector{"dogs"}); + EXPECT_THAT(EncodeNamespace(Namespace{.levels = {"dogs"}}), + HasValue(::testing::Eq("dogs"))); + EXPECT_THAT(DecodeNamespace("dogs"), + HasValue(::testing::Eq(Namespace{.levels = {"dogs"}}))); // {"dogs.named.hank"} - EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs.named.hank"}}), - "dogs.named.hank"); - EXPECT_EQ(DecodeNamespaceFromUrl("dogs.named.hank").levels, - std::vector{"dogs.named.hank"}); + EXPECT_THAT(EncodeNamespace(Namespace{.levels = {"dogs.named.hank"}}), + HasValue(::testing::Eq("dogs.named.hank"))); + EXPECT_THAT(DecodeNamespace("dogs.named.hank"), + HasValue(::testing::Eq(Namespace{.levels = {"dogs.named.hank"}}))); // {"dogs/named/hank"} - EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs/named/hank"}}), - "dogs%2Fnamed%2Fhank"); - EXPECT_EQ(DecodeNamespaceFromUrl("dogs%2Fnamed%2Fhank").levels, - std::vector{"dogs/named/hank"}); + EXPECT_THAT(EncodeNamespace(Namespace{.levels = {"dogs/named/hank"}}), + HasValue(::testing::Eq("dogs%2Fnamed%2Fhank"))); + EXPECT_THAT(DecodeNamespace("dogs%2Fnamed%2Fhank"), + HasValue(::testing::Eq(Namespace{.levels = {"dogs/named/hank"}}))); // {"dogs", "named", "hank"} - EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {"dogs", "named", "hank"}}), - "dogs%1Fnamed%1Fhank"); - EXPECT_EQ(DecodeNamespaceFromUrl("dogs%1Fnamed%1Fhank").levels, - (std::vector{"dogs", "named", "hank"})); + EXPECT_THAT(EncodeNamespace(Namespace{.levels = {"dogs", "named", "hank"}}), + HasValue(::testing::Eq("dogs%1Fnamed%1Fhank"))); + EXPECT_THAT(DecodeNamespace("dogs%1Fnamed%1Fhank"), + HasValue(::testing::Eq(Namespace{.levels = {"dogs", "named", "hank"}}))); // {"dogs.and.cats", "named", "hank.or.james-westfall"} - EXPECT_EQ(EncodeNamespaceForUrl(Namespace{ - .levels = {"dogs.and.cats", "named", "hank.or.james-westfall"}}), - "dogs.and.cats%1Fnamed%1Fhank.or.james-westfall"); - EXPECT_EQ( - DecodeNamespaceFromUrl("dogs.and.cats%1Fnamed%1Fhank.or.james-westfall").levels, - (std::vector{"dogs.and.cats", "named", "hank.or.james-westfall"})); + EXPECT_THAT(EncodeNamespace(Namespace{ + .levels = {"dogs.and.cats", "named", "hank.or.james-westfall"}}), + HasValue(::testing::Eq("dogs.and.cats%1Fnamed%1Fhank.or.james-westfall"))); + EXPECT_THAT(DecodeNamespace("dogs.and.cats%1Fnamed%1Fhank.or.james-westfall"), + HasValue(::testing::Eq(Namespace{ + .levels = {"dogs.and.cats", "named", "hank.or.james-westfall"}}))); // empty namespace - EXPECT_EQ(EncodeNamespaceForUrl(Namespace{.levels = {}}), ""); - EXPECT_EQ(DecodeNamespaceFromUrl("").levels, std::vector{}); + EXPECT_THAT(EncodeNamespace(Namespace{.levels = {}}), HasValue(::testing::Eq(""))); + EXPECT_THAT(DecodeNamespace(""), HasValue(::testing::Eq(Namespace{.levels = {}}))); } TEST(RestUtilTest, EncodeString) { // RFC 3986 unreserved characters should not be encoded - EXPECT_EQ(EncodeString("abc123XYZ"), "abc123XYZ"); - EXPECT_EQ(EncodeString("test-file_name.txt~backup"), "test-file_name.txt~backup"); + EXPECT_THAT(EncodeString("abc123XYZ"), HasValue(::testing::Eq("abc123XYZ"))); + EXPECT_THAT(EncodeString("test-file_name.txt~backup"), + HasValue(::testing::Eq("test-file_name.txt~backup"))); // Spaces and special characters should be encoded - EXPECT_EQ(EncodeString("hello world"), "hello%20world"); - EXPECT_EQ(EncodeString("test@example.com"), "test%40example.com"); - EXPECT_EQ(EncodeString("path/to/file"), "path%2Fto%2Ffile"); - EXPECT_EQ(EncodeString("key=value&foo=bar"), "key%3Dvalue%26foo%3Dbar"); - EXPECT_EQ(EncodeString("100%"), "100%25"); - EXPECT_EQ(EncodeString("hello\x1Fworld"), "hello%1Fworld"); - EXPECT_EQ(EncodeString(""), ""); + EXPECT_THAT(EncodeString("hello world"), HasValue(::testing::Eq("hello%20world"))); + EXPECT_THAT(EncodeString("test@example.com"), + HasValue(::testing::Eq("test%40example.com"))); + EXPECT_THAT(EncodeString("path/to/file"), HasValue(::testing::Eq("path%2Fto%2Ffile"))); + EXPECT_THAT(EncodeString("key=value&foo=bar"), + HasValue(::testing::Eq("key%3Dvalue%26foo%3Dbar"))); + EXPECT_THAT(EncodeString("100%"), HasValue(::testing::Eq("100%25"))); + EXPECT_THAT(EncodeString("hello\x1Fworld"), HasValue(::testing::Eq("hello%1Fworld"))); + EXPECT_THAT(EncodeString(""), HasValue(::testing::Eq(""))); } TEST(RestUtilTest, DecodeString) { // Decode percent-encoded strings - EXPECT_EQ(DecodeString("hello%20world"), "hello world"); - EXPECT_EQ(DecodeString("test%40example.com"), "test@example.com"); - EXPECT_EQ(DecodeString("path%2Fto%2Ffile"), "path/to/file"); - EXPECT_EQ(DecodeString("key%3Dvalue%26foo%3Dbar"), "key=value&foo=bar"); - EXPECT_EQ(DecodeString("100%25"), "100%"); + EXPECT_THAT(DecodeString("hello%20world"), HasValue(::testing::Eq("hello world"))); + EXPECT_THAT(DecodeString("test%40example.com"), + HasValue(::testing::Eq("test@example.com"))); + EXPECT_THAT(DecodeString("path%2Fto%2Ffile"), HasValue(::testing::Eq("path/to/file"))); + EXPECT_THAT(DecodeString("key%3Dvalue%26foo%3Dbar"), + HasValue(::testing::Eq("key=value&foo=bar"))); + EXPECT_THAT(DecodeString("100%25"), HasValue(::testing::Eq("100%"))); // ASCII Unit Separator (0x1F) - EXPECT_EQ(DecodeString("hello%1Fworld"), "hello\x1Fworld"); + EXPECT_THAT(DecodeString("hello%1Fworld"), HasValue(::testing::Eq("hello\x1Fworld"))); // Unreserved characters remain unchanged - EXPECT_EQ(DecodeString("test-file_name.txt~backup"), "test-file_name.txt~backup"); - EXPECT_EQ(DecodeString(""), ""); + EXPECT_THAT(DecodeString("test-file_name.txt~backup"), + HasValue(::testing::Eq("test-file_name.txt~backup"))); + EXPECT_THAT(DecodeString(""), HasValue(::testing::Eq(""))); } TEST(RestUtilTest, EncodeDecodeStringRoundTrip) { @@ -110,8 +119,8 @@ TEST(RestUtilTest, EncodeDecodeStringRoundTrip) { ""}; for (const auto& test : test_cases) { - std::string encoded = EncodeString(test); - std::string decoded = DecodeString(encoded); + ICEBERG_UNWRAP_OR_FAIL(std::string encoded, EncodeString(test)); + ICEBERG_UNWRAP_OR_FAIL(std::string decoded, DecodeString(encoded)); EXPECT_EQ(decoded, test) << "Round-trip failed for: " << test; } } @@ -121,14 +130,14 @@ TEST(RestUtilTest, MergeConfigs) { {"default1", "value1"}, {"default2", "value2"}, {"common", "default_value"}}; std::unordered_map client_configs = { - {"client1", "value1"}, {"common", "client_value"}}; + {"client1", "value1"}, {"common", "client_value"}, {"extra", "client_value"}}; std::unordered_map server_overrides = { {"override1", "value1"}, {"common", "override_value"}}; auto merged = MergeConfigs(server_defaults, client_configs, server_overrides); - EXPECT_EQ(merged.size(), 5); + EXPECT_EQ(merged.size(), 6); // Check precedence: server_overrides > client_configs > server_defaults EXPECT_EQ(merged["default1"], "value1"); @@ -136,6 +145,7 @@ TEST(RestUtilTest, MergeConfigs) { EXPECT_EQ(merged["client1"], "value1"); EXPECT_EQ(merged["override1"], "value1"); EXPECT_EQ(merged["common"], "override_value"); + EXPECT_EQ(merged["extra"], "client_value"); // Test with empty maps auto merged_empty = MergeConfigs({}, {{"key", "value"}}, {}); diff --git a/src/iceberg/util/config.h b/src/iceberg/util/config.h index 7a3a28b40..8fb715534 100644 --- a/src/iceberg/util/config.h +++ b/src/iceberg/util/config.h @@ -112,6 +112,19 @@ class ConfigBase { const std::unordered_map& configs() const { return configs_; } + /// \brief Extracts the prefix from the configuration. + /// \param prefix The prefix to extract. + /// \return A map of entries that match the prefix with prefix removed. + std::unordered_map Extract(std::string_view prefix) const { + std::unordered_map extracted; + for (const auto& [key, value] : configs_) { + if (key.starts_with(prefix)) { + extracted[key.substr(prefix.length())] = value; + } + } + return extracted; + } + protected: std::unordered_map configs_; }; From ec69dfa19187ce07b24826bd59741274880435fe Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Wed, 26 Nov 2025 15:01:11 +0800 Subject: [PATCH 11/12] fix windows ci --- src/iceberg/catalog/rest/rest_catalog.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 5afa91f54..dff52e2af 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -72,7 +72,7 @@ Result> RestCatalog::Make( // Update resource paths based on the final config ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config->Uri()); - paths->SetBaseUri(std::string(TrimTrailingSlash(final_uri))); + ICEBERG_RETURN_UNEXPECTED(paths->SetBaseUri(std::string(TrimTrailingSlash(final_uri)))); return std::unique_ptr( new RestCatalog(std::move(final_config), std::move(paths))); From 5f79c97a5744a45cc01772aeb9d63e00f888aed2 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Wed, 26 Nov 2025 17:07:52 +0800 Subject: [PATCH 12/12] 1 --- src/iceberg/catalog/rest/http_client.cc | 4 ++-- src/iceberg/catalog/rest/rest_util.cc | 1 + src/iceberg/catalog/rest/rest_util.h | 1 - src/iceberg/catalog/rest/types.h | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index 60a8eb142..1b026c66e 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -48,10 +48,10 @@ class HttpResponse::Impl { cpr::Response response_; }; -HttpResponse::HttpResponse(HttpResponse&&) noexcept = default; -HttpResponse& HttpResponse::operator=(HttpResponse&&) noexcept = default; HttpResponse::HttpResponse() = default; HttpResponse::~HttpResponse() = default; +HttpResponse::HttpResponse(HttpResponse&&) noexcept = default; +HttpResponse& HttpResponse::operator=(HttpResponse&&) noexcept = default; int32_t HttpResponse::status_code() const { return impl_->status_code(); } diff --git a/src/iceberg/catalog/rest/rest_util.cc b/src/iceberg/catalog/rest/rest_util.cc index 51447ad8a..5a0f166d5 100644 --- a/src/iceberg/catalog/rest/rest_util.cc +++ b/src/iceberg/catalog/rest/rest_util.cc @@ -23,6 +23,7 @@ #include "iceberg/table_identifier.h" #include "iceberg/util/macros.h" + namespace iceberg::rest { namespace { diff --git a/src/iceberg/catalog/rest/rest_util.h b/src/iceberg/catalog/rest/rest_util.h index 71e81a30d..895bb2fb4 100644 --- a/src/iceberg/catalog/rest/rest_util.h +++ b/src/iceberg/catalog/rest/rest_util.h @@ -24,7 +24,6 @@ #include #include "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" diff --git a/src/iceberg/catalog/rest/types.h b/src/iceberg/catalog/rest/types.h index 50cdcc01a..0b589bdfd 100644 --- a/src/iceberg/catalog/rest/types.h +++ b/src/iceberg/catalog/rest/types.h @@ -19,7 +19,6 @@ #pragma once -#include #include #include #include