From a42e802f8f90adc2abcf9ee40e0d944b8a911a33 Mon Sep 17 00:00:00 2001 From: Samuel Paccoud - DINUM Date: Thu, 1 Jan 2026 12:16:23 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=EF=B8=8F(backend)=20avoid=20N+1=20que?= =?UTF-8?q?ries=20to=20compute=20nb=5Faccesses=20on=20children=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can calculate a child's number of accesses by adding the count for its parent item and adding the count directly attached to it. This addition can be done in the same list query via an annotation so we don't generate N+1 queries (N equals 200 by default on the children view). --- src/backend/core/api/viewsets.py | 11 ++++- src/backend/core/models.py | 19 ++++--- .../items/test_api_items_children_list.py | 49 +++++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index c9ba1e7e..e095869d 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -16,6 +16,7 @@ from django.core.files.storage import default_storage from django.db import models as db from django.db import transaction from django.db.models.expressions import RawSQL +from django.db.models.functions import Coalesce from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.text import slugify @@ -918,10 +919,18 @@ class ItemViewSet( raise drf.exceptions.ValidationError(filterset.errors) queryset = filterset.qs - # Apply ordering only now that everyting is filtered and annotated + # Apply ordering only now that everything is filtered and annotated queryset = filters.OrderingFilter().filter_queryset( self.request, queryset, self ) + + # Pre-compute number of accesses + item_nb_accesses = item.nb_accesses + queryset = queryset.annotate( + _nb_accesses=db.Value(item_nb_accesses) + + Coalesce(db.Count("accesses", distinct=True), 0), + ) + return self.get_response_for_queryset(queryset) @drf.decorators.action(detail=True, methods=["get"]) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 5178c27d..fae29fef 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -677,16 +677,19 @@ class Item(TreeModel, BaseModel): @property def nb_accesses(self): """Calculate the number of accesses.""" - cache_key = self.get_nb_accesses_cache_key() - nb_accesses = cache.get(cache_key) + try: + return self._nb_accesses + except AttributeError: + cache_key = self.get_nb_accesses_cache_key() + nb_accesses = cache.get(cache_key) - if nb_accesses is None: - nb_accesses = ItemAccess.objects.filter( - item__path__ancestors=self.path, - ).count() - cache.set(cache_key, nb_accesses) + if nb_accesses is None: + nb_accesses = ItemAccess.objects.filter( + item__path__ancestors=self.path, + ).count() + cache.set(cache_key, nb_accesses) - return nb_accesses + return nb_accesses @property def is_root(self): diff --git a/src/backend/core/tests/items/test_api_items_children_list.py b/src/backend/core/tests/items/test_api_items_children_list.py index e0229d4e..cd4ee1ea 100644 --- a/src/backend/core/tests/items/test_api_items_children_list.py +++ b/src/backend/core/tests/items/test_api_items_children_list.py @@ -968,6 +968,55 @@ def test_api_items_children_list_filter_type(): ], } +def test_api_items_children_list_nb_accesses(): + """The number of accesses is computed correctly when retrieving children list (pre-computed).""" + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + grand_parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + parent = factories.ItemFactory( + parent=grand_parent, type=models.ItemTypeChoices.FOLDER + ) + item = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + factories.UserItemAccessFactory(item=item, user=user) + children = factories.ItemFactory.create_batch(5, parent=item) + + nb_accesses_grand_parent = random.randint(0, 5) + nb_accesses_parent = random.randint(0, 5) + nb_accesses_item = random.randint(0, 5) + nb_accesses_children = [random.randint(0, 5) for _ in range(5)] + + factories.UserItemAccessFactory.create_batch( + nb_accesses_grand_parent, item=grand_parent + ) + factories.UserItemAccessFactory.create_batch(nb_accesses_parent, item=parent) + factories.UserItemAccessFactory.create_batch(nb_accesses_item, item=item) + expected_nb_accesses = {} + for child, number in zip(children, nb_accesses_children, strict=True): + factories.UserItemAccessFactory.create_batch(number, item=child) + expected_nb_accesses[str(child.id)] = ( + nb_accesses_grand_parent + + nb_accesses_parent + + nb_accesses_item + + 1 + + number + ) + + response = client.get( + f"/api/v1.0/items/{item.id!s}/children/", + ) + + assert response.status_code == 200 + returned_children = response.json()["results"] + + for child_data in returned_children: + expected_number = expected_nb_accesses[child_data["id"]] + assert child_data["nb_accesses"] == expected_number, ( + f"Child {child_data['id']} has nb_accesses={child_data['nb_accesses']}, " + f"expected {expected_number}" + ) def test_api_items_children_list_filter_wrong_type(): """