🐛(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
This commit is contained in:
@@ -16,6 +16,7 @@ and this project adheres to
|
||||
### Fixed
|
||||
|
||||
- 🐛(backend) create a link_trace record for on-boarding documents
|
||||
- 🐛(backend) manage race condition when creating sandbox document
|
||||
|
||||
## [v4.7.0] - 2026-03-09
|
||||
|
||||
|
||||
+31
-21
@@ -19,7 +19,7 @@ from django.core.cache import cache
|
||||
from django.core.files.base import ContentFile
|
||||
from django.core.files.storage import default_storage
|
||||
from django.core.mail import send_mail
|
||||
from django.db import models, transaction
|
||||
from django.db import connection, models, transaction
|
||||
from django.db.models.functions import Left, Length
|
||||
from django.template.loader import render_to_string
|
||||
from django.utils import timezone
|
||||
@@ -265,28 +265,38 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin):
|
||||
duplicate the sandbox document for the user
|
||||
"""
|
||||
if settings.USER_ONBOARDING_SANDBOX_DOCUMENT:
|
||||
sandbox_id = settings.USER_ONBOARDING_SANDBOX_DOCUMENT
|
||||
try:
|
||||
template_document = Document.objects.get(id=sandbox_id)
|
||||
except Document.DoesNotExist:
|
||||
logger.warning(
|
||||
"Onboarding sandbox document with id %s does not exist. Skipping.",
|
||||
sandbox_id,
|
||||
# transaction.atomic is used in a context manager to avoid a transaction if
|
||||
# the settings USER_ONBOARDING_SANDBOX_DOCUMENT is unused
|
||||
with transaction.atomic():
|
||||
# locks the table to ensure safe concurrent access
|
||||
with connection.cursor() as cursor:
|
||||
cursor.execute(
|
||||
f'LOCK TABLE "{Document._meta.db_table}" ' # noqa: SLF001
|
||||
"IN SHARE ROW EXCLUSIVE MODE;"
|
||||
)
|
||||
|
||||
sandbox_id = settings.USER_ONBOARDING_SANDBOX_DOCUMENT
|
||||
try:
|
||||
template_document = Document.objects.get(id=sandbox_id)
|
||||
except Document.DoesNotExist:
|
||||
logger.warning(
|
||||
"Onboarding sandbox document with id %s does not exist. Skipping.",
|
||||
sandbox_id,
|
||||
)
|
||||
return
|
||||
|
||||
sandbox_document = template_document.add_sibling(
|
||||
"right",
|
||||
title=template_document.title,
|
||||
content=template_document.content,
|
||||
attachments=template_document.attachments,
|
||||
duplicated_from=template_document,
|
||||
creator=self,
|
||||
)
|
||||
return
|
||||
|
||||
sandbox_document = template_document.add_sibling(
|
||||
"right",
|
||||
title=template_document.title,
|
||||
content=template_document.content,
|
||||
attachments=template_document.attachments,
|
||||
duplicated_from=template_document,
|
||||
creator=self,
|
||||
)
|
||||
|
||||
DocumentAccess.objects.create(
|
||||
user=self, document=sandbox_document, role=RoleChoices.OWNER
|
||||
)
|
||||
DocumentAccess.objects.create(
|
||||
user=self, document=sandbox_document, role=RoleChoices.OWNER
|
||||
)
|
||||
|
||||
def _convert_valid_invitations(self):
|
||||
"""
|
||||
|
||||
@@ -114,7 +114,7 @@ def test_models_invitations_new_user_convert_invitations_to_accesses():
|
||||
).exists() # the other invitation remains
|
||||
|
||||
|
||||
def test_models_invitationd_new_user_filter_expired_invitations():
|
||||
def test_models_invitations_new_user_filter_expired_invitations():
|
||||
"""
|
||||
Upon creating a new identity, valid invitations should be converted into accesses
|
||||
and expired invitations should remain unchanged.
|
||||
@@ -145,7 +145,7 @@ def test_models_invitationd_new_user_filter_expired_invitations():
|
||||
|
||||
|
||||
@pytest.mark.parametrize("num_invitations, num_queries", [(0, 3), (1, 7), (20, 7)])
|
||||
def test_models_invitationd_new_userd_user_creation_constant_num_queries(
|
||||
def test_models_invitations_new_userd_user_creation_constant_num_queries(
|
||||
django_assert_num_queries, num_invitations, num_queries
|
||||
):
|
||||
"""
|
||||
|
||||
@@ -3,6 +3,7 @@ Unit tests for the User model
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
@@ -188,9 +189,8 @@ def test_models_users_handle_onboarding_documents_on_restricted_document_is_not_
|
||||
"""On-boarding document can be used when restricted"""
|
||||
|
||||
document = factories.DocumentFactory(link_reach=models.LinkReachChoices.RESTRICTED)
|
||||
settings.USER_ONBOARDING_DOCUMENTS = [str(document.id)]
|
||||
|
||||
user = factories.UserFactory()
|
||||
with override_settings(USER_ONBOARDING_DOCUMENTS=[str(document.id)]):
|
||||
user = factories.UserFactory()
|
||||
|
||||
assert not models.LinkTrace.objects.filter(user=user, document=document).exists()
|
||||
|
||||
@@ -302,3 +302,30 @@ def test_models_users_duplicate_onboarding_sandbox_document_integration_with_oth
|
||||
document=sandbox_doc, user=user, role=models.RoleChoices.OWNER
|
||||
).exists()
|
||||
assert models.LinkTrace.objects.filter(document=onboarding_doc, user=user).exists()
|
||||
|
||||
|
||||
@pytest.mark.django_db(transaction=True)
|
||||
def test_models_users_duplicate_onboarding_sandbox_race_condition():
|
||||
"""
|
||||
It should be possible to create several documents at the same time
|
||||
without causing any race conditions or data integrity issues.
|
||||
"""
|
||||
|
||||
def create_user():
|
||||
return factories.UserFactory()
|
||||
|
||||
template_document = factories.DocumentFactory(title="Getting started with Docs")
|
||||
with (
|
||||
override_settings(
|
||||
USER_ONBOARDING_SANDBOX_DOCUMENT=str(template_document.id),
|
||||
),
|
||||
ThreadPoolExecutor(max_workers=2) as executor,
|
||||
):
|
||||
future1 = executor.submit(create_user)
|
||||
future2 = executor.submit(create_user)
|
||||
|
||||
user1 = future1.result()
|
||||
user2 = future2.result()
|
||||
|
||||
assert isinstance(user1, models.User)
|
||||
assert isinstance(user2, models.User)
|
||||
|
||||
Reference in New Issue
Block a user