From 33a02a9154dab2bdaafdbba1a1629a2003a56d2a Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 15:19:40 -0400 Subject: [PATCH 1/5] Fix bug with user-defined JFactoryGenerators User-defined JFactoryGenerators can't be trusted to furnish the JFactories they create with a JApplication pointer. We don't have any opportunity to inject the JApplication pointer and logger until JFactory::Create, so we do it there. --- src/libraries/JANA/JFactory.cc | 7 +++ src/programs/unit_tests/CMakeLists.txt | 1 + .../Components/JFactoryGeneratorTests.cc | 44 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 src/programs/unit_tests/Components/JFactoryGeneratorTests.cc diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 5a1dc7b0f..aa98d7ebe 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -10,6 +10,13 @@ void JFactory::Create(const std::shared_ptr& event) { + if (m_app == nullptr && event->GetJApplication() != nullptr) { + // These are usually set by JFactoryGeneratorT, but some user code has custom JFactoryGenerators which don't! + // The design of JFactoryGenerator doesn't give us a better place to inject things + m_app = event->GetJApplication(); + m_logger = m_app->GetJParameterManager()->GetLogger(GetLoggerName()); + } + if (mStatus == Status::Uninitialized) { CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); mStatus = Status::Unprocessed; diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index 9b34afecd..adcc18650 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -21,6 +21,7 @@ set(TEST_SOURCES Components/JEventTests.cc Components/JFactoryDefTagsTests.cc Components/JFactoryTests.cc + Components/JFactoryGeneratorTests.cc Components/JMultiFactoryTests.cc Components/UnfoldTests.cc Components/UserExceptionTests.cc diff --git a/src/programs/unit_tests/Components/JFactoryGeneratorTests.cc b/src/programs/unit_tests/Components/JFactoryGeneratorTests.cc new file mode 100644 index 000000000..6d1212f0f --- /dev/null +++ b/src/programs/unit_tests/Components/JFactoryGeneratorTests.cc @@ -0,0 +1,44 @@ + + +#include +#include +#include +#include + + +// ----------------------------------- +// UserDefined +// ----------------------------------- +namespace jana::components::jfactorygeneratortests_userdefined { + +struct MyData { int x; }; + +class MyFac : public JFactoryT { + void Init() override { + GetApplication(); // This will throw if nullptr + } + void Process(const std::shared_ptr&) override { + std::vector results; + results.push_back(new MyData {22}); + Set(results); + } +}; + +class MyFacGen : public JFactoryGenerator { + + void GenerateFactories(JFactorySet *factory_set) override { + factory_set->Add(new MyFac); + } +}; + +TEST_CASE("JFactoryGeneratorTests_UserDefined") { + JApplication app; + app.Add(new MyFacGen()); + app.Initialize(); + auto event = std::make_shared(); + app.GetService()->configure_event(*event); + + auto data = event->Get(); + REQUIRE(data.at(0)->x == 22); +} +} From 8ed069da7f307d12cafd6b075eae8401aec6f5aa Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 15:25:39 -0400 Subject: [PATCH 2/5] Stop printing the component summary On real-world-sized projects, this summary is far too long. It overwhelms the other log output and isn't particularly helpful for debugging. --- src/libraries/JANA/CLI/JSignalHandler.cc | 1 - src/libraries/JANA/JApplication.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/src/libraries/JANA/CLI/JSignalHandler.cc b/src/libraries/JANA/CLI/JSignalHandler.cc index dec44a793..721bb6e06 100644 --- a/src/libraries/JANA/CLI/JSignalHandler.cc +++ b/src/libraries/JANA/CLI/JSignalHandler.cc @@ -62,7 +62,6 @@ std::string produce_overall_report() { // Include detailed report from JApplication auto t = time(nullptr); ss << "JANA status report: " << ctime(&t) << std::endl; - ss << g_app->GetComponentSummary() << std::endl; // Include backtraces from each individual thread if( typeid(std::thread::native_handle_type) == typeid(pthread_t) ){ diff --git a/src/libraries/JANA/JApplication.cc b/src/libraries/JANA/JApplication.cc index 0c67c998b..faff332f9 100644 --- a/src/libraries/JANA/JApplication.cc +++ b/src/libraries/JANA/JApplication.cc @@ -197,7 +197,6 @@ void JApplication::Run(bool wait_until_finished) { // Print summary of all config parameters m_params->PrintParameters(); - LOG_INFO(m_logger) << GetComponentSummary() << LOG_END; LOG_WARN(m_logger) << "Starting processing with " << m_desired_nthreads << " threads requested..." << LOG_END; m_processing_controller->run(m_desired_nthreads); From 5e6c10c9d4c0217e83c1e5d2e22ed2b2f4982b01 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 15:23:02 -0400 Subject: [PATCH 3/5] Remove deprecated JMetadata --- src/libraries/JANA/JEvent.h | 13 ------------- src/libraries/JANA/JFactoryT.h | 17 ----------------- 2 files changed, 30 deletions(-) diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index e60e7cbf7..ad87250e8 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -75,8 +75,6 @@ class JEvent : public std::enable_shared_from_this template JFactoryT* GetFactory(const std::string& tag = "", bool throw_on_missing=false) const; template std::vector*> GetFactoryAll(bool throw_on_missing = false) const; - template JMetadata GetMetadata(const std::string& tag = "") const; - //OBJECTS // C style getters template JFactoryT* Get(const T** item, const std::string& tag="") const; @@ -299,17 +297,6 @@ inline JFactoryT* JEvent::GetFactory(const std::string& tag, bool throw_on_mi } -/// GetMetadata() provides access to any metadata generated by the underlying JFactory during Process() -template -[[deprecated("Use JMultifactory instead")]] -inline JMetadata JEvent::GetMetadata(const std::string& tag) const { - - auto factory = GetFactory(tag, true); - // Make sure that JFactoryT::Process has already been called before returning the metadata - factory->CreateAndGetData(this->shared_from_this()); - return factory->GetMetadata(); -} - /// C-style getters diff --git a/src/libraries/JANA/JFactoryT.h b/src/libraries/JANA/JFactoryT.h index 46fbccb33..8ac878f5e 100644 --- a/src/libraries/JANA/JFactoryT.h +++ b/src/libraries/JANA/JFactoryT.h @@ -18,12 +18,6 @@ #endif -/// Class template for metadata. This constrains JFactoryT to use the same (user-defined) -/// metadata structure, JMetadata for that T. This is essential for retrieving metadata from -/// JFactoryT's without breaking the Liskov substitution property. -template -struct JMetadata {}; - template class JFactoryT : public JFactory { public: @@ -167,20 +161,9 @@ class JFactoryT : public JFactory { mCreationStatus = CreationStatus::NotCreatedYet; } - /// Set the JFactory's metadata. This is meant to be called by user during their JFactoryT::Process - /// Metadata will *not* be cleared on ClearData(), but will be destroyed when the JFactoryT is. - [[deprecated("Use JMultifactory instead")]] - void SetMetadata(JMetadata metadata) { mMetadata = metadata; } - - /// Get the JFactory's metadata. This is meant to be called by user during their JFactoryT::Process - /// and also used by JEvent under the hood. - /// Metadata will *not* be cleared on ClearData(), but will be destroyed when the JFactoryT is. - JMetadata GetMetadata() { return mMetadata; } - protected: std::vector mData; - JMetadata mMetadata; }; template From 3804c45ee0d93629b3d1fc0607a1f171032b00b0 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 15:29:27 -0400 Subject: [PATCH 4/5] Remove deprecated JParameterManager::PrintParameters(bool,bool,bool) --- src/libraries/JANA/Services/JParameterManager.cc | 16 ---------------- src/libraries/JANA/Services/JParameterManager.h | 3 --- 2 files changed, 19 deletions(-) diff --git a/src/libraries/JANA/Services/JParameterManager.cc b/src/libraries/JANA/Services/JParameterManager.cc index 8ed883bb5..f507bfe1f 100644 --- a/src/libraries/JANA/Services/JParameterManager.cc +++ b/src/libraries/JANA/Services/JParameterManager.cc @@ -93,22 +93,6 @@ void JParameterManager::PrintParameters() { PrintParameters(m_verbosity, m_strictness); } -void JParameterManager::PrintParameters(bool show_defaulted, bool show_advanced, bool warn_on_unused) { - int verbosity = 1; - int strictness = 0; - - if (show_advanced) { - verbosity = 3; - } - else if (show_defaulted) { - verbosity = 2; - } - if (warn_on_unused) { - strictness = 1; - } - PrintParameters(verbosity, strictness); -} - /// @brief Prints parameters to stdout /// diff --git a/src/libraries/JANA/Services/JParameterManager.h b/src/libraries/JANA/Services/JParameterManager.h index f7ed6ced5..6950d9357 100644 --- a/src/libraries/JANA/Services/JParameterManager.h +++ b/src/libraries/JANA/Services/JParameterManager.h @@ -87,9 +87,6 @@ class JParameterManager : public JService { void PrintParameters(int verbosity, int strictness); - [[deprecated]] - void PrintParameters(bool show_defaulted, bool show_advanced=true, bool warn_on_unused=false); - std::map GetAllParameters(); template From 5dbbbfa40ea826250c68565775c145f0017aaa0c Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Sep 2024 15:33:06 -0400 Subject: [PATCH 5/5] Remove deprecated JFactoryT constructors --- src/libraries/JANA/JFactoryT.h | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/libraries/JANA/JFactoryT.h b/src/libraries/JANA/JFactoryT.h index 8ac878f5e..bf95ada57 100644 --- a/src/libraries/JANA/JFactoryT.h +++ b/src/libraries/JANA/JFactoryT.h @@ -25,24 +25,6 @@ class JFactoryT : public JFactory { using IteratorType = typename std::vector::const_iterator; using PairType = std::pair; - /// JFactoryT constructor requires a name and a tag. - /// Name should always be JTypeInfo::demangle(), tag is usually "". - JFactoryT(const std::string& aName, const std::string& aTag) __attribute__ ((deprecated)) : JFactory(aName, aTag) { - EnableGetAs(); - EnableGetAs( std::is_convertible() ); // Automatically add JObject if this can be converted to it -#if JANA2_HAVE_ROOT - EnableGetAs( std::is_convertible() ); // Automatically add TObject if this can be converted to it -#endif - } - - JFactoryT(const std::string& aName) __attribute__ ((deprecated)) : JFactory(aName, "") { - EnableGetAs(); - EnableGetAs( std::is_convertible() ); // Automatically add JObject if this can be converted to it -#if JANA2_HAVE_ROOT - EnableGetAs( std::is_convertible() ); // Automatically add TObject if this can be converted to it -#endif - } - JFactoryT() : JFactory(JTypeInfo::demangle(), ""){ EnableGetAs(); EnableGetAs( std::is_convertible() ); // Automatically add JObject if this can be converted to it