BusinessTemplate: Fix a unindex/reindex catalog problem when upgraiding broken objects.

How it happened while upgrading PathTemplateItem:
  1. Tries to remove the existing broken object and succeeds removing
  2. However it is a broken object so self.unindexCatalog() is
     never called and the old catalog is remaining
  3. Add new object of PathTemplateItem and tries to reindex the object
  4. Since old catalog is remaining, the catalog uid and object uid is
     different so catalogObjectList fails with FATAL error.

The test is tests/BusinessTemplate.py#test_UpgradeBrokenObject.

product/ERP5/Document/BusinessTemplate.py

@@ -85,6 +85,7 @@ import posixpath
 import transaction
 
 import threading
+from ZODB.broken import Broken
 
 CACHE_DATABASE_PATH = None
 try:
@@ -917,6 +918,70 @@ class ObjectTemplateItem(BaseTemplateItem):
     keys.sort()
     return keys
 
+  def unindexBrokenObject(self, new_obj, item_path):
+    """
+      Unindex broken objects.
+
+      Corresponding catalog record is not unindexed even after a broken object
+      is removed, since the broken object does not implement 'CopySupport'.
+      This situation triggers a FATAL problem on SQLCatalog.catalogObjectList
+      when upgrading a broken path by ObjectTemplateItem with BusinessTemplate.
+      We often get this problem when we are upgrading a quite old ERP5 site
+      to new one, as several old classes may be already removed/replaced 
+      in the file system, thus several objects tend to be broken.
+
+      Keyword arguments:
+      new_obj -- the object in the BusinessTemplate
+      item_path -- the path specified by the ObjectTemplateItem
+    """
+    def flushActivity(obj, invoke=0, **kw):
+      try:
+        activity_tool = self.getPortalObject().portal_activities
+      except AttributeError:
+        return # Do nothing if no portal_activities
+      # flush all activities related to this object
+      activity_tool.flush(obj, invoke=invoke, **kw)
+
+    class fakeobject:
+       def __init__(self, path):
+         self._physical_path = tuple(path.split('/')) 
+       def getPhysicalPath(self):
+         return self._physical_path
+ 
+    def recursiveUnindex(catalog, item_path, root_document_path):
+      # search the object + sub-objects
+      result = catalog(relative_url=(item_path, item_path + '/%'))
+      for x in result:
+        uid = x.uid
+        path = x.path
+        unindex(root_document_path, path, uid)
+
+    def unindex(root_document_path, path, uid):
+      LOG('Products.ERP5.Document.BusinessTemplate', WARNING,
+          'Unindex Broken object at %r.' % (path,))
+      # Set the path as deleted without lock
+      catalog.beforeUnindexObject(None,path=path,uid=uid)
+      # Then start activity in order to remove lines in catalog,
+      # sql wich generate locks
+      catalog.activate(activity='SQLQueue',
+                       tag='%s' % uid,
+                       group_method_id='portal_catalog/uncatalogObjectList',
+                       serialization_tag=root_document_path
+                       ).unindexObject(uid=uid)
+
+    # check isIndexable with new one, because the old one is broken
+    if new_obj.isIndexable():
+      portal = self.getPortalObject()
+      try:
+        catalog = portal.portal_catalog
+      except AttributeError:
+        pass
+      else:
+        # given item_path is a relative_url in reality
+        root_path = "/".join(item_path.split('/')[:2])
+        root_document_path = '/%s/%s' % (portal.getId(), root_path)
+        recursiveUnindex(catalog, item_path, root_document_path)
+
   def install(self, context, trashbin, **kw):
     self.beforeInstall()
     update_dict = kw.get('object_to_update')
@@ -1015,6 +1080,10 @@ class ObjectTemplateItem(BaseTemplateItem):
               portal_type_dict['workflow_chain'] = \
                 getChainByType(context)[1].get('chain_' + object_id, '')
             container.manage_delObjects([object_id])
+            # unindex here when it is a broken object
+            if isinstance(old_obj, Broken):
+              new_obj = self._objects[path]
+              self.unindexBrokenObject(new_obj, path)
 
           # install object
           obj = self._objects[path]

product/ERP5/tests/testBusinessTemplate.py

@@ -49,6 +49,7 @@ import random
 import string
 import tempfile
 import glob
+import sys
 
 WORKFLOW_TYPE = 'erp5_workflow'
 
