FORM RNTuple Accessor Functions

form/root_storage/root_rfield_write_container.cpp

@@ -30,21 +30,13 @@ namespace form::detail::experimental {
 
   void root_rfield_write_container_imp::set_file(std::shared_ptr<i_storage_file> file)
   {
-    storage_write_container::set_file(file);
-
-    auto form_root_file = dynamic_pointer_cast<root_tfile_imp>(file);
-    if (form_root_file) {
-      tfile_ = form_root_file->get_tfile();
-    } else {
-      throw std::runtime_error("root_rfield_write_container_imp::set_file failed to convert an "
-                               "i_storage_file to a root_tfile_imp.  "
-                               "root_rfield_write_container_imp only works with TFiles.");
-    }
-
-    if (!tfile_) {
+    //The test below is required by FORM's testing infrastructure
+    if (!dynamic_pointer_cast<root_tfile_imp>(file)) {
       throw std::runtime_error(
-        "root_rfield_write_container_imp::set_file failed to get a TFile from a root_tfile_imp");
+        "root_rfield_write_container_imp::set_file was passed a file that is not a ROOT file.");
     }
+
+    storage_write_container::set_file(file);
   }
 
   void root_rfield_write_container_imp::set_parent(
@@ -67,22 +59,12 @@ namespace form::detail::experimental {
         "root_rfield_write_container_imp::fill No parent RNTuple set up before first fill() call");
     }
 
-    if (!rntuple_parent_->writer) {
-      if (!tfile_) {
-        throw std::runtime_error(
-          "root_rfield_write_container_imp::fill No file loaded to write to on first fill() call");
-      }
-
-      rntuple_parent_->writer =
-        ROOT::RNTupleWriter::Append(std::move(rntuple_parent_->model), top_name(), *tfile_);
-      rntuple_parent_->entry = rntuple_parent_->writer->CreateRawPtrWriteEntry();
-    }
-    rntuple_parent_->entry->BindRawPtr(col_name(), data);
+    rntuple_parent_->get_entry().BindRawPtr(col_name(), data);
 
     // Unlike a TBranch, an RNTuple entry is only written on commit();
     // every field bound before that commit shares one entry.
     // Return the 0-based index that pending entry will occupy (the current entry count).
-    return static_cast<std::uint64_t>(rntuple_parent_->writer->GetNEntries());
+    return static_cast<std::uint64_t>(rntuple_parent_->get_writer().GetNEntries());
   }
 
   void root_rfield_write_container_imp::commit()
@@ -92,13 +74,14 @@ namespace form::detail::experimental {
                                "You may have called commit() without calling set_parent() first.");
     }
 
-    if (!rntuple_parent_->entry) {
-      throw std::runtime_error(
-        "root_rfield_write_container_imp::commit No RRawPtrWriteEntry set up.  "
-        "You may have called commit() without calling fill() first.");
+    //If get_model() is not nullptr, then root_rntuple_write_container_imp guarantees that get_writer()
+    //has not yet been run.
+    if (rntuple_parent_->get_model()) {
+      throw std::runtime_error("root_rfield_write_container_imp::commit No RNTupleWriter set up.  "
+                               "You may have called commit() without calling setup_write() first.");
     }
-    assert(rntuple_parent_->writer); // writer and entry are set in the same place: fill()
-    rntuple_parent_->writer->Fill(*rntuple_parent_->entry);
+
+    rntuple_parent_->get_writer().Fill(rntuple_parent_->get_entry());
   }
 
   //setup_write() may not be called after the first time fill() is called.
@@ -132,7 +115,7 @@ namespace form::detail::experimental {
       }
     }
 
-    rntuple_parent_->model->AddField(std::move(field));
+    assert(rntuple_parent_->get_model());
+    rntuple_parent_->get_model()->AddField(std::move(field));
   }
-
 }

form/root_storage/root_rfield_write_container.hpp

