mirror of
https://github.com/suitenumerique/drive.git
synced 2026-09-12 04:37:56 +02:00
⚡️(backend) avoid N+1 queries to compute nb_accesses on children list
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).
This commit is contained in:
@@ -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"])
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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():
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user