From 3d73a067ba37cd2863e6b5e693b6a9b08b4de8ba Mon Sep 17 00:00:00 2001
From: jmcarcell <jmcarcell@users.noreply.github.com>
Date: Mon, 15 Apr 2024 07:57:52 +0200
Subject: [PATCH] Fix issue with relations after reading and add test

Fix an issue where it's not possible to push_back to a cloned object that has
been read because all the OneToManyRelations in the same collection
point to the same vector. The fix is to actually make a copy of the
part of the vector that belongs to that object.
---
 python/templates/Object.cc.jinja2              |  2 +-
 python/templates/macros/implementations.jinja2 | 10 ++++++++--
 tests/read_frame.h                             |  9 +++++++++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/python/templates/Object.cc.jinja2 b/python/templates/Object.cc.jinja2
index e95548fcc..bfd97ad8d 100644
--- a/python/templates/Object.cc.jinja2
+++ b/python/templates/Object.cc.jinja2
@@ -16,7 +16,7 @@
 
 {{ utils.namespace_open(class.namespace) }}
 
-{{ macros.constructors_destructors(class.bare_type, Members) }}
+{{ macros.constructors_destructors(class.bare_type, Members, OneToManyRelations) }}
 
 {{ class.bare_type }}::{{ class.bare_type }}(const Mutable{{ class.bare_type }}& other): {{ class.bare_type }}(other.m_obj) {}
 
diff --git a/python/templates/macros/implementations.jinja2 b/python/templates/macros/implementations.jinja2
index cd453eefa..eb6b2f82c 100644
--- a/python/templates/macros/implementations.jinja2
+++ b/python/templates/macros/implementations.jinja2
@@ -1,4 +1,4 @@
-{% macro constructors_destructors(type, members, prefix='') %}
+{% macro constructors_destructors(type, members, relations=[], prefix='') %}
 {% set full_type = prefix + type %}
 
 {{ full_type }}::{{ full_type }}() :
@@ -19,7 +19,13 @@
 }
 
 Mutable{{ type }} {{ full_type }}::clone() const {
-  return Mutable{{ type }}(podio::utils::MaybeSharedPtr(new {{ type }}Obj(*m_obj), podio::utils::MarkOwned));
+  auto tmp = Mutable{{ type }}(podio::utils::MaybeSharedPtr(new {{ type }}Obj(*m_obj), podio::utils::MarkOwned));
+{% for relation in relations %}
+  if (m_obj->data.{{ relation.name }}_end != m_obj->m_{{ relation.name }}->size()) {
+    tmp.m_obj->m_{{ relation.name }} = new std::vector<{{ relation.full_type }}>(m_obj->m_{{ relation.name }}->begin() + m_obj->data.{{ relation.name }}_begin, m_obj->m_{{ relation.name }}->begin() + m_obj->data.{{ relation.name }}_end);
+  }
+{% endfor %}
+  return tmp;
 }
 
 {{ full_type }}::{{ full_type }}(podio::utils::MaybeSharedPtr<{{ type }}Obj> obj) : m_obj(std::move(obj)) {}
diff --git a/tests/read_frame.h b/tests/read_frame.h
index 3a9ec2929..b8886d05f 100644
--- a/tests/read_frame.h
+++ b/tests/read_frame.h
@@ -59,6 +59,15 @@ void processExtensions(const podio::Frame& event, int iEvent, podio::version::Ve
   ASSERT(structs[0].y == 0, "struct value not as expected");
   ASSERT(structs[1].y == iEvent, "struct value not as expected");
   ASSERT(structs[2].y == 2 * iEvent, "struct value not as expected");
+
+  auto ncluster = clusters[0].clone();
+  ASSERT(ncluster.Hits().size() == 1, "cluster should have 1 hit");
+
+  auto hit = ExampleHit(420, {}, {}, {}, {});
+  ncluster.addHits(hit);
+  ASSERT(ncluster.Hits().size() == 2, "cluster should have 2 hits");
+  ASSERT(ncluster.Hits(1).cellID() == 420, "cellID should be 420");
+
 }
 
 void checkVecMemSubsetColl(const podio::Frame& event) {