From 94dd0d0c4e895be153ca5127ede28baedf1a4a1c Mon Sep 17 00:00:00 2001
From: Mateusz Jakub Fila <37295697+m-fila@users.noreply.github.com>
Date: Wed, 4 Sep 2024 16:56:25 +0200
Subject: [PATCH] Fix accessing metadata with `MetaDataHandle` when
 `MetadataSvc` is present (#231)

---
 k4FWCore/include/k4FWCore/MetaDataHandle.h    | 32 +++++++------------
 .../components/k4FWCoreTest_cellID_reader.cpp | 12 +++++++
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/k4FWCore/include/k4FWCore/MetaDataHandle.h b/k4FWCore/include/k4FWCore/MetaDataHandle.h
index c923b785..0a906428 100644
--- a/k4FWCore/include/k4FWCore/MetaDataHandle.h
+++ b/k4FWCore/include/k4FWCore/MetaDataHandle.h
@@ -26,7 +26,6 @@
 
 template <typename T> class MetaDataHandle {
 public:
-  MetaDataHandle();
   MetaDataHandle(const std::string& descriptor, Gaudi::DataHandle::Mode a);
   MetaDataHandle(const Gaudi::DataHandle& handle, const std::string& descriptor, Gaudi::DataHandle::Mode a);
 
@@ -89,25 +88,20 @@ MetaDataHandle<T>::MetaDataHandle(const Gaudi::DataHandle& handle, const std::st
 
 //---------------------------------------------------------------------------
 template <typename T> std::optional<T> MetaDataHandle<T>::get_optional() const {
-  const auto& frame = m_podio_data_service->getMetaDataFrame();
-  return frame.getParameter<T>(fullDescriptor());
+  if (m_podio_data_service) {
+    return m_podio_data_service->getMetaDataFrame().getParameter<T>(fullDescriptor());
+  }
+  return k4FWCore::getParameter<T>(fullDescriptor());
 }
 
 //---------------------------------------------------------------------------
 template <typename T> const T MetaDataHandle<T>::get() const {
-  std::optional<T> maybeVal;
-  // DataHandle based algorithms
-  if (m_podio_data_service) {
-    maybeVal = get_optional();
-    if (!maybeVal.has_value()) {
-      throw GaudiException("MetaDataHandle empty handle access",
-                           "MetaDataHandle " + fullDescriptor() + " not (yet?) available", StatusCode::FAILURE);
-    }
-    // Functional algorithms
-  } else {
-    maybeVal = k4FWCore::getParameter<T>(fullDescriptor());
+  auto optional_parameter = get_optional();
+  if (!optional_parameter.has_value()) {
+    throw GaudiException("MetaDataHandle empty handle access",
+                         "MetaDataHandle " + fullDescriptor() + " not (yet?) available", StatusCode::FAILURE);
   }
-  return maybeVal.value();
+  return optional_parameter.value();
 }
 
 //---------------------------------------------------------------------------
@@ -159,12 +153,8 @@ template <typename T> void MetaDataHandle<T>::checkPodioDataSvc() {
   if (cmd.find("genconf") != std::string::npos)
     return;
 
-  // The proper check would be the following:
-  // if (!m_podio_data_service && !Gaudi::svcLocator()->service<IMetadataSvc>("MetadataSvc")) {
-  // However, it seems there is always a service called "MetadataSvc" from Gaudi,
-  // so the check will always pass
-  if (!m_podio_data_service) {
-    std::cout << "Warning: MetaDataHandles require the PodioDataSvc (ignore if using IOSvc)" << std::endl;
+  if (!m_podio_data_service && !Gaudi::svcLocator()->service<IMetadataSvc>("MetadataSvc", false)) {
+    std::cout << "Warning: MetaDataHandles require the PodioDataSvc or for compatibility the MetadataSvc" << std::endl;
   }
 }
 
diff --git a/test/k4FWCoreTest/src/components/k4FWCoreTest_cellID_reader.cpp b/test/k4FWCoreTest/src/components/k4FWCoreTest_cellID_reader.cpp
index dcc37779..9d48733c 100644
--- a/test/k4FWCoreTest/src/components/k4FWCoreTest_cellID_reader.cpp
+++ b/test/k4FWCoreTest/src/components/k4FWCoreTest_cellID_reader.cpp
@@ -39,5 +39,17 @@ StatusCode k4FWCoreTest_cellID_reader::execute(const EventContext&) const {
     return StatusCode::FAILURE;
   }
 
+  auto optCellIDstr = m_cellIDHandle.get_optional();
+
+  if (!optCellIDstr.has_value()) {
+    error() << "ERROR: cellID is empty but was expected to hold a value" << endmsg;
+    return StatusCode::FAILURE;
+  }
+
+  if (optCellIDstr.value() != cellIDstr) {
+    error() << "ERROR: metadata accessed with by optional and value differs" << endmsg;
+    return StatusCode::FAILURE;
+  }
+
   return StatusCode::SUCCESS;
 }