[19.0][FIX] base_tier_validation: systray recount cost grows with the reviewer's backlog

base_tier_validation/models/res_users.py

@@ -33,14 +33,25 @@ def review_user_count(self):
             aggregates=["id:recordset"],
         )
         for model, tier_review in review_groups:
-            Model = self.env[model]
             # Skip Models not having Tier Validation enabled (example: was unistalled)
-            if tier_review and hasattr(Model, "can_review"):
-                records_domain = (
-                    Domain("id", "in", tier_review.mapped("res_id"))
-                    & Domain("validation_status", "!=", "rejected")
-                    & Domain("can_review", "=", True)
-                )
+            if not tier_review or model not in self.env:
+                continue
+            Model = self.env[model]
+            if hasattr(Model, "can_review"):
+                records_domain = Domain(
+                    "id", "in", tier_review.mapped("res_id")
+                ) & Domain("validation_status", "!=", "rejected")
+                # Excludes any cancelled records depending on the structure of
+                # the model. Let PostgreSQL do it whenever the state field is a
+                # real column -- which it is on every model that stores its
+                # state -- and keep the Python pass only as the fallback for
+                # models whose state field is computed and not stored.
+                state_field = Model._fields.get(Model._state_field)
+                state_in_db = state_field is not None and state_field.store
+                if state_in_db:
+                    records_domain &= Domain(
+                        Model._state_field, "!=", Model._cancel_state
+                    )
                 try:
                     records = (
                         Model.with_user(user)
@@ -60,8 +71,16 @@ def review_user_count(self):
                         model,
                     )
                     continue
-                # Excludes any cancelled records depending on the structure of the model
-                if Model._state_field in Model._fields:
+                # ``can_review`` is deliberately *not* part of ``records_domain``.
+                # Its search method re-searches the reviewer's whole backlog on
+                # this model and then evaluates the field in Python over all of
+                # it, so putting it in the domain made the systray cost grow
+                # with the backlog -- on every call, for every reviewer. The
+                # candidate set is already narrowed to the pending reviews found
+                # above, so evaluate the field on those records instead: same
+                # answer, one prefetched batch instead of a second search.
+                records = records.filtered("can_review")
+                if not state_in_db and Model._state_field in Model._fields:
                     records = records.filtered(
                         lambda x: x[x._state_field] != x._cancel_state
                     )

base_tier_validation/models/tier_review.py

@@ -2,6 +2,7 @@
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
 import logging
+from collections import defaultdict
 
 from odoo import api, fields, models
 from odoo.exceptions import ValidationError
@@ -93,9 +94,39 @@ def _compute_reviewed_formated_date(self):
 
     @api.depends("status", "approve_sequence", "sequence", "model", "res_id")
     def _compute_can_review(self):
+        # Only sequential definitions reach the branch of ``_can_review_value``
+        # that looks at the document, so only those are worth warming.
+        self.filtered("approve_sequence")._prefetch_resource_reviews()
         for record in self:
             record.can_review = record._can_review_value()
 
+    def _prefetch_resource_reviews(self):
+        """Warm the cache with the reviews of every resource in ``self``.
+
+        ``_can_review_value`` browses its own ``res_id`` to find the lowest
+        pending sequence on that document. Record by record, that browse has a
+        prefetch set of exactly one id, so every review pays a fresh read of
+        its document's ``review_ids`` -- a handful of queries per review, on
+        every recompute of this (stored) field. The systray recount flushes
+        ``can_review``, so the cost of the counter grew with the reviewer's
+        backlog.
+
+        Reading the same relation once per model puts the values in the
+        environment cache, where the per-record browse then finds them for
+        free. Callers pass the subset whose documents they are about to look
+        at.
+        """
+        res_ids_per_model = defaultdict(list)
+        for record in self:
+            if record.model and record.res_id:
+                res_ids_per_model[record.model].append(record.res_id)
+        for model, res_ids in res_ids_per_model.items():
+            # The model may have been uninstalled, or have dropped tier
+            # validation, while reviews pointing at it survive.
+            if model not in self.env or "review_ids" not in self.env[model]._fields:
+                continue
+            self.env[model].browse(res_ids).review_ids.fetch(["status", "sequence"])
+
     def _update_review_status(self):
         """Promote reviews that are currently available to pending."""
         # To defer recompute, use context key

