Experimental undo and redo Python prototype (#1539)

* Experimental undo and redo Python prototype

* Simplify history, use transaction jargon, use walk for more robust serialisation

* Black file, set history size, add file reference to entity_instance constructor

* Ensure that files are always passed when entities are wrapped

* Add support for undo/redo of all project module operations

* Minor fix

* Blender to IFC mappings are now managed by Blender RNA, so they don't break on undo operations. See #1475.

* Implement undo for all attribute operations. See #1475.

* Update documentation for installation of add-on

* Revert Blender RNA approach for Blender-IFC mappings, because it didn't scale, but still fix the undo/redo object memory corruption with new "reload_linked_elements" method.

src/ifcopenshell-python/ifcopenshell/entity_instance.py

@@ -48,20 +48,21 @@ class entity_instance(object):
         >>> #423=IfcProductDefinitionShape($,$,(#409,#421))
     """
 
-    def __init__(self, e):
+    def __init__(self, e, file):
         if isinstance(e, tuple):
             e = ifcopenshell_wrapper.new_IfcBaseClass(*e)
         super(entity_instance, self).__setattr__("wrapped_data", e)
+        self.wrapped_data.file = file
 
     def __getattr__(self, name):
         INVALID, FORWARD, INVERSE = range(3)
         attr_cat = self.wrapped_data.get_attribute_category(name)
         if attr_cat == FORWARD:
             return entity_instance.wrap_value(
-                self.wrapped_data.get_argument(self.wrapped_data.get_argument_index(name))
+                self.wrapped_data.get_argument(self.wrapped_data.get_argument_index(name)), self.wrapped_data.file
             )
         elif attr_cat == INVERSE:
-            return entity_instance.wrap_value(self.wrapped_data.get_inverse(name))
+            return entity_instance.wrap_value(self.wrapped_data.get_inverse(name), self.wrapped_data.file)
         else:
             raise AttributeError(
                 "entity instance of type '%s' has no attribute '%s'" % (self.wrapped_data.is_a(), name)
@@ -77,9 +78,9 @@ class entity_instance(object):
             return value
 
     @staticmethod
-    def wrap_value(v):
+    def wrap_value(v, file):
         def wrap(e):
-            return entity_instance(e)
+            return entity_instance(e, file)
 
         def is_instance(e):
             return isinstance(e, ifcopenshell_wrapper.entity_instance)
@@ -116,12 +117,15 @@ class entity_instance(object):
         return self.wrapped_data.get_argument_name(attr_idx)
 
     def __setattr__(self, key, value):
-        self[self.wrapped_data.get_argument_index(key)] = value
+        index = self.wrapped_data.get_argument_index(key)
+        if self.wrapped_data.file.transaction:
+            self.wrapped_data.file.transaction.store_edit(self, index, value)
+        self[index] = value
 
     def __getitem__(self, key):
         if key < 0 or key >= len(self):
             raise IndexError("Attribute index {} out of range for instance of type {}".format(key, self.is_a()))
-        return entity_instance.wrap_value(self.wrapped_data.get_argument(key))
+        return entity_instance.wrap_value(self.wrapped_data.get_argument(key), self.wrapped_data.file)
 
     def __setitem__(self, idx, value):
         attr_type = real_attr_type = self.attribute_type(idx).title().replace(" ", "")
@@ -271,7 +275,7 @@ class entity_instance(object):
         return return_type(_())
 
     __dict__ = property(get_info)
-    
+
     def get_info_2(self, include_identifier=True, recursive=False, return_type=dict, ignore=()):
         assert include_identifier
         assert recursive

src/ifcopenshell-python/ifcopenshell/file.py

@@ -35,6 +35,105 @@ except NameError:
     basestring = (str, bytes)
 
 
+class Transaction:
+    def __init__(self, ifc_file):
+        self.file = ifc_file
+        self.operations = []
+
+    def serialise_entity_instance(self, element):
+        info = element.get_info()
+        for key, value in info.items():
+            info[key] = self.serialise_value(element, value)
+        return info
+
+    def serialise_value(self, element, value):
+        return element.walk(lambda v: isinstance(v, entity_instance), lambda v: {"id": v.id()}, value)
+
+    def unserialise_value(self, element, value):
+        return element.walk(lambda v: isinstance(v, dict), lambda v: self.file.by_id(v["id"]), value)
+
+    def store_create(self, element):
+        self.operations.append({"action": "create", "value": self.serialise_entity_instance(element)})
+
+    def store_edit(self, element, index, value):
+        self.operations.append(
+            {
+                "action": "edit",
+                "id": element.id(),
+                "index": index,
+                "old": self.serialise_value(element, element[index]),
+                "new": self.serialise_value(element, value),
+            }
+        )
+
+    def store_delete(self, element):
+        inverses = {}
+        for inverse in self.file.get_inverse(element):
+            inverse_references = []
+            for i, attribute in enumerate(inverse):
+                if attribute == element:
+                    inverse_references.append((i, "single"))
+                elif isinstance(attribute, tuple) and element in attribute:
+                    inverse_references.append((i, "multiple"))
+            inverses[inverse.id()] = inverse_references
+        self.operations.append(
+            {"action": "delete", "inverses": inverses, "value": self.serialise_entity_instance(element)}
+        )
+
+    def rollback(self):
+        for operation in self.operations[::-1]:
+            if operation["action"] == "create":
+                element = self.file.by_id(operation["value"]["id"])
+                if hasattr(element, "GlobalId"):
+                    # hack, otherwise ifcopenshell gets upset
+                    element.GlobalId = "x"
+                self.file.remove(element)
+            elif operation["action"] == "edit":
+                element = self.file.by_id(operation["id"])
+                try:
+                    element[operation["index"]] = self.unserialise_value(element, operation["old"])
+                except:
+                    # Catch discrepancy where IfcOpenShell creates but doesn't allow editing of invalid values
+                    pass
+            elif operation["action"] == "delete":
+                e = self.file.create_entity(operation["value"]["type"], id=operation["value"]["id"])
+                for k, v in operation["value"].items():
+                    try:
+                        setattr(e, k, self.unserialise_value(e, v))
+                    except:
+                        # Catch discrepancy where IfcOpenShell creates but doesn't allow editing of invalid values
+                        pass
+                for inverse_id, data in operation["inverses"].items():
+                    inverse = self.file.by_id(inverse_id)
+                    for index, data_type in data:
+                        if data_type == "single":
+                            inverse[index] = e
+                        elif data_type == "multiple":
+                            if inverse[index] is None:
+                                inverse[index] = e
+                            else:
+                                new = list(inverse[index])
+                                new.append(e)
+                                inverse[index] = new
+
+    def commit(self):
+        for operation in self.operations:
+            if operation["action"] == "create":
+                e = self.file.create_entity(operation["value"]["type"], id=operation["value"]["id"])
+                for k, v in operation["value"].items():
+                    try:
+                        setattr(e, k, self.unserialise_value(e, v))
+                    except:
+                        # Catch discrepancy where IfcOpenShell creates but doesn't allow editing of invalid values
+                        pass
+            elif operation["action"] == "edit":
+                element = self.file.by_id(operation["id"])
+                element[operation["index"]] = self.unserialise_value(element, operation["new"])
+            elif operation["action"] == "delete":
+                element = self.file.by_id(operation["value"]["id"])
+                self.file.remove(element)
+
+
 class file(object):
     """Base class for containing IFC files.
 
@@ -59,6 +158,45 @@ class file(object):
             args = filter(None, [schema])
             args = map(ifcopenshell_wrapper.schema_by_name, args)
             self.wrapped_data = ifcopenshell_wrapper.file(*args)
+        self.history_size = 64
+        self.history = []
+        self.future = []
+        self.transaction = None
+
+    def set_history_size(self, size):
+        self.history_size = size
+        while len(self.history) > self.history_size:
+            self.history.pop(0)
+
+    def begin_transaction(self):
+        self.transaction = Transaction(self)
+
+    def end_transaction(self):
+        if self.transaction:
+            self.history.append(self.transaction)
+            if len(self.history) > self.history_size:
+                self.history.pop(0)
+            self.future = []
+            self.transaction = None
+
+    def discard_transaction(self):
+        if self.transaction:
+            self.transaction.rollback()
+        self.transaction = None
+
+    def undo(self):
+        if not self.history:
+            return
+        transaction = self.history.pop()
+        transaction.rollback()
+        self.future.append(transaction)
+
+    def redo(self):
+        if not self.future:
+            return
+        transaction = self.future.pop()
+        transaction.commit()
+        self.history.append(transaction)
 
     def create_entity(self, type, *args, **kwargs):
         """Create a new IFC entity in the file.
@@ -82,14 +220,17 @@ class file(object):
         """
         eid = -1
         try:
-            eid = kwargs.pop("_id", -1)
-        except: pass
-        e = entity_instance((self.schema, type))
+            eid = kwargs.pop("id", -1)
+        except:
+            pass
+        e = entity_instance((self.schema, type), self)
         self.wrapped_data.add(e.wrapped_data, eid)
         e.wrapped_data.this.disown()
         attrs = list(enumerate(args)) + [(e.wrapped_data.get_argument_index(name), arg) for name, arg in kwargs.items()]
         for idx, arg in attrs:
             e[idx] = arg
+        if self.transaction:
+            self.transaction.store_create(e)
         return e
 
     def __getattr__(self, attr):
@@ -100,9 +241,9 @@ class file(object):
 
     def __getitem__(self, key):
         if isinstance(key, numbers.Integral):
-            return entity_instance(self.wrapped_data.by_id(key))
+            return entity_instance(self.wrapped_data.by_id(key), self)
         elif isinstance(key, basestring):
-            return entity_instance(self.wrapped_data.by_guid(str(key)))
+            return entity_instance(self.wrapped_data.by_guid(str(key)), self)
 
     def by_id(self, id):
         """Return an IFC entity instance filtered by IFC ID.
@@ -129,7 +270,7 @@ class file(object):
 
         If the entity already exists, it is not re-added."""
         inst.wrapped_data.this.disown()
-        return entity_instance(self.wrapped_data.add(inst.wrapped_data, -1 if _id is None else _id))
+        return entity_instance(self.wrapped_data.add(inst.wrapped_data, -1 if _id is None else _id), self)
 
     def by_type(self, type, include_subtypes=True):
         """Return IFC objects filtered by IFC Type and wrapped with the entity_instance class.
@@ -144,8 +285,8 @@ class file(object):
         :rtype: list
         """
         if include_subtypes:
-            return [entity_instance(e) for e in self.wrapped_data.by_type(type)]
-        return [entity_instance(e) for e in self.wrapped_data.by_type_excl_subtypes(type)]
+            return [entity_instance(e, self) for e in self.wrapped_data.by_type(type)]
+        return [entity_instance(e, self) for e in self.wrapped_data.by_type_excl_subtypes(type)]
 
     def traverse(self, inst, max_levels=None):
         """Get a list of all referenced instances for a particular instance including itself
@@ -159,7 +300,7 @@ class file(object):
         """
         if max_levels is None:
             max_levels = -1
-        return [entity_instance(e) for e in self.wrapped_data.traverse(inst.wrapped_data, max_levels)]
+        return [entity_instance(e, self) for e in self.wrapped_data.traverse(inst.wrapped_data, max_levels)]
 
     def get_inverse(self, inst):
         """Return a list of entities that reference this entity
@@ -169,7 +310,7 @@ class file(object):
         :returns: A list of ifcopenshell.entity_instance.entity_instance objects
         :rtype: list
         """
-        return [entity_instance(e) for e in self.wrapped_data.get_inverse(inst.wrapped_data)]
+        return [entity_instance(e, self) for e in self.wrapped_data.get_inverse(inst.wrapped_data)]
 
     def remove(self, inst):
         """Deletes an IFC object in the file.
@@ -182,12 +323,14 @@ class file(object):
         :type inst: ifcopenshell.entity_instance.entity_instance
         :rtype: None
         """
+        if self.transaction:
+            self.transaction.store_delete(inst)
         return self.wrapped_data.remove(inst.wrapped_data)
-        
+
     def batch(self):
         """Low-level mechanism to speed up deletion of large subgraphs"""
         return self.wrapped_data.batch()
-        
+
     def unbatch(self):
         """Low-level mechanism to speed up deletion of large subgraphs"""
         return self.wrapped_data.unbatch()