switch MOs in MonitorObjectCollection when run number is higher on new (#2443)

Co-authored-by: Michal Tichák <michal.tichak@cern.ch>

Author: justonedev1

Framework/src/MonitorObjectCollection.cxx

@@ -46,16 +46,32 @@ void MonitorObjectCollection::merge(mergers::MergeInterface* const other)
       if (!otherMO || !targetMO) {
         throw std::runtime_error("The target object or the other object could not be casted to MonitorObject.");
       }
-      if (!reportedMismatchingRunNumbers && targetMO->getActivity().mId != otherMO->getActivity().mId) {
+
+      if (otherMO->getActivity().mId > targetMO->getActivity().mId) {
+        ILOG(Error, Ops) << "The run number of the input object '" << otherMO->GetName() << "' ("
+                         << otherMO->getActivity().mId << ") "
+                         << "is higher than the one of the target object '"
+                         << targetMO->GetName() << "' (" << targetMO->getActivity().mId
+                         << "). Replacing the merged object with input, "
+                         << "but THIS SHOULD BE IMMEDIATELY ADDRESSED IN PRODUCTION. "
+                         << "QC objects from other setups are reaching this one."
+                         << ENDM;
+        otherMO->Copy(*targetMO);
+        continue;
+      }
+
+      if (!reportedMismatchingRunNumbers && otherMO->getActivity().mId < targetMO->getActivity().mId) {
         ILOG(Error, Ops) << "The run number of the input object '" << otherMO->GetName() << "' ("
                          << otherMO->getActivity().mId << ") "
                          << "does not match the run number of the target object '"
                          << targetMO->GetName() << "' (" << targetMO->getActivity().mId
-                         << "). Trying to continue, but THIS SHOULD BE IMMEDIATELY ADDRESSED IN PRODUCTION. "
+                         << "). Ignoring this object and trying to continue, but THIS SHOULD BE IMMEDIATELY ADDRESSED IN PRODUCTION. "
                          << "QC objects from other setups are reaching this one. Will not report more mismatches in this collection."
                          << ENDM;
         reportedMismatchingRunNumbers = true;
+        continue;
       }
+
       // That might be another collection or a concrete object to be merged, we walk on the collection recursively.
       algorithm::merge(targetMO->getObject(), otherMO->getObject());
       if (otherMO->getValidity().isValid()) {

Framework/test/testMonitorObjectCollection.cxx

@@ -16,8 +16,8 @@
 
 #include "QualityControl/MonitorObjectCollection.h"
 #include "QualityControl/MonitorObject.h"
-#include "QualityControl/QcInfoLogger.h"
 
+#include <TH1.h>
 #include <TH1I.h>
 #include <TH2I.h>
 #include <TH2I.h>
@@ -96,6 +96,77 @@ TEST_CASE("monitor_object_collection_merge")
   delete target;
 }
 
+TEST_CASE("monitor_object_collection_merge_different_id")
+{
+  const auto toHisto = [](std::unique_ptr<MonitorObjectCollection>& collection) -> TH1I* {
+    return dynamic_cast<TH1I*>(dynamic_cast<MonitorObject*>(collection->At(0))->getObject());
+  };
+
+  constexpr size_t bins = 10;
+  constexpr size_t min = 0;
+  constexpr size_t max = 10;
+
+  SECTION("other has higher run number than target")
+  {
+    auto target = std::make_unique<MonitorObjectCollection>();
+
+    auto* targetTH1I = new TH1I("histo 1d", "original", bins, min, max);
+    targetTH1I->Fill(5);
+    auto* targetMoTH1I = new MonitorObject(targetTH1I, "histo 1d", "class", "DET");
+    targetMoTH1I->setActivity({ 123, "PHYSICS", "LHC32x", "apass2", "qc_async", { 10, 20 } });
+    targetMoTH1I->setIsOwner(true);
+    target->Add(targetMoTH1I);
+
+    auto other = std::make_unique<MonitorObjectCollection>();
+    other->SetOwner(true);
+
+    auto* otherTH1I = new TH1I("histo 1d", "input", bins, min, max);
+    otherTH1I->Fill(2);
+    auto* otherMoTH1I = new MonitorObject(otherTH1I, "histo 1d", "class", "DET");
+    otherMoTH1I->setActivity({ 1234, "PHYSICS", "LHC32x", "apass2", "qc_async", { 43, 60 } });
+    otherMoTH1I->setIsOwner(true);
+    other->Add(otherMoTH1I);
+
+    CHECK_NOTHROW(algorithm::merge(target.get(), other.get()));
+    auto* h1orig = toHisto(target);
+    auto* h1other = toHisto(other);
+    REQUIRE(h1orig->GetAt(3) == 1);
+    for (size_t i = 0; i != h1orig->GetSize(); ++i) {
+      REQUIRE(h1orig->GetAt(i) == h1other->GetAt(i));
+    }
+  }
+
+  SECTION("other has lower run number than target")
+  {
+    auto target = std::make_unique<MonitorObjectCollection>();
+
+    auto* targetTH1I = new TH1I("histo 1d", "original", bins, min, max);
+    targetTH1I->Fill(5);
+    auto* targetMoTH1I = new MonitorObject(targetTH1I, "histo 1d", "class", "DET");
+    targetMoTH1I->setActivity({ 1234, "PHYSICS", "LHC32x", "apass2", "qc_async", { 10, 20 } });
+    targetMoTH1I->setIsOwner(true);
+    target->Add(targetMoTH1I);
+
+    auto other = std::make_unique<MonitorObjectCollection>();
+    other->SetOwner(true);
+
+    auto* otherTH1I = new TH1I("histo 1d", "input", bins, min, max);
+    otherTH1I->Fill(2);
+    auto* otherMoTH1I = new MonitorObject(otherTH1I, "histo 1d", "class", "DET");
+    otherMoTH1I->setActivity({ 123, "PHYSICS", "LHC32x", "apass2", "qc_async", { 43, 60 } });
+    otherMoTH1I->setIsOwner(true);
+    other->Add(otherMoTH1I);
+
+    CHECK_NOTHROW(algorithm::merge(target.get(), other.get()));
+    auto* h1orig = toHisto(target);
+    auto* h1other = toHisto(other);
+    REQUIRE(h1orig->At(h1orig->FindBin(5)) == 1);
+    REQUIRE(h1other->At(h1other->FindBin(5)) == 0);
+    REQUIRE(h1orig->At(h1orig->FindBin(2)) == 0);
+    REQUIRE(h1other->At(h1other->FindBin(2)) == 1);
+  }
+}
+
 TEST_CASE("monitor_object_collection_post_deserialization")
 {
   const size_t bins = 10;
@@ -171,4 +242,4 @@ TEST_CASE("monitor_object_collection_clone_mw")
   delete mwMOC2;
 }
 
-} // namespace o2::quality_control::core
+} // namespace o2::quality_control::core