wgtmac commented on code in PR #296: URL: https://github.com/apache/iceberg-cpp/pull/296#discussion_r2513054154
########## src/iceberg/catalog/rest/catalog.h: ########## @@ -0,0 +1,165 @@ +/* + * 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 <memory> +#include <string> + +#include <cpr/cpr.h> Review Comment: We cannot include any cpr header to an installed header file. ########## src/iceberg/catalog/rest/constant.h: ########## @@ -17,28 +17,27 @@ * under the License. */ -#include "iceberg/catalog/rest/rest_catalog.h" +#pragma once -#include <utility> +#include <string_view> -#include <cpr/cpr.h> +#include "iceberg/version.h" -#include "iceberg/catalog/rest/types.h" +/// \file iceberg/catalog/rest/constant.h +/// Constant values for Iceberg REST API. -namespace iceberg::catalog::rest { +namespace iceberg::rest { -RestCatalog::RestCatalog(const std::string& base_url) : base_url_(std::move(base_url)) {} +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"; -cpr::Response RestCatalog::GetConfig() { - cpr::Url url = cpr::Url{base_url_ + "/v1/config"}; - cpr::Response r = cpr::Get(url); - return r; -} +constexpr std::string_view kMimeTypeApplicationJson = "application/json"; +constexpr std::string_view kClientVersion = "0.14.1"; Review Comment: What is this? ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" Review Comment: ditto ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { + /// \brief Returns the endpoint string for listing all catalog configuration settings. + std::string GetConfigEndpoint() const; + + /// \brief Returns the endpoint string for OAuth2 token operations (DEPRECATED). + std::string GetOAuth2TokensEndpoint() const; Review Comment: I think it is a little bit an anti-pattern to let the config object to build endpoint path. `RestCatalogConfig` better to be a data-carrying object instead of an utility class. I'd recommend to go with the same approach as the Java impl to introduce `ResourcePaths` to deal with endpoints. The reason is that endpoint does not only belong to the client, but also can be a building block for a rest catalog server and test cases. ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,199 @@ +/* + * 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 <memory> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<RestCatalog> RestCatalog::Make(RestCatalogConfig config) { + // Validate that uri is not empty + if (config.uri.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + ICEBERG_ASSIGN_OR_RAISE(auto tmp_client, HttpClient::Make(config)); + const std::string endpoint = config.GetConfigEndpoint(); + cpr::Parameters params; + if (config.warehouse.has_value()) { + params.Add({"warehouse", config.warehouse.value()}); + } + 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.properties_) { + 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); + } + RestCatalogConfig final_config = { + .uri = config.uri, + .name = config.name, + .warehouse = config.warehouse, + .properties_ = std::move(final_props), + }; + ICEBERG_ASSIGN_OR_RAISE(auto client, HttpClient::Make(final_config)); + return RestCatalog(std::make_shared<RestCatalogConfig>(std::move(final_config)), + std::move(client)); + }; + 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::shared_ptr<RestCatalogConfig> config, + std::unique_ptr<HttpClient> client) + : config_(std::move(config)), client_(std::move(client)) {} + +std::string_view RestCatalog::name() const { + return config_->name.has_value() ? std::string_view(*config_->name) + : std::string_view(""); +} + +Result<std::vector<Namespace>> RestCatalog::ListNamespaces(const Namespace& ns) const { + const std::string endpoint = config_->GetNamespacesEndpoint(); + std::vector<Namespace> result; + std::string next_token; + while (true) { + cpr::Parameters params; + if (!ns.levels.empty()) { + params.Add({"parent", EncodeNamespaceForUrl(ns)}); + } + if (!next_token.empty()) { + params.Add({"page_token", 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: { Review Comment: We don't want to copy and paste error handling in each function. Perhaps adding a dedicated `ErrorHandler` class for this? ########## src/iceberg/catalog/rest/config.cc: ########## @@ -0,0 +1,142 @@ +/* + * 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 <cctype> +#include <ranges> + +#include "iceberg/catalog/rest/constant.h" + +namespace iceberg::rest { + +std::string RestCatalogConfig::GetConfigEndpoint() const { + return std::format("{}/{}/config", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetOAuth2TokensEndpoint() const { + return std::format("{}/{}/oauth/tokens", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetNamespacesEndpoint() const { + return std::format("{}/{}/namespaces", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetNamespaceEndpoint(const Namespace& ns) const { + return std::format("{}/{}/namespaces/{}", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(ns)); +} + +std::string RestCatalogConfig::GetNamespacePropertiesEndpoint(const Namespace& ns) const { + return std::format("{}/{}/namespaces/{}/properties", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(ns)); +} + +std::string RestCatalogConfig::GetTablesEndpoint(const Namespace& ns) const { + return std::format("{}/{}/namespaces/{}/tables", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(ns)); +} + +std::string RestCatalogConfig::GetTableScanPlanEndpoint( + const TableIdentifier& table) const { + return std::format("{}/{}/namespaces/{}/tables/{}/plan", TrimTrailingSlash(uri), + kPathV1, EncodeNamespaceForUrl(table.ns), table.name); +} + +std::string RestCatalogConfig::GetTableScanPlanResultEndpoint( + const TableIdentifier& table, const std::string& plan_id) const { + return std::format("{}/{}/namespaces/{}/tables/{}/plan/{}", TrimTrailingSlash(uri), + kPathV1, EncodeNamespaceForUrl(table.ns), table.name, plan_id); +} + +std::string RestCatalogConfig::GetTableTasksEndpoint(const TableIdentifier& table) const { + return std::format("{}/{}/namespaces/{}/tables/{}/tasks", TrimTrailingSlash(uri), + kPathV1, EncodeNamespaceForUrl(table.ns), table.name); +} + +std::string RestCatalogConfig::GetRegisterTableEndpoint(const Namespace& ns) const { + return std::format("{}/{}/namespaces/{}/register", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(ns)); +} + +std::string RestCatalogConfig::GetTableEndpoint(const TableIdentifier& table) const { + return std::format("{}/{}/namespaces/{}/tables/{}", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(table.ns), table.name); +} + +std::string RestCatalogConfig::GetTableCredentialsEndpoint( + const TableIdentifier& table) const { + return std::format("{}/{}/namespaces/{}/tables/{}/credentials", TrimTrailingSlash(uri), + kPathV1, EncodeNamespaceForUrl(table.ns), table.name); +} + +std::string RestCatalogConfig::GetRenameTableEndpoint() const { + return std::format("{}/{}/tables/rename", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetTableMetricsEndpoint( + const TableIdentifier& table) const { + return std::format("{}/{}/namespaces/{}/tables/{}/metrics", TrimTrailingSlash(uri), + kPathV1, EncodeNamespaceForUrl(table.ns), table.name); +} + +std::string RestCatalogConfig::GetTransactionCommitEndpoint() const { + return std::format("{}/{}/transactions/commit", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetViewsEndpoint(const Namespace& ns) const { + return std::format("{}/{}/namespaces/{}/views", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(ns)); +} + +std::string RestCatalogConfig::GetViewEndpoint(const TableIdentifier& view) const { + return std::format("{}/{}/namespaces/{}/views/{}", TrimTrailingSlash(uri), kPathV1, + EncodeNamespaceForUrl(view.ns), view.name); +} + +std::string RestCatalogConfig::GetRenameViewEndpoint() const { + return std::format("{}/{}/views/rename", TrimTrailingSlash(uri), kPathV1); +} + +Result<cpr::Header> RestCatalogConfig::GetExtraHeaders() const { + cpr::Header headers; + + headers[std::string(kHeaderContentType)] = std::string(kMimeTypeApplicationJson); Review Comment: If the major usage of these magics are as string types, we should better define them as strings. ########## src/iceberg/catalog/rest/util.h: ########## @@ -0,0 +1,83 @@ +/* + * 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 <cctype> +#include <format> +#include <string> +#include <string_view> + +#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) { Review Comment: What about renaming the file to `endpoint_util.h`? ########## src/iceberg/catalog/rest/catalog.h: ########## @@ -0,0 +1,165 @@ +/* + * 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 <memory> +#include <string> + +#include <cpr/cpr.h> + +#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/types.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&&) = default; + RestCatalog& operator=(RestCatalog&&) = default; + + /// \brief Create a RestCatalog instance + /// + /// \param config the configuration for the RestCatalog + /// \return a RestCatalog instance + static Result<RestCatalog> Make(RestCatalogConfig config); Review Comment: ```suggestion static Result<std::unique_ptr<RestCatalog>> Make(const RestCatalogConfig& config); ``` ########## src/iceberg/catalog/rest/catalog.h: ########## @@ -0,0 +1,165 @@ +/* + * 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 <memory> +#include <string> + +#include <cpr/cpr.h> + +#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/types.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&&) = default; + RestCatalog& operator=(RestCatalog&&) = default; + + /// \brief Create a RestCatalog instance + /// + /// \param config the configuration for the RestCatalog + /// \return a RestCatalog instance + static Result<RestCatalog> Make(RestCatalogConfig config); + + /// \brief Return the name for this catalog Review Comment: It is a common practice that override functions do not need to write any comment unless the behavior has changed or attention should be paid. ########## src/iceberg/catalog/rest/catalog.h: ########## @@ -0,0 +1,165 @@ +/* + * 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 <memory> +#include <string> + +#include <cpr/cpr.h> + +#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/types.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&&) = default; + RestCatalog& operator=(RestCatalog&&) = default; + + /// \brief Create a RestCatalog instance + /// + /// \param config the configuration for the RestCatalog + /// \return a RestCatalog instance + static Result<RestCatalog> Make(RestCatalogConfig config); + + /// \brief Return the name for this catalog + std::string_view name() const override; + + /// \brief List child namespaces from the given namespace. + Result<std::vector<Namespace>> ListNamespaces(const Namespace& ns) const override; + + /// \brief Create a namespace with associated properties. + Status CreateNamespace( + const Namespace& ns, + const std::unordered_map<std::string, std::string>& properties) override; + + /// \brief Get metadata properties for a namespace. + Result<std::unordered_map<std::string, std::string>> GetNamespaceProperties( + const Namespace& ns) const override; + + /// \brief Drop a namespace. + Status DropNamespace(const Namespace& ns) override; + + /// \brief Check whether the namespace exists. + Result<bool> NamespaceExists(const Namespace& ns) const override; + + /// \brief Update a namespace's properties by applying additions and removals. + Status UpdateNamespaceProperties( + const Namespace& ns, const std::unordered_map<std::string, std::string>& updates, + const std::unordered_set<std::string>& removals) override; + + /// \brief Return all the identifiers under this namespace + Result<std::vector<TableIdentifier>> ListTables(const Namespace& ns) const override; + + /// \brief Create a table + Result<std::unique_ptr<Table>> CreateTable( + const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, + const std::string& location, + const std::unordered_map<std::string, std::string>& properties) override; + + /// \brief Update a table + /// + /// \param identifier a table identifier + /// \param requirements a list of table requirements + /// \param updates a list of table updates + /// \return a Table instance or ErrorKind::kAlreadyExists if the table already exists + Result<std::unique_ptr<Table>> UpdateTable( + const TableIdentifier& identifier, + const std::vector<std::unique_ptr<TableRequirement>>& requirements, + const std::vector<std::unique_ptr<TableUpdate>>& updates) override; + + /// \brief Start a transaction to create a table + /// + /// \param identifier a table identifier + /// \param schema a schema + /// \param spec a partition spec + /// \param location a location for the table; leave empty if unspecified + /// \param properties a string map of table properties + /// \return a Transaction to create the table or ErrorKind::kAlreadyExists if the + /// table already exists + Result<std::shared_ptr<Transaction>> StageCreateTable( + const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec, + const std::string& location, + const std::unordered_map<std::string, std::string>& properties) override; + + /// \brief Check whether table exists + /// + /// \param identifier a table identifier + /// \return Result<bool> indicating table exists or not. + /// - On success, the table existence was successfully checked (actual + /// existence may be inferred elsewhere). + /// - On failure, contains error information. + Result<bool> TableExists(const TableIdentifier& identifier) const override; + + /// \brief Drop a table; optionally delete data and metadata files + /// + /// If purge is set to true the implementation should delete all data and metadata + /// files. + /// + /// \param identifier a table identifier + /// \param purge if true, delete all data and metadata files in the table + /// \return Status indicating the outcome of the operation. + /// - On success, the table was dropped (or did not exist). + /// - On failure, contains error information. + Status DropTable(const TableIdentifier& identifier, bool purge) override; + + /// \brief Load a table + /// + /// \param identifier a table identifier + /// \return instance of Table implementation referred to by identifier or + /// ErrorKind::kNoSuchTable if the table does not exist + Result<std::unique_ptr<Table>> LoadTable(const TableIdentifier& identifier) override; + + /// \brief Register a table with the catalog if it does not exist + /// + /// \param identifier a table identifier + /// \param metadata_file_location the location of a metadata file + /// \return a Table instance or ErrorKind::kAlreadyExists if the table already exists + Result<std::shared_ptr<Table>> RegisterTable( + const TableIdentifier& identifier, + const std::string& metadata_file_location) override; + + /// \brief A builder used to create valid tables or start create/replace transactions + /// + /// \param identifier a table identifier + /// \param schema a schema + /// \return the builder to create a table or start a create/replace transaction + std::unique_ptr<RestCatalog::TableBuilder> BuildTable( + const TableIdentifier& identifier, const Schema& schema) const override; + + private: + RestCatalog(std::shared_ptr<RestCatalogConfig> config, + std::unique_ptr<HttpClient> client); + + std::shared_ptr<RestCatalogConfig> config_; Review Comment: What is the reason that config is shared but client is unique? Shouldn't both be unique? ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be Review Comment: > It's a simple data structure with public members that can be initialized directly via aggregate initialization. TBH, I don't think this is the case. It is more than a simple structure because it should be immutable and queryable. Let's change it to a typical class as suggested below. ########## src/iceberg/catalog/rest/config.cc: ########## @@ -0,0 +1,142 @@ +/* + * 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 <cctype> +#include <ranges> + +#include "iceberg/catalog/rest/constant.h" + +namespace iceberg::rest { + +std::string RestCatalogConfig::GetConfigEndpoint() const { + return std::format("{}/{}/config", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetOAuth2TokensEndpoint() const { + return std::format("{}/{}/oauth/tokens", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetNamespacesEndpoint() const { + return std::format("{}/{}/namespaces", TrimTrailingSlash(uri), kPathV1); Review Comment: This is not correct. `prefix` should be considered though in most cases it is an empty string. ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { + /// \brief Returns the endpoint string for listing all catalog configuration settings. + std::string GetConfigEndpoint() const; + + /// \brief Returns the endpoint string for OAuth2 token operations (DEPRECATED). + std::string GetOAuth2TokensEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating namespaces. + std::string GetNamespacesEndpoint() const; + + /// \brief Returns the endpoint string for loading or managing a specific namespace. + /// \param ns The namespace to get the endpoint for. + std::string GetNamespaceEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for setting or removing namespace properties. + /// \param ns The namespace to get the properties endpoint for. + std::string GetNamespacePropertiesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for listing or creating tables in a namespace. + /// \param ns The namespace to get the tables endpoint for. + std::string GetTablesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for submitting a table scan for planning. + /// \param table The table identifier to get the scan plan endpoint for. + std::string GetTableScanPlanEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for fetching scan planning results. + /// \param table The table identifier to get the scan plan result endpoint for. + /// \param plan_id The plan ID to fetch results for. + std::string GetTableScanPlanResultEndpoint(const TableIdentifier& table, + const std::string& plan_id) const; + + /// \brief Returns the endpoint string for fetching execution tasks for a plan. + /// \param table The table identifier to get the tasks endpoint for. + std::string GetTableTasksEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for registering a table using metadata file + /// location. + /// \param ns The namespace to register the table in. + std::string GetRegisterTableEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, committing, or dropping a table. + /// \param table The table identifier to get the endpoint for. + std::string GetTableEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for loading vended credentials for a table. + /// \param table The table identifier to get the credentials endpoint for. + std::string GetTableCredentialsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for renaming a table. + std::string GetRenameTableEndpoint() const; + + /// \brief Returns the endpoint string for submitting a metrics report for a table. + /// \param table The table identifier to get the metrics endpoint for. + std::string GetTableMetricsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for atomic multi-table commit operations. + std::string GetTransactionCommitEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating views in a namespace. + /// \param ns The namespace to get the views endpoint for. + std::string GetViewsEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, replacing, or dropping a view. + /// \param view The view identifier to get the endpoint for. + std::string GetViewEndpoint(const TableIdentifier& view) const; + + /// \brief Returns the endpoint string for renaming a view. + std::string GetRenameViewEndpoint() const; + + /// \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<cpr::Header> GetExtraHeaders() const; + + /// \brief The catalog's URI (required). + std::string uri; + + /// \brief The logical name of the catalog (optional). + std::optional<std::string> name; Review Comment: ```suggestion std::string name; ``` ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { Review Comment: ```suggestion class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase<RestCatalogConfig> { ``` Consider extending `ConfigBase` for all key-value properties. This will be a lot easier to add predefined property like https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java and https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/CatalogProperties.java. See `src/iceberg/table_properties.h` on how table properties are defined. ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { + /// \brief Returns the endpoint string for listing all catalog configuration settings. + std::string GetConfigEndpoint() const; + + /// \brief Returns the endpoint string for OAuth2 token operations (DEPRECATED). + std::string GetOAuth2TokensEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating namespaces. + std::string GetNamespacesEndpoint() const; + + /// \brief Returns the endpoint string for loading or managing a specific namespace. + /// \param ns The namespace to get the endpoint for. + std::string GetNamespaceEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for setting or removing namespace properties. + /// \param ns The namespace to get the properties endpoint for. + std::string GetNamespacePropertiesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for listing or creating tables in a namespace. + /// \param ns The namespace to get the tables endpoint for. + std::string GetTablesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for submitting a table scan for planning. + /// \param table The table identifier to get the scan plan endpoint for. + std::string GetTableScanPlanEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for fetching scan planning results. + /// \param table The table identifier to get the scan plan result endpoint for. + /// \param plan_id The plan ID to fetch results for. + std::string GetTableScanPlanResultEndpoint(const TableIdentifier& table, + const std::string& plan_id) const; + + /// \brief Returns the endpoint string for fetching execution tasks for a plan. + /// \param table The table identifier to get the tasks endpoint for. + std::string GetTableTasksEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for registering a table using metadata file + /// location. + /// \param ns The namespace to register the table in. + std::string GetRegisterTableEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, committing, or dropping a table. + /// \param table The table identifier to get the endpoint for. + std::string GetTableEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for loading vended credentials for a table. + /// \param table The table identifier to get the credentials endpoint for. + std::string GetTableCredentialsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for renaming a table. + std::string GetRenameTableEndpoint() const; + + /// \brief Returns the endpoint string for submitting a metrics report for a table. + /// \param table The table identifier to get the metrics endpoint for. + std::string GetTableMetricsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for atomic multi-table commit operations. + std::string GetTransactionCommitEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating views in a namespace. + /// \param ns The namespace to get the views endpoint for. + std::string GetViewsEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, replacing, or dropping a view. + /// \param view The view identifier to get the endpoint for. + std::string GetViewEndpoint(const TableIdentifier& view) const; + + /// \brief Returns the endpoint string for renaming a view. + std::string GetRenameViewEndpoint() const; + + /// \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<cpr::Header> GetExtraHeaders() const; + + /// \brief The catalog's URI (required). + std::string uri; + + /// \brief The logical name of the catalog (optional). + std::optional<std::string> name; + + /// \brief The warehouse location associated with the catalog (optional). + std::optional<std::string> warehouse; Review Comment: ```suggestion std::string warehouse; ``` ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { + /// \brief Returns the endpoint string for listing all catalog configuration settings. + std::string GetConfigEndpoint() const; + + /// \brief Returns the endpoint string for OAuth2 token operations (DEPRECATED). + std::string GetOAuth2TokensEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating namespaces. + std::string GetNamespacesEndpoint() const; + + /// \brief Returns the endpoint string for loading or managing a specific namespace. + /// \param ns The namespace to get the endpoint for. + std::string GetNamespaceEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for setting or removing namespace properties. + /// \param ns The namespace to get the properties endpoint for. + std::string GetNamespacePropertiesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for listing or creating tables in a namespace. + /// \param ns The namespace to get the tables endpoint for. + std::string GetTablesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for submitting a table scan for planning. + /// \param table The table identifier to get the scan plan endpoint for. + std::string GetTableScanPlanEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for fetching scan planning results. + /// \param table The table identifier to get the scan plan result endpoint for. + /// \param plan_id The plan ID to fetch results for. + std::string GetTableScanPlanResultEndpoint(const TableIdentifier& table, + const std::string& plan_id) const; + + /// \brief Returns the endpoint string for fetching execution tasks for a plan. + /// \param table The table identifier to get the tasks endpoint for. + std::string GetTableTasksEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for registering a table using metadata file + /// location. + /// \param ns The namespace to register the table in. + std::string GetRegisterTableEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, committing, or dropping a table. + /// \param table The table identifier to get the endpoint for. + std::string GetTableEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for loading vended credentials for a table. + /// \param table The table identifier to get the credentials endpoint for. + std::string GetTableCredentialsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for renaming a table. + std::string GetRenameTableEndpoint() const; + + /// \brief Returns the endpoint string for submitting a metrics report for a table. + /// \param table The table identifier to get the metrics endpoint for. + std::string GetTableMetricsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for atomic multi-table commit operations. + std::string GetTransactionCommitEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating views in a namespace. + /// \param ns The namespace to get the views endpoint for. + std::string GetViewsEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, replacing, or dropping a view. + /// \param view The view identifier to get the endpoint for. + std::string GetViewEndpoint(const TableIdentifier& view) const; + + /// \brief Returns the endpoint string for renaming a view. + std::string GetRenameViewEndpoint() const; + + /// \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<cpr::Header> GetExtraHeaders() const; + + /// \brief The catalog's URI (required). + std::string uri; + + /// \brief The logical name of the catalog (optional). + std::optional<std::string> name; + + /// \brief The warehouse location associated with the catalog (optional). + std::optional<std::string> warehouse; + + /// \brief A string-to-string map of properties to store all other configurations. + std::unordered_map<std::string, std::string> properties_; Review Comment: We can remove this if inheriting ConfigBase. ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { + /// \brief Returns the endpoint string for listing all catalog configuration settings. + std::string GetConfigEndpoint() const; + + /// \brief Returns the endpoint string for OAuth2 token operations (DEPRECATED). + std::string GetOAuth2TokensEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating namespaces. + std::string GetNamespacesEndpoint() const; + + /// \brief Returns the endpoint string for loading or managing a specific namespace. + /// \param ns The namespace to get the endpoint for. + std::string GetNamespaceEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for setting or removing namespace properties. + /// \param ns The namespace to get the properties endpoint for. + std::string GetNamespacePropertiesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for listing or creating tables in a namespace. + /// \param ns The namespace to get the tables endpoint for. + std::string GetTablesEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for submitting a table scan for planning. + /// \param table The table identifier to get the scan plan endpoint for. + std::string GetTableScanPlanEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for fetching scan planning results. + /// \param table The table identifier to get the scan plan result endpoint for. + /// \param plan_id The plan ID to fetch results for. + std::string GetTableScanPlanResultEndpoint(const TableIdentifier& table, + const std::string& plan_id) const; + + /// \brief Returns the endpoint string for fetching execution tasks for a plan. + /// \param table The table identifier to get the tasks endpoint for. + std::string GetTableTasksEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for registering a table using metadata file + /// location. + /// \param ns The namespace to register the table in. + std::string GetRegisterTableEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, committing, or dropping a table. + /// \param table The table identifier to get the endpoint for. + std::string GetTableEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for loading vended credentials for a table. + /// \param table The table identifier to get the credentials endpoint for. + std::string GetTableCredentialsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for renaming a table. + std::string GetRenameTableEndpoint() const; + + /// \brief Returns the endpoint string for submitting a metrics report for a table. + /// \param table The table identifier to get the metrics endpoint for. + std::string GetTableMetricsEndpoint(const TableIdentifier& table) const; + + /// \brief Returns the endpoint string for atomic multi-table commit operations. + std::string GetTransactionCommitEndpoint() const; + + /// \brief Returns the endpoint string for listing or creating views in a namespace. + /// \param ns The namespace to get the views endpoint for. + std::string GetViewsEndpoint(const Namespace& ns) const; + + /// \brief Returns the endpoint string for loading, replacing, or dropping a view. + /// \param view The view identifier to get the endpoint for. + std::string GetViewEndpoint(const TableIdentifier& view) const; + + /// \brief Returns the endpoint string for renaming a view. + std::string GetRenameViewEndpoint() const; + + /// \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<cpr::Header> GetExtraHeaders() const; Review Comment: We cannot expose cpr symbols in the public api. ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,199 @@ +/* + * 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 <memory> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_client_interal.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<RestCatalog> RestCatalog::Make(RestCatalogConfig config) { + // Validate that uri is not empty + if (config.uri.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + ICEBERG_ASSIGN_OR_RAISE(auto tmp_client, HttpClient::Make(config)); + const std::string endpoint = config.GetConfigEndpoint(); + cpr::Parameters params; + if (config.warehouse.has_value()) { + params.Add({"warehouse", config.warehouse.value()}); + } + 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.properties_) { + 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); + } + RestCatalogConfig final_config = { + .uri = config.uri, + .name = config.name, + .warehouse = config.warehouse, + .properties_ = std::move(final_props), + }; + ICEBERG_ASSIGN_OR_RAISE(auto client, HttpClient::Make(final_config)); + return RestCatalog(std::make_shared<RestCatalogConfig>(std::move(final_config)), + std::move(client)); + }; + 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::shared_ptr<RestCatalogConfig> config, + std::unique_ptr<HttpClient> client) + : config_(std::move(config)), client_(std::move(client)) {} + +std::string_view RestCatalog::name() const { + return config_->name.has_value() ? std::string_view(*config_->name) + : std::string_view(""); +} + +Result<std::vector<Namespace>> RestCatalog::ListNamespaces(const Namespace& ns) const { + const std::string endpoint = config_->GetNamespacesEndpoint(); + std::vector<Namespace> result; + std::string next_token; + while (true) { + cpr::Parameters params; + if (!ns.levels.empty()) { + params.Add({"parent", EncodeNamespaceForUrl(ns)}); Review Comment: Define these magic strings as constants. ########## src/iceberg/catalog/rest/config.cc: ########## @@ -0,0 +1,142 @@ +/* + * 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 <cctype> +#include <ranges> + +#include "iceberg/catalog/rest/constant.h" + +namespace iceberg::rest { + +std::string RestCatalogConfig::GetConfigEndpoint() const { + return std::format("{}/{}/config", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetOAuth2TokensEndpoint() const { + return std::format("{}/{}/oauth/tokens", TrimTrailingSlash(uri), kPathV1); +} + +std::string RestCatalogConfig::GetNamespacesEndpoint() const { + return std::format("{}/{}/namespaces", TrimTrailingSlash(uri), kPathV1); Review Comment: For example, the Java code is as below: ``` public String namespaces() { return SLASH.join("v1", prefix, "namespaces"); } ``` ########## src/iceberg/catalog/rest/config.h: ########## @@ -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. + */ + +#pragma once + +#include <memory> +#include <optional> +#include <string> + +#include "cpr/cpr.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/util.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration struct for a Rest Catalog. +/// +/// This struct holds all the necessary configuration for connecting and interacting with +/// a Rest service. It's a simple data structure with public members that can be +/// initialized directly via aggregate initialization. It provides helper methods to +/// construct specific endpoint URLs and HTTP headers from the configuration properties. +struct ICEBERG_REST_EXPORT RestCatalogConfig { + /// \brief Returns the endpoint string for listing all catalog configuration settings. Review Comment: Yes that's a good point. I think Iceberg spec (at least the Java impl) has defined some properties to address this as my comment above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
