refactor: Move validation logic to struct member methods

Author: HeartLinked

src/iceberg/CMakeLists.txt

@@ -50,6 +50,7 @@ set(ICEBERG_SOURCES
     sort_order.cc
     statistics_file.cc
     table.cc
+    table_identifier.cc
     table_metadata.cc
     table_properties.cc
     table_requirement.cc

src/iceberg/catalog/rest/CMakeLists.txt

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set(ICEBERG_REST_SOURCES rest_catalog.cc json_internal.cc validator.cc)
+set(ICEBERG_REST_SOURCES rest_catalog.cc json_internal.cc types.cc)
 
 set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS)
 set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS)

src/iceberg/catalog/rest/json_internal.cc

@@ -26,7 +26,6 @@
 #include <nlohmann/json.hpp>
 
 #include "iceberg/catalog/rest/types.h"
-#include "iceberg/catalog/rest/validator.h"
 #include "iceberg/json_internal.h"
 #include "iceberg/table_identifier.h"
 #include "iceberg/util/json_util_internal.h"
@@ -88,7 +87,7 @@ Result<CatalogConfig> CatalogConfigFromJson(const nlohmann::json& json) {
   ICEBERG_ASSIGN_OR_RAISE(
       config.endpoints,
       GetJsonValueOrDefault<std::vector<std::string>>(json, kEndpoints));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(config));
+  ICEBERG_RETURN_UNEXPECTED(config.Validate());
   return config;
 }
 
@@ -111,7 +110,7 @@ Result<ErrorModel> ErrorModelFromJson(const nlohmann::json& json) {
   ICEBERG_ASSIGN_OR_RAISE(error.code, GetJsonValue<uint32_t>(json, kCode));
   ICEBERG_ASSIGN_OR_RAISE(error.stack,
                           GetJsonValueOrDefault<std::vector<std::string>>(json, kStack));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(error));
+  ICEBERG_RETURN_UNEXPECTED(error.Validate());
   return error;
 }
 
@@ -125,7 +124,7 @@ Result<ErrorResponse> ErrorResponseFromJson(const nlohmann::json& json) {
   ErrorResponse response;
   ICEBERG_ASSIGN_OR_RAISE(auto error_json, GetJsonValue<nlohmann::json>(json, kError));
   ICEBERG_ASSIGN_OR_RAISE(response.error, ErrorModelFromJson(error_json));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(response));
+  ICEBERG_RETURN_UNEXPECTED(response.Validate());
   return response;
 }
 
@@ -144,7 +143,7 @@ Result<CreateNamespaceRequest> CreateNamespaceRequestFromJson(
   ICEBERG_ASSIGN_OR_RAISE(
       request.properties,
       GetJsonValueOrDefault<decltype(request.properties)>(json, kProperties));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request));
+  ICEBERG_RETURN_UNEXPECTED(request.Validate());
   return request;
 }
 
@@ -162,7 +161,7 @@ Result<UpdateNamespacePropertiesRequest> UpdateNamespacePropertiesRequestFromJso
       request.removals, GetJsonValueOrDefault<std::vector<std::string>>(json, kRemovals));
   ICEBERG_ASSIGN_OR_RAISE(
       request.updates, GetJsonValueOrDefault<decltype(request.updates)>(json, kUpdates));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request));
+  ICEBERG_RETURN_UNEXPECTED(request.Validate());
   return request;
 }
 
@@ -183,7 +182,7 @@ Result<RegisterTableRequest> RegisterTableRequestFromJson(const nlohmann::json&
                           GetJsonValue<std::string>(json, kMetadataLocation));
   ICEBERG_ASSIGN_OR_RAISE(request.overwrite,
                           GetJsonValueOrDefault<bool>(json, kOverwrite, false));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request));
+  ICEBERG_RETURN_UNEXPECTED(request.Validate());
   return request;
 }
 
