diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index 881b3d39a..7b36298a6 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -16,12 +16,13 @@ # under the License. set(ICEBERG_REST_SOURCES - rest_catalog.cc catalog_properties.cc + endpoint.cc error_handlers.cc http_client.cc json_internal.cc resource_paths.cc + rest_catalog.cc rest_util.cc) set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/rest/endpoint.cc b/src/iceberg/catalog/rest/endpoint.cc new file mode 100644 index 000000000..bf457c879 --- /dev/null +++ b/src/iceberg/catalog/rest/endpoint.cc @@ -0,0 +1,90 @@ +/* + * 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/endpoint.h" + +#include +#include + +namespace iceberg::rest { + +constexpr std::string_view ToString(HttpMethod method) { + switch (method) { + case HttpMethod::kGet: + return "GET"; + case HttpMethod::kPost: + return "POST"; + case HttpMethod::kPut: + return "PUT"; + case HttpMethod::kDelete: + return "DELETE"; + case HttpMethod::kHead: + return "HEAD"; + } + return "UNKNOWN"; +} + +Result Endpoint::Make(HttpMethod method, std::string_view path) { + if (path.empty()) { + return InvalidArgument("Endpoint cannot have empty path"); + } + return Endpoint(method, path); +} + +Result Endpoint::FromString(std::string_view str) { + auto space_pos = str.find(' '); + if (space_pos == std::string_view::npos || + str.find(' ', space_pos + 1) != std::string_view::npos) { + return InvalidArgument( + "Invalid endpoint format (must consist of two elements separated by a single " + "space): '{}'", + str); + } + + auto method_str = str.substr(0, space_pos); + auto path_str = str.substr(space_pos + 1); + + if (path_str.empty()) { + return InvalidArgument("Invalid endpoint format: path is empty"); + } + + // Parse HTTP method + HttpMethod method; + if (method_str == "GET") { + method = HttpMethod::kGet; + } else if (method_str == "POST") { + method = HttpMethod::kPost; + } else if (method_str == "PUT") { + method = HttpMethod::kPut; + } else if (method_str == "DELETE") { + method = HttpMethod::kDelete; + } else if (method_str == "HEAD") { + method = HttpMethod::kHead; + } else { + return InvalidArgument("Invalid HTTP method: '{}'", method_str); + } + + return Make(method, std::string(path_str)); +} + +std::string Endpoint::ToString() const { + return std::format("{} {}", rest::ToString(method_), path_); +} + +} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/endpoint.h b/src/iceberg/catalog/rest/endpoint.h new file mode 100644 index 000000000..7382955ce --- /dev/null +++ b/src/iceberg/catalog/rest/endpoint.h @@ -0,0 +1,150 @@ +/* + * 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/result.h" + +/// \file iceberg/catalog/rest/endpoint.h +/// Endpoint definitions for Iceberg REST API operations. + +namespace iceberg::rest { + +/// \brief HTTP method enumeration. +enum class HttpMethod : uint8_t { kGet, kPost, kPut, kDelete, kHead }; + +/// \brief Convert HttpMethod to string representation. +constexpr std::string_view ToString(HttpMethod method); + +/// \brief An Endpoint is an immutable value object identifying a specific REST API +/// operation. It consists of: +/// - HTTP method (GET, POST, DELETE, etc.) +/// - Path template (e.g., "/v1/{prefix}/namespaces/{namespace}") +class ICEBERG_REST_EXPORT Endpoint { + public: + /// \brief Make an endpoint with method and path template. + /// + /// \param method HTTP method (GET, POST, etc.) + /// \param path Path template with placeholders (e.g., "/v1/{prefix}/tables") + /// \return Endpoint instance or error if invalid + static Result Make(HttpMethod method, std::string_view path); + + /// \brief Parse endpoint from string representation. "METHOD" have to be all + /// upper-cased. + /// + /// \param str String in format "METHOD /path/template" (e.g., "GET /v1/namespaces") + /// \return Endpoint instance or error if malformed. + static Result FromString(std::string_view str); + + /// \brief Get the HTTP method. + constexpr HttpMethod method() const { return method_; } + + /// \brief Get the path template. + std::string_view path() const { return path_; } + + /// \brief Serialize to "METHOD /path" format. + std::string ToString() const; + + constexpr bool operator==(const Endpoint& other) const { + return method_ == other.method_ && path_ == other.path_; + } + + // Namespace endpoints + static Endpoint ListNamespaces() { + return {HttpMethod::kGet, "/v1/{prefix}/namespaces"}; + } + static Endpoint GetNamespaceProperties() { + return {HttpMethod::kGet, "/v1/{prefix}/namespaces/{namespace}"}; + } + static Endpoint NamespaceExists() { + return {HttpMethod::kHead, "/v1/{prefix}/namespaces/{namespace}"}; + } + static Endpoint CreateNamespace() { + return {HttpMethod::kPost, "/v1/{prefix}/namespaces"}; + } + static Endpoint UpdateNamespace() { + return {HttpMethod::kPost, "/v1/{prefix}/namespaces/{namespace}/properties"}; + } + static Endpoint DropNamespace() { + return {HttpMethod::kDelete, "/v1/{prefix}/namespaces/{namespace}"}; + } + + // Table endpoints + static Endpoint ListTables() { + return {HttpMethod::kGet, "/v1/{prefix}/namespaces/{namespace}/tables"}; + } + static Endpoint LoadTable() { + return {HttpMethod::kGet, "/v1/{prefix}/namespaces/{namespace}/tables/{table}"}; + } + static Endpoint TableExists() { + return {HttpMethod::kHead, "/v1/{prefix}/namespaces/{namespace}/tables/{table}"}; + } + static Endpoint CreateTable() { + return {HttpMethod::kPost, "/v1/{prefix}/namespaces/{namespace}/tables"}; + } + static Endpoint UpdateTable() { + return {HttpMethod::kPost, "/v1/{prefix}/namespaces/{namespace}/tables/{table}"}; + } + static Endpoint DeleteTable() { + return {HttpMethod::kDelete, "/v1/{prefix}/namespaces/{namespace}/tables/{table}"}; + } + static Endpoint RenameTable() { + return {HttpMethod::kPost, "/v1/{prefix}/tables/rename"}; + } + static Endpoint RegisterTable() { + return {HttpMethod::kPost, "/v1/{prefix}/namespaces/{namespace}/register"}; + } + static Endpoint ReportMetrics() { + return {HttpMethod::kPost, + "/v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics"}; + } + static Endpoint TableCredentials() { + return {HttpMethod::kGet, + "/v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials"}; + } + + // Transaction endpoints + static Endpoint CommitTransaction() { + return {HttpMethod::kPost, "/v1/{prefix}/transactions/commit"}; + } + + private: + Endpoint(HttpMethod method, std::string_view path) : method_(method), path_(path) {} + + HttpMethod method_; + std::string path_; +}; + +} // namespace iceberg::rest + +// Specialize std::hash for Endpoint +namespace std { +template <> +struct hash { + std::size_t operator()(const iceberg::rest::Endpoint& endpoint) const noexcept { + std::size_t h1 = std::hash{}(static_cast(endpoint.method())); + std::size_t h2 = std::hash{}(endpoint.path()); + return h1 ^ (h2 + 0x9e3779b9 + (h1 << 6) + (h1 >> 2)); + } +}; +} // namespace std diff --git a/src/iceberg/catalog/rest/json_internal.cc b/src/iceberg/catalog/rest/json_internal.cc index c60b406d9..66e690258 100644 --- a/src/iceberg/catalog/rest/json_internal.cc +++ b/src/iceberg/catalog/rest/json_internal.cc @@ -73,7 +73,9 @@ nlohmann::json ToJson(const CatalogConfig& config) { nlohmann::json json; json[kOverrides] = config.overrides; json[kDefaults] = config.defaults; - SetContainerField(json, kEndpoints, config.endpoints); + for (const auto& endpoint : config.endpoints) { + json[kEndpoints].emplace_back(endpoint.ToString()); + } return json; } @@ -85,8 +87,16 @@ Result CatalogConfigFromJson(const nlohmann::json& json) { ICEBERG_ASSIGN_OR_RAISE( config.defaults, GetJsonValueOrDefault(json, kDefaults)); ICEBERG_ASSIGN_OR_RAISE( - config.endpoints, - GetJsonValueOrDefault>(json, kEndpoints)); + auto endpoints, GetJsonValueOrDefault>(json, kEndpoints)); + config.endpoints.reserve(endpoints.size()); + for (const auto& endpoint_str : endpoints) { + auto endpoint_result = Endpoint::FromString(endpoint_str); + if (!endpoint_result.has_value()) { + // Convert to JsonParseError in JSON deserialization context + return JsonParseError("{}", endpoint_result.error().message); + } + config.endpoints.emplace_back(std::move(endpoint_result.value())); + } ICEBERG_RETURN_UNEXPECTED(config.Validate()); return config; } diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index 8378b2a8c..cda90c616 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -17,6 +17,7 @@ iceberg_rest_sources = files( 'catalog_properties.cc', + 'endpoint.cc', 'error_handlers.cc', 'http_client.cc', 'json_internal.cc', @@ -58,6 +59,7 @@ install_headers( [ 'catalog_properties.h', 'constant.h', + 'endpoint.h', 'error_handlers.h', 'http_client.h', 'iceberg_rest_export.h', diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 4a77f6585..b9dfaafc7 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -21,18 +21,21 @@ #include #include +#include #include #include #include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint.h" #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/partition_spec.h" #include "iceberg/result.h" @@ -44,20 +47,30 @@ namespace iceberg::rest { 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()); +/// \brief Get the default set of endpoints for backwards compatibility according to the +/// iceberg rest spec. +std::unordered_set GetDefaultEndpoints() { + return { + Endpoint::ListNamespaces(), Endpoint::GetNamespaceProperties(), + Endpoint::CreateNamespace(), Endpoint::UpdateNamespace(), + Endpoint::DropNamespace(), Endpoint::ListTables(), + Endpoint::LoadTable(), Endpoint::CreateTable(), + Endpoint::UpdateTable(), Endpoint::DeleteTable(), + Endpoint::RenameTable(), Endpoint::RegisterTable(), + Endpoint::ReportMetrics(), Endpoint::CommitTransaction(), + }; +} + +/// \brief Fetch server config and merge it with client config +Result FetchServerConfig(const ResourcePaths& paths, + const RestCatalogProperties& current_config) { + ICEBERG_ASSIGN_OR_RAISE(auto config_path, paths.Config()); + HttpClient client(current_config.ExtractHeaders()); ICEBERG_ASSIGN_OR_RAISE(const auto response, - client.Get(config_endpoint, /*params=*/{}, /*headers=*/{}, + client.Get(config_path, /*params=*/{}, /*headers=*/{}, *DefaultErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); - ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); - - // Merge priorities: server overrides > client config > server defaults - return RestCatalogProperties::FromMap( - MergeConfigs(server_config.overrides, config.configs(), server_config.defaults)); + return CatalogConfigFromJson(json); } } // namespace @@ -70,27 +83,46 @@ Result> RestCatalog::Make( 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)); + ICEBERG_ASSIGN_OR_RAISE(auto server_config, FetchServerConfig(*paths, config)); + + std::unique_ptr final_config = RestCatalogProperties::FromMap( + MergeConfigs(server_config.overrides, config.configs(), server_config.defaults)); + + std::unordered_set endpoints; + if (!server_config.endpoints.empty()) { + // Endpoints are already parsed during JSON deserialization, just convert to set + endpoints = std::unordered_set(server_config.endpoints.begin(), + server_config.endpoints.end()); + } else { + // If a server does not send the endpoints field, use default set of endpoints + // for backwards compatibility with legacy servers + endpoints = GetDefaultEndpoints(); + } // Update resource paths based on the final config ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config->Uri()); ICEBERG_RETURN_UNEXPECTED(paths->SetBaseUri(std::string(TrimTrailingSlash(final_uri)))); return std::unique_ptr( - new RestCatalog(std::move(final_config), std::move(paths))); + new RestCatalog(std::move(final_config), std::move(paths), std::move(endpoints))); } RestCatalog::RestCatalog(std::unique_ptr config, - std::unique_ptr paths) + std::unique_ptr paths, + std::unordered_set endpoints) : config_(std::move(config)), client_(std::make_unique(config_->ExtractHeaders())), paths_(std::move(paths)), - name_(config_->Get(RestCatalogProperties::kName)) {} + name_(config_->Get(RestCatalogProperties::kName)), + supported_endpoints_(std::move(endpoints)) {} std::string_view RestCatalog::name() const { return name_; } Result> RestCatalog::ListNamespaces(const Namespace& ns) const { - ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->Namespaces()); + ICEBERG_RETURN_UNEXPECTED( + CheckEndpoint(supported_endpoints_, Endpoint::ListNamespaces())); + + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespaces()); std::vector result; std::string next_token; while (true) { @@ -101,9 +133,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, /*headers=*/{}, - *NamespaceErrorHandler::Instance())); + ICEBERG_ASSIGN_OR_RAISE( + const auto response, + client_->Get(path, 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(), @@ -118,11 +150,14 @@ Result> RestCatalog::ListNamespaces(const Namespace& ns) Status RestCatalog::CreateNamespace( const Namespace& ns, const std::unordered_map& properties) { - ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->Namespaces()); + ICEBERG_RETURN_UNEXPECTED( + CheckEndpoint(supported_endpoints_, Endpoint::CreateNamespace())); + + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespaces()); CreateNamespaceRequest request{.namespace_ = ns, .properties = properties}; ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Post(endpoint, json_request, /*headers=*/{}, + client_->Post(path, json_request, /*headers=*/{}, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto create_response, CreateNamespaceResponseFromJson(json)); @@ -131,9 +166,12 @@ Status RestCatalog::CreateNamespace( Result> RestCatalog::GetNamespaceProperties( const Namespace& ns) const { - ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->Namespace_(ns)); + ICEBERG_RETURN_UNEXPECTED( + CheckEndpoint(supported_endpoints_, Endpoint::GetNamespaceProperties())); + + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns)); ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Get(endpoint, /*params=*/{}, /*headers=*/{}, + client_->Get(path, /*params=*/{}, /*headers=*/{}, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto get_response, GetNamespaceResponseFromJson(json)); @@ -141,19 +179,31 @@ Result> RestCatalog::GetNamespacePr } Status RestCatalog::DropNamespace(const Namespace& ns) { - ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->Namespace_(ns)); + ICEBERG_RETURN_UNEXPECTED( + CheckEndpoint(supported_endpoints_, Endpoint::DropNamespace())); + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns)); ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Delete(endpoint, /*headers=*/{}, *DropNamespaceErrorHandler::Instance())); + client_->Delete(path, /*headers=*/{}, *DropNamespaceErrorHandler::Instance())); return {}; } Result RestCatalog::NamespaceExists(const Namespace& ns) const { - ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->Namespace_(ns)); - // TODO(Feiyang Li): checks if the server supports the namespace exists endpoint, if - // not, triggers a fallback mechanism + auto check = CheckEndpoint(supported_endpoints_, Endpoint::NamespaceExists()); + if (!check.has_value()) { + // Fall back to GetNamespaceProperties + auto result = GetNamespaceProperties(ns); + if (!result.has_value() && result.error().kind == ErrorKind::kNoSuchNamespace) { + return false; + } + ICEBERG_RETURN_UNEXPECTED(result); + // GET succeeded, namespace exists + return true; + } + + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns)); auto response_or_error = - client_->Head(endpoint, /*headers=*/{}, *NamespaceErrorHandler::Instance()); + client_->Head(path, /*headers=*/{}, *NamespaceErrorHandler::Instance()); if (!response_or_error.has_value()) { const auto& error = response_or_error.error(); // catch NoSuchNamespaceException/404 and return false @@ -168,13 +218,16 @@ Result RestCatalog::NamespaceExists(const Namespace& ns) const { Status RestCatalog::UpdateNamespaceProperties( const Namespace& ns, const std::unordered_map& updates, const std::unordered_set& removals) { - ICEBERG_ASSIGN_OR_RAISE(auto endpoint, paths_->NamespaceProperties(ns)); + ICEBERG_RETURN_UNEXPECTED( + CheckEndpoint(supported_endpoints_, Endpoint::UpdateNamespace())); + + ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->NamespaceProperties(ns)); UpdateNamespacePropertiesRequest request{ .removals = std::vector(removals.begin(), removals.end()), .updates = updates}; ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Post(endpoint, json_request, /*headers=*/{}, + client_->Post(path, json_request, /*headers=*/{}, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto update_response, diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h index 4e191e86f..c8ddca587 100644 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -20,9 +20,11 @@ #pragma once #include +#include #include #include "iceberg/catalog.h" +#include "iceberg/catalog/rest/endpoint.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" @@ -98,12 +100,14 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog { private: RestCatalog(std::unique_ptr config, - std::unique_ptr paths); + std::unique_ptr paths, + std::unordered_set endpoints); std::unique_ptr config_; std::unique_ptr client_; std::unique_ptr paths_; std::string name_; + std::unordered_set supported_endpoints_; }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_util.cc b/src/iceberg/catalog/rest/rest_util.cc index a1a63fa1c..62b9bfc33 100644 --- a/src/iceberg/catalog/rest/rest_util.cc +++ b/src/iceberg/catalog/rest/rest_util.cc @@ -20,9 +20,11 @@ #include "iceberg/catalog/rest/rest_util.h" #include +#include #include +#include "iceberg/catalog/rest/endpoint.h" #include "iceberg/table_identifier.h" #include "iceberg/util/macros.h" @@ -251,4 +253,12 @@ std::string GetStandardReasonPhrase(int32_t status_code) { } } +Status CheckEndpoint(const std::unordered_set& supported_endpoints, + const Endpoint& endpoint) { + if (!supported_endpoints.contains(endpoint)) { + return NotSupported("Server does not support endpoint: {}", endpoint.ToString()); + } + return {}; +} + } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/rest_util.h b/src/iceberg/catalog/rest/rest_util.h index fde67a842..5734bbc72 100644 --- a/src/iceberg/catalog/rest/rest_util.h +++ b/src/iceberg/catalog/rest/rest_util.h @@ -22,8 +22,11 @@ #include #include #include +#include +#include "iceberg/catalog/rest/endpoint.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -90,4 +93,12 @@ ICEBERG_REST_EXPORT std::unordered_map MergeConfigs( /// Error"). ICEBERG_REST_EXPORT std::string GetStandardReasonPhrase(int32_t status_code); +/// \brief Check whether the given endpoint is in the set of supported endpoints. +/// +/// \param supported_endpoints Set of endpoints advertised by the server +/// \param endpoint Endpoint to validate +/// \return Status::OK if supported, NotSupported error otherwise +ICEBERG_REST_EXPORT Status CheckEndpoint( + const std::unordered_set& supported_endpoints, const Endpoint& endpoint); + } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/type_fwd.h b/src/iceberg/catalog/rest/type_fwd.h index f63984b31..e7fddb91a 100644 --- a/src/iceberg/catalog/rest/type_fwd.h +++ b/src/iceberg/catalog/rest/type_fwd.h @@ -26,6 +26,7 @@ namespace iceberg::rest { struct ErrorResponse; +class Endpoint; class ErrorHandler; class HttpClient; class ResourcePaths; diff --git a/src/iceberg/catalog/rest/types.h b/src/iceberg/catalog/rest/types.h index 7760e1781..afcd65b97 100644 --- a/src/iceberg/catalog/rest/types.h +++ b/src/iceberg/catalog/rest/types.h @@ -24,6 +24,7 @@ #include #include +#include "iceberg/catalog/rest/endpoint.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/result.h" #include "iceberg/table_identifier.h" @@ -39,16 +40,10 @@ namespace iceberg::rest { struct ICEBERG_REST_EXPORT CatalogConfig { std::unordered_map defaults; // required std::unordered_map overrides; // required - std::vector endpoints; + std::vector endpoints; /// \brief Validates the CatalogConfig. - Status Validate() const { - // TODO(Li Feiyang): Add an invalidEndpoint test that validates endpoint format. - // See: - // https://github.com/apache/iceberg/blob/main/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java#L164 - // for reference. - return {}; - } + Status Validate() const { return {}; } }; /// \brief JSON error payload returned in a response with further details on the error. diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index a48567132..c831ce023 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -170,7 +170,10 @@ if(ICEBERG_BUILD_REST) add_test(NAME ${test_name} COMMAND ${test_name}) endfunction() - add_rest_iceberg_test(rest_catalog_test SOURCES rest_json_internal_test.cc + add_rest_iceberg_test(rest_catalog_test + SOURCES + endpoint_test.cc + rest_json_internal_test.cc rest_util_test.cc) if(ICEBERG_BUILD_REST_INTEGRATION_TESTS) diff --git a/src/iceberg/test/endpoint_test.cc b/src/iceberg/test/endpoint_test.cc new file mode 100644 index 000000000..fcdc92a78 --- /dev/null +++ b/src/iceberg/test/endpoint_test.cc @@ -0,0 +1,261 @@ +/* + * 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/endpoint.h" + +#include +#include + +#include "iceberg/test/matchers.h" + +namespace iceberg::rest { + +TEST(EndpointTest, InvalidCreate) { + // Empty path template should fail + auto result = Endpoint::Make(HttpMethod::kGet, ""); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, HasErrorMessage("Endpoint cannot have empty path")); +} + +TEST(EndpointTest, ValidFromString) { + auto result = Endpoint::FromString("GET /path"); + EXPECT_THAT(result, IsOk()); + + auto endpoint = result.value(); + EXPECT_EQ(endpoint.method(), HttpMethod::kGet); + EXPECT_EQ(endpoint.path(), "/path"); +} + +// Test all HTTP methods +TEST(EndpointTest, AllHttpMethods) { + auto get = Endpoint::Make(HttpMethod::kGet, "/path"); + ASSERT_THAT(get, IsOk()); + EXPECT_EQ(get->ToString(), "GET /path"); + + auto post = Endpoint::Make(HttpMethod::kPost, "/path"); + ASSERT_THAT(post, IsOk()); + EXPECT_EQ(post->ToString(), "POST /path"); + + auto put = Endpoint::Make(HttpMethod::kPut, "/path"); + ASSERT_THAT(put, IsOk()); + EXPECT_EQ(put->ToString(), "PUT /path"); + + auto del = Endpoint::Make(HttpMethod::kDelete, "/path"); + ASSERT_THAT(del, IsOk()); + EXPECT_EQ(del->ToString(), "DELETE /path"); + + auto head = Endpoint::Make(HttpMethod::kHead, "/path"); + ASSERT_THAT(head, IsOk()); + EXPECT_EQ(head->ToString(), "HEAD /path"); +} + +// Test predefined namespace endpoints +TEST(EndpointTest, NamespaceEndpoints) { + auto list_namespaces = Endpoint::ListNamespaces(); + EXPECT_EQ(list_namespaces.method(), HttpMethod::kGet); + EXPECT_EQ(list_namespaces.path(), "/v1/{prefix}/namespaces"); + EXPECT_EQ(list_namespaces.ToString(), "GET /v1/{prefix}/namespaces"); + + auto get_namespace = Endpoint::GetNamespaceProperties(); + EXPECT_EQ(get_namespace.method(), HttpMethod::kGet); + EXPECT_EQ(get_namespace.path(), "/v1/{prefix}/namespaces/{namespace}"); + + auto namespace_exists = Endpoint::NamespaceExists(); + EXPECT_EQ(namespace_exists.method(), HttpMethod::kHead); + EXPECT_EQ(namespace_exists.path(), "/v1/{prefix}/namespaces/{namespace}"); + + auto create_namespace = Endpoint::CreateNamespace(); + EXPECT_EQ(create_namespace.method(), HttpMethod::kPost); + EXPECT_EQ(create_namespace.path(), "/v1/{prefix}/namespaces"); + + auto update_namespace = Endpoint::UpdateNamespace(); + EXPECT_EQ(update_namespace.method(), HttpMethod::kPost); + EXPECT_EQ(update_namespace.path(), "/v1/{prefix}/namespaces/{namespace}/properties"); + + auto drop_namespace = Endpoint::DropNamespace(); + EXPECT_EQ(drop_namespace.method(), HttpMethod::kDelete); + EXPECT_EQ(drop_namespace.path(), "/v1/{prefix}/namespaces/{namespace}"); +} + +// Test predefined table endpoints +TEST(EndpointTest, TableEndpoints) { + auto list_tables = Endpoint::ListTables(); + EXPECT_EQ(list_tables.method(), HttpMethod::kGet); + EXPECT_EQ(list_tables.path(), "/v1/{prefix}/namespaces/{namespace}/tables"); + + auto load_table = Endpoint::LoadTable(); + EXPECT_EQ(load_table.method(), HttpMethod::kGet); + EXPECT_EQ(load_table.path(), "/v1/{prefix}/namespaces/{namespace}/tables/{table}"); + + auto table_exists = Endpoint::TableExists(); + EXPECT_EQ(table_exists.method(), HttpMethod::kHead); + EXPECT_EQ(table_exists.path(), "/v1/{prefix}/namespaces/{namespace}/tables/{table}"); + + auto create_table = Endpoint::CreateTable(); + EXPECT_EQ(create_table.method(), HttpMethod::kPost); + EXPECT_EQ(create_table.path(), "/v1/{prefix}/namespaces/{namespace}/tables"); + + auto update_table = Endpoint::UpdateTable(); + EXPECT_EQ(update_table.method(), HttpMethod::kPost); + EXPECT_EQ(update_table.path(), "/v1/{prefix}/namespaces/{namespace}/tables/{table}"); + + auto delete_table = Endpoint::DeleteTable(); + EXPECT_EQ(delete_table.method(), HttpMethod::kDelete); + EXPECT_EQ(delete_table.path(), "/v1/{prefix}/namespaces/{namespace}/tables/{table}"); + + auto rename_table = Endpoint::RenameTable(); + EXPECT_EQ(rename_table.method(), HttpMethod::kPost); + EXPECT_EQ(rename_table.path(), "/v1/{prefix}/tables/rename"); + + auto register_table = Endpoint::RegisterTable(); + EXPECT_EQ(register_table.method(), HttpMethod::kPost); + EXPECT_EQ(register_table.path(), "/v1/{prefix}/namespaces/{namespace}/register"); + + auto report_metrics = Endpoint::ReportMetrics(); + EXPECT_EQ(report_metrics.method(), HttpMethod::kPost); + EXPECT_EQ(report_metrics.path(), + "/v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics"); + + auto table_credentials = Endpoint::TableCredentials(); + EXPECT_EQ(table_credentials.method(), HttpMethod::kGet); + EXPECT_EQ(table_credentials.path(), + "/v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials"); +} + +// Test predefined transaction endpoints +TEST(EndpointTest, TransactionEndpoints) { + auto commit_transaction = Endpoint::CommitTransaction(); + EXPECT_EQ(commit_transaction.method(), HttpMethod::kPost); + EXPECT_EQ(commit_transaction.path(), "/v1/{prefix}/transactions/commit"); +} + +// Test endpoint equality +TEST(EndpointTest, Equality) { + auto endpoint1 = Endpoint::Make(HttpMethod::kGet, "/path"); + auto endpoint2 = Endpoint::Make(HttpMethod::kGet, "/path"); + auto endpoint3 = Endpoint::Make(HttpMethod::kPost, "/path"); + auto endpoint4 = Endpoint::Make(HttpMethod::kGet, "/other"); + + ASSERT_THAT(endpoint1, IsOk()); + ASSERT_THAT(endpoint2, IsOk()); + ASSERT_THAT(endpoint3, IsOk()); + ASSERT_THAT(endpoint4, IsOk()); + + // Equality + EXPECT_EQ(*endpoint1, *endpoint2); + EXPECT_NE(*endpoint1, *endpoint3); + EXPECT_NE(*endpoint1, *endpoint4); +} + +// Test string serialization +TEST(EndpointTest, ToStringFormat) { + auto endpoint1 = Endpoint::Make(HttpMethod::kGet, "/v1/{prefix}/namespaces"); + ASSERT_THAT(endpoint1, IsOk()); + EXPECT_EQ(endpoint1->ToString(), "GET /v1/{prefix}/namespaces"); + + auto endpoint2 = Endpoint::Make(HttpMethod::kPost, "/v1/{prefix}/tables"); + ASSERT_THAT(endpoint2, IsOk()); + EXPECT_EQ(endpoint2->ToString(), "POST /v1/{prefix}/tables"); + + // Test with all HTTP methods + auto endpoint3 = Endpoint::Make(HttpMethod::kDelete, "/path"); + ASSERT_THAT(endpoint3, IsOk()); + EXPECT_EQ(endpoint3->ToString(), "DELETE /path"); + + auto endpoint4 = Endpoint::Make(HttpMethod::kPut, "/path"); + ASSERT_THAT(endpoint4, IsOk()); + EXPECT_EQ(endpoint4->ToString(), "PUT /path"); + + auto endpoint5 = Endpoint::Make(HttpMethod::kHead, "/path"); + ASSERT_THAT(endpoint5, IsOk()); + EXPECT_EQ(endpoint5->ToString(), "HEAD /path"); +} + +// Test string deserialization +TEST(EndpointTest, FromStringParsing) { + auto result1 = Endpoint::FromString("GET /v1/{prefix}/namespaces"); + ASSERT_THAT(result1, IsOk()); + EXPECT_EQ(result1->method(), HttpMethod::kGet); + EXPECT_EQ(result1->path(), "/v1/{prefix}/namespaces"); + + auto result2 = Endpoint::FromString("POST /v1/{prefix}/namespaces/{namespace}/tables"); + ASSERT_THAT(result2, IsOk()); + EXPECT_EQ(result2->method(), HttpMethod::kPost); + EXPECT_EQ(result2->path(), "/v1/{prefix}/namespaces/{namespace}/tables"); + + // Test all HTTP methods + auto result3 = Endpoint::FromString("DELETE /path"); + ASSERT_THAT(result3, IsOk()); + EXPECT_EQ(result3->method(), HttpMethod::kDelete); + + auto result4 = Endpoint::FromString("PUT /path"); + ASSERT_THAT(result4, IsOk()); + EXPECT_EQ(result4->method(), HttpMethod::kPut); + + auto result5 = Endpoint::FromString("HEAD /path"); + ASSERT_THAT(result5, IsOk()); + EXPECT_EQ(result5->method(), HttpMethod::kHead); +} + +// Test string parsing with invalid inputs +TEST(EndpointTest, FromStringInvalid) { + // Invalid endpoint format should fail - missing space + auto result1 = Endpoint::FromString("/path/without/method"); + EXPECT_THAT(result1, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result1, + HasErrorMessage("Invalid endpoint format (must consist of two elements " + "separated by a single space)")); + + // Invalid HTTP method should fail + auto result2 = Endpoint::FromString("INVALID /path"); + EXPECT_THAT(result2, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result2, HasErrorMessage("Invalid HTTP method")); + + // Invalid endpoint format - extra element after path + auto result3 = Endpoint::FromString("GET /path INVALID"); + EXPECT_THAT(result3, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result3, + HasErrorMessage("Invalid endpoint format (must consist of two elements " + "separated by a single space)")); +} + +// Test string round-trip +TEST(EndpointTest, StringRoundTrip) { + // Create various endpoints and verify they survive string round-trip + std::vector endpoints = { + Endpoint::ListNamespaces(), Endpoint::GetNamespaceProperties(), + Endpoint::CreateNamespace(), Endpoint::LoadTable(), + Endpoint::CreateTable(), Endpoint::DeleteTable(), + }; + + for (const auto& original : endpoints) { + // Serialize to string + std::string str = original.ToString(); + + // Deserialize from string + auto deserialized = Endpoint::FromString(str); + ASSERT_THAT(deserialized, IsOk()); + + // Verify they are equal + EXPECT_EQ(original, *deserialized); + EXPECT_EQ(original.ToString(), deserialized->ToString()); + } +} + +} // namespace iceberg::rest diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc index 49c527f64..725ad7ece 100644 --- a/src/iceberg/test/rest_catalog_test.cc +++ b/src/iceberg/test/rest_catalog_test.cc @@ -32,9 +32,13 @@ #include #include #include +#include #include #include "iceberg/catalog/rest/catalog_properties.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/json_internal.h" #include "iceberg/result.h" #include "iceberg/table_identifier.h" #include "iceberg/test/matchers.h" @@ -137,6 +141,40 @@ TEST_F(RestCatalogIntegrationTest, MakeCatalogSuccess) { EXPECT_EQ(catalog->name(), kCatalogName); } +/// Verifies that the server's /v1/config endpoint returns a valid response +/// and that the endpoints field (if present) can be parsed correctly. +TEST_F(RestCatalogIntegrationTest, FetchServerConfigDirect) { + // Create HTTP client and fetch config directly + HttpClient client({}); + std::string config_url = + std::format("{}:{}/v1/config", kLocalhostUri, kRestCatalogPort); + + auto response_result = client.Get(config_url, {}, {}, *DefaultErrorHandler::Instance()); + ASSERT_THAT(response_result, IsOk()); + auto json_result = FromJsonString(response_result->body()); + ASSERT_THAT(json_result, IsOk()); + auto& json = json_result.value(); + + EXPECT_TRUE(json.contains("defaults")); + EXPECT_TRUE(json.contains("overrides")); + + if (json.contains("endpoints")) { + EXPECT_TRUE(json["endpoints"].is_array()); + + // Parse the config to ensure all endpoints are valid + auto config_result = CatalogConfigFromJson(json); + ASSERT_THAT(config_result, IsOk()); + auto& config = config_result.value(); + std::println("[INFO] Server provided {} endpoints", config.endpoints.size()); + EXPECT_GT(config.endpoints.size(), 0) + << "Server should provide at least one endpoint"; + } else { + std::println( + "[INFO] Server did not provide endpoints field, client will use default " + "endpoints"); + } +} + TEST_F(RestCatalogIntegrationTest, ListNamespaces) { auto catalog_result = CreateCatalog(); ASSERT_THAT(catalog_result, IsOk()); diff --git a/src/iceberg/test/rest_json_internal_test.cc b/src/iceberg/test/rest_json_internal_test.cc index ca2671fae..f95ab09cb 100644 --- a/src/iceberg/test/rest_json_internal_test.cc +++ b/src/iceberg/test/rest_json_internal_test.cc @@ -783,7 +783,7 @@ INSTANTIATE_TEST_SUITE_P( CatalogConfigParam{.test_name = "BothEmpty", .expected_json_str = R"({"defaults":{},"overrides":{}})", .model = {}}, - // With endpoints + // With valid endpoints CatalogConfigParam{ .test_name = "WithEndpoints", .expected_json_str = @@ -791,13 +791,14 @@ INSTANTIATE_TEST_SUITE_P( .model = {.defaults = {{"warehouse", "s3://bucket/warehouse"}}, .overrides = {{"clients", "5"}}, - .endpoints = {"GET /v1/config", "POST /v1/tables"}}}, + .endpoints = {*Endpoint::Make(HttpMethod::kGet, "/v1/config"), + *Endpoint::Make(HttpMethod::kPost, "/v1/tables")}}}, // Only endpoints CatalogConfigParam{ .test_name = "OnlyEndpoints", .expected_json_str = R"({"defaults":{},"overrides":{},"endpoints":["GET /v1/config"]})", - .model = {.endpoints = {"GET /v1/config"}}}), + .model = {.endpoints = {*Endpoint::Make(HttpMethod::kGet, "/v1/config")}}}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); @@ -834,7 +835,12 @@ INSTANTIATE_TEST_SUITE_P( // Both fields null CatalogConfigDeserializeParam{.test_name = "BothNull", .json_str = R"({"defaults":null,"overrides":null})", - .expected_model = {}}), + .expected_model = {}}, + // Missing endpoints field, client will uses default endpoints + CatalogConfigDeserializeParam{ + .test_name = "MissingEndpoints", + .json_str = R"({"defaults":{},"overrides":{}})", + .expected_model = {.defaults = {}, .overrides = {}, .endpoints = {}}}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); @@ -855,7 +861,22 @@ INSTANTIATE_TEST_SUITE_P( .test_name = "WrongOverridesType", .invalid_json_str = R"({"defaults":{"warehouse":"s3://bucket/warehouse"},"overrides":"clients"})", - .expected_error_message = "type must be object, but is string"}), + .expected_error_message = "type must be object, but is string"}, + // Invalid endpoint format - missing space separator + CatalogConfigInvalidParam{ + .test_name = "InvalidEndpointMissingSpace", + .invalid_json_str = R"({"endpoints":["GET_v1/namespaces/{namespace}"]})", + .expected_error_message = + "Invalid endpoint format (must consist of two elements separated by a " + "single space)"}, + // Invalid endpoint format - extra element after path + CatalogConfigInvalidParam{ + .test_name = "InvalidEndpointExtraElement", + .invalid_json_str = + R"({"endpoints":["GET v1/namespaces/{namespace} INVALID"]})", + .expected_error_message = + "Invalid endpoint format (must consist of two elements separated by a " + "single space)"}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); diff --git a/src/iceberg/test/rest_util_test.cc b/src/iceberg/test/rest_util_test.cc index b95a220bb..5d2abf558 100644 --- a/src/iceberg/test/rest_util_test.cc +++ b/src/iceberg/test/rest_util_test.cc @@ -21,6 +21,7 @@ #include +#include "iceberg/catalog/rest/endpoint.h" #include "iceberg/table_identifier.h" #include "iceberg/test/matchers.h" @@ -153,4 +154,20 @@ TEST(RestUtilTest, MergeConfigs) { EXPECT_EQ(merged_empty["key"], "value"); } +TEST(RestUtilTest, CheckEndpointSupported) { + std::unordered_set supported = { + Endpoint::ListNamespaces(), Endpoint::LoadTable(), Endpoint::CreateTable()}; + + // Supported endpoints should pass + EXPECT_THAT(CheckEndpoint(supported, Endpoint::ListNamespaces()), IsOk()); + EXPECT_THAT(CheckEndpoint(supported, Endpoint::LoadTable()), IsOk()); + EXPECT_THAT(CheckEndpoint(supported, Endpoint::CreateTable()), IsOk()); + + // Unsupported endpoints should fail + EXPECT_THAT(CheckEndpoint(supported, Endpoint::DeleteTable()), + IsError(ErrorKind::kNotSupported)); + EXPECT_THAT(CheckEndpoint(supported, Endpoint::UpdateTable()), + IsError(ErrorKind::kNotSupported)); +} + } // namespace iceberg::rest