Make sure automatic migration of core tools does not modify ZODB when if fails

product/ERP5Type/dynamic/portal_type_class.py

@@ -30,6 +30,7 @@
 import sys
 import os
 import inspect
+import transaction
 from types import ModuleType
 
 from Products.ERP5Type.dynamic.dynamic_module import registerDynamicModule
@@ -42,7 +43,7 @@ from Products.ERP5Type.dynamic.accessor_holder import AccessorHolderModuleType,
     createAllAccessorHolderList
 from Products.ERP5Type.TransactionalVariable import TransactionalResource
 
-from zLOG import LOG, ERROR, INFO, WARNING
+from zLOG import LOG, ERROR, INFO, WARNING, PANIC
 
 def _importClass(classpath):
   try:
@@ -346,32 +347,40 @@ def synchronizeDynamicModules(context, force=False):
       migrate = False
       from Products.ERP5Type.Tool.PropertySheetTool import PropertySheetTool
       from Products.ERP5Type.Tool.TypesTool import TypesTool
-      for tool_class in TypesTool, PropertySheetTool:
-        # if the instance has no property sheet tool, or incomplete
-        # property sheets, we need to import some data to bootstrap
-        # (only likely to happen on the first run ever)
-        tool_id = tool_class.id
-        tool = getattr(portal, tool_id, None)
-        if tool is None:
-          tool = tool_class()
-          portal._setObject(tool_id, tool, set_owner=False, suppress_events=True)
-          tool = getattr(portal, tool_id)
-        elif tool._isBootstrapRequired():
-          migrate = True
+      try:
+        for tool_class in TypesTool, PropertySheetTool:
+          # if the instance has no property sheet tool, or incomplete
+          # property sheets, we need to import some data to bootstrap
+          # (only likely to happen on the first run ever)
+          tool_id = tool_class.id
+          tool = getattr(portal, tool_id, None)
+          if tool is None:
+            tool = tool_class()
+            portal._setObject(tool_id, tool, set_owner=False,
+                              suppress_events=True)
+            tool = getattr(portal, tool_id)
+          elif tool._isBootstrapRequired():
+            migrate = True
+          else:
+            continue
+          tool._bootstrap()
+          tool.__class__ = getattr(erp5.portal_type, tool.portal_type)
+
+        if migrate:
+          portal.migrateToPortalTypeClass()
+          portal.portal_skins.changeSkin(None)
+          TransactionalResource(tpc_finish=lambda txn:
+              _bootstrapped.add(portal.id))
+          LOG('ERP5Site', INFO, 'Transition successful, please update your'
+              ' business templates')
         else:
-          continue
-        tool._bootstrap()
-        tool.__class__ = getattr(erp5.portal_type, tool.portal_type)
-
-      if migrate:
-        portal.migrateToPortalTypeClass()
-        portal.portal_skins.changeSkin(None)
-        TransactionalResource(tpc_finish=lambda txn:
-            _bootstrapped.add(portal.id))
-        LOG('ERP5Site', INFO, 'Transition successful, please update your'
-            ' business templates')
-      else:
-        _bootstrapped.add(portal.id)
+          _bootstrapped.add(portal.id)
+      except:
+        # Required because the exception may be silently dropped by the caller.
+        transaction.doom()
+        LOG('ERP5Site', PANIC, "Automatic migration of type and"
+            " property sheet tool failed", error=sys.exc_info())
+        raise
 
     LOG("ERP5Type.dynamic", 0, "Resetting dynamic classes")
     try: