From c9acf948c7493be7e0fb9917173901e0c855f020 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Wed, 19 Aug 2026 13:41:12 -0500 Subject: [PATCH 1/7] return the row number from registerWrite --- form/persistence/ipersistence_writer.hpp | 9 +-- form/persistence/persistence_writer.cpp | 10 ++-- form/persistence/persistence_writer.hpp | 8 +-- .../root_rfield_write_container.cpp | 7 ++- .../root_rfield_write_container.hpp | 2 +- .../root_rntuple_write_container.cpp | 2 +- .../root_rntuple_write_container.hpp | 2 +- .../root_tbranch_write_container.cpp | 5 +- .../root_tbranch_write_container.hpp | 2 +- .../root_ttree_write_container.cpp | 2 +- .../root_ttree_write_container.hpp | 2 +- form/storage/istorage.hpp | 10 ++-- form/storage/storage_write_container.cpp | 2 +- form/storage/storage_write_container.hpp | 2 +- form/storage/storage_writer.cpp | 8 +-- form/storage/storage_writer.hpp | 6 +- test/form/form_storage_test.cpp | 55 +++++++++++++++++++ 17 files changed, 100 insertions(+), 34 deletions(-) diff --git a/form/persistence/ipersistence_writer.hpp b/form/persistence/ipersistence_writer.hpp index a0ff7b62c..4f15f3797 100644 --- a/form/persistence/ipersistence_writer.hpp +++ b/form/persistence/ipersistence_writer.hpp @@ -27,10 +27,11 @@ namespace form::detail::experimental { virtual void createContainers(std::string const& creator, std::map const& products) = 0; - virtual void registerWrite(std::string const& creator, - std::string const& label, - void const* data, - std::type_info const& type) = 0; + // Write one product and return the 0-based row (entry) number it landed in. + virtual int registerWrite(std::string const& creator, + std::string const& label, + void const* data, + std::type_info const& type) = 0; virtual void commitOutput(std::string const& creator, std::string const& id) = 0; }; diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index c6e13241a..0d381c7d1 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -48,13 +48,13 @@ void PersistenceWriter::createContainers( m_store_writer->createContainers(containers, m_tech_settings); } -void PersistenceWriter::registerWrite(std::string const& creator, - std::string const& label, - void const* data, - std::type_info const& type) +int PersistenceWriter::registerWrite(std::string const& creator, + std::string const& label, + void const* data, + std::type_info const& type) { std::unique_ptr plcmnt = getPlacement(creator, label); - m_store_writer->fillContainer(*plcmnt, data, type); + return m_store_writer->fillContainer(*plcmnt, data, type); } void PersistenceWriter::commitOutput(std::string const& creator, std::string const& id) diff --git a/form/persistence/persistence_writer.hpp b/form/persistence/persistence_writer.hpp index ac198a7ff..c171fc0cb 100644 --- a/form/persistence/persistence_writer.hpp +++ b/form/persistence/persistence_writer.hpp @@ -31,10 +31,10 @@ namespace form::detail::experimental { void createContainers(std::string const& creator, std::map const& products) override; - void registerWrite(std::string const& creator, - std::string const& label, - void const* data, - std::type_info const& type) override; + int registerWrite(std::string const& creator, + std::string const& label, + void const* data, + std::type_info const& type) override; void commitOutput(std::string const& creator, std::string const& id) override; private: diff --git a/form/root_storage/root_rfield_write_container.cpp b/form/root_storage/root_rfield_write_container.cpp index f09a4e921..04e66ba91 100644 --- a/form/root_storage/root_rfield_write_container.cpp +++ b/form/root_storage/root_rfield_write_container.cpp @@ -61,7 +61,7 @@ namespace form::detail::experimental { m_rntuple_parent = parentDerived; } - void ROOT_RField_Write_ContainerImp::fill(void const* data) + int ROOT_RField_Write_ContainerImp::fill(void const* data) { if (!m_rntuple_parent) { throw std::runtime_error( @@ -79,6 +79,11 @@ namespace form::detail::experimental { m_rntuple_parent->m_entry = m_rntuple_parent->m_writer->CreateRawPtrWriteEntry(); } m_rntuple_parent->m_entry->BindRawPtr(col_name(), data); + + // Unlike a TBranch, an RNTuple entry is only written on commit(); + // every field bound before that commit shares one entry. + // Return the 0-based index that pending entry will occupy (the current entry count). + return static_cast(m_rntuple_parent->m_writer->GetNEntries()); } void ROOT_RField_Write_ContainerImp::commit() diff --git a/form/root_storage/root_rfield_write_container.hpp b/form/root_storage/root_rfield_write_container.hpp index d5387582b..f163e1918 100644 --- a/form/root_storage/root_rfield_write_container.hpp +++ b/form/root_storage/root_rfield_write_container.hpp @@ -23,7 +23,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type) override; void setParent(std::shared_ptr const parent) override; - void fill(void const* data) override; + int fill(void const* data) override; void commit() override; private: diff --git a/form/root_storage/root_rntuple_write_container.cpp b/form/root_storage/root_rntuple_write_container.cpp index 09d861da4..92381277b 100644 --- a/form/root_storage/root_rntuple_write_container.cpp +++ b/form/root_storage/root_rntuple_write_container.cpp @@ -29,7 +29,7 @@ namespace form::detail::experimental { return; } - void ROOT_RNTuple_Write_ContainerImp::fill(void const* /*data*/) + int ROOT_RNTuple_Write_ContainerImp::fill(void const* /*data*/) { throw std::runtime_error("ROOT_RNTuple_Write_ContainerImp::fill not implemented"); } diff --git a/form/root_storage/root_rntuple_write_container.hpp b/form/root_storage/root_rntuple_write_container.hpp index e386e120a..423edc5f7 100644 --- a/form/root_storage/root_rntuple_write_container.hpp +++ b/form/root_storage/root_rntuple_write_container.hpp @@ -52,7 +52,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type) override; - void fill(void const* data) override; + int fill(void const* data) override; void commit() override; //State shared by ROOT_RField_ContainerImps diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index c786d00fd..c4aaac35f 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -94,7 +94,7 @@ void ROOT_TBranch_Write_ContainerImp::setupWrite(std::type_info const& type) } } -void ROOT_TBranch_Write_ContainerImp::fill(void const* data) +int ROOT_TBranch_Write_ContainerImp::fill(void const* data) { // NOTE: incoming parameter `data` is `const` due to the constraints on how we // expect users to interact with the data; however, ROOT's SetBranchAddress @@ -114,6 +114,9 @@ void ROOT_TBranch_Write_ContainerImp::fill(void const* data) } m_branch->Fill(); m_branch->ResetAddress(); + + // 0-based entries: GetEntries() is the total count after this Fill(); row = count - 1 + return static_cast(m_branch->GetEntries()) - 1; } void ROOT_TBranch_Write_ContainerImp::commit() diff --git a/form/root_storage/root_tbranch_write_container.hpp b/form/root_storage/root_tbranch_write_container.hpp index d61533342..485e9f627 100644 --- a/form/root_storage/root_tbranch_write_container.hpp +++ b/form/root_storage/root_tbranch_write_container.hpp @@ -25,7 +25,7 @@ namespace form::detail::experimental { void setParent(std::shared_ptr parent) override; void setupWrite(std::type_info const& type = typeid(void)) override; - void fill(void const* data) override; + int fill(void const* data) override; void commit() override; private: diff --git a/form/root_storage/root_ttree_write_container.cpp b/form/root_storage/root_ttree_write_container.cpp index 1a769d100..604a4e766 100644 --- a/form/root_storage/root_ttree_write_container.cpp +++ b/form/root_storage/root_ttree_write_container.cpp @@ -45,7 +45,7 @@ void ROOT_TTree_Write_ContainerImp::setupWrite(std::type_info const& /* type*/) } } -void ROOT_TTree_Write_ContainerImp::fill(void const* /* data*/) +int ROOT_TTree_Write_ContainerImp::fill(void const* /* data*/) { throw std::runtime_error("ROOT_TTree_Write_ContainerImp::fill not implemented"); } diff --git a/form/root_storage/root_ttree_write_container.hpp b/form/root_storage/root_ttree_write_container.hpp index 6a5c90a2a..7f0e22c91 100644 --- a/form/root_storage/root_ttree_write_container.hpp +++ b/form/root_storage/root_ttree_write_container.hpp @@ -25,7 +25,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type) override; - void fill(void const* data) override; + int fill(void const* data) override; void commit() override; TTree* getTTree(); diff --git a/form/storage/istorage.hpp b/form/storage/istorage.hpp index a755e4279..b6a6f9643 100644 --- a/form/storage/istorage.hpp +++ b/form/storage/istorage.hpp @@ -41,9 +41,10 @@ namespace form::detail::experimental { virtual void createContainers( std::map, std::type_info const*> const& containers, form::experimental::config::tech_setting_config const& settings) = 0; - virtual void fillContainer(Placement const& plcmnt, - void const* data, - std::type_info const& type) = 0; + // Returns the 0-based row (entry) number the data was written to + virtual int fillContainer(Placement const& plcmnt, + void const* data, + std::type_info const& type) = 0; virtual void commitContainers(Placement const& plcmnt) = 0; }; @@ -67,7 +68,8 @@ namespace form::detail::experimental { virtual void setFile(std::shared_ptr file) = 0; virtual void setupWrite(std::type_info const& type = typeid(void)) = 0; - virtual void fill(void const* data) = 0; + // Returns the 0-based row (entry) number written or -1 if no rows + virtual int fill(void const* data) = 0; virtual void commit() = 0; virtual void setAttribute(std::string const& name, std::string const& value) = 0; diff --git a/form/storage/storage_write_container.cpp b/form/storage/storage_write_container.cpp index 785207754..7a812afda 100644 --- a/form/storage/storage_write_container.cpp +++ b/form/storage/storage_write_container.cpp @@ -18,7 +18,7 @@ void Storage_Write_Container::setFile(std::shared_ptr file) { m_f void Storage_Write_Container::setupWrite(std::type_info const& /* type*/) {} -void Storage_Write_Container::fill(void const* /* data*/) {} +int Storage_Write_Container::fill(void const* /* data*/) { return -1; } void Storage_Write_Container::commit() {} diff --git a/form/storage/storage_write_container.hpp b/form/storage/storage_write_container.hpp index 997e47b17..fc4557dde 100644 --- a/form/storage/storage_write_container.hpp +++ b/form/storage/storage_write_container.hpp @@ -20,7 +20,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type = typeid(void)) override; - void fill(void const* data) override; + int fill(void const* data) override; void commit() override; void setAttribute(std::string const& name, std::string const& value) override; diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index a6b081ee7..48beafd33 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -103,9 +103,9 @@ void StorageWriter::createContainers( } } -void StorageWriter::fillContainer(Placement const& plcmnt, - void const* data, - std::type_info const& /* type*/) +int StorageWriter::fillContainer(Placement const& plcmnt, + void const* data, + std::type_info const& /* type*/) { // Use file+container as composite key auto contKey = std::make_pair(plcmnt.fileName(), plcmnt.containerName()); @@ -115,7 +115,7 @@ void StorageWriter::fillContainer(Placement const& plcmnt, throw std::runtime_error("StorageWriter::fillContainer Container doesn't exist: " + plcmnt.containerName()); } - cont->second->fill(data); + return cont->second->fill(data); } void StorageWriter::commitContainers(Placement const& plcmnt) diff --git a/form/storage/storage_writer.hpp b/form/storage/storage_writer.hpp index 50679d317..0ae627ba7 100644 --- a/form/storage/storage_writer.hpp +++ b/form/storage/storage_writer.hpp @@ -23,9 +23,9 @@ namespace form::detail::experimental { void createContainers( std::map, std::type_info const*> const& containers, form::experimental::config::tech_setting_config const& settings) override; - void fillContainer(Placement const& plcmnt, - void const* data, - std::type_info const& type) override; + int fillContainer(Placement const& plcmnt, + void const* data, + std::type_info const& type) override; void commitContainers(Placement const& plcmnt) override; private: diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index e3632f4c0..7cd4bc27c 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -331,6 +331,61 @@ TEST_CASE("Persistence round-trip: structured index normalization and listing", CHECK((*read_first == first || *read_first == second)); } +TEST_CASE("registerWrite returns the row id used to build a Token", "[form]") +{ + using namespace form::experimental::config; + + std::string const file_name = + "registerwrite_rowid_" + form::technology::to_string(technology) + ".root"; + std::string const creator = "rowid_creator"; + std::string const container = creator + "/prod"; + + ItemConfig cfg; + cfg.addItem("prod", file_name, technology); + + std::vector const first = {11, 22, 33}; + std::vector const second = {44, 55, 66}; + + int row_first = -1; + int row_second = -1; + { + auto writer = createPersistenceWriter(); + REQUIRE(writer != nullptr); + writer->configure(cfg); + writer->configureTechSettings(tech_setting_config{}); + writer->createContainers(creator, {{"prod", &typeid(std::vector)}}); + + row_first = writer->registerWrite(creator, "prod", &first, typeid(std::vector)); + writer->commitOutput(creator, "[event:1, segment:1]"); + + row_second = writer->registerWrite(creator, "prod", &second, typeid(std::vector)); + writer->commitOutput(creator, "[event:1, segment:2]"); + } + + // registerWrite returns 0-based, monotonically increasing rows. + CHECK(row_first == 0); + CHECK(row_second == 1); + + // The returned row is all a Token needs (beyond the placement) to locate the + // data again: read each row straight back through a hand-built Token. + StorageReader reader; + tech_setting_config const settings{}; + + void const* raw = nullptr; + Token const token_first{file_name, container, technology, row_first}; + reader.readContainer(token_first, &raw, typeid(std::vector), settings); + auto const* got_first = static_cast const*>(raw); + REQUIRE(got_first != nullptr); + CHECK(*got_first == first); + + raw = nullptr; + Token const token_second{file_name, container, technology, row_second}; + reader.readContainer(token_second, &raw, typeid(std::vector), settings); + auto const* got_second = static_cast const*>(raw); + REQUIRE(got_second != nullptr); + CHECK(*got_second == second); +} + TEST_CASE("Persistence round-trip: all-zero structured id fallback", "[form]") { using namespace form::experimental::config; From 7c9d86044d45258cee4d9a226765d96c17e141cf Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Wed, 19 Aug 2026 15:49:59 -0500 Subject: [PATCH 2/7] return from 'registerWrite' is changed to a Token; std::uint64_t is used for the row number (write side) --- form/core/token.cpp | 27 +++++++--- form/core/token.hpp | 25 ++++++--- form/persistence/ipersistence_writer.hpp | 13 +++-- form/persistence/persistence_writer.cpp | 14 +++-- form/persistence/persistence_writer.hpp | 8 +-- .../root_rfield_write_container.cpp | 4 +- .../root_rfield_write_container.hpp | 2 +- .../root_rntuple_write_container.cpp | 2 +- .../root_rntuple_write_container.hpp | 2 +- .../root_tbranch_write_container.cpp | 7 +-- .../root_tbranch_write_container.hpp | 2 +- .../root_ttree_write_container.cpp | 2 +- .../root_ttree_write_container.hpp | 2 +- form/storage/istorage.hpp | 18 ++++--- form/storage/storage_reader.cpp | 3 +- form/storage/storage_write_container.cpp | 2 +- form/storage/storage_write_container.hpp | 2 +- form/storage/storage_writer.cpp | 6 +-- form/storage/storage_writer.hpp | 6 +-- test/form/form_basics_test.cpp | 7 +-- test/form/form_storage_test.cpp | 54 ++++++++++++++----- 21 files changed, 140 insertions(+), 68 deletions(-) diff --git a/form/core/token.cpp b/form/core/token.cpp index dfd7529ff..2ef66d8c0 100644 --- a/form/core/token.cpp +++ b/form/core/token.cpp @@ -6,12 +6,26 @@ using namespace form::detail::experimental; -/// Constructor with initialization -Token::Token(std::string fileName, std::string containerName, technology::Id technology, int id) : +/// Placement-only constructor; id is left unset +Token::Token(std::string fileName, std::string containerName, technology::Id technology) : m_technology(technology), m_fileName(std::move(fileName)), m_containerName(std::move(containerName)), - m_id(id) + m_id(0), + m_hasId(false) +{ +} + +/// Fully-specified constructor; id is set +Token::Token(std::string fileName, + std::string containerName, + technology::Id technology, + std::uint64_t id) : + m_technology(technology), + m_fileName(std::move(fileName)), + m_containerName(std::move(containerName)), + m_id(id), + m_hasId(true) { } @@ -21,6 +35,7 @@ std::string const& Token::fileName() const { return m_fileName; } std::string const& Token::containerName() const { return m_containerName; } /// Access technology type form::technology::Id Token::technology() const { return m_technology; } -/// Set technology type -/// Access identifier/entry number -int Token::id() const { return m_id; } +/// Access identifier/entry number (0-based row) +std::uint64_t Token::id() const { return m_id; } +/// Whether an id has been set on this token +bool Token::hasId() const { return m_hasId; } diff --git a/form/core/token.hpp b/form/core/token.hpp index 821ef181a..4484b0db1 100644 --- a/form/core/token.hpp +++ b/form/core/token.hpp @@ -5,6 +5,7 @@ #include "core/technology.hpp" +#include #include /* @class Token @@ -13,11 +14,17 @@ namespace form::detail::experimental { class Token { public: - /// Default constructor; delegates to the named constructor so the -1 sentinel for id is defined once + /// Default constructor; a token with no id set (delegates to the placement-only constructor) Token() : Token("", "", {}) {} - /// Named constructor; id defaults to -1 as a "not set" sentinel - Token(std::string fileName, std::string containerName, technology::Id technology, int id = -1); + /// Placement-only constructor; leaves the id unset (hasId() == false) + Token(std::string fileName, std::string containerName, technology::Id technology); + + /// Fully-specified constructor; sets the 0-based row/entry id (hasId() == true) + Token(std::string fileName, + std::string containerName, + technology::Id technology, + std::uint64_t id); /// Access file name std::string const& fileName() const; @@ -26,8 +33,10 @@ namespace form::detail::experimental { /// Access technology type technology::Id technology() const; - /// Access identifier/entry number - int id() const; + /// Access identifier/entry number (0-based row). Only meaningful when hasId() is true. + std::uint64_t id() const; + /// Whether an id has been set on this token + bool hasId() const; private: /// Technology identifier @@ -36,8 +45,10 @@ namespace form::detail::experimental { std::string m_fileName; /// Container name std::string m_containerName; - /// Identifier/entry number - int m_id; + /// Identifier/entry number (0-based row) + std::uint64_t m_id; + /// Whether m_id holds a valid, set value + bool m_hasId; }; } // namespace form::detail::experimental #endif // FORM_CORE_TOKEN_HPP diff --git a/form/persistence/ipersistence_writer.hpp b/form/persistence/ipersistence_writer.hpp index 4f15f3797..39a51826d 100644 --- a/form/persistence/ipersistence_writer.hpp +++ b/form/persistence/ipersistence_writer.hpp @@ -3,6 +3,8 @@ #ifndef FORM_PERSISTENCE_IPERSISTENCE_WRITER_HPP #define FORM_PERSISTENCE_IPERSISTENCE_WRITER_HPP +#include "core/token.hpp" + #include #include #include @@ -27,11 +29,12 @@ namespace form::detail::experimental { virtual void createContainers(std::string const& creator, std::map const& products) = 0; - // Write one product and return the 0-based row (entry) number it landed in. - virtual int registerWrite(std::string const& creator, - std::string const& label, - void const* data, - std::type_info const& type) = 0; + // Write one product and return a Token locating it: placement plus 0-based row (entry) number + // If the backend does not address rows, the returned Token has no id set (hasId() == false). + virtual Token registerWrite(std::string const& creator, + std::string const& label, + void const* data, + std::type_info const& type) = 0; virtual void commitOutput(std::string const& creator, std::string const& id) = 0; }; diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index 0d381c7d1..bdc333534 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -48,13 +48,17 @@ void PersistenceWriter::createContainers( m_store_writer->createContainers(containers, m_tech_settings); } -int PersistenceWriter::registerWrite(std::string const& creator, - std::string const& label, - void const* data, - std::type_info const& type) +Token PersistenceWriter::registerWrite(std::string const& creator, + std::string const& label, + void const* data, + std::type_info const& type) { std::unique_ptr plcmnt = getPlacement(creator, label); - return m_store_writer->fillContainer(*plcmnt, data, type); + std::uint64_t const row = m_store_writer->fillContainer(*plcmnt, data, type); + if (row == kInvalidRowId) { + return Token{plcmnt->fileName(), plcmnt->containerName(), plcmnt->technology()}; + } + return Token{plcmnt->fileName(), plcmnt->containerName(), plcmnt->technology(), row}; } void PersistenceWriter::commitOutput(std::string const& creator, std::string const& id) diff --git a/form/persistence/persistence_writer.hpp b/form/persistence/persistence_writer.hpp index c171fc0cb..6d514255b 100644 --- a/form/persistence/persistence_writer.hpp +++ b/form/persistence/persistence_writer.hpp @@ -31,10 +31,10 @@ namespace form::detail::experimental { void createContainers(std::string const& creator, std::map const& products) override; - int registerWrite(std::string const& creator, - std::string const& label, - void const* data, - std::type_info const& type) override; + Token registerWrite(std::string const& creator, + std::string const& label, + void const* data, + std::type_info const& type) override; void commitOutput(std::string const& creator, std::string const& id) override; private: diff --git a/form/root_storage/root_rfield_write_container.cpp b/form/root_storage/root_rfield_write_container.cpp index 04e66ba91..6a7b753df 100644 --- a/form/root_storage/root_rfield_write_container.cpp +++ b/form/root_storage/root_rfield_write_container.cpp @@ -61,7 +61,7 @@ namespace form::detail::experimental { m_rntuple_parent = parentDerived; } - int ROOT_RField_Write_ContainerImp::fill(void const* data) + std::uint64_t ROOT_RField_Write_ContainerImp::fill(void const* data) { if (!m_rntuple_parent) { throw std::runtime_error( @@ -83,7 +83,7 @@ namespace form::detail::experimental { // Unlike a TBranch, an RNTuple entry is only written on commit(); // every field bound before that commit shares one entry. // Return the 0-based index that pending entry will occupy (the current entry count). - return static_cast(m_rntuple_parent->m_writer->GetNEntries()); + return static_cast(m_rntuple_parent->m_writer->GetNEntries()); } void ROOT_RField_Write_ContainerImp::commit() diff --git a/form/root_storage/root_rfield_write_container.hpp b/form/root_storage/root_rfield_write_container.hpp index f163e1918..833086b87 100644 --- a/form/root_storage/root_rfield_write_container.hpp +++ b/form/root_storage/root_rfield_write_container.hpp @@ -23,7 +23,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type) override; void setParent(std::shared_ptr const parent) override; - int fill(void const* data) override; + std::uint64_t fill(void const* data) override; void commit() override; private: diff --git a/form/root_storage/root_rntuple_write_container.cpp b/form/root_storage/root_rntuple_write_container.cpp index 92381277b..e3fb4bbe5 100644 --- a/form/root_storage/root_rntuple_write_container.cpp +++ b/form/root_storage/root_rntuple_write_container.cpp @@ -29,7 +29,7 @@ namespace form::detail::experimental { return; } - int ROOT_RNTuple_Write_ContainerImp::fill(void const* /*data*/) + std::uint64_t ROOT_RNTuple_Write_ContainerImp::fill(void const* /*data*/) { throw std::runtime_error("ROOT_RNTuple_Write_ContainerImp::fill not implemented"); } diff --git a/form/root_storage/root_rntuple_write_container.hpp b/form/root_storage/root_rntuple_write_container.hpp index 423edc5f7..8b2e2a6d5 100644 --- a/form/root_storage/root_rntuple_write_container.hpp +++ b/form/root_storage/root_rntuple_write_container.hpp @@ -52,7 +52,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type) override; - int fill(void const* data) override; + std::uint64_t fill(void const* data) override; void commit() override; //State shared by ROOT_RField_ContainerImps diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index c4aaac35f..25cca07fb 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -94,7 +94,7 @@ void ROOT_TBranch_Write_ContainerImp::setupWrite(std::type_info const& type) } } -int ROOT_TBranch_Write_ContainerImp::fill(void const* data) +std::uint64_t ROOT_TBranch_Write_ContainerImp::fill(void const* data) { // NOTE: incoming parameter `data` is `const` due to the constraints on how we // expect users to interact with the data; however, ROOT's SetBranchAddress @@ -115,8 +115,9 @@ int ROOT_TBranch_Write_ContainerImp::fill(void const* data) m_branch->Fill(); m_branch->ResetAddress(); - // 0-based entries: GetEntries() is the total count after this Fill(); row = count - 1 - return static_cast(m_branch->GetEntries()) - 1; + // 0-based entries: GetEntries() is the total count after this Fill(); row = count - 1. + // GetEntries() >= 1 here (we just filled), so the row is non-negative. + return static_cast(m_branch->GetEntries() - 1); } void ROOT_TBranch_Write_ContainerImp::commit() diff --git a/form/root_storage/root_tbranch_write_container.hpp b/form/root_storage/root_tbranch_write_container.hpp index 485e9f627..154ceadb7 100644 --- a/form/root_storage/root_tbranch_write_container.hpp +++ b/form/root_storage/root_tbranch_write_container.hpp @@ -25,7 +25,7 @@ namespace form::detail::experimental { void setParent(std::shared_ptr parent) override; void setupWrite(std::type_info const& type = typeid(void)) override; - int fill(void const* data) override; + std::uint64_t fill(void const* data) override; void commit() override; private: diff --git a/form/root_storage/root_ttree_write_container.cpp b/form/root_storage/root_ttree_write_container.cpp index 604a4e766..7f74a4cc8 100644 --- a/form/root_storage/root_ttree_write_container.cpp +++ b/form/root_storage/root_ttree_write_container.cpp @@ -45,7 +45,7 @@ void ROOT_TTree_Write_ContainerImp::setupWrite(std::type_info const& /* type*/) } } -int ROOT_TTree_Write_ContainerImp::fill(void const* /* data*/) +std::uint64_t ROOT_TTree_Write_ContainerImp::fill(void const* /* data*/) { throw std::runtime_error("ROOT_TTree_Write_ContainerImp::fill not implemented"); } diff --git a/form/root_storage/root_ttree_write_container.hpp b/form/root_storage/root_ttree_write_container.hpp index 7f0e22c91..cc34a7a24 100644 --- a/form/root_storage/root_ttree_write_container.hpp +++ b/form/root_storage/root_ttree_write_container.hpp @@ -25,7 +25,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type) override; - int fill(void const* data) override; + std::uint64_t fill(void const* data) override; void commit() override; TTree* getTTree(); diff --git a/form/storage/istorage.hpp b/form/storage/istorage.hpp index b6a6f9643..71f9fd169 100644 --- a/form/storage/istorage.hpp +++ b/form/storage/istorage.hpp @@ -7,6 +7,8 @@ #include "core/token.hpp" #include "form/config.hpp" +#include +#include #include #include #include @@ -14,6 +16,10 @@ namespace form::detail::experimental { + // Sentinel returned by the write chain when no addressable row was written + // (e.g. the generic no-op container). A real row is always < this value. + inline constexpr std::uint64_t kInvalidRowId = std::numeric_limits::max(); + class IStorageReader { public: IStorageReader() = default; @@ -41,10 +47,10 @@ namespace form::detail::experimental { virtual void createContainers( std::map, std::type_info const*> const& containers, form::experimental::config::tech_setting_config const& settings) = 0; - // Returns the 0-based row (entry) number the data was written to - virtual int fillContainer(Placement const& plcmnt, - void const* data, - std::type_info const& type) = 0; + // Returns the 0-based row (entry) number written, or kInvalidRowId if no rows + virtual std::uint64_t fillContainer(Placement const& plcmnt, + void const* data, + std::type_info const& type) = 0; virtual void commitContainers(Placement const& plcmnt) = 0; }; @@ -68,8 +74,8 @@ namespace form::detail::experimental { virtual void setFile(std::shared_ptr file) = 0; virtual void setupWrite(std::type_info const& type = typeid(void)) = 0; - // Returns the 0-based row (entry) number written or -1 if no rows - virtual int fill(void const* data) = 0; + // Returns the 0-based row (entry) number written, or kInvalidRowId if no rows + virtual std::uint64_t fill(void const* data) = 0; virtual void commit() = 0; virtual void setAttribute(std::string const& name, std::string const& value) = 0; diff --git a/form/storage/storage_reader.cpp b/form/storage/storage_reader.cpp index ec642483a..42bdbcc80 100644 --- a/form/storage/storage_reader.cpp +++ b/form/storage/storage_reader.cpp @@ -388,5 +388,6 @@ void StorageReader::readContainer(Token const& token, cont->second->setAttribute(key, value); } } - cont->second->read(token.id(), data, type); + // TODO: Token::id() is a 64-bit row; the read container interface still takes an int entry. Narrow explicitly here (exact for all realistic row counts). Widening the read path to 64-bit is a follow-up PR. + cont->second->read(static_cast(token.id()), data, type); } diff --git a/form/storage/storage_write_container.cpp b/form/storage/storage_write_container.cpp index 7a812afda..cabead3a8 100644 --- a/form/storage/storage_write_container.cpp +++ b/form/storage/storage_write_container.cpp @@ -18,7 +18,7 @@ void Storage_Write_Container::setFile(std::shared_ptr file) { m_f void Storage_Write_Container::setupWrite(std::type_info const& /* type*/) {} -int Storage_Write_Container::fill(void const* /* data*/) { return -1; } +std::uint64_t Storage_Write_Container::fill(void const* /* data*/) { return kInvalidRowId; } void Storage_Write_Container::commit() {} diff --git a/form/storage/storage_write_container.hpp b/form/storage/storage_write_container.hpp index fc4557dde..a68ef2987 100644 --- a/form/storage/storage_write_container.hpp +++ b/form/storage/storage_write_container.hpp @@ -20,7 +20,7 @@ namespace form::detail::experimental { void setFile(std::shared_ptr file) override; void setupWrite(std::type_info const& type = typeid(void)) override; - int fill(void const* data) override; + std::uint64_t fill(void const* data) override; void commit() override; void setAttribute(std::string const& name, std::string const& value) override; diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 48beafd33..42ed42831 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -103,9 +103,9 @@ void StorageWriter::createContainers( } } -int StorageWriter::fillContainer(Placement const& plcmnt, - void const* data, - std::type_info const& /* type*/) +std::uint64_t StorageWriter::fillContainer(Placement const& plcmnt, + void const* data, + std::type_info const& /* type*/) { // Use file+container as composite key auto contKey = std::make_pair(plcmnt.fileName(), plcmnt.containerName()); diff --git a/form/storage/storage_writer.hpp b/form/storage/storage_writer.hpp index 0ae627ba7..28e2f2e41 100644 --- a/form/storage/storage_writer.hpp +++ b/form/storage/storage_writer.hpp @@ -23,9 +23,9 @@ namespace form::detail::experimental { void createContainers( std::map, std::type_info const*> const& containers, form::experimental::config::tech_setting_config const& settings) override; - int fillContainer(Placement const& plcmnt, - void const* data, - std::type_info const& type) override; + std::uint64_t fillContainer(Placement const& plcmnt, + void const* data, + std::type_info const& type) override; void commitContainers(Placement const& plcmnt) override; private: diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 12b6e4c21..0e2340929 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -37,8 +37,8 @@ TEST_CASE("Token default constructor", "[form]") CHECK(t.fileName().empty()); CHECK(t.containerName().empty()); CHECK(t.technology() == form::technology::Id{}); - // Default-constructed token must carry the -1 sentinel for id - CHECK(t.id() == -1); + // Default-constructed token has no id set + CHECK_FALSE(t.hasId()); } TEST_CASE("Token basics", "[form]") @@ -47,7 +47,8 @@ TEST_CASE("Token basics", "[form]") CHECK(t.fileName() == "file.root"); CHECK(t.containerName() == "container"); CHECK(t.technology() == form::technology::ROOT_TTREE); - CHECK(t.id() == 42); + CHECK(t.hasId()); + CHECK(t.id() == 42u); } TEST_CASE("technology::Id string conversions", "[form]") diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index 7cd4bc27c..336f54699 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -331,7 +331,7 @@ TEST_CASE("Persistence round-trip: structured index normalization and listing", CHECK((*read_first == first || *read_first == second)); } -TEST_CASE("registerWrite returns the row id used to build a Token", "[form]") +TEST_CASE("registerWrite returns a Token locating the written product", "[form]") { using namespace form::experimental::config; @@ -346,8 +346,8 @@ TEST_CASE("registerWrite returns the row id used to build a Token", "[form]") std::vector const first = {11, 22, 33}; std::vector const second = {44, 55, 66}; - int row_first = -1; - int row_second = -1; + Token token_first; + Token token_second; { auto writer = createPersistenceWriter(); REQUIRE(writer != nullptr); @@ -355,37 +355,67 @@ TEST_CASE("registerWrite returns the row id used to build a Token", "[form]") writer->configureTechSettings(tech_setting_config{}); writer->createContainers(creator, {{"prod", &typeid(std::vector)}}); - row_first = writer->registerWrite(creator, "prod", &first, typeid(std::vector)); + token_first = writer->registerWrite(creator, "prod", &first, typeid(std::vector)); writer->commitOutput(creator, "[event:1, segment:1]"); - row_second = writer->registerWrite(creator, "prod", &second, typeid(std::vector)); + token_second = writer->registerWrite(creator, "prod", &second, typeid(std::vector)); writer->commitOutput(creator, "[event:1, segment:2]"); } - // registerWrite returns 0-based, monotonically increasing rows. - CHECK(row_first == 0); - CHECK(row_second == 1); + // The returned Token carries the placement and the 0-based, monotonically increasing row + CHECK(token_first.hasId()); + CHECK(token_first.id() == 0u); + CHECK(token_first.containerName() == container); + CHECK(token_second.hasId()); + CHECK(token_second.id() == 1u); - // The returned row is all a Token needs (beyond the placement) to locate the - // data again: read each row straight back through a hand-built Token. + // Token returned by the write is directly usable on the read side: no hand-buit Token or re-scan StorageReader reader; tech_setting_config const settings{}; void const* raw = nullptr; - Token const token_first{file_name, container, technology, row_first}; reader.readContainer(token_first, &raw, typeid(std::vector), settings); auto const* got_first = static_cast const*>(raw); REQUIRE(got_first != nullptr); CHECK(*got_first == first); raw = nullptr; - Token const token_second{file_name, container, technology, row_second}; reader.readContainer(token_second, &raw, typeid(std::vector), settings); auto const* got_second = static_cast const*>(raw); REQUIRE(got_second != nullptr); CHECK(*got_second == second); } +TEST_CASE("registerWrite returns a not-set Token when the backend does not address rows", "[form]") +{ + using namespace form::experimental::config; + + // The generic backend's write container is a no-op whose fill() returns kInvalidRowId (no rows) + // registerWrite must map that to a Token with the placement filled in but no id set. + form::technology::Id const generic{}; + std::string const file_name = "registerwrite_notset_row.generic"; + std::string const creator = "notset_creator"; + std::string const container = creator + "/prod"; + + ItemConfig cfg; + cfg.addItem("prod", file_name, generic); + + std::vector const payload = {1, 2, 3}; + + auto writer = createPersistenceWriter(); + REQUIRE(writer != nullptr); + writer->configure(cfg); + writer->configureTechSettings(tech_setting_config{}); + writer->createContainers(creator, {{"prod", &typeid(std::vector)}}); + + Token const token = writer->registerWrite(creator, "prod", &payload, typeid(std::vector)); + + CHECK_FALSE(token.hasId()); + CHECK(token.fileName() == file_name); + CHECK(token.containerName() == container); + CHECK(token.technology() == generic); +} + TEST_CASE("Persistence round-trip: all-zero structured id fallback", "[form]") { using namespace form::experimental::config; From 6c1ee09bf15f0b51961059a03655dbc41caa6740 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Thu, 20 Aug 2026 14:31:51 -0500 Subject: [PATCH 3/7] addressed comments on Handle a failed TBranch::Fill() before creating the Token row' --- form/root_storage/root_tbranch_write_container.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index 25cca07fb..06d1cd8e8 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -112,11 +112,17 @@ std::uint64_t ROOT_TBranch_Write_ContainerImp::fill(void const* data) } else { m_branch->SetAddress(reinterpret_cast(&data)); } - m_branch->Fill(); + // TBranch::Fill() returns the number of bytes committed, or a negative value on a write error. + // ROOT increments entry count before a basket write can fail, so check return value first + Int_t const nbytes = m_branch->Fill(); m_branch->ResetAddress(); + if (nbytes < 0) { + throw std::runtime_error("ROOT_TBranch_Write_ContainerImp::fill TBranch::Fill() failed for " + + col_name()); + } // 0-based entries: GetEntries() is the total count after this Fill(); row = count - 1. - // GetEntries() >= 1 here (we just filled), so the row is non-negative. + // GetEntries() >= 1 here (Fill() succeeded), so the row is non-negative. return static_cast(m_branch->GetEntries() - 1); } From 8f4992088a5d664316b1a3cd2a68e7c609402bbb Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Thu, 20 Aug 2026 14:33:16 -0500 Subject: [PATCH 4/7] addressed comment on 'Own the values returned by StorageReader::readContainer' --- test/form/form_storage_test.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index 336f54699..a38b1dced 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -373,15 +374,18 @@ TEST_CASE("registerWrite returns a Token locating the written product", "[form]" StorageReader reader; tech_setting_config const settings{}; + // readContainer allocates the payload and transfers ownership to the caller. void const* raw = nullptr; reader.readContainer(token_first, &raw, typeid(std::vector), settings); - auto const* got_first = static_cast const*>(raw); + std::unique_ptr const> const got_first( + static_cast const*>(raw)); REQUIRE(got_first != nullptr); CHECK(*got_first == first); raw = nullptr; reader.readContainer(token_second, &raw, typeid(std::vector), settings); - auto const* got_second = static_cast const*>(raw); + std::unique_ptr const> const got_second( + static_cast const*>(raw)); REQUIRE(got_second != nullptr); CHECK(*got_second == second); } From f4be9ab93fbc4fed734241686137071fd9dd96a5 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 21 Aug 2026 10:11:31 -0500 Subject: [PATCH 5/7] add test to cover the case when TBranch::Fill() reports a write error --- test/form/form_storage_test.cpp | 65 +++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index a38b1dced..8f50f24a2 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -5,10 +5,13 @@ #include "form/config.hpp" #include "persistence/persistence_reader.hpp" #include "persistence/persistence_writer.hpp" +#include "root_storage/root_tfile.hpp" +#include "root_storage/root_ttree_write_container.hpp" #include "storage/storage_file.hpp" #include "storage/storage_reader.hpp" #include "storage/storage_write_container.hpp" +#include "TBranch.h" #include "TFile.h" #include "TTree.h" @@ -282,6 +285,68 @@ TEST_CASE("Root TTree write container: fill and commit are not implemented", "[f CHECK_THROWS_AS(writeAssoc->commit(), std::runtime_error); } +TEST_CASE("Root TBranch fill: throws when TBranch::Fill() reports a write error", "[form]") +{ + // Exercises the defensive guard in ROOT_TBranch_Write_ContainerImp::fill(): + // TBranch::Fill() returns a negative value when a basket flush to disk fails, and + // fill() must throw rather than hand back a row id for data that was never persisted. + // + // TBranch is ROOT_TTREE-specific, so this test hard-codes that technology instead of + // using the (CLI-overridable) global `technology`, which may select ROOT_RNTUPLE. + // + // To provoke a deterministic write failure we (1) shrink the branch basket so that a + // handful of fills force a basket flush to disk, and (2) mark the underlying TFile + // non-writable so that flush fails and Fill() returns a negative value. + auto const tech = form::technology::ROOT_TTREE; + + auto file = createFile(tech, "tbranch_fill_write_error.root", 'o'); + auto tree = createWriteAssociation(tech, "faketree"); + auto branch = createWriteContainer(tech, "faketree/fakebranch"); + + tree->setFile(file); + tree->setupWrite(typeid(double)); + + auto branchAssoc = dynamic_pointer_cast(branch); + REQUIRE(branchAssoc != nullptr); + branchAssoc->setParent(tree); + branch->setFile(file); + branch->setupWrite(typeid(double)); + + // Reach the raw ROOT objects created through the factory wiring above. + auto root_file = dynamic_pointer_cast(file); + REQUIRE(root_file != nullptr); + auto* root_tree = dynamic_cast(tree.get()); + REQUIRE(root_tree != nullptr); + + TTree* raw_tree = root_tree->getTTree(); + REQUIRE(raw_tree != nullptr); + TBranch* raw_branch = raw_tree->GetBranch("fakebranch"); + REQUIRE(raw_branch != nullptr); + + // A tiny basket forces a flush after only a few fills. ROOT clamps the minimum to 100 + // bytes, so a handful of 8-byte doubles is enough to overflow it. + raw_branch->SetBasketSize(100); + + // Make the file non-writable so the forced basket flush fails. + std::shared_ptr raw_tfile = root_file->getTFile(); + REQUIRE(raw_tfile != nullptr); + raw_tfile->SetWritable(false); + + // Keep filling until a basket flush is triggered; the flush cannot reach the read-only + // file, TBranch::Fill() returns a negative value, and fill() surfaces it as a throw. + double value = std::numbers::pi; + CHECK_THROWS_AS( + [&] { + for (int i = 0; i < 100000; ++i) { + branch->fill(&value); + } + }(), + std::runtime_error); + + // Restore writability so container teardown (which writes the tree) does not error. + raw_tfile->SetWritable(true); +} + TEST_CASE("Persistence round-trip: structured index normalization and listing", "[form]") { using namespace form::experimental::config; From 3b6d5fb3a8f84025187292e51f970e347586dc05 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 21 Aug 2026 10:12:20 -0500 Subject: [PATCH 6/7] add comment for information --- form/persistence/persistence_writer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index bdc333534..73159f80a 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -55,6 +55,9 @@ Token PersistenceWriter::registerWrite(std::string const& creator, { std::unique_ptr plcmnt = getPlacement(creator, label); std::uint64_t const row = m_store_writer->fillContainer(*plcmnt, data, type); + // kInvalidRowId is not an error: it means the backend does not address rows + // (e.g. the generic no-op container). Real write failures throw in the backend. + // Represent "no addressable row" as a Token with hasId() == false. if (row == kInvalidRowId) { return Token{plcmnt->fileName(), plcmnt->containerName(), plcmnt->technology()}; } From 4c8cc07cbb3539ec583672162a57d273218ec197 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 21 Aug 2026 11:49:54 -0500 Subject: [PATCH 7/7] throw in case of no valid rows for Token --- form/persistence/ipersistence_writer.hpp | 2 +- form/persistence/persistence_writer.cpp | 9 +++++---- test/form/form_storage_test.cpp | 17 +++++++---------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/form/persistence/ipersistence_writer.hpp b/form/persistence/ipersistence_writer.hpp index 39a51826d..6738d18db 100644 --- a/form/persistence/ipersistence_writer.hpp +++ b/form/persistence/ipersistence_writer.hpp @@ -30,7 +30,7 @@ namespace form::detail::experimental { virtual void createContainers(std::string const& creator, std::map const& products) = 0; // Write one product and return a Token locating it: placement plus 0-based row (entry) number - // If the backend does not address rows, the returned Token has no id set (hasId() == false). + // Throws if backend isn't row-addressed, causing Token read lookup to fail virtual Token registerWrite(std::string const& creator, std::string const& label, void const* data, diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index 73159f80a..e808ba4b2 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -55,11 +55,12 @@ Token PersistenceWriter::registerWrite(std::string const& creator, { std::unique_ptr plcmnt = getPlacement(creator, label); std::uint64_t const row = m_store_writer->fillContainer(*plcmnt, data, type); - // kInvalidRowId is not an error: it means the backend does not address rows - // (e.g. the generic no-op container). Real write failures throw in the backend. - // Represent "no addressable row" as a Token with hasId() == false. + // A returned Token must locate a readable product: its row is the read-side navigation key. + // kInvalidRowId means backend does not address rows,so a product routed there could not be located on read, so throw here for such an unusable Token if (row == kInvalidRowId) { - return Token{plcmnt->fileName(), plcmnt->containerName(), plcmnt->technology()}; + throw std::runtime_error("PersistenceWriter::registerWrite backend for product '" + label + + "' from creator '" + creator + "' does not address rows; " + + "cannot produce a Token locating the written product"); } return Token{plcmnt->fileName(), plcmnt->containerName(), plcmnt->technology(), row}; } diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index 8f50f24a2..21a7e128f 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -455,16 +455,17 @@ TEST_CASE("registerWrite returns a Token locating the written product", "[form]" CHECK(*got_second == second); } -TEST_CASE("registerWrite returns a not-set Token when the backend does not address rows", "[form]") +TEST_CASE("registerWrite throws when the backend does not address rows", "[form]") { using namespace form::experimental::config; - // The generic backend's write container is a no-op whose fill() returns kInvalidRowId (no rows) - // registerWrite must map that to a Token with the placement filled in but no id set. + // The generic ("no technology specified") backend's write container is a no-op whose fill() + // returns kInvalidRowId, and its read side is a no-op too, so a product routed there could + // never be located on read. registerWrite must reject that rather than return an unusable + // Token whose row would later be used as the read-side navigation key. form::technology::Id const generic{}; std::string const file_name = "registerwrite_notset_row.generic"; std::string const creator = "notset_creator"; - std::string const container = creator + "/prod"; ItemConfig cfg; cfg.addItem("prod", file_name, generic); @@ -477,12 +478,8 @@ TEST_CASE("registerWrite returns a not-set Token when the backend does not addre writer->configureTechSettings(tech_setting_config{}); writer->createContainers(creator, {{"prod", &typeid(std::vector)}}); - Token const token = writer->registerWrite(creator, "prod", &payload, typeid(std::vector)); - - CHECK_FALSE(token.hasId()); - CHECK(token.fileName() == file_name); - CHECK(token.containerName() == container); - CHECK(token.technology() == generic); + CHECK_THROWS_AS(writer->registerWrite(creator, "prod", &payload, typeid(std::vector)), + std::runtime_error); } TEST_CASE("Persistence round-trip: all-zero structured id fallback", "[form]")