From 4ee6efd9d6c3482a623af9f438625ef00d3b935a Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Thu, 11 Jun 2026 22:03:32 -0500 Subject: [PATCH 01/13] FORM: implement single-entry FileCatalog TTree for file-level metadata in output FORM file --- form/form/form_writer.cpp | 5 ++ form/form/form_writer.hpp | 2 + form/persistence/ipersistence_writer.hpp | 1 + form/persistence/persistence_writer.cpp | 5 ++ form/persistence/persistence_writer.hpp | 1 + form/root_storage/root_tfile.cpp | 21 +++++- form/storage/istorage.hpp | 1 + form/storage/storage_writer.cpp | 92 ++++++++++++++++++++++++ form/storage/storage_writer.hpp | 1 + test/form/writer.cpp | 58 +++++++++++++++ 10 files changed, 185 insertions(+), 2 deletions(-) diff --git a/form/form/form_writer.cpp b/form/form/form_writer.cpp index 6a30c82cb..628f99ab9 100644 --- a/form/form/form_writer.cpp +++ b/form/form/form_writer.cpp @@ -72,4 +72,9 @@ namespace form::experimental { m_pers_writer->commitOutput(creator, segment_id); } + void form_writer_interface::finalize() + { + m_pers_writer->finalize(); + } + } diff --git a/form/form/form_writer.hpp b/form/form/form_writer.hpp index cd6e04e30..597e689bb 100644 --- a/form/form/form_writer.hpp +++ b/form/form/form_writer.hpp @@ -28,6 +28,8 @@ namespace form::experimental { std::string const& segment_id, std::vector const& products); + void finalize(); + private: std::unique_ptr m_pers_writer; std::map m_product_to_config; diff --git a/form/persistence/ipersistence_writer.hpp b/form/persistence/ipersistence_writer.hpp index 0ab36d28a..9e51208eb 100644 --- a/form/persistence/ipersistence_writer.hpp +++ b/form/persistence/ipersistence_writer.hpp @@ -32,6 +32,7 @@ namespace form::detail::experimental { void const* data, std::type_info const& type) = 0; virtual void commitOutput(std::string const& creator, std::string const& id) = 0; + virtual void finalize() = 0; }; std::unique_ptr createPersistenceWriter(); diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index 5df720339..1f07d5b69 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -80,3 +80,8 @@ std::unique_ptr PersistenceWriter::getPlacement(std::string const& cr std::string const full_label = buildFullLabel(creator, label); return std::make_unique(config_item->file_name, full_label, config_item->technology); } + +void PersistenceWriter::finalize() +{ + m_store_writer->finalize(m_tech_settings); +} diff --git a/form/persistence/persistence_writer.hpp b/form/persistence/persistence_writer.hpp index b5f3043cd..9bb207b11 100644 --- a/form/persistence/persistence_writer.hpp +++ b/form/persistence/persistence_writer.hpp @@ -36,6 +36,7 @@ namespace form::detail::experimental { void const* data, std::type_info const& type) override; void commitOutput(std::string const& creator, std::string const& id) override; + void finalize() override; private: std::unique_ptr getPlacement(std::string const& creator, std::string const& label); diff --git a/form/root_storage/root_tfile.cpp b/form/root_storage/root_tfile.cpp index 5d2beb191..0a6f8a5a0 100644 --- a/form/root_storage/root_tfile.cpp +++ b/form/root_storage/root_tfile.cpp @@ -4,14 +4,31 @@ #include "TFile.h" +#include +#include + using namespace form::detail::experimental; ROOT_TFileImp::ROOT_TFileImp(std::string const& name, char mode) : Storage_File(name, mode), m_file(nullptr) { - if (mode == 'c' || mode == 'r' || mode == 'o') { + if (mode == 'c' || mode == 'o') { + // Preserve existing semantics: 'o' recreates the file, matching prior behavior m_file.reset(TFile::Open(name.c_str(), "RECREATE")); - } else { + } else if (mode == 'u') { + // 'u' explicitly means reopen/update an existing file while preserving metadata. + if (std::filesystem::exists(name)) { + m_file.reset(TFile::Open(name.c_str(), "UPDATE")); + } else { + m_file.reset(TFile::Open(name.c_str(), "RECREATE")); + } + } else if (mode == 'r' || mode == 'i') { m_file.reset(TFile::Open(name.c_str(), "READ")); + } else { + throw std::runtime_error(std::string("Unsupported ROOT file open mode: ") + mode); + } + + if (!m_file || m_file->IsZombie()) { + throw std::runtime_error("Failed to open ROOT file: " + name); } } diff --git a/form/storage/istorage.hpp b/form/storage/istorage.hpp index 555d58b2b..3ffcadddb 100644 --- a/form/storage/istorage.hpp +++ b/form/storage/istorage.hpp @@ -40,6 +40,7 @@ namespace form::detail::experimental { void const* data, std::type_info const& type) = 0; virtual void commitContainers(Placement const& plcmnt) = 0; + virtual void finalize(form::experimental::config::tech_setting_config const& settings) = 0; }; class IStorage_File { diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index a4a9194cf..c86536464 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -6,6 +6,17 @@ #include "storage_write_association.hpp" #include "util/factories.hpp" +#include "form/technology.hpp" + +#include +#include +#include + +#ifdef USE_ROOT_STORAGE +#include "root_storage/root_tfile.hpp" +#include "TFile.h" +#include "TTree.h" +#endif using namespace form::detail::experimental; @@ -91,3 +102,84 @@ void StorageWriter::commitContainers(Placement const& plcmnt) cont->second->commit(); return; } + +static std::string generateUUID() +{ + std::random_device rd; + std::mt19937_64 generator(rd()); + std::uniform_int_distribution distribution(0, 0xFFFFFFFFu); + + uint32_t w0 = distribution(generator); + uint32_t w1 = distribution(generator); + uint32_t w2 = distribution(generator); + uint32_t w3 = distribution(generator); + + // RFC 4122 variant 4 UUID: set the version and variant bits. + uint16_t time_hi_and_version = static_cast((w1 >> 16) & 0x0FFFu); + time_hi_and_version |= 0x4000u; + uint16_t clock_seq_hi_and_reserved = static_cast((w2 >> 16) & 0x3FFFu); + clock_seq_hi_and_reserved |= 0x8000u; + + std::ostringstream oss; + oss << std::hex << std::nouppercase << std::setfill('0'); + oss << std::setw(8) << w0 << '-'; + oss << std::setw(4) << static_cast(w1 >> 16) << '-'; + oss << std::setw(4) << time_hi_and_version << '-'; + oss << std::setw(4) << clock_seq_hi_and_reserved << '-'; + oss << std::setw(4) << static_cast(w2 & 0xFFFFu); + oss << std::setw(8) << w3; + + return oss.str(); +} + +void StorageWriter::finalize(form::experimental::config::tech_setting_config const& /* settings */) +{ + // Write FileCatalog metadata tree for each file +#ifdef USE_ROOT_STORAGE + for (auto const& [fileName, file] : m_files) { + // Try to downcast to ROOT file implementation + ROOT_TFileImp* root_file = dynamic_cast(file.get()); + if (root_file != nullptr) { + auto tfile = root_file->getTFile(); + if (tfile) { + // Preserve any existing FileCatalog metadata if reopening the file. + TObject* existing_catalog_obj = tfile->Get("FileCatalog"); + TTree* existing_catalog = static_cast(existing_catalog_obj); + if (existing_catalog != nullptr) { + // Existing FileCatalog already contains the persisted FileUUID. + continue; + } + + // Create FileCatalog tree for new files. + TTree* catalog = new TTree("FileCatalog", "File-level metadata catalog"); + + // Determine FileFormatVersion based on technology + // We need to get technology from somewhere - for now, infer from existing containers + int fileFormatVersion = 1; // Default: ROOT TTree + + // Check if any container in this file is ROOT_RNTUPLE + for (auto const& [key, container] : m_write_containers) { + if (key.first == fileName) { + // We could check technology here, but for now assume ROOT_TTREE (version 1) + // In future, this could be enhanced to detect ROOT_RNTUPLE (version 2) + break; + } + } + + std::string fileUUID = generateUUID(); + catalog->Branch("FileUUID", &fileUUID); + // Add FileFormatVersion branch + catalog->Branch("FileFormatVersion", &fileFormatVersion, "FileFormatVersion/I"); + + // Fill the tree with one entry + catalog->Fill(); + + // Write to file + tfile->WriteTObject(catalog); + delete catalog; + } + } + } +#endif + return; +} diff --git a/form/storage/storage_writer.hpp b/form/storage/storage_writer.hpp index 50679d317..74b8303df 100644 --- a/form/storage/storage_writer.hpp +++ b/form/storage/storage_writer.hpp @@ -27,6 +27,7 @@ namespace form::detail::experimental { void const* data, std::type_info const& type) override; void commitContainers(Placement const& plcmnt) override; + void finalize(form::experimental::config::tech_setting_config const& settings) override; private: std::map> m_files; diff --git a/test/form/writer.cpp b/test/form/writer.cpp index 8f109102d..e5b1c6b68 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -7,6 +7,9 @@ #include "test_utils.hpp" #include "toy_tracker.hpp" +#include +#include + #include #include #include @@ -14,6 +17,7 @@ #include #include #include +#include #include static int const NUMBER_EVENT = 4; @@ -152,5 +156,59 @@ int main(int argc, char** argv) checksum_file.close(); std::cout << "PHLEX: Write done. Checksums saved to " << checksum_filename << '\n'; + + // Finalize to write FileCatalog metadata + form.finalize(); + std::cout << "PHLEX: Finalize done. FileCatalog written to file." << '\n'; + + // Verify that the generated FileUUID exists and is a canonical UUID string. + TFile* root_file = TFile::Open(filename.c_str(), "READ"); + if (root_file == nullptr || root_file->IsZombie()) { + std::cerr << "ERROR: Could not open generated ROOT file for validation: " << filename << '\n'; + return 1; + } + + TTree* catalog = root_file->Get("FileCatalog"); + if (catalog == nullptr) { + std::cerr << "ERROR: FileCatalog tree not found in generated ROOT file." << '\n'; + root_file->Close(); + return 1; + } + + std::string* fileUUID = nullptr; + int fileFormatVersion = -1; + catalog->SetBranchAddress("FileUUID", &fileUUID); + catalog->SetBranchAddress("FileFormatVersion", &fileFormatVersion); + if (catalog->GetEntries() < 1) { + std::cerr << "ERROR: FileCatalog tree has no entries." << '\n'; + root_file->Close(); + return 1; + } + catalog->GetEntry(0); + if (fileUUID == nullptr) { + std::cerr << "ERROR: FileUUID branch did not populate a valid pointer." << '\n'; + root_file->Close(); + return 1; + } + std::string fileUUIDValue = *fileUUID; + + auto isHexChar = [](char c) { + return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f'); + }; + bool validUUID = fileUUIDValue.size() == 36 && + fileUUIDValue[8] == '-' && fileUUIDValue[13] == '-' && + fileUUIDValue[18] == '-' && fileUUIDValue[23] == '-' && + std::all_of(fileUUIDValue.begin(), fileUUIDValue.end(), + [&](char c) { return c == '-' || isHexChar(c); }); + + if (!validUUID) { + std::cerr << "ERROR: FileUUID is not canonical: " << fileUUIDValue << '\n'; + root_file->Close(); + return 1; + } + + std::cout << "PHLEX: FileUUID validated: " << fileUUIDValue << " (version=" << fileFormatVersion << ")" << '\n'; + root_file->Close(); + return 0; } From 55b79ec73b6437634ad936f60beabd2b2337b52a Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Fri, 12 Jun 2026 13:05:16 -0500 Subject: [PATCH 02/13] Added ProductRegistry tree with ProductName, ProcessName, and Producer --- form/form_module.cpp | 9 +++++ form/storage/storage_writer.cpp | 35 ++++++++++++++++++ test/form/form_test_with_output.jsonnet | 33 +++++++++++++++++ test/form/writer.cpp | 47 ++++++++++++++++++++++++- 4 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 test/form/form_test_with_output.jsonnet diff --git a/form/form_module.cpp b/form/form_module.cpp index 573fe9864..dc9efc3df 100644 --- a/form/form_module.cpp +++ b/form/form_module.cpp @@ -47,6 +47,15 @@ namespace { std::make_unique(output_cfg, tech_cfg); } + ~FormOutputModule() + { + if (m_form_interface) { + std::cout << "FormOutputModule destructor: calling finalize() to write metadata\n"; + m_form_interface->finalize(); + std::cout << "FormOutputModule destructor: finalize() completed\n"; + } + } + // This method is called by Phlex - signature must be: void(product_store const&) void save_data_products(phlex::experimental::product_store const& store) { diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index c86536464..ae913f7cf 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #ifdef USE_ROOT_STORAGE #include "root_storage/root_tfile.hpp" @@ -177,6 +178,40 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con // Write to file tfile->WriteTObject(catalog); delete catalog; + + // Create ProductRegistry tree listing payload data product trees for this file. + // The product registry should describe top-level payload TTrees, not individual + // branch or container labels within those trees. + std::set product_names; + for (auto const& [key, container] : m_write_containers) { + if (key.first != fileName) + continue; + std::string const& container_name = key.second; + if (container_name.size() >= 6 && container_name.compare(container_name.size() - 6, 6, "/index") == 0) + continue; + if (container_name.find('/') != std::string::npos) + continue; + product_names.insert(container_name); + } + if (!product_names.empty()) { + TTree* registry = new TTree("ProductRegistry", "Product-level metadata catalog"); + std::string productName; + std::string processName; + std::string producer; + + registry->Branch("ProductName", &productName); + registry->Branch("ProcessName", &processName); + registry->Branch("Producer", &producer); + + for (auto const& nm : product_names) { + productName = nm; + processName = std::string(); // default empty; user can annotate later + producer = nm; // default producer is the top-level creator/tree name + registry->Fill(); + } + tfile->WriteTObject(registry); + delete registry; + } } } } diff --git a/test/form/form_test_with_output.jsonnet b/test/form/form_test_with_output.jsonnet new file mode 100644 index 000000000..ee04ffb58 --- /dev/null +++ b/test/form/form_test_with_output.jsonnet @@ -0,0 +1,33 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { total: 10 }, + }, + }, + sources: { + provider: { + cpp: 'ij_source', + }, + }, + modules: { + add: { + cpp: 'module', + }, + form_output: { + cpp: 'form_module', + // Products written by the form module + products: ['sum', 'i', 'j'], + }, + }, + + // Optional outputs mapping: file -> products written into that file. + // Some test runners/CLI invocations can pick this up to produce a concrete + // ROOT output file for easy inspection (e.g. "output.root"). + outputs: [ + { + file: 'output.root', + products: ['sum', 'i', 'j'], + }, + ], +} diff --git a/test/form/writer.cpp b/test/form/writer.cpp index e5b1c6b68..7f3450312 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -208,7 +209,51 @@ int main(int argc, char** argv) } std::cout << "PHLEX: FileUUID validated: " << fileUUIDValue << " (version=" << fileFormatVersion << ")" << '\n'; - root_file->Close(); + TTree* registry = root_file->Get("ProductRegistry"); + if (registry == nullptr) { + std::cerr << "ERROR: ProductRegistry tree not found in generated ROOT file." << '\n'; + root_file->Close(); + return 1; + } + + std::string* productName = nullptr; + std::string* processName = nullptr; + std::string* producer = nullptr; + registry->SetBranchAddress("ProductName", &productName); + registry->SetBranchAddress("ProcessName", &processName); + registry->SetBranchAddress("Producer", &producer); + + if (registry->GetEntries() == 0) { + std::cerr << "ERROR: ProductRegistry tree has no entries." << '\n'; + root_file->Close(); + return 1; + } + + std::set product_names; + for (int entry = 0; entry < registry->GetEntries(); ++entry) { + registry->GetEntry(entry); + if (productName == nullptr || processName == nullptr || producer == nullptr) { + std::cerr << "ERROR: ProductRegistry branches did not populate valid pointers." << '\n'; + root_file->Close(); + return 1; + } + product_names.insert(*productName); + std::cout << "PHLEX: ProductRegistry entry: ProductName='" << *productName + << "' ProcessName='" << *processName << "' Producer='" << *producer << "'\n"; + } + + if (product_names.empty()) { + std::cerr << "ERROR: ProductRegistry tree contains no product names." << '\n'; + root_file->Close(); + return 1; + } + + std::cout << "PHLEX: ProductRegistry validated: "; + for (auto const& name : product_names) + std::cout << name << " "; + std::cout << '\n'; + + root_file->Close(); return 0; } From 33f089dd181fefafc57a86bb4304ced676de809b Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Sun, 14 Jun 2026 22:26:25 -0500 Subject: [PATCH 03/13] add IndexRegistry TTree for logical and physical locator; use structured ProductID; add ContainerName and PayloadRow for physical location; support 0-based PayloadRow --- form/form/form_writer.cpp | 16 +- form/form/form_writer.hpp | 5 + form/form_module.cpp | 5 + form/persistence/ipersistence_writer.hpp | 3 +- form/persistence/persistence_writer.cpp | 9 +- form/persistence/persistence_writer.hpp | 4 +- .../root_tbranch_write_container.cpp | 8 + .../root_tbranch_write_container.hpp | 2 + .../root_ttree_write_container.cpp | 8 + .../root_ttree_write_container.hpp | 1 + form/storage/istorage.hpp | 4 +- form/storage/storage_write_container.cpp | 5 + form/storage/storage_write_container.hpp | 1 + form/storage/storage_writer.cpp | 515 +++++++++++++++--- form/storage/storage_writer.hpp | 14 +- test/form/writer.cpp | 270 ++++++++- 16 files changed, 793 insertions(+), 77 deletions(-) diff --git a/form/form/form_writer.cpp b/form/form/form_writer.cpp index 628f99ab9..9aa5a5fe3 100644 --- a/form/form/form_writer.cpp +++ b/form/form/form_writer.cpp @@ -37,7 +37,10 @@ namespace form::experimental { std::map products = {{pb.label, pb.type}}; m_pers_writer->createContainers(creator, products); - m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type); + auto declared_it = m_label_to_product_name.find(pb.label); + std::string const& declared_name = + (declared_it != m_label_to_product_name.end()) ? declared_it->second : std::string{}; + m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type, declared_name); m_pers_writer->commitOutput(creator, segment_id); } @@ -66,12 +69,21 @@ namespace form::experimental { for (auto const& pb : products) { // FIXME: We could consider checking id to be identical for all product bases here - m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type); + auto declared_it = m_label_to_product_name.find(pb.label); + std::string const& declared_name = + (declared_it != m_label_to_product_name.end()) ? declared_it->second : std::string{}; + m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type, declared_name); } m_pers_writer->commitOutput(creator, segment_id); } + void form_writer_interface::declareProductName(std::string const& routing_label, + std::string const& product_name) + { + m_label_to_product_name[routing_label] = product_name; + } + void form_writer_interface::finalize() { m_pers_writer->finalize(); diff --git a/form/form/form_writer.hpp b/form/form/form_writer.hpp index 597e689bb..12d588a8f 100644 --- a/form/form/form_writer.hpp +++ b/form/form/form_writer.hpp @@ -30,9 +30,14 @@ namespace form::experimental { void finalize(); + // Explicitly declare the logical ProductName for a given routing label. + // If not called, ProductName defaults to the creator name at write time. + void declareProductName(std::string const& routing_label, std::string const& product_name); + private: std::unique_ptr m_pers_writer; std::map m_product_to_config; + std::map m_label_to_product_name; }; } diff --git a/form/form_module.cpp b/form/form_module.cpp index dc9efc3df..4a6868511 100644 --- a/form/form_module.cpp +++ b/form/form_module.cpp @@ -45,6 +45,11 @@ namespace { // Initialize FORM interface m_form_interface = std::make_unique(output_cfg, tech_cfg); + + // Explicitly declare ProductNames from user config (products list in jsonnet) + for (auto const& product : products_to_save) { + m_form_interface->declareProductName(product, product); + } } ~FormOutputModule() diff --git a/form/persistence/ipersistence_writer.hpp b/form/persistence/ipersistence_writer.hpp index 9e51208eb..89552a4f0 100644 --- a/form/persistence/ipersistence_writer.hpp +++ b/form/persistence/ipersistence_writer.hpp @@ -30,7 +30,8 @@ namespace form::detail::experimental { virtual void registerWrite(std::string const& creator, std::string const& label, void const* data, - std::type_info const& type) = 0; + std::type_info const& type, + std::string const& product_name = "") = 0; virtual void commitOutput(std::string const& creator, std::string const& id) = 0; virtual void finalize() = 0; }; diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index 1f07d5b69..87bd95a8b 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -52,17 +52,20 @@ void PersistenceWriter::createContainers( void PersistenceWriter::registerWrite(std::string const& creator, std::string const& label, void const* data, - std::type_info const& type) + std::type_info const& type, + std::string const& product_name) { + m_current_creator = creator; // Cache creator for use in commitOutput std::unique_ptr plcmnt = getPlacement(creator, label); - m_store_writer->fillContainer(*plcmnt, data, type); + m_store_writer->fillContainer(*plcmnt, data, type, product_name); return; } void PersistenceWriter::commitOutput(std::string const& creator, std::string const& id) { std::unique_ptr plcmnt = getPlacement(creator, "index"); - m_store_writer->fillContainer(*plcmnt, &id, typeid(std::string)); + // Pass m_current_creator as product_name for IndexRegistry + m_store_writer->fillContainer(*plcmnt, &id, typeid(std::string), m_current_creator); m_store_writer->commitContainers(*plcmnt); return; } diff --git a/form/persistence/persistence_writer.hpp b/form/persistence/persistence_writer.hpp index 9bb207b11..be1522054 100644 --- a/form/persistence/persistence_writer.hpp +++ b/form/persistence/persistence_writer.hpp @@ -34,7 +34,8 @@ namespace form::detail::experimental { void registerWrite(std::string const& creator, std::string const& label, void const* data, - std::type_info const& type) override; + std::type_info const& type, + std::string const& product_name = "") override; void commitOutput(std::string const& creator, std::string const& id) override; void finalize() override; @@ -45,6 +46,7 @@ namespace form::detail::experimental { std::unique_ptr m_store_writer; form::experimental::config::ItemConfig m_config_items; form::experimental::config::tech_setting_config m_tech_settings; + std::string m_current_creator; }; } // namespace form::detail::experimental diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index be13fc01e..19ea594fb 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -130,3 +130,11 @@ void ROOT_TBranch_Write_ContainerImp::commit() m_tree->SetEntries(m_branch->GetEntries()); return; } + +std::uint64_t ROOT_TBranch_Write_ContainerImp::getEntryCount() +{ + if (m_tree == nullptr) { + return 0; + } + return m_tree->GetEntries(); +} diff --git a/form/root_storage/root_tbranch_write_container.hpp b/form/root_storage/root_tbranch_write_container.hpp index d61533342..a33f7d42d 100644 --- a/form/root_storage/root_tbranch_write_container.hpp +++ b/form/root_storage/root_tbranch_write_container.hpp @@ -28,6 +28,8 @@ namespace form::detail::experimental { void fill(void const* data) override; void commit() override; + std::uint64_t getEntryCount() override; + private: std::shared_ptr m_tfile; TTree* m_tree{nullptr}; diff --git a/form/root_storage/root_ttree_write_container.cpp b/form/root_storage/root_ttree_write_container.cpp index 5ad222673..4f88c496c 100644 --- a/form/root_storage/root_ttree_write_container.cpp +++ b/form/root_storage/root_ttree_write_container.cpp @@ -57,6 +57,14 @@ void ROOT_TTree_Write_ContainerImp::commit() TTree* ROOT_TTree_Write_ContainerImp::getTTree() { return m_tree.get(); } +std::uint64_t ROOT_TTree_Write_ContainerImp::getEntryCount() +{ + if (m_tree == nullptr) { + return 0; + } + return m_tree->GetEntries(); +} + void ROOT_TTree_Write_ContainerImp::TTreeDeleter::operator()(gsl::owner t) const { if (t) { diff --git a/form/root_storage/root_ttree_write_container.hpp b/form/root_storage/root_ttree_write_container.hpp index 4e20f1a7e..38edfa2e0 100644 --- a/form/root_storage/root_ttree_write_container.hpp +++ b/form/root_storage/root_ttree_write_container.hpp @@ -28,6 +28,7 @@ namespace form::detail::experimental { void commit() override; TTree* getTTree(); + std::uint64_t getEntryCount() override; private: // Be absolutely explicit about the ownership semantics of TTree*, diff --git a/form/storage/istorage.hpp b/form/storage/istorage.hpp index 3ffcadddb..10c73ecd0 100644 --- a/form/storage/istorage.hpp +++ b/form/storage/istorage.hpp @@ -38,7 +38,8 @@ namespace form::detail::experimental { form::experimental::config::tech_setting_config const& settings) = 0; virtual void fillContainer(Placement const& plcmnt, void const* data, - std::type_info const& type) = 0; + std::type_info const& type, + std::string const& product_name = "") = 0; virtual void commitContainers(Placement const& plcmnt) = 0; virtual void finalize(form::experimental::config::tech_setting_config const& settings) = 0; }; @@ -67,6 +68,7 @@ namespace form::detail::experimental { virtual void commit() = 0; virtual void setAttribute(std::string const& name, std::string const& value) = 0; + virtual std::uint64_t getEntryCount() = 0; }; class IStorage_Read_Container { diff --git a/form/storage/storage_write_container.cpp b/form/storage/storage_write_container.cpp index 37742aca3..cd6bf1962 100644 --- a/form/storage/storage_write_container.cpp +++ b/form/storage/storage_write_container.cpp @@ -27,3 +27,8 @@ void Storage_Write_Container::setAttribute(std::string const& /*name*/, "Storage_Write_Container::setAttribute does not accept any attributes for a container named " + m_name); } + +std::uint64_t Storage_Write_Container::getEntryCount() +{ + return 0; +} diff --git a/form/storage/storage_write_container.hpp b/form/storage/storage_write_container.hpp index 198604357..dd8ce4ab5 100644 --- a/form/storage/storage_write_container.hpp +++ b/form/storage/storage_write_container.hpp @@ -24,6 +24,7 @@ namespace form::detail::experimental { void commit() override; void setAttribute(std::string const& name, std::string const& value) override; + std::uint64_t getEntryCount() override; private: std::string m_name; diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index ae913f7cf..482b04819 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -9,6 +9,9 @@ #include "form/technology.hpp" #include +#include +#include +#include #include #include #include @@ -16,11 +19,222 @@ #ifdef USE_ROOT_STORAGE #include "root_storage/root_tfile.hpp" #include "TFile.h" +#include "TObjString.h" #include "TTree.h" #endif using namespace form::detail::experimental; +namespace { + form::experimental::config::tech_setting_config::table_t lookup_file_table( + form::experimental::config::tech_setting_config const& settings, + int technology, + std::string const& file_name) + { + auto const per_tech_it = settings.file_settings.find(technology); + if (per_tech_it == settings.file_settings.end()) { + return {}; + } + auto const file_it = per_tech_it->second.find(file_name); + if (file_it == per_tech_it->second.end()) { + return {}; + } + return file_it->second; + } + + form::experimental::config::tech_setting_config::table_t lookup_container_table( + form::experimental::config::tech_setting_config const& settings, + int technology, + std::string const& container_name) + { + auto const per_tech_it = settings.container_settings.find(technology); + if (per_tech_it == settings.container_settings.end()) { + return {}; + } + auto const container_it = per_tech_it->second.find(container_name); + if (container_it == per_tech_it->second.end()) { + return {}; + } + return container_it->second; + } + + std::string trim_copy(std::string_view input) + { + auto is_space = [](unsigned char c) { return std::isspace(c) != 0; }; + while (!input.empty() && is_space(static_cast(input.front()))) { + input.remove_prefix(1); + } + while (!input.empty() && is_space(static_cast(input.back()))) { + input.remove_suffix(1); + } + return std::string{input}; + } + + bool parse_uint64(std::string const& value_text, std::uint64_t& value) + { + if (value_text.empty()) { + return false; + } + + int base = 10; + if (value_text.size() > 2 && value_text[0] == '0' && + (value_text[1] == 'x' || value_text[1] == 'X')) { + base = 16; + } else { + bool has_hex_letter = false; + for (char ch : value_text) { + if ((ch >= 'a' && ch <= 'f') || (ch >= 'A' && ch <= 'F')) { + has_hex_letter = true; + break; + } + } + if (has_hex_letter) { + base = 16; + } + } + + auto const* begin = value_text.data(); + auto const* end = value_text.data() + value_text.size(); + auto [ptr, ec] = std::from_chars(begin, end, value, base); + return ec == std::errc{} && ptr == end; + } + + bool parse_layer_index_string(std::string const& index_text, + std::vector& schema, + std::vector& values) + { + schema.clear(); + values.clear(); + + auto content = trim_copy(index_text); + if (content.size() >= 2 && content.front() == '[' && content.back() == ']') { + content = content.substr(1, content.size() - 2); + } + + std::size_t start = 0; + while (start < content.size()) { + auto end = content.find_first_of(",;", start); + auto token = trim_copy(content.substr(start, end == std::string::npos ? std::string::npos : end - start)); + if (!token.empty()) { + auto sep_pos = token.find(':'); + if (sep_pos == std::string::npos) { + sep_pos = token.find('='); + } + if (sep_pos == std::string::npos) { + return false; + } + + auto layer_name = trim_copy(token.substr(0, sep_pos)); + auto value_text = trim_copy(token.substr(sep_pos + 1)); + if (layer_name.empty()) { + return false; + } + + std::uint64_t value = 0; + if (!parse_uint64(value_text, value)) { + return false; + } + + schema.push_back(std::move(layer_name)); + values.push_back(value); + } + + if (end == std::string::npos) { + break; + } + start = end + 1; + } + + return !schema.empty() && schema.size() == values.size(); + } + + bool is_index_container(std::string const& container_name) + { + return container_name.size() >= 6 && + container_name.compare(container_name.size() - 6, 6, "/index") == 0; + } + + std::pair split_creator_and_product(std::string const& container_name) + { + std::size_t const slash = container_name.find('/'); + if (slash == std::string::npos) { + return {std::string{}, container_name}; + } + + std::string creator = container_name.substr(0, slash); + std::string product = container_name.substr(slash + 1); + return {std::move(creator), std::move(product)}; + } + + bool setting_key_is_process_name(std::string const& key) + { + std::string lowered; + lowered.reserve(key.size()); + for (char c : key) { + lowered.push_back(static_cast(std::tolower(static_cast(c)))); + } + return lowered == "processname" || lowered == "process_name"; + } + + std::string resolve_process_name(form::experimental::config::tech_setting_config const& settings, + std::string const& file_name) + { + for (auto const& [technology, by_file] : settings.file_settings) { + (void)technology; + auto const file_it = by_file.find(file_name); + if (file_it == by_file.end()) { + continue; + } + for (auto const& [key, value] : file_it->second) { + if (setting_key_is_process_name(key) && !value.empty()) { + return value; + } + } + } + return std::string(); + } + + std::string build_product_id(std::string const& product_name, + std::string const& producer, + std::string const& process_name) + { + std::string product_id; + product_id.reserve(product_name.size() + producer.size() + process_name.size() + 2); + product_id += product_name; + product_id += '|'; + product_id += producer; + product_id += '|'; + product_id += process_name; + return product_id; + } + + std::string serialize_layer_schema(std::vector const& schema) + { + std::ostringstream oss; + oss << '['; + for (std::size_t i = 0; i < schema.size(); ++i) { + if (i > 0) { + oss << ','; + } + oss << '"' << schema[i] << '"'; + } + oss << ']'; + return oss.str(); + } + + std::string schema_key_for_match(std::vector const& schema) + { + std::ostringstream oss; + for (std::size_t i = 0; i < schema.size(); ++i) { + if (i > 0) { + oss << ','; + } + oss << schema[i]; + } + return oss.str(); + } +} + // Factory function implementation namespace form::detail::experimental { std::unique_ptr createStorageWriter() @@ -45,8 +259,8 @@ void StorageWriter::createContainers( m_files .insert({plcmnt->fileName(), createFile(plcmnt->technology(), plcmnt->fileName(), 'o')}) .first; - for (auto const& [key, value] : - settings.getFileTable(plcmnt->technology(), plcmnt->fileName())) + for (auto const& [key, value] : + lookup_file_table(settings, plcmnt->technology(), plcmnt->fileName())) file->second->setAttribute(key, value); } // Create and bind container to file @@ -70,8 +284,8 @@ void StorageWriter::createContainers( } } - for (auto const& [key, value] : - settings.getContainerTable(plcmnt->technology(), plcmnt->containerName())) + for (auto const& [key, value] : + lookup_container_table(settings, plcmnt->technology(), plcmnt->containerName())) container->setAttribute(key, value); container->setFile(file->second); container->setupWrite(*type); @@ -82,7 +296,8 @@ void StorageWriter::createContainers( void StorageWriter::fillContainer(Placement const& plcmnt, void const* data, - std::type_info const& /* type*/) + std::type_info const& /* type*/, + std::string const& product_name) { // Use file+container as composite key auto key = std::make_pair(plcmnt.fileName(), plcmnt.containerName()); @@ -92,6 +307,76 @@ void StorageWriter::fillContainer(Placement const& plcmnt, throw std::runtime_error("StorageWriter::fillContainer Container doesn't exist: " + plcmnt.containerName()); } + + auto const [creator_name, product_from_container] = + split_creator_and_product(plcmnt.containerName()); + (void)product_from_container; + + if (!is_index_container(plcmnt.containerName())) { + if (!creator_name.empty()) { + std::string const logical_product_name = + !product_name.empty() ? product_name : creator_name; + m_productsByProducer[plcmnt.fileName()][creator_name].insert(logical_product_name); + auto& pending_products = m_pendingProductsByProducer[plcmnt.fileName()][creator_name]; + if (std::find(pending_products.begin(), pending_products.end(), logical_product_name) == + pending_products.end()) { + pending_products.push_back(logical_product_name); + } + } + } + + std::uint64_t payload_row = 0; + if (is_index_container(plcmnt.containerName())) { + // Keep payload row 0-based so it maps directly to entry/row ids. + payload_row = cont->second->getEntryCount(); + } + + if (is_index_container(plcmnt.containerName()) && data != nullptr) { + auto const* segment_id = static_cast(data); + std::vector schema; + std::vector values; + if (parse_layer_index_string(*segment_id, schema, values)) { + auto& pending_by_producer = m_pendingProductsByProducer[plcmnt.fileName()]; + auto pending_it = pending_by_producer.find(creator_name); + + std::vector products_for_index; + if (pending_it != pending_by_producer.end()) { + products_for_index = std::move(pending_it->second); + pending_by_producer.erase(pending_it); + } + + if (products_for_index.empty() && !creator_name.empty()) { + auto const all_by_producer = m_productsByProducer.find(plcmnt.fileName()); + if (all_by_producer != m_productsByProducer.end()) { + auto const all_products_it = all_by_producer->second.find(creator_name); + if (all_products_it != all_by_producer->second.end()) { + products_for_index.assign(all_products_it->second.begin(), all_products_it->second.end()); + } + } + } + + if (products_for_index.empty()) { + m_indexLayerSchemas[plcmnt.fileName()].push_back(std::move(schema)); + m_indexLayerValues[plcmnt.fileName()].push_back(std::move(values)); + m_indexProductNames[plcmnt.fileName()].push_back(std::string()); + m_indexProducers[plcmnt.fileName()].push_back(std::string()); + m_indexContainerNames[plcmnt.fileName()].push_back(std::string()); + m_indexPayloadRows[plcmnt.fileName()].push_back(payload_row); + } else { + for (auto const& pending_product : products_for_index) { + m_indexLayerSchemas[plcmnt.fileName()].push_back(schema); + m_indexLayerValues[plcmnt.fileName()].push_back(values); + m_indexProductNames[plcmnt.fileName()].push_back(pending_product); + m_indexProducers[plcmnt.fileName()].push_back(creator_name); + m_indexContainerNames[plcmnt.fileName()].push_back(creator_name); + m_indexPayloadRows[plcmnt.fileName()].push_back(payload_row); + } + } + } + } + + (void)product_name; + cont->second->fill(data); return; } @@ -133,85 +418,183 @@ static std::string generateUUID() return oss.str(); } -void StorageWriter::finalize(form::experimental::config::tech_setting_config const& /* settings */) +void StorageWriter::finalize(form::experimental::config::tech_setting_config const& settings) { // Write FileCatalog metadata tree for each file #ifdef USE_ROOT_STORAGE for (auto const& [fileName, file] : m_files) { // Try to downcast to ROOT file implementation ROOT_TFileImp* root_file = dynamic_cast(file.get()); - if (root_file != nullptr) { - auto tfile = root_file->getTFile(); - if (tfile) { - // Preserve any existing FileCatalog metadata if reopening the file. - TObject* existing_catalog_obj = tfile->Get("FileCatalog"); - TTree* existing_catalog = static_cast(existing_catalog_obj); - if (existing_catalog != nullptr) { - // Existing FileCatalog already contains the persisted FileUUID. - continue; - } + if (root_file == nullptr) { + continue; + } + + auto tfile = root_file->getTFile(); + if (!tfile) { + continue; + } + + // Preserve any existing FileCatalog metadata if reopening the file. + TObject* existing_catalog_obj = tfile->Get("FileCatalog"); + TTree* existing_catalog = static_cast(existing_catalog_obj); + if (existing_catalog != nullptr) { + // Existing FileCatalog already contains the persisted FileUUID. + continue; + } + + // Create FileCatalog tree for new files. + TTree* catalog = new TTree("FileCatalog", "File-level metadata catalog"); + + // Determine FileFormatVersion based on technology + // We need to get technology from somewhere - for now, infer from existing containers + int fileFormatVersion = 1; // Default: ROOT TTree + + // Check if any container in this file is ROOT_RNTUPLE + for (auto const& [key, container] : m_write_containers) { + if (key.first == fileName) { + // We could check technology here, but for now assume ROOT_TTREE (version 1) + // In future, this could be enhanced to detect ROOT_RNTUPLE (version 2) + break; + } + } + + std::string fileUUID = generateUUID(); + catalog->Branch("FileUUID", &fileUUID); + // Add FileFormatVersion branch + catalog->Branch("FileFormatVersion", &fileFormatVersion, "FileFormatVersion/I"); + + // Fill the tree with one entry + catalog->Fill(); + + // Write to file + tfile->WriteTObject(catalog); + delete catalog; - // Create FileCatalog tree for new files. - TTree* catalog = new TTree("FileCatalog", "File-level metadata catalog"); + // Create ProductRegistry tree listing logical product names from user/framework config. + // Use explicit logical product names instead of deriving from container structure. + std::string const user_provided_process_name = resolve_process_name(settings, fileName); - // Determine FileFormatVersion based on technology - // We need to get technology from somewhere - for now, infer from existing containers - int fileFormatVersion = 1; // Default: ROOT TTree + auto products_it = m_productsByProducer.find(fileName); + if (products_it != m_productsByProducer.end() && !products_it->second.empty()) { + auto const& products_by_producer = products_it->second; + TTree* registry = new TTree("ProductRegistry", "Product-level metadata catalog"); + std::string productName; + std::string processName; + std::string producer; + std::string productID; - // Check if any container in this file is ROOT_RNTUPLE - for (auto const& [key, container] : m_write_containers) { - if (key.first == fileName) { - // We could check technology here, but for now assume ROOT_TTREE (version 1) - // In future, this could be enhanced to detect ROOT_RNTUPLE (version 2) - break; + registry->Branch("ProductName", &productName); + registry->Branch("ProcessName", &processName); + registry->Branch("Producer", &producer); + registry->Branch("ProductID", &productID); + + for (auto const& [creator, product_names] : products_by_producer) { + for (auto const& nm : product_names) { + productName = nm; + processName = user_provided_process_name; + producer = creator; + productID = build_product_id(productName, producer, processName); + registry->Fill(); + } + } + tfile->WriteTObject(registry); + delete registry; + } + + auto schema_it = m_indexLayerSchemas.find(fileName); + auto values_it = m_indexLayerValues.find(fileName); + auto idx_products_it = m_indexProductNames.find(fileName); + auto idx_producers_it = m_indexProducers.find(fileName); + auto containers_it = m_indexContainerNames.find(fileName); + auto payload_rows_it = m_indexPayloadRows.find(fileName); + if (schema_it != m_indexLayerSchemas.end() && values_it != m_indexLayerValues.end() && + idx_products_it != m_indexProductNames.end() && idx_producers_it != m_indexProducers.end() && + containers_it != m_indexContainerNames.end() && + payload_rows_it != m_indexPayloadRows.end()) { + auto const& schemas = schema_it->second; + auto const& values = values_it->second; + auto const& product_names = idx_products_it->second; + auto const& producers = idx_producers_it->second; + auto const& container_names = containers_it->second; + auto const& payload_rows = payload_rows_it->second; + if (!schemas.empty() && schemas.size() == values.size() && + schemas.size() == product_names.size() && schemas.size() == producers.size() && + schemas.size() == container_names.size() && schemas.size() == payload_rows.size()) { + std::string canonical_schema_key; + std::size_t canonical_schema_size = 0; + std::size_t canonical_schema_count = 0; + std::map> schema_stats; + for (auto const& schema : schemas) { + std::string schema_key = schema_key_for_match(schema); + auto& [count, size] = schema_stats[schema_key]; + ++count; + size = schema.size(); + if (count > canonical_schema_count || + (count == canonical_schema_count && size > canonical_schema_size)) { + canonical_schema_count = count; + canonical_schema_size = size; + canonical_schema_key = schema_key; } } - std::string fileUUID = generateUUID(); - catalog->Branch("FileUUID", &fileUUID); - // Add FileFormatVersion branch - catalog->Branch("FileFormatVersion", &fileFormatVersion, "FileFormatVersion/I"); + if (canonical_schema_key.empty()) { + continue; + } + + TTree* index_registry = new TTree("IndexRegistry", "Layer index metadata catalog"); + std::vector canonical_schema; + { + std::size_t start = 0; + while (start <= canonical_schema_key.size()) { + std::size_t end = canonical_schema_key.find(',', start); + std::string token = canonical_schema_key.substr(start, + end == std::string::npos + ? std::string::npos + : end - start); + if (!token.empty()) { + canonical_schema.push_back(token); + } + if (end == std::string::npos) { + break; + } + start = end + 1; + } + } - // Fill the tree with one entry - catalog->Fill(); + std::vector layer_values_by_branch(canonical_schema.size(), 0); + for (std::size_t i = 0; i < canonical_schema.size(); ++i) { + std::string const leaf_list = canonical_schema[i] + "/l"; + index_registry->Branch(canonical_schema[i].c_str(), + &layer_values_by_branch[i], + leaf_list.c_str()); + } + std::string index_product_id; + index_registry->Branch("ProductID", &index_product_id); + std::string index_container_name; + index_registry->Branch("ContainerName", &index_container_name); + std::uint64_t index_payload_row = 0; + index_registry->Branch("PayloadRow", &index_payload_row, "PayloadRow/l"); - // Write to file - tfile->WriteTObject(catalog); - delete catalog; + // Keep schema in tree header metadata as ["layer1","layer2",...]. + std::string schema_header = serialize_layer_schema(canonical_schema); + index_registry->GetUserInfo()->Add(new TObjString(schema_header.c_str())); - // Create ProductRegistry tree listing payload data product trees for this file. - // The product registry should describe top-level payload TTrees, not individual - // branch or container labels within those trees. - std::set product_names; - for (auto const& [key, container] : m_write_containers) { - if (key.first != fileName) - continue; - std::string const& container_name = key.second; - if (container_name.size() >= 6 && container_name.compare(container_name.size() - 6, 6, "/index") == 0) + for (std::size_t i = 0; i < schemas.size(); ++i) { + if (schema_key_for_match(schemas[i]) != canonical_schema_key) { continue; - if (container_name.find('/') != std::string::npos) - continue; - product_names.insert(container_name); - } - if (!product_names.empty()) { - TTree* registry = new TTree("ProductRegistry", "Product-level metadata catalog"); - std::string productName; - std::string processName; - std::string producer; - - registry->Branch("ProductName", &productName); - registry->Branch("ProcessName", &processName); - registry->Branch("Producer", &producer); - - for (auto const& nm : product_names) { - productName = nm; - processName = std::string(); // default empty; user can annotate later - producer = nm; // default producer is the top-level creator/tree name - registry->Fill(); } - tfile->WriteTObject(registry); - delete registry; + for (std::size_t layer_idx = 0; layer_idx < canonical_schema.size(); ++layer_idx) { + layer_values_by_branch[layer_idx] = values[i][layer_idx]; + } + index_product_id = + build_product_id(product_names[i], producers[i], user_provided_process_name); + index_container_name = container_names[i]; + index_payload_row = payload_rows[i]; + index_registry->Fill(); } + + tfile->WriteTObject(index_registry); + delete index_registry; } } } diff --git a/form/storage/storage_writer.hpp b/form/storage/storage_writer.hpp index 74b8303df..efec22775 100644 --- a/form/storage/storage_writer.hpp +++ b/form/storage/storage_writer.hpp @@ -7,10 +7,13 @@ #include "storage_utils.hpp" #include +#include #include +#include #include #include #include // for std::pair +#include namespace form::detail::experimental { @@ -25,7 +28,8 @@ namespace form::detail::experimental { form::experimental::config::tech_setting_config const& settings) override; void fillContainer(Placement const& plcmnt, void const* data, - std::type_info const& type) override; + std::type_info const& type, + std::string const& product_name = "") override; void commitContainers(Placement const& plcmnt) override; void finalize(form::experimental::config::tech_setting_config const& settings) override; @@ -36,6 +40,14 @@ namespace form::detail::experimental { pair_hash> m_write_containers; std::map> m_indexMaps; + std::map>> m_indexLayerSchemas; + std::map>> m_indexLayerValues; + std::map> m_indexProductNames; + std::map> m_indexProducers; + std::map> m_indexContainerNames; + std::map> m_indexPayloadRows; + std::map>> m_productsByProducer; + std::map>> m_pendingProductsByProducer; }; } // namespace form::detail::experimental diff --git a/test/form/writer.cpp b/test/form/writer.cpp index 7f3450312..fa35a392f 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -8,6 +8,7 @@ #include "toy_tracker.hpp" #include +#include #include #include @@ -220,9 +221,11 @@ int main(int argc, char** argv) std::string* productName = nullptr; std::string* processName = nullptr; std::string* producer = nullptr; + std::string* productID = nullptr; registry->SetBranchAddress("ProductName", &productName); registry->SetBranchAddress("ProcessName", &processName); registry->SetBranchAddress("Producer", &producer); + registry->SetBranchAddress("ProductID", &productID); if (registry->GetEntries() == 0) { std::cerr << "ERROR: ProductRegistry tree has no entries." << '\n'; @@ -231,16 +234,28 @@ int main(int argc, char** argv) } std::set product_names; + std::set product_ids; for (int entry = 0; entry < registry->GetEntries(); ++entry) { registry->GetEntry(entry); - if (productName == nullptr || processName == nullptr || producer == nullptr) { + if (productName == nullptr || processName == nullptr || producer == nullptr || + productID == nullptr) { std::cerr << "ERROR: ProductRegistry branches did not populate valid pointers." << '\n'; root_file->Close(); return 1; } + std::string const expected_product_id = + *productName + "|" + *producer + "|" + *processName; + if (*productID != expected_product_id) { + std::cerr << "ERROR: ProductRegistry ProductID mismatch. expected='" + << expected_product_id << "' got='" << *productID << "'." << '\n'; + root_file->Close(); + return 1; + } product_names.insert(*productName); + product_ids.insert(*productID); std::cout << "PHLEX: ProductRegistry entry: ProductName='" << *productName - << "' ProcessName='" << *processName << "' Producer='" << *producer << "'\n"; + << "' ProcessName='" << *processName << "' Producer='" << *producer + << "' ProductID='" << *productID << "'\n"; } if (product_names.empty()) { @@ -254,6 +269,257 @@ int main(int argc, char** argv) std::cout << name << " "; std::cout << '\n'; + TTree* index_registry = root_file->Get("IndexRegistry"); + if (index_registry == nullptr) { + std::cerr << "ERROR: IndexRegistry tree not found in generated ROOT file." << '\n'; + root_file->Close(); + return 1; + } + + TObjString* layer_schema_meta = nullptr; + auto* user_info = index_registry->GetUserInfo(); + if (user_info != nullptr) { + for (int i = 0; i < user_info->GetEntries(); ++i) { + auto* obj = user_info->At(i); + auto* candidate = dynamic_cast(obj); + if (candidate == nullptr) { + continue; + } + std::string payload = candidate->GetString().Data(); + if (payload.size() >= 2 && payload.front() == '[' && payload.back() == ']') { + layer_schema_meta = candidate; + break; + } + } + } + + if (layer_schema_meta == nullptr) { + std::cerr << "ERROR: IndexRegistry header does not contain LayerSchema metadata." << '\n'; + root_file->Close(); + return 1; + } + + std::vector header_schema; + { + std::string schema_text = layer_schema_meta->GetString().Data(); + if (schema_text.size() >= 2 && schema_text.front() == '[' && schema_text.back() == ']') { + schema_text = schema_text.substr(1, schema_text.size() - 2); + } + + std::size_t start = 0; + while (start <= schema_text.size()) { + std::size_t end = schema_text.find(',', start); + std::string token = schema_text.substr(start, + end == std::string::npos ? std::string::npos : end - start); + if (token.size() >= 2 && token.front() == '"' && token.back() == '"') { + token = token.substr(1, token.size() - 2); + } + if (!token.empty()) { + header_schema.push_back(token); + } + if (end == std::string::npos) { + break; + } + start = end + 1; + } + } + + if (header_schema.empty()) { + std::cerr << "ERROR: IndexRegistry LayerSchema header is empty." << '\n'; + root_file->Close(); + return 1; + } + + if (index_registry->GetEntries() == 0) { + std::cerr << "ERROR: IndexRegistry tree has no entries." << '\n'; + root_file->Close(); + return 1; + } + + auto* branch_list = index_registry->GetListOfBranches(); + if (branch_list == nullptr) { + std::cerr << "ERROR: IndexRegistry has no branch list." << '\n'; + root_file->Close(); + return 1; + } + if (branch_list->GetEntries() != static_cast(header_schema.size()) + 3) { + std::cerr << "ERROR: IndexRegistry branch count does not match LayerSchema size + ProductID + ContainerName + PayloadRow." << '\n'; + root_file->Close(); + return 1; + } + for (std::size_t i = 0; i < header_schema.size(); ++i) { + auto* branch_obj = branch_list->At(static_cast(i)); + if (branch_obj == nullptr) { + std::cerr << "ERROR: IndexRegistry has null branch object at position " << i << "." << '\n'; + root_file->Close(); + return 1; + } + if (header_schema[i] != branch_obj->GetName()) { + std::cerr << "ERROR: IndexRegistry branch order does not match LayerSchema at position " + << i << ": expected '" << header_schema[i] << "' got '" << branch_obj->GetName() + << "'." << '\n'; + root_file->Close(); + return 1; + } + } + auto* product_branch_obj = branch_list->At(static_cast(header_schema.size())); + if (product_branch_obj == nullptr || std::string(product_branch_obj->GetName()) != "ProductID") { + std::cerr << "ERROR: IndexRegistry branch order missing ProductID after layer branches." << '\n'; + root_file->Close(); + return 1; + } + auto* container_branch_obj = branch_list->At(static_cast(header_schema.size()) + 1); + if (container_branch_obj == nullptr || + std::string(container_branch_obj->GetName()) != "ContainerName") { + std::cerr << "ERROR: IndexRegistry branch order missing ContainerName after ProductID." << '\n'; + root_file->Close(); + return 1; + } + auto* payload_row_branch_obj = branch_list->At(static_cast(header_schema.size()) + 2); + if (payload_row_branch_obj == nullptr || + std::string(payload_row_branch_obj->GetName()) != "PayloadRow") { + std::cerr << "ERROR: IndexRegistry branch order missing PayloadRow after ContainerName." << '\n'; + root_file->Close(); + return 1; + } + + std::vector layer_branch_values(header_schema.size(), 0); + for (std::size_t i = 0; i < header_schema.size(); ++i) { + if (index_registry->GetBranch(header_schema[i].c_str()) == nullptr) { + std::cerr << "ERROR: IndexRegistry branch missing for layer '" << header_schema[i] << "'." << '\n'; + root_file->Close(); + return 1; + } + index_registry->SetBranchAddress(header_schema[i].c_str(), &layer_branch_values[i]); + } + std::string* product_id = nullptr; + if (index_registry->GetBranch("ProductID") == nullptr) { + std::cerr << "ERROR: IndexRegistry ProductID branch missing." << '\n'; + root_file->Close(); + return 1; + } + index_registry->SetBranchAddress("ProductID", &product_id); + std::string* container_name = nullptr; + if (index_registry->GetBranch("ContainerName") == nullptr) { + std::cerr << "ERROR: IndexRegistry ContainerName branch missing." << '\n'; + root_file->Close(); + return 1; + } + index_registry->SetBranchAddress("ContainerName", &container_name); + unsigned long long payload_row = 0; + if (index_registry->GetBranch("PayloadRow") == nullptr) { + std::cerr << "ERROR: IndexRegistry PayloadRow branch missing." << '\n'; + root_file->Close(); + return 1; + } + index_registry->SetBranchAddress("PayloadRow", &payload_row); + + int const sample_entries = index_registry->GetEntries() < 8 ? index_registry->GetEntries() : 8; + unsigned long long prev_event = 0; + unsigned long long prev_seg = 0; + std::set observed_product_ids; + bool has_prev = false; + for (int entry = 0; entry < index_registry->GetEntries(); ++entry) { + index_registry->GetEntry(entry); + if (layer_branch_values.empty()) { + std::cerr << "ERROR: IndexRegistry layer branch values are empty." << '\n'; + root_file->Close(); + return 1; + } + if (product_id == nullptr || product_id->empty()) { + std::cerr << "ERROR: IndexRegistry ProductID branch did not populate valid value." << '\n'; + root_file->Close(); + return 1; + } + if (container_name == nullptr || container_name->empty()) { + std::cerr << "ERROR: IndexRegistry ContainerName branch did not populate valid value." << '\n'; + root_file->Close(); + return 1; + } + // ContainerName must be a top-level TTree/RNTuple name (no '/' slash) + if (container_name->find('/') != std::string::npos) { + std::cerr << "ERROR: IndexRegistry ContainerName should be top-level container name, got: '" + << *container_name << "'." << '\n'; + root_file->Close(); + return 1; + } + + unsigned long long const expected_payload_row = static_cast(entry); + if (payload_row != expected_payload_row) { + std::cerr << "ERROR: IndexRegistry PayloadRow mismatch at entry " << entry + << ": expected " << expected_payload_row << " got " << payload_row << "." << '\n'; + root_file->Close(); + return 1; + } + + observed_product_ids.insert(*product_id); + + if (entry < sample_entries && header_schema.size() >= 2) { + unsigned long long const event_val = layer_branch_values[0]; + unsigned long long const seg_val = layer_branch_values[1]; + + if (event_val >= static_cast(NUMBER_EVENT)) { + std::cerr << "ERROR: EVENT value out of range in IndexRegistry sample: " << event_val << '\n'; + root_file->Close(); + return 1; + } + if (seg_val >= static_cast(NUMBER_SEGMENT)) { + std::cerr << "ERROR: SEG value out of range in IndexRegistry sample: " << seg_val << '\n'; + root_file->Close(); + return 1; + } + + if (has_prev) { + bool const monotonic_ok = + (event_val > prev_event && seg_val == 0) || + (event_val == prev_event && seg_val > prev_seg) || + (event_val == prev_event && seg_val == prev_seg); + if (!monotonic_ok) { + std::cerr << "ERROR: IndexRegistry sample entries are not monotonic by EVENT/SEG ordering." << '\n'; + root_file->Close(); + return 1; + } + } + + prev_event = event_val; + prev_seg = seg_val; + has_prev = true; + } + } + + if (header_schema.size() != 2 || header_schema[0] != "EVENT" || header_schema[1] != "SEG") { + std::cerr << "ERROR: IndexRegistry LayerSchema header is not EVENT,SEG as expected for this test." << '\n'; + root_file->Close(); + return 1; + } + + if (product_ids.empty()) { + std::cerr << "ERROR: ProductRegistry ProductID values are empty." << '\n'; + root_file->Close(); + return 1; + } + + if (observed_product_ids.empty()) { + std::cerr << "ERROR: IndexRegistry ProductID values are empty." << '\n'; + root_file->Close(); + return 1; + } + + for (auto const& observed_id : observed_product_ids) { + if (!product_ids.contains(observed_id)) { + std::cerr << "ERROR: IndexRegistry ProductID value not present in ProductRegistry: " + << observed_id << '\n'; + root_file->Close(); + return 1; + } + } + + std::cout << "PHLEX: ProductRegistry validated with " << product_names.size() + << " product names and " << product_ids.size() << " product IDs" << '\n'; + + std::cout << "PHLEX: IndexRegistry validated with " << index_registry->GetEntries() + << " entries" << '\n'; + root_file->Close(); return 0; } From d96ea02b48a38c55b135681b17d6b90f89e0e36f Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 11:22:39 -0500 Subject: [PATCH 04/13] apply clang-format --- form/form/form_writer.cpp | 7 +- form/persistence/persistence_writer.cpp | 7 +- form/storage/storage_write_container.cpp | 5 +- form/storage/storage_writer.cpp | 43 ++++++------ form/storage/storage_writer.hpp | 5 +- test/form/writer.cpp | 83 +++++++++++++----------- 6 files changed, 75 insertions(+), 75 deletions(-) diff --git a/form/form/form_writer.cpp b/form/form/form_writer.cpp index 9aa5a5fe3..b2aeb449a 100644 --- a/form/form/form_writer.cpp +++ b/form/form/form_writer.cpp @@ -79,14 +79,11 @@ namespace form::experimental { } void form_writer_interface::declareProductName(std::string const& routing_label, - std::string const& product_name) + std::string const& product_name) { m_label_to_product_name[routing_label] = product_name; } - void form_writer_interface::finalize() - { - m_pers_writer->finalize(); - } + void form_writer_interface::finalize() { m_pers_writer->finalize(); } } diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index 87bd95a8b..d53a7fc8a 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -55,7 +55,7 @@ void PersistenceWriter::registerWrite(std::string const& creator, std::type_info const& type, std::string const& product_name) { - m_current_creator = creator; // Cache creator for use in commitOutput + m_current_creator = creator; // Cache creator for use in commitOutput std::unique_ptr plcmnt = getPlacement(creator, label); m_store_writer->fillContainer(*plcmnt, data, type, product_name); return; @@ -84,7 +84,4 @@ std::unique_ptr PersistenceWriter::getPlacement(std::string const& cr return std::make_unique(config_item->file_name, full_label, config_item->technology); } -void PersistenceWriter::finalize() -{ - m_store_writer->finalize(m_tech_settings); -} +void PersistenceWriter::finalize() { m_store_writer->finalize(m_tech_settings); } diff --git a/form/storage/storage_write_container.cpp b/form/storage/storage_write_container.cpp index cd6bf1962..5aaceda6e 100644 --- a/form/storage/storage_write_container.cpp +++ b/form/storage/storage_write_container.cpp @@ -28,7 +28,4 @@ void Storage_Write_Container::setAttribute(std::string const& /*name*/, m_name); } -std::uint64_t Storage_Write_Container::getEntryCount() -{ - return 0; -} +std::uint64_t Storage_Write_Container::getEntryCount() { return 0; } diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 482b04819..898b92549 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -5,22 +5,22 @@ #include "storage_file.hpp" #include "storage_write_association.hpp" -#include "util/factories.hpp" #include "form/technology.hpp" +#include "util/factories.hpp" -#include #include -#include #include +#include +#include #include -#include #include +#include #ifdef USE_ROOT_STORAGE -#include "root_storage/root_tfile.hpp" #include "TFile.h" #include "TObjString.h" #include "TTree.h" +#include "root_storage/root_tfile.hpp" #endif using namespace form::detail::experimental; @@ -114,7 +114,8 @@ namespace { std::size_t start = 0; while (start < content.size()) { auto end = content.find_first_of(",;", start); - auto token = trim_copy(content.substr(start, end == std::string::npos ? std::string::npos : end - start)); + auto token = trim_copy( + content.substr(start, end == std::string::npos ? std::string::npos : end - start)); if (!token.empty()) { auto sep_pos = token.find(':'); if (sep_pos == std::string::npos) { @@ -259,7 +260,7 @@ void StorageWriter::createContainers( m_files .insert({plcmnt->fileName(), createFile(plcmnt->technology(), plcmnt->fileName(), 'o')}) .first; - for (auto const& [key, value] : + for (auto const& [key, value] : lookup_file_table(settings, plcmnt->technology(), plcmnt->fileName())) file->second->setAttribute(key, value); } @@ -284,8 +285,8 @@ void StorageWriter::createContainers( } } - for (auto const& [key, value] : - lookup_container_table(settings, plcmnt->technology(), plcmnt->containerName())) + for (auto const& [key, value] : + lookup_container_table(settings, plcmnt->technology(), plcmnt->containerName())) container->setAttribute(key, value); container->setFile(file->second); container->setupWrite(*type); @@ -314,8 +315,7 @@ void StorageWriter::fillContainer(Placement const& plcmnt, if (!is_index_container(plcmnt.containerName())) { if (!creator_name.empty()) { - std::string const logical_product_name = - !product_name.empty() ? product_name : creator_name; + std::string const logical_product_name = !product_name.empty() ? product_name : creator_name; m_productsByProducer[plcmnt.fileName()][creator_name].insert(logical_product_name); auto& pending_products = m_pendingProductsByProducer[plcmnt.fileName()][creator_name]; if (std::find(pending_products.begin(), pending_products.end(), logical_product_name) == @@ -350,7 +350,8 @@ void StorageWriter::fillContainer(Placement const& plcmnt, if (all_by_producer != m_productsByProducer.end()) { auto const all_products_it = all_by_producer->second.find(creator_name); if (all_products_it != all_by_producer->second.end()) { - products_for_index.assign(all_products_it->second.begin(), all_products_it->second.end()); + products_for_index.assign(all_products_it->second.begin(), + all_products_it->second.end()); } } } @@ -508,7 +509,8 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con auto containers_it = m_indexContainerNames.find(fileName); auto payload_rows_it = m_indexPayloadRows.find(fileName); if (schema_it != m_indexLayerSchemas.end() && values_it != m_indexLayerValues.end() && - idx_products_it != m_indexProductNames.end() && idx_producers_it != m_indexProducers.end() && + idx_products_it != m_indexProductNames.end() && + idx_producers_it != m_indexProducers.end() && containers_it != m_indexContainerNames.end() && payload_rows_it != m_indexPayloadRows.end()) { auto const& schemas = schema_it->second; @@ -518,8 +520,8 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con auto const& container_names = containers_it->second; auto const& payload_rows = payload_rows_it->second; if (!schemas.empty() && schemas.size() == values.size() && - schemas.size() == product_names.size() && schemas.size() == producers.size() && - schemas.size() == container_names.size() && schemas.size() == payload_rows.size()) { + schemas.size() == product_names.size() && schemas.size() == producers.size() && + schemas.size() == container_names.size() && schemas.size() == payload_rows.size()) { std::string canonical_schema_key; std::size_t canonical_schema_size = 0; std::size_t canonical_schema_count = 0; @@ -547,10 +549,8 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con std::size_t start = 0; while (start <= canonical_schema_key.size()) { std::size_t end = canonical_schema_key.find(',', start); - std::string token = canonical_schema_key.substr(start, - end == std::string::npos - ? std::string::npos - : end - start); + std::string token = canonical_schema_key.substr( + start, end == std::string::npos ? std::string::npos : end - start); if (!token.empty()) { canonical_schema.push_back(token); } @@ -564,9 +564,8 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con std::vector layer_values_by_branch(canonical_schema.size(), 0); for (std::size_t i = 0; i < canonical_schema.size(); ++i) { std::string const leaf_list = canonical_schema[i] + "/l"; - index_registry->Branch(canonical_schema[i].c_str(), - &layer_values_by_branch[i], - leaf_list.c_str()); + index_registry->Branch( + canonical_schema[i].c_str(), &layer_values_by_branch[i], leaf_list.c_str()); } std::string index_product_id; index_registry->Branch("ProductID", &index_product_id); diff --git a/form/storage/storage_writer.hpp b/form/storage/storage_writer.hpp index efec22775..1f8dcc2bf 100644 --- a/form/storage/storage_writer.hpp +++ b/form/storage/storage_writer.hpp @@ -6,8 +6,8 @@ #include "istorage.hpp" #include "storage_utils.hpp" -#include #include +#include #include #include #include @@ -47,7 +47,8 @@ namespace form::detail::experimental { std::map> m_indexContainerNames; std::map> m_indexPayloadRows; std::map>> m_productsByProducer; - std::map>> m_pendingProductsByProducer; + std::map>> + m_pendingProductsByProducer; }; } // namespace form::detail::experimental diff --git a/test/form/writer.cpp b/test/form/writer.cpp index fa35a392f..562c43eab 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -194,14 +194,13 @@ int main(int argc, char** argv) } std::string fileUUIDValue = *fileUUID; - auto isHexChar = [](char c) { - return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f'); - }; - bool validUUID = fileUUIDValue.size() == 36 && - fileUUIDValue[8] == '-' && fileUUIDValue[13] == '-' && - fileUUIDValue[18] == '-' && fileUUIDValue[23] == '-' && - std::all_of(fileUUIDValue.begin(), fileUUIDValue.end(), - [&](char c) { return c == '-' || isHexChar(c); }); + auto isHexChar = [](char c) { return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f'); }; + bool validUUID = fileUUIDValue.size() == 36 && fileUUIDValue[8] == '-' && + fileUUIDValue[13] == '-' && fileUUIDValue[18] == '-' && + fileUUIDValue[23] == '-' && + std::all_of(fileUUIDValue.begin(), fileUUIDValue.end(), [&](char c) { + return c == '-' || isHexChar(c); + }); if (!validUUID) { std::cerr << "ERROR: FileUUID is not canonical: " << fileUUIDValue << '\n'; @@ -209,7 +208,8 @@ int main(int argc, char** argv) return 1; } - std::cout << "PHLEX: FileUUID validated: " << fileUUIDValue << " (version=" << fileFormatVersion << ")" << '\n'; + std::cout << "PHLEX: FileUUID validated: " << fileUUIDValue << " (version=" << fileFormatVersion + << ")" << '\n'; TTree* registry = root_file->Get("ProductRegistry"); if (registry == nullptr) { @@ -243,19 +243,18 @@ int main(int argc, char** argv) root_file->Close(); return 1; } - std::string const expected_product_id = - *productName + "|" + *producer + "|" + *processName; + std::string const expected_product_id = *productName + "|" + *producer + "|" + *processName; if (*productID != expected_product_id) { - std::cerr << "ERROR: ProductRegistry ProductID mismatch. expected='" - << expected_product_id << "' got='" << *productID << "'." << '\n'; + std::cerr << "ERROR: ProductRegistry ProductID mismatch. expected='" << expected_product_id + << "' got='" << *productID << "'." << '\n'; root_file->Close(); return 1; } product_names.insert(*productName); product_ids.insert(*productID); - std::cout << "PHLEX: ProductRegistry entry: ProductName='" << *productName - << "' ProcessName='" << *processName << "' Producer='" << *producer - << "' ProductID='" << *productID << "'\n"; + std::cout << "PHLEX: ProductRegistry entry: ProductName='" << *productName << "' ProcessName='" + << *processName << "' Producer='" << *producer << "' ProductID='" << *productID + << "'\n"; } if (product_names.empty()) { @@ -309,8 +308,8 @@ int main(int argc, char** argv) std::size_t start = 0; while (start <= schema_text.size()) { std::size_t end = schema_text.find(',', start); - std::string token = schema_text.substr(start, - end == std::string::npos ? std::string::npos : end - start); + std::string token = + schema_text.substr(start, end == std::string::npos ? std::string::npos : end - start); if (token.size() >= 2 && token.front() == '"' && token.back() == '"') { token = token.substr(1, token.size() - 2); } @@ -343,7 +342,9 @@ int main(int argc, char** argv) return 1; } if (branch_list->GetEntries() != static_cast(header_schema.size()) + 3) { - std::cerr << "ERROR: IndexRegistry branch count does not match LayerSchema size + ProductID + ContainerName + PayloadRow." << '\n'; + std::cerr << "ERROR: IndexRegistry branch count does not match LayerSchema size + ProductID + " + "ContainerName + PayloadRow." + << '\n'; root_file->Close(); return 1; } @@ -355,16 +356,17 @@ int main(int argc, char** argv) return 1; } if (header_schema[i] != branch_obj->GetName()) { - std::cerr << "ERROR: IndexRegistry branch order does not match LayerSchema at position " - << i << ": expected '" << header_schema[i] << "' got '" << branch_obj->GetName() - << "'." << '\n'; + std::cerr << "ERROR: IndexRegistry branch order does not match LayerSchema at position " << i + << ": expected '" << header_schema[i] << "' got '" << branch_obj->GetName() << "'." + << '\n'; root_file->Close(); return 1; } } auto* product_branch_obj = branch_list->At(static_cast(header_schema.size())); if (product_branch_obj == nullptr || std::string(product_branch_obj->GetName()) != "ProductID") { - std::cerr << "ERROR: IndexRegistry branch order missing ProductID after layer branches." << '\n'; + std::cerr << "ERROR: IndexRegistry branch order missing ProductID after layer branches." + << '\n'; root_file->Close(); return 1; } @@ -378,7 +380,8 @@ int main(int argc, char** argv) auto* payload_row_branch_obj = branch_list->At(static_cast(header_schema.size()) + 2); if (payload_row_branch_obj == nullptr || std::string(payload_row_branch_obj->GetName()) != "PayloadRow") { - std::cerr << "ERROR: IndexRegistry branch order missing PayloadRow after ContainerName." << '\n'; + std::cerr << "ERROR: IndexRegistry branch order missing PayloadRow after ContainerName." + << '\n'; root_file->Close(); return 1; } @@ -386,7 +389,8 @@ int main(int argc, char** argv) std::vector layer_branch_values(header_schema.size(), 0); for (std::size_t i = 0; i < header_schema.size(); ++i) { if (index_registry->GetBranch(header_schema[i].c_str()) == nullptr) { - std::cerr << "ERROR: IndexRegistry branch missing for layer '" << header_schema[i] << "'." << '\n'; + std::cerr << "ERROR: IndexRegistry branch missing for layer '" << header_schema[i] << "'." + << '\n'; root_file->Close(); return 1; } @@ -432,7 +436,8 @@ int main(int argc, char** argv) return 1; } if (container_name == nullptr || container_name->empty()) { - std::cerr << "ERROR: IndexRegistry ContainerName branch did not populate valid value." << '\n'; + std::cerr << "ERROR: IndexRegistry ContainerName branch did not populate valid value." + << '\n'; root_file->Close(); return 1; } @@ -446,8 +451,8 @@ int main(int argc, char** argv) unsigned long long const expected_payload_row = static_cast(entry); if (payload_row != expected_payload_row) { - std::cerr << "ERROR: IndexRegistry PayloadRow mismatch at entry " << entry - << ": expected " << expected_payload_row << " got " << payload_row << "." << '\n'; + std::cerr << "ERROR: IndexRegistry PayloadRow mismatch at entry " << entry << ": expected " + << expected_payload_row << " got " << payload_row << "." << '\n'; root_file->Close(); return 1; } @@ -459,7 +464,8 @@ int main(int argc, char** argv) unsigned long long const seg_val = layer_branch_values[1]; if (event_val >= static_cast(NUMBER_EVENT)) { - std::cerr << "ERROR: EVENT value out of range in IndexRegistry sample: " << event_val << '\n'; + std::cerr << "ERROR: EVENT value out of range in IndexRegistry sample: " << event_val + << '\n'; root_file->Close(); return 1; } @@ -470,12 +476,13 @@ int main(int argc, char** argv) } if (has_prev) { - bool const monotonic_ok = - (event_val > prev_event && seg_val == 0) || - (event_val == prev_event && seg_val > prev_seg) || - (event_val == prev_event && seg_val == prev_seg); + bool const monotonic_ok = (event_val > prev_event && seg_val == 0) || + (event_val == prev_event && seg_val > prev_seg) || + (event_val == prev_event && seg_val == prev_seg); if (!monotonic_ok) { - std::cerr << "ERROR: IndexRegistry sample entries are not monotonic by EVENT/SEG ordering." << '\n'; + std::cerr + << "ERROR: IndexRegistry sample entries are not monotonic by EVENT/SEG ordering." + << '\n'; root_file->Close(); return 1; } @@ -488,7 +495,9 @@ int main(int argc, char** argv) } if (header_schema.size() != 2 || header_schema[0] != "EVENT" || header_schema[1] != "SEG") { - std::cerr << "ERROR: IndexRegistry LayerSchema header is not EVENT,SEG as expected for this test." << '\n'; + std::cerr + << "ERROR: IndexRegistry LayerSchema header is not EVENT,SEG as expected for this test." + << '\n'; root_file->Close(); return 1; } @@ -517,8 +526,8 @@ int main(int argc, char** argv) std::cout << "PHLEX: ProductRegistry validated with " << product_names.size() << " product names and " << product_ids.size() << " product IDs" << '\n'; - std::cout << "PHLEX: IndexRegistry validated with " << index_registry->GetEntries() - << " entries" << '\n'; + std::cout << "PHLEX: IndexRegistry validated with " << index_registry->GetEntries() << " entries" + << '\n'; root_file->Close(); return 0; From 02623eee065d00a0b8961db951ddc30fb7f6c1e8 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 15:16:57 -0500 Subject: [PATCH 05/13] use boost UUID and remove lines not really needed again --- form/root_storage/root_tfile.cpp | 4 ---- form/storage/storage_writer.cpp | 30 +++++------------------------- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/form/root_storage/root_tfile.cpp b/form/root_storage/root_tfile.cpp index 0a6f8a5a0..f88816b9b 100644 --- a/form/root_storage/root_tfile.cpp +++ b/form/root_storage/root_tfile.cpp @@ -26,10 +26,6 @@ ROOT_TFileImp::ROOT_TFileImp(std::string const& name, char mode) : } else { throw std::runtime_error(std::string("Unsupported ROOT file open mode: ") + mode); } - - if (!m_file || m_file->IsZombie()) { - throw std::runtime_error("Failed to open ROOT file: " + name); - } } ROOT_TFileImp::~ROOT_TFileImp() = default; diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 898b92549..f4d93cf79 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -8,6 +8,9 @@ #include "form/technology.hpp" #include "util/factories.hpp" +#include +#include +#include #include #include #include @@ -392,31 +395,8 @@ void StorageWriter::commitContainers(Placement const& plcmnt) static std::string generateUUID() { - std::random_device rd; - std::mt19937_64 generator(rd()); - std::uniform_int_distribution distribution(0, 0xFFFFFFFFu); - - uint32_t w0 = distribution(generator); - uint32_t w1 = distribution(generator); - uint32_t w2 = distribution(generator); - uint32_t w3 = distribution(generator); - - // RFC 4122 variant 4 UUID: set the version and variant bits. - uint16_t time_hi_and_version = static_cast((w1 >> 16) & 0x0FFFu); - time_hi_and_version |= 0x4000u; - uint16_t clock_seq_hi_and_reserved = static_cast((w2 >> 16) & 0x3FFFu); - clock_seq_hi_and_reserved |= 0x8000u; - - std::ostringstream oss; - oss << std::hex << std::nouppercase << std::setfill('0'); - oss << std::setw(8) << w0 << '-'; - oss << std::setw(4) << static_cast(w1 >> 16) << '-'; - oss << std::setw(4) << time_hi_and_version << '-'; - oss << std::setw(4) << clock_seq_hi_and_reserved << '-'; - oss << std::setw(4) << static_cast(w2 & 0xFFFFu); - oss << std::setw(8) << w3; - - return oss.str(); + static thread_local boost::uuids::random_generator gen; + return boost::uuids::to_string(gen()); } void StorageWriter::finalize(form::experimental::config::tech_setting_config const& settings) From 4fec9fb151042ca5e830fa5f755dc4706e1184ed Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 19:59:33 -0500 Subject: [PATCH 06/13] Use ROOT TUUID instead of Boost for UUID generation to avoid additional Boost UUID dependency --- form/storage/storage_writer.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index f4d93cf79..98dfaf770 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -8,9 +8,6 @@ #include "form/technology.hpp" #include "util/factories.hpp" -#include -#include -#include #include #include #include @@ -23,6 +20,7 @@ #include "TFile.h" #include "TObjString.h" #include "TTree.h" +#include #include "root_storage/root_tfile.hpp" #endif @@ -395,8 +393,8 @@ void StorageWriter::commitContainers(Placement const& plcmnt) static std::string generateUUID() { - static thread_local boost::uuids::random_generator gen; - return boost::uuids::to_string(gen()); +TUUID uuid; + return uuid.AsString(); } void StorageWriter::finalize(form::experimental::config::tech_setting_config const& settings) From ffce03c4489918a642e4df792eb777d6449a109e Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 20:02:11 -0500 Subject: [PATCH 07/13] apply clang-format --- form/storage/storage_writer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 98dfaf770..704675eb8 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -20,8 +20,8 @@ #include "TFile.h" #include "TObjString.h" #include "TTree.h" -#include #include "root_storage/root_tfile.hpp" +#include #endif using namespace form::detail::experimental; @@ -393,8 +393,8 @@ void StorageWriter::commitContainers(Placement const& plcmnt) static std::string generateUUID() { -TUUID uuid; - return uuid.AsString(); + TUUID uuid; + return uuid.AsString(); } void StorageWriter::finalize(form::experimental::config::tech_setting_config const& settings) From 294aeb59489fea69409e702c909d26b660700550 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 22:02:04 -0500 Subject: [PATCH 08/13] fix: address clang-tidy issues (dynamic_cast, unique_ptr, SetDirectory) --- form/storage/storage_writer.cpp | 29 ++++++++++++++++++----------- test/form/writer.cpp | 3 ++- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 704675eb8..a4c0fafc6 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -415,14 +415,15 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con // Preserve any existing FileCatalog metadata if reopening the file. TObject* existing_catalog_obj = tfile->Get("FileCatalog"); - TTree* existing_catalog = static_cast(existing_catalog_obj); + TTree* existing_catalog = dynamic_cast(existing_catalog_obj); if (existing_catalog != nullptr) { // Existing FileCatalog already contains the persisted FileUUID. continue; } // Create FileCatalog tree for new files. - TTree* catalog = new TTree("FileCatalog", "File-level metadata catalog"); + auto catalog = std::make_unique("FileCatalog", "File-level metadata catalog"); + catalog->SetDirectory(nullptr); // Determine FileFormatVersion based on technology // We need to get technology from somewhere - for now, infer from existing containers @@ -446,8 +447,7 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con catalog->Fill(); // Write to file - tfile->WriteTObject(catalog); - delete catalog; + tfile->WriteTObject(catalog.get()); // Create ProductRegistry tree listing logical product names from user/framework config. // Use explicit logical product names instead of deriving from container structure. @@ -456,7 +456,9 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con auto products_it = m_productsByProducer.find(fileName); if (products_it != m_productsByProducer.end() && !products_it->second.empty()) { auto const& products_by_producer = products_it->second; - TTree* registry = new TTree("ProductRegistry", "Product-level metadata catalog"); + auto registry = std::make_unique("ProductRegistry", "Product-level metadata catalog"); + registry->SetDirectory(nullptr); + std::string productName; std::string processName; std::string producer; @@ -476,8 +478,7 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con registry->Fill(); } } - tfile->WriteTObject(registry); - delete registry; + tfile->WriteTObject(registry.get()); } auto schema_it = m_indexLayerSchemas.find(fileName); @@ -521,7 +522,10 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con continue; } - TTree* index_registry = new TTree("IndexRegistry", "Layer index metadata catalog"); + auto index_registry = + std::make_unique("IndexRegistry", "Layer index metadata catalog"); + index_registry->SetDirectory(nullptr); + std::vector canonical_schema; { std::size_t start = 0; @@ -554,7 +558,11 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con // Keep schema in tree header metadata as ["layer1","layer2",...]. std::string schema_header = serialize_layer_schema(canonical_schema); - index_registry->GetUserInfo()->Add(new TObjString(schema_header.c_str())); + TList* userInfo = index_registry->GetUserInfo(); + if (userInfo) { + userInfo->SetOwner(kTRUE); + userInfo->Add(new TObjString(schema_header.c_str())); + } for (std::size_t i = 0; i < schemas.size(); ++i) { if (schema_key_for_match(schemas[i]) != canonical_schema_key) { @@ -570,8 +578,7 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con index_registry->Fill(); } - tfile->WriteTObject(index_registry); - delete index_registry; + tfile->WriteTObject(index_registry.get()); } } } diff --git a/test/form/writer.cpp b/test/form/writer.cpp index 562c43eab..c69f35286 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -418,7 +418,8 @@ int main(int argc, char** argv) } index_registry->SetBranchAddress("PayloadRow", &payload_row); - int const sample_entries = index_registry->GetEntries() < 8 ? index_registry->GetEntries() : 8; + Long64_t const sample_entries = + index_registry->GetEntries() < 8 ? index_registry->GetEntries() : 8; unsigned long long prev_event = 0; unsigned long long prev_seg = 0; std::set observed_product_ids; From e43ce9d3b6ed38ddfad4698b8907ce40601093b5 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 22:21:34 -0500 Subject: [PATCH 09/13] add NOLINTNEXTLINE to fix clang-tidy warning --- form/storage/storage_writer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index a4c0fafc6..91a883e23 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -561,6 +561,7 @@ void StorageWriter::finalize(form::experimental::config::tech_setting_config con TList* userInfo = index_registry->GetUserInfo(); if (userInfo) { userInfo->SetOwner(kTRUE); + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) userInfo->Add(new TObjString(schema_header.c_str())); } From ce70d49bd0c42af1015594def73769eaedd32fbd Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 22:52:36 -0500 Subject: [PATCH 10/13] add more code coverage test --- test/form/form_storage_test.cpp | 171 ++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index 409d350b1..5a351677e 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -2,6 +2,15 @@ #include "test/form/test_utils.hpp" +#include "core/placement.hpp" +#include "form/config.hpp" +#include "form/technology.hpp" +#include "root_storage/root_tbranch_write_container.hpp" +#include "root_storage/root_tfile.hpp" +#include "root_storage/root_ttree_write_container.hpp" +#include "storage/storage_write_container.hpp" +#include "storage/storage_writer.hpp" + #include "TFile.h" #include "TTree.h" @@ -9,6 +18,7 @@ #include #include +#include #include using namespace form::detail::experimental; @@ -227,3 +237,164 @@ TEST_CASE("Root TTree write container: fill and commit are not implemented", "[f CHECK_THROWS_AS(writeAssoc->fill(dummy), std::runtime_error); CHECK_THROWS_AS(writeAssoc->commit(), std::runtime_error); } + +TEST_CASE("Root file open modes and attribute validation", "[form]") +{ + std::string const file_name = "testRootTFileModes.root"; + + { + auto seed = TFile::Open(file_name.c_str(), "RECREATE"); + REQUIRE(seed != nullptr); + seed->Write(); + seed->Close(); + } + + SECTION("update mode reopens existing file") + { + ROOT_TFileImp file(file_name, 'u'); + REQUIRE(file.getTFile() != nullptr); + } + + SECTION("update mode creates file when missing") + { + std::string const missing_name = "testRootTFileModesMissing.root"; + ROOT_TFileImp file(missing_name, 'u'); + REQUIRE(file.getTFile() != nullptr); + } + + SECTION("read mode opens file") + { + ROOT_TFileImp file(file_name, 'r'); + REQUIRE(file.getTFile() != nullptr); + } + + SECTION("unknown compression token keeps ROOT default") + { + ROOT_TFileImp file(file_name, 'o'); + CHECK_NOTHROW(file.setAttribute("compression", "NotARealROOTCompressionAlgo")); + } + + SECTION("unsupported attribute throws") + { + ROOT_TFileImp file(file_name, 'o'); + CHECK_THROWS_AS(file.setAttribute("does_not_exist", "1"), std::runtime_error); + } + + SECTION("unsupported file mode throws") + { + CHECK_THROWS_AS(ROOT_TFileImp(file_name, 'z'), std::runtime_error); + } +} + +TEST_CASE("Root TTree write container error paths", "[form]") +{ + ROOT_TTree_Write_ContainerImp container("testTree"); + + CHECK(container.getEntryCount() == 0); + CHECK_THROWS_AS(container.setupWrite(typeid(int)), std::runtime_error); + + std::shared_ptr wrong_file(new Storage_File("ttree_error.root", 'o')); + CHECK_THROWS_AS(container.setFile(wrong_file), std::runtime_error); +} + +TEST_CASE("Storage write container default attribute rejection", "[form]") +{ + Storage_Write_Container container("test/container"); + CHECK_THROWS_AS(container.setAttribute("auto_flush", "1"), std::runtime_error); +} + +TEST_CASE("Root branch write container rejects unknown attributes", "[form]") +{ + ROOT_TBranch_Write_ContainerImp container("test/branch"); + CHECK_THROWS_AS(container.setAttribute("not_supported", "1"), std::runtime_error); +} + +TEST_CASE("StorageWriter finalize skips IndexRegistry for unparsable index", "[form]") +{ + std::string const file_name = "testStorageWriterInvalidIndex.root"; + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + + std::map, std::type_info const*> containers; + containers.emplace(std::make_unique(file_name, "UnitTest/value", technology), + &typeid(std::vector)); + containers.emplace(std::make_unique(file_name, "UnitTest/index", technology), + &typeid(std::string)); + writer.createContainers(containers, settings); + + Placement payload_placement(file_name, "UnitTest/value", technology); + std::vector payload = {1, 2, 3}; + writer.fillContainer(payload_placement, &payload, typeid(std::vector), "DeclaredProduct"); + + Placement index_placement(file_name, "UnitTest/index", technology); + std::string bad_index = "EVENT0001"; + writer.fillContainer(index_placement, &bad_index, typeid(std::string), "UnitTest"); + writer.commitContainers(index_placement); + writer.finalize(settings); + + auto file = TFile::Open(file_name.c_str(), "READ"); + REQUIRE(file != nullptr); + REQUIRE_FALSE(file->IsZombie()); + + CHECK(file->Get("FileCatalog") != nullptr); + CHECK(file->Get("ProductRegistry") != nullptr); + CHECK(file->Get("IndexRegistry") == nullptr); + + file->Close(); +} + +TEST_CASE("StorageWriter parses colon indices and honors process_name key", "[form]") +{ + std::string const file_name = "testStorageWriterColonIndex.root"; + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + + std::map, std::type_info const*> containers; + containers.emplace(std::make_unique(file_name, "UnitTest/value", technology), + &typeid(std::vector)); + containers.emplace(std::make_unique(file_name, "UnitTest/index", technology), + &typeid(std::string)); + writer.createContainers(containers, settings); + + Placement payload_placement(file_name, "UnitTest/value", technology); + std::vector payload = {10, 20, 30}; + writer.fillContainer(payload_placement, &payload, typeid(std::vector), "DeclaredProduct"); + + Placement index_placement(file_name, "UnitTest/index", technology); + std::string index_text = "[EVENT:0000000A;SEG:0000000B]"; + writer.fillContainer(index_placement, &index_text, typeid(std::string), "UnitTest"); + writer.commitContainers(index_placement); + writer.finalize(settings); + + auto file = TFile::Open(file_name.c_str(), "READ"); + REQUIRE(file != nullptr); + REQUIRE_FALSE(file->IsZombie()); + + TTree* registry = file->Get("ProductRegistry"); + REQUIRE(registry != nullptr); + REQUIRE(registry->GetEntries() > 0); + std::string* process_name = nullptr; + registry->SetBranchAddress("ProcessName", &process_name); + registry->GetEntry(0); + REQUIRE(process_name != nullptr); + CHECK(process_name->empty()); + + TTree* index_registry = file->Get("IndexRegistry"); + REQUIRE(index_registry != nullptr); + REQUIRE(index_registry->GetEntries() > 0); + + unsigned long long event_value = 0; + unsigned long long seg_value = 0; + std::string* product_id = nullptr; + index_registry->SetBranchAddress("EVENT", &event_value); + index_registry->SetBranchAddress("SEG", &seg_value); + index_registry->SetBranchAddress("ProductID", &product_id); + index_registry->GetEntry(0); + + CHECK(event_value == 10ULL); + CHECK(seg_value == 11ULL); + REQUIRE(product_id != nullptr); + CHECK(*product_id == "DeclaredProduct|UnitTest|"); + + file->Close(); +} From 5553417b1227c4073eb2096ce67e962c82649ec8 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Mon, 15 Jun 2026 23:44:13 -0500 Subject: [PATCH 11/13] address doderabbitai committable suggestions --- form/form/form_writer.cpp | 13 ++++++++++--- form/form/form_writer.hpp | 3 ++- form/form_module.cpp | 13 ++++++++++--- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/form/form/form_writer.cpp b/form/form/form_writer.cpp index b2aeb449a..cdfbfb651 100644 --- a/form/form/form_writer.cpp +++ b/form/form/form_writer.cpp @@ -78,12 +78,19 @@ namespace form::experimental { m_pers_writer->commitOutput(creator, segment_id); } - void form_writer_interface::declareProductName(std::string const& routing_label, - std::string const& product_name) + void form_writer_interface::declare_product_name(std::string const& routing_label, + std::string const& product_name) { m_label_to_product_name[routing_label] = product_name; } - void form_writer_interface::finalize() { m_pers_writer->finalize(); } + void form_writer_interface::finalize() + { + if (m_finalized) { + return; + } + m_finalized = true; + m_pers_writer->finalize(); + } } diff --git a/form/form/form_writer.hpp b/form/form/form_writer.hpp index 12d588a8f..5189710ef 100644 --- a/form/form/form_writer.hpp +++ b/form/form/form_writer.hpp @@ -32,12 +32,13 @@ namespace form::experimental { // Explicitly declare the logical ProductName for a given routing label. // If not called, ProductName defaults to the creator name at write time. - void declareProductName(std::string const& routing_label, std::string const& product_name); + void declare_product_name(std::string const& routing_label, std::string const& product_name); private: std::unique_ptr m_pers_writer; std::map m_product_to_config; std::map m_label_to_product_name; + bool m_finalized = false; }; } diff --git a/form/form_module.cpp b/form/form_module.cpp index 4a6868511..05494a126 100644 --- a/form/form_module.cpp +++ b/form/form_module.cpp @@ -48,7 +48,7 @@ namespace { // Explicitly declare ProductNames from user config (products list in jsonnet) for (auto const& product : products_to_save) { - m_form_interface->declareProductName(product, product); + m_form_interface->declare_product_name(product, product); } } @@ -56,8 +56,15 @@ namespace { { if (m_form_interface) { std::cout << "FormOutputModule destructor: calling finalize() to write metadata\n"; - m_form_interface->finalize(); - std::cout << "FormOutputModule destructor: finalize() completed\n"; + try { + m_form_interface->finalize(); + std::cout << "FormOutputModule destructor: finalize() completed\n"; + } catch (std::exception const& e) { + std::cerr << "ERROR: FormOutputModule destructor: finalize() failed: " << e.what() + << std::endl; + } catch (...) { + std::cerr << "Unknown error in FormOutputModule destructor during finalize.\n"; + } } } From 8183569c6d22a83d379c3c18929296c0ef945c75 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Tue, 16 Jun 2026 14:30:39 -0500 Subject: [PATCH 12/13] addressed comments from PR review --- .../root_tbranch_write_container.cpp | 4 +- form/root_storage/root_tfile.cpp | 4 + form/storage/storage_writer.cpp | 6 +- test/form/form_storage_test.cpp | 23 ++- test/form/writer.cpp | 194 +++++++----------- 5 files changed, 101 insertions(+), 130 deletions(-) diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index 19ea594fb..d10eaa3e1 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -133,8 +133,8 @@ void ROOT_TBranch_Write_ContainerImp::commit() std::uint64_t ROOT_TBranch_Write_ContainerImp::getEntryCount() { - if (m_tree == nullptr) { + if (m_branch == nullptr) { return 0; } - return m_tree->GetEntries(); + return m_branch->GetEntries(); } diff --git a/form/root_storage/root_tfile.cpp b/form/root_storage/root_tfile.cpp index f88816b9b..0a6f8a5a0 100644 --- a/form/root_storage/root_tfile.cpp +++ b/form/root_storage/root_tfile.cpp @@ -26,6 +26,10 @@ ROOT_TFileImp::ROOT_TFileImp(std::string const& name, char mode) : } else { throw std::runtime_error(std::string("Unsupported ROOT file open mode: ") + mode); } + + if (!m_file || m_file->IsZombie()) { + throw std::runtime_error("Failed to open ROOT file: " + name); + } } ROOT_TFileImp::~ROOT_TFileImp() = default; diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 91a883e23..488f6a4bf 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -377,8 +377,6 @@ void StorageWriter::fillContainer(Placement const& plcmnt, } } - (void)product_name; - cont->second->fill(data); return; } @@ -393,8 +391,12 @@ void StorageWriter::commitContainers(Placement const& plcmnt) static std::string generateUUID() { +#ifdef USE_ROOT_STORAGE TUUID uuid; return uuid.AsString(); +#else +#error "ROOT storage is disabled, please provide an alternative UUID generator." +#endif } void StorageWriter::finalize(form::experimental::config::tech_setting_config const& settings) diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index 5a351677e..fa2087e31 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -271,7 +271,9 @@ TEST_CASE("Root file open modes and attribute validation", "[form]") SECTION("unknown compression token keeps ROOT default") { ROOT_TFileImp file(file_name, 'o'); + int default_level = file.getTFile()->GetCompressionLevel(); CHECK_NOTHROW(file.setAttribute("compression", "NotARealROOTCompressionAlgo")); + CHECK(file.getTFile()->GetCompressionLevel() == default_level); } SECTION("unsupported attribute throws") @@ -286,15 +288,16 @@ TEST_CASE("Root file open modes and attribute validation", "[form]") } } -TEST_CASE("Root TTree write container error paths", "[form]") +TEST_CASE("Storage write container error paths", "[form]") { - ROOT_TTree_Write_ContainerImp container("testTree"); + auto container = createWriteContainer(technology, "testTree"); + REQUIRE(container != nullptr); - CHECK(container.getEntryCount() == 0); - CHECK_THROWS_AS(container.setupWrite(typeid(int)), std::runtime_error); + CHECK(container->getEntryCount() == 0); + CHECK_THROWS_AS(container->setupWrite(typeid(int)), std::runtime_error); - std::shared_ptr wrong_file(new Storage_File("ttree_error.root", 'o')); - CHECK_THROWS_AS(container.setFile(wrong_file), std::runtime_error); + std::shared_ptr wrong_file(new Storage_File("container_error.root", 'o')); + CHECK_THROWS_AS(container->setFile(wrong_file), std::runtime_error); } TEST_CASE("Storage write container default attribute rejection", "[form]") @@ -303,10 +306,12 @@ TEST_CASE("Storage write container default attribute rejection", "[form]") CHECK_THROWS_AS(container.setAttribute("auto_flush", "1"), std::runtime_error); } -TEST_CASE("Root branch write container rejects unknown attributes", "[form]") +TEST_CASE("Storage write container rejects unknown attributes", "[form]") { - ROOT_TBranch_Write_ContainerImp container("test/branch"); - CHECK_THROWS_AS(container.setAttribute("not_supported", "1"), std::runtime_error); + auto container = createWriteContainer(technology, "test/branch"); + REQUIRE(container != nullptr); + + CHECK_THROWS_AS(container->setAttribute("not_supported", "1"), std::runtime_error); } TEST_CASE("StorageWriter finalize skips IndexRegistry for unparsable index", "[form]") diff --git a/test/form/writer.cpp b/test/form/writer.cpp index c69f35286..ff71d38b8 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -67,104 +68,107 @@ int main(int argc, char** argv) tech_config.container_settings[form::technology::ROOT_RNTUPLE]["Toy_Tracker/trackStartPoints"] .emplace_back("force_streamer_field", "true"); - form::experimental::form_writer_interface form(config_items, tech_config); + { + form::experimental::form_writer_interface form(config_items, tech_config); - ToyTracker tracker(4 * 1024); + ToyTracker tracker(4 * 1024); - // Open checksum file for writing - std::ofstream checksum_file(checksum_filename); - if (!checksum_file.is_open()) { - std::cerr << "ERROR: Could not open checksum file: " << checksum_filename << '\n'; - return 1; - } + // Open checksum file for writing + std::ofstream checksum_file(checksum_filename); + if (!checksum_file.is_open()) { + std::cerr << "ERROR: Could not open checksum file: " << checksum_filename << '\n'; + return 1; + } - for (int nevent = 0; nevent < NUMBER_EVENT; nevent++) { - std::cout << "PHLEX: Write Event No. " << nevent << '\n'; + for (int nevent = 0; nevent < NUMBER_EVENT; nevent++) { + std::cout << "PHLEX: Write Event No. " << nevent << '\n'; - std::vector track_x; + std::vector track_x; - for (int nseg = 0; nseg < NUMBER_SEGMENT; nseg++) { + for (int nseg = 0; nseg < NUMBER_SEGMENT; nseg++) { - std::vector track_start_x; - generate(track_start_x, 4 * 1024 /* * 1024*/); // sub-event processing - float check = 0.0; - for (float val : track_start_x) - check += val; + std::vector track_start_x; + generate(track_start_x, 4 * 1024 /* * 1024*/); // sub-event processing + float check = 0.0; + for (float val : track_start_x) + check += val; - std::string const seg_id_text = std::format("[EVENT={:08X};SEG={:08X}]", nevent, nseg); + std::string const seg_id_text = std::format("[EVENT={:08X};SEG={:08X}]", nevent, nseg); - std::string const& segment_id = seg_id_text; + std::string const& segment_id = seg_id_text; - std::vector products; - std::string const creator = "Toy_Tracker"; + std::vector products; + std::string const creator = "Toy_Tracker"; - form::experimental::product_with_name pb = { - "trackStart", &track_start_x, &typeid(std::vector)}; - products.push_back(pb); + form::experimental::product_with_name pb = { + "trackStart", &track_start_x, &typeid(std::vector)}; + products.push_back(pb); - std::vector track_n_hits(std::from_range, std::views::iota(0, 100)); - for (int val : track_n_hits) - check += static_cast(val); - std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text - << ", check = " << check << '\n'; + std::vector track_n_hits(std::from_range, std::views::iota(0, 100)); + for (int val : track_n_hits) + check += static_cast(val); + std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text + << ", check = " << check << '\n'; - form::experimental::product_with_name pb_int = { - "trackNumberHits", &track_n_hits, &typeid(std::vector)}; - products.push_back(pb_int); + form::experimental::product_with_name pb_int = { + "trackNumberHits", &track_n_hits, &typeid(std::vector)}; + products.push_back(pb_int); - std::vector start_points = tracker(); - TrackStart checkPoints; - for (TrackStart const& point : start_points) - checkPoints += point; - std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text - << ", checkPoints = " << checkPoints << '\n'; + std::vector start_points = tracker(); + TrackStart checkPoints; + for (TrackStart const& point : start_points) + checkPoints += point; + std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text + << ", checkPoints = " << checkPoints << '\n'; - form::experimental::product_with_name pb_points = { - "trackStartPoints", &start_points, &typeid(std::vector)}; - products.push_back(pb_points); + form::experimental::product_with_name pb_points = { + "trackStartPoints", &start_points, &typeid(std::vector)}; + products.push_back(pb_points); - form.write(creator, segment_id, products); + form.write(creator, segment_id, products); - // Save segment checksums - checksum_file << std::setprecision(10) << "SEG " << nevent << " " << nseg << " " << check - << " " << checkPoints.getX() << " " << checkPoints.getY() << " " - << checkPoints.getZ() << "\n"; - track_x.insert(track_x.end(), track_start_x.begin(), track_start_x.end()); - } + // Save segment checksums + checksum_file << std::setprecision(10) << "SEG " << nevent << " " << nseg << " " << check + << " " << checkPoints.getX() << " " << checkPoints.getY() << " " + << checkPoints.getZ() << "\n"; + track_x.insert(track_x.end(), track_start_x.begin(), track_start_x.end()); + } - std::cout << "PHLEX: Write Event segments done " << nevent << '\n'; + std::cout << "PHLEX: Write Event segments done " << nevent << '\n'; - float check = 0.0; - for (float val : track_x) - check += val; + float check = 0.0; + for (float val : track_x) + check += val; - std::string const evt_id_text = std::format("[EVENT={:08X}]", nevent); + std::string const evt_id_text = std::format("[EVENT={:08X}]", nevent); - std::string const& event_id = evt_id_text; + std::string const& event_id = evt_id_text; - std::string const creator = "Toy_Tracker_Event"; + std::string const creator = "Toy_Tracker_Event"; - form::experimental::product_with_name pb = { - "trackStartX", &track_x, &typeid(std::vector)}; - std::cout << "PHLEX: Event = " << nevent << ": evt_id_text = " << evt_id_text - << ", check = " << check << '\n'; + form::experimental::product_with_name pb = { + "trackStartX", &track_x, &typeid(std::vector)}; + std::cout << "PHLEX: Event = " << nevent << ": evt_id_text = " << evt_id_text + << ", check = " << check << '\n'; - form.write(creator, event_id, pb); + form.write(creator, event_id, pb); - // Save event checksum - checksum_file << std::setprecision(10) << "EVT " << nevent << " " << check << "\n"; - std::cout << "PHLEX: Write Event done " << nevent << '\n'; - } + // Save event checksum + checksum_file << std::setprecision(10) << "EVT " << nevent << " " << check << "\n"; + std::cout << "PHLEX: Write Event done " << nevent << '\n'; + } - checksum_file.close(); - std::cout << "PHLEX: Write done. Checksums saved to " << checksum_filename << '\n'; + checksum_file.close(); + std::cout << "PHLEX: Write done. Checksums saved to " << checksum_filename << '\n'; - // Finalize to write FileCatalog metadata - form.finalize(); - std::cout << "PHLEX: Finalize done. FileCatalog written to file." << '\n'; + // Finalize to write FileCatalog metadata + form.finalize(); + std::cout << "PHLEX: Finalize done. FileCatalog written to file." << '\n'; + } // Verify that the generated FileUUID exists and is a canonical UUID string. - TFile* root_file = TFile::Open(filename.c_str(), "READ"); + std::unique_ptr root_file(TFile::Open(filename.c_str(), "READ")); + if (root_file == nullptr || root_file->IsZombie()) { std::cerr << "ERROR: Could not open generated ROOT file for validation: " << filename << '\n'; return 1; @@ -173,7 +177,6 @@ int main(int argc, char** argv) TTree* catalog = root_file->Get("FileCatalog"); if (catalog == nullptr) { std::cerr << "ERROR: FileCatalog tree not found in generated ROOT file." << '\n'; - root_file->Close(); return 1; } @@ -183,28 +186,18 @@ int main(int argc, char** argv) catalog->SetBranchAddress("FileFormatVersion", &fileFormatVersion); if (catalog->GetEntries() < 1) { std::cerr << "ERROR: FileCatalog tree has no entries." << '\n'; - root_file->Close(); return 1; } catalog->GetEntry(0); if (fileUUID == nullptr) { std::cerr << "ERROR: FileUUID branch did not populate a valid pointer." << '\n'; - root_file->Close(); return 1; } std::string fileUUIDValue = *fileUUID; - auto isHexChar = [](char c) { return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f'); }; - bool validUUID = fileUUIDValue.size() == 36 && fileUUIDValue[8] == '-' && - fileUUIDValue[13] == '-' && fileUUIDValue[18] == '-' && - fileUUIDValue[23] == '-' && - std::all_of(fileUUIDValue.begin(), fileUUIDValue.end(), [&](char c) { - return c == '-' || isHexChar(c); - }); - - if (!validUUID) { - std::cerr << "ERROR: FileUUID is not canonical: " << fileUUIDValue << '\n'; - root_file->Close(); + TUUID uuidObj(fileUUIDValue.c_str()); + if (uuidObj == TUUID()) { + std::cerr << "ERROR: FileUUID is not valid: " << fileUUIDValue << '\n'; return 1; } @@ -214,7 +207,6 @@ int main(int argc, char** argv) TTree* registry = root_file->Get("ProductRegistry"); if (registry == nullptr) { std::cerr << "ERROR: ProductRegistry tree not found in generated ROOT file." << '\n'; - root_file->Close(); return 1; } @@ -229,7 +221,6 @@ int main(int argc, char** argv) if (registry->GetEntries() == 0) { std::cerr << "ERROR: ProductRegistry tree has no entries." << '\n'; - root_file->Close(); return 1; } @@ -240,14 +231,12 @@ int main(int argc, char** argv) if (productName == nullptr || processName == nullptr || producer == nullptr || productID == nullptr) { std::cerr << "ERROR: ProductRegistry branches did not populate valid pointers." << '\n'; - root_file->Close(); return 1; } std::string const expected_product_id = *productName + "|" + *producer + "|" + *processName; if (*productID != expected_product_id) { std::cerr << "ERROR: ProductRegistry ProductID mismatch. expected='" << expected_product_id << "' got='" << *productID << "'." << '\n'; - root_file->Close(); return 1; } product_names.insert(*productName); @@ -259,7 +248,6 @@ int main(int argc, char** argv) if (product_names.empty()) { std::cerr << "ERROR: ProductRegistry tree contains no product names." << '\n'; - root_file->Close(); return 1; } @@ -271,7 +259,6 @@ int main(int argc, char** argv) TTree* index_registry = root_file->Get("IndexRegistry"); if (index_registry == nullptr) { std::cerr << "ERROR: IndexRegistry tree not found in generated ROOT file." << '\n'; - root_file->Close(); return 1; } @@ -294,7 +281,6 @@ int main(int argc, char** argv) if (layer_schema_meta == nullptr) { std::cerr << "ERROR: IndexRegistry header does not contain LayerSchema metadata." << '\n'; - root_file->Close(); return 1; } @@ -325,41 +311,35 @@ int main(int argc, char** argv) if (header_schema.empty()) { std::cerr << "ERROR: IndexRegistry LayerSchema header is empty." << '\n'; - root_file->Close(); return 1; } if (index_registry->GetEntries() == 0) { std::cerr << "ERROR: IndexRegistry tree has no entries." << '\n'; - root_file->Close(); return 1; } auto* branch_list = index_registry->GetListOfBranches(); if (branch_list == nullptr) { std::cerr << "ERROR: IndexRegistry has no branch list." << '\n'; - root_file->Close(); return 1; } if (branch_list->GetEntries() != static_cast(header_schema.size()) + 3) { std::cerr << "ERROR: IndexRegistry branch count does not match LayerSchema size + ProductID + " "ContainerName + PayloadRow." << '\n'; - root_file->Close(); return 1; } for (std::size_t i = 0; i < header_schema.size(); ++i) { auto* branch_obj = branch_list->At(static_cast(i)); if (branch_obj == nullptr) { std::cerr << "ERROR: IndexRegistry has null branch object at position " << i << "." << '\n'; - root_file->Close(); return 1; } if (header_schema[i] != branch_obj->GetName()) { std::cerr << "ERROR: IndexRegistry branch order does not match LayerSchema at position " << i << ": expected '" << header_schema[i] << "' got '" << branch_obj->GetName() << "'." << '\n'; - root_file->Close(); return 1; } } @@ -367,14 +347,12 @@ int main(int argc, char** argv) if (product_branch_obj == nullptr || std::string(product_branch_obj->GetName()) != "ProductID") { std::cerr << "ERROR: IndexRegistry branch order missing ProductID after layer branches." << '\n'; - root_file->Close(); return 1; } auto* container_branch_obj = branch_list->At(static_cast(header_schema.size()) + 1); if (container_branch_obj == nullptr || std::string(container_branch_obj->GetName()) != "ContainerName") { std::cerr << "ERROR: IndexRegistry branch order missing ContainerName after ProductID." << '\n'; - root_file->Close(); return 1; } auto* payload_row_branch_obj = branch_list->At(static_cast(header_schema.size()) + 2); @@ -382,7 +360,6 @@ int main(int argc, char** argv) std::string(payload_row_branch_obj->GetName()) != "PayloadRow") { std::cerr << "ERROR: IndexRegistry branch order missing PayloadRow after ContainerName." << '\n'; - root_file->Close(); return 1; } @@ -391,7 +368,6 @@ int main(int argc, char** argv) if (index_registry->GetBranch(header_schema[i].c_str()) == nullptr) { std::cerr << "ERROR: IndexRegistry branch missing for layer '" << header_schema[i] << "'." << '\n'; - root_file->Close(); return 1; } index_registry->SetBranchAddress(header_schema[i].c_str(), &layer_branch_values[i]); @@ -399,21 +375,18 @@ int main(int argc, char** argv) std::string* product_id = nullptr; if (index_registry->GetBranch("ProductID") == nullptr) { std::cerr << "ERROR: IndexRegistry ProductID branch missing." << '\n'; - root_file->Close(); return 1; } index_registry->SetBranchAddress("ProductID", &product_id); std::string* container_name = nullptr; if (index_registry->GetBranch("ContainerName") == nullptr) { std::cerr << "ERROR: IndexRegistry ContainerName branch missing." << '\n'; - root_file->Close(); return 1; } index_registry->SetBranchAddress("ContainerName", &container_name); unsigned long long payload_row = 0; if (index_registry->GetBranch("PayloadRow") == nullptr) { std::cerr << "ERROR: IndexRegistry PayloadRow branch missing." << '\n'; - root_file->Close(); return 1; } index_registry->SetBranchAddress("PayloadRow", &payload_row); @@ -428,25 +401,21 @@ int main(int argc, char** argv) index_registry->GetEntry(entry); if (layer_branch_values.empty()) { std::cerr << "ERROR: IndexRegistry layer branch values are empty." << '\n'; - root_file->Close(); return 1; } if (product_id == nullptr || product_id->empty()) { std::cerr << "ERROR: IndexRegistry ProductID branch did not populate valid value." << '\n'; - root_file->Close(); return 1; } if (container_name == nullptr || container_name->empty()) { std::cerr << "ERROR: IndexRegistry ContainerName branch did not populate valid value." << '\n'; - root_file->Close(); return 1; } // ContainerName must be a top-level TTree/RNTuple name (no '/' slash) if (container_name->find('/') != std::string::npos) { std::cerr << "ERROR: IndexRegistry ContainerName should be top-level container name, got: '" << *container_name << "'." << '\n'; - root_file->Close(); return 1; } @@ -454,7 +423,6 @@ int main(int argc, char** argv) if (payload_row != expected_payload_row) { std::cerr << "ERROR: IndexRegistry PayloadRow mismatch at entry " << entry << ": expected " << expected_payload_row << " got " << payload_row << "." << '\n'; - root_file->Close(); return 1; } @@ -467,12 +435,10 @@ int main(int argc, char** argv) if (event_val >= static_cast(NUMBER_EVENT)) { std::cerr << "ERROR: EVENT value out of range in IndexRegistry sample: " << event_val << '\n'; - root_file->Close(); return 1; } if (seg_val >= static_cast(NUMBER_SEGMENT)) { std::cerr << "ERROR: SEG value out of range in IndexRegistry sample: " << seg_val << '\n'; - root_file->Close(); return 1; } @@ -484,7 +450,6 @@ int main(int argc, char** argv) std::cerr << "ERROR: IndexRegistry sample entries are not monotonic by EVENT/SEG ordering." << '\n'; - root_file->Close(); return 1; } } @@ -499,19 +464,16 @@ int main(int argc, char** argv) std::cerr << "ERROR: IndexRegistry LayerSchema header is not EVENT,SEG as expected for this test." << '\n'; - root_file->Close(); return 1; } if (product_ids.empty()) { std::cerr << "ERROR: ProductRegistry ProductID values are empty." << '\n'; - root_file->Close(); return 1; } if (observed_product_ids.empty()) { std::cerr << "ERROR: IndexRegistry ProductID values are empty." << '\n'; - root_file->Close(); return 1; } @@ -519,7 +481,6 @@ int main(int argc, char** argv) if (!product_ids.contains(observed_id)) { std::cerr << "ERROR: IndexRegistry ProductID value not present in ProductRegistry: " << observed_id << '\n'; - root_file->Close(); return 1; } } @@ -530,6 +491,5 @@ int main(int argc, char** argv) std::cout << "PHLEX: IndexRegistry validated with " << index_registry->GetEntries() << " entries" << '\n'; - root_file->Close(); return 0; } From bf5988ad60cffa0b6fb03a76e576d0b256b1bc13 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Tue, 16 Jun 2026 16:19:21 -0500 Subject: [PATCH 13/13] added more code test to increase codecov --- test/form/form_storage_test.cpp | 185 ++++++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index fa2087e31..0f67a2dcc 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -8,6 +8,7 @@ #include "root_storage/root_tbranch_write_container.hpp" #include "root_storage/root_tfile.hpp" #include "root_storage/root_ttree_write_container.hpp" +#include "storage/storage_file.hpp" #include "storage/storage_write_container.hpp" #include "storage/storage_writer.hpp" @@ -348,6 +349,190 @@ TEST_CASE("StorageWriter finalize skips IndexRegistry for unparsable index", "[f file->Close(); } +TEST_CASE("Storage_Write_Container base class exercises all default methods", "[form]") +{ + Storage_Write_Container c("my/container"); + + // name() and getEntryCount() on base class + CHECK(c.name() == "my/container"); + CHECK(c.getEntryCount() == 0u); + + // setFile, setupWrite, fill, commit are all no-ops on the base class + auto dummy_file = std::make_shared("dummy_base_test.root", 'o'); + CHECK_NOTHROW(c.setFile(dummy_file)); + CHECK_NOTHROW(c.setupWrite(typeid(int))); + int val = 99; + CHECK_NOTHROW(c.fill(&val)); + CHECK_NOTHROW(c.commit()); +} + +TEST_CASE("StorageWriter fillContainer throws when container is not registered", "[form]") +{ + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + // No createContainers called, so m_write_containers is empty + Placement p("ghost.root", "NoSuch/container", technology); + int d = 1; + CHECK_THROWS_AS(writer.fillContainer(p, &d, typeid(int), ""), std::runtime_error); +} + +TEST_CASE("StorageWriter fillContainer with empty product_name falls back to creator_name", + "[form]") +{ + std::string const file_name = "testEmptyProductName.root"; + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + + std::map, std::type_info const*> containers; + containers.emplace(std::make_unique(file_name, "Creator/value", technology), + &typeid(std::vector)); + containers.emplace(std::make_unique(file_name, "Creator/index", technology), + &typeid(std::string)); + writer.createContainers(containers, settings); + + // Empty product_name → logical_product_name falls back to creator_name ("Creator") + Placement pp(file_name, "Creator/value", technology); + std::vector d = {7}; + CHECK_NOTHROW(writer.fillContainer(pp, &d, typeid(std::vector), "")); + + Placement ip(file_name, "Creator/index", technology); + std::string idx = "[RUN=00000001;EVT=00000002]"; + writer.fillContainer(ip, &idx, typeid(std::string), "Creator"); + writer.commitContainers(ip); + writer.finalize(settings); + + auto f = TFile::Open(file_name.c_str(), "READ"); + REQUIRE(f != nullptr); + TTree* reg = f->Get("ProductRegistry"); + REQUIRE(reg != nullptr); + std::string* prod_name = nullptr; + reg->SetBranchAddress("ProductName", &prod_name); + reg->GetEntry(0); + REQUIRE(prod_name != nullptr); + // product_name was empty, so creator_name ("Creator") was used + CHECK(*prod_name == "Creator"); + f->Close(); +} + +TEST_CASE("StorageWriter createContainers reuses existing parent TTree", "[form]") +{ + std::string const file_name = "testSharedParentTTree.root"; + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + + // First call: creates "SharedTree" parent + "branch1" container + { + std::map, std::type_info const*> c1; + c1.emplace(std::make_unique(file_name, "SharedTree/branch1", technology), + &typeid(std::vector)); + writer.createContainers(c1, settings); + } + + // Second call: "SharedTree" parent already in m_write_containers → hits else branch + { + std::map, std::type_info const*> c2; + c2.emplace(std::make_unique(file_name, "SharedTree/branch2", technology), + &typeid(float)); + CHECK_NOTHROW(writer.createContainers(c2, settings)); + } + + // Both branches must be usable without errors + std::vector d1 = {1, 2}; + float d2 = 2.71f; + CHECK_NOTHROW(writer.fillContainer( + Placement(file_name, "SharedTree/branch1", technology), &d1, typeid(std::vector), "p1")); + CHECK_NOTHROW(writer.fillContainer( + Placement(file_name, "SharedTree/branch2", technology), &d2, typeid(float), "p2")); + CHECK_NOTHROW(writer.finalize(settings)); +} + +TEST_CASE("StorageWriter fillContainer index with nullptr data skips index recording", "[form]") +{ + std::string const file_name = "testNullIndexData.root"; + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + + std::map, std::type_info const*> containers; + containers.emplace(std::make_unique(file_name, "Prod/index", technology), + &typeid(std::string)); + writer.createContainers(containers, settings); + + Placement ip(file_name, "Prod/index", technology); + // data == nullptr → is_index_container && data != nullptr evaluates false → inner block skipped + CHECK_NOTHROW(writer.fillContainer(ip, nullptr, typeid(std::string), "Prod")); + writer.commitContainers(ip); + writer.finalize(settings); + + auto f = TFile::Open(file_name.c_str(), "READ"); + REQUIRE(f != nullptr); + CHECK(f->Get("FileCatalog") != nullptr); + // No parsable index recorded → no IndexRegistry + CHECK(f->Get("IndexRegistry") == nullptr); + f->Close(); +} + +TEST_CASE( + "StorageWriter finalize with no payload: no ProductRegistry; index entry with empty fields", + "[form]") +{ + std::string const file_name = "testNoPayloadProducts.root"; + StorageWriter writer; + form::experimental::config::tech_setting_config settings; + + // Register only an index container — no payload container + std::map, std::type_info const*> containers; + containers.emplace(std::make_unique(file_name, "Anon/index", technology), + &typeid(std::string)); + writer.createContainers(containers, settings); + + // Fill index with valid parsable data; no payload registered for "Anon" + // → products_for_index empty + m_productsByProducer empty → push empty strings into index + Placement ip(file_name, "Anon/index", technology); + std::string idx = "[RUN=00000001]"; + writer.fillContainer(ip, &idx, typeid(std::string), "Anon"); + writer.commitContainers(ip); + writer.finalize(settings); + + auto f = TFile::Open(file_name.c_str(), "READ"); + REQUIRE(f != nullptr); + CHECK(f->Get("FileCatalog") != nullptr); + // m_productsByProducer empty → no ProductRegistry + CHECK(f->Get("ProductRegistry") == nullptr); + // Index data was recorded (with empty product info) → IndexRegistry is written + CHECK(f->Get("IndexRegistry") != nullptr); + f->Close(); +} + +TEST_CASE("Root TTree setupWrite finds pre-existing TTree; getEntryCount reflects entries", + "[form]") +{ + std::string const file_name = "testPreExistingTTree.root"; + + // Step 1: create a file with a TTree that has some entries using plain ROOT + { + auto* seed_file = TFile::Open(file_name.c_str(), "RECREATE"); + REQUIRE(seed_file != nullptr); + auto* tree = new TTree("ExistingTree", "seed"); + int x = 0; + tree->Branch("x", &x, "x/I"); + for (x = 0; x < 3; ++x) { + tree->Fill(); + } + seed_file->Write(); + seed_file->Close(); + } + + // Step 2: open in update mode; setupWrite must find the existing TTree + auto root_file = std::make_shared(file_name, 'u'); + ROOT_TTree_Write_ContainerImp container("ExistingTree"); + container.setFile(root_file); + // setupWrite: first m_tree.reset(Get()) returns non-null → skips new-tree branch + CHECK_NOTHROW(container.setupWrite(typeid(void))); + CHECK(container.getTTree() != nullptr); + // getEntryCount() where m_tree != nullptr returns m_tree->GetEntries() + CHECK(container.getEntryCount() == 3u); +} + TEST_CASE("StorageWriter parses colon indices and honors process_name key", "[form]") { std::string const file_name = "testStorageWriterColonIndex.root";