studio/docs
Archived
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

⚡️(backend) optimize number of queries on document list view

Browse Source 
I realized most of the database queries made when getting a document
list view were to include nested accesses. This detailed information
about accesses in only necessary for the document detail view.

I introduced a specific serializer for the document list view with
less fields. For a list of 20 documents with 5 accesses, we go down
from 3x5x20= 300 queries to just 3 queries.
This commit is contained in:
Samuel Paccoud - DINUM
2024-11-09 10:27:21 +01:00
committed by Anthony LC
parent 797d9442ac
commit 2c915d53f4
6 changed files with 97 additions and 262 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -39,6 +39,7 @@ and this project adheres to
## Changed ## Changed
- ⚡️(backend) optimize number of queries on document list view #411
- 🚸(backend) improve users similarity search and sort results #391 - 🚸(backend) improve users similarity search and sort results #391
- ♻️(frontend) simplify stores #402 - ♻️(frontend) simplify stores #402
- ✨(frontend) update $css Box props type to add styled components RuleSet #423 - ✨(frontend) update $css Box props type to add styled components RuleSet #423

41
src/backend/core/api/serializers.py
View File

@@ -137,32 +137,55 @@ class BaseResourceSerializer(serializers.ModelSerializer):
return {} return {}
class DocumentSerializer(BaseResourceSerializer): class ListDocumentSerializer(BaseResourceSerializer):
"""Serialize documents.""" """Serialize documents with limited fields for display in lists."""
content = serializers.CharField(required=False)
accesses = DocumentAccessSerializer(many=True, read_only=True)
class Meta: class Meta:
model = models.Document model = models.Document
fields = [ fields = [
"id", "id",
"content",
"title",
"accesses",
"abilities", "abilities",
"content",
"created_at",
"link_role", "link_role",
"link_reach", "link_reach",
"created_at", "title",
"updated_at", "updated_at",
] ]
read_only_fields = [ read_only_fields = [
"id", "id",
"accesses",
"abilities", "abilities",
"created_at",
"link_role", "link_role",
"link_reach", "link_reach",
"updated_at",
]
class DocumentSerializer(ListDocumentSerializer):
"""Serialize documents with all fields for display in detail views."""
content = serializers.CharField(required=False)
class Meta:
model = models.Document
fields = [
"id",
"abilities",
"content",
"created_at", "created_at",
"link_role",
"link_reach",
"title",
"updated_at",
]
read_only_fields = [
"id",
"abilities",
"created_at",
"link_role",
"link_reach",
"updated_at", "updated_at",
] ]

18
src/backend/core/api/viewsets.py
View File

@@ -346,15 +346,23 @@ class DocumentViewSet(
): ):
"""Document ViewSet""" """Document ViewSet"""
access_model_class = models.DocumentAccess
metadata_class = DocumentMetadata
ordering = ["-updated_at"]
permission_classes = [ permission_classes = [
permissions.AccessPermission, permissions.AccessPermission,
] ]
serializer_class = serializers.DocumentSerializer
access_model_class = models.DocumentAccess
resource_field_name = "document"
queryset = models.Document.objects.all() queryset = models.Document.objects.all()
ordering = ["-updated_at"] resource_field_name = "document"
metadata_class = DocumentMetadata serializer_class = serializers.DocumentSerializer
def get_serializer_class(self):
"""
Use ListDocumentSerializer for list actions, otherwise use DocumentSerializer.
"""
if self.action == "list":
return serializers.ListDocumentSerializer
return self.serializer_class
def list(self, request, *args, **kwargs): def list(self, request, *args, **kwargs):
"""Restrict resources returned by the list endpoint""" """Restrict resources returned by the list endpoint"""

38
src/backend/core/tests/documents/test_api_documents_list.py
View File

@@ -32,7 +32,7 @@ def test_api_documents_list_anonymous(reach, role):
assert len(results) == 0 assert len(results) == 0
def test_api_documents_list_authenticated_direct(): def test_api_documents_list_authenticated_direct(django_assert_num_queries):
""" """
Authenticated users should be able to list documents they are a direct Authenticated users should be able to list documents they are a direct
owner/administrator/member of or documents that have a link reach other owner/administrator/member of or documents that have a link reach other
@@ -55,9 +55,10 @@ def test_api_documents_list_authenticated_direct():
expected_ids = {str(document.id) for document in documents} expected_ids = {str(document.id) for document in documents}
response = client.get( with django_assert_num_queries(3):
"/api/v1.0/documents/", response = client.get(
) "/api/v1.0/documents/",
)
assert response.status_code == 200 assert response.status_code == 200
results = response.json()["results"] results = response.json()["results"]
@@ -66,7 +67,9 @@ def test_api_documents_list_authenticated_direct():
assert expected_ids == results_id assert expected_ids == results_id
def test_api_documents_list_authenticated_via_team(mock_user_teams): def test_api_documents_list_authenticated_via_team(
django_assert_num_queries, mock_user_teams
):
""" """
Authenticated users should be able to list documents they are a Authenticated users should be able to list documents they are a
owner/administrator/member of via a team. owner/administrator/member of via a team.
@@ -89,7 +92,8 @@ def test_api_documents_list_authenticated_via_team(mock_user_teams):
expected_ids = {str(document.id) for document in documents_team1 + documents_team2} expected_ids = {str(document.id) for document in documents_team1 + documents_team2}
response = client.get("/api/v1.0/documents/") with django_assert_num_queries(3):
response = client.get("/api/v1.0/documents/")
assert response.status_code == 200 assert response.status_code == 200
results = response.json()["results"] results = response.json()["results"]
@@ -98,7 +102,9 @@ def test_api_documents_list_authenticated_via_team(mock_user_teams):
assert expected_ids == results_id assert expected_ids == results_id
def test_api_documents_list_authenticated_link_reach_restricted(): def test_api_documents_list_authenticated_link_reach_restricted(
django_assert_num_queries,
):
""" """
An authenticated user who has link traces to a document that is restricted should not An authenticated user who has link traces to a document that is restricted should not
see it on the list view see it on the list view
@@ -115,9 +121,10 @@ def test_api_documents_list_authenticated_link_reach_restricted():
other_document = factories.DocumentFactory(link_reach="public") other_document = factories.DocumentFactory(link_reach="public")
models.LinkTrace.objects.create(document=other_document, user=user) models.LinkTrace.objects.create(document=other_document, user=user)
response = client.get( with django_assert_num_queries(3):
"/api/v1.0/documents/", response = client.get(
) "/api/v1.0/documents/",
)
assert response.status_code == 200 assert response.status_code == 200
results = response.json()["results"] results = response.json()["results"]
@@ -127,7 +134,9 @@ def test_api_documents_list_authenticated_link_reach_restricted():
assert results[0]["id"] == str(other_document.id) assert results[0]["id"] == str(other_document.id)
def test_api_documents_list_authenticated_link_reach_public_or_authenticated(): def test_api_documents_list_authenticated_link_reach_public_or_authenticated(
django_assert_num_queries,
):
""" """
An authenticated user who has link traces to a document with public or authenticated An authenticated user who has link traces to a document with public or authenticated
link reach should see it on the list view. link reach should see it on the list view.
@@ -144,9 +153,10 @@ def test_api_documents_list_authenticated_link_reach_public_or_authenticated():
] ]
expected_ids = {str(document.id) for document in documents} expected_ids = {str(document.id) for document in documents}
response = client.get( with django_assert_num_queries(3):
"/api/v1.0/documents/", response = client.get(
) "/api/v1.0/documents/",
)
assert response.status_code == 200 assert response.status_code == 200
results = response.json()["results"] results = response.json()["results"]

257
src/backend/core/tests/documents/test_api_documents_retrieve.py
View File

@@ -36,12 +36,12 @@ def test_api_documents_retrieve_anonymous_public():
"versions_list": False, "versions_list": False,
"versions_retrieve": False, "versions_retrieve": False,
}, },
"accesses": [], "content": document.content,
"created_at": document.created_at.isoformat().replace("+00:00", "Z"),
"is_user_favorite": False,
"link_reach": "public", "link_reach": "public",
"link_role": document.link_role, "link_role": document.link_role,
"title": document.title, "title": document.title,
"content": document.content,
"created_at": document.created_at.isoformat().replace("+00:00", "Z"),
"updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"),
} }
@@ -94,7 +94,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
"versions_list": False, "versions_list": False,
"versions_retrieve": False, "versions_retrieve": False,
}, },
"accesses": [], "is_user_favorite": False,
"link_reach": reach, "link_reach": reach,
"link_role": document.link_role, "link_role": document.link_role,
"title": document.title, "title": document.title,
@@ -168,35 +168,15 @@ def test_api_documents_retrieve_authenticated_related_direct():
client.force_login(user) client.force_login(user)
document = factories.DocumentFactory() document = factories.DocumentFactory()
access1 = factories.UserDocumentAccessFactory(document=document, user=user) factories.UserDocumentAccessFactory(document=document, user=user)
access2 = factories.UserDocumentAccessFactory(document=document) access2 = factories.UserDocumentAccessFactory(document=document)
access1_user = serializers.UserSerializer(instance=user).data serializers.UserSerializer(instance=user)
access2_user = serializers.UserSerializer(instance=access2.user).data serializers.UserSerializer(instance=access2.user)
response = client.get( response = client.get(
f"/api/v1.0/documents/{document.id!s}/", f"/api/v1.0/documents/{document.id!s}/",
) )
assert response.status_code == 200 assert response.status_code == 200
content = response.json()
assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted(
[
{
"id": str(access1.id),
"user": access1_user,
"team": "",
"role": access1.role,
"abilities": access1.get_abilities(user),
},
{
"id": str(access2.id),
"user": access2_user,
"team": "",
"role": access2.role,
"abilities": access2.get_abilities(user),
},
],
key=lambda x: x["id"],
)
assert response.json() == { assert response.json() == {
"id": str(document.id), "id": str(document.id),
"title": document.title, "title": document.title,
@@ -257,7 +237,7 @@ def test_api_documents_retrieve_authenticated_related_team_members(
): ):
""" """
Authenticated users should be allowed to retrieve a document to which they Authenticated users should be allowed to retrieve a document to which they
are related via a team whatever the role and see all its accesses. are related via a team whatever the role.
""" """
mock_user_teams.return_value = teams mock_user_teams.return_value = teams
@@ -268,73 +248,23 @@ def test_api_documents_retrieve_authenticated_related_team_members(
document = factories.DocumentFactory(link_reach="restricted") document = factories.DocumentFactory(link_reach="restricted")
access_reader = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="readers", role="reader" document=document, team="readers", role="reader"
) )
access_editor = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="editors", role="editor" document=document, team="editors", role="editor"
) )
access_administrator = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="administrators", role="administrator" document=document, team="administrators", role="administrator"
) )
access_owner = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(document=document, team="owners", role="owner")
document=document, team="owners", role="owner" factories.TeamDocumentAccessFactory(document=document)
)
other_access = factories.TeamDocumentAccessFactory(document=document)
factories.TeamDocumentAccessFactory() factories.TeamDocumentAccessFactory()
response = client.get(f"/api/v1.0/documents/{document.id!s}/") response = client.get(f"/api/v1.0/documents/{document.id!s}/")
# pylint: disable=R0801 # pylint: disable=R0801
assert response.status_code == 200 assert response.status_code == 200
content = response.json()
expected_abilities = {
"destroy": False,
"retrieve": True,
"set_role_to": [],
"update": False,
"partial_update": False,
}
assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted(
[
{
"id": str(access_reader.id),
"user": None,
"team": "readers",
"role": access_reader.role,
"abilities": expected_abilities,
},
{
"id": str(access_editor.id),
"user": None,
"team": "editors",
"role": access_editor.role,
"abilities": expected_abilities,
},
{
"id": str(access_administrator.id),
"user": None,
"team": "administrators",
"role": access_administrator.role,
"abilities": expected_abilities,
},
{
"id": str(access_owner.id),
"user": None,
"team": "owners",
"role": access_owner.role,
"abilities": expected_abilities,
},
{
"id": str(other_access.id),
"user": None,
"team": other_access.team,
"role": other_access.role,
"abilities": expected_abilities,
},
],
key=lambda x: x["id"],
)
assert response.json() == { assert response.json() == {
"id": str(document.id), "id": str(document.id),
"title": document.title, "title": document.title,
@@ -360,7 +290,7 @@ def test_api_documents_retrieve_authenticated_related_team_administrators(
): ):
""" """
Authenticated users should be allowed to retrieve a document to which they Authenticated users should be allowed to retrieve a document to which they
are related via a team whatever the role and see all its accesses. are related via a team whatever the role.
""" """
mock_user_teams.return_value = teams mock_user_teams.return_value = teams
@@ -371,90 +301,23 @@ def test_api_documents_retrieve_authenticated_related_team_administrators(
document = factories.DocumentFactory(link_reach="restricted") document = factories.DocumentFactory(link_reach="restricted")
access_reader = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="readers", role="reader" document=document, team="readers", role="reader"
) )
access_editor = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="editors", role="editor" document=document, team="editors", role="editor"
) )
access_administrator = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="administrators", role="administrator" document=document, team="administrators", role="administrator"
) )
access_owner = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(document=document, team="owners", role="owner")
document=document, team="owners", role="owner" factories.TeamDocumentAccessFactory(document=document)
)
other_access = factories.TeamDocumentAccessFactory(document=document)
factories.TeamDocumentAccessFactory() factories.TeamDocumentAccessFactory()
response = client.get(f"/api/v1.0/documents/{document.id!s}/") response = client.get(f"/api/v1.0/documents/{document.id!s}/")
# pylint: disable=R0801 # pylint: disable=R0801
assert response.status_code == 200 assert response.status_code == 200
content = response.json()
assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted(
[
{
"id": str(access_reader.id),
"user": None,
"team": "readers",
"role": "reader",
"abilities": {
"destroy": True,
"retrieve": True,
"set_role_to": ["administrator", "editor"],
"update": True,
"partial_update": True,
},
},
{
"id": str(access_editor.id),
"user": None,
"team": "editors",
"role": "editor",
"abilities": {
"destroy": True,
"retrieve": True,
"set_role_to": ["administrator", "reader"],
"update": True,
"partial_update": True,
},
},
{
"id": str(access_administrator.id),
"user": None,
"team": "administrators",
"role": "administrator",
"abilities": {
"destroy": True,
"retrieve": True,
"set_role_to": ["editor", "reader"],
"update": True,
"partial_update": True,
},
},
{
"id": str(access_owner.id),
"user": None,
"team": "owners",
"role": "owner",
"abilities": {
"destroy": False,
"retrieve": True,
"set_role_to": [],
"update": False,
"partial_update": False,
},
},
{
"id": str(other_access.id),
"user": None,
"team": other_access.team,
"role": other_access.role,
"abilities": other_access.get_abilities(user),
},
],
key=lambda x: x["id"],
)
assert response.json() == { assert response.json() == {
"id": str(document.id), "id": str(document.id),
"title": document.title, "title": document.title,
@@ -481,7 +344,7 @@ def test_api_documents_retrieve_authenticated_related_team_owners(
): ):
""" """
Authenticated users should be allowed to retrieve a restricted document to which Authenticated users should be allowed to retrieve a restricted document to which
they are related via a team whatever the role and see all its accesses. they are related via a team whatever the role.
""" """
mock_user_teams.return_value = teams mock_user_teams.return_value = teams
@@ -492,93 +355,23 @@ def test_api_documents_retrieve_authenticated_related_team_owners(
document = factories.DocumentFactory(link_reach="restricted") document = factories.DocumentFactory(link_reach="restricted")
access_reader = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="readers", role="reader" document=document, team="readers", role="reader"
) )
access_editor = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="editors", role="editor" document=document, team="editors", role="editor"
) )
access_administrator = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(
document=document, team="administrators", role="administrator" document=document, team="administrators", role="administrator"
) )
access_owner = factories.TeamDocumentAccessFactory( factories.TeamDocumentAccessFactory(document=document, team="owners", role="owner")
document=document, team="owners", role="owner" factories.TeamDocumentAccessFactory(document=document)
)
other_access = factories.TeamDocumentAccessFactory(document=document)
factories.TeamDocumentAccessFactory() factories.TeamDocumentAccessFactory()
response = client.get(f"/api/v1.0/documents/{document.id!s}/") response = client.get(f"/api/v1.0/documents/{document.id!s}/")
# pylint: disable=R0801 # pylint: disable=R0801
assert response.status_code == 200 assert response.status_code == 200
content = response.json()
assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted(
[
{
"id": str(access_reader.id),
"user": None,
"team": "readers",
"role": "reader",
"abilities": {
"destroy": True,
"retrieve": True,
"set_role_to": ["owner", "administrator", "editor"],
"update": True,
"partial_update": True,
},
},
{
"id": str(access_editor.id),
"user": None,
"team": "editors",
"role": "editor",
"abilities": {
"destroy": True,
"retrieve": True,
"set_role_to": ["owner", "administrator", "reader"],
"update": True,
"partial_update": True,
},
},
{
"id": str(access_administrator.id),
"user": None,
"team": "administrators",
"role": "administrator",
"abilities": {
"destroy": True,
"retrieve": True,
"set_role_to": ["owner", "editor", "reader"],
"update": True,
"partial_update": True,
},
},
{
"id": str(access_owner.id),
"user": None,
"team": "owners",
"role": "owner",
"abilities": {
# editable only if there is another owner role than the user's team...
"destroy": other_access.role == "owner",
"retrieve": True,
"set_role_to": ["administrator", "editor", "reader"]
if other_access.role == "owner"
else [],
"update": other_access.role == "owner",
"partial_update": other_access.role == "owner",
},
},
{
"id": str(other_access.id),
"user": None,
"team": other_access.team,
"role": other_access.role,
"abilities": other_access.get_abilities(user),
},
],
key=lambda x: x["id"],
)
assert response.json() == { assert response.json() == {
"id": str(document.id), "id": str(document.id),
"title": document.title, "title": document.title,

4
src/backend/core/tests/documents/test_api_documents_update.py
View File

@@ -216,7 +216,7 @@ def test_api_documents_update_authenticated_editor_administrator_or_owner(
document = models.Document.objects.get(pk=document.pk) document = models.Document.objects.get(pk=document.pk)
document_values = serializers.DocumentSerializer(instance=document).data document_values = serializers.DocumentSerializer(instance=document).data
for key, value in document_values.items(): for key, value in document_values.items():
if key in ["id", "accesses", "created_at", "link_reach", "link_role"]: if key in ["id", "created_at", "link_reach", "link_role"]:
assert value == old_document_values[key] assert value == old_document_values[key]
elif key == "updated_at": elif key == "updated_at":
assert value > old_document_values[key] assert value > old_document_values[key]
@@ -255,7 +255,7 @@ def test_api_documents_update_authenticated_owners(via, mock_user_teams):
document = models.Document.objects.get(pk=document.pk) document = models.Document.objects.get(pk=document.pk)
document_values = serializers.DocumentSerializer(instance=document).data document_values = serializers.DocumentSerializer(instance=document).data
for key, value in document_values.items(): for key, value in document_values.items():
if key in ["id", "accesses", "created_at", "link_reach", "link_role"]: if key in ["id", "created_at", "link_reach", "link_role"]:
assert value == old_document_values[key] assert value == old_document_values[key]
elif key == "updated_at": elif key == "updated_at":
assert value > old_document_values[key] assert value > old_document_values[key]