From 6fc384b48703eab7ffa32740218534777c72734e Mon Sep 17 00:00:00 2001 From: David Glick Date: Fri, 18 Aug 2023 12:23:11 -0700 Subject: [PATCH 1/4] Experiment with optimizing moves --- Products/CMFPlone/CatalogTool.py | 11 ++++ Products/CMFPlone/patches/__init__.py | 2 +- Products/CMFPlone/patches/cmfcatalogaware.py | 53 ++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 Products/CMFPlone/patches/cmfcatalogaware.py diff --git a/Products/CMFPlone/CatalogTool.py b/Products/CMFPlone/CatalogTool.py index af4e31610d..ec8ff5d9f7 100644 --- a/Products/CMFPlone/CatalogTool.py +++ b/Products/CMFPlone/CatalogTool.py @@ -290,6 +290,17 @@ def _listAllowedRolesAndUsers(self, user): result.append("Anonymous") return result + def __url(self, ob): + try: + return aq_base(ob).UID() + except AttributeError: + return "/".join(ob.getPhysicalPath()) + + @security.protected(SearchZCatalog) + def getpath(self, rid): + # Return the path to a cataloged object given a 'data_record_id_' + return self.Indexes["path"]._unindex[rid] + @security.private def indexObject(self, object, idxs=None): # Add object to catalog. diff --git a/Products/CMFPlone/patches/__init__.py b/Products/CMFPlone/patches/__init__.py index e8234dc395..1ad897edc1 100644 --- a/Products/CMFPlone/patches/__init__.py +++ b/Products/CMFPlone/patches/__init__.py @@ -30,6 +30,6 @@ from . import publishing from . import templatecookcheck # Make sure templates aren't re-read in from . import z3c_form - +from . import cmfcatalogaware # production sites diff --git a/Products/CMFPlone/patches/cmfcatalogaware.py b/Products/CMFPlone/patches/cmfcatalogaware.py new file mode 100644 index 0000000000..8280c1231f --- /dev/null +++ b/Products/CMFPlone/patches/cmfcatalogaware.py @@ -0,0 +1,53 @@ +import inspect +from Acquisition import aq_base +from OFS.interfaces import IObjectWillBeMovedEvent +from Products.CMFCore.CMFCatalogAware import handleContentishEvent as orig +from Products.CMFCore.interfaces import ICatalogTool +from zope.container.interfaces import IObjectAddedEvent +from zope.container.interfaces import IObjectMovedEvent +from zope.lifecycleevent.interfaces import IObjectCopiedEvent +from zope.lifecycleevent.interfaces import IObjectCreatedEvent +from zope.component import queryUtility + + +def handleContentishEvent(ob, event): + """ Event subscriber for (IContentish, IObjectEvent) events. + """ + if IObjectAddedEvent.providedBy(event): + ob.notifyWorkflowCreated() + ob.indexObject() + + elif IObjectWillBeMovedEvent.providedBy(event): + if event.oldParent is not None: + catalog = queryUtility(ICatalogTool) + uid = catalog._CatalogTool__url(ob) + if uid.startswith("/") or not event.newParent: + ob.unindexObject() + + elif IObjectMovedEvent.providedBy(event): + if event.newParent is not None: + catalog = queryUtility(ICatalogTool) + uid = catalog._CatalogTool__url(ob) + if uid.startswith("/"): + # path-based catalog key. + # the object was unindexed at the old path + # and needs to be completely re-added + ob.indexObject() + else: + # UID-based catalog key. + # we can update only the indexes that are path-dependent + ob.reindexObject(idxs=["path", "allowedRolesAndUsers", "id", "getId"]) + + elif IObjectCopiedEvent.providedBy(event): + if hasattr(aq_base(ob), 'workflow_history'): + del ob.workflow_history + + elif IObjectCreatedEvent.providedBy(event): + if hasattr(aq_base(ob), 'addCreator'): + ob.addCreator() + + +globals = orig.__globals__ +source = inspect.getsource(handleContentishEvent) +exec(source, globals) +orig.func_code = globals["handleContentishEvent"].__code__ From b5d0a0e0d22602b6d27ca676cf5471d675042988 Mon Sep 17 00:00:00 2001 From: David Glick Date: Fri, 18 Aug 2023 14:33:38 -0700 Subject: [PATCH 2/4] Don't use UID --- Products/CMFPlone/CatalogTool.py | 11 ---- Products/CMFPlone/patches/cmfcatalogaware.py | 58 +++++++++++++++----- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/Products/CMFPlone/CatalogTool.py b/Products/CMFPlone/CatalogTool.py index ec8ff5d9f7..af4e31610d 100644 --- a/Products/CMFPlone/CatalogTool.py +++ b/Products/CMFPlone/CatalogTool.py @@ -290,17 +290,6 @@ def _listAllowedRolesAndUsers(self, user): result.append("Anonymous") return result - def __url(self, ob): - try: - return aq_base(ob).UID() - except AttributeError: - return "/".join(ob.getPhysicalPath()) - - @security.protected(SearchZCatalog) - def getpath(self, rid): - # Return the path to a cataloged object given a 'data_record_id_' - return self.Indexes["path"]._unindex[rid] - @security.private def indexObject(self, object, idxs=None): # Add object to catalog. diff --git a/Products/CMFPlone/patches/cmfcatalogaware.py b/Products/CMFPlone/patches/cmfcatalogaware.py index 8280c1231f..5d53b88584 100644 --- a/Products/CMFPlone/patches/cmfcatalogaware.py +++ b/Products/CMFPlone/patches/cmfcatalogaware.py @@ -8,6 +8,7 @@ from zope.lifecycleevent.interfaces import IObjectCopiedEvent from zope.lifecycleevent.interfaces import IObjectCreatedEvent from zope.component import queryUtility +from zope.component import ComponentLookupError def handleContentishEvent(ob, event): @@ -18,25 +19,54 @@ def handleContentishEvent(ob, event): ob.indexObject() elif IObjectWillBeMovedEvent.providedBy(event): - if event.oldParent is not None: - catalog = queryUtility(ICatalogTool) - uid = catalog._CatalogTool__url(ob) - if uid.startswith("/") or not event.newParent: + # Move/Rename + if event.oldParent is not None and event.newParent is not None: + try: + catalog = queryUtility(ICatalogTool) + except ComponentLookupError: + # Happens when renaming a Plone Site in the ZMI. + # Then it is best to manually clear and rebuild + # the catalog later anyway. + # But for now do what would happen without our patch. ob.unindexObject() + else: + ob_path = '/'.join(ob.getPhysicalPath()) + rid = catalog._catalog.uids.get(ob_path) + if rid is not None: + setattr(ob, '_v_rid', rid) + else: + # This may happen if deferred indexing is active and an + # object is added and renamed/moved in the same transaction + # (e.g. moved in an IObjectAddedEvent handler) + return + elif event.oldParent is not None: + # Delete + ob.unindexObject() elif IObjectMovedEvent.providedBy(event): if event.newParent is not None: - catalog = queryUtility(ICatalogTool) - uid = catalog._CatalogTool__url(ob) - if uid.startswith("/"): - # path-based catalog key. - # the object was unindexed at the old path - # and needs to be completely re-added - ob.indexObject() + rid = getattr(ob, '_v_rid', None) + if rid: + catalog = queryUtility(ICatalogTool) + _catalog = catalog._catalog + + new_path = '/'.join(ob.getPhysicalPath()) + old_path = _catalog.paths[rid] + + del _catalog.uids[old_path] + _catalog.uids[new_path] = rid + _catalog.paths[rid] = new_path + + ob.reindexObject( + idxs=["allowedRolesAndUsers", "path", "getId", "id"] + ) + + delattr(ob, '_v_rid') else: - # UID-based catalog key. - # we can update only the indexes that are path-dependent - ob.reindexObject(idxs=["path", "allowedRolesAndUsers", "id", "getId"]) + # This may happen if deferred indexing is active and an + # object is added and renamed/moved in the same transaction + # (e.g. moved in an IObjectAddedEvent handler) + ob.indexObject() elif IObjectCopiedEvent.providedBy(event): if hasattr(aq_base(ob), 'workflow_history'): From 7079330a60d0a8229fe2d344e653e6c06e0cdc93 Mon Sep 17 00:00:00 2001 From: David Glick Date: Sat, 19 Aug 2023 09:21:16 -0700 Subject: [PATCH 3/4] don't use volatile attribute --- Products/CMFPlone/patches/cmfcatalogaware.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Products/CMFPlone/patches/cmfcatalogaware.py b/Products/CMFPlone/patches/cmfcatalogaware.py index 5d53b88584..708b412d54 100644 --- a/Products/CMFPlone/patches/cmfcatalogaware.py +++ b/Products/CMFPlone/patches/cmfcatalogaware.py @@ -33,7 +33,7 @@ def handleContentishEvent(ob, event): ob_path = '/'.join(ob.getPhysicalPath()) rid = catalog._catalog.uids.get(ob_path) if rid is not None: - setattr(ob, '_v_rid', rid) + setattr(ob, '__rid', rid) else: # This may happen if deferred indexing is active and an # object is added and renamed/moved in the same transaction @@ -45,7 +45,7 @@ def handleContentishEvent(ob, event): elif IObjectMovedEvent.providedBy(event): if event.newParent is not None: - rid = getattr(ob, '_v_rid', None) + rid = getattr(ob, '__rid', None) if rid: catalog = queryUtility(ICatalogTool) _catalog = catalog._catalog @@ -61,7 +61,7 @@ def handleContentishEvent(ob, event): idxs=["allowedRolesAndUsers", "path", "getId", "id"] ) - delattr(ob, '_v_rid') + delattr(ob, '__rid') else: # This may happen if deferred indexing is active and an # object is added and renamed/moved in the same transaction From 259d72ffcdcb81a298d0c863f10f5e77540e62aa Mon Sep 17 00:00:00 2001 From: David Glick Date: Sun, 20 Aug 2023 20:05:50 -0700 Subject: [PATCH 4/4] fix problems from old path in queue, fix debugging --- Products/CMFPlone/patches/cmfcatalogaware.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Products/CMFPlone/patches/cmfcatalogaware.py b/Products/CMFPlone/patches/cmfcatalogaware.py index 708b412d54..5b81ef1390 100644 --- a/Products/CMFPlone/patches/cmfcatalogaware.py +++ b/Products/CMFPlone/patches/cmfcatalogaware.py @@ -2,6 +2,7 @@ from Acquisition import aq_base from OFS.interfaces import IObjectWillBeMovedEvent from Products.CMFCore.CMFCatalogAware import handleContentishEvent as orig +from Products.CMFCore.indexing import getQueue from Products.CMFCore.interfaces import ICatalogTool from zope.container.interfaces import IObjectAddedEvent from zope.container.interfaces import IObjectMovedEvent @@ -53,6 +54,9 @@ def handleContentishEvent(ob, event): new_path = '/'.join(ob.getPhysicalPath()) old_path = _catalog.paths[rid] + # Make sure the queue is empty before we update catalog internals + getQueue().process() + del _catalog.uids[old_path] _catalog.uids[new_path] = rid _catalog.paths[rid] = new_path @@ -78,6 +82,8 @@ def handleContentishEvent(ob, event): globals = orig.__globals__ -source = inspect.getsource(handleContentishEvent) -exec(source, globals) -orig.func_code = globals["handleContentishEvent"].__code__ +globals.update({"getQueue": getQueue}) +source = "\n" * (handleContentishEvent.__code__.co_firstlineno - 1) + inspect.getsource(handleContentishEvent) +code = compile(source, handleContentishEvent.__code__.co_filename, 'exec') +exec(code, globals) +orig.__code__ = globals["handleContentishEvent"].__code__