startPublishing in ObjectManager uses check for mergeable items

Michal Tichák · Michal Tichák · commit d288105d19d2 · 2024-08-30T17:28:48.000+02:00
diff --git a/Framework/include/QualityControl/ObjectsManager.h b/Framework/include/QualityControl/ObjectsManager.h
@@ -21,17 +21,14 @@
 #include "QualityControl/Activity.h"
 #include "QualityControl/MonitorObject.h"
 #include "QualityControl/MonitorObjectCollection.h"
+#include <Mergers/Mergeable.h>
 // stl
+#include <concepts>
 #include <string>
 #include <memory>
+#include <type_traits>
 
 class TObject;
-class TObjArray;
-class TH1;
-class TTree;
-class THnBase;
-class TEfficiency;
-class TGraph;
 
 namespace o2::quality_control::core
 {
@@ -84,17 +81,15 @@ class ObjectsManager
    * @param obj The object to publish.
    * @throws DuplicateObjectError
    */
-  template <typename T>
-  void startPublishing(T* obj, PublicationPolicy policy = PublicationPolicy::Forever)
+  template <bool IgnoreMergeable = false, typename T>
+  void startPublishing(T obj, PublicationPolicy policy = PublicationPolicy::Forever)
   {
-    if constexpr (!std::is_base_of_v<mergers::MergeInterface, T>() &&
-                  !std::is_base_of_v<TCollection, T>() &&
-                  !std::is_base_of_v<TH1, T>() &&
-                  !std::is_base_of_v<TTree, T>() &&
-                  !std::is_base_of_v<TGraph, T>() &&
-                  !std::is_base_of_v<TEfficiency, T>()) {
-      static_assert(true, "You are trying to start publish object, that is not mergeable");
-    }
+    static_assert(std::same_as<std::remove_pointer_t<T>, TObject> ||
+                    // there is TCanvas mentioned here because users are using it a lot when they should not
+                    std::same_as<std::remove_pointer_t<T>, TCanvas> ||
+                    mergers::Mergeable<IgnoreMergeable, T>,
+                  "you are trying to startPublishing object that is not mergeable."
+                  " If you know what you are doing use startPublishing<true>(...)");
     startPublishingImpl(obj, policy);
   }
 
@@ -103,7 +98,8 @@ class ObjectsManager
    * @param obj
    * @throw ObjectNotFoundError if object is not found.
    */
-  void stopPublishing(TObject* obj);
+  void
+    stopPublishing(TObject* obj);
 
   /**
    * Stop publishing this object
diff --git a/Framework/src/ObjectsManager.cxx b/Framework/src/ObjectsManager.cxx
@@ -60,7 +60,7 @@ ObjectsManager::~ObjectsManager()
   ILOG(Debug, Devel) << "ObjectsManager destructor" << ENDM;
 }
 
-void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicationPolicy)
+void ObjectsManager::startPublishingImpl(TObject* object, PublicationPolicy publicationPolicy)
 {
   if (!object) {
     ILOG(Warning, Support) << "A nullptr provided to ObjectManager::startPublishing" << ENDM;
@@ -70,6 +70,9 @@ void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicat
     ILOG(Warning, Support) << "Object is already being published (" << object->GetName() << "), will remove it and add the new one" << ENDM;
     stopPublishing(object->GetName());
   }
+  if (mergers::isMergeable(object)) {
+    ILOG(Warning, Support) << "Object '" + std::string(object->GetName()) + "' with type '" + std::string(object->ClassName()) + "' is not one of the mergeable types, it might cause issues during publishing";
+  }
   auto* newObject = new MonitorObject(object, mTaskName, mTaskClass, mDetectorName);
   newObject->setIsOwner(false);
   newObject->setActivity(mActivity);
diff --git a/Framework/src/SliceTrendingTask.cxx b/Framework/src/SliceTrendingTask.cxx
@@ -253,7 +253,7 @@ void SliceTrendingTask::generatePlots()
     }
 
     mPlots[plot.name] = c;
-    getObjectsManager()->startPublishing(c, PublicationPolicy::Once);
+    getObjectsManager()->startPublishing<true>(c, PublicationPolicy::Once);
   }
 } // void SliceTrendingTask::generatePlots()
 
@@ -666,4 +666,4 @@ std::string SliceTrendingTask::beautifyTitle(const std::string_view rawtitle, co
   }
 
   return beautified;
-}
+}
diff --git a/Framework/src/TrendingTask.cxx b/Framework/src/TrendingTask.cxx
@@ -257,7 +257,7 @@ void TrendingTask::generatePlots()
     }
     auto c = drawPlot(plotConfig);
     mPlots[plotConfig.name].reset(c);
-    getObjectsManager()->startPublishing(c, PublicationPolicy::Once);
+    getObjectsManager()->startPublishing<true>(c, PublicationPolicy::Once);
   }
 }
 
diff --git a/Framework/test/testObjectsManager.cxx b/Framework/test/testObjectsManager.cxx
@@ -53,12 +53,12 @@ BOOST_AUTO_TEST_CASE(duplicate_object_test)
   config.consulUrl = "";
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
   TObjString s("content");
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(&s, PublicationPolicy::Forever));
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever));
   BOOST_REQUIRE(objectsManager.getMonitorObject("content") != nullptr);
 
   TObjString s2("content");
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(&s2, PublicationPolicy::Forever));
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(&s2, PublicationPolicy::Forever));
   auto mo2 = objectsManager.getMonitorObject("content");
   BOOST_REQUIRE(mo2 != nullptr);
   BOOST_REQUIRE(mo2->getObject() != &s);
@@ -73,8 +73,8 @@ BOOST_AUTO_TEST_CASE(is_being_published_test)
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
   TObjString s("content");
   BOOST_CHECK(!objectsManager.isBeingPublished("content"));
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(&s, PublicationPolicy::Forever));
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever));
   BOOST_CHECK(objectsManager.isBeingPublished("content"));
 }
 
@@ -84,46 +84,46 @@ BOOST_AUTO_TEST_CASE(unpublish_test)
   config.taskName = "test";
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
   TObjString s("content");
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishing(&s);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishing("content");
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
   BOOST_CHECK_THROW(objectsManager.stopPublishing("content"), ObjectNotFoundError);
   BOOST_CHECK_THROW(objectsManager.stopPublishing("asdf"), ObjectNotFoundError);
 
   // unpublish all
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishingAll();
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
   BOOST_CHECK_NO_THROW(objectsManager.stopPublishingAll());
 
   // unpublish after deletion
   auto s2 = new TObjString("content");
-  objectsManager.startPublishing(s2, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(s2, PublicationPolicy::Forever);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   delete s2;
   objectsManager.stopPublishing(s2);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
 
   // unpublish for publication policy
   auto s3 = new TObjString("content3");
-  objectsManager.startPublishing(s3, PublicationPolicy::Once);
+  objectsManager.startPublishing<true>(s3, PublicationPolicy::Once);
   auto s4 = new TObjString("content4");
-  objectsManager.startPublishing(s4, PublicationPolicy::Once);
+  objectsManager.startPublishing<true>(s4, PublicationPolicy::Once);
   auto s5 = new TObjString("content5");
-  objectsManager.startPublishing(s5, PublicationPolicy::ThroughStop);
+  objectsManager.startPublishing<true>(s5, PublicationPolicy::ThroughStop);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 3);
   objectsManager.stopPublishing(PublicationPolicy::Once);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 1);
   objectsManager.stopPublishing(PublicationPolicy::ThroughStop);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
 
-  objectsManager.startPublishing(s3, PublicationPolicy::Once);
+  objectsManager.startPublishing<true>(s3, PublicationPolicy::Once);
   objectsManager.stopPublishing(s3);
   BOOST_CHECK_EQUAL(objectsManager.getNumberPublishedObjects(), 0);
   BOOST_CHECK_NO_THROW(objectsManager.stopPublishing(PublicationPolicy::Once));
@@ -145,8 +145,8 @@ BOOST_AUTO_TEST_CASE(getters_test)
   TObjString s("content");
   TH1F h("histo", "h", 100, 0, 99);
 
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  objectsManager.startPublishing(&h, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&h, PublicationPolicy::Forever);
 
   // basic gets
   BOOST_CHECK_NO_THROW(objectsManager.getMonitorObject("content"));
@@ -174,8 +174,8 @@ BOOST_AUTO_TEST_CASE(metadata_test)
 
   TObjString s("content");
   TH1F h("histo", "h", 100, 0, 99);
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
-  objectsManager.startPublishing(&h, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&h, PublicationPolicy::Forever);
 
   objectsManager.addMetadata("content", "aaa", "bbb");
   BOOST_CHECK_EQUAL(objectsManager.getMonitorObject("content")->getMetadataMap().at("aaa"), "bbb");
@@ -211,7 +211,7 @@ BOOST_AUTO_TEST_CASE(feed_with_nullptr)
   config.consulUrl = "";
   ObjectsManager objectsManager(config.taskName, config.taskClass, config.detectorName, config.consulUrl, 0, true);
 
-  BOOST_CHECK_NO_THROW(objectsManager.startPublishing(nullptr, PublicationPolicy::Forever));
+  BOOST_CHECK_NO_THROW(objectsManager.startPublishing<true>(nullptr, PublicationPolicy::Forever));
   BOOST_CHECK_NO_THROW(objectsManager.setDefaultDrawOptions(nullptr, ""));
   BOOST_CHECK_NO_THROW(objectsManager.setDisplayHint(nullptr, ""));
   BOOST_CHECK_NO_THROW(objectsManager.stopPublishing(nullptr));
diff --git a/Framework/test/testPublisher.cxx b/Framework/test/testPublisher.cxx
@@ -38,7 +38,7 @@ BOOST_AUTO_TEST_CASE(publisher_test)
   std::string consulUrl = "invalid";
   ObjectsManager objectsManager(taskName, "taskClass", detectorName, consulUrl, 0, true);
   TObjString s("content");
-  objectsManager.startPublishing(&s, PublicationPolicy::Forever);
+  objectsManager.startPublishing<true>(&s, PublicationPolicy::Forever);
 
   TObjString* s2 = (TObjString*)(objectsManager.getMonitorObject("content")->getObject());
   BOOST_CHECK_EQUAL(s.GetString(), s2->GetString());

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ ObjectsManager::~ObjectsManager()`
`60`	`60`	`ILOG(Debug, Devel) << "ObjectsManager destructor" << ENDM;`
`61`	`61`	`}`
`62`	`62`
`63`		`-void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicationPolicy)`
	`63`	`+void ObjectsManager::startPublishingImpl(TObject* object, PublicationPolicy publicationPolicy)`
`64`	`64`	`{`
`65`	`65`	`if (!object) {`
`66`	`66`	`ILOG(Warning, Support) << "A nullptr provided to ObjectManager::startPublishing" << ENDM;`
`@@ -70,6 +70,9 @@ void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicat`
`70`	`70`	`ILOG(Warning, Support) << "Object is already being published (" << object->GetName() << "), will remove it and add the new one" << ENDM;`
`71`	`71`	`stopPublishing(object->GetName());`
`72`	`72`	`}`
	`73`	`+ if (mergers::isMergeable(object)) {`
	`74`	`+ ILOG(Warning, Support) << "Object '" + std::string(object->GetName()) + "' with type '" + std::string(object->ClassName()) + "' is not one of the mergeable types, it might cause issues during publishing";`
	`75`	`+ }`
`73`	`76`	`auto* newObject = new MonitorObject(object, mTaskName, mTaskClass, mDetectorName);`
`74`	`77`	`newObject->setIsOwner(false);`
`75`	`78`	`newObject->setActivity(mActivity);`
Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ void SliceTrendingTask::generatePlots()`
`253`	`253`	`}`
`254`	`254`
`255`	`255`	`mPlots[plot.name] = c;`
`256`		`- getObjectsManager()->startPublishing(c, PublicationPolicy::Once);`
	`256`	`+ getObjectsManager()->startPublishing<true>(c, PublicationPolicy::Once);`
`257`	`257`	`}`
`258`	`258`	`} // void SliceTrendingTask::generatePlots()`
`259`	`259`
`@@ -666,4 +666,4 @@ std::string SliceTrendingTask::beautifyTitle(const std::string_view rawtitle, co`
`666`	`666`	`}`
`667`	`667`
`668`	`668`	`return beautified;`
`669`		`-}`
	`669`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -257,7 +257,7 @@ void TrendingTask::generatePlots()`
`257`	`257`	`}`
`258`	`258`	`auto c = drawPlot(plotConfig);`
`259`	`259`	`mPlots[plotConfig.name].reset(c);`
`260`		`- getObjectsManager()->startPublishing(c, PublicationPolicy::Once);`
	`260`	`+ getObjectsManager()->startPublishing<true>(c, PublicationPolicy::Once);`
`261`	`261`	`}`
`262`	`262`	`}`
`263`	`263`