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

@@ -20,14 +20,18 @@
 #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/http_client_interal.h"
+#include "iceberg/catalog/rest/error_handlers.h"
+#include "iceberg/catalog/rest/http_client.h"
+#include "iceberg/catalog/rest/http_response.h"
 #include "iceberg/catalog/rest/json_internal.h"
 #include "iceberg/catalog/rest/types.h"
 #include "iceberg/json_internal.h"
@@ -41,37 +45,27 @@ Result<std::unique_ptr<RestCatalog>> RestCatalog::Make(const RestCatalogConfig&
   // Create ResourcePaths and validate URI
   ICEBERG_ASSIGN_OR_RAISE(auto paths, ResourcePaths::Make(config));
 
-  ICEBERG_ASSIGN_OR_RAISE(auto tmp_client, HttpClient::Make(config));
-
+  auto tmp_client = std::make_unique<HttpClient>(config);
   const std::string endpoint = paths->V1Config();
-  cpr::Parameters params;
-  ICEBERG_ASSIGN_OR_RAISE(const auto& response, tmp_client->Get(endpoint, params));
-  switch (response.status_code) {
-    case cpr::status::HTTP_OK: {
-      ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text));
-      ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json));
-      // Merge server config into client config, server config overrides > client config
-      // properties > server config defaults
-      auto final_props = std::move(server_config.defaults);
-      for (const auto& kv : config.configs()) {
-        final_props.insert_or_assign(kv.first, kv.second);
-      }
-
-      for (const auto& kv : server_config.overrides) {
-        final_props.insert_or_assign(kv.first, kv.second);
-      }
-      auto final_config = RestCatalogConfig::FromMap(final_props);
-      ICEBERG_ASSIGN_OR_RAISE(auto client, HttpClient::Make(*final_config));
-      ICEBERG_ASSIGN_OR_RAISE(auto final_paths, ResourcePaths::Make(*final_config));
-      return std::unique_ptr<RestCatalog>(new RestCatalog(
-          std::move(final_config), std::move(client), std::move(*final_paths)));
-    };
-    default: {
-      ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text));
-      ICEBERG_ASSIGN_OR_RAISE(auto list_response, ErrorResponseFromJson(json));
-      return UnknownError("Error listing namespaces: {}", list_response.error.message);
-    }
+  ICEBERG_ASSIGN_OR_RAISE(const HttpResponse& response,
+                          tmp_client->Get(endpoint, {}, {}, DefaultErrorHandler()));
+  ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
+  ICEBERG_ASSIGN_OR_RAISE(auto server_config, CatalogConfigFromJson(json));
+  // Merge server config into client config, server config overrides > client config
+  // properties > server config defaults
+  auto final_props = std::move(server_config.defaults);
+  for (const auto& kv : config.configs()) {
+    final_props.insert_or_assign(kv.first, kv.second);
+  }
+
+  for (const auto& kv : server_config.overrides) {
+    final_props.insert_or_assign(kv.first, kv.second);
   }
+  auto final_config = RestCatalogConfig::FromMap(final_props);
+  auto client = std::make_unique<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,
@@ -91,35 +85,26 @@ Result<std::vector<Namespace>> RestCatalog::ListNamespaces(const Namespace& ns)
   std::vector<Namespace> result;
   std::string next_token;
   while (true) {
-    cpr::Parameters params;
+    std::unordered_map<std::string, std::string> params;
     if (!ns.levels.empty()) {
-      params.Add({std::string(kQueryParamParent), EncodeNamespaceForUrl(ns)});
+      params[kQueryParamParent] = EncodeNamespaceForUrl(ns);
     }
     if (!next_token.empty()) {
-      params.Add({std::string(kQueryParamPageToken), next_token});
+      params[kQueryParamPageToken] = next_token;
     }
-    ICEBERG_ASSIGN_OR_RAISE(const auto& response, client_->Get(endpoint, params));
-    switch (response.status_code) {
-      case cpr::status::HTTP_OK: {
-        ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text));
-        ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json));
-        result.insert(result.end(), list_response.namespaces.begin(),
-                      list_response.namespaces.end());
-        if (list_response.next_page_token.empty()) {
-          return result;
-        }
-        next_token = list_response.next_page_token;
-        continue;
-      }
-      case cpr::status::HTTP_NOT_FOUND: {
-        return NoSuchNamespace("Namespace not found");
-      }
-      default:
-        ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text));
-        ICEBERG_ASSIGN_OR_RAISE(auto list_response, ErrorResponseFromJson(json));
-        return UnknownError("Error listing namespaces: {}", list_response.error.message);
+    ICEBERG_ASSIGN_OR_RAISE(const auto& response,
+                            client_->Get(endpoint, params, {}, NamespaceErrorHandler()));
+    ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
+    ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json));
+    result.insert(result.end(), list_response.namespaces.begin(),
+                  list_response.namespaces.end());
+    if (list_response.next_page_token.empty()) {
+      return result;
     }
+    next_token = list_response.next_page_token;
+    continue;
   }
+  return result;
 }
 
 Status RestCatalog::CreateNamespace(

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/config.cc

@@ -23,7 +23,7 @@
 
 namespace iceberg::rest {
 
-std::unique_ptr<RestCatalogConfig> RestCatalogConfig::default_properties() {
+std::unique_ptr<RestCatalogConfig> RestCatalogConfig::DefaultProperties() {
   return std::make_unique<RestCatalogConfig>();
 }
 

src/iceberg/catalog/rest/config.h

@@ -50,7 +50,7 @@ class ICEBERG_REST_EXPORT RestCatalogConfig : public ConfigBase<RestCatalogConfi
   inline static std::string_view kWarehouse{"warehouse"};
 
   /// \brief Create a default RestCatalogConfig instance.
-  static std::unique_ptr<RestCatalogConfig> default_properties();
+  static std::unique_ptr<RestCatalogConfig> DefaultProperties();
 
   /// \brief Create a RestCatalogConfig instance from a map of key-value pairs.
   static std::unique_ptr<RestCatalogConfig> FromMap(

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;
 }

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") {
+          return NoSuchNamespace("{}", error.message);
+        }
+        return NoSuchTable("{}", error.message);
+
+      case 409:
+        return AlreadyExists("{}", error.message);
+    }
+
+    return DefaultErrorHandler::Accept(error);
+  }
+};
+
+/// \brief View-level error handler.
+///
+/// Handles view-specific errors including NoSuchView, NoSuchNamespace,
+/// and AlreadyExists scenarios.
+class ICEBERG_REST_EXPORT ViewErrorHandler : public DefaultErrorHandler {
+ public:
+  Status Accept(const ErrorModel& error) const override {
+    switch (error.code) {
+      case 404:
+        if (error.type == "NoSuchNamespaceException") {
+          return NoSuchNamespace("{}", error.message);
+        }
+        return NoSuchView("{}", error.message);
+
+      case 409:
+        return AlreadyExists("{}", error.message);
+    }
+
+    return DefaultErrorHandler::Accept(error);
+  }
+};
+
+/// \brief Table commit operation error handler.
+class ICEBERG_REST_EXPORT TableCommitErrorHandler : public DefaultErrorHandler {
+ public:
+  Status Accept(const ErrorModel& error) const override {
+    switch (error.code) {
+      case 404:
+        return NoSuchTable("{}", error.message);
+
+      case 409:
+        return CommitFailed("Commit failed: {}", error.message);
+
+      case 500:
+      case 502:
+      case 503:
+      case 504:
+        return CommitStateUnknown("Service failed: {}: {}", error.code, error.message);
+    }
+
+    return DefaultErrorHandler::Accept(error);
+  }
+};
+
+/// \brief View commit operation error handler.
+class ICEBERG_REST_EXPORT ViewCommitErrorHandler : public DefaultErrorHandler {
+ public:
+  Status Accept(const ErrorModel& error) const override {
+    switch (error.code) {
+      case 404:
+        return NoSuchView("{}", error.message);
+
+      case 409:
+        return CommitFailed("Commit failed: {}", error.message);
+
+      case 500:
+      case 502:
+      case 503:
+      case 504:
+        return CommitStateUnknown("Service failed: {}: {}", error.code, error.message);
+    }
+
+    return DefaultErrorHandler::Accept(error);
+  }
+};
+
+}  // namespace iceberg::rest