Repository: zopefoundation/Products.CMFCore

Issue:
reindexObjectSecurity missing update_metadata parameter (has the effect of full reindex)
## BUG/PROBLEM REPORT

When adding or removing a local role on a section to a user, a full reindex of all the objects in that section takes place, instead of reindexing only the `allowedRolesAndUsers`. This is because the `update_metadata=0` is not passed on https://github.com/zopefoundation/Products.CMFCore/blob/48d0a67a6fee004f51e8048b652c3ca8c1174476/src/Products/CMFCore/CMFCatalogAware.py#L128
[Further down the execution tree](https://github.com/zopefoundation/Products.CMFCore/blob/48d0a67a6fee004f51e8048b652c3ca8c1174476/src/Products/CMFCore/CMFCatalogAware.py#L82), `update_metadata` gets a default value of `1`, thus triggering a full reindex of all objects. In the case of `reindexObjectSecurity` we really don't want or need the metadata updated. This is also related to #140, since it vastly increases the amout of time it takes to reindex.

### What I did:

Added (or removed) some local role from a user, on a folder with a very large number of objects inside.

### What I expect to happen:

Receive an answer in a reasonable amount of time.

### What actually happened:

In my case, the answer took so much as 5 hours (of course depending on the number and type of objects - many images, PDFs, etc.).

### What version of Python and Zope/Addons I am using:

python 3.11, Zope 5.10, Plone 6.0.9 (6020)

Discussion:
@avoinea: To me, it looks like a regression introduced in https://github.com/zopefoundation/Products.CMFCore/commit/a7b1e9cbf6fe5ab948fc0dccd81478e8e81281a2 Or maybe I miss something :thinking: @ale-rt?


Solution patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 4dab6fe..858a776 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,6 +5,7 @@ Products.CMFCore Changelog
 ----------------
 
 - Drop support for Python 3.7.
+- Fix reindexObjectSecurity to avoid a full reindex of affected objects
 
 
 3.5 (2024-03-23)
diff --git a/src/Products/CMFCore/CMFCatalogAware.py b/src/Products/CMFCore/CMFCatalogAware.py
index 502161f..93b85be 100644
--- a/src/Products/CMFCore/CMFCatalogAware.py
+++ b/src/Products/CMFCore/CMFCatalogAware.py
@@ -125,7 +125,8 @@ def reindexObjectSecurity(self, skip_self=False):
                                'catalog', brain_path)
                 continue
             s = getattr(ob, '_p_changed', 0)
-            ob.reindexObject(idxs=self._cmf_security_indexes)
+            ob.reindexObject(idxs=self._cmf_security_indexes,
+                             update_metadata=0)
             if s is None:
                 ob._p_deactivate()
 


Test patch:
diff --git a/src/Products/CMFCore/tests/test_CMFCatalogAware.py b/src/Products/CMFCore/tests/test_CMFCatalogAware.py
index a3ba09a..00fb5ce 100644
--- a/src/Products/CMFCore/tests/test_CMFCatalogAware.py
+++ b/src/Products/CMFCore/tests/test_CMFCatalogAware.py
@@ -194,9 +194,9 @@ def test_reindexObjectSecurity(self):
         foo.reindexObjectSecurity()
         log = sorted(cat.log)
         self.assertEqual(log, [
-            'reindex /site/foo %s 1' % str(CMF_SECURITY_INDEXES),
-            'reindex /site/foo/bar %s 1' % str(CMF_SECURITY_INDEXES),
-            'reindex /site/foo/hop %s 1' % str(CMF_SECURITY_INDEXES),
+            'reindex /site/foo %s 0' % str(CMF_SECURITY_INDEXES),
+            'reindex /site/foo/bar %s 0' % str(CMF_SECURITY_INDEXES),
+            'reindex /site/foo/hop %s 0' % str(CMF_SECURITY_INDEXES),
             ])
         self.assertFalse(foo.notified)
         self.assertFalse(bar.notified)
@@ -227,7 +227,7 @@ def test_reindexObjectSecurity_missing_noraise(self):
         foo.reindexObjectSecurity()
         self.assertEqual(
             cat.log,
-            ['reindex /site/foo %s 1' % str(CMF_SECURITY_INDEXES)])
+            ['reindex /site/foo %s 0' % str(CMF_SECURITY_INDEXES)])
         self.assertFalse(foo.notified)
         self.assertFalse(missing.notified)
         self.assertEqual(len(self.logged), 1)  # logging because no raise