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(): """