diff --git a/src/backend/core/models.py b/src/backend/core/models.py index e2da00912..091c0f759 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -2038,6 +2038,14 @@ class Mention(BaseModel): .exists() ) + @property + def notification_guard_key(self): + """Cache key claiming a notification slot for this mention's context.""" + return ( + "mention-notify:" + f"{self.document_id}:{self.mentioned_user_id}:{self.thread_id or ''}" + ) + def notify(self, language=None): """Send the mention notification email unless the context is in cooldown. @@ -2048,6 +2056,16 @@ class Mention(BaseModel): if user is None or not user.email or self.is_notification_in_cooldown(): return False + # Guard against concurrent notification tasks for the same context. + # cache.add is atomic on the shared Redis backend: only the first task + # acquires the key and proceeds, the others get False + if not cache.add( + self.notification_guard_key, + str(self.pk), + timeout=settings.MENTION_NOTIFICATION_COOLDOWN_MINUTES * 60, + ): + return False + sender = self.mentioned_by_user language = ( language or user.language or sender.language or settings.LANGUAGE_CODE diff --git a/src/backend/core/tests/documents/test_api_documents_mention.py b/src/backend/core/tests/documents/test_api_documents_mention.py index a139c878e..5b65b3c4f 100644 --- a/src/backend/core/tests/documents/test_api_documents_mention.py +++ b/src/backend/core/tests/documents/test_api_documents_mention.py @@ -6,6 +6,7 @@ import random from datetime import timedelta from django.core import mail +from django.core.cache import cache from django.utils import timezone import pytest @@ -446,11 +447,14 @@ def test_api_documents_mention_cooldown_expired(settings): assert response.status_code == 201 assert len(mail.outbox) == 1 - # Age the first mention beyond the cooldown period + # Age the first mention beyond the cooldown period. The concurrency guard + # key expires together with the cooldown in real time, so drop it too. + first_mention = models.Mention.objects.get() expired = timezone.now() - timedelta( minutes=settings.MENTION_NOTIFICATION_COOLDOWN_MINUTES + 1 ) models.Mention.objects.update(created_at=expired, notified_at=expired) + cache.delete(first_mention.notification_guard_key) response = client.post( f"/api/v1.0/documents/{document.id!s}/mention/", diff --git a/src/backend/core/tests/test_models_mentions.py b/src/backend/core/tests/test_models_mentions.py index b906299c7..da6197e5b 100644 --- a/src/backend/core/tests/test_models_mentions.py +++ b/src/backend/core/tests/test_models_mentions.py @@ -81,3 +81,28 @@ def test_models_mentions_notify_mentioned_user_deleted(): assert mention.notified_at is None # pylint: disable-next=no-member assert len(mail.outbox) == 0 + + +def test_models_mentions_notify_concurrent_duplicate(): + """A notification should be suppressed when the context guard is already claimed. + + Simulates two notification tasks racing on the same context: the second + task reads the database before the first one commits its `notified_at` + update, so the cache guard is the only thing preventing a duplicate email. + """ + document = factories.DocumentFactory() + mentioned_user = factories.UserFactory() + first = factories.MentionFactory(document=document, mentioned_user=mentioned_user) + second = factories.MentionFactory(document=document, mentioned_user=mentioned_user) + + assert first.notify() is True + + # Hide the first notification from the database cooldown check, as a + # concurrent task would see it before the first task commits. + models.Mention.objects.filter(pk=first.pk).update(notified_at=None) + assert second.is_notification_in_cooldown() is False + + assert second.notify() is False + assert second.notified_at is None + # pylint: disable-next=no-member + assert len(mail.outbox) == 1