Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix catalog inconsistency when creating a dossier from a dossiertemplate. #6952

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/1361.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix catalog inconsistency when creating a dossier from a dossiertemplate. [njohner]
2 changes: 0 additions & 2 deletions opengever/base/monkey/patches/__init__.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -45,7 +44,6 @@
PatchBuilderCreate()()
PatchCASAuthSetLoginTimes()()
PatchCatalogToFilterTrashedDocs()()
PatchCMFCatalogAware()()
PatchCMFCatalogAwareHandlers()()
PatchCMFEditonsHistoryHandlerTool()()
PatchContentRulesHandlerOnLogin()()
Expand Down
73 changes: 35 additions & 38 deletions opengever/base/monkey/patches/cmf_catalog_aware.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't create multiple instances of the monkey patch, instead do

  1. self.catalogpatch = PatchCMFCatalogAware() (and call it separately)


def __exit__(self, exc_type, exc_val, exc_tb):
noLongerProvides(getRequest(), IDisableCatalogIndexing)
PatchCMFCatalogAware().unpatch()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then use the same instance with the thread-safe mapping to unpatch

  1. self.catalogpatch.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?

Expand All @@ -58,38 +49,44 @@ class PatchCMFCatalogAware(MonkeyPatch):
and do it manually at the end of your tasks.
"""

original_indexing_methods = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having a very bad feeling about this class attribute. It will be shared between threads. Please redesign this a bit, so that we use the same instances of PatchCMFCatalogAware for patching/unpatching withing one request/thread.

  1. move creation of the mapping into __init__ or create lazily in __call__


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
Expand Down
26 changes: 26 additions & 0 deletions opengever/base/tests/test_patch_catalog_aware.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
3 changes: 3 additions & 0 deletions opengever/dossier/dossiertemplate/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ def create_dossier_content_from_template(self, container, template):
self.recursive_reindex(container)

def recursive_reindex(self, obj):
# because we have created objects inside the obj, we also
# need to reindex obj, not only its children.
obj.reindexObject()
for child_obj in obj.listFolderContents():
child_obj.reindexObject()

Expand Down