@@ -57,6 +58,14 @@ from Products.PortalTransforms.Transform import Transform
 Transform_tr_init = Transform._tr_init
 Transform_manage_beforeDelete = Transform.manage_beforeDelete
 
+from Products.ERP5.Document.Organisation import Organisation
+from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
+from ZODB.broken import Broken
+
+class MockBrokenOrganisation(Organisation, Broken):
+  meta_type = 'ERP5 Mock Broken Organisation'
+  portal_type = 'Mock Broken Organisation'
+
 class BusinessTemplateMixin(ERP5TypeTestCase, LogInterceptor):
   def getBusinessTemplateList(self):
     return ('erp5_base',
@@ -7604,6 +7613,181 @@ class TestTestTemplateItem(TestDocumentTemplateItem):
     sequence_list.addSequenceString(sequence_string)
     sequence_list.play(self)
 
+  def stepCreateOrganisation(self, sequence=None, **kw):
+    """
+    Organisation
+    """
+    organisation_module = self.portal.organisation_module
+    id_list = []
+    for i in range(self.organisation_amount):
+      organisation = organisation_module.newContent(
+                       portal_type = 'Organisation')
+      self.failUnless(organisation is not None)
+      organisation.setTitle('organisation %d' % (i + 1))
+      for j in range(self.email_amount):
+        organisation.newContent(id='email%d' % (j+1),
+                                title='my email%d' % (j+1),
+                                portal_type='Email')
+      id_list.append(organisation.getId())
+    self.assertNotEquals(id_list, [])
+    sequence.edit(organisation_id_list=id_list)
+
+  def stepModifyOrganisation(self, sequence=None, **kw):
+    """ Modify Organisation """
+    organisation_id_list = sequence.get('organisation_id_list', [])
+    self.assertNotEquals(organisation_id_list, [])
+    for organisation_id in organisation_id_list:
+      organisation_module = self.portal.organisation_module
+      organisation = organisation_module[organisation_id]
+      organisation.setTitle('[modified] ' + organisation.getTitle())
+      for j in range(self.email_amount):
+        email = organisation['email%d' % (j+1)]
+        email.setTitle('[modified] ' + email.getTitle())
+
+  def stepRewriteWithBrokenOrganisation(self, sequence=None, **kw):
+    """
+      Rewrite the organisation with a broken object.
+
+      [Note]: In fact, it is a *mock* broken object. It behave like a broken
+      object but it's not broken. To use *real* broken object is better.
+      However it is rather difficult to create a real broken
+      object in ZODB without restart Zope. Even if removing
+      sys.modules['Products.ERP5.Document'].MockBrokenOrganisation, and 
+      retrieve it with a new connection, it does not become a broken object. 
+      Probablly there is remaing the object-cache somewhere in Zope.
+    """
+    setattr(sys.modules['Products.ERP5.Document'], 
+           'MockBrokenOrganisation', MockBrokenOrganisation)
+
+    from Products.ERP5Type.Utils import registerDocumentClass
+    registerDocumentClass('Products.ERP5.Document',
+                          'MockBrokenOrganisation')
+    self.commit()
+    pt = self.getTypeTool()
+    # create module object portal type
+    pt.newContent('Mock Broken Organisation', 'Base Type',
+                  type_class='MockBrokenOrganisation')
+    pt['Organisation Module'].edit(
+      type_allowed_content_type_list=('Organisation',
+                                      'Mock Broken Organisation',))
+
+    self.commit()
+    self.portal.organisation_module.manage_delObjects(['1'])
+    broken = self.portal.organisation_module.newContent(
+                   portal_type='Mock Broken Organisation', id='1')
+    self.commit()
+    self.tic() # triger undex/index the document
+
+    # set unindexable so that it will act as Broken object
+    self.portal.organisation_module['1'].isIndexable = \
+        ConstantGetter('isIndexable', value=False)
+    self.commit()
+ 
+    # to reproduce this problem we need to clear portal_caches
+    self.portal.portal_caches.clearAllCache()
+
+  def stepCheckOrganisationModified(self, sequence=None, **kw):
+    organisation_id_list = sequence.get('organisation_id_list', [])
+    self.assertNotEquals(organisation_id_list, [])
+    for organisation_id in organisation_id_list:
+      organisation_module = self.portal.organisation_module
+      organisation = organisation_module[organisation_id]
+      self.assertTrue(organisation.getTitle().startswith('[modified]'))
+      for j in range(self.email_amount):
+        email = organisation['email%d' % (j+1)]
+        self.assertTrue(email.getTitle().startswith('[modified]'))
+ 
+  def stepAddOrganisationToBusinessTemplate(self, sequence=None, **kw):
+    bt = sequence.get('current_bt', None)
+    self.failUnless(bt is not None) 
+    if bt.getTemplatePathList():
+      path_list = bt.getTemplatePathList()[:]
+      path_list = path_list + ('organisation_module/**',)
+      bt.edit(template_path_list=path_list)
+    else:
+      bt.edit(template_path_list=['organisation_module/**'])
+
+  def stepRevertOrganisation(self, sequence=None, **kw):
+    organisation_id_list = sequence.get('organisation_id_list', [])
+    self.assertNotEquals(organisation_id_list, [])
+    for organisation_id in organisation_id_list:
+      organisation_module = self.portal.organisation_module
+      organisation = organisation_module[organisation_id]
+      organisation.setTitle(organisation.getTitle().replace('[modified] ', ''))
+      for j in range(self.email_amount):
+        email = organisation['email%d' % (j+1)]
+        email.setTitle(email.getTitle().replace('[modified] ', ''))
+  
+  def stepRemoveOrganisation(self, sequence=None, **kw):
+    id_list = sequence.get('organisation_id_list', None)
+    self.assertNotEquals(id_list, [])
+    organisation_id_list = id_list[:]
+    organisation_module = self.portal.organisation_module
+    organisation_module.manage_delObjects(organisation_id_list)
+    self.assertNotEquals(id_list, [])
+
+  def stepCheckOrganisationRestored(self, sequence=None, **kw):
+    organisation_id_list = sequence.get('organisation_id_list', [])
+    self.assertNotEquals(organisation_id_list, [])
+    for organisation_id in organisation_id_list:
+      organisation_module = self.portal.organisation_module
+      organisation = organisation_module[organisation_id]
+      self.assertTrue(organisation.getTitle().startswith('organisation'))
+      for j in range(self.email_amount):
+        email = organisation['email%d' % (j+1)]
+        self.assertTrue(email.getTitle().startswith('my email'))
+
+  def test_UpgradeBrokenObject(self):
+    """
+      Test a case that there is an broken object and upgrade the path.
+
+      [Test summary]
+      1. create organisation_module/1
+      2. set organisation_module/1 as a broken object (in fact it's a mock)
+      3. upgrade organisation_module/1 by the PathTemplateItem
+    """
+    self.portal.portal_activities.manage_enableActivityTracking()
+    self.organisation_amount = 2 
+    self.email_amount = 2 
+    sequence_list = SequenceList()
+    sequence_string = '\
+                       CreateOrganisation \
+                       CreateNewBusinessTemplate \
+                       UseExportBusinessTemplate \
+                       AddOrganisationToBusinessTemplate \
+                       Tic \
+                       BuildBusinessTemplate \
+                       SaveBusinessTemplate \
+                       Tic \
+                       RemoveOrganisation \
+                       RemoveBusinessTemplate \
+                       RemoveAllTrashBins \
+                       Tic \
+                       ImportBusinessTemplate \
+                       UseImportBusinessTemplate \
+                       InstallBusinessTemplate \
+                       Tic \
+		       CheckOrganisationRestored\
+                       ModifyOrganisation \
+                       CreateNewBusinessTemplate \
+		       UseExportBusinessTemplate \
+	               AddOrganisationToBusinessTemplate \
+		       BuildBusinessTemplate \
+		       SaveBusinessTemplate \
+		       RevertOrganisation \
+                       Tic \
+ 		       CheckOrganisationRestored \
+                       RewriteWithBrokenOrganisation \
+		       Tic \
+                       ImportBusinessTemplate \
+                       UseImportBusinessTemplate \
+		       InstallBusinessTemplate \
+                       Tic \
+                       CheckOrganisationModified \
+                       '
+    sequence_list.addSequenceString(sequence_string)
+    sequence_list.play(self)
+
 # XXX-arnau: Skip until ZODB Constraints have been implemented (not
 # expectedFailure because following tests would fail after the ZODB Component
 # has been created)