Merge pull request #366 from JeffersonLab/nbrei_facgen_fix

Bugfix: User-defined factory generators

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) ){

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);
 

src/libraries/JANA/JEvent.h

@@ -75,8 +75,6 @@ class JEvent : public std::enable_shared_from_this<JEvent>
         template<class T> JFactoryT<T>* GetFactory(const std::string& tag = "", bool throw_on_missing=false) const;
         template<class T> std::vector<JFactoryT<T>*> GetFactoryAll(bool throw_on_missing = false) const;
 
-        template<class T> JMetadata<T> GetMetadata(const std::string& tag = "") const;
-
         //OBJECTS
         // C style getters
         template<class T> JFactoryT<T>* Get(const T** item, const std::string& tag="") const;
@@ -299,17 +297,6 @@ inline JFactoryT<T>* JEvent::GetFactory(const std::string& tag, bool throw_on_mi
 }
 
 
-/// GetMetadata() provides access to any metadata generated by the underlying JFactory during Process()
-template<class T>
-[[deprecated("Use JMultifactory instead")]]
-inline JMetadata<T> JEvent::GetMetadata(const std::string& tag) const {
-
-    auto factory = GetFactory<T>(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
 

src/libraries/JANA/JFactory.cc

@@ -10,6 +10,13 @@
 
 void JFactory::Create(const std::shared_ptr<const JEvent>& 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;

src/libraries/JANA/JFactoryT.h

@@ -18,37 +18,13 @@
 #endif
 
 
-/// Class template for metadata. This constrains JFactoryT<T> to use the same (user-defined)
-/// metadata structure, JMetadata<T> for that T. This is essential for retrieving metadata from
-/// JFactoryT's without breaking the Liskov substitution property.
-template<typename T>
-struct JMetadata {};
-
 template<typename T>
 class JFactoryT : public JFactory {
 public:
 
     using IteratorType = typename std::vector<T*>::const_iterator;
     using PairType = std::pair<IteratorType, IteratorType>;
 
-    /// JFactoryT constructor requires a name and a tag.
-    /// Name should always be JTypeInfo::demangle<T>(), tag is usually "".
-    JFactoryT(const std::string& aName, const std::string& aTag) __attribute__ ((deprecated)) : JFactory(aName, aTag) {
-        EnableGetAs<T>();
-        EnableGetAs<JObject>( std::is_convertible<T,JObject>() ); // Automatically add JObject if this can be converted to it
-#if JANA2_HAVE_ROOT
-        EnableGetAs<TObject>( std::is_convertible<T,TObject>() ); // Automatically add TObject if this can be converted to it
-#endif
-    }
-
-    JFactoryT(const std::string& aName) __attribute__ ((deprecated))  : JFactory(aName, "") {
-        EnableGetAs<T>();
-        EnableGetAs<JObject>( std::is_convertible<T,JObject>() ); // Automatically add JObject if this can be converted to it
-#if JANA2_HAVE_ROOT
-        EnableGetAs<TObject>( std::is_convertible<T,TObject>() ); // Automatically add TObject if this can be converted to it
-#endif
-    }
-
     JFactoryT() : JFactory(JTypeInfo::demangle<T>(), ""){
         EnableGetAs<T>();
         EnableGetAs<JObject>( std::is_convertible<T,JObject>() ); // Automatically add JObject if this can be converted to it
@@ -167,20 +143,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<T> 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<T> GetMetadata() { return mMetadata; }
-
 
 protected:
     std::vector<T*> mData;
-    JMetadata<T> mMetadata;
 };
 
 template<typename T>

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
 ///

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<std::string, JParameter*> GetAllParameters();
 
     template<typename T>

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

src/programs/unit_tests/Components/JFactoryGeneratorTests.cc

@@ -0,0 +1,44 @@
+
+
+#include <catch.hpp>
+#include <JANA/JApplication.h>
+#include <JANA/JFactoryGenerator.h>
+#include <JANA/Services/JComponentManager.h>
+
+
+// -----------------------------------
+// UserDefined
+// -----------------------------------
+namespace jana::components::jfactorygeneratortests_userdefined {
+
+struct MyData { int x; };
+
+class MyFac : public JFactoryT<MyData> {
+    void Init() override {
+        GetApplication(); // This will throw if nullptr
+    }
+    void Process(const std::shared_ptr<const JEvent>&) override {
+        std::vector<MyData*> 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<JEvent>();
+    app.GetService<JComponentManager>()->configure_event(*event);
+
+    auto data = event->Get<MyData>();
+    REQUIRE(data.at(0)->x == 22);
+}
+}