wgtmac commented on code in PR #296: URL: https://github.com/apache/iceberg-cpp/pull/296#discussion_r2544171045
########## src/iceberg/catalog/rest/CMakeLists.txt: ########## @@ -15,7 +15,12 @@ # specific language governing permissions and limitations # under the License. -set(ICEBERG_REST_SOURCES rest_catalog.cc json_internal.cc) +set(ICEBERG_REST_SOURCES + catalog.cc + json_internal.cc + config.cc Review Comment: Sort them alphabetically ########## src/iceberg/catalog/rest/config.h: ########## @@ -0,0 +1,68 @@ +/* + * 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 <string_view> +#include <unordered_map> + +#include <cpr/cprtypes.h> Review Comment: No cpr ########## src/iceberg/catalog/rest/CMakeLists.txt: ########## @@ -15,7 +15,12 @@ # specific language governing permissions and limitations # under the License. -set(ICEBERG_REST_SOURCES rest_catalog.cc json_internal.cc) +set(ICEBERG_REST_SOURCES + catalog.cc Review Comment: ```suggestion rest_catalog.cc ``` Rename it to be consistent with other catalog implementations to improve readability. ########## src/iceberg/catalog/rest/config.h: ########## @@ -0,0 +1,68 @@ +/* + * 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 <string_view> +#include <unordered_map> + +#include <cpr/cprtypes.h> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/util/config.h" + +/// \file iceberg/catalog/rest/config.h Review Comment: ```suggestion /// \file iceberg/catalog/rest/catalog_config.h ``` Rename it to catalog_config or catalog_properties. I'm inclined to use `catalog_properties.h` and `CatalogProperties` to be consistent with other properties in this repo. ########## src/iceberg/catalog/rest/http_response.h: ########## @@ -0,0 +1,50 @@ +/* + * 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 <string> +#include <unordered_map> + +#include "cpr/response.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" + +/// \file iceberg/catalog/rest/http_response.h +/// \brief A simple wrapper for cpr::Response. This class encapsulates the details of the +/// underlying cpr library's response, providing a consistent interface that is +/// independent of the specific network library used. + +class ICEBERG_REST_EXPORT HttpResponse { Review Comment: What about moving it to `http_client.h`? ########## src/iceberg/result.h: ########## @@ -42,13 +44,19 @@ enum class ErrorKind { kInvalidManifestList, kIOError, kJsonParseError, + kNamespaceNotEmpty, kNoSuchNamespace, kNoSuchTable, + kNoSuchView, kNotAllowed, + kNotAuthorized, kNotFound, kNotImplemented, kNotSupported, + kServiceFailure, Review Comment: I would replace this by `kInternalServerError` ########## src/iceberg/catalog/rest/config.h: ########## @@ -0,0 +1,68 @@ +/* + * 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 <string_view> +#include <unordered_map> + +#include <cpr/cprtypes.h> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/util/config.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration class for a REST Catalog. +class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase<RestCatalogConfig> { + public: + template <typename T> + using Entry = const ConfigBase<RestCatalogConfig>::Entry<T>; + + /// \brief The URI of the REST catalog server. + inline static std::string_view kUri{"uri"}; + + /// \brief The name of the catalog. + inline static std::string_view kName{"name"}; + + /// \brief The warehouse path. + inline static std::string_view kWarehouse{"warehouse"}; + + /// \brief Create a default RestCatalogConfig instance. + static std::unique_ptr<RestCatalogConfig> DefaultProperties(); Review Comment: ```suggestion static std::unique_ptr<RestCatalogConfig> default_properties(); ``` This function is trivial. ########## src/iceberg/catalog/rest/config.cc: ########## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/config.h" + +#include "iceberg/catalog/rest/constant.h" + +namespace iceberg::rest { + +std::unique_ptr<RestCatalogConfig> RestCatalogConfig::DefaultProperties() { + return std::make_unique<RestCatalogConfig>(); +} + +std::unique_ptr<RestCatalogConfig> RestCatalogConfig::FromMap( + const std::unordered_map<std::string, std::string>& properties) { + auto rest_catalog_config = std::make_unique<RestCatalogConfig>(); + rest_catalog_config->configs_ = properties; + return rest_catalog_config; +} + +std::unordered_map<std::string, std::string> RestCatalogConfig::GetExtraHeaders() const { + std::unordered_map<std::string, std::string> headers; + headers[kHeaderContentType] = kMimeTypeApplicationJson; + headers[kHeaderUserAgent] = kUserAgent; Review Comment: ```suggestion ``` Do not add them here. ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once Review Comment: I'd suggest renaming this file to `rest_util.h` ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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 to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). +/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are +/// percent-encoded using uppercase hexadecimal (e.g., space -> "%20"). +/// \param value The string to encode. +/// \return The encoded string. +inline std::string EncodeUriComponent(std::string_view value) { + std::string escaped; + escaped.reserve(value.length()); + for (const unsigned char c : value) { + if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { + escaped += c; + } else { + std::format_to(std::back_inserter(escaped), "%{:02X}", c); + } + } + return escaped; +} + +/// \brief Encode a Namespace into a URL-safe component. +/// \details Joins \p ns.levels with the ASCII Unit Separator (0x1F), then percent-encodes +/// the result via EncodeUriComponent. Returns an empty string if there are no levels. +/// \param ns The namespace (sequence of path-like levels) to encode. +/// \return The percent-encoded namespace string suitable for URLs. +inline std::string EncodeNamespaceForUrl(const Namespace& ns) { + if (ns.levels.empty()) { + return ""; + } + + std::string joined_string; + joined_string.append(ns.levels.front()); + for (size_t i = 1; i < ns.levels.size(); ++i) { + joined_string.append("\x1F"); Review Comment: This has been deprecated. Should we use `"%1F"`? ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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 to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). +/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are +/// percent-encoded using uppercase hexadecimal (e.g., space -> "%20"). +/// \param value The string to encode. +/// \return The encoded string. +inline std::string EncodeUriComponent(std::string_view value) { + std::string escaped; + escaped.reserve(value.length()); + for (const unsigned char c : value) { + if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { Review Comment: I don't think this match the Java impl. See below: ```java dontNeedEncoding = new BitSet(256); int i; for (i = 'a'; i <= 'z'; i++) { dontNeedEncoding.set(i); } for (i = 'A'; i <= 'Z'; i++) { dontNeedEncoding.set(i); } for (i = '0'; i <= '9'; i++) { dontNeedEncoding.set(i); } dontNeedEncoding.set(' '); /* encoding a space to a + is done * in the encode() method */ dontNeedEncoding.set('-'); dontNeedEncoding.set('_'); dontNeedEncoding.set('.'); dontNeedEncoding.set('*'); ``` ########## src/iceberg/catalog/rest/error_handlers.h: ########## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <format> +#include <string> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/error_handlers.h +/// Error handlers for different HTTP error types in Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Error handler interface for processing REST API error responses. Maps HTTP +/// status codes to appropriate ErrorKind values following the Iceberg REST specification. +class ICEBERG_REST_EXPORT ErrorHandler { + public: + virtual ~ErrorHandler() = default; + + /// \brief Process an error response and return an appropriate Error. + /// + /// \param error The error model parsed from the HTTP response body + /// \return An Error object with appropriate ErrorKind and message + virtual Status Accept(const ErrorModel& error) const = 0; +}; + +/// \brief Default error handler for REST API responses. +class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 400: + if (error.type == "IllegalArgumentException") { + return InvalidArgument("{}", error.message); Review Comment: ```suggestion return InvalidArgument(error.message); ``` Of course you need to modify this: ``` /// \brief Macro to define error creation functions #define DEFINE_ERROR_FUNCTION(name) \ template <typename... Args> \ inline auto name(const std::format_string<Args...> fmt, \ Args&&... args) -> std::unexpected<Error> { \ return std::unexpected<Error>( \ {ErrorKind::k##name, std::format(fmt, std::forward<Args>(args)...)}); \ } \ inline auto name(const std::string& message) -> std::unexpected<Error> { \ return std::unexpected<Error>({ErrorKind::k##name, message}); \ } ``` ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; Review Comment: ```suggestion std::string Config() const; ``` Let's remove the `V1` prefix. I would expect `ResourcePaths` can handle the version evolution in the future instead of letting caller to handle this. ########## src/iceberg/catalog/rest/error_handlers.h: ########## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <format> +#include <string> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/error_handlers.h +/// Error handlers for different HTTP error types in Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Error handler interface for processing REST API error responses. Maps HTTP +/// status codes to appropriate ErrorKind values following the Iceberg REST specification. +class ICEBERG_REST_EXPORT ErrorHandler { + public: + virtual ~ErrorHandler() = default; + + /// \brief Process an error response and return an appropriate Error. + /// + /// \param error The error model parsed from the HTTP response body + /// \return An Error object with appropriate ErrorKind and message + virtual Status Accept(const ErrorModel& error) const = 0; +}; + +/// \brief Default error handler for REST API responses. +class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 400: + if (error.type == "IllegalArgumentException") { + return InvalidArgument("{}", error.message); + } + return BadRequest("Malformed request: {}", error.message); + + case 401: + return NotAuthorized("Not authorized: {}", error.message); + + case 403: + return Forbidden("Forbidden: {}", error.message); + + case 405: + case 406: + break; + + case 500: + return ServiceFailure("Server error: {}: {}", error.type, error.message); + + case 501: + return NotSupported("{}", error.message); + + case 503: + return ServiceUnavailable("Service unavailable: {}", error.message); Review Comment: ```suggestion case 400: if (error.type == "IllegalArgumentException") { return InvalidArgument("{}", error.message); } return BadRequest("Malformed request: {}", error.message); case 401: return NotAuthorized("Not authorized: {}", error.message); case 403: return Forbidden("Forbidden: {}", error.message); case 405: case 406: break; case 500: return ServiceFailure("Server error: {}: {}", error.type, error.message); case 501: return NotSupported("{}", error.message); case 503: return ServiceUnavailable("Service unavailable: {}", error.message); ``` Let's make it compact. ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + std::string V1Table(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. + std::string V1RegisterTable(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/tables/rename endpoint path. + std::string V1RenameTable() const; Review Comment: ```suggestion std::string Rename() const; ``` Let's be consistent with the endpoint name. ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + std::string V1Table(const TableIdentifier& table) const; Review Comment: ```suggestion std::string V1Table(const TableIdentifier& ident) const; ``` Either `table_ident` or `ident` is enough. `table` harms readability since we may assume `Table` object. ########## src/iceberg/catalog/rest/resource_paths.cc: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/resource_paths.h" + +#include <format> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/result.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<ResourcePaths>> ResourcePaths::Make( + const RestCatalogConfig& config) { + // Validate and extract URI + auto it = config.configs().find(std::string(RestCatalogConfig::kUri)); + if (it == config.configs().end() || it->second.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + + std::string base_uri = std::string(TrimTrailingSlash(it->second)); + std::string prefix; + auto prefix_it = config.configs().find("prefix"); + if (prefix_it != config.configs().end() && !prefix_it->second.empty()) { + prefix = prefix_it->second; + } + + return std::unique_ptr<ResourcePaths>( + new ResourcePaths(std::move(base_uri), std::move(prefix))); +} + +ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) + : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} + +std::string ResourcePaths::BuildPath(std::string_view path) const { + if (prefix_.empty()) { + return std::format("{}/v1/{}", base_uri_, path); + } + return std::format("{}/v1/{}/{}", base_uri_, prefix_, path); Review Comment: ```suggestion return std::format("{}/v1/{}{}", base_uri_, (prefix_.empty() ? "" : (prefix_ + "/")), path); ``` ########## src/iceberg/catalog/rest/resource_paths.cc: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/resource_paths.h" + +#include <format> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/result.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<ResourcePaths>> ResourcePaths::Make( + const RestCatalogConfig& config) { + // Validate and extract URI + auto it = config.configs().find(std::string(RestCatalogConfig::kUri)); Review Comment: Can we add `Result<std::string> RestCatalogConfig::Uri() const` instead? Similarly, we can add getters for warehouse and prefix. It is not a good practice to expose internal logic (RestCatalogConfig) to the caller (ResourcePaths). ########## src/iceberg/catalog/rest/catalog.h: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <memory> +#include <string> + +#include "iceberg/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/resource_paths.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/catalog.h +/// RestCatalog implementation for Iceberg REST API. + +namespace iceberg::rest { + +class ICEBERG_REST_EXPORT RestCatalog : public Catalog { + public: + RestCatalog(const RestCatalog&) = delete; + RestCatalog& operator=(const RestCatalog&) = delete; + RestCatalog(RestCatalog&&) = delete; + RestCatalog& operator=(RestCatalog&&) = delete; + + /// \brief Create a RestCatalog instance + /// + /// \param config the configuration for the RestCatalog + /// \return a unique_ptr to RestCatalog instance + static Result<std::unique_ptr<RestCatalog>> Make(const RestCatalogConfig& config); + + std::string_view name() const override; + + Result<std::vector<Namespace>> ListNamespaces(const Namespace& ns) const override; + + Status CreateNamespace( + const Namespace& ns, + const std::unordered_map<std::string, std::string>& properties) override; + + Result<std::unordered_map<std::string, std::string>> GetNamespaceProperties( + const Namespace& ns) const override; + + Status DropNamespace(const Namespace& ns) override; + + Result<bool> NamespaceExists(const Namespace& ns) const override; + + Status UpdateNamespaceProperties( + const Namespace& ns, const std::unordered_map<std::string, std::string>& updates, + const std::unordered_set<std::string>& removals) override; + + Result<std::vector<TableIdentifier>> ListTables(const Namespace& ns) const override; + + 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; + + 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; + + 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; + + Result<bool> TableExists(const TableIdentifier& identifier) const override; + + Status RenameTable(const TableIdentifier& from, const TableIdentifier& to) override; + + Status DropTable(const TableIdentifier& identifier, bool purge) override; + + Result<std::unique_ptr<Table>> LoadTable(const TableIdentifier& identifier) override; + + Result<std::shared_ptr<Table>> RegisterTable( + const TableIdentifier& identifier, + const std::string& metadata_file_location) override; + + std::unique_ptr<RestCatalog::TableBuilder> BuildTable( + const TableIdentifier& identifier, const Schema& schema) const override; + + private: + RestCatalog(std::unique_ptr<RestCatalogConfig> config, + std::unique_ptr<HttpClient> client, ResourcePaths paths); Review Comment: Two comments: 1. client should be `std::shared_ptr<HttpClient>` because we may want to support reusing. 2. `ResourcePaths` should be created in it so remove it from ctor. ctor of ResourcePaths should not have any error. If you are worried about any error, consider use an empty private ctor of RestCatalog and add `RestCatalog::Init` to deal with errors. ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,185 @@ +/* + * 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 <unordered_map> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/http_response.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<RestCatalog>> RestCatalog::Make(const RestCatalogConfig& config) { + // Create ResourcePaths and validate URI + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + + auto tmp_client = std::make_unique<HttpClient>(config); + const std::string endpoint = paths->V1Config(); + ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, + tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); + ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); + // Merge server config into client config, server config overrides > client config Review Comment: Add an utility function for merge configs to rest_util.h ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,185 @@ +/* + * 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 <unordered_map> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/http_response.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<RestCatalog>> RestCatalog::Make(const RestCatalogConfig& config) { + // Create ResourcePaths and validate URI + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + + auto tmp_client = std::make_unique<HttpClient>(config); + const std::string endpoint = paths->V1Config(); + ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, + tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); Review Comment: Please use singleton for error handlers. ########## src/iceberg/catalog/rest/http_client.cc: ########## @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/http_client.h" + +#include <nlohmann/json.hpp> + +#include "cpr/body.h" +#include "cpr/cprtypes.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/json_internal.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +namespace { + +/// \brief Merges global default headers with request-specific headers. +/// +/// Combines the global headers derived from RestCatalogConfig with the headers +/// passed in the specific request. Request-specific headers have higher priority +/// and will override global defaults if the keys conflict (e.g., overriding +/// the default "Content-Type"). +cpr::Header MergeHeaders(const std::unordered_map<std::string, std::string>& defaults, + const std::unordered_map<std::string, std::string>& overrides) { + cpr::Header combined_headers = {defaults.begin(), defaults.end()}; + for (const auto& [key, val] : overrides) { + combined_headers.insert_or_assign(key, val); + } + return combined_headers; +} + +/// \brief Converts a map of string key-value pairs to cpr::Parameters. +cpr::Parameters GetParameters( + const std::unordered_map<std::string, std::string>& params) { + cpr::Parameters cpr_params; + for (const auto& [key, val] : params) { + cpr_params.Add({key, val}); + } + return cpr_params; +} + +bool IsSuccessful(int32_t status_code) { + return status_code == 200 // OK + || status_code == 202 // Accepted + || status_code == 204 // No Content + || status_code == 304; // Not Modified +} + +Status HandleFailureResponse(const cpr::Response& response, + const ErrorHandler& error_handler) { + if (!IsSuccessful(response.status_code)) { + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text)); + ICEBERG_ASSIGN_OR_RAISE(auto error_response, ErrorResponseFromJson(json)); + return error_handler.Accept(error_response.error); + } + return {}; +} + +} // namespace + +void HttpClient::PrepareSession( + const std::string& path, + const std::unordered_map<std::string, std::string>& request_headers, + const std::unordered_map<std::string, std::string>& params) { + session_->SetUrl(cpr::Url{path}); + session_->SetParameters(GetParameters(params)); + session_->RemoveContent(); + auto final_headers = MergeHeaders(default_headers_, request_headers); + session_->SetHeader(final_headers); +} + +HttpClient::HttpClient(const RestCatalogConfig& config) Review Comment: I don't think it is a good practice to let http client know the `RestCatalogConfig`. ########## src/iceberg/catalog/rest/http_response.h: ########## @@ -0,0 +1,50 @@ +/* + * 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 <string> +#include <unordered_map> + +#include "cpr/response.h" Review Comment: To address this, we can use pimpl idiom: ```cpp // Header file class HttpResponse { public: int32_t status_code() const; std::string_view body() const; private: class Impl; std::unique_ptr<Impl> impl_; }; // Source file class HttpResponse::Impl { public: Impl(cpr::Response&& response) : response_(std::move(response)) {} ~Impl() noexcept= default; int32_t status_code() const { return static_cast<int32_t>(response_.status_code); } std::string_view body() const { return response_.text; } private: cpr::Response response_; }; int32_t HttpResponse::status_code() const { return impl_->status_code(); } std::string_view HttpResponse::body() const { return impl_->body(); } ``` ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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 to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). Review Comment: Are you sure this is `RFC 3986`? I found that the Java references to `RFC 2396`. ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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. Review Comment: BTW, this function can be general enough so we don't need to mention `uri`. ########## src/iceberg/catalog/rest/http_response.h: ########## @@ -0,0 +1,50 @@ +/* + * 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 <string> +#include <unordered_map> + +#include "cpr/response.h" Review Comment: We cannot include cpr header in any non-internal header. ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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. Review Comment: > Trim a single trailing slash No, we should use a loop to trim all trailing slashes. ########## src/iceberg/catalog/rest/config.h: ########## @@ -0,0 +1,68 @@ +/* + * 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 <string_view> +#include <unordered_map> + +#include <cpr/cprtypes.h> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/util/config.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration class for a REST Catalog. +class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase<RestCatalogConfig> { + public: + template <typename T> + using Entry = const ConfigBase<RestCatalogConfig>::Entry<T>; + + /// \brief The URI of the REST catalog server. + inline static std::string_view kUri{"uri"}; + + /// \brief The name of the catalog. + inline static std::string_view kName{"name"}; + + /// \brief The warehouse path. + inline static std::string_view kWarehouse{"warehouse"}; + + /// \brief Create a default RestCatalogConfig instance. + static std::unique_ptr<RestCatalogConfig> DefaultProperties(); + + /// \brief Create a RestCatalogConfig instance from a map of key-value pairs. + static std::unique_ptr<RestCatalogConfig> FromMap( + const std::unordered_map<std::string, std::string>& properties); + + /// \brief Generates extra HTTP headers to be added to every request from the + /// configuration. + /// + /// This includes default headers like Content-Type, User-Agent, X-Client-Version and + /// any custom headers prefixed with "header." in the properties. + /// \return A map of header names to values. + std::unordered_map<std::string, std::string> GetExtraHeaders() const; Review Comment: ```suggestion /// \brief Returns HTTP headers to be added to every request. /// /// This includes any key prefixed with "header." in the properties. /// \return A map of headers with the prefix removed from the keys. std::unordered_map<std::string, std::string> ExtractHeaders() const; ``` We should only extract and return headers specified in this config. Adding extra headers should be done in the other places with clearer context. ########## src/iceberg/catalog/rest/http_client.h: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <memory> +#include <mutex> +#include <string> +#include <unordered_map> + +#include <cpr/cpr.h> Review Comment: After removing this header, you need to add - forward declaration of `cpr::Session` - add`~HttpClient()` to this file - add `HttpClient::~HttpClient() = default;` to the source file. ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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 to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). +/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are +/// percent-encoded using uppercase hexadecimal (e.g., space -> "%20"). +/// \param value The string to encode. +/// \return The encoded string. +inline std::string EncodeUriComponent(std::string_view value) { Review Comment: ```suggestion inline std::string EncodeString(std::string_view value) { ``` I think this will be reused by several places in addition to url. ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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 to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). Review Comment: The Java impl uses `URLEncoder.encode(toEncode, StandardCharsets.UTF_8)`. According to its javadoc, I think the comment here should be `Translates a string into application/x-www-form-urlencoded format using UTF-8`. ########## src/iceberg/catalog/rest/endpoint_util.h: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <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 to trim. +/// \return The (possibly) trimmed URI string. +inline std::string TrimTrailingSlash(std::string uri_sv) { + if (!uri_sv.empty() && uri_sv.back() == '/') { + uri_sv.pop_back(); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). +/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are +/// percent-encoded using uppercase hexadecimal (e.g., space -> "%20"). +/// \param value The string to encode. +/// \return The encoded string. +inline std::string EncodeUriComponent(std::string_view value) { + std::string escaped; + escaped.reserve(value.length()); + for (const unsigned char c : value) { + if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { + escaped += c; + } else { + std::format_to(std::back_inserter(escaped), "%{:02X}", c); + } + } + return escaped; +} + +/// \brief Encode a Namespace into a URL-safe component. +/// \details Joins \p ns.levels with the ASCII Unit Separator (0x1F), then percent-encodes +/// the result via EncodeUriComponent. Returns an empty string if there are no levels. +/// \param ns The namespace (sequence of path-like levels) to encode. +/// \return The percent-encoded namespace string suitable for URLs. +inline std::string EncodeNamespaceForUrl(const Namespace& ns) { + if (ns.levels.empty()) { + return ""; + } + + std::string joined_string; + joined_string.append(ns.levels.front()); + for (size_t i = 1; i < ns.levels.size(); ++i) { + joined_string.append("\x1F"); + joined_string.append(ns.levels[i]); + } + + return EncodeUriComponent(joined_string); Review Comment: Is this correct? Shouldn't we encode each level before concatenating them? BTW, this is a good reason to prove `EncodeString` might be a better name. ########## src/iceberg/catalog/rest/config.cc: ########## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/config.h" + +#include "iceberg/catalog/rest/constant.h" + +namespace iceberg::rest { + +std::unique_ptr<RestCatalogConfig> RestCatalogConfig::DefaultProperties() { + return std::make_unique<RestCatalogConfig>(); Review Comment: Make it ctor private. ########## src/iceberg/catalog/rest/error_handlers.h: ########## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <format> +#include <string> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/error_handlers.h +/// Error handlers for different HTTP error types in Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Error handler interface for processing REST API error responses. Maps HTTP +/// status codes to appropriate ErrorKind values following the Iceberg REST specification. +class ICEBERG_REST_EXPORT ErrorHandler { + public: + virtual ~ErrorHandler() = default; + + /// \brief Process an error response and return an appropriate Error. + /// + /// \param error The error model parsed from the HTTP response body + /// \return An Error object with appropriate ErrorKind and message + virtual Status Accept(const ErrorModel& error) const = 0; Review Comment: ```suggestion virtual Status Accept(const ErrorResponse& error) const = 0; ``` What about removing `ErrorModel` as the inner layer and directly using `ErrorResponse`? This is independent and can be a separate PR before checking in this one. ########## src/iceberg/result.h: ########## @@ -42,13 +44,19 @@ enum class ErrorKind { kInvalidManifestList, kIOError, kJsonParseError, + kNamespaceNotEmpty, kNoSuchNamespace, kNoSuchTable, + kNoSuchView, kNotAllowed, + kNotAuthorized, kNotFound, kNotImplemented, kNotSupported, + kServiceFailure, + kServiceUnavailable, kUnknownError, + kRESTError, Review Comment: ```suggestion kRestError, ``` ########## src/iceberg/catalog/rest/error_handlers.h: ########## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <format> +#include <string> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/error_handlers.h +/// Error handlers for different HTTP error types in Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Error handler interface for processing REST API error responses. Maps HTTP +/// status codes to appropriate ErrorKind values following the Iceberg REST specification. +class ICEBERG_REST_EXPORT ErrorHandler { + public: + virtual ~ErrorHandler() = default; + + /// \brief Process an error response and return an appropriate Error. + /// + /// \param error The error model parsed from the HTTP response body + /// \return An Error object with appropriate ErrorKind and message + virtual Status Accept(const ErrorModel& error) const = 0; +}; + +/// \brief Default error handler for REST API responses. +class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 400: + if (error.type == "IllegalArgumentException") { + return InvalidArgument("{}", error.message); + } + return BadRequest("Malformed request: {}", error.message); + + case 401: + return NotAuthorized("Not authorized: {}", error.message); + + case 403: + return Forbidden("Forbidden: {}", error.message); + + case 405: + case 406: + break; + + case 500: + return ServiceFailure("Server error: {}: {}", error.type, error.message); + + case 501: + return NotSupported("{}", error.message); + + case 503: + return ServiceUnavailable("Service unavailable: {}", error.message); + } + return RESTError("Unable to process: {}", error.message); + } +}; + +/// \brief Namespace-specific error handler for create/read/update operations. +class ICEBERG_REST_EXPORT NamespaceErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 400: + if (error.type == "NamespaceNotEmptyException") { + return NamespaceNotEmpty("{}", error.message); + } + return BadRequest("Malformed request: {}", error.message); + + case 404: + return NoSuchNamespace("{}", error.message); + + case 409: + return AlreadyExists("{}", error.message); + + case 422: + return RESTError("Unable to process: {}", error.message); + } + + return DefaultErrorHandler::Accept(error); + } +}; + +/// \brief Error handler for drop namespace operations. +class ICEBERG_REST_EXPORT DropNamespaceErrorHandler : public NamespaceErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + if (error.code == 409) { + return NamespaceNotEmpty("{}", error.message); + } + + // Delegate to parent handler + return NamespaceErrorHandler::Accept(error); + } +}; + +/// \brief Table-level error handler. +class ICEBERG_REST_EXPORT TableErrorHandler : public DefaultErrorHandler { + public: + Status Accept(const ErrorModel& error) const override { + switch (error.code) { + case 404: + if (error.type == "NoSuchNamespaceException") { Review Comment: nit: define `kNoSuchNamespaceException` as the magic string to the top? ########## src/iceberg/catalog/rest/resource_paths.cc: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/resource_paths.h" + +#include <format> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/result.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<ResourcePaths>> ResourcePaths::Make( + const RestCatalogConfig& config) { + // Validate and extract URI + auto it = config.configs().find(std::string(RestCatalogConfig::kUri)); + if (it == config.configs().end() || it->second.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + + std::string base_uri = std::string(TrimTrailingSlash(it->second)); + std::string prefix; + auto prefix_it = config.configs().find("prefix"); + if (prefix_it != config.configs().end() && !prefix_it->second.empty()) { + prefix = prefix_it->second; + } + + return std::unique_ptr<ResourcePaths>( + new ResourcePaths(std::move(base_uri), std::move(prefix))); +} + +ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) + : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} + +std::string ResourcePaths::BuildPath(std::string_view path) const { + if (prefix_.empty()) { + return std::format("{}/v1/{}", base_uri_, path); + } + return std::format("{}/v1/{}/{}", base_uri_, prefix_, path); +} + +std::string ResourcePaths::V1Config() const { + return std::format("{}/v1/config", base_uri_); +} + +std::string ResourcePaths::V1OAuth2Tokens() const { + return std::format("{}/v1/oauth/tokens", base_uri_); +} + +std::string ResourcePaths::V1Namespaces() const { return BuildPath("namespaces"); } + +std::string ResourcePaths::V1Namespace(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}", EncodeNamespaceForUrl(ns))); Review Comment: This under the hood creates strings twice since you have two std::format calls. I'd suggest removing `BuildPath`. ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + std::string V1Table(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. + std::string V1RegisterTable(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/tables/rename endpoint path. + std::string V1RenameTable() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint + /// path. + std::string V1TableMetrics(const TableIdentifier& table) const; Review Comment: ```suggestion std::string Metrics(const TableIdentifier& table) const; ``` ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + std::string V1Table(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. + std::string V1RegisterTable(const Namespace& ns) const; Review Comment: ```suggestion std::string Register(const Namespace& ns) const; ``` ########## src/iceberg/catalog/rest/resource_paths.cc: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/resource_paths.h" + +#include <format> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/result.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<ResourcePaths>> ResourcePaths::Make( + const RestCatalogConfig& config) { + // Validate and extract URI + auto it = config.configs().find(std::string(RestCatalogConfig::kUri)); + if (it == config.configs().end() || it->second.empty()) { + return InvalidArgument("Rest catalog configuration property 'uri' is required."); + } + + std::string base_uri = std::string(TrimTrailingSlash(it->second)); + std::string prefix; + auto prefix_it = config.configs().find("prefix"); + if (prefix_it != config.configs().end() && !prefix_it->second.empty()) { + prefix = prefix_it->second; + } + + return std::unique_ptr<ResourcePaths>( + new ResourcePaths(std::move(base_uri), std::move(prefix))); +} + +ResourcePaths::ResourcePaths(std::string base_uri, std::string prefix) + : base_uri_(std::move(base_uri)), prefix_(std::move(prefix)) {} + +std::string ResourcePaths::BuildPath(std::string_view path) const { + if (prefix_.empty()) { + return std::format("{}/v1/{}", base_uri_, path); + } + return std::format("{}/v1/{}/{}", base_uri_, prefix_, path); +} + +std::string ResourcePaths::V1Config() const { + return std::format("{}/v1/config", base_uri_); +} + +std::string ResourcePaths::V1OAuth2Tokens() const { + return std::format("{}/v1/oauth/tokens", base_uri_); +} + +std::string ResourcePaths::V1Namespaces() const { return BuildPath("namespaces"); } + +std::string ResourcePaths::V1Namespace(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1NamespaceProperties(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/properties", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1Tables(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/tables", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1Table(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}", EncodeNamespaceForUrl(table.ns), + table.name)); +} + +std::string ResourcePaths::V1RegisterTable(const Namespace& ns) const { + return BuildPath(std::format("namespaces/{}/register", EncodeNamespaceForUrl(ns))); +} + +std::string ResourcePaths::V1RenameTable() const { return BuildPath("tables/rename"); } + +std::string ResourcePaths::V1TableMetrics(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/metrics", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TableCredentials(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/credentials", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TableScanPlan(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/plan", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TableScanPlanResult(const TableIdentifier& table, + const std::string& plan_id) const { + return BuildPath(std::format("namespaces/{}/tables/{}/plan/{}", + EncodeNamespaceForUrl(table.ns), table.name, plan_id)); +} + +std::string ResourcePaths::V1TableTasks(const TableIdentifier& table) const { + return BuildPath(std::format("namespaces/{}/tables/{}/tasks", + EncodeNamespaceForUrl(table.ns), table.name)); +} + +std::string ResourcePaths::V1TransactionCommit() const { + return BuildPath("transactions/commit"); +} + +std::string ResourcePaths::V1Views(const Namespace& ns) const { Review Comment: What about removing endpoints for view at this moment? ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + std::string V1Table(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. + std::string V1RegisterTable(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/tables/rename endpoint path. + std::string V1RenameTable() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint + /// path. + std::string V1TableMetrics(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials + /// endpoint path. + std::string V1TableCredentials(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint + /// path. + std::string V1TableScanPlan(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{planId} + /// endpoint path. + std::string V1TableScanPlanResult(const TableIdentifier& table, + const std::string& plan_id) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint + /// path. + std::string V1TableTasks(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/transactions/commit endpoint path. + std::string V1TransactionCommit() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views endpoint path. + std::string V1Views(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/views/{view} endpoint path. + std::string V1View(const TableIdentifier& view) const; + + /// \brief Get the /v1/{prefix}/views/rename endpoint path. + std::string V1RenameView() const; + + private: + explicit ResourcePaths(std::string base_uri, std::string prefix); + + // Helper to build path with optional prefix: {base_uri_}/{prefix_?}/{path} + std::string BuildPath(std::string_view path) const; + + std::string base_uri_; Review Comment: What is the benefit of handling uri here compared to http client? ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,185 @@ +/* + * 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 <unordered_map> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/http_response.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<RestCatalog>> RestCatalog::Make(const RestCatalogConfig& config) { + // Create ResourcePaths and validate URI + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + + auto tmp_client = std::make_unique<HttpClient>(config); + const std::string endpoint = paths->V1Config(); Review Comment: Please add a dedicated function to deal with the config call. ########## src/iceberg/catalog/rest/catalog.h: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <memory> +#include <string> + +#include "iceberg/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/catalog/rest/resource_paths.h" +#include "iceberg/result.h" + +/// \file iceberg/catalog/rest/catalog.h +/// RestCatalog implementation for Iceberg REST API. + +namespace iceberg::rest { + +class ICEBERG_REST_EXPORT RestCatalog : public Catalog { + public: + RestCatalog(const RestCatalog&) = delete; + RestCatalog& operator=(const RestCatalog&) = delete; + RestCatalog(RestCatalog&&) = delete; + RestCatalog& operator=(RestCatalog&&) = delete; + + /// \brief Create a RestCatalog instance + /// + /// \param config the configuration for the RestCatalog + /// \return a unique_ptr to RestCatalog instance + static Result<std::unique_ptr<RestCatalog>> Make(const RestCatalogConfig& config); + + std::string_view name() const override; + + Result<std::vector<Namespace>> ListNamespaces(const Namespace& ns) const override; + + Status CreateNamespace( + const Namespace& ns, + const std::unordered_map<std::string, std::string>& properties) override; + + Result<std::unordered_map<std::string, std::string>> GetNamespaceProperties( + const Namespace& ns) const override; + + Status DropNamespace(const Namespace& ns) override; + + Result<bool> NamespaceExists(const Namespace& ns) const override; + + Status UpdateNamespaceProperties( + const Namespace& ns, const std::unordered_map<std::string, std::string>& updates, + const std::unordered_set<std::string>& removals) override; + + Result<std::vector<TableIdentifier>> ListTables(const Namespace& ns) const override; + + 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; + + 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; + + 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; + + Result<bool> TableExists(const TableIdentifier& identifier) const override; + + Status RenameTable(const TableIdentifier& from, const TableIdentifier& to) override; + + Status DropTable(const TableIdentifier& identifier, bool purge) override; + + Result<std::unique_ptr<Table>> LoadTable(const TableIdentifier& identifier) override; + + Result<std::shared_ptr<Table>> RegisterTable( + const TableIdentifier& identifier, + const std::string& metadata_file_location) override; + + std::unique_ptr<RestCatalog::TableBuilder> BuildTable( Review Comment: This has been removed. You need to rebase on the latest main branch. ########## src/iceberg/catalog/rest/config.h: ########## @@ -0,0 +1,68 @@ +/* + * 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 <string_view> +#include <unordered_map> + +#include <cpr/cprtypes.h> + +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/util/config.h" + +/// \file iceberg/catalog/rest/config.h +/// \brief RestCatalogConfig implementation for Iceberg REST API. + +namespace iceberg::rest { + +/// \brief Configuration class for a REST Catalog. +class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase<RestCatalogConfig> { + public: + template <typename T> + using Entry = const ConfigBase<RestCatalogConfig>::Entry<T>; + + /// \brief The URI of the REST catalog server. + inline static std::string_view kUri{"uri"}; Review Comment: ```suggestion inline static Entry<std::string> kUri{"uri", ""}; ``` After scanning through this PR, I think making it as an entry will be much easier to use. Same for other keys below. ########## src/iceberg/catalog/rest/http_client.h: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <memory> +#include <mutex> +#include <string> +#include <unordered_map> + +#include <cpr/cpr.h> Review Comment: ditto ########## src/iceberg/catalog/rest/endpoint_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) { + if (uri_sv.ends_with('/')) { + uri_sv.remove_suffix(1); + } + return uri_sv; +} + +/// \brief Percent-encode a string as a URI component (RFC 3986). +/// \details Leaves unreserved characters [A–Z a–z 0–9 - _ . ~] as-is; all others are +/// percent-encoded +/// using uppercase hexadecimal (e.g., space -> "%20"). +/// \param value The string to encode. +/// \return The encoded string. +inline std::string EncodeUriComponent(std::string_view value) { + std::string escaped; + escaped.reserve(value.length()); + for (const unsigned char c : value) { + if (isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { + escaped += c; + } else { + std::format_to(std::back_inserter(escaped), "{:02X}", c); + } + } + return escaped; +} + +/// \brief Encode a Namespace into a URL-safe component. +/// \details Joins \p ns.levels with the ASCII Unit Separator (0x1F), then percent-encodes +/// the result +/// via EncodeUriComponent. Returns an empty string if there are no levels. +/// \param ns The namespace (sequence of path-like levels) to encode. +/// \return The percent-encoded namespace string suitable for URLs. +inline std::string EncodeNamespaceForUrl(const Namespace& ns) { Review Comment: +1 For each EncodeXXX, we need to add DecodeXXX for testing. ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,185 @@ +/* + * 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 <unordered_map> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/http_response.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<RestCatalog>> RestCatalog::Make(const RestCatalogConfig& config) { + // Create ResourcePaths and validate URI + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + + auto tmp_client = std::make_unique<HttpClient>(config); + const std::string endpoint = paths->V1Config(); + ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, + tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler())); + ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); + ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json)); + // Merge server config into client config, server config overrides > client config + // properties > server config defaults + auto final_props = std::move(server_config.defaults); + for (const auto& kv : config.configs()) { + final_props.insert_or_assign(kv.first, kv.second); + } + + for (const auto& kv : server_config.overrides) { + final_props.insert_or_assign(kv.first, kv.second); + } + auto final_config = RestCatalogConfig::FromMap(final_props); + auto client = std::make_unique<HttpClient>(*final_config); + ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config)); + return std::unique_ptr<RestCatalog>(new RestCatalog( + std::move(final_config), std::move(client), std::move(*final_paths))); +} + +RestCatalog::RestCatalog(std::unique_ptr<RestCatalogConfig> config, + std::unique_ptr<HttpClient> client, ResourcePaths paths) + : config_(std::move(config)), client_(std::move(client)), paths_(std::move(paths)) {} + +std::string_view RestCatalog::name() const { + auto it = config_->configs().find(std::string(RestCatalogConfig::kName)); Review Comment: Use config_ to return name directly. ########## src/iceberg/catalog/rest/catalog.cc: ########## @@ -0,0 +1,185 @@ +/* + * 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 <unordered_map> +#include <utility> + +#include <cpr/cpr.h> + +#include "iceberg/catalog/rest/catalog.h" +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/constant.h" +#include "iceberg/catalog/rest/endpoint_util.h" +#include "iceberg/catalog/rest/error_handlers.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/catalog/rest/http_response.h" +#include "iceberg/catalog/rest/json_internal.h" +#include "iceberg/catalog/rest/types.h" +#include "iceberg/json_internal.h" +#include "iceberg/result.h" +#include "iceberg/table.h" +#include "iceberg/util/macros.h" + +namespace iceberg::rest { + +Result<std::unique_ptr<RestCatalog>> RestCatalog::Make(const RestCatalogConfig& config) { + // Create ResourcePaths and validate URI + ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config)); + + auto tmp_client = std::make_unique<HttpClient>(config); + const std::string endpoint = paths->V1Config(); + ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response, Review Comment: ```suggestion ICEBERG_ASSIGN_OR_RAISE(auto response, ``` You can't capture a temporary variable using a reference. This may have lifetime issue. ########## src/iceberg/catalog/rest/resource_paths.h: ########## @@ -0,0 +1,114 @@ +/* + * 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 <string> +#include <string_view> + +#include "iceberg/catalog/rest/config.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" +#include "iceberg/table_identifier.h" + +/// \file iceberg/catalog/rest/resource_paths.h +/// \brief Resource path construction for Iceberg REST API endpoints. + +namespace iceberg::rest { + +/// \brief Resource path builder for Iceberg REST catalog endpoints. +/// +/// This class constructs REST API endpoint URLs for various catalog operations. +class ICEBERG_REST_EXPORT ResourcePaths { + public: + /// \brief Construct a ResourcePaths with REST catalog configuration. + /// \param config The REST catalog configuration containing the base URI + static Result<std::unique_ptr<ResourcePaths>> Make(const RestCatalogConfig& config); + + /// \brief Get the /v1/{prefix}/config endpoint path. + std::string V1Config() const; + + /// \brief Get the /v1/{prefix}/oauth/tokens endpoint path. + std::string V1OAuth2Tokens() const; + + /// \brief Get the /v1/{prefix}/namespaces endpoint path. + std::string V1Namespaces() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace} endpoint path. + std::string V1Namespace(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/properties endpoint path. + std::string V1NamespaceProperties(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables endpoint path. + std::string V1Tables(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table} endpoint path. + std::string V1Table(const TableIdentifier& table) const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/register endpoint path. + std::string V1RegisterTable(const Namespace& ns) const; + + /// \brief Get the /v1/{prefix}/tables/rename endpoint path. + std::string V1RenameTable() const; + + /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics endpoint + /// path. + std::string V1TableMetrics(const TableIdentifier& table) const; Review Comment: Same for below. -- 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]
