diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index 02440514e..38d897270 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/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) set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS) diff --git a/src/iceberg/catalog/rest/json_internal.cc b/src/iceberg/catalog/rest/json_internal.cc index 404fe21bf..bdec822cc 100644 --- a/src/iceberg/catalog/rest/json_internal.cc +++ b/src/iceberg/catalog/rest/json_internal.cc @@ -26,7 +26,6 @@ #include #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 CatalogConfigFromJson(const nlohmann::json& json) { ICEBERG_ASSIGN_OR_RAISE( config.endpoints, GetJsonValueOrDefault>(json, kEndpoints)); - ICEBERG_RETURN_UNEXPECTED(Validator::Validate(config)); + ICEBERG_RETURN_UNEXPECTED(config.Validate()); return config; } @@ -111,7 +110,7 @@ Result ErrorModelFromJson(const nlohmann::json& json) { ICEBERG_ASSIGN_OR_RAISE(error.code, GetJsonValue(json, kCode)); ICEBERG_ASSIGN_OR_RAISE(error.stack, GetJsonValueOrDefault>(json, kStack)); - ICEBERG_RETURN_UNEXPECTED(Validator::Validate(error)); + ICEBERG_RETURN_UNEXPECTED(error.Validate()); return error; } @@ -125,7 +124,7 @@ Result ErrorResponseFromJson(const nlohmann::json& json) { ErrorResponse response; ICEBERG_ASSIGN_OR_RAISE(auto error_json, GetJsonValue(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 CreateNamespaceRequestFromJson( ICEBERG_ASSIGN_OR_RAISE( request.properties, GetJsonValueOrDefault(json, kProperties)); - ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request)); + ICEBERG_RETURN_UNEXPECTED(request.Validate()); return request; } @@ -162,7 +161,7 @@ Result UpdateNamespacePropertiesRequestFromJso request.removals, GetJsonValueOrDefault>(json, kRemovals)); ICEBERG_ASSIGN_OR_RAISE( request.updates, GetJsonValueOrDefault(json, kUpdates)); - ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request)); + ICEBERG_RETURN_UNEXPECTED(request.Validate()); return request; } @@ -183,7 +182,7 @@ Result RegisterTableRequestFromJson(const nlohmann::json& GetJsonValue(json, kMetadataLocation)); ICEBERG_ASSIGN_OR_RAISE(request.overwrite, GetJsonValueOrDefault(json, kOverwrite, false)); - ICEBERG_RETURN_UNEXPECTED(Validator::Validate(request)); + ICEBERG_RETURN_UNEXPECTED(request.Validate()); return request; } @@ -201,7 +200,7 @@ Result RenameTableRequestFromJson(const nlohmann::json& json ICEBERG_ASSIGN_OR_RAISE(auto dest_json, GetJsonValue(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 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 UpdateNamespacePropertiesResponseFromJ response.removed, GetJsonValueOrDefault>(json, kRemoved)); ICEBERG_ASSIGN_OR_RAISE( response.missing, GetJsonValueOrDefault>(json, kMissing)); - ICEBERG_RETURN_UNEXPECTED(Validator::Validate(response)); + ICEBERG_RETURN_UNEXPECTED(response.Validate()); return response; } @@ -329,7 +328,7 @@ Result 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; } diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index e8edc35c0..5f1f635ab 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/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') # cpr does not export symbols, so on Windows it must # be used as a static lib cpr_needs_static = ( @@ -50,7 +46,4 @@ iceberg_rest_dep = declare_dependency( 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'], - subdir: 'iceberg/catalog/rest', -) +install_headers(['rest_catalog.h', 'types.h'], subdir: 'iceberg/catalog/rest') diff --git a/src/iceberg/catalog/rest/types.h b/src/iceberg/catalog/rest/types.h index 15e9831b8..2e32f967f 100644 --- a/src/iceberg/catalog/rest/types.h +++ b/src/iceberg/catalog/rest/types.h @@ -19,14 +19,18 @@ #pragma once +#include +#include #include #include #include #include #include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" #include "iceberg/table_identifier.h" #include "iceberg/type_fwd.h" +#include "iceberg/util/macros.h" /// \file iceberg/catalog/rest/types.h /// Request and response types for Iceberg REST Catalog API. @@ -38,6 +42,15 @@ struct ICEBERG_REST_EXPORT CatalogConfig { std::unordered_map defaults; // required std::unordered_map overrides; // required std::vector endpoints; + + /// \brief Validates the CatalogConfig. + Status 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 {}; + } }; /// \brief JSON error payload returned in a response with further details on the error. @@ -46,23 +59,55 @@ struct ICEBERG_REST_EXPORT ErrorModel { std::string type; // required uint32_t code; // required std::vector stack; + + /// \brief Validates the ErrorModel. + Status Validate() const { + if (message.empty() || type.empty()) { + return Invalid("Invalid error model: missing required fields"); + } + + if (code < 400 || code > 600) { + return Invalid("Invalid error model: code {} is out of range [400, 600]", code); + } + + // stack is optional, no validation needed + return {}; + } }; /// \brief Error response body returned in a response. struct ICEBERG_REST_EXPORT ErrorResponse { ErrorModel error; // required + + /// \brief Validates the ErrorResponse. + // We don't validate the error field because ErrorModel::Validate has been called in the + // FromJson. + Status Validate() const { return {}; } }; /// \brief Request to create a namespace. struct ICEBERG_REST_EXPORT CreateNamespaceRequest { Namespace namespace_; // required std::unordered_map properties; + + /// \brief Validates the CreateNamespaceRequest. + Status Validate() const { return {}; } }; /// \brief Update or delete namespace properties request. struct ICEBERG_REST_EXPORT UpdateNamespacePropertiesRequest { std::vector removals; std::unordered_map updates; + + /// \brief Validates the UpdateNamespacePropertiesRequest. + Status Validate() const { + for (const auto& key : removals) { + if (updates.contains(key)) { + return Invalid("Duplicate key to update and remove: {}", key); + } + } + return {}; + } }; /// \brief Request to register a table. @@ -70,12 +115,32 @@ struct ICEBERG_REST_EXPORT RegisterTableRequest { std::string name; // required std::string metadata_location; // required bool overwrite = false; + + /// \brief Validates the RegisterTableRequest. + Status Validate() const { + if (name.empty()) { + return Invalid("Missing table name"); + } + + if (metadata_location.empty()) { + return Invalid("Empty metadata location"); + } + + return {}; + } }; /// \brief Request to rename a table. struct ICEBERG_REST_EXPORT RenameTableRequest { TableIdentifier source; // required TableIdentifier destination; // required + + /// \brief Validates the RenameTableRequest. + Status Validate() const { + ICEBERG_RETURN_UNEXPECTED(source.Validate()); + ICEBERG_RETURN_UNEXPECTED(destination.Validate()); + return {}; + } }; /// \brief An opaque token that allows clients to make use of pagination for list APIs. @@ -87,6 +152,14 @@ struct ICEBERG_REST_EXPORT LoadTableResult { std::shared_ptr metadata; // required std::unordered_map config; // TODO(Li Feiyang): Add std::shared_ptr storage_credential; + + /// \brief Validates the LoadTableResult. + Status Validate() const { + if (!metadata) { + return Invalid("Invalid metadata: null"); + } + return {}; + } }; /// \brief Alias of LoadTableResult used as the body of CreateTableResponse @@ -99,18 +172,27 @@ using LoadTableResponse = LoadTableResult; struct ICEBERG_REST_EXPORT ListNamespacesResponse { PageToken next_page_token; std::vector namespaces; + + /// \brief Validates the ListNamespacesResponse. + Status Validate() const { return {}; } }; /// \brief Response body after creating a namespace. struct ICEBERG_REST_EXPORT CreateNamespaceResponse { Namespace namespace_; // required std::unordered_map properties; + + /// \brief Validates the CreateNamespaceResponse. + Status Validate() const { return {}; } }; /// \brief Response body for loading namespace properties. struct ICEBERG_REST_EXPORT GetNamespaceResponse { Namespace namespace_; // required std::unordered_map properties; + + /// \brief Validates the GetNamespaceResponse. + Status Validate() const { return {}; } }; /// \brief Response body after updating namespace properties. @@ -118,12 +200,18 @@ struct ICEBERG_REST_EXPORT UpdateNamespacePropertiesResponse { std::vector updated; // required std::vector removed; // required std::vector missing; + + /// \brief Validates the UpdateNamespacePropertiesResponse. + Status Validate() const { return {}; } }; /// \brief Response body for listing tables in a namespace. struct ICEBERG_REST_EXPORT ListTablesResponse { PageToken next_page_token; std::vector identifiers; + + /// \brief Validates the ListTablesResponse. + Status Validate() const { return {}; } }; } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/validator.cc b/src/iceberg/catalog/rest/validator.cc deleted file mode 100644 index de25fa334..000000000 --- a/src/iceberg/catalog/rest/validator.cc +++ /dev/null @@ -1,142 +0,0 @@ -/* - * 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/validator.h" - -#include -#include - -#include "iceberg/catalog/rest/types.h" -#include "iceberg/result.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) { - // 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()) { - 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); - } - - // stack is optional, no validation needed - return {}; -} - -// We don't validate the error field because ErrorModel::Validate has been called in the -// FromJson. -Status Validator::Validate(const ErrorResponse& response) { return {}; } - -// Namespace operations - -Status Validator::Validate(const ListNamespacesResponse& response) { return {}; } - -Status Validator::Validate(const CreateNamespaceRequest& request) { return {}; } - -Status Validator::Validate(const CreateNamespaceResponse& response) { return {}; } - -Status Validator::Validate(const GetNamespaceResponse& response) { return {}; } - -Status Validator::Validate(const UpdateNamespacePropertiesRequest& request) { - // keys in updates and removals must not overlap - if (request.removals.empty() || request.updates.empty()) { - return {}; - } - - auto extract_and_sort = [](const auto& container, auto key_extractor) { - std::vector result; - result.reserve(container.size()); - for (const auto& item : container) { - result.push_back(std::string_view{key_extractor(item)}); - } - std::ranges::sort(result); - return result; - }; - - auto sorted_removals = - extract_and_sort(request.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; }); - - std::vector common; - std::ranges::set_intersection(sorted_removals, sorted_update_keys, - std::back_inserter(common)); - - if (!common.empty()) { - return Invalid( - "Invalid namespace update: cannot simultaneously set and remove keys: {}", - common); - } - return {}; -} - -Status Validator::Validate(const UpdateNamespacePropertiesResponse& response) { - return {}; -} - -// Table operations - -Status Validator::Validate(const ListTablesResponse& response) { return {}; } - -Status Validator::Validate(const LoadTableResult& result) { - if (!result.metadata) { - return Invalid("Invalid metadata: null"); - } - return {}; -} - -Status Validator::Validate(const RegisterTableRequest& request) { - if (request.name.empty()) { - return Invalid("Missing table name"); - } - - if (request.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"); - } - return {}; -} - -} // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/validator.h b/src/iceberg/catalog/rest/validator.h deleted file mode 100644 index 2c80cabc0..000000000 --- a/src/iceberg/catalog/rest/validator.h +++ /dev/null @@ -1,87 +0,0 @@ -/* - * 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 "iceberg/catalog/rest/iceberg_rest_export.h" -#include "iceberg/catalog/rest/types.h" -#include "iceberg/result.h" - -/// \file iceberg/catalog/rest/validator.h -/// Validator for REST Catalog API types. - -namespace iceberg::rest { - -/// \brief Validator for REST Catalog API types. Validation should be called after -/// deserializing objects from external sources to ensure data integrity before the -/// objects are used. -class ICEBERG_REST_EXPORT Validator { - public: - // Configuration and Error types - - /// \brief Validates a CatalogConfig object. - static Status Validate(const CatalogConfig& config); - - /// \brief Validates an ErrorModel object. - static Status Validate(const ErrorModel& error); - - /// \brief Validates an ErrorResponse object. - static Status Validate(const ErrorResponse& response); - - // Namespace operations - - /// \brief Validates a ListNamespacesResponse object. - static Status Validate(const ListNamespacesResponse& response); - - /// \brief Validates a CreateNamespaceRequest object. - static Status Validate(const CreateNamespaceRequest& request); - - /// \brief Validates a CreateNamespaceResponse object. - static Status Validate(const CreateNamespaceResponse& response); - - /// \brief Validates a GetNamespaceResponse object. - static Status Validate(const GetNamespaceResponse& response); - - /// \brief Validates an UpdateNamespacePropertiesRequest object. - static Status Validate(const UpdateNamespacePropertiesRequest& request); - - /// \brief Validates an UpdateNamespacePropertiesResponse object. - static Status Validate(const UpdateNamespacePropertiesResponse& response); - - // Table operations - - /// \brief Validates a ListTablesResponse object. - static Status Validate(const ListTablesResponse& response); - - /// \brief Validates a LoadTableResult object. - static Status Validate(const LoadTableResult& result); - - /// \brief Validates a RegisterTableRequest object. - static Status Validate(const RegisterTableRequest& request); - - /// \brief Validates a RenameTableRequest object. - static Status Validate(const RenameTableRequest& request); - - // Other types - - /// \brief Validates a TableIdentifier object. - static Status Validate(const TableIdentifier& identifier); -}; - -} // namespace iceberg::rest diff --git a/src/iceberg/table_identifier.h b/src/iceberg/table_identifier.h index 9aa5770a1..b145e75f3 100644 --- a/src/iceberg/table_identifier.h +++ b/src/iceberg/table_identifier.h @@ -26,6 +26,7 @@ #include #include "iceberg/iceberg_export.h" +#include "iceberg/result.h" namespace iceberg { @@ -38,6 +39,14 @@ struct ICEBERG_EXPORT Namespace { struct ICEBERG_EXPORT TableIdentifier { Namespace ns; std::string name; + + /// \brief Validates the TableIdentifier. + Status Validate() const { + if (name.empty()) { + return Invalid("Invalid table identifier: missing table name"); + } + return {}; + } }; } // namespace iceberg