This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 7f7f85b refactor: remove Validator and add Validate methods directly
to rest models (#304)
7f7f85b is described below
commit 7f7f85b7b7b666a398bfc4ebd47f385578ce9c4e
Author: Li Feiyang <[email protected]>
AuthorDate: Wed Nov 12 10:21:52 2025 +0800
refactor: remove Validator and add Validate methods directly to rest models
(#304)
---
src/iceberg/catalog/rest/CMakeLists.txt | 2 +-
src/iceberg/catalog/rest/json_internal.cc | 21 +++--
src/iceberg/catalog/rest/meson.build | 11 +--
src/iceberg/catalog/rest/types.h | 88 ++++++++++++++++++
src/iceberg/catalog/rest/validator.cc | 142 ------------------------------
src/iceberg/catalog/rest/validator.h | 87 ------------------
src/iceberg/table_identifier.h | 9 ++
7 files changed, 110 insertions(+), 250 deletions(-)
diff --git a/src/iceberg/catalog/rest/CMakeLists.txt
b/src/iceberg/catalog/rest/CMakeLists.txt
index 0244051..38d8972 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 404fe21..bdec822 100644
--- a/src/iceberg/catalog/rest/json_internal.cc
+++ b/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;
}
diff --git a/src/iceberg/catalog/rest/meson.build
b/src/iceberg/catalog/rest/meson.build
index e8edc35..5f1f635 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 15e9831..2e32f96 100644
--- a/src/iceberg/catalog/rest/types.h
+++ b/src/iceberg/catalog/rest/types.h
@@ -19,14 +19,18 @@
#pragma once
+#include <algorithm>
+#include <format>
#include <memory>
#include <string>
#include <unordered_map>
#include <vector>
#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<std::string, std::string> defaults; // required
std::unordered_map<std::string, std::string> overrides; // required
std::vector<std::string> 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<std::string> 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<std::string, std::string> properties;
+
+ /// \brief Validates the CreateNamespaceRequest.
+ Status Validate() const { return {}; }
};
/// \brief Update or delete namespace properties request.
struct ICEBERG_REST_EXPORT UpdateNamespacePropertiesRequest {
std::vector<std::string> removals;
std::unordered_map<std::string, std::string> 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<TableMetadata> metadata; // required
std::unordered_map<std::string, std::string> config;
// TODO(Li Feiyang): Add std::shared_ptr<StorageCredential>
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<Namespace> 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<std::string, std::string> 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<std::string, std::string> 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<std::string> updated; // required
std::vector<std::string> removed; // required
std::vector<std::string> 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<TableIdentifier> 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 de25fa3..0000000
--- 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 <algorithm>
-#include <format>
-
-#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<std::string_view> 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<std::string_view> 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 2c80cab..0000000
--- 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 9aa5770..b145e75 100644
--- a/src/iceberg/table_identifier.h
+++ b/src/iceberg/table_identifier.h
@@ -26,6 +26,7 @@
#include <vector>
#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