base_tier_validation/models/tier_validation.py

@@ -112,6 +112,13 @@ def _get_sequences_to_approve(self, user):
         "review_ids.status",
     )
     def _compute_can_review(self):
+        # ``_get_sequences_to_approve`` filters ``review_ids`` and then reads
+        # ``reviewer_ids`` on what is left. ``filtered`` returns a recordset
+        # whose prefetch set is narrowed to the records it kept, so that read
+        # is issued one document at a time -- one query per pending review,
+        # every time the systray recounts. Read the relation once, for the
+        # whole batch, before the loop narrows anything.
+        self.review_ids.mapped("reviewer_ids")
         for rec in self:
             rec.can_review = rec._get_sequences_to_approve(self.env.user)
 

base_tier_validation/tests/test_tier_validation.py

@@ -662,6 +662,70 @@ def test_16b_review_user_count_no_model_access(self):
         result = self.test_user_2.with_user(self.test_user_2).review_user_count()
         self.assertEqual(result, [])
 
+    def test_16c_review_user_count_cost_flat_in_backlog(self):
+        """The systray recount must not get dearer as the backlog grows.
+
+        ``review_user_count`` used to put ``can_review`` in the *document*
+        domain, whose search method re-searches the reviewer's entire backlog
+        on that model and then evaluates the field in Python over all of it;
+        and every recompute of the stored ``tier.review.can_review`` browsed
+        its document one row at a time. Both made the endpoint cost a handful
+        of queries per pending review, paid by every reviewer on every
+        notification -- enough to occupy every HTTP worker on a batch approval.
+        """
+        self.tier_def_obj.create(
+            {
+                "model_id": self.tester_model.id,
+                "review_type": "individual",
+                "reviewer_id": self.test_user_1.id,
+                "definition_domain": "[('test_field', '=', 2.0)]",
+                "approve_sequence": True,
+                "notify_on_pending": False,
+                "sequence": 5,
+                "name": "Definition for test 16c - backlog",
+            }
+        )
+
+        def add_backlog(count):
+            for _i in range(count):
+                record = self.test_model.create({"test_field": 2.0})
+                record.with_user(self.test_user_2).request_validation()
+
+        def measure():
+            # A batch approval keeps moving review statuses, so the stored
+            # ``can_review`` is permanently dirty in production. Reproduce that,
+            # then drop the cache: marking the field to recompute must not go
+            # through a read of the reviews, or the cache it fills would hide
+            # exactly the queries this test is about.
+            self.env.flush_all()
+            self.env.add_to_compute(
+                self.env["tier.review"]._fields["can_review"],
+                self.test_user_1.review_ids,
+            )
+            self.env.invalidate_all(flush=False)
+            before = self.env.cr.sql_log_count
+            result = self.test_user_1.with_user(self.test_user_1).review_user_count()
+            return self.env.cr.sql_log_count - before, result
+
+        add_backlog(2)
+        small_queries, small = measure()
+        self.assertEqual(small[0]["pending_count"], 2)
+
+        add_backlog(20)
+        large_queries, large = measure()
+        self.assertEqual(large[0]["pending_count"], 22)
+
+        # Twenty more pending documents may not cost twenty more round trips.
+        # The tolerance is deliberately loose: the point is that the growth is
+        # bounded, not that the absolute query count never moves.
+        self.assertLessEqual(
+            large_queries,
+            small_queries + 3,
+            "review_user_count scales with the reviewer's backlog: "
+            f"{small_queries} queries for 2 pending documents, "
+            f"{large_queries} for 22.",
+        )
+
     def test_17_search_records_no_validation(self):
         """Search for records that have no validation process started"""
         records = self.env["tier.validation.tester"].search(