Correct clone behavior for algorithm mixins

- Deep copy of Properties
- No copy for Resources (force re-inializiation to properly relocate the
  internal Resource pointers)

JugAlgo/JugAlgo/Algorithm.h

@@ -88,8 +88,6 @@ template <class AlgoImpl> class Algorithm : public GaudiAlgorithm {
       // need to get info on MC-ness of this sample somehow TODO
       c.mc(true);
       m_algo.executeInContext(m_input.get(), m_output.get(), c);
-      //m_algo.context(c);
-      //m_algo.execute(m_input.get(), m_output.get());
     } catch (const std::exception& e) {
       error() << e.what() << endmsg;
       return StatusCode::FAILURE;

external/algorithms/calorimetry/include/algorithms/calorimetry/ClusterRecoCoG.h

@@ -35,15 +35,14 @@ class ClusterRecoCoG : public ClusteringAlgorithm {
 public:
   using WeightFunc = std::function<double(double, double, double)>;
 
-  // TODO: get rid of "Collection" in names
+  // FIXME: get rid of "Collection" in names
   ClusterRecoCoG(std::string_view name)
       : ClusteringAlgorithm{name,
                             {"inputProtoClusterCollection", "mcHits"},
                             {"outputClusterCollection", "outputAssociations"},
                             "Reconstruct a cluster with the Center of Gravity method. For "
                             "simulation results it optionally creates a Cluster <-> MCParticle "
                             "association provided both optional arguments are provided."} {}
-
 private:
   void init() final;
   void process(const Input&, const Output&) const final;

external/algorithms/core/include/algorithms/algorithm.h

@@ -96,7 +96,14 @@ template <class InputType, class OutputType> class Algorithm : public AlgorithmH
     copy->context(c, copy.get());
     copy->execute(i, o);
   }
-  auto clone() const { return std::make_unique<Algorithm>(*this); }
+  auto clone() const {
+    auto copy = std::make_unique<Algorithm>(*this);
+    // copy over property values
+    for (const auto& [key, prop] : getProperties()) {
+      std::visit([&](auto&& val) { copy->setProperty(key, val); }, prop.get());
+    }
+    return copy;
+  }
 
   const InputNames& inputNames() const { return m_input_names; }
   const OutputNames& outputNames() const { return m_output_names; }
@@ -105,6 +112,12 @@ template <class InputType, class OutputType> class Algorithm : public AlgorithmH
   virtual void init() {}
   virtual void process(const Input&, const Output&) const {}
 
+  // Private copy constructor as a valid copy can only be obtained through the clone method
+  Algorithm(const Algorithm& rhs)
+      : AlgorithmHandle(rhs)
+      , m_input_names{rhs.m_input_names}
+      , m_output_names{rhs.m_output_names} {}
+
   const InputNames m_input_names;
   const OutputNames m_output_names;
 };
@@ -197,4 +210,3 @@ template <class T> constexpr bool is_input_v  = detail::is_input<T>::value;
 template <class T> constexpr bool is_output_v = detail::is_output<T>::value;
 
 } // namespace algorithms
-

external/algorithms/core/include/algorithms/property.h

@@ -41,13 +41,20 @@ class PropertyMixin {
   class PropertyHandle;
   using PropertyMap = std::map<std::string_view, PropertyHandle&>;
 
+  // Copy constructor for the PropertyMixin forces new auto-registration of properties in
+  // copied object. The class using this mixin is responsible for copying over the Property
+  // contents. This is done instead of using the default copy constructor as depending on
+  // default copy behavior in the class using this mixin works for Properties but breaks in other
+  // cases (e.g. Resources where the pointer needs to be relocated)
+  PropertyMixin(const PropertyMixin& rhs) : m_props() { m_props.reserve(rhs.m_props); }
+
   template <typename T> void setProperty(std::string_view name, T&& value) {
     m_props.at(name).set(static_cast<detail::upcast_type_t<T>>(value));
   }
   template <typename T> T getProperty(std::string_view name) const {
     return std::get<T>(m_props.at(name).get());
   }
-  const PropertyMap& getProperties() const { return m_props; }
+  PropertyMap& getProperties const PropertyMap& getProperties() const { return m_props; }
   bool hasProperty(std::string_view name) const {
     return m_props.count(name) && m_props.at(name).hasValue();
   }
@@ -114,8 +121,8 @@ class PropertyMixin {
       set(static_cast<impl_type>(v));
     }
 
-    Property()                = delete;
-    Property(const Property&) = default;
+    Property()                           = delete;
+    Property(const Property&)            = default;
     Property& operator=(const Property&) = default;
 
     // Only settable by explicitly calling the ::set() member function
@@ -142,10 +149,10 @@ class PropertyMixin {
     template <typename U> bool operator>=(const U& rhs) const { return m_value >= rhs; }
     template <typename U> bool operator<(const U& rhs) const { return m_value < rhs; }
     template <typename U> bool operator<=(const U& rhs) const { return m_value <= rhs; }
-    template <typename U> decltype(auto) operator+(const U& rhs) const { return m_value + rhs; }
-    template <typename U> decltype(auto) operator-(const U& rhs) const { return m_value - rhs; }
-    template <typename U> decltype(auto) operator*(const U& rhs) const { return m_value * rhs; }
-    template <typename U> decltype(auto) operator/(const U& rhs) const { return m_value / rhs; }
+    template <typename U> decltype(auto) operator+(const U & rhs) const { return m_value + rhs; }
+    template <typename U> decltype(auto) operator-(const U & rhs) const { return m_value - rhs; }
+    template <typename U> decltype(auto) operator*(const U & rhs) const { return m_value * rhs; }
+    template <typename U> decltype(auto) operator/(const U & rhs) const { return m_value / rhs; }
 
     // stl collection helpers if needed
     // forced to be templated so we only declare them when used
@@ -157,8 +164,8 @@ class PropertyMixin {
     template <class U = const value_type> decltype(auto) end() const { return value().end(); }
     template <class U = value_type> decltype(auto) begin() { return value().begin(); }
     template <class U = value_type> decltype(auto) end() { return value().end(); }
-    template <class Arg> decltype(auto) operator[](const Arg& arg) const { return value()[arg]; }
-    template <class Arg> decltype(auto) operator[](const Arg& arg) { return value()[arg]; }
+    template <class Arg> decltype(auto) operator[](const Arg & arg) const { return value()[arg]; }
+    template <class Arg> decltype(auto) operator[](const Arg & arg) { return value()[arg]; }
     template <class U = const value_type>
     decltype(auto) find(const typename U::key_type& key) const {
       return value().find(key);

external/algorithms/core/include/algorithms/resource.h

@@ -41,7 +41,7 @@ template <class SvcType> class SvcResource : public NameMixin {
 };
 
 // Mixin for Resource handling at the algorithm (or service) level (mirroring the
-// PropertyMixin)
+// PropertyMixin), also provides Context handling.
 class ResourceMixin {
 public:
   class ResourceHandle;
@@ -50,7 +50,9 @@ class ResourceMixin {
 
   // Copy constructor for the ResourceMixin assumes new auto-registration (as
   // pointers need to be relocated to the new instance)
-  ResourceMixin(const ResourceMixin&) : m_resources() {}
+  ResourceMixin(const ResourceMixin& rhs) : m_resources() {
+    m_resources.reserve(rhs.m_resources.size());
+  }
 
   ResourceMixin& operator=(const ResourceMixin& rhs) = delete;