diff --git a/README.md b/README.md
index f213c8b..8b898c9 100644
--- a/README.md
+++ b/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
-
-
+
```
-```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
diff --git a/news/+contextless.breaking b/news/+contextless.breaking
new file mode 100644
index 0000000..1b2c638
--- /dev/null
+++ b/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.
diff --git a/news/+profile.feature b/news/+profile.feature
new file mode 100644
index 0000000..9ec48b4
--- /dev/null
+++ b/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.
diff --git a/src/experimental/catalogmoveopt/configure.zcml b/src/experimental/catalogmoveopt/configure.zcml
index 3875821..b979895 100644
--- a/src/experimental/catalogmoveopt/configure.zcml
+++ b/src/experimental/catalogmoveopt/configure.zcml
@@ -1,27 +1,25 @@
-
-
-
-
-
+
+
diff --git a/src/experimental/catalogmoveopt/providers.py b/src/experimental/catalogmoveopt/providers.py
deleted file mode 100644
index 696da0d..0000000
--- a/src/experimental/catalogmoveopt/providers.py
+++ /dev/null
@@ -1,60 +0,0 @@
-from .interfaces import IContextAwareIndexProvider
-from zope.component import getUtilitiesFor
-from zope.interface import implementer
-
-
-@implementer(IContextAwareIndexProvider)
-class _BuiltinLocationIndexProvider:
- """Default provider for location-based context-aware indexes.
-
- These indexes change when an object moves to a different container
- (its path and id change).
- """
-
- def getIndexNames(self):
- return ("path", "getId", "id")
-
-
-@implementer(IContextAwareIndexProvider)
-class _BuiltinSecurityIndexProvider:
- """Default provider for security context-aware indexes.
-
- allowedRolesAndUsers must be recomputed whenever the object moves
- into a differently-protected part of the tree.
- """
-
- def getIndexNames(self):
- return ("allowedRolesAndUsers",)
-
-
-@implementer(IContextAwareIndexProvider)
-class _BuiltinTemporalIndexProvider:
- """Default provider for modification-date context-aware indexes.
-
- Moving an object changes its context (path, security tree), which is a
- meaningful content change from the perspective of caches and change-tracking
- systems. Updating ``modified`` / ``Date`` on move ensures that cache keys
- built on modification dates are correctly invalidated.
- """
-
- def getIndexNames(self):
- return ("modified", "Date")
-
-
-#: Module-level singletons registered as named utilities in configure.zcml.
-builtin_location_provider = _BuiltinLocationIndexProvider()
-builtin_security_provider = _BuiltinSecurityIndexProvider()
-builtin_temporal_provider = _BuiltinTemporalIndexProvider()
-
-
-def get_context_aware_indexes():
- """Return frozenset of all context-aware catalog index names.
-
- Aggregates all named IContextAwareIndexProvider utilities registered in
- the global site manager. Returns an empty frozenset if no providers are
- registered (which disables the optimization).
- """
- indexes = set()
- for _name, provider in getUtilitiesFor(IContextAwareIndexProvider):
- indexes.update(provider.getIndexNames())
- return frozenset(indexes)
diff --git a/src/experimental/catalogmoveopt/testing.py b/src/experimental/catalogmoveopt/testing.py
index f568d3d..9a540d2 100644
--- a/src/experimental/catalogmoveopt/testing.py
+++ b/src/experimental/catalogmoveopt/testing.py
@@ -1,4 +1,5 @@
from plone.app.contenttypes.testing import PLONE_APP_CONTENTTYPES_FIXTURE
+from plone.app.testing import applyProfile
from plone.app.testing import IntegrationTesting
from plone.app.testing import PloneSandboxLayer
@@ -17,6 +18,9 @@ def setUpZope(self, app, configurationContext):
apply_patches()
+ def setUpPloneSite(self, portal):
+ applyProfile(portal, "experimental.catalogmoveopt:default")
+
FIXTURE = Layer()
diff --git a/tests/setup/test_patches_applied.py b/tests/setup/test_patches_applied.py
index 12f9b13..c6b8e0a 100644
--- a/tests/setup/test_patches_applied.py
+++ b/tests/setup/test_patches_applied.py
@@ -32,3 +32,18 @@ def test_catalog_tool_has_move_object(self, integration):
assert hasattr(CatalogTool, "moveObject")
assert CatalogTool.moveObject is _catalog_tool_move_object
+
+
+class TestProfile:
+ def test_contextless_indexes_installed(self, portal):
+ catalog = portal.portal_catalog
+ assert catalog.getProperty("contextless_indexes") == ("SearchableText",)
+
+ def test_uninstall_empties_property(self, portal):
+ from Products.CMFCore.utils import getToolByName
+
+ setup = getToolByName(portal, "portal_setup")
+ setup.runAllImportStepsFromProfile(
+ "profile-experimental.catalogmoveopt:uninstall"
+ )
+ assert portal.portal_catalog.getProperty("contextless_indexes") == ()
diff --git a/tests/test_move_optimization.py b/tests/test_move_optimization.py
index 3fff551..e44b079 100644
--- a/tests/test_move_optimization.py
+++ b/tests/test_move_optimization.py
@@ -147,57 +147,168 @@ def test_new_path_findable_after_move(
assert catalog._catalog.uids.get(new_path) is not None
-class TestOnlyContextAwareIndexesReindexed:
- def test_rename_reindexes_only_declared_indexes(self, portal, doc, integration):
- """reindexObject is called with only the context-aware index set."""
- from Acquisition import aq_base
+def _capture_reindex(catalog, obj):
+ """Patch context manager helper: record ``idxs`` of reindexes of *obj*."""
+ from Acquisition import aq_base
+ from unittest.mock import patch
+
+ obj_base = aq_base(obj)
+ calls = []
+ original = catalog.reindexObject
+
+ # ``CMFCatalogAware.reindexObject`` forwards a ``uid`` keyword to the
+ # catalog tool, so the wrapper must accept it. Only calls for the moved
+ # object are recorded (renaming a child also reindexes the container).
+ def capturing_reindex(o, idxs=None, update_metadata=0, uid=None):
+ if aq_base(o) is obj_base:
+ calls.append(frozenset(idxs or ()))
+ return original(o, idxs=idxs, update_metadata=update_metadata, uid=uid)
+
+ return calls, patch.object(catalog, "reindexObject", capturing_reindex)
+
+
+class TestContextlessIndexes:
+ def test_rename_skips_contextless_indexes(self, portal, doc, integration):
+ """With the profile installed, SearchableText is not reindexed."""
from Products.CMFCore.utils import getToolByName
- from unittest.mock import patch
import plone.api
catalog = getToolByName(portal, "portal_catalog")
- doc_path_base = aq_base(doc)
- reindex_calls = []
+ calls, patcher = _capture_reindex(catalog, doc)
- original = catalog.reindexObject
+ with patcher, plone.api.env.adopt_roles(["Manager"]):
+ plone.api.content.rename(obj=doc, new_id="test-doc-renamed")
- # ``CMFCatalogAware.reindexObject`` forwards a ``uid`` keyword to the
- # catalog tool, so the wrapper must accept it. We only record calls for
- # the moved object itself (renaming a child also reindexes the container
- # folder via a ContainerModifiedEvent — standard Plone behaviour that is
- # unrelated to the move optimization).
- def capturing_reindex(obj, idxs=None, update_metadata=0, uid=None):
- if aq_base(obj) is doc_path_base:
- reindex_calls.append(frozenset(idxs or ()))
- return original(obj, idxs=idxs, update_metadata=update_metadata, uid=uid)
+ moved = [c for c in calls if "SearchableText" not in c and "path" in c]
+ assert moved, f"move must reindex all but contextless indexes; got {calls}"
+ assert frozenset(catalog.indexes()) - moved[0] == {"SearchableText"}
+ # Never a full reindex (empty idxs == all indexes).
+ assert all(calls), f"move must not full-reindex the object; got {calls}"
- with (
- patch.object(catalog, "reindexObject", capturing_reindex),
- plone.api.env.adopt_roles(["Manager"]),
- ):
+ def test_rename_skips_default_without_property(self, portal, doc, integration):
+ """Without the property (no profile) SearchableText is still skipped."""
+ from Products.CMFCore.utils import getToolByName
+
+ import plone.api
+
+ catalog = getToolByName(portal, "portal_catalog")
+ catalog.manage_delProperties(["contextless_indexes"])
+ calls, patcher = _capture_reindex(catalog, doc)
+
+ with patcher, plone.api.env.adopt_roles(["Manager"]):
plone.api.content.rename(obj=doc, new_id="test-doc-renamed")
- # The optimized move handler must reindex exactly the context-aware
- # index set and nothing more.
- expected = frozenset((
- "path",
- "getId",
- "id",
- "allowedRolesAndUsers",
- "modified",
- "Date",
- ))
- assert expected in reindex_calls, (
- f"move handler must reindex the context-aware set; got {reindex_calls}"
- )
- # Crucially it must never trigger a full reindex of the object (empty
- # idxs == all indexes) — that is precisely the expensive operation the
- # optimization exists to avoid. Any other reindex (e.g. the ordering
- # support's single getObjPositionInParent reindex) is targeted, not full.
- assert all(reindex_calls), (
- f"move must not full-reindex the object; got {reindex_calls}"
- )
+ assert frozenset(catalog.indexes()) - {"SearchableText"} in calls
+
+ def test_rename_reindexes_all_with_empty_property(self, portal, doc, integration):
+ """An empty ``contextless_indexes`` opts out: every index is reindexed."""
+ from Products.CMFCore.utils import getToolByName
+
+ import plone.api
+
+ catalog = getToolByName(portal, "portal_catalog")
+ catalog.manage_changeProperties(contextless_indexes=[])
+ calls, patcher = _capture_reindex(catalog, doc)
+
+ with patcher, plone.api.env.adopt_roles(["Manager"]):
+ plone.api.content.rename(obj=doc, new_id="test-doc-renamed")
+
+ assert frozenset(catalog.indexes()) in calls
+
+ def test_skipped_index_keeps_value_on_move(self, portal, doc, integration):
+ from Products.CMFCore.utils import getToolByName
+
+ import plone.api
+
+ from Products.CMFCore.indexing import processQueue
+
+ catalog = getToolByName(portal, "portal_catalog")
+ processQueue()
+ rid = _rid(catalog, doc)
+ index = catalog._catalog.getIndex("SearchableText")
+ # Make the stored value differ from the object's, then move.
+ before = index.getEntryForObject(rid)
+ doc.title = "Changed without reindex"
+
+ with plone.api.env.adopt_roles(["Manager"]):
+ plone.api.content.rename(obj=doc, new_id="test-doc-renamed")
+
+ assert before
+ assert index.getEntryForObject(rid) == before
+
+ def test_move_updates_modification_date(self, portal, doc, integration):
+ import plone.api
+
+ before = doc.modified()
+
+ with plone.api.env.adopt_roles(["Manager"]):
+ plone.api.content.rename(obj=doc, new_id="test-doc-renamed")
+
+ assert doc.modified() > before
+
+
+class TestMoveObject:
+ def test_returns_false_when_not_cataloged(self, portal, doc, integration):
+ from Products.CMFCore.utils import getToolByName
+
+ catalog = getToolByName(portal, "portal_catalog")
+ assert catalog.moveObject(doc, "/plone/nowhere") is False
+
+ def test_drops_stale_entry_at_new_path(self, portal, doc, integration):
+ from Products.CMFCore.utils import getToolByName
+
+ catalog = getToolByName(portal, "portal_catalog")
+ new_path = "/".join(doc.getPhysicalPath())
+ old_path = "/plone/old-doc"
+ rid = _rid(catalog, doc)
+ # Pretend doc was cataloged at old_path, with a leftover at new_path.
+ catalog._catalog.uids[old_path] = rid
+ catalog._catalog.uids.pop(new_path)
+ catalog.catalog_object(doc, new_path)
+ stale_rid = catalog._catalog.uids[new_path]
+ assert stale_rid != rid
+
+ assert catalog.moveObject(doc, old_path) is True
+
+ assert catalog._catalog.uids[new_path] == rid
+ assert old_path not in catalog._catalog.uids
+ assert stale_rid not in catalog._catalog.paths
+
+
+class TestFallbackWithoutMoveObject:
+ def test_catalog_without_moveObject_unindexes(self, portal, integration):
+ """Catalogs lacking ``moveObject`` keep the stock unindex + index."""
+ from experimental.catalogmoveopt.patches import handleContentishEvent
+ from OFS.interfaces import IObjectWillBeMovedEvent
+ from Products.CMFCore.interfaces import ICatalogTool
+ from unittest.mock import MagicMock
+ from zope.component import getSiteManager
+ from zope.interface import implementer
+
+ @implementer(ICatalogTool)
+ class NoMoveCatalog:
+ pass
+
+ sm = getSiteManager()
+ original = sm.getUtility(ICatalogTool)
+ sm.registerUtility(NoMoveCatalog(), ICatalogTool)
+ try:
+ ob = MagicMock()
+ ob._p_oid = b"\x00" * 8
+
+ @implementer(IObjectWillBeMovedEvent)
+ class Event:
+ oldParent = object()
+ newParent = object()
+
+ event = Event()
+
+ handleContentishEvent(ob, event)
+ finally:
+ sm.registerUtility(original, ICatalogTool)
+
+ ob.unindexObject.assert_called_once()
class TestFallbackOnNoOid:
@@ -207,7 +318,6 @@ def test_object_without_oid_falls_back_to_full_reindex(self, portal, integration
from OFS.interfaces import IObjectWillBeMovedEvent
from unittest.mock import MagicMock
from unittest.mock import patch
- from zope.lifecycleevent.interfaces import IObjectMovedEvent
ob = MagicMock()
ob._p_oid = None # no OID
@@ -216,8 +326,6 @@ def test_object_without_oid_falls_back_to_full_reindex(self, portal, integration
will_be_moved = MagicMock(spec=IObjectWillBeMovedEvent)
will_be_moved.oldParent = MagicMock()
will_be_moved.newParent = MagicMock()
- IObjectWillBeMovedEvent.providedBy = lambda e: e is will_be_moved
- IObjectMovedEvent.providedBy = lambda e: False
with (
patch(
@@ -247,3 +355,27 @@ def test_object_without_oid_falls_back_to_full_reindex(self, portal, integration
handleContentishEvent(ob, will_be_moved)
ob.unindexObject.assert_called_once()
+
+
+class TestPendingQueue:
+ def test_rename_with_pending_reindex_leaves_single_entry(
+ self, portal, doc, integration
+ ):
+ """A reindex queued before the rename must not create a second entry."""
+ from Products.CMFCore.indexing import processQueue
+ from Products.CMFCore.utils import getToolByName
+
+ import plone.api
+
+ catalog = getToolByName(portal, "portal_catalog")
+ processQueue()
+ rid = _rid(catalog, doc)
+ doc.reindexObject() # queued, not yet processed
+
+ with plone.api.env.adopt_roles(["Manager"]):
+ plone.api.content.rename(obj=doc, new_id="test-doc-renamed")
+ processQueue()
+
+ cat = catalog._catalog
+ assert len(cat.uids) == len(cat.paths)
+ assert _rid(catalog, doc) == rid
diff --git a/tests/test_providers.py b/tests/test_providers.py
deleted file mode 100644
index 61f14b8..0000000
--- a/tests/test_providers.py
+++ /dev/null
@@ -1,80 +0,0 @@
-"""Unit tests for get_context_aware_indexes() and IContextAwareIndexProvider."""
-
-from experimental.catalogmoveopt.interfaces import IContextAwareIndexProvider
-from experimental.catalogmoveopt.providers import _BuiltinLocationIndexProvider
-from experimental.catalogmoveopt.providers import _BuiltinSecurityIndexProvider
-from experimental.catalogmoveopt.providers import get_context_aware_indexes
-from zope.component import getGlobalSiteManager
-from zope.interface import implementer
-
-import pytest
-
-
-@implementer(IContextAwareIndexProvider)
-class _CustomProvider:
- def getIndexNames(self):
- return ("my_custom_index",)
-
-
-@pytest.fixture(autouse=True)
-def clean_gsm():
- """Ensure no stray test utilities leak between tests."""
- yield
- gsm = getGlobalSiteManager()
- for name in ("_test.location", "_test.security", "_test.custom"):
- gsm.unregisterUtility(provided=IContextAwareIndexProvider, name=name)
-
-
-class TestGetContextAwareIndexes:
- def test_empty_when_no_providers(self):
- """With no utilities registered the result is an empty frozenset."""
- assert get_context_aware_indexes() == frozenset()
-
- def test_location_provider_indexes(self):
- gsm = getGlobalSiteManager()
- provider = _BuiltinLocationIndexProvider()
- gsm.registerUtility(provider, IContextAwareIndexProvider, "_test.location")
- result = get_context_aware_indexes()
- assert "path" in result
- assert "getId" in result
- assert "id" in result
-
- def test_security_provider_indexes(self):
- gsm = getGlobalSiteManager()
- provider = _BuiltinSecurityIndexProvider()
- gsm.registerUtility(provider, IContextAwareIndexProvider, "_test.security")
- result = get_context_aware_indexes()
- assert "allowedRolesAndUsers" in result
-
- def test_providers_merged(self):
- """Indexes from multiple providers are merged into one frozenset."""
- gsm = getGlobalSiteManager()
- gsm.registerUtility(
- _BuiltinLocationIndexProvider(),
- IContextAwareIndexProvider,
- "_test.location",
- )
- gsm.registerUtility(
- _BuiltinSecurityIndexProvider(),
- IContextAwareIndexProvider,
- "_test.security",
- )
- result = get_context_aware_indexes()
- assert result >= frozenset(("path", "getId", "id", "allowedRolesAndUsers"))
-
- def test_custom_provider_included(self):
- gsm = getGlobalSiteManager()
- gsm.registerUtility(
- _CustomProvider(), IContextAwareIndexProvider, "_test.custom"
- )
- result = get_context_aware_indexes()
- assert "my_custom_index" in result
-
- def test_result_is_frozenset(self):
- gsm = getGlobalSiteManager()
- gsm.registerUtility(
- _BuiltinLocationIndexProvider(),
- IContextAwareIndexProvider,
- "_test.location",
- )
- assert isinstance(get_context_aware_indexes(), frozenset)