From 088354a96b1b8b8cd02132333b0002131cc9990e Mon Sep 17 00:00:00 2001
From: Dion Moult <dion@thinkmoult.com>
Date: Sun, 19 Mar 2023 22:03:45 +1100
Subject: [PATCH] Fix critical bug where copied representations would be
 inefficiently or incorrectly replicated.

---
 .../ifcopenshell/util/element.py              | 24 ++++++++++++++++---
 .../test/util/test_element.py                 | 16 +++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/ifcopenshell-python/ifcopenshell/util/element.py b/src/ifcopenshell-python/ifcopenshell/util/element.py
index ca75d91aeb..59f5a7c7d8 100644
--- a/src/ifcopenshell-python/ifcopenshell/util/element.py
+++ b/src/ifcopenshell-python/ifcopenshell/util/element.py
@@ -808,7 +808,7 @@ def copy(ifc_file, element):
     return new
 
 
-def copy_deep(ifc_file, element, exclude=None, exclude_callback=None):
+def copy_deep(ifc_file, element, exclude=None, exclude_callback=None, copied_entities=None):
     """
     Recursively copy an element and all of its directly related subelements.
 
@@ -825,10 +825,22 @@ def copy_deep(ifc_file, element, exclude=None, exclude_callback=None):
     :param exclude_callback: A callback to determine whether or not to exclude
         an entity or not. Returns True to exclude and False to exclude.
     :type exclude_callback: function,optional
+    :param copied_entities: A dictionary of IDs as keys and entities as values
+        to reuse when coming across the same entity twice. This can typically
+        be left as None.
+    :type copied_entities: dict[int:ifcopenshell.entity_instance.entity_instance]
     :return: The newly copied element
     :rtype: ifcopenshell.entity_instance.entity_instance
     """
+    if copied_entities is None:
+        copied_entities = {}
+    else:
+        copied_entity = copied_entities.get(element.id(), None)
+        if copied_entity:
+            return copied_entity
     new = ifc_file.create_entity(element.is_a())
+    if element.id():
+        copied_entities[element.id()] = new
     for i, attribute in enumerate(element):
         if attribute is None:
             continue
@@ -838,7 +850,7 @@ def copy_deep(ifc_file, element, exclude=None, exclude_callback=None):
             elif exclude_callback and exclude_callback(attribute):
                 pass
             else:
-                attribute = copy_deep(ifc_file, attribute, exclude=exclude)
+                attribute = copy_deep(ifc_file, attribute, exclude=exclude, copied_entities=copied_entities)
         elif isinstance(attribute, tuple) and attribute and isinstance(attribute[0], ifcopenshell.entity_instance):
             if exclude and any([attribute[0].is_a(e) for e in exclude]):
                 pass
@@ -847,7 +859,13 @@ def copy_deep(ifc_file, element, exclude=None, exclude_callback=None):
             else:
                 attribute = list(attribute)
                 for j, item in enumerate(attribute):
-                    attribute[j] = copy_deep(ifc_file, item, exclude=exclude, exclude_callback=exclude_callback)
+                    attribute[j] = copy_deep(
+                        ifc_file,
+                        item,
+                        exclude=exclude,
+                        exclude_callback=exclude_callback,
+                        copied_entities=copied_entities,
+                    )
         if new.attribute_name(i) == "GlobalId":
             new[i] = ifcopenshell.guid.new()
         else:
diff --git a/src/ifcopenshell-python/test/util/test_element.py b/src/ifcopenshell-python/test/util/test_element.py
index 9d5a1f1589..798c06349f 100644
--- a/src/ifcopenshell-python/test/util/test_element.py
+++ b/src/ifcopenshell-python/test/util/test_element.py
@@ -707,3 +707,19 @@ class TestCopyDeepIFC4(test.bootstrap.IFC4):
         rel.RelatedObjects = [element]
         rel2 = subject.copy_deep(self.file, rel, exclude_callback=lambda x: x.is_a("IfcWall"))
         assert rel.RelatedObjects == rel2.RelatedObjects
+
+    def test_copying_and_reusing_element_references(self):
+        points = self.file.createIfcCartesianPointList2D()
+        subelement1 = self.file.createIfcIndexedPolyCurve(points)
+        subelement2 = self.file.createIfcIndexedPolyCurve(points)
+        element = self.file.createIfcGeometricCurveSet([subelement1, subelement2])
+        element2 = subject.copy_deep(self.file, element)
+        assert element2.Elements[0].Points.id() == element2.Elements[1].Points.id()
+
+    def test_copying_primitive_entities(self):
+        element = self.file.createIfcIndexedPolyCurve(
+            Segments=(self.file.createIfcLineIndex((1, 2)), self.file.createIfcLineIndex((3, 4)))
+        )
+        element2 = subject.copy_deep(self.file, element)
+        assert element2.Segments[0][0] == (1, 2)
+        assert element2.Segments[1][0] == (3, 4)