From e86c6071e3b2eb0410b947a58fe1dfed30b0f32c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Fri, 1 May 2020 07:41:46 +0200
Subject: [PATCH] TrashTool: fail if backup object container already exist

This is not supposed to happen and can hide errors.
---
 product/ERP5/Tool/TrashTool.py | 103 ++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 53 deletions(-)

diff --git a/product/ERP5/Tool/TrashTool.py b/product/ERP5/Tool/TrashTool.py
index f9ab0f04da..88d0af66f9 100644
--- a/product/ERP5/Tool/TrashTool.py
+++ b/product/ERP5/Tool/TrashTool.py
@@ -80,59 +80,56 @@ class TrashTool(BaseTool):
       # backup the object
       # here we choose export/import to copy because cut/paste
       # do too many things and check for what we want to do
-      obj = None
-      if object_id not in backup_object_container.objectIds():
-        # export object
-        object_path = container_path + [object_id]
-        obj = self.unrestrictedTraverse(object_path, None)
-        if obj is not None:
-          connection = obj._p_jar
-          o = obj
-          while connection is None:
-            o = o.aq_parent
-            connection=o._p_jar
-          if obj._p_oid is None:
-            LOG("Trash Tool backupObject", WARNING,
-                "Trying to backup uncommitted object %s" % object_path)
-            return {}
-          if isinstance(obj, Broken):
-            LOG("Trash Tool backupObject", WARNING,
-                "Can't backup broken object %s" % object_path)
-            klass = obj.__class__
-            if klass.__module__[:27] in ('Products.ERP5Type.Document.',
-                                         'erp5.portal_type'):
-              # meta_type is required so that a broken object
-              # can be removed properly from a BTreeFolder2
-              # (unfortunately, we can only guess it)
-              klass.meta_type = 'ERP5' + re.subn('(?=[A-Z])', ' ',
-                                                 klass.__name__)[0]
-            return {}
-          copy = connection.exportFile(obj._p_oid)
-          # import object in trash
-          connection = backup_object_container._p_jar
-          o = backup_object_container
-          while connection is None:
-            o = o.aq_parent
-            connection=o._p_jar
-          copy.seek(0)
-          try:
-            backup = connection.importFile(copy)
-            backup.isIndexable = ConstantGetter('isIndexable', value=False)
-            # the isIndexable setting above avoids the recursion of
-            # manage_afterAdd on
-            # Products.ERP5Type.CopySupport.CopySupport.manage_afterAdd()
-            # but not on event subscribers, so we need to suppress_events,
-            # otherwise subobjects will be reindexed
-            backup_object_container._setObject(object_id, backup,
-                                               suppress_events=True)
-          except (AttributeError, ImportError):
-            # XXX we can go here due to formulator because attribute
-            # field_added doesn't not exists on parent if it is a Trash
-            # Folder and not a Form, or a module for the old object is
-            # already removed, and we cannot backup the object
-            LOG("Trash Tool backupObject", WARNING,
-                "Can't backup object %s" % object_path)
-            return {}
+      object_path = container_path + [object_id]
+      obj = self.unrestrictedTraverse(object_path, None)
+      if obj is not None:
+        connection = obj._p_jar
+        o = obj
+        while connection is None:
+          o = o.aq_parent
+          connection=o._p_jar
+        if obj._p_oid is None:
+          LOG("Trash Tool backupObject", WARNING,
+              "Trying to backup uncommitted object %s" % object_path)
+          return {}
+        if isinstance(obj, Broken):
+          LOG("Trash Tool backupObject", WARNING,
+              "Can't backup broken object %s" % object_path)
+          klass = obj.__class__
+          if klass.__module__[:27] in ('Products.ERP5Type.Document.',
+                                        'erp5.portal_type'):
+            # meta_type is required so that a broken object
+            # can be removed properly from a BTreeFolder2
+            # (unfortunately, we can only guess it)
+            klass.meta_type = 'ERP5' + re.subn('(?=[A-Z])', ' ',
+                                                klass.__name__)[0]
+          return {}
+        copy = connection.exportFile(obj._p_oid)
+        # import object in trash
+        connection = backup_object_container._p_jar
+        o = backup_object_container
+        while connection is None:
+          o = o.aq_parent
+          connection=o._p_jar
+        copy.seek(0)
+        try:
+          backup = connection.importFile(copy)
+          backup.isIndexable = ConstantGetter('isIndexable', value=False)
+          # the isIndexable setting above avoids the recursion of
+          # manage_afterAdd on
+          # Products.ERP5Type.CopySupport.CopySupport.manage_afterAdd()
+          # but not on event subscribers, so we need to suppress_events,
+          # otherwise subobjects will be reindexed
+          backup_object_container._setObject(object_id, backup,
+                                              suppress_events=True)
+        except (AttributeError, ImportError):
+          # XXX we can go here due to formulator because attribute
+          # field_added doesn't not exists on parent if it is a Trash
+          # Folder and not a Form, or a module for the old object is
+          # already removed, and we cannot backup the object
+          LOG("Trash Tool backupObject", WARNING,
+              "Can't backup object %s" % object_path)
+          return {}
 
     keep_sub = kw.get('keep_subobjects', 0)
     subobjects_dict = {}
-- 
2.30.9