diff --git a/src/iceberg/pending_update.h b/src/iceberg/pending_update.h index 4c6fe095f..8c1f7f270 100644 --- a/src/iceberg/pending_update.h +++ b/src/iceberg/pending_update.h @@ -25,6 +25,7 @@ #include "iceberg/iceberg_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" +#include "iceberg/util/error_collector.h" namespace iceberg { @@ -74,7 +75,7 @@ class ICEBERG_EXPORT PendingUpdate { /// /// \tparam T The type of result returned by Apply() template -class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate { +class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate, public ErrorCollector { public: ~PendingUpdateTyped() override = default; diff --git a/src/iceberg/table_metadata.cc b/src/iceberg/table_metadata.cc index df23e2f92..393eebfc3 100644 --- a/src/iceberg/table_metadata.cc +++ b/src/iceberg/table_metadata.cc @@ -275,9 +275,6 @@ struct TableMetadataBuilder::Impl { // Change tracking std::vector> changes; - // Error collection (since methods return *this and cannot throw) - std::vector errors; - // Metadata location tracking std::optional metadata_location; std::optional previous_metadata_location; @@ -348,8 +345,7 @@ TableMetadataBuilder& TableMetadataBuilder::AssignUUID(std::string_view uuid) { // Validation: UUID cannot be empty if (uuid_str.empty()) { - impl_->errors.emplace_back(ErrorKind::kInvalidArgument, "Cannot assign empty UUID"); - return *this; + return AddError(ErrorKind::kInvalidArgument, "Cannot assign empty UUID"); } // Check if UUID is already set to the same value (no-op) @@ -451,7 +447,7 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveSnapshots( throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -TableMetadataBuilder& TableMetadataBuilder::suppressHistoricalSnapshots() { +TableMetadataBuilder& TableMetadataBuilder::SuppressHistoricalSnapshots() { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } @@ -499,13 +495,7 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveEncryptionKey(std::string_view Result> TableMetadataBuilder::Build() { // 1. Check for accumulated errors - if (!impl_->errors.empty()) { - std::string error_msg = "Failed to build TableMetadata due to validation errors:\n"; - for (const auto& [kind, message] : impl_->errors) { - error_msg += " - " + message + "\n"; - } - return CommitFailed("{}", error_msg); - } + ICEBERG_RETURN_UNEXPECTED(CheckErrors()); // 2. Validate metadata consistency through TableMetadata#Validate diff --git a/src/iceberg/table_metadata.h b/src/iceberg/table_metadata.h index 5ecbe4766..503b9b143 100644 --- a/src/iceberg/table_metadata.h +++ b/src/iceberg/table_metadata.h @@ -30,6 +30,7 @@ #include "iceberg/iceberg_export.h" #include "iceberg/type_fwd.h" +#include "iceberg/util/error_collector.h" #include "iceberg/util/lazy.h" #include "iceberg/util/timepoint.h" @@ -193,7 +194,7 @@ ICEBERG_EXPORT std::string ToString(const MetadataLogEntry& entry); /// If a modification violates Iceberg table constraints (e.g., setting a current /// schema ID that does not exist), an error will be recorded and returned when /// Build() is called. -class ICEBERG_EXPORT TableMetadataBuilder { +class ICEBERG_EXPORT TableMetadataBuilder : public ErrorCollector { public: /// \brief Create a builder for a new table /// @@ -353,7 +354,7 @@ class ICEBERG_EXPORT TableMetadataBuilder { /// RemoveSnapshot changes are created. A snapshot is historical if no ref directly /// references its ID. /// \return Reference to this builder for method chaining - TableMetadataBuilder& suppressHistoricalSnapshots(); + TableMetadataBuilder& SuppressHistoricalSnapshots(); /// \brief Set table statistics /// diff --git a/src/iceberg/util/error_collector.h b/src/iceberg/util/error_collector.h new file mode 100644 index 000000000..f94967f97 --- /dev/null +++ b/src/iceberg/util/error_collector.h @@ -0,0 +1,139 @@ +/* + * 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 + +/// \file iceberg/util/error_collector.h +/// Base class for collecting validation errors in builder patterns + +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/result.h" + +namespace iceberg { + +/// \brief Base class for collecting validation errors in builder patterns +/// +/// This class provides error accumulation functionality for builders that +/// cannot throw exceptions. Builder methods can call AddError() to accumulate +/// validation errors, and CheckErrors() returns all errors at once. +/// +/// Example usage: +/// \code +/// class MyBuilder : public ErrorCollectorBase { +/// public: +/// MyBuilder& SetValue(int val) { +/// if (val < 0) { +/// return AddError(ErrorKind::kInvalidArgument, "Value must be non-negative"); +/// } +/// value_ = val; +/// return *this; +/// } +/// +/// Result Build() { +/// ICEBERG_RETURN_UNEXPECTED(CheckErrors()); +/// return MyObject{value_}; +/// } +/// +/// private: +/// int value_ = 0; +/// }; +/// \endcode +class ICEBERG_EXPORT ErrorCollector { + public: + ErrorCollector() = default; + virtual ~ErrorCollector() = default; + + ErrorCollector(ErrorCollector&&) = default; + ErrorCollector& operator=(ErrorCollector&&) = default; + + ErrorCollector(const ErrorCollector&) = default; + ErrorCollector& operator=(const ErrorCollector&) = default; + + /// \brief Add a specific error and return reference to derived class + /// + /// \param self Deduced reference to the derived class instance + /// \param kind The kind of error + /// \param message The error message + /// \return Reference to the derived class for method chaining + auto& AddError(this auto& self, ErrorKind kind, std::string message) { + self.errors_.emplace_back(kind, std::move(message)); + return self; + } + + /// \brief Add an existing error object and return reference to derived class + /// + /// Useful when propagating errors from other components or reusing + /// error objects without deconstructing and reconstructing them. + /// + /// \param self Deduced reference to the derived class instance + /// \param err The error to add + /// \return Reference to the derived class for method chaining + auto& AddError(this auto& self, Error err) { + self.errors_.push_back(std::move(err)); + return self; + } + + /// \brief Check if any errors have been collected + /// + /// \return true if there are accumulated errors + [[nodiscard]] bool HasErrors() const { return !errors_.empty(); } + + /// \brief Get the number of errors collected + /// + /// \return The count of accumulated errors + [[nodiscard]] size_t ErrorCount() const { return errors_.size(); } + + /// \brief Check for accumulated errors and return them if any exist + /// + /// This should be called before completing a builder operation (e.g., + /// in Build(), Apply(), or Commit() methods) to validate that no errors + /// were accumulated during the builder method calls. + /// + /// \return Status::OK if no errors, or a ValidationFailed error with + /// all accumulated error messages + [[nodiscard]] Status CheckErrors() const { + if (!errors_.empty()) { + std::string error_msg = "Validation failed due to the following errors:\n"; + for (const auto& [kind, message] : errors_) { + error_msg += " - " + message + "\n"; + } + return ValidationFailed("{}", error_msg); + } + return {}; + } + + /// \brief Clear all accumulated errors + /// + /// This can be useful for resetting the error state in tests or + /// when reusing a builder instance. + void ClearErrors() { errors_.clear(); } + + /// \brief Get read-only access to all collected errors + /// + /// \return A const reference to the vector of errors + [[nodiscard]] const std::vector& Errors() const { return errors_; } + + protected: + std::vector errors_; +}; + +} // namespace iceberg diff --git a/src/iceberg/util/meson.build b/src/iceberg/util/meson.build index 30c42d02d..0e0508476 100644 --- a/src/iceberg/util/meson.build +++ b/src/iceberg/util/meson.build @@ -23,6 +23,7 @@ install_headers( 'conversions.h', 'decimal.h', 'endian.h', + 'error_collector.h', 'formattable.h', 'formatter.h', 'int128.h',