Fix DeactivatedCatalogIndexing context-manager.

DeactivatedCatalogIndexing was not working properly because collective.indexing
was still adding indexing operations to the indexing queue, and the patched
methods were really only used when the queue was processed. This led to issues
as whether an object would be indexed or not depended on when the queue was
processed instead of when the object was added to the queue.
To fix that issue we modify the Monkey patch to always skip indexing when
active and modify the context-manager to patch and unpatch the indexing
methods instead of adding an interface to the request as was done
previously.

changes/1361.bugfix

@@ -0,0 +1 @@
+Fix catalog inconsistency when creating a dossier from a dossiertemplate. [njohner]

opengever/base/monkey/patches/__init__.py

@@ -1,5 +1,4 @@
 from .action_info import PatchActionInfo
-from .cmf_catalog_aware import PatchCMFCatalogAware
 from .cmf_catalog_aware import PatchCMFCatalogAwareHandlers
 from .default_values import PatchBuilderCreate
 from .default_values import PatchDeserializeFromJson
@@ -45,7 +44,6 @@
 PatchBuilderCreate()()
 PatchCASAuthSetLoginTimes()()
 PatchCatalogToFilterTrashedDocs()()
-PatchCMFCatalogAware()()
 PatchCMFCatalogAwareHandlers()()
 PatchCMFEditonsHistoryHandlerTool()()
 PatchContentRulesHandlerOnLogin()()

opengever/base/monkey/patches/cmf_catalog_aware.py

@@ -4,47 +4,38 @@
 from plone import api
 from Products.CMFCore.utils import _getAuthenticatedUser
 from zope.globalrequest import getRequest
-from zope.interface import alsoProvides
-from zope.interface import Interface
-from zope.interface import noLongerProvides
-from zope.i18n import translate
-
-
-class IDisableCatalogIndexing(Interface):
-    """Marker-interface to disable the catalog
-    indexing functions.
-
-    If this interface is provided by the request, all the
-    catalog-index-methods will be disabled.
-
-    Use the DeactivatedCatalogIndexing contextmanager to
-    get in use of this functionality.
-    """
 
 
 class DeactivatedCatalogIndexing(object):
     """Contextmanager: Deactivates catalog-indexing
     """
     def __enter__(self):
-        alsoProvides(getRequest(), IDisableCatalogIndexing)
+        PatchCMFCatalogAware()()
 
     def __exit__(self, exc_type, exc_val, exc_tb):
-        noLongerProvides(getRequest(), IDisableCatalogIndexing)
+        PatchCMFCatalogAware().unpatch()
+
+
+class CatalogAlreadyPatched(Exception):
+    """Will be raised if we try to patch the catalog indexing methods
+    more than once.
+    """
 
 
 class PatchCMFCatalogAware(MonkeyPatch):
     """Patch the Products.CMFCore.CMFCatalogAware indexObject, reindexObject
     and unindexObject methods.
 
-    This patch is deactivated by default and can be activated through
-    the DeactivatedCatalogIndexing context manager:
+    This patch is deactivated is not applied by default and can be activated
+    through the DeactivatedCatalogIndexing context manager:
 
     >>> with DeactivatedCatalogIndexing():
     ...     object.reindexObject  # Does nothing
     ...     object.unindexObject  # Does nothing
     ...     object.indexObject  # Does nothing
 
-    If the patch is activated, it skips the catalog index-methods.
+    If the patch is activated, it skips the catalog index-methods. The patch
+    gets removed when exiting the context manager.
 
     What's the motivation behind this patch?
 
@@ -58,38 +49,44 @@ class PatchCMFCatalogAware(MonkeyPatch):
     and do it manually at the end of your tasks.
     """
 
+    original_indexing_methods = {}
+
     def __call__(self):
 
-        def _is_indexing_disabled():
-            return IDisableCatalogIndexing.providedBy(getRequest())
+        if self.is_already_applied():
+            raise CatalogAlreadyPatched()
 
         def indexObject(self):
-            if _is_indexing_disabled():
-                # do nothing if indexing is disabled
-                return
-            return original_indexObject(self)
+            return
 
         def unindexObject(self):
-            if _is_indexing_disabled():
-                # do nothing if indexing is disabled
-                return
-            return original_unindexObject(self)
+            return
 
         def reindexObject(self, idxs=[]):
-            if _is_indexing_disabled():
-                # do nothing if indexing is disabled
-                return
-            return original_reindexObject(self, idxs)
+            return
 
         from Products.CMFCore.CMFCatalogAware import CMFCatalogAware
         locals()['__patch_refs__'] = False
-        original_indexObject = CMFCatalogAware.indexObject
-        original_unindexObject = CMFCatalogAware.unindexObject
-        original_reindexObject = CMFCatalogAware.reindexObject
+        self.original_indexing_methods['indexObject'] = CMFCatalogAware.indexObject
+        self.original_indexing_methods['unindexObject'] = CMFCatalogAware.unindexObject
+        self.original_indexing_methods['reindexObject'] = CMFCatalogAware.reindexObject
         self.patch_refs(CMFCatalogAware, 'indexObject', indexObject)
         self.patch_refs(CMFCatalogAware, 'unindexObject', unindexObject)
         self.patch_refs(CMFCatalogAware, 'reindexObject', reindexObject)
 
+    def is_already_applied(self):
+        return bool(self.original_indexing_methods)
+
+    def unpatch(self):
+        from Products.CMFCore.CMFCatalogAware import CMFCatalogAware
+        locals()['__patch_refs__'] = False
+        self.patch_refs(CMFCatalogAware, 'indexObject',
+                        self.original_indexing_methods.pop('indexObject'))
+        self.patch_refs(CMFCatalogAware, 'unindexObject',
+                        self.original_indexing_methods.pop('unindexObject'))
+        self.patch_refs(CMFCatalogAware, 'reindexObject',
+                        self.original_indexing_methods.pop('reindexObject'))
+
 
 class PatchCMFCatalogAwareHandlers(MonkeyPatch):
     """Customize `handleDynamicTypeCopiedEvent` to use RoleAssignmentManager

opengever/base/tests/test_patch_catalog_aware.py

@@ -1,4 +1,6 @@
+from opengever.base.monkey.patches.cmf_catalog_aware import CatalogAlreadyPatched
 from opengever.base.monkey.patches.cmf_catalog_aware import DeactivatedCatalogIndexing
+from opengever.base.monkey.patches.cmf_catalog_aware import PatchCMFCatalogAware
 from opengever.testing import IntegrationTestCase
 from opengever.testing import obj2brain
 from plone import api
@@ -55,3 +57,27 @@ def test_do_not_indexObject_in_context_manager(self):
             self.dossier.indexObject()
 
         self.assertEqual(0, len(self.catalog(UID=self.dossier.UID())))
+
+    def test_cannot_be_applied_twice(self):
+        with DeactivatedCatalogIndexing():
+            with self.assertRaises(CatalogAlreadyPatched):
+                with DeactivatedCatalogIndexing():
+                    pass
+
+    def test_patch_gets_removed_when_exiting_context_manager(self):
+        self.dossier.title = "Foo Bar"
+        self.assertFalse(PatchCMFCatalogAware().is_already_applied())
+
+        with DeactivatedCatalogIndexing():
+            self.assertTrue(PatchCMFCatalogAware().is_already_applied())
+            self.dossier.reindexObject()
+
+        brain = obj2brain(self.dossier)
+        self.assertEqual(
+            'Vertr\xc3\xa4ge mit der kantonalen Finanzverwaltung',
+            brain.Title)
+
+        self.assertFalse(PatchCMFCatalogAware().is_already_applied())
+        self.dossier.reindexObject()
+        brain = obj2brain(self.dossier)
+        self.assertEqual('Foo Bar', brain.Title)