update

Author: HeartLinked

src/iceberg/catalog/rest/CMakeLists.txt

@@ -19,7 +19,7 @@ set(ICEBERG_REST_SOURCES
     catalog.cc
     json_internal.cc
     config.cc
-    http_client_internal.cc
+    http_client.cc
     resource_paths.cc)
 
 set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS)

src/iceberg/catalog/rest/catalog.cc

@@ -30,7 +30,7 @@
 #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_interal.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"

src/iceberg/catalog/rest/catalog.h

@@ -24,7 +24,7 @@
 
 #include "iceberg/catalog.h"
 #include "iceberg/catalog/rest/config.h"
-#include "iceberg/catalog/rest/http_client_interal.h"
+#include "iceberg/catalog/rest/http_client.h"
 #include "iceberg/catalog/rest/iceberg_rest_export.h"
 #include "iceberg/catalog/rest/resource_paths.h"
 #include "iceberg/result.h"

src/iceberg/catalog/rest/endpoint_util.h

@@ -31,11 +31,11 @@ namespace iceberg::rest {
 /// \brief Trim a single trailing slash from a URI string_view.
 /// \details If \p uri_sv ends with '/', remove that last character; otherwise the input
 /// is returned unchanged.
-/// \param uri_sv The URI string view to trim.
-/// \return The (possibly) trimmed URI string view.
-inline std::string_view TrimTrailingSlash(std::string_view uri_sv) {
-  if (uri_sv.ends_with('/')) {
-    uri_sv.remove_suffix(1);
+/// \param uri_sv The URI string to trim.
+/// \return The (possibly) trimmed URI string.
+inline std::string TrimTrailingSlash(std::string uri_sv) {
+  if (!uri_sv.empty() && uri_sv.back() == '/') {
+    uri_sv.pop_back();
   }
   return uri_sv;
 }

…erg/catalog/rest/http_client_internal.cc src/iceberg/catalog/rest/http_client.ccsrc/iceberg/catalog/rest/http_client_internal.cc renamed to src/iceberg/catalog/rest/http_client.cc src/iceberg/catalog/rest/http_client_internal.cc renamed to src/iceberg/catalog/rest/http_client.cc

@@ -17,12 +17,13 @@
  * 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/http_client_interal.h"
 #include "iceberg/catalog/rest/json_internal.h"
 #include "iceberg/json_internal.h"
 #include "iceberg/util/macros.h"
@@ -94,6 +95,8 @@ Result<HttpResponse> HttpClient::Get(
     const std::string& path, const std::unordered_map<std::string, std::string>& params,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler) {
+  std::lock_guard<std::mutex> lock(session_mutex_);
+
   PrepareSession(path, headers, params);
   cpr::Response response = session_->Get();
   ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler));
@@ -104,6 +107,8 @@ Result<HttpResponse> HttpClient::Post(
     const std::string& path, const std::string& body,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler) {
+  std::lock_guard<std::mutex> lock(session_mutex_);
+
   PrepareSession(path, headers);
   session_->SetBody(cpr::Body{body});
   cpr::Response response = session_->Post();
@@ -116,6 +121,8 @@ Result<HttpResponse> HttpClient::PostForm(
     const std::unordered_map<std::string, std::string>& form_data,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler) {
+  std::lock_guard<std::mutex> lock(session_mutex_);
+
   PrepareSession(path, headers);
   std::vector<cpr::Pair> pair_list;
   pair_list.reserve(form_data.size());
@@ -131,6 +138,8 @@ Result<HttpResponse> HttpClient::PostForm(
 Result<HttpResponse> HttpClient::Head(
     const std::string& path, const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler) {
+  std::lock_guard<std::mutex> lock(session_mutex_);
+
   PrepareSession(path, headers);
   cpr::Response response = session_->Head();
   ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler));
@@ -140,6 +149,8 @@ Result<HttpResponse> HttpClient::Head(
 Result<HttpResponse> HttpClient::Delete(
     const std::string& path, const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler) {
+  std::lock_guard<std::mutex> lock(session_mutex_);
+
   PrepareSession(path, headers);
   cpr::Response response = session_->Delete();
   ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler));

…eberg/catalog/rest/http_client_interal.h src/iceberg/catalog/rest/http_client.hsrc/iceberg/catalog/rest/http_client_interal.h renamed to src/iceberg/catalog/rest/http_client.h src/iceberg/catalog/rest/http_client_interal.h renamed to src/iceberg/catalog/rest/http_client.h

@@ -45,8 +45,8 @@ class ICEBERG_REST_EXPORT HttpClient {
 
   HttpClient(const HttpClient&) = delete;
   HttpClient& operator=(const HttpClient&) = delete;
-  HttpClient(HttpClient&&) = default;
-  HttpClient& operator=(HttpClient&&) = default;
+  HttpClient(HttpClient&&) = delete;
+  HttpClient& operator=(HttpClient&&) = delete;
 
   /// \brief Sends a GET request.
   Result<HttpResponse> Get(const std::string& path,
@@ -82,6 +82,12 @@ class ICEBERG_REST_EXPORT HttpClient {
                       const std::unordered_map<std::string, std::string>& params = {});
 
   std::unordered_map<std::string, std::string> default_headers_;
+
+  // Mutex to protect the non-thread-safe cpr::Session.
+  mutable std::mutex session_mutex_;
+
+  // TODO(Li Feiyang): use connection pool to support external multi-threaded concurrent
+  // calls
   std::unique_ptr<cpr::Session> session_;
 };
 

src/iceberg/catalog/rest/meson.build

@@ -18,7 +18,7 @@
 iceberg_rest_sources = files(
     'catalog.cc',
     'config.cc',
-    'http_client_internal.cc',
+    'http_client.cc',
     'json_internal.cc',
     'resource_path.cc',
 )
@@ -57,11 +57,14 @@ install_headers(
         'catalog.h',
         'config.h',
         'constant.h',
-        'http_client_interal.h',
+        'http_client.h',
+        'http_response.h',
         'json_internal.h',
+        'iceberg_rest_export.h',
         'types.h',
-        'util.h',
         'resource_paths.h',
+        'endpoint_util.h',
+        'error_handlers.h',
     ],
     subdir: 'iceberg/catalog/rest',
 )

src/iceberg/catalog/rest/types.h

@@ -19,7 +19,6 @@
 
 #pragma once
 
-#include <algorithm>
 #include <format>
 #include <memory>
 #include <string>

src/iceberg/test/rest_catalog_test.cc

@@ -61,7 +61,7 @@ class RestCatalogTest : public ::testing::Test {
   RestCatalogConfig config_;
 };
 
-TEST_F(RestCatalogTest, DISABLED_MakeCatalogSuccess) {
+TEST_F(RestCatalogTest, MakeCatalogSuccess) {
   auto catalog_result = RestCatalog::Make(config_);
   EXPECT_THAT(catalog_result, IsOk());
 
@@ -82,7 +82,7 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogEmptyUri) {
   EXPECT_THAT(catalog_result, HasErrorMessage("uri"));
 }
 
-TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) {
+TEST_F(RestCatalogTest, MakeCatalogWithCustomProperties) {
   RestCatalogConfig custom_config = config_;
   custom_config
       .Set(RestCatalogConfig::Entry<std::string>{"custom_prop", ""},
@@ -93,7 +93,7 @@ TEST_F(RestCatalogTest, DISABLED_MakeCatalogWithCustomProperties) {
   EXPECT_THAT(catalog_result, IsOk());
 }
 
-TEST_F(RestCatalogTest, DISABLED_ListNamespaces) {
+TEST_F(RestCatalogTest, ListNamespaces) {
   auto catalog_result = RestCatalog::Make(config_);
   ASSERT_THAT(catalog_result, IsOk());
   auto& catalog = catalog_result.value();