diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index d9fb08912..5de3aee08 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -154,7 +154,12 @@ class JFactory : public jana::components::JComponent, // Overloaded by JFactoryT - virtual void ClearData() {}; + virtual void ClearData() { + if (mStatus == Status::Processed) { + mStatus = Status::Unprocessed; + mCreationStatus = CreationStatus::NotCreatedYet; + } + }; // Overloaded by user Factories diff --git a/src/libraries/JANA/JFactorySet.cc b/src/libraries/JANA/JFactorySet.cc index ccf99d6f7..a85a41590 100644 --- a/src/libraries/JANA/JFactorySet.cc +++ b/src/libraries/JANA/JFactorySet.cc @@ -295,9 +295,11 @@ void JFactorySet::Release() { fac->ClearData(); } for (auto& it : mCollectionsFromName) { - if (it.second->GetFactory() == nullptr) { - it.second->ClearData(); - } + // fac->ClearData() only clears JFactoryT's, because that's how it always worked. + // Clearing is fundamentally an operation on the data bundle, not on the factory itself. + // Furthermore, "clearing" the factory is misleading because factories can cache arbitrary + // state inside member variables, and there's no way to clear that. + it.second->ClearData(); } } diff --git a/src/libraries/JANA/Podio/JFactoryPodioT.h b/src/libraries/JANA/Podio/JFactoryPodioT.h index 792da15b4..cbd198dc7 100644 --- a/src/libraries/JANA/Podio/JFactoryPodioT.h +++ b/src/libraries/JANA/Podio/JFactoryPodioT.h @@ -35,7 +35,6 @@ class JFactoryPodioT : public JFactory { void Finish() override {} std::size_t GetNumObjects() const final { return m_output.GetCollection()->GetSize(); } - void ClearData() final; void SetCollection(CollectionT&& collection); void SetCollection(std::unique_ptr collection); @@ -71,9 +70,4 @@ void JFactoryPodioT::SetCollection(std::unique_ptr collection) { } -template -void JFactoryPodioT::ClearData() { - // Happens automatically now -} - diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index ef8e3c15b..6aae53718 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -2,10 +2,15 @@ #include #include + #include #include #include #include +#include +#include +#include +#include namespace podiotests { @@ -176,21 +181,26 @@ struct TestMultiFac : public JMultifactory { void Init() override { init_called = true; } - void Process(const std::shared_ptr&) override { + void Process(const std::shared_ptr& event) override { ExampleClusterCollection c; - c.push_back(MutableExampleCluster(16.0)); + c.push_back(MutableExampleCluster(16.0 + event->GetEventNumber())); SetCollection("sillyclusters", std::move(c)); } }; -TEST_CASE("Podio JMultifactory::Init gets called") { +TEST_CASE("PodioTests_JMultifactoryInit") { JApplication app; auto event = std::make_shared(&app); + event->SetEventNumber(0); auto fs = new JFactorySet; fs->Add(new TestMultiFac); event->SetFactorySet(fs); - event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool + + // Simulate a trip to the event pool _before_ calling JFactory::Process() + // This is important because a badly designed JFactory::ClearData() could + // mangle the Unprocessed status and consequently skip Init(). + event->GetFactorySet()->Release(); // In theory this shouldn't hurt auto r = event->GetCollectionBase("sillyclusters"); REQUIRE(r != nullptr); @@ -204,6 +214,30 @@ TEST_CASE("Podio JMultifactory::Init gets called") { REQUIRE(multifac_typed != nullptr); REQUIRE(multifac_typed->init_called == true); } + + +TEST_CASE("PodioTests_MultifacMultiple") { + + JApplication app; + auto event = std::make_shared(&app); + event->SetEventNumber(0); + auto fs = new JFactorySet; + fs->Add(new TestMultiFac); + event->SetFactorySet(fs); + + auto r = event->GetCollection("sillyclusters"); + REQUIRE(r->at(0).energy() == 16.0); + + event->GetFactorySet()->Release(); // Simulate a trip to the JEventPool + + event->SetEventNumber(4); + r = event->GetCollection("sillyclusters"); + REQUIRE(r->at(0).energy() == 20.0); + +} +} // namespace multifactory + + TEST_CASE("PodioTests_InsertMultiple") { JApplication app; @@ -247,6 +281,67 @@ TEST_CASE("PodioTests_InsertMultiple") { } -} // namespace multifactory +namespace omnifacmultiple { + +struct MyClusterFactory : public JOmniFactory { + + PodioOutput m_clusters_out{this, "clusters"}; + + void Configure() { + } + + void ChangeRun(int32_t /*run_nr*/) { + } + + void Execute(int32_t /*run_nr*/, uint64_t evt_nr) { + + auto cs = std::make_unique(); + auto cluster = MutableExampleCluster(101.0 + evt_nr); + cs->push_back(cluster); + m_clusters_out() = std::move(cs); + } +}; + +TEST_CASE("PodioTests_OmniFacMultiple") { + + JApplication app; + app.SetParameterValue("jana:loglevel", "error"); + app.Add(new JOmniFactoryGeneratorT("cluster_fac", {}, {"clusters"})); + app.Initialize(); + auto event = std::make_shared(&app); + app.GetService()->configure_event(*event); + event->SetEventNumber(22); + + // Check that storage is already present + auto storage = event->GetStorage("clusters", false); + REQUIRE(storage != nullptr); + REQUIRE(storage->GetStatus() == JStorage::Status::Empty); + + // Retrieve triggers factory + auto coll = event->GetCollection("clusters"); + REQUIRE(coll->size() == 1); + REQUIRE(coll->at(0).energy() == 123.0); + + // Clear everything + event->GetFactorySet()->Release(); + event->SetEventNumber(1010); + + // Check that storage has been reset + storage = event->GetStorage("clusters", false); + REQUIRE(storage != nullptr); + REQUIRE(storage->GetStatus() == JStorage::Status::Empty); + + REQUIRE(storage->GetFactory() != nullptr); + + // Retrieve triggers factory + auto coll2 = event->GetCollection("clusters"); + REQUIRE(coll2->size() == 1); + REQUIRE(coll2->at(0).energy() == 1111.0); +} + +} // namespace omnifacmultiple + + + } // namespace podiotests