@@ -30,7 +30,6 @@ namespace form::detail::experimental {
     void commit() override;
 
   private:
-    std::shared_ptr<TFile> tfile_;
     std::shared_ptr<root_rntuple_write_container_imp> rntuple_parent_;
 
     bool force_streamer_field_ = false;

form/root_storage/root_rntuple_write_container.cpp

@@ -12,20 +12,33 @@
 
 namespace form::detail::experimental {
   root_rntuple_write_container_imp::root_rntuple_write_container_imp(std::string const& name) :
-    storage_write_association(name), model(ROOT::RNTupleModel::Create())
+    storage_write_association(name), model_(ROOT::RNTupleModel::Create())
   {
   }
 
   root_rntuple_write_container_imp::~root_rntuple_write_container_imp()
   {
-    if (writer) {
-      writer->CommitDataset();
+    if (writer_) {
+      try {
+        writer_->CommitDataset();
+      } catch (ROOT::RException const& e) {
+        std::cerr << "Failed to commit RNTuple " << name() << " at destruction.\n";
+      }
     }
   }
 
   void root_rntuple_write_container_imp::set_file(std::shared_ptr<i_storage_file> file)
   {
     storage_write_container::set_file(file);
+
+    auto form_root_file = dynamic_pointer_cast<root_tfile_imp>(file);
+    if (form_root_file) {
+      tfile_ = form_root_file->get_tfile();
+    } else {
+      throw std::runtime_error("root_rntuple_write_container_imp::set_file failed to convert an "
+                               "i_storage_file to a root_tfile_imp.  "
+                               "root_rntuple_write_container_imp only works with TFiles.");
+    }
   }
 
   std::uint64_t root_rntuple_write_container_imp::fill(void const* /*data*/)
@@ -38,5 +51,30 @@ namespace form::detail::experimental {
     throw std::runtime_error("root_rntuple_write_container_imp::commit not implemented");
   }
 
+  ROOT::RNTupleWriter& root_rntuple_write_container_imp::get_writer()
+  {
+    if (!writer_) {
+      if (!tfile_)
+        throw std::runtime_error("root_rntuple_write_container_imp::setup_write no file loaded to "
+                                 "write to on first fill() call");
+      writer_ = ROOT::RNTupleWriter::Append(std::move(model_), name(), *tfile_);
+    }
+
+    return *writer_;
+  }
+
+  std::unique_ptr<ROOT::RNTupleModel> const& root_rntuple_write_container_imp::get_model() const
+  {
+    return model_;
+  }
+
+  RRawPtrWriteEntry& root_rntuple_write_container_imp::get_entry()
+  {
+    if (!entry_) {
+      entry_ = get_writer().CreateRawPtrWriteEntry();
+    }
+    return *entry_;
+  }
+
   void root_rntuple_write_container_imp::setup_write(std::type_info const& /*type*/) {}
 }

form/root_storage/root_rntuple_write_container.hpp

@@ -57,10 +57,21 @@ namespace form::detail::experimental {
     std::uint64_t fill(void const* data) override;
     void commit() override;
 
+    ROOT::RNTupleWriter& get_writer();
+    //get_model() also signals whether model_ has already been moved from.
+    //If model_ has been moved from, the c++ standard guarantees it will contain nullptr.
+    //This is important for this RNTuple backend to meet FORM's testing
+    //requirement that commit() shall fail if fill() has not been called yet.
+    std::unique_ptr<ROOT::RNTupleModel> const& get_model() const;
+    RRawPtrWriteEntry& get_entry();
+
+  private:
+    std::shared_ptr<TFile> tfile_;
+
     //State shared by root_rfield_write_container_imps
-    std::unique_ptr<ROOT::RNTupleWriter> writer;
-    std::unique_ptr<ROOT::RNTupleModel> model;
-    std::unique_ptr<RRawPtrWriteEntry> entry;
+    std::unique_ptr<ROOT::RNTupleWriter> writer_;
+    std::unique_ptr<ROOT::RNTupleModel> model_;
+    std::unique_ptr<RRawPtrWriteEntry> entry_;
   };
 }
 

test/form/form_storage_test.cpp

@@ -164,6 +164,13 @@ TEST_CASE("FORM Container setup error handling")
       write_container->setup_write(type_info);
       CHECK_THROWS_AS(write_container->commit(), std::runtime_error);
     }
+
+    SECTION("set_file() on parent with wrong file type")
+    {
+      std::shared_ptr<i_storage_file> wrong_file(
+        new storage_file("testContainerErrorHandling.root", 'o'));
+      CHECK_THROWS_AS(parent->set_file(wrong_file), std::runtime_error);
+    }
   }
 
   auto read_container = create_read_container(technology, "test/test_data");