@@ -201,7 +200,7 @@ Result<RenameTableRequest> RenameTableRequestFromJson(const nlohmann::json& json
   ICEBERG_ASSIGN_OR_RAISE(auto dest_json,
                           GetJsonValue<nlohmann::json>(json, kDestination));
   ICEBERG_ASSIGN_OR_RAISE(request.destination, TableIdentifierFromJson(dest_json));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request));
+  ICEBERG_RETURN_UNEXPECTED(request.Validate());
   return request;
 }
 
@@ -248,7 +247,7 @@ Result<ListNamespacesResponse> ListNamespacesResponseFromJson(
     ICEBERG_ASSIGN_OR_RAISE(auto ns, NamespaceFromJson(ns_json));
     response.namespaces.push_back(std::move(ns));
   }
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(response));
+  ICEBERG_RETURN_UNEXPECTED(response.Validate());
   return response;
 }
 
@@ -304,7 +303,7 @@ Result<UpdateNamespacePropertiesResponse> UpdateNamespacePropertiesResponseFromJ
       response.removed, GetJsonValueOrDefault<std::vector<std::string>>(json, kRemoved));
   ICEBERG_ASSIGN_OR_RAISE(
       response.missing, GetJsonValueOrDefault<std::vector<std::string>>(json, kMissing));
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(response));
+  ICEBERG_RETURN_UNEXPECTED(response.Validate());
   return response;
 }
 
@@ -329,7 +328,7 @@ Result<ListTablesResponse> ListTablesResponseFromJson(const nlohmann::json& json
     ICEBERG_ASSIGN_OR_RAISE(auto identifier, TableIdentifierFromJson(id_json));
     response.identifiers.push_back(std::move(identifier));
   }
-  ICEBERG_RETURN_UNEXPECTED(Validator::Validate(response));
+  ICEBERG_RETURN_UNEXPECTED(response.Validate());
   return response;
 }
 

src/iceberg/catalog/rest/meson.build

@@ -15,11 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
-iceberg_rest_sources = files(
-    'json_internal.cc',
-    'rest_catalog.cc',
-    'validator.cc',
-)
+iceberg_rest_sources = files('json_internal.cc', 'rest_catalog.cc', 'types.cc')
 # cpr does not export symbols, so on Windows it must
 # be used as a static lib
 cpr_needs_static = (
@@ -51,6 +47,6 @@ meson.override_dependency('iceberg-rest', iceberg_rest_dep)
 pkg.generate(iceberg_rest_lib)
 
 install_headers(
-    ['rest_catalog.h', 'types.h', 'json_internal.h', 'validator.h'],
+    ['rest_catalog.h', 'types.h', 'json_internal.h'],
     subdir: 'iceberg/catalog/rest',
 )

src/iceberg/catalog/rest/validator.cc renamed to src/iceberg/catalog/rest/types.cc

@@ -17,35 +17,35 @@
  * under the License.
  */
 
-#include "iceberg/catalog/rest/validator.h"
+#include "iceberg/catalog/rest/types.h"
 
 #include <algorithm>
 #include <format>
 
-#include "iceberg/catalog/rest/types.h"
 #include "iceberg/result.h"
+#include "iceberg/table_identifier.h"
 #include "iceberg/util/formatter_internal.h"
 #include "iceberg/util/macros.h"
 
 namespace iceberg::rest {
 
 // Configuration and Error types
 
-Status Validator::Validate(const CatalogConfig& config) {
+Status CatalogConfig::Validate() const {
   // TODO(Li Feiyang): Add an invalidEndpoint test that validates endpoint format.
   // See:
   // https://github.com/apache/iceberg/blob/main/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java#L164
   // for reference.
   return {};
 }
 
-Status Validator::Validate(const ErrorModel& error) {
-  if (error.message.empty() || error.type.empty()) {
+Status ErrorModel::Validate() const {
+  if (message.empty() || type.empty()) {
     return Invalid("Invalid error model: missing required fields");
   }
 
-  if (error.code < 400 || error.code > 600) {
-    return Invalid("Invalid error model: code {} is out of range [400, 600]", error.code);
+  if (code < 400 || code > 600) {
+    return Invalid("Invalid error model: code {} is out of range [400, 600]", code);
   }
 
   // stack is optional, no validation needed
@@ -54,21 +54,21 @@ Status Validator::Validate(const ErrorModel& error) {
 
 // We don't validate the error field because ErrorModel::Validate has been called in the
 // FromJson.
-Status Validator::Validate(const ErrorResponse& response) { return {}; }
+Status ErrorResponse::Validate() const { return {}; }
 
 // Namespace operations
 
-Status Validator::Validate(const ListNamespacesResponse& response) { return {}; }
+Status ListNamespacesResponse::Validate() const { return {}; }
 
-Status Validator::Validate(const CreateNamespaceRequest& request) { return {}; }
+Status CreateNamespaceRequest::Validate() const { return {}; }
 
-Status Validator::Validate(const CreateNamespaceResponse& response) { return {}; }
+Status CreateNamespaceResponse::Validate() const { return {}; }
 
-Status Validator::Validate(const GetNamespaceResponse& response) { return {}; }
+Status GetNamespaceResponse::Validate() const { return {}; }
 
-Status Validator::Validate(const UpdateNamespacePropertiesRequest& request) {
+Status UpdateNamespacePropertiesRequest::Validate() const {
   // keys in updates and removals must not overlap
-  if (request.removals.empty() || request.updates.empty()) {
+  if (removals.empty() || updates.empty()) {
     return {};
   }
 
@@ -83,9 +83,9 @@ Status Validator::Validate(const UpdateNamespacePropertiesRequest& request) {
   };
 
   auto sorted_removals =
-      extract_and_sort(request.removals, [](const auto& s) -> const auto& { return s; });
+      extract_and_sort(removals, [](const auto& s) -> const auto& { return s; });
   auto sorted_update_keys = extract_and_sort(
-      request.updates, [](const auto& pair) -> const auto& { return pair.first; });
+      updates, [](const auto& pair) -> const auto& { return pair.first; });
 
   std::vector<std::string_view> common;
   std::ranges::set_intersection(sorted_removals, sorted_update_keys,
@@ -99,43 +99,34 @@ Status Validator::Validate(const UpdateNamespacePropertiesRequest& request) {
   return {};
 }
 
-Status Validator::Validate(const UpdateNamespacePropertiesResponse& response) {
-  return {};
-}
+Status UpdateNamespacePropertiesResponse::Validate() const { return {}; }
 
 // Table operations
 
-Status Validator::Validate(const ListTablesResponse& response) { return {}; }
+Status ListTablesResponse::Validate() const { return {}; }
 
-Status Validator::Validate(const LoadTableResult& result) {
-  if (!result.metadata) {
+Status LoadTableResult::Validate() const {
+  if (!metadata) {
     return Invalid("Invalid metadata: null");
   }
   return {};
 }
 
-Status Validator::Validate(const RegisterTableRequest& request) {
-  if (request.name.empty()) {
+Status RegisterTableRequest::Validate() const {
+  if (name.empty()) {
     return Invalid("Missing table name");
   }
 
-  if (request.metadata_location.empty()) {
+  if (metadata_location.empty()) {
     return Invalid("Empty metadata location");
   }
 
   return {};
 }
 
-Status Validator::Validate(const RenameTableRequest& request) {
-  ICEBERG_RETURN_UNEXPECTED(Validate(request.source));
-  ICEBERG_RETURN_UNEXPECTED(Validate(request.destination));
-  return {};
-}
-
-Status Validator::Validate(const TableIdentifier& identifier) {
-  if (identifier.name.empty()) {
-    return Invalid("Invalid table identifier: missing table name");
-  }
+Status RenameTableRequest::Validate() const {
+  ICEBERG_RETURN_UNEXPECTED(source.Validate());
+  ICEBERG_RETURN_UNEXPECTED(destination.Validate());
   return {};
 }