Skip to content

Commit 24f753e

Browse files
committed
🐛(backend) manage race condition when creating sandbox document
When a user is created and a sandbox document should be created, we can have a race condition on the document creation leading to an error for the user. To avoid this we have to manage this part in a transaction and locking the document table
1 parent 607228a commit 24f753e

File tree

4 files changed

+66
-30
lines changed

4 files changed

+66
-30
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to
99
### Fixed
1010

1111
- 🐛(backend) create a link_trace record for onboarded documents
12+
- 🐛(backend) manage race condition when creating sandbox document
1213

1314
## [v4.7.0] - 2026-03-09
1415

src/backend/core/models.py

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from django.core.files.base import ContentFile
2020
from django.core.files.storage import default_storage
2121
from django.core.mail import send_mail
22-
from django.db import models, transaction
22+
from django.db import connection, models, transaction
2323
from django.db.models.functions import Left, Length
2424
from django.template.loader import render_to_string
2525
from django.utils import timezone
@@ -260,29 +260,39 @@ def _duplicate_onboarding_sandbox_document(self):
260260
duplicate the sandbox document for the user
261261
"""
262262
if settings.USER_ONBOARDING_SANDBOX_DOCUMENT:
263-
sandbox_id = settings.USER_ONBOARDING_SANDBOX_DOCUMENT
264-
try:
265-
template_document = Document.objects.get(id=sandbox_id)
263+
# transaction.atomic is used in a context manager to avoid a transaction if
264+
# the settings USER_ONBOARDING_SANDBOX_DOCUMENT is unused
265+
with transaction.atomic():
266+
# locks the table to ensure safe concurrent access
267+
with connection.cursor() as cursor:
268+
cursor.execute(
269+
f'LOCK TABLE "{Document._meta.db_table}" ' # noqa: SLF001
270+
"IN SHARE ROW EXCLUSIVE MODE;"
271+
)
266272

267-
except Document.DoesNotExist:
268-
logger.warning(
269-
"Onboarding sandbox document with id %s does not exist. Skipping.",
270-
sandbox_id,
273+
sandbox_id = settings.USER_ONBOARDING_SANDBOX_DOCUMENT
274+
try:
275+
template_document = Document.objects.get(id=sandbox_id)
276+
277+
except Document.DoesNotExist:
278+
logger.warning(
279+
"Onboarding sandbox document with id %s does not exist. Skipping.",
280+
sandbox_id,
281+
)
282+
return
283+
284+
sandbox_document = template_document.add_sibling(
285+
"right",
286+
title=template_document.title,
287+
content=template_document.content,
288+
attachments=template_document.attachments,
289+
duplicated_from=template_document,
290+
creator=self,
271291
)
272-
return
273-
274-
sandbox_document = template_document.add_sibling(
275-
"right",
276-
title=template_document.title,
277-
content=template_document.content,
278-
attachments=template_document.attachments,
279-
duplicated_from=template_document,
280-
creator=self,
281-
)
282292

283-
DocumentAccess.objects.create(
284-
user=self, document=sandbox_document, role=RoleChoices.OWNER
285-
)
293+
DocumentAccess.objects.create(
294+
user=self, document=sandbox_document, role=RoleChoices.OWNER
295+
)
286296

287297
def _convert_valid_invitations(self):
288298
"""

src/backend/core/tests/test_models_invitations.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def test_models_invitations_is_expired():
7979
assert expired_invitation.is_expired is True
8080

8181

82-
def test_models_invitationd_new_userd_convert_invitations_to_accesses():
82+
def test_models_invitations_new_userd_convert_invitations_to_accesses():
8383
"""
8484
Upon creating a new user, invitations linked to the email
8585
should be converted to accesses and then deleted.
@@ -114,7 +114,7 @@ def test_models_invitationd_new_userd_convert_invitations_to_accesses():
114114
).exists() # the other invitation remains
115115

116116

117-
def test_models_invitationd_new_user_filter_expired_invitations():
117+
def test_models_invitations_new_user_filter_expired_invitations():
118118
"""
119119
Upon creating a new identity, valid invitations should be converted into accesses
120120
and expired invitations should remain unchanged.
@@ -145,7 +145,7 @@ def test_models_invitationd_new_user_filter_expired_invitations():
145145

146146

147147
@pytest.mark.parametrize("num_invitations, num_queries", [(0, 3), (1, 7), (20, 7)])
148-
def test_models_invitationd_new_userd_user_creation_constant_num_queries(
148+
def test_models_invitations_new_userd_user_creation_constant_num_queries(
149149
django_assert_num_queries, num_invitations, num_queries
150150
):
151151
"""

src/backend/core/tests/test_models_users.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import uuid
6+
from concurrent.futures import ThreadPoolExecutor
67
from unittest.mock import patch
78

89
from django.core.exceptions import ValidationError
@@ -184,15 +185,12 @@ def test_models_users_handle_onboarding_documents_access_duplicate_prevention():
184185
assert link_traces.count() == 1
185186

186187

187-
def test_models_users_handle_onboarding_documents_on_restricted_document_is_not_allowed(
188-
settings,
189-
):
188+
def test_models_users_handle_onboarding_documents_on_restricted_document_is_not_allowed():
190189
"""Onboarding document can be used when restricted"""
191190

192191
document = factories.DocumentFactory(link_reach=models.LinkReachChoices.RESTRICTED)
193-
settings.USER_ONBOARDING_DOCUMENTS = [str(document.id)]
194-
195-
user = factories.UserFactory()
192+
with override_settings(USER_ONBOARDING_DOCUMENTS=[str(document.id)]):
193+
user = factories.UserFactory()
196194

197195
assert not models.LinkTrace.objects.filter(user=user, document=document).exists()
198196

@@ -304,3 +302,30 @@ def test_models_users_duplicate_onboarding_sandbox_document_integration_with_oth
304302
document=sandbox_doc, user=user, role=models.RoleChoices.OWNER
305303
).exists()
306304
assert models.LinkTrace.objects.filter(document=onboarding_doc, user=user).exists()
305+
306+
307+
@pytest.mark.django_db(transaction=True)
308+
def test_models_users_duplicate_onboarding_sandbox_race_condition():
309+
"""
310+
It should be possible to create several documents at the same time
311+
without causing any race conditions or data integrity issues.
312+
"""
313+
314+
def create_user():
315+
return factories.UserFactory()
316+
317+
template_document = factories.DocumentFactory(title="Getting started with Docs")
318+
with (
319+
override_settings(
320+
USER_ONBOARDING_SANDBOX_DOCUMENT=str(template_document.id),
321+
),
322+
ThreadPoolExecutor(max_workers=2) as executor,
323+
):
324+
future1 = executor.submit(create_user)
325+
future2 = executor.submit(create_user)
326+
327+
user1 = future1.result()
328+
user2 = future2.result()
329+
330+
assert isinstance(user1, models.User)
331+
assert isinstance(user2, models.User)

0 commit comments

Comments
 (0)