Align with upstream CMFCore#161: contextless_indexes + install profile

README.md

@@ -21,16 +21,17 @@ with an optimized version.
 On a true object move (old parent ≠ new parent, i.e. cut-paste):
 
 1. **`IObjectWillBeMovedEvent`** — instead of calling `unindexObject()`, the
-   object's current physical path is saved in the transaction-local registry
-   keyed by its ZODB `_p_oid`.  The catalog entry is left untouched.
+   indexing queue is flushed (queued objects still report their old path) and
+   the object's current physical path is saved in the transaction-local
+   registry keyed by its ZODB `_p_oid`.  The catalog entry is left untouched.
 2. **`IObjectMovedEvent`** — the saved old path is retrieved, and
    `CatalogTool.moveObject()` (injected by this add-on) is called.  It remaps
    `old_path → same RID → new_path` in the catalog's internal BTree structures,
-   then calls `reindexObject()` with **only the context-aware indexes**.
+   updates the modification date, then calls `reindexObject()` with **all
+   indexes except the contextless ones** (see below).
 
-The net result: the RID is preserved, only the path-dependent and
-security-dependent indexes are recomputed, and the full reindex of expensive
-text/metadata indexes is skipped entirely.
+The net result: the RID is preserved, and the contextless indexes
+(`SearchableText` unless configured otherwise) are not recomputed.
 
 For **renames** (same parent, new id) the same path is followed — the object
 stays in the same container, only its path and id change.
@@ -48,41 +49,45 @@ for large subtrees, objects can be evicted from the ZODB cache between the
 reindex.  Transaction-attached data lives outside the ZODB object graph and is
 discarded automatically on commit or abort.
 
-### Context-aware indexes
+### Contextless indexes
 
-Only indexes whose values change when an object moves need to be reindexed.
-This add-on ships with two built-in providers:
+The catalog entry is remapped on move (RID preserved) instead of being
+unindexed and indexed again.  Every index is then reindexed, except the
+*contextless* ones: indexes whose value does not depend on the object's
+location or security context.  They are listed in the optional
+`contextless_indexes` lines property of `portal_catalog`.
 
-| Provider name | Indexes |
-|---|---|
-| `cmf.location` | `path`, `getId`, `id` |
-| `cmf.security` | `allowedRolesAndUsers` |
-
-Third-party packages can contribute additional indexes by registering a named
-utility providing `IContextAwareIndexProvider`:
+**Without the property, `SearchableText` is treated as contextless**, where
+most of the cost is.  This differs from upstream CMFCore#161, which reindexes
+every index when the property is missing.  The property can be set from a
+GenericSetup `catalog.xml`:
 
 ```xml
-<!-- my.package/configure.zcml -->
-<utility
-    provides="experimental.catalogmoveopt.interfaces.IContextAwareIndexProvider"
-    name="my.package.myindex"
-    component=".providers.MyIndexProvider"
-    />
+<object name="portal_catalog">
+  <property name="contextless_indexes" type="lines">
+    <element value="SearchableText" />
+  </property>
+</object>
 ```
 
-```python
-# my.package/providers.py
-from zope.interface import implementer
-from experimental.catalogmoveopt.interfaces import IContextAwareIndexProvider
+Set the property to an empty list to opt out of the default and reindex every
+index on move.  Add other indexes to the list only if their value cannot change
+on move.
 
-@implementer(IContextAwareIndexProvider)
-class MyIndexProvider:
-    def getIndexNames(self):
-        return ("my_custom_index",)
-```
+An index added later (e.g. through the ZMI) is reindexed on move unless it is
+listed.  Do not list an index if a subscriber changes its value on move.
+
+### Catalogs without `moveObject`
 
-If no providers are registered the optimization is disabled and the stock
-full-reindex path is used as a safe fallback.
+`moveObject(object, old_path)` is optional for `ICatalogTool` implementations.
+If the registered catalog does not provide it (e.g. `plone-pgcatalog`), the
+stock unindex + index flow is used.  `CatalogTool.moveObject` also updates the
+modification date of the moved object, and returns `False` if nothing is
+cataloged under `old_path` (the object is then indexed like a regular add).
+
+Note for `IIndexQueueProcessor` implementations: a move now queues a
+`reindex` at the new path instead of an `unindex` of the old path followed by
+an `index`.
 
 ## Installation
 
@@ -96,9 +101,19 @@ dependencies = [
 ]
 ```
 
-No further configuration is required.  The add-on uses
-`z3c.autoinclude.plugin` so its ZCML is loaded automatically when installed in
-a Plone site.
+The add-on uses `z3c.autoinclude.plugin` so its ZCML is loaded automatically
+and the optimization is active as soon as the package is installed.
+
+Note that this changes behavior without any further step: `SearchableText` is
+no longer reindexed on move.  To keep reindexing it, set `contextless_indexes`
+to an empty list (see above) or install the `uninstall` profile below.
+
+No profile is required: when `portal_catalog` has no `contextless_indexes`
+property, `SearchableText` is skipped on move.  Installing the
+`experimental.catalogmoveopt:default` GenericSetup profile makes this explicit
+by setting the property on `portal_catalog`, so it can be edited or exported.
+The `experimental.catalogmoveopt:uninstall` profile sets it to an empty list,
+so all indexes are reindexed on move.
 
 ## Compatibility
 
@@ -135,8 +150,8 @@ way into the Plone/CMFCore ecosystem proper.  Key references:
 
 - **[zopefoundation/Products.CMFCore#161](https://github.com/zopefoundation/Products.CMFCore/pull/161)**
   — the upstream CMFCore pull request (by the author of this package) that
-  proposes adding `CatalogTool.moveObject()` and the `IContextAwareIndexProvider`
-  interface directly to CMFCore.  Once merged, this add-on will become
+  proposes adding `CatalogTool.moveObject()` and the `contextless_indexes`
+  catalog property directly to CMFCore.  Once merged, this add-on will become
   unnecessary.
 
 ## Contribute

news/+contextless.breaking

@@ -0,0 +1,5 @@
+Align with the updated upstream Products.CMFCore#161: every index is now reindexed on move, except the ones listed in the `contextless_indexes` property of `portal_catalog`.
+`IContextAwareIndexProvider` and the built-in providers are removed.
+`CatalogTool.moveObject(object, old_path)` no longer takes `idxs`, updates the modification date and returns `True`/`False`.
+The indexing queue is flushed before a move, and a leftover catalog entry at the new path is dropped.
+Catalogs without `moveObject` keep the stock unindex + index behavior.

news/+profile.feature

@@ -0,0 +1,2 @@
+Skip `SearchableText` on move by default, even without installing a profile: it applies when `portal_catalog` has no `contextless_indexes` property. An empty property opts out.
+Add the `experimental.catalogmoveopt:default` GenericSetup profile, which sets the property explicitly, and an `uninstall` profile that sets it empty.

src/experimental/catalogmoveopt/configure.zcml

@@ -1,27 +1,25 @@
 <configure
     xmlns="http://namespaces.zope.org/zope"
+    xmlns:genericsetup="http://namespaces.zope.org/genericsetup"
     i18n_domain="experimental.catalogmoveopt"
     >
 
   <include file="dependencies.zcml" />
 
-  <!-- Default context-aware index providers (location + security + temporal). -->
-  <utility
-      provides=".interfaces.IContextAwareIndexProvider"
-      name="cmf.location"
-      component=".providers.builtin_location_provider"
+  <genericsetup:registerProfile
+      name="default"
+      title="experimental.catalogmoveopt"
+      description="Skip SearchableText reindexing when content is moved"
+      provides="Products.GenericSetup.interfaces.EXTENSION"
+      directory="profiles/default"
       />
 
-  <utility
-      provides=".interfaces.IContextAwareIndexProvider"
-      name="cmf.security"
-      component=".providers.builtin_security_provider"
-      />
-
-  <utility
-      provides=".interfaces.IContextAwareIndexProvider"
-      name="cmf.temporal"
-      component=".providers.builtin_temporal_provider"
+  <genericsetup:registerProfile
+      name="uninstall"
+      title="experimental.catalogmoveopt: uninstall"
+      description="Restore reindexing of all indexes when content is moved"
+      provides="Products.GenericSetup.interfaces.EXTENSION"
+      directory="profiles/uninstall"
       />
 
   <!--

src/experimental/catalogmoveopt/patches.py

@@ -4,21 +4,23 @@
 
 1. ``handleContentishEvent`` is replaced with a version that, on a true object
    move, skips the full unindex + reindex cycle and instead calls
-   ``CatalogTool.moveObject`` to remap the catalog RID and reindex only the
-   context-aware indexes.
+   ``CatalogTool.moveObject`` to remap the catalog RID and reindex all
+   indexes except the ones listed in the ``contextless_indexes`` property of
+   ``portal_catalog``.
 
 2. ``CatalogTool.moveObject`` is injected (it does not exist in stock CMFCore).
 
 The replacement is functionally identical to the original for every event type
 except ``IObjectWillBeMovedEvent`` and ``IObjectMovedEvent`` on true moves.
+Catalog tools that do not provide ``moveObject`` keep the stock unindex +
+index behavior.
 
 The old path is stored via ``Transaction.set_data`` / ``Transaction.data``
 (keyed by a module-level singleton) rather than a volatile ``_v_`` attribute,
 making it immune to ZODB cache eviction (ghostification) for large subtrees.
 See ``_pending_move_paths()`` for details.
 """
 
-from .providers import get_context_aware_indexes
 from Acquisition import aq_base
 from OFS.interfaces import IObjectWillBeMovedEvent
 from zope.component import getGlobalSiteManager
@@ -87,28 +89,39 @@ def _handle_object_moved(ob, event):
     oid = getattr(ob, "_p_oid", None)
     old_path = _pending_move_paths().pop(oid, None) if oid else None
     if old_path is not None:
+        # True move: optimization path, preserve the catalog RID.
         catalog = queryUtility(ICatalogTool)
-        if catalog is not None:
-            if hasattr(aq_base(ob), "notifyModified"):
-                ob.notifyModified()
-            catalog.moveObject(ob, old_path, get_context_aware_indexes())
+        if catalog is not None and catalog.moveObject(ob, old_path):
             return
+        # Not cataloged under the old path: index it like a regular add,
+        # honouring any indexObject override.
     ob.indexObject()
 
 
 def _handle_object_will_be_moved(ob, event):
+    from Products.CMFCore.indexing import processQueue
     from Products.CMFCore.interfaces import ICatalogTool
 
     if event.oldParent is None:
         return
     if event.newParent is not None:
         catalog = queryUtility(ICatalogTool)
-        idxs = get_context_aware_indexes()
-        if catalog is not None and idxs:
-            oid = getattr(ob, "_p_oid", None)
-            if oid is not None:
-                _pending_move_paths()[oid] = "/".join(ob.getPhysicalPath())
-                return  # skip unindexObject; catalog entry preserved
+        oid = getattr(ob, "_p_oid", None)
+        # aq_base: never acquire a moveObject from the catalog's container
+        # (e.g. plone.folder's moveObject(id, position)).
+        if (
+            catalog is not None
+            and oid is not None
+            and getattr(aq_base(catalog), "moveObject", None) is not None
+        ):
+            # Flush the indexing queue while every queued wrapper still
+            # reports the old path.  Processed after the move, a pending
+            # operation would be applied to the new path (rename) or the old
+            # one (cut/paste), creating a second catalog entry next to the
+            # remapped one.
+            processQueue()
+            _pending_move_paths()[oid] = "/".join(ob.getPhysicalPath())
+            return  # skip unindexObject; catalog entry preserved
     ob.unindexObject()
 
 
@@ -142,34 +155,58 @@ def handleContentishEvent(ob, event):
 # ---------------------------------------------------------------------------
 
 
-def _catalog_tool_move_object(self, obj, old_path, idxs):
-    """Update the catalog when ``obj`` is moved, preserving its RID.
+#: Indexes skipped on move when ``portal_catalog`` has no
+#: ``contextless_indexes`` property.
+DEFAULT_CONTEXTLESS_INDEXES = ("SearchableText",)
 
-    Flushes the index queue, remaps the old path to the same RID at the new
-    path, and reindexes only ``idxs``.  Injected into ``CatalogTool`` by
-    ``apply_patches()``.
-    """
-    from Products.CMFCore.indexing import getQueue
 
-    getQueue().process()
+def _catalog_tool_move_object(self, obj, old_path):
+    """Update the catalog when ``obj`` is moved, preserving its RID.
+
+        Updates the modification date of the object, remaps the catalog entry at
+        ``old_path`` to the object's new path, then reindexes all indexes except
+        the ones listed in the optional ``contextless_indexes`` property (default:
+    ``SearchableText`` when the property does not exist).
 
-    new_path = "/".join(obj.getPhysicalPath())
+        Returns False, leaving the catalog and the object untouched, if
+        ``old_path`` is not cataloged.  Injected into ``CatalogTool`` by
+        ``apply_patches()``.
+    """
     cat = self._catalog
     rid = cat.uids.get(old_path)
-
     if rid is None:
-        # Object not yet in catalog (added and moved in the same transaction;
-        # INDEX already ran at the new path via queue.process()).
-        self.reindexObject(obj, idxs=list(idxs), update_metadata=1)
-        return
+        return False
+
+    # Without the optimization, CMFPlone updates the modification date as a
+    # side-effect of its indexObject override, which calls
+    # reindexObject(idxs=[]).  Keep that behavior: HTTP caches and ETags built
+    # on the modification date must be invalidated when the URL of the object
+    # changes.
+    if hasattr(aq_base(obj), "notifyModified"):
+        obj.notifyModified()
 
-    # Remap old path → same RID → new path (preserves RID).
+    new_path = "/".join(obj.getPhysicalPath())
+    stale_rid = cat.uids.get(new_path)
+    if stale_rid is not None and stale_rid != rid:
+        # Drop a leftover entry at the new path, or its RID would be orphaned
+        # by the remap below.
+        self.uncatalog_object(new_path)
+
+    # Remap old path -> same RID -> new path (preserves RID).
     cat.uids[new_path] = rid
     cat.paths[rid] = new_path
-    if old_path in cat.uids:
-        del cat.uids[old_path]
-
-    self.reindexObject(obj, idxs=list(idxs), update_metadata=1)
+    del cat.uids[old_path]
+
+    # Property missing: default to DEFAULT_CONTEXTLESS_INDEXES.  An existing
+    # but empty property explicitly opts out: all indexes are reindexed.
+    contextless = self.getProperty("contextless_indexes", None)
+    if contextless is None:
+        contextless = DEFAULT_CONTEXTLESS_INDEXES
+    idxs = [i for i in self.indexes() if i not in contextless]
+    if idxs:
+        # An empty list would mean "all indexes".
+        self.reindexObject(obj, idxs=idxs, update_metadata=1)
+    return True
 
 
 # ---------------------------------------------------------------------------

src/experimental/catalogmoveopt/profiles/default/catalog.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<object name="portal_catalog">
+  <property name="contextless_indexes"
+            type="lines"
+  >
+    <element value="SearchableText" />
+  </property>
+</object>

src/experimental/catalogmoveopt/profiles/default/metadata.xml

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<metadata>
+  <version>1</version>
+</metadata>

src/experimental/catalogmoveopt/profiles/uninstall/catalog.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<object name="portal_catalog">
+  <!-- An empty property opts out of the default: all indexes are reindexed. -->
+  <property name="contextless_indexes"
+            type="lines"
+  />
+</object>