mirror of
https://github.com/suitenumerique/docs.git
synced 2026-10-01 05:55:16 +02:00
🐛(backend) guard mention notifications against concurrent duplicates
Use cache.add as a distributed lock keyed by document/user/thread to ensure only the first concurrent mention sends the notification Signed-off-by: BOUKERFA Mohamed El Amine <boukerfa.ma@gmail.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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/",
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user