From db3a17650f004491109c43df92b085ba4fc0340a Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Thu, 29 Jan 2026 08:06:46 +0600 Subject: [PATCH 01/22] implement initial pipeline for snapshot notifications --- backend/Makefile | 3 + backend/apps/owasp/admin/notification.py | 23 ++++ backend/apps/owasp/apps.py | 3 + .../commands/owasp_run_notification_worker.py | 118 ++++++++++++++++++ .../0073_notification_subscription.py | 49 ++++++++ backend/apps/owasp/models/__init__.py | 1 + backend/apps/owasp/models/notification.py | 46 +++++++ backend/apps/owasp/signals/__init__.py | 0 backend/apps/owasp/signals/snapshot.py | 16 +++ backend/apps/owasp/utils/notifications.py | 29 +++++ backend/settings/base.py | 2 + 11 files changed, 290 insertions(+) create mode 100644 backend/apps/owasp/admin/notification.py create mode 100644 backend/apps/owasp/management/commands/owasp_run_notification_worker.py create mode 100644 backend/apps/owasp/migrations/0073_notification_subscription.py create mode 100644 backend/apps/owasp/models/notification.py create mode 100644 backend/apps/owasp/signals/__init__.py create mode 100644 backend/apps/owasp/signals/snapshot.py create mode 100644 backend/apps/owasp/utils/notifications.py diff --git a/backend/Makefile b/backend/Makefile index 1411db9f60..2c0b46ee36 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -152,6 +152,9 @@ run-backend-fuzz: @COMPOSE_BAKE=true DOCKER_BUILDKIT=1 \ docker compose --project-name nest-fuzz -f docker-compose/fuzz/compose.yaml up --build --remove-orphans --abort-on-container-exit backend db cache +run-notification-worker: + @CMD="python manage.py owasp_run_notification_worker" $(MAKE) exec-backend-command + save-backup: @echo "Saving Nest backup" @CMD="python manage.py dumpdata --natural-primary --natural-foreign --indent=2" $(MAKE) exec-backend-command > backend/data/backup.json diff --git a/backend/apps/owasp/admin/notification.py b/backend/apps/owasp/admin/notification.py new file mode 100644 index 0000000000..81ee0ab382 --- /dev/null +++ b/backend/apps/owasp/admin/notification.py @@ -0,0 +1,23 @@ +"""Admin registration for notification models.""" + +from django.contrib import admin +from apps.owasp.models.notification import Subscription, Notification + + +@admin.register(Subscription) +class SubscriptionAdmin(admin.ModelAdmin): + """Admin for Subscription model.""" + + list_display = ("user", "content_type", "object_id", "content_object", "created_at") + list_filter = ("content_type", "created_at") + search_fields = ("user__email", "user__username") + + +@admin.register(Notification) +class NotificationAdmin(admin.ModelAdmin): + """Admin for Notification model.""" + + list_display = ("recipient", "type", "title", "is_read", "created_at") + list_filter = ("type", "is_read", "created_at") + search_fields = ("recipient__email", "recipient__username", "title", "message") + readonly_fields = ("created_at",) diff --git a/backend/apps/owasp/apps.py b/backend/apps/owasp/apps.py index c004421c5b..8ea08f7d1f 100644 --- a/backend/apps/owasp/apps.py +++ b/backend/apps/owasp/apps.py @@ -7,3 +7,6 @@ class OwaspConfig(AppConfig): """Owasp app config.""" name = "apps.owasp" + + def ready(self): + import apps.owasp.signals.snapshot # noqa: F401 diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py new file mode 100644 index 0000000000..9a708bc1a6 --- /dev/null +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -0,0 +1,118 @@ +"""Management command to run notification worker.""" + +import logging +import time + +from django.conf import settings +from django.contrib.contenttypes.models import ContentType +from django.core.mail import send_mail +from django.core.management.base import BaseCommand +from django_redis import get_redis_connection + +from apps.owasp.models.notification import Notification, Subscription +from apps.owasp.models.snapshot import Snapshot + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + """Run notification worker.""" + + help = "Run notification worker to process Redis stream messages." + + def handle(self, *args, **options): + """Handle execution.""" + self.stdout.write("Starting notification worker...") + redis_conn = get_redis_connection("default") + stream_key = "owasp_notifications" + group_name = "notification_group" + consumer_name = "worker_1" + + # Create consumer group if it doesn't exist + try: + redis_conn.xgroup_create(stream_key, group_name, id="0", mkstream=True) + print("Consumer group created successfully.") + except Exception: + # Group likely already exists + pass + + while True: + try: + # Read new messages specifically for this group + # ">" means "messages never delivered to other consumers so far" + events = redis_conn.xreadgroup(group_name, consumer_name, {stream_key: ">"}, count=1, block=5000) + print("Events: ", events) + if not events: + continue + + for _, messages in events: + for message_id, data in messages: + try: + self.process_message(data) + # Explicitly acknowledge the message + redis_conn.xack(stream_key, group_name, message_id) + print("Message processed successfully.") + except Exception: + logger.exception(f"Error processing message {message_id}") + + except Exception: + logger.exception("Error reading from stream group") + time.sleep(1) + + def process_message(self, data): + """Process a single message from the stream.""" + msg_type = data.get(b"type", b"").decode("utf-8") + if msg_type == "snapshot_published": + self.handle_snapshot_published(data) + + def handle_snapshot_published(self, data): + """Handle snapshot published event.""" + try: + snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) + snapshot = Snapshot.objects.get(id=snapshot_id) + + # Find subscribers + snapshot_ct = ContentType.objects.get_for_model(Snapshot) + + subscriptions = Subscription.objects.filter( + content_type=snapshot_ct, + object_id=0 + ) + + if not subscriptions.exists(): + logger.info("No subscribers found for Snapshot updates.") + return + + for sub in subscriptions: + self.send_notification(sub.user, snapshot) + + except Snapshot.DoesNotExist: + logger.error(f"Snapshot matching ID {snapshot_id} not found.") + except Exception: + logger.exception("Error handling snapshot published event") + + def send_notification(self, user, snapshot): + """Send notification to user.""" + title = f"New Snapshot Published: {snapshot.title}" + message = f"Check out the latest OWASP snapshot: {snapshot.title}" + + # Create DB record + Notification.objects.create( + recipient=user, + type="snapshot_published", + title=title, + message=message, + ) + + # Send Email + try: + send_mail( + subject=title, + message=message, + from_email="noreply@owasp.org", + recipient_list=[user.email], + fail_silently=False, + ) + logger.info(f"Sent email to {user.email}") + except Exception: + logger.exception(f"Failed to send email to {user.email}") diff --git a/backend/apps/owasp/migrations/0073_notification_subscription.py b/backend/apps/owasp/migrations/0073_notification_subscription.py new file mode 100644 index 0000000000..286b433341 --- /dev/null +++ b/backend/apps/owasp/migrations/0073_notification_subscription.py @@ -0,0 +1,49 @@ +# Generated by Django 6.0.1 on 2026-01-28 23:49 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + ('owasp', '0072_project_project_name_gin_idx_and_more'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='Notification', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('type', models.CharField(max_length=50)), + ('title', models.CharField(max_length=255)), + ('message', models.TextField()), + ('related_link', models.URLField(blank=True, null=True)), + ('is_read', models.BooleanField(default=False)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('recipient', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='notifications', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'db_table': 'owasp_notifications', + 'ordering': ['-created_at'], + }, + ), + migrations.CreateModel( + name='Subscription', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('object_id', models.PositiveIntegerField()), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.contenttype')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='subscriptions', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'db_table': 'owasp_subscriptions', + 'indexes': [models.Index(fields=['user', 'content_type', 'object_id'], name='owasp_subsc_user_id_33ae6d_idx')], + 'unique_together': {('user', 'content_type', 'object_id')}, + }, + ), + ] diff --git a/backend/apps/owasp/models/__init__.py b/backend/apps/owasp/models/__init__.py index 3cbb120b8b..a85fe0cac9 100644 --- a/backend/apps/owasp/models/__init__.py +++ b/backend/apps/owasp/models/__init__.py @@ -12,3 +12,4 @@ from .project_health_requirements import ProjectHealthRequirements from .snapshot import Snapshot from .sponsor import Sponsor +from .notification import Subscription, Notification diff --git a/backend/apps/owasp/models/notification.py b/backend/apps/owasp/models/notification.py new file mode 100644 index 0000000000..ddf3900970 --- /dev/null +++ b/backend/apps/owasp/models/notification.py @@ -0,0 +1,46 @@ +"""Notification and Subscription models.""" + +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType +from django.db import models + +from apps.nest.models import User + + +class Subscription(models.Model): + """Model representing a user's subscription to a specific entity.""" + + user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="subscriptions") + content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey("content_type", "object_id") + created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + db_table = "owasp_subscriptions" + unique_together = ("user", "content_type", "object_id") + indexes = [ + models.Index(fields=["user", "content_type", "object_id"]), + ] + + def __str__(self): + return f"{self.user} -> {self.content_object}" + + +class Notification(models.Model): + """Model representing a notification sent to a user.""" + + recipient = models.ForeignKey(User, on_delete=models.CASCADE, related_name="notifications") + type = models.CharField(max_length=50) # e.g., 'snapshot_published', 'release', etc. + title = models.CharField(max_length=255) + message = models.TextField() + related_link = models.URLField(blank=True, null=True) + is_read = models.BooleanField(default=False) + created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + db_table = "owasp_notifications" + ordering = ["-created_at"] + + def __str__(self): + return f"{self.title} -> {self.recipient}" diff --git a/backend/apps/owasp/signals/__init__.py b/backend/apps/owasp/signals/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/apps/owasp/signals/snapshot.py b/backend/apps/owasp/signals/snapshot.py new file mode 100644 index 0000000000..409c45d4d1 --- /dev/null +++ b/backend/apps/owasp/signals/snapshot.py @@ -0,0 +1,16 @@ +"""Snapshot signals.""" + +from django.db.models.signals import post_save +from django.dispatch import receiver + +from apps.owasp.models.snapshot import Snapshot +from apps.owasp.utils.notifications import publish_snapshot_notification + + +@receiver(post_save, sender=Snapshot) +def snapshot_published(sender, instance, created, **kwargs): + """Signal handler for snapshot publication.""" + print(f"Signal fired for Snapshot {instance.id}, Status: {instance.status}") + if instance.status == Snapshot.Status.COMPLETED: + print(f"Publishing notification for Snapshot {instance.id}") + publish_snapshot_notification(instance) diff --git a/backend/apps/owasp/utils/notifications.py b/backend/apps/owasp/utils/notifications.py new file mode 100644 index 0000000000..d64bdd19d6 --- /dev/null +++ b/backend/apps/owasp/utils/notifications.py @@ -0,0 +1,29 @@ +"""Notification utils.""" + +import logging +from datetime import datetime + +from django.conf import settings +from django.utils.timezone import now +from django_redis import get_redis_connection + +from apps.owasp.models.snapshot import Snapshot + +logger = logging.getLogger(__name__) + + +def publish_snapshot_notification(snapshot: Snapshot) -> None: + """Publish a notification for a published snapshot.""" + try: + redis_conn = get_redis_connection("default") + stream_key = "owasp_notifications" + message = { + "type": "snapshot_published", + "snapshot_id": str(snapshot.id), + "timestamp": str(now().timestamp()), + } + redis_conn.xadd(stream_key, message) + print(f"Successfully added message to Redis stream '{stream_key}'") + logger.info(f"Published snapshot notification for snapshot {snapshot.id}") + except Exception: + logger.exception(f"Failed to publish snapshot notification for snapshot {snapshot.id}") diff --git a/backend/settings/base.py b/backend/settings/base.py index 748615dab3..200850c991 100644 --- a/backend/settings/base.py +++ b/backend/settings/base.py @@ -231,3 +231,5 @@ class Base(Configuration): SLACK_COMMANDS_ENABLED = True SLACK_EVENTS_ENABLED = True SLACK_SIGNING_SECRET = values.SecretValue() + + EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" From de606f428b68fa7435d6a84bcfae4cc55ad67a35 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sat, 31 Jan 2026 09:12:44 +0600 Subject: [PATCH 02/22] notify all users on snapshot publish --- .../commands/owasp_run_notification_worker.py | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 9a708bc1a6..0601371b61 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -3,13 +3,12 @@ import logging import time -from django.conf import settings -from django.contrib.contenttypes.models import ContentType from django.core.mail import send_mail from django.core.management.base import BaseCommand from django_redis import get_redis_connection -from apps.owasp.models.notification import Notification, Subscription +from apps.nest.models import User +from apps.owasp.models.notification import Notification from apps.owasp.models.snapshot import Snapshot logger = logging.getLogger(__name__) @@ -71,20 +70,16 @@ def handle_snapshot_published(self, data): snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) snapshot = Snapshot.objects.get(id=snapshot_id) - # Find subscribers - snapshot_ct = ContentType.objects.get_for_model(Snapshot) - - subscriptions = Subscription.objects.filter( - content_type=snapshot_ct, - object_id=0 - ) + users = User.objects.filter(is_active=True) - if not subscriptions.exists(): - logger.info("No subscribers found for Snapshot updates.") + if not users.exists(): + logger.info("No active users found.") return - for sub in subscriptions: - self.send_notification(sub.user, snapshot) + logger.info(f"Sending snapshot notification to {users.count()} users") + + for user in users: + self.send_notification(user, snapshot) except Snapshot.DoesNotExist: logger.error(f"Snapshot matching ID {snapshot_id} not found.") From 27b29846c28682aea9f2568071db7762bca60bec Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Mon, 2 Feb 2026 09:03:37 +0600 Subject: [PATCH 03/22] feat: add exponential backoff retry and DLQ storage to notification worker --- .../commands/owasp_run_notification_worker.py | 83 ++++++++++++++++--- 1 file changed, 71 insertions(+), 12 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 0601371b61..ef8fe4927d 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -19,6 +19,12 @@ class Command(BaseCommand): help = "Run notification worker to process Redis stream messages." + # Retry configuration + MAX_RETRIES = 5 + BASE_DELAY = 2 # seconds + DELAY_MULTIPLIER = 2 + DLQ_STREAM_KEY = "owasp_notifications_dlq" + def handle(self, *args, **options): """Handle execution.""" self.stdout.write("Starting notification worker...") @@ -66,6 +72,8 @@ def process_message(self, data): def handle_snapshot_published(self, data): """Handle snapshot published event.""" + redis_conn = get_redis_connection("default") + try: snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) snapshot = Snapshot.objects.get(id=snapshot_id) @@ -78,14 +86,68 @@ def handle_snapshot_published(self, data): logger.info(f"Sending snapshot notification to {users.count()} users") + failed_users = [] + for user in users: - self.send_notification(user, snapshot) + success = self.send_notification_with_retry(user, snapshot) + if not success: + failed_users.append({ + 'user_id': str(user.id), + 'email': user.email, + 'snapshot_id': str(snapshot_id) + }) + + # Send failed users to DLQ + if failed_users: + for failed_user in failed_users: + dlq_message = { + 'type': 'failed_notification', + 'user_id': failed_user['user_id'], + 'email': failed_user['email'], + 'snapshot_id': failed_user['snapshot_id'], + 'retry_count': str(self.MAX_RETRIES), + 'timestamp': str(time.time()), + 'last_attempt': str(time.time()) + } + redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_message) + + logger.warning(f"Sent {len(failed_users)} failed notifications to DLQ") except Snapshot.DoesNotExist: logger.error(f"Snapshot matching ID {snapshot_id} not found.") except Exception: logger.exception("Error handling snapshot published event") + def send_notification_with_retry(self, user, snapshot): + """Send notification with exponential backoff retry logic.""" + retry_count = 0 + last_error = None + + while retry_count <= self.MAX_RETRIES: + try: + self.send_notification(user, snapshot) + if retry_count > 0: + logger.info(f"Email to {user.email} succeeded after {retry_count} retries") + return True + except Exception as e: + retry_count += 1 + last_error = e + if retry_count <= self.MAX_RETRIES: + delay = self.BASE_DELAY * (self.DELAY_MULTIPLIER ** (retry_count - 1)) + logger.warning( + f"Email to {user.email} failed (attempt {retry_count}/{self.MAX_RETRIES}). " + f"Retrying in {delay}s. Error: {str(e)}" + ) + time.sleep(delay) + else: + logger.error( + f"Email to {user.email} failed after {self.MAX_RETRIES} retries. " + f"Error: {str(last_error)}" + ) + return False + + return False + def send_notification(self, user, snapshot): """Send notification to user.""" title = f"New Snapshot Published: {snapshot.title}" @@ -100,14 +162,11 @@ def send_notification(self, user, snapshot): ) # Send Email - try: - send_mail( - subject=title, - message=message, - from_email="noreply@owasp.org", - recipient_list=[user.email], - fail_silently=False, - ) - logger.info(f"Sent email to {user.email}") - except Exception: - logger.exception(f"Failed to send email to {user.email}") + send_mail( + subject=title, + message=message, + from_email="noreply@owasp.org", + recipient_list=[user.email], + fail_silently=False, + ) + logger.info(f"Sent email to {user.email}") From 81053836c1d86b4f8d116595e7cfab63aa585e7a Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Wed, 4 Feb 2026 10:06:35 +0600 Subject: [PATCH 04/22] feat:enhance notification worker reliability and DLQ logic --- backend/apps/owasp/admin/notification.py | 3 +- backend/apps/owasp/apps.py | 1 + .../commands/owasp_run_notification_worker.py | 198 +++++++++++++----- .../0073_notification_subscription.py | 79 +++++-- backend/apps/owasp/models/__init__.py | 2 +- backend/apps/owasp/models/notification.py | 6 +- backend/apps/owasp/signals/snapshot.py | 4 +- backend/apps/owasp/utils/notifications.py | 10 +- cspell/custom-dict.txt | 6 + 9 files changed, 216 insertions(+), 93 deletions(-) diff --git a/backend/apps/owasp/admin/notification.py b/backend/apps/owasp/admin/notification.py index 81ee0ab382..2962e8c332 100644 --- a/backend/apps/owasp/admin/notification.py +++ b/backend/apps/owasp/admin/notification.py @@ -1,7 +1,8 @@ """Admin registration for notification models.""" from django.contrib import admin -from apps.owasp.models.notification import Subscription, Notification + +from apps.owasp.models.notification import Notification, Subscription @admin.register(Subscription) diff --git a/backend/apps/owasp/apps.py b/backend/apps/owasp/apps.py index 8ea08f7d1f..c6f2323380 100644 --- a/backend/apps/owasp/apps.py +++ b/backend/apps/owasp/apps.py @@ -9,4 +9,5 @@ class OwaspConfig(AppConfig): name = "apps.owasp" def ready(self): + """Import signals when app is ready.""" import apps.owasp.signals.snapshot # noqa: F401 diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index ef8fe4927d..fccd9d4fdb 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -24,46 +24,71 @@ class Command(BaseCommand): BASE_DELAY = 2 # seconds DELAY_MULTIPLIER = 2 DLQ_STREAM_KEY = "owasp_notifications_dlq" + DLQ_CHECK_INTERVAL = 300 # seconds def handle(self, *args, **options): """Handle execution.""" self.stdout.write("Starting notification worker...") + count = 0 redis_conn = get_redis_connection("default") stream_key = "owasp_notifications" group_name = "notification_group" consumer_name = "worker_1" - # Create consumer group if it doesn't exist - try: - redis_conn.xgroup_create(stream_key, group_name, id="0", mkstream=True) - print("Consumer group created successfully.") - except Exception: - # Group likely already exists - pass + self.ensure_consumer_group(redis_conn, stream_key, group_name) + + last_dlq_check = time.time() while True: try: # Read new messages specifically for this group # ">" means "messages never delivered to other consumers so far" - events = redis_conn.xreadgroup(group_name, consumer_name, {stream_key: ">"}, count=1, block=5000) + events = redis_conn.xreadgroup( + group_name, + consumer_name, + {stream_key: ">"}, + count=1, + block=5000, + ) print("Events: ", events) - if not events: - continue - - for _, messages in events: - for message_id, data in messages: - try: - self.process_message(data) - # Explicitly acknowledge the message - redis_conn.xack(stream_key, group_name, message_id) - print("Message processed successfully.") - except Exception: - logger.exception(f"Error processing message {message_id}") - - except Exception: - logger.exception("Error reading from stream group") + count += 1 + print(f"count:{count}") + # Process main stream messages + if events: + for _, messages in events: + for message_id, data in messages: + try: + self.process_message(data) + # Explicitly acknowledge the message + redis_conn.xack(stream_key, group_name, message_id) + print("Message processed successfully.") + except Exception: + logger.exception("Error processing message %s", message_id) + + # Check DLQ every 300 seconds + if time.time() - last_dlq_check > self.DLQ_CHECK_INTERVAL: + self.process_dlq(redis_conn) + last_dlq_check = time.time() + + except Exception as e: + if "NOGROUP" in str(e): + logger.warning("Consumer group missing, attempting to recreate...") + self.ensure_consumer_group(redis_conn, stream_key, group_name) + else: + logger.exception("Error reading from stream group") time.sleep(1) + def ensure_consumer_group(self, redis_conn, stream_key, group_name): + """Ensure the consumer group exists.""" + try: + redis_conn.xgroup_create(stream_key, group_name, id="0", mkstream=True) + self.stdout.write(self.style.SUCCESS(f"Consumer group '{group_name}' created.")) + except Exception as e: # noqa: BLE001 + if "BUSYGROUP" in str(e): + self.stdout.write(f"Consumer group '{group_name}' already exists.") + else: + self.stdout.write(self.style.ERROR(f"Error creating group: {e}")) + def process_message(self, data): """Process a single message from the stream.""" msg_type = data.get(b"type", b"").decode("utf-8") @@ -73,48 +98,50 @@ def process_message(self, data): def handle_snapshot_published(self, data): """Handle snapshot published event.""" redis_conn = get_redis_connection("default") - + try: snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) snapshot = Snapshot.objects.get(id=snapshot_id) - + users = User.objects.filter(is_active=True) - + if not users.exists(): logger.info("No active users found.") return - logger.info(f"Sending snapshot notification to {users.count()} users") - + logger.info("Sending snapshot notification to %d users", users.count()) + failed_users = [] - + for user in users: success = self.send_notification_with_retry(user, snapshot) if not success: - failed_users.append({ - 'user_id': str(user.id), - 'email': user.email, - 'snapshot_id': str(snapshot_id) - }) - + failed_users.append( + { + "user_id": str(user.id), + "email": user.email, + "snapshot_id": str(snapshot_id), + } + ) + # Send failed users to DLQ if failed_users: for failed_user in failed_users: dlq_message = { - 'type': 'failed_notification', - 'user_id': failed_user['user_id'], - 'email': failed_user['email'], - 'snapshot_id': failed_user['snapshot_id'], - 'retry_count': str(self.MAX_RETRIES), - 'timestamp': str(time.time()), - 'last_attempt': str(time.time()) + "type": "failed_notification", + "user_id": failed_user["user_id"], + "email": failed_user["email"], + "snapshot_id": failed_user["snapshot_id"], + "retry_count": str(self.MAX_RETRIES), + "timestamp": str(time.time()), + "last_attempt": str(time.time()), } redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_message) - - logger.warning(f"Sent {len(failed_users)} failed notifications to DLQ") + + logger.warning("Sent %d failed notifications to DLQ", len(failed_users)) except Snapshot.DoesNotExist: - logger.error(f"Snapshot matching ID {snapshot_id} not found.") + logger.exception("Snapshot matching ID %s not found.", snapshot_id) except Exception: logger.exception("Error handling snapshot published event") @@ -122,37 +149,47 @@ def send_notification_with_retry(self, user, snapshot): """Send notification with exponential backoff retry logic.""" retry_count = 0 last_error = None - + while retry_count <= self.MAX_RETRIES: try: self.send_notification(user, snapshot) - if retry_count > 0: - logger.info(f"Email to {user.email} succeeded after {retry_count} retries") - return True except Exception as e: retry_count += 1 last_error = e if retry_count <= self.MAX_RETRIES: delay = self.BASE_DELAY * (self.DELAY_MULTIPLIER ** (retry_count - 1)) logger.warning( - f"Email to {user.email} failed (attempt {retry_count}/{self.MAX_RETRIES}). " - f"Retrying in {delay}s. Error: {str(e)}" + "Email to %s failed (attempt %d/%d). Retrying in %ds. Error: %s", + user.email, + retry_count, + self.MAX_RETRIES, + delay, + last_error, ) time.sleep(delay) else: - logger.error( - f"Email to {user.email} failed after {self.MAX_RETRIES} retries. " - f"Error: {str(last_error)}" + logger.exception( + "Email to %s failed after %d retries", + user.email, + self.MAX_RETRIES, ) return False - + else: + if retry_count > 0: + logger.info( + "Email to %s succeeded after %d retries", + user.email, + retry_count, + ) + return True + return False def send_notification(self, user, snapshot): """Send notification to user.""" title = f"New Snapshot Published: {snapshot.title}" message = f"Check out the latest OWASP snapshot: {snapshot.title}" - + # Create DB record Notification.objects.create( recipient=user, @@ -169,4 +206,51 @@ def send_notification(self, user, snapshot): recipient_list=[user.email], fail_silently=False, ) - logger.info(f"Sent email to {user.email}") + logger.info("Sent email to %s", user.email) + + def process_dlq(self, redis_conn): + """Process messages from DLQ - retry failed notifications.""" + self.stdout.write("Checking DLQ for failed notifications...") + + try: + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") + + if not messages: + self.stdout.write("No messages in DLQ") + return + + processed_count = 0 + failed_count = 0 + + for msg_id, data in messages: + try: + user_id = int(data.get(b"user_id").decode("utf-8")) + snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) + + user = User.objects.get(id=user_id) + snapshot = Snapshot.objects.get(id=snapshot_id) + + self.send_notification(user, snapshot) + + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + processed_count += 1 + logger.info( + "Successfully reprocessed notification for user %s", + user.email, + ) + + except Exception: + failed_count += 1 + logger.exception( + "Failed to reprocess DLQ message %s - keeping in DLQ", + msg_id, + ) + + logger.info( + "DLQ processing complete: %d successful, %d failed", + processed_count, + failed_count, + ) + + except Exception: + logger.exception("Error processing DLQ") diff --git a/backend/apps/owasp/migrations/0073_notification_subscription.py b/backend/apps/owasp/migrations/0073_notification_subscription.py index 286b433341..fa5f403f68 100644 --- a/backend/apps/owasp/migrations/0073_notification_subscription.py +++ b/backend/apps/owasp/migrations/0073_notification_subscription.py @@ -6,44 +6,77 @@ class Migration(migrations.Migration): - dependencies = [ - ('contenttypes', '0002_remove_content_type_name'), - ('owasp', '0072_project_project_name_gin_idx_and_more'), + ("contenttypes", "0002_remove_content_type_name"), + ("owasp", "0072_project_project_name_gin_idx_and_more"), migrations.swappable_dependency(settings.AUTH_USER_MODEL), ] operations = [ migrations.CreateModel( - name='Notification', + name="Notification", fields=[ - ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('type', models.CharField(max_length=50)), - ('title', models.CharField(max_length=255)), - ('message', models.TextField()), - ('related_link', models.URLField(blank=True, null=True)), - ('is_read', models.BooleanField(default=False)), - ('created_at', models.DateTimeField(auto_now_add=True)), - ('recipient', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='notifications', to=settings.AUTH_USER_MODEL)), + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ("type", models.CharField(max_length=50)), + ("title", models.CharField(max_length=255)), + ("message", models.TextField()), + ("related_link", models.URLField(blank=True, null=True)), + ("is_read", models.BooleanField(default=False)), + ("created_at", models.DateTimeField(auto_now_add=True)), + ( + "recipient", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="notifications", + to=settings.AUTH_USER_MODEL, + ), + ), ], options={ - 'db_table': 'owasp_notifications', - 'ordering': ['-created_at'], + "db_table": "owasp_notifications", + "ordering": ["-created_at"], }, ), migrations.CreateModel( - name='Subscription', + name="Subscription", fields=[ - ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('object_id', models.PositiveIntegerField()), - ('created_at', models.DateTimeField(auto_now_add=True)), - ('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.contenttype')), - ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='subscriptions', to=settings.AUTH_USER_MODEL)), + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ("object_id", models.PositiveIntegerField()), + ("created_at", models.DateTimeField(auto_now_add=True)), + ( + "content_type", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="contenttypes.contenttype" + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="subscriptions", + to=settings.AUTH_USER_MODEL, + ), + ), ], options={ - 'db_table': 'owasp_subscriptions', - 'indexes': [models.Index(fields=['user', 'content_type', 'object_id'], name='owasp_subsc_user_id_33ae6d_idx')], - 'unique_together': {('user', 'content_type', 'object_id')}, + "db_table": "owasp_subscriptions", + "indexes": [ + models.Index( + fields=["user", "content_type", "object_id"], + name="owasp_subsc_user_id_33ae6d_idx", + ) + ], + "unique_together": {("user", "content_type", "object_id")}, }, ), ] diff --git a/backend/apps/owasp/models/__init__.py b/backend/apps/owasp/models/__init__.py index a85fe0cac9..6fbe39b8b6 100644 --- a/backend/apps/owasp/models/__init__.py +++ b/backend/apps/owasp/models/__init__.py @@ -6,10 +6,10 @@ from .event import Event from .member_profile import MemberProfile from .member_snapshot import MemberSnapshot +from .notification import Notification, Subscription from .post import Post from .project import Project from .project_health_metrics import ProjectHealthMetrics from .project_health_requirements import ProjectHealthRequirements from .snapshot import Snapshot from .sponsor import Sponsor -from .notification import Subscription, Notification diff --git a/backend/apps/owasp/models/notification.py b/backend/apps/owasp/models/notification.py index ddf3900970..ad0c69bebe 100644 --- a/backend/apps/owasp/models/notification.py +++ b/backend/apps/owasp/models/notification.py @@ -23,7 +23,7 @@ class Meta: models.Index(fields=["user", "content_type", "object_id"]), ] - def __str__(self): + def __str__(self): # noqa: D105 return f"{self.user} -> {self.content_object}" @@ -34,7 +34,7 @@ class Notification(models.Model): type = models.CharField(max_length=50) # e.g., 'snapshot_published', 'release', etc. title = models.CharField(max_length=255) message = models.TextField() - related_link = models.URLField(blank=True, null=True) + related_link = models.URLField(blank=True) is_read = models.BooleanField(default=False) created_at = models.DateTimeField(auto_now_add=True) @@ -42,5 +42,5 @@ class Meta: db_table = "owasp_notifications" ordering = ["-created_at"] - def __str__(self): + def __str__(self): # noqa: D105 return f"{self.title} -> {self.recipient}" diff --git a/backend/apps/owasp/signals/snapshot.py b/backend/apps/owasp/signals/snapshot.py index 409c45d4d1..1041b0834f 100644 --- a/backend/apps/owasp/signals/snapshot.py +++ b/backend/apps/owasp/signals/snapshot.py @@ -8,9 +8,7 @@ @receiver(post_save, sender=Snapshot) -def snapshot_published(sender, instance, created, **kwargs): +def snapshot_published(sender, instance, created, **kwargs): # noqa: ARG001 """Signal handler for snapshot publication.""" - print(f"Signal fired for Snapshot {instance.id}, Status: {instance.status}") if instance.status == Snapshot.Status.COMPLETED: - print(f"Publishing notification for Snapshot {instance.id}") publish_snapshot_notification(instance) diff --git a/backend/apps/owasp/utils/notifications.py b/backend/apps/owasp/utils/notifications.py index d64bdd19d6..4afaab36f3 100644 --- a/backend/apps/owasp/utils/notifications.py +++ b/backend/apps/owasp/utils/notifications.py @@ -1,9 +1,7 @@ """Notification utils.""" import logging -from datetime import datetime -from django.conf import settings from django.utils.timezone import now from django_redis import get_redis_connection @@ -23,7 +21,9 @@ def publish_snapshot_notification(snapshot: Snapshot) -> None: "timestamp": str(now().timestamp()), } redis_conn.xadd(stream_key, message) - print(f"Successfully added message to Redis stream '{stream_key}'") - logger.info(f"Published snapshot notification for snapshot {snapshot.id}") + logger.info("Published snapshot notification for snapshot %s", snapshot.id) except Exception: - logger.exception(f"Failed to publish snapshot notification for snapshot {snapshot.id}") + logger.exception( + "Failed to publish snapshot notification for snapshot %s", + snapshot.id, + ) diff --git a/cspell/custom-dict.txt b/cspell/custom-dict.txt index a75033249e..0a2940bd00 100644 --- a/cspell/custom-dict.txt +++ b/cspell/custom-dict.txt @@ -9,6 +9,7 @@ Birda CCSP CISSP Cañón +DLQ DRF ELEVENLABS FOSS @@ -78,6 +79,7 @@ cva defectdojo demojize dismissable +dlq dockerhub dsn elevenlabs @@ -179,10 +181,14 @@ vcodec webgoat winsrdf wsgi +xack xapp +xdel xdg xdist +xgroup xoxb +xreadgroup xsser xzf zapconfig From 1fecdc7a6d766292e84a8792c7c0177dbe34bbd4 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Wed, 4 Feb 2026 10:52:07 +0600 Subject: [PATCH 05/22] adress coderabbit and cubic --- .../commands/owasp_run_notification_worker.py | 24 +++++++++---------- ...owasp_subsc_user_id_33ae6d_idx_and_more.py | 22 +++++++++++++++++ backend/apps/owasp/models/notification.py | 5 +--- 3 files changed, 35 insertions(+), 16 deletions(-) create mode 100644 backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index fccd9d4fdb..b2ac96a015 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -50,9 +50,9 @@ def handle(self, *args, **options): count=1, block=5000, ) - print("Events: ", events) + logger.info("Events: %s", events) count += 1 - print(f"count:{count}") + logger.info("count:%s", count) # Process main stream messages if events: for _, messages in events: @@ -61,7 +61,7 @@ def handle(self, *args, **options): self.process_message(data) # Explicitly acknowledge the message redis_conn.xack(stream_key, group_name, message_id) - print("Message processed successfully.") + logger.info("Message processed successfully.") except Exception: logger.exception("Error processing message %s", message_id) @@ -190,14 +190,6 @@ def send_notification(self, user, snapshot): title = f"New Snapshot Published: {snapshot.title}" message = f"Check out the latest OWASP snapshot: {snapshot.title}" - # Create DB record - Notification.objects.create( - recipient=user, - type="snapshot_published", - title=title, - message=message, - ) - # Send Email send_mail( subject=title, @@ -208,12 +200,20 @@ def send_notification(self, user, snapshot): ) logger.info("Sent email to %s", user.email) + # Create DB record + Notification.objects.create( + recipient=user, + type="snapshot_published", + title=title, + message=message, + ) + def process_dlq(self, redis_conn): """Process messages from DLQ - retry failed notifications.""" self.stdout.write("Checking DLQ for failed notifications...") try: - messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+", count=100) if not messages: self.stdout.write("No messages in DLQ") diff --git a/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py b/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py new file mode 100644 index 0000000000..202547dfdf --- /dev/null +++ b/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py @@ -0,0 +1,22 @@ +# Generated by Django 6.0.1 on 2026-02-04 04:46 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('owasp', '0073_notification_subscription'), + ] + + operations = [ + migrations.RemoveIndex( + model_name='subscription', + name='owasp_subsc_user_id_33ae6d_idx', + ), + migrations.AlterField( + model_name='notification', + name='related_link', + field=models.URLField(blank=True, default=''), + ), + ] diff --git a/backend/apps/owasp/models/notification.py b/backend/apps/owasp/models/notification.py index ad0c69bebe..62edb246e7 100644 --- a/backend/apps/owasp/models/notification.py +++ b/backend/apps/owasp/models/notification.py @@ -19,9 +19,6 @@ class Subscription(models.Model): class Meta: db_table = "owasp_subscriptions" unique_together = ("user", "content_type", "object_id") - indexes = [ - models.Index(fields=["user", "content_type", "object_id"]), - ] def __str__(self): # noqa: D105 return f"{self.user} -> {self.content_object}" @@ -34,7 +31,7 @@ class Notification(models.Model): type = models.CharField(max_length=50) # e.g., 'snapshot_published', 'release', etc. title = models.CharField(max_length=255) message = models.TextField() - related_link = models.URLField(blank=True) + related_link = models.URLField(blank=True, default="") is_read = models.BooleanField(default=False) created_at = models.DateTimeField(auto_now_add=True) From 409e1d39f60b076f4dfe56e6ee08ddcb13498995 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Thu, 5 Feb 2026 08:02:57 +0600 Subject: [PATCH 06/22] fix(worker): prevent duplicate emails and recover stuck messages --- backend/apps/owasp/admin/notification.py | 2 +- .../commands/owasp_run_notification_worker.py | 43 ++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/backend/apps/owasp/admin/notification.py b/backend/apps/owasp/admin/notification.py index 2962e8c332..9f84a25d63 100644 --- a/backend/apps/owasp/admin/notification.py +++ b/backend/apps/owasp/admin/notification.py @@ -9,7 +9,7 @@ class SubscriptionAdmin(admin.ModelAdmin): """Admin for Subscription model.""" - list_display = ("user", "content_type", "object_id", "content_object", "created_at") + list_display = ("user", "content_type", "object_id", "created_at") list_filter = ("content_type", "created_at") search_fields = ("user__email", "user__username") diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index b2ac96a015..43b65a5d4a 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -2,6 +2,8 @@ import logging import time +import os +import socket from django.core.mail import send_mail from django.core.management.base import BaseCommand @@ -33,10 +35,12 @@ def handle(self, *args, **options): redis_conn = get_redis_connection("default") stream_key = "owasp_notifications" group_name = "notification_group" - consumer_name = "worker_1" + consumer_name = f"{socket.gethostname()}_{os.getpid()}" self.ensure_consumer_group(redis_conn, stream_key, group_name) + self.recover_pending_messages(redis_conn, stream_key, group_name, consumer_name) + last_dlq_check = time.time() while True: @@ -100,7 +104,10 @@ def handle_snapshot_published(self, data): redis_conn = get_redis_connection("default") try: - snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) + raw_id = data.get(b"snapshot_id") + if not raw_id: + return + snapshot_id = int(raw_id.decode("utf-8")) snapshot = Snapshot.objects.get(id=snapshot_id) users = User.objects.filter(is_active=True) @@ -188,6 +195,11 @@ def send_notification_with_retry(self, user, snapshot): def send_notification(self, user, snapshot): """Send notification to user.""" title = f"New Snapshot Published: {snapshot.title}" + + if Notification.objects.filter(recipient=user, title=title).exists(): + logger.info("Already notified %s for this snapshot, skipping", user.email) + return + message = f"Check out the latest OWASP snapshot: {snapshot.title}" # Send Email @@ -254,3 +266,30 @@ def process_dlq(self, redis_conn): except Exception: logger.exception("Error processing DLQ") + + + + def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_name): + """Recover and reprocess stuck messages from PEL.""" + self.stdout.write("Checking for stuck messages in PEL...") + try: + # Claim messages idle for more than 5 minutes (300000 ms) + result = redis_conn.xautoclaim( + stream_key, group_name, consumer_name, + min_idle_time=300000, # 5 minutes + start_id="0-0", + count=10 + ) + if result and result[1]: + for message_id, data in result[1]: + self.stdout.write(f"Recovering stuck message: {message_id}") + try: + self.process_message(data) + redis_conn.xack(stream_key, group_name, message_id) + self.stdout.write(f"Successfully recovered message {message_id}") + except Exception: + logger.exception("Failed to recover message %s", message_id) + else: + self.stdout.write("No stuck messages found.") + except Exception: + logger.exception("Error checking PEL for stuck messages") \ No newline at end of file From 262eee8a63adb27737c7f73dcd64f2915dfe24fd Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Thu, 5 Feb 2026 08:38:59 +0600 Subject: [PATCH 07/22] adress code rabit --- .../commands/owasp_run_notification_worker.py | 34 ++++++++++++------- ...owasp_subsc_user_id_33ae6d_idx_and_more.py | 13 ++++--- cspell/custom-dict.txt | 4 ++- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 43b65a5d4a..70f2422c6a 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -1,9 +1,9 @@ """Management command to run notification worker.""" import logging -import time import os import socket +import time from django.core.mail import send_mail from django.core.management.base import BaseCommand @@ -106,7 +106,7 @@ def handle_snapshot_published(self, data): try: raw_id = data.get(b"snapshot_id") if not raw_id: - return + return snapshot_id = int(raw_id.decode("utf-8")) snapshot = Snapshot.objects.get(id=snapshot_id) @@ -197,7 +197,7 @@ def send_notification(self, user, snapshot): title = f"New Snapshot Published: {snapshot.title}" if Notification.objects.filter(recipient=user, title=title).exists(): - logger.info("Already notified %s for this snapshot, skipping", user.email) + logger.info("Already notified %s for this snapshot, skipping", user.email) return message = f"Check out the latest OWASP snapshot: {snapshot.title}" @@ -236,8 +236,18 @@ def process_dlq(self, redis_conn): for msg_id, data in messages: try: - user_id = int(data.get(b"user_id").decode("utf-8")) - snapshot_id = int(data.get(b"snapshot_id").decode("utf-8")) + raw_user_id = data.get(b"user_id") + raw_snapshot_id = data.get(b"snapshot_id") + + if not raw_user_id or not raw_snapshot_id: + logger.warning( + "DLQ message %s missing required fields, removing", msg_id + ) + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + continue + + user_id = int(raw_user_id.decode("utf-8")) + snapshot_id = int(raw_snapshot_id.decode("utf-8")) user = User.objects.get(id=user_id) snapshot = Snapshot.objects.get(id=snapshot_id) @@ -267,20 +277,20 @@ def process_dlq(self, redis_conn): except Exception: logger.exception("Error processing DLQ") - - def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_name): """Recover and reprocess stuck messages from PEL.""" self.stdout.write("Checking for stuck messages in PEL...") try: # Claim messages idle for more than 5 minutes (300000 ms) result = redis_conn.xautoclaim( - stream_key, group_name, consumer_name, - min_idle_time=300000, # 5 minutes + stream_key, + group_name, + consumer_name, + min_idle_time=300000, # 5 minutes start_id="0-0", - count=10 + count=10, ) - if result and result[1]: + if result and result[1]: for message_id, data in result[1]: self.stdout.write(f"Recovering stuck message: {message_id}") try: @@ -292,4 +302,4 @@ def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_ else: self.stdout.write("No stuck messages found.") except Exception: - logger.exception("Error checking PEL for stuck messages") \ No newline at end of file + logger.exception("Error checking PEL for stuck messages") diff --git a/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py b/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py index 202547dfdf..a7f7232d35 100644 --- a/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py +++ b/backend/apps/owasp/migrations/0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more.py @@ -4,19 +4,18 @@ class Migration(migrations.Migration): - dependencies = [ - ('owasp', '0073_notification_subscription'), + ("owasp", "0073_notification_subscription"), ] operations = [ migrations.RemoveIndex( - model_name='subscription', - name='owasp_subsc_user_id_33ae6d_idx', + model_name="subscription", + name="owasp_subsc_user_id_33ae6d_idx", ), migrations.AlterField( - model_name='notification', - name='related_link', - field=models.URLField(blank=True, default=''), + model_name="notification", + name="related_link", + field=models.URLField(blank=True, default=""), ), ] diff --git a/cspell/custom-dict.txt b/cspell/custom-dict.txt index d279c300b3..f3e264a629 100644 --- a/cspell/custom-dict.txt +++ b/cspell/custom-dict.txt @@ -144,6 +144,7 @@ owasppcitoolkit owtf pdfgen pdfium +pel pentest pentesting pgvector @@ -186,13 +187,14 @@ winsrdf wsgi xack xapp +xautoclaim xdel xdg xdist xgroup xoxb -xreadgroup xoxp +xreadgroup xsser xzf zapconfig From 042d95df9d16d1fc73b31eeec678455768d30133 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Thu, 5 Feb 2026 12:08:49 +0600 Subject: [PATCH 08/22] feat:improve notification worker robustness --- .../commands/owasp_run_notification_worker.py | 47 ++++++++++++++++--- .../0075_alter_subscription_object_id.py | 18 +++++++ backend/apps/owasp/models/notification.py | 2 +- 3 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 backend/apps/owasp/migrations/0075_alter_subscription_object_id.py diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 70f2422c6a..69416fa7cc 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -1,10 +1,12 @@ """Management command to run notification worker.""" +import contextlib import logging import os import socket import time +from django.core.exceptions import ObjectDoesNotExist from django.core.mail import send_mail from django.core.management.base import BaseCommand from django_redis import get_redis_connection @@ -195,8 +197,13 @@ def send_notification_with_retry(self, user, snapshot): def send_notification(self, user, snapshot): """Send notification to user.""" title = f"New Snapshot Published: {snapshot.title}" + related_link = f"snapshot:{snapshot.id}" - if Notification.objects.filter(recipient=user, title=title).exists(): + if Notification.objects.filter( + recipient=user, + type="snapshot_published", + related_link=related_link, + ).exists(): logger.info("Already notified %s for this snapshot, skipping", user.email) return @@ -218,13 +225,17 @@ def send_notification(self, user, snapshot): type="snapshot_published", title=title, message=message, + related_link=related_link, ) def process_dlq(self, redis_conn): """Process messages from DLQ - retry failed notifications.""" - self.stdout.write("Checking DLQ for failed notifications...") + lock = redis_conn.lock("owasp_notifications_dlq_lock", timeout=60, blocking=False) + if not lock.acquire(): + return try: + self.stdout.write("Checking DLQ for failed notifications...") messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+", count=100) if not messages: @@ -236,13 +247,21 @@ def process_dlq(self, redis_conn): for msg_id, data in messages: try: + msg_type = (data.get(b"type") or b"").decode("utf-8") + if msg_type == "recovery_failed": + logger.error( + "Permanent recovery failure for message %s: %s", + (data.get(b"message_id") or b"unknown").decode("utf-8"), + (data.get(b"error") or b"unknown").decode("utf-8"), + ) + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + continue + raw_user_id = data.get(b"user_id") raw_snapshot_id = data.get(b"snapshot_id") if not raw_user_id or not raw_snapshot_id: - logger.warning( - "DLQ message %s missing required fields, removing", msg_id - ) + logger.warning("DLQ message %s missing required fields, removing", msg_id) redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) continue @@ -261,6 +280,10 @@ def process_dlq(self, redis_conn): user.email, ) + except ObjectDoesNotExist: + logger.warning("User or snapshot not found, removing from DLQ") + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + failed_count += 1 except Exception: failed_count += 1 logger.exception( @@ -276,6 +299,9 @@ def process_dlq(self, redis_conn): except Exception: logger.exception("Error processing DLQ") + finally: + with contextlib.suppress(Exception): + lock.release() def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_name): """Recover and reprocess stuck messages from PEL.""" @@ -297,8 +323,17 @@ def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_ self.process_message(data) redis_conn.xack(stream_key, group_name, message_id) self.stdout.write(f"Successfully recovered message {message_id}") - except Exception: + except Exception as exc: logger.exception("Failed to recover message %s", message_id) + redis_conn.xadd( + self.DLQ_STREAM_KEY, + { + "type": "recovery_failed", + "message_id": str(message_id), + "error": str(exc), + }, + ) + redis_conn.xack(stream_key, group_name, message_id) else: self.stdout.write("No stuck messages found.") except Exception: diff --git a/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py b/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py new file mode 100644 index 0000000000..b52466e443 --- /dev/null +++ b/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py @@ -0,0 +1,18 @@ +# Generated by Django 6.0.1 on 2026-02-05 05:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('owasp', '0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='subscription', + name='object_id', + field=models.PositiveBigIntegerField(), + ), + ] diff --git a/backend/apps/owasp/models/notification.py b/backend/apps/owasp/models/notification.py index 62edb246e7..1d8353d2dd 100644 --- a/backend/apps/owasp/models/notification.py +++ b/backend/apps/owasp/models/notification.py @@ -12,7 +12,7 @@ class Subscription(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="subscriptions") content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) - object_id = models.PositiveIntegerField() + object_id = models.PositiveBigIntegerField() content_object = GenericForeignKey("content_type", "object_id") created_at = models.DateTimeField(auto_now_add=True) From cd92fd0e75e41b6821f6a98baea160ab19c3d309 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Fri, 6 Feb 2026 09:10:09 +0600 Subject: [PATCH 09/22] feat: implement robust notification recovery and transition-aware signals --- .../commands/owasp_run_notification_worker.py | 35 ++++++++++++++----- .../0075_alter_subscription_object_id.py | 7 ++-- backend/apps/owasp/signals/snapshot.py | 17 +++++++-- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 69416fa7cc..75c0f7f7ff 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -27,6 +27,7 @@ class Command(BaseCommand): MAX_RETRIES = 5 BASE_DELAY = 2 # seconds DELAY_MULTIPLIER = 2 + MAX_DLQ_RETRIES = 5 DLQ_STREAM_KEY = "owasp_notifications_dlq" DLQ_CHECK_INTERVAL = 300 # seconds @@ -68,7 +69,7 @@ def handle(self, *args, **options): # Explicitly acknowledge the message redis_conn.xack(stream_key, group_name, message_id) logger.info("Message processed successfully.") - except Exception: + except Exception: # noqa: BLE001 logger.exception("Error processing message %s", message_id) # Check DLQ every 300 seconds @@ -200,7 +201,7 @@ def send_notification(self, user, snapshot): related_link = f"snapshot:{snapshot.id}" if Notification.objects.filter( - recipient=user, + recipient_id=user.id, type="snapshot_published", related_link=related_link, ).exists(): @@ -257,6 +258,9 @@ def process_dlq(self, redis_conn): redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) continue + raw_dlq_retries = data.get(b"dlq_retries", b"0") + dlq_retries = int(raw_dlq_retries.decode("utf-8")) + raw_user_id = data.get(b"user_id") raw_snapshot_id = data.get(b"snapshot_id") @@ -272,7 +276,6 @@ def process_dlq(self, redis_conn): snapshot = Snapshot.objects.get(id=snapshot_id) self.send_notification(user, snapshot) - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) processed_count += 1 logger.info( @@ -284,12 +287,28 @@ def process_dlq(self, redis_conn): logger.warning("User or snapshot not found, removing from DLQ") redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) failed_count += 1 - except Exception: + except Exception: # noqa: BLE001 failed_count += 1 - logger.exception( - "Failed to reprocess DLQ message %s - keeping in DLQ", - msg_id, - ) + dlq_retries += 1 + + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + + if dlq_retries > self.MAX_DLQ_RETRIES: + logger.exception( + "DLQ message %s exceeded max retries (%d). Dropping.", + msg_id, + self.MAX_DLQ_RETRIES, + ) + else: + new_data = {k.decode("utf-8"): v.decode("utf-8") for k, v in data.items()} + new_data["dlq_retries"] = str(dlq_retries) + redis_conn.xadd(self.DLQ_STREAM_KEY, new_data) + logger.warning( + "Failed to reprocess DLQ message %s (attempt %d/%d). Re-queued.", + msg_id, + dlq_retries, + self.MAX_DLQ_RETRIES, + ) logger.info( "DLQ processing complete: %d successful, %d failed", diff --git a/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py b/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py index b52466e443..c54a10ca93 100644 --- a/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py +++ b/backend/apps/owasp/migrations/0075_alter_subscription_object_id.py @@ -4,15 +4,14 @@ class Migration(migrations.Migration): - dependencies = [ - ('owasp', '0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more'), + ("owasp", "0074_remove_subscription_owasp_subsc_user_id_33ae6d_idx_and_more"), ] operations = [ migrations.AlterField( - model_name='subscription', - name='object_id', + model_name="subscription", + name="object_id", field=models.PositiveBigIntegerField(), ), ] diff --git a/backend/apps/owasp/signals/snapshot.py b/backend/apps/owasp/signals/snapshot.py index 1041b0834f..f8b283dd28 100644 --- a/backend/apps/owasp/signals/snapshot.py +++ b/backend/apps/owasp/signals/snapshot.py @@ -1,14 +1,27 @@ """Snapshot signals.""" -from django.db.models.signals import post_save +from django.db.models.signals import post_save, pre_save from django.dispatch import receiver from apps.owasp.models.snapshot import Snapshot from apps.owasp.utils.notifications import publish_snapshot_notification +@receiver(pre_save, sender=Snapshot) +def snapshot_pre_save(sender, instance, **kwargs): # noqa: ARG001 + """Store the previous status before saving.""" + if instance.pk: + instance._previous_status = ( # noqa: SLF001 + Snapshot.objects.filter(pk=instance.pk).values_list("status", flat=True).first() + ) + else: + instance._previous_status = None # noqa: SLF001 + + @receiver(post_save, sender=Snapshot) def snapshot_published(sender, instance, created, **kwargs): # noqa: ARG001 """Signal handler for snapshot publication.""" - if instance.status == Snapshot.Status.COMPLETED: + if instance.status == Snapshot.Status.COMPLETED and ( + created or instance._previous_status != Snapshot.Status.COMPLETED # noqa: SLF001 + ): publish_snapshot_notification(instance) From a83a18da57188c23e3911ad3971210b5b2ffeffb Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sat, 7 Feb 2026 07:33:16 +0600 Subject: [PATCH 10/22] add unit test --- .../commands/owasp_run_notification_worker.py | 25 +-- backend/settings/base.py | 4 +- .../owasp_run_notification_worker_test.py | 196 ++++++++++++++++++ 3 files changed, 210 insertions(+), 15 deletions(-) create mode 100644 backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 75c0f7f7ff..357050d13c 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -6,6 +6,7 @@ import socket import time +from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.mail import send_mail from django.core.management.base import BaseCommand @@ -34,7 +35,6 @@ class Command(BaseCommand): def handle(self, *args, **options): """Handle execution.""" self.stdout.write("Starting notification worker...") - count = 0 redis_conn = get_redis_connection("default") stream_key = "owasp_notifications" group_name = "notification_group" @@ -57,9 +57,6 @@ def handle(self, *args, **options): count=1, block=5000, ) - logger.info("Events: %s", events) - count += 1 - logger.info("count:%s", count) # Process main stream messages if events: for _, messages in events: @@ -69,7 +66,7 @@ def handle(self, *args, **options): # Explicitly acknowledge the message redis_conn.xack(stream_key, group_name, message_id) logger.info("Message processed successfully.") - except Exception: # noqa: BLE001 + except Exception: logger.exception("Error processing message %s", message_id) # Check DLQ every 300 seconds @@ -140,11 +137,9 @@ def handle_snapshot_published(self, data): dlq_message = { "type": "failed_notification", "user_id": failed_user["user_id"], - "email": failed_user["email"], "snapshot_id": failed_user["snapshot_id"], - "retry_count": str(self.MAX_RETRIES), "timestamp": str(time.time()), - "last_attempt": str(time.time()), + "dlq_retries": "0", } redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_message) @@ -198,7 +193,7 @@ def send_notification_with_retry(self, user, snapshot): def send_notification(self, user, snapshot): """Send notification to user.""" title = f"New Snapshot Published: {snapshot.title}" - related_link = f"snapshot:{snapshot.id}" + related_link = f"{settings.SITE_URL}/community/snapshots/{snapshot.id}" if Notification.objects.filter( recipient_id=user.id, @@ -259,8 +254,10 @@ def process_dlq(self, redis_conn): continue raw_dlq_retries = data.get(b"dlq_retries", b"0") - dlq_retries = int(raw_dlq_retries.decode("utf-8")) - + try: + dlq_retries = int(raw_dlq_retries.decode("utf-8")) + except (ValueError, UnicodeDecodeError, AttributeError): + dlq_retries = 0 raw_user_id = data.get(b"user_id") raw_snapshot_id = data.get(b"snapshot_id") @@ -287,13 +284,12 @@ def process_dlq(self, redis_conn): logger.warning("User or snapshot not found, removing from DLQ") redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) failed_count += 1 - except Exception: # noqa: BLE001 + except Exception: failed_count += 1 dlq_retries += 1 - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - if dlq_retries > self.MAX_DLQ_RETRIES: + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) logger.exception( "DLQ message %s exceeded max retries (%d). Dropping.", msg_id, @@ -303,6 +299,7 @@ def process_dlq(self, redis_conn): new_data = {k.decode("utf-8"): v.decode("utf-8") for k, v in data.items()} new_data["dlq_retries"] = str(dlq_retries) redis_conn.xadd(self.DLQ_STREAM_KEY, new_data) + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) logger.warning( "Failed to reprocess DLQ message %s (attempt %d/%d). Re-queued.", msg_id, diff --git a/backend/settings/base.py b/backend/settings/base.py index 120f5f12b7..ca96c28917 100644 --- a/backend/settings/base.py +++ b/backend/settings/base.py @@ -230,4 +230,6 @@ class Base(Configuration): SLACK_EVENTS_ENABLED = True SLACK_SIGNING_SECRET = values.SecretValue() - EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" + EMAIL_BACKEND = values.Value( + environ_name="EMAIL_BACKEND", default="django.core.mail.backends.console.EmailBackend" + ) diff --git a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py new file mode 100644 index 0000000000..aace7205ea --- /dev/null +++ b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py @@ -0,0 +1,196 @@ +from unittest import mock +import pytest +from django.core.exceptions import ObjectDoesNotExist +from apps.owasp.management.commands.owasp_run_notification_worker import Command +from apps.nest.models import User +from apps.owasp.models.notification import Notification +from apps.owasp.models.snapshot import Snapshot + +class TestOwaspRunNotificationWorker: + @pytest.fixture + def command(self): + return Command() + + @pytest.fixture + def mock_user(self): + user = mock.MagicMock(spec=User) + user.email = "test@example.com" + user.id = 123 + return user + + @pytest.fixture + def mock_snapshot(self): + snapshot = mock.MagicMock(spec=Snapshot) + snapshot.title = "Test Snapshot" + snapshot.id = 456 + return snapshot + + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.send_mail") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Notification") + def test_send_notification_success(self, mock_notification, mock_send_mail, command, mock_user, mock_snapshot): + """Test successful notification sending.""" + mock_notification.objects.filter.return_value.exists.return_value = False + + command.send_notification(mock_user, mock_snapshot) + + mock_send_mail.assert_called_once() + mock_notification.objects.create.assert_called_once_with( + recipient=mock_user, + type="snapshot_published", + title=f"New Snapshot Published: {mock_snapshot.title}", + message=f"Check out the latest OWASP snapshot: {mock_snapshot.title}", + related_link=f"http://localhost:8000/community/snapshots/{mock_snapshot.id}", + ) + + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.send_mail") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Notification") + def test_send_notification_idempotency(self, mock_notification, mock_send_mail, command, mock_user, mock_snapshot): + """Test that notification is skipped if it already exists.""" + mock_notification.objects.filter.return_value.exists.return_value = True + + command.send_notification(mock_user, mock_snapshot) + + mock_send_mail.assert_not_called() + mock_notification.objects.create.assert_not_called() + + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Snapshot") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Command.send_notification") + def test_process_dlq_success(self, mock_send_notify, mock_snapshot_model, mock_user_model, command, mock_user, mock_snapshot): + """Test successful DLQ processing.""" + redis_conn = mock.Mock() + redis_conn.lock.return_value.acquire.return_value = True + + # Mock message data + redis_conn.xrange.return_value = [ + (b"123-0", {b"user_id": b"123", b"snapshot_id": b"456"}) + ] + + mock_user_model.objects.get.return_value = mock_user + mock_snapshot_model.objects.get.return_value = mock_snapshot + + command.process_dlq(redis_conn) + + mock_send_notify.assert_called_once_with(mock_user, mock_snapshot) + redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") + + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") + def test_process_dlq_missing_object(self, mock_user, command): + """Test DLQ processing handles ObjectDoesNotExist by removing message.""" + redis_conn = mock.Mock() + redis_conn.lock.return_value.acquire.return_value = True + + redis_conn.xrange.return_value = [ + (b"123-0", {b"user_id": b"999", b"snapshot_id": b"456"}) + ] + + mock_user.objects.get.side_effect = ObjectDoesNotExist + + command.process_dlq(redis_conn) + + redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") + + def test_process_dlq_missing_fields(self, command): + """Test DLQ processing handles messages with missing fields.""" + redis_conn = mock.Mock() + redis_conn.lock.return_value.acquire.return_value = True + + # Missing user_id + redis_conn.xrange.return_value = [ + (b"123-0", {b"snapshot_id": b"456"}) + ] + + command.process_dlq(redis_conn) + + redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") + + def test_process_dlq_recovery_failure(self, command): + """Test DLQ processing handles recovery_failed messages by removing them.""" + redis_conn = mock.Mock() + redis_conn.lock.return_value.acquire.return_value = True + + redis_conn.xrange.return_value = [ + (b"123-0", { + b"type": b"recovery_failed", + b"message_id": b"original-999", + b"error": b"Something went wrong" + }) + ] + + command.process_dlq(redis_conn) + + redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") + + @mock.patch.object(Command, "process_message") + def test_recover_pending_messages(self, mock_process, command): + """Test recovery of pending messages.""" + redis_conn = mock.Mock() + redis_conn.xautoclaim.return_value = ( + b"0-0", + [ + (b"123-0", {b"data": b"test"}) + ], + [] + ) + + command.recover_pending_messages(redis_conn, "stream", "group", "consumer") + + mock_process.assert_called_once() + redis_conn.xack.assert_called_once_with("stream", "group", b"123-0") + + @mock.patch.object(Command, "process_message") + def test_recover_pending_messages_failure(self, mock_process, command): + """Test recovery failure moves message to DLQ.""" + redis_conn = mock.Mock() + redis_conn.xautoclaim.return_value = ( + b"0-0", + [(b"123-0", {b"data": b"test"})], + [] + ) + + mock_process.side_effect = Exception("Boom") + + command.recover_pending_messages(redis_conn, "stream", "group", "consumer") + + assert redis_conn.xadd.called + assert redis_conn.xadd.call_args[0][0] == command.DLQ_STREAM_KEY + redis_conn.xack.assert_called_once() + + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") + def test_process_dlq_retry_increment(self, mock_user, command): + """Test that DLQ processing increments retry count and re-adds on failure.""" + redis_conn = mock.Mock() + redis_conn.lock.return_value.acquire.return_value = True + + redis_conn.xrange.return_value = [ + (b"123-0", {b"user_id": b"123", b"snapshot_id": b"456", b"dlq_retries": b"1"}) + ] + + + mock_user.objects.get.side_effect = Exception("Temp Error") + + command.process_dlq(redis_conn) + + # Should be deleted and re-added with count 2 + redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") + assert redis_conn.xadd.called + new_msg = redis_conn.xadd.call_args[0][1] + assert new_msg["dlq_retries"] == "2" + + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") + def test_process_dlq_max_retries_exceeded(self, mock_user, command): + """Test that DLQ processing drops message after max retries.""" + redis_conn = mock.Mock() + redis_conn.lock.return_value.acquire.return_value = True + + # Already at max retries (5) + redis_conn.xrange.return_value = [ + (b"123-0", {b"user_id": b"123", b"snapshot_id": b"456", b"dlq_retries": b"5"}) + ] + + mock_user.objects.get.side_effect = Exception("Final Error") + + command.process_dlq(redis_conn) + + redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") + assert redis_conn.xadd.call_count == 0 From c6b47f2f639bfe9ed685a812090bbd1fa1565841 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sat, 7 Feb 2026 09:17:27 +0600 Subject: [PATCH 11/22] fix sonarcloud issue --- .../owasp/management/commands/owasp_run_notification_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 357050d13c..f35e4ae564 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -256,7 +256,7 @@ def process_dlq(self, redis_conn): raw_dlq_retries = data.get(b"dlq_retries", b"0") try: dlq_retries = int(raw_dlq_retries.decode("utf-8")) - except (ValueError, UnicodeDecodeError, AttributeError): + except (ValueError, AttributeError): dlq_retries = 0 raw_user_id = data.get(b"user_id") raw_snapshot_id = data.get(b"snapshot_id") From 3780cf8d78783fae6ee46ae41e97af22080ddab7 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sat, 7 Feb 2026 10:09:49 +0600 Subject: [PATCH 12/22] adress coderabbit issues --- .../management/commands/owasp_run_notification_worker.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index f35e4ae564..b708c50427 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -226,13 +226,13 @@ def send_notification(self, user, snapshot): def process_dlq(self, redis_conn): """Process messages from DLQ - retry failed notifications.""" - lock = redis_conn.lock("owasp_notifications_dlq_lock", timeout=60, blocking=False) + lock = redis_conn.lock("owasp_notifications_dlq_lock", timeout=600, blocking=False) if not lock.acquire(): return try: self.stdout.write("Checking DLQ for failed notifications...") - messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+", count=100) + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+", count=50) if not messages: self.stdout.write("No messages in DLQ") @@ -243,6 +243,7 @@ def process_dlq(self, redis_conn): for msg_id, data in messages: try: + dlq_retries = 0 msg_type = (data.get(b"type") or b"").decode("utf-8") if msg_type == "recovery_failed": logger.error( From a5ddf381252dc64a726af63fd159828dea70a87c Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sat, 7 Feb 2026 10:42:00 +0600 Subject: [PATCH 13/22] adress coderabbit --- .../owasp/management/commands/owasp_run_notification_worker.py | 2 +- .../management/commands/owasp_run_notification_worker_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index b708c50427..376b241019 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -193,7 +193,7 @@ def send_notification_with_retry(self, user, snapshot): def send_notification(self, user, snapshot): """Send notification to user.""" title = f"New Snapshot Published: {snapshot.title}" - related_link = f"{settings.SITE_URL}/community/snapshots/{snapshot.id}" + related_link = f"{settings.SITE_URL}/community/snapshots/{snapshot.key}" if Notification.objects.filter( recipient_id=user.id, diff --git a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py index 731d60f1c8..dc539cee97 100644 --- a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py +++ b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py @@ -25,6 +25,7 @@ def mock_snapshot(self): snapshot = mock.MagicMock(spec=Snapshot) snapshot.title = "Test Snapshot" snapshot.id = 456 + snapshot.key = "2025-02" return snapshot @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.send_mail") @@ -45,7 +46,7 @@ def test_send_notification_success( type="snapshot_published", title=f"New Snapshot Published: {mock_snapshot.title}", message=f"Check out the latest OWASP snapshot: {mock_snapshot.title}", - related_link=f"https://example.com/community/snapshots/{mock_snapshot.id}", + related_link=f"https://example.com/community/snapshots/{mock_snapshot.key}", ) @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.send_mail") From 9350dfd5fd489750ef3a077a658861d5752433ad Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Thu, 26 Feb 2026 11:02:17 +0600 Subject: [PATCH 14/22] feat(notifications): add idempotent processing and DLQ management - Add deadline reminder emails with days_remaining (7, 3, 1 days) - Implement meaningful field change detection for entity updates - Add message-based idempotency to prevent duplicate notifications - Replace automatic DLQ processing with manual management commands - Add signal handlers for chapter and event change detection - Update tests for new functionality --- backend/Makefile | 2 +- backend/apps/owasp/Makefile | 4 + backend/apps/owasp/apps.py | 2 + .../commands/owasp_check_event_deadlines.py | 40 ++ .../commands/owasp_notification_dlq.py | 176 ++++++++ .../commands/owasp_run_notification_worker.py | 401 +++++++++++------- backend/apps/owasp/signals/chapter.py | 43 ++ backend/apps/owasp/signals/event.py | 50 +++ backend/apps/owasp/utils/notifications.py | 78 +++- .../owasp_check_event_deadlines_test.py | 83 ++++ .../owasp_run_notification_worker_test.py | 336 ++++++++++----- backend/tests/apps/owasp/signals/__init__.py | 0 .../tests/apps/owasp/signals/chapter_test.py | 38 ++ .../tests/apps/owasp/signals/event_test.py | 40 ++ .../apps/owasp/utils/notifications_test.py | 132 ++++++ cron/production | 1 + 16 files changed, 1153 insertions(+), 273 deletions(-) create mode 100644 backend/apps/owasp/management/commands/owasp_check_event_deadlines.py create mode 100644 backend/apps/owasp/management/commands/owasp_notification_dlq.py create mode 100644 backend/apps/owasp/signals/chapter.py create mode 100644 backend/apps/owasp/signals/event.py create mode 100644 backend/tests/apps/owasp/management/commands/owasp_check_event_deadlines_test.py create mode 100644 backend/tests/apps/owasp/signals/__init__.py create mode 100644 backend/tests/apps/owasp/signals/chapter_test.py create mode 100644 backend/tests/apps/owasp/signals/event_test.py create mode 100644 backend/tests/apps/owasp/utils/notifications_test.py diff --git a/backend/Makefile b/backend/Makefile index ddd2abe4a0..73d0859186 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -157,7 +157,7 @@ run-backend-fuzz: docker compose --project-name nest-fuzz -f docker-compose/fuzz/compose.yaml up --build --remove-orphans --abort-on-container-exit backend db cache run-notification-worker: - @CMD="python manage.py owasp_run_notification_worker" $(MAKE) exec-backend-command + @CMD="python manage.py owasp_run_notification_worker" $(MAKE) exec-backend-command-it save-backup: @echo "Saving Nest backup" diff --git a/backend/apps/owasp/Makefile b/backend/apps/owasp/Makefile index 7c5f7733eb..a717cfe06e 100644 --- a/backend/apps/owasp/Makefile +++ b/backend/apps/owasp/Makefile @@ -11,6 +11,10 @@ owasp-aggregate-member-contributions: @echo "Aggregating OWASP community member contributions" @CMD="python manage.py owasp_aggregate_member_contributions" $(MAKE) exec-backend-command +owasp-check-event-deadlines: + @echo "Checking OWASP event deadlines" + @CMD="python manage.py owasp_check_event_deadlines" $(MAKE) exec-backend-command + owasp-create-project-metadata-file: @echo "Generating metadata" @CMD="python manage.py owasp_create_project_metadata_file $(entity_key)" $(MAKE) exec-backend-command diff --git a/backend/apps/owasp/apps.py b/backend/apps/owasp/apps.py index c6f2323380..403184215f 100644 --- a/backend/apps/owasp/apps.py +++ b/backend/apps/owasp/apps.py @@ -10,4 +10,6 @@ class OwaspConfig(AppConfig): def ready(self): """Import signals when app is ready.""" + import apps.owasp.signals.chapter + import apps.owasp.signals.event import apps.owasp.signals.snapshot # noqa: F401 diff --git a/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py b/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py new file mode 100644 index 0000000000..7279c7d114 --- /dev/null +++ b/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py @@ -0,0 +1,40 @@ +"""Management command to check for approaching event deadlines.""" + +import logging + +from django.core.management.base import BaseCommand +from django.utils import timezone + +from apps.owasp.models.event import Event +from apps.owasp.utils.notifications import publish_event_notification + +logger = logging.getLogger(__name__) + +REMINDER_DAYS = (7, 3, 1) + + +class Command(BaseCommand): + """Check for events with approaching deadlines and queue reminder notifications.""" + + help = "Check for events with approaching deadlines and send reminder notifications." + + def handle(self, *args, **options): + """Handle execution.""" + self.stdout.write("Checking for approaching event deadlines...") + today = timezone.now().date() + total_reminders = 0 + + for days in REMINDER_DAYS: + target_date = today + timezone.timedelta(days=days) + events = Event.objects.filter(start_date=target_date) + + for event in events: + self.stdout.write( + f" Event '{event.name}' starts in {days} days ({target_date})" + ) + publish_event_notification(event, "deadline_reminder", days_remaining=days) + total_reminders += 1 + + self.stdout.write( + self.style.SUCCESS(f"Queued {total_reminders} deadline reminder(s).") + ) diff --git a/backend/apps/owasp/management/commands/owasp_notification_dlq.py b/backend/apps/owasp/management/commands/owasp_notification_dlq.py new file mode 100644 index 0000000000..c798a3f57e --- /dev/null +++ b/backend/apps/owasp/management/commands/owasp_notification_dlq.py @@ -0,0 +1,176 @@ +"""Management command to manage notification DLQ.""" + +import sys +import time + +from django.conf import settings +from django.core.mail import send_mail +from django.core.management.base import BaseCommand +from django_redis import get_redis_connection + + +class Command(BaseCommand): + """Manage notification DLQ manually.""" + + help = "Manage notification DLQ: list, retry, or remove failed notifications" + + DLQ_STREAM_KEY = "owasp_notifications_dlq" + + def add_arguments(self, parser): + parser.add_argument( + "action", + type=str, + choices=["list", "retry", "remove"], + help="Action to perform: list, retry, or remove", + ) + parser.add_argument( + "--id", + type=str, + help="Specific message ID to act on (required for retry/remove unless --all is used)", + ) + parser.add_argument( + "--all", + action="store_true", + help="Apply action to all messages", + ) + + def handle(self, *args, **options): + action = options["action"] + message_id = options.get("id") + all_messages = options.get("all") + + redis_conn = get_redis_connection("default") + + if action == "list": + self.list_dlq(redis_conn) + elif action == "retry": + if not message_id and not all_messages: + self.stdout.write(self.style.ERROR("Error: --id or --all is required for retry")) + sys.exit(1) + self.retry_dlq(redis_conn, message_id, all_messages) + elif action == "remove": + if not message_id and not all_messages: + self.stdout.write(self.style.ERROR("Error: --id or --all is required for remove")) + sys.exit(1) + self.remove_dlq(redis_conn, message_id, all_messages) + + def list_dlq(self, redis_conn): + """List all failed notifications in DLQ.""" + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") + + if not messages: + self.stdout.write(self.style.SUCCESS("DLQ is empty - no failed notifications")) + return + + self.stdout.write("\n" + "=" * 100) + self.stdout.write( + f"{'ID':<20} | {'Email':<25} | {'Type':<18} | {'Entity':<15} | {'Retries':<8}" + ) + self.stdout.write("=" * 100) + + for msg_id, data in messages: + msg_id_str = msg_id.decode("utf-8") if isinstance(msg_id, bytes) else msg_id + user_email = self._get_value(data, b"user_email", "unknown") + notif_type = self._get_value(data, b"notification_type", "unknown") + entity_name = self._get_value(data, b"entity_name", "unknown")[:15] + retries = self._get_value(data, b"dlq_retries", "0") + + self.stdout.write( + f"{msg_id_str:<20} | {user_email:<25} | {notif_type:<18} | {entity_name:<15} | {retries:<8}" + ) + + self.stdout.write("=" * 100) + self.stdout.write(f"Total: {len(messages)} failed notification(s)\n") + + def retry_dlq(self, redis_conn, message_id, all_messages): + """Retry failed notification(s).""" + if all_messages: + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") + else: + messages = ( + [ + ( + message_id.encode(), + redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id), + ) + ] + if redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id) + else [] + ) + + if not messages or (not all_messages and not messages[0][1]): + self.stdout.write(self.style.ERROR("Message(s) not found")) + return + + success_count = 0 + error_count = 0 + + for msg_id, data in messages: + if not data: + continue + + try: + user_email = self._get_value(data, b"user_email") + title = self._get_value(data, b"title") + message = self._get_value(data, b"message") + notif_type = self._get_value(data, b"notification_type") + related_link = self._get_value(data, b"related_link") + + if user_email and title and message: + send_mail( + subject=title, + message=message, + from_email="noreply@owasp.org", + recipient_list=[user_email], + fail_silently=False, + ) + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + success_count += 1 + self.stdout.write(f"Retried: {msg_id} -> {user_email}") + else: + self.stdout.write(self.style.WARNING(f"Skipped (missing data): {msg_id}")) + error_count += 1 + + except Exception as e: + error_count += 1 + retries = int(self._get_value(data, b"dlq_retries", "0")) + new_retries = str(retries + 1) + data[b"dlq_retries"] = new_retries.encode() + new_msg = {k.decode(): v.decode() for k, v in data.items()} + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + redis_conn.xadd(self.DLQ_STREAM_KEY, new_msg) + self.stdout.write( + self.style.ERROR(f"Failed to retry {msg_id}: {e}, incremented retries") + ) + + self.stdout.write( + self.style.SUCCESS( + f"\nRetry complete: {success_count} succeeded, {error_count} failed/retried" + ) + ) + + def remove_dlq(self, redis_conn, message_id, all_messages): + """Remove failed notification(s) from DLQ.""" + if all_messages: + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") + else: + messages = [(message_id.encode(), None)] + + if not messages: + self.stdout.write(self.style.ERROR("No messages found")) + return + + count = 0 + for msg_id, _ in messages: + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) + count += 1 + self.stdout.write(f"Removed: {msg_id}") + + self.stdout.write(self.style.SUCCESS(f"\nRemoved {count} message(s) from DLQ")) + + def _get_value(self, data, key, default=None): + """Helper to get value from message data.""" + value = data.get(key.encode() if isinstance(key, str) else key) + if value: + return value.decode("utf-8") + return default diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 376b241019..4490c11545 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -1,19 +1,20 @@ """Management command to run notification worker.""" -import contextlib +import json import logging import os import socket import time from django.conf import settings -from django.core.exceptions import ObjectDoesNotExist +from django.contrib.contenttypes.models import ContentType from django.core.mail import send_mail from django.core.management.base import BaseCommand from django_redis import get_redis_connection -from apps.nest.models import User -from apps.owasp.models.notification import Notification +from apps.owasp.models.chapter import Chapter +from apps.owasp.models.event import Event +from apps.owasp.models.notification import Notification, Subscription from apps.owasp.models.snapshot import Snapshot logger = logging.getLogger(__name__) @@ -28,9 +29,7 @@ class Command(BaseCommand): MAX_RETRIES = 5 BASE_DELAY = 2 # seconds DELAY_MULTIPLIER = 2 - MAX_DLQ_RETRIES = 5 DLQ_STREAM_KEY = "owasp_notifications_dlq" - DLQ_CHECK_INTERVAL = 300 # seconds def handle(self, *args, **options): """Handle execution.""" @@ -44,8 +43,6 @@ def handle(self, *args, **options): self.recover_pending_messages(redis_conn, stream_key, group_name, consumer_name) - last_dlq_check = time.time() - while True: try: # Read new messages specifically for this group @@ -58,6 +55,7 @@ def handle(self, *args, **options): block=5000, ) # Process main stream messages + logger.info("event: %s", events) if events: for _, messages in events: for message_id, data in messages: @@ -69,11 +67,6 @@ def handle(self, *args, **options): except Exception: logger.exception("Error processing message %s", message_id) - # Check DLQ every 300 seconds - if time.time() - last_dlq_check > self.DLQ_CHECK_INTERVAL: - self.process_dlq(redis_conn) - last_dlq_check = time.time() - except Exception as e: if "NOGROUP" in str(e): logger.warning("Consumer group missing, attempting to recreate...") @@ -96,68 +89,38 @@ def ensure_consumer_group(self, redis_conn, stream_key, group_name): def process_message(self, data): """Process a single message from the stream.""" msg_type = data.get(b"type", b"").decode("utf-8") - if msg_type == "snapshot_published": - self.handle_snapshot_published(data) - - def handle_snapshot_published(self, data): - """Handle snapshot published event.""" - redis_conn = get_redis_connection("default") - try: - raw_id = data.get(b"snapshot_id") - if not raw_id: - return - snapshot_id = int(raw_id.decode("utf-8")) - snapshot = Snapshot.objects.get(id=snapshot_id) - - users = User.objects.filter(is_active=True) - - if not users.exists(): - logger.info("No active users found.") - return - - logger.info("Sending snapshot notification to %d users", users.count()) - - failed_users = [] - - for user in users: - success = self.send_notification_with_retry(user, snapshot) - if not success: - failed_users.append( - { - "user_id": str(user.id), - "email": user.email, - "snapshot_id": str(snapshot_id), - } - ) - - # Send failed users to DLQ - if failed_users: - for failed_user in failed_users: - dlq_message = { - "type": "failed_notification", - "user_id": failed_user["user_id"], - "snapshot_id": failed_user["snapshot_id"], - "timestamp": str(time.time()), - "dlq_retries": "0", - } - redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_message) - - logger.warning("Sent %d failed notifications to DLQ", len(failed_users)) - - except Snapshot.DoesNotExist: - logger.exception("Snapshot matching ID %s not found.", snapshot_id) - except Exception: - logger.exception("Error handling snapshot published event") - - def send_notification_with_retry(self, user, snapshot): + handlers = { + "snapshot_published": self.handle_snapshot_published, + "chapter_created": self.handle_chapter_created, + "chapter_updated": self.handle_chapter_updated, + "event_created": self.handle_event_created, + "event_updated": self.handle_event_updated, + "event_deadline_reminder": self.handle_event_deadline_reminder, + } + + handler = handlers.get(msg_type) + if handler: + handler(data) + else: + logger.warning("Unknown message type: %s", msg_type) + + def send_notification_with_retry( + self, *, user, title, message, notification_type, related_link + ): """Send notification with exponential backoff retry logic.""" retry_count = 0 last_error = None while retry_count <= self.MAX_RETRIES: try: - self.send_notification(user, snapshot) + self.send_notification( + user=user, + title=title, + message=message, + notification_type=notification_type, + related_link=related_link, + ) except Exception as e: retry_count += 1 last_error = e @@ -190,22 +153,17 @@ def send_notification_with_retry(self, user, snapshot): return False - def send_notification(self, user, snapshot): + def send_notification(self, *, user, title, message, notification_type, related_link): """Send notification to user.""" - title = f"New Snapshot Published: {snapshot.title}" - related_link = f"{settings.SITE_URL}/community/snapshots/{snapshot.key}" - if Notification.objects.filter( recipient_id=user.id, - type="snapshot_published", + type=notification_type, related_link=related_link, + message=message, ).exists(): - logger.info("Already notified %s for this snapshot, skipping", user.email) + logger.info("Already notified %s for %s, skipping", user.email, notification_type) return - message = f"Check out the latest OWASP snapshot: {snapshot.title}" - - # Send Email send_mail( subject=title, message=message, @@ -213,112 +171,227 @@ def send_notification(self, user, snapshot): recipient_list=[user.email], fail_silently=False, ) - logger.info("Sent email to %s", user.email) + logger.info("Sent %s email to %s", notification_type, user.email) - # Create DB record Notification.objects.create( recipient=user, - type="snapshot_published", + type=notification_type, title=title, message=message, related_link=related_link, ) - def process_dlq(self, redis_conn): - """Process messages from DLQ - retry failed notifications.""" - lock = redis_conn.lock("owasp_notifications_dlq_lock", timeout=600, blocking=False) - if not lock.acquire(): - return + def handle_snapshot_published(self, data): + """Handle snapshot published event.""" + self._handle_entity_notification( + data=data, + id_field=b"snapshot_id", + model_class=Snapshot, + notification_type="snapshot_published", + global_subscription=True, + ) + + def handle_chapter_created(self, data): + """Handle chapter created event — notify 'all chapters' subscribers.""" + self._handle_entity_notification( + data=data, + id_field=b"chapter_id", + model_class=Chapter, + notification_type="chapter_created", + global_subscription=True, + ) + + def handle_chapter_updated(self, data): + """Handle chapter updated event — notify specific chapter subscribers.""" + self._handle_entity_notification( + data=data, + id_field=b"chapter_id", + model_class=Chapter, + notification_type="chapter_updated", + global_subscription=False, + ) + + def handle_event_created(self, data): + """Handle event created — notify 'all events' subscribers.""" + self._handle_entity_notification( + data=data, + id_field=b"event_id", + model_class=Event, + notification_type="event_created", + global_subscription=True, + ) + + def handle_event_updated(self, data): + """Handle event updated — notify specific event subscribers.""" + self._handle_entity_notification( + data=data, + id_field=b"event_id", + model_class=Event, + notification_type="event_updated", + global_subscription=False, + ) + + def handle_event_deadline_reminder(self, data): + """Handle event deadline reminder — notify specific event subscribers.""" + self._handle_entity_notification( + data=data, + id_field=b"event_id", + model_class=Event, + notification_type="event_deadline_reminder", + global_subscription=False, + ) + + def _handle_entity_notification( + self, *, data, id_field, model_class, notification_type, global_subscription=False + ): + """Handle entity notification for chapters, events, and snapshots. + + Args: + data: Redis stream message data. + id_field: The byte field name for the entity ID. + model_class: The Django model class. + notification_type: The notification type string. + global_subscription: If True, query subscribers with object_id=0 (all entities). + If False, query subscribers with object_id=entity.id (specific entity). + + """ + redis_conn = get_redis_connection("default") try: - self.stdout.write("Checking DLQ for failed notifications...") - messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+", count=50) + raw_id = data.get(id_field) + if not raw_id: + return + entity_id = int(raw_id.decode("utf-8")) + entity = model_class.objects.get(id=entity_id) + + content_type = ContentType.objects.get_for_model(model_class) + subscription_filter = { + "content_type": content_type, + "object_id": 0 if global_subscription else entity_id, + } + subscriptions = Subscription.objects.filter(**subscription_filter).select_related( + "user" + ) + users = [sub.user for sub in subscriptions if sub.user.is_active] - if not messages: - self.stdout.write("No messages in DLQ") + if not users: + logger.info("No recipients found for %s.", notification_type) return - processed_count = 0 - failed_count = 0 - - for msg_id, data in messages: - try: - dlq_retries = 0 - msg_type = (data.get(b"type") or b"").decode("utf-8") - if msg_type == "recovery_failed": - logger.error( - "Permanent recovery failure for message %s: %s", - (data.get(b"message_id") or b"unknown").decode("utf-8"), - (data.get(b"error") or b"unknown").decode("utf-8"), - ) - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - continue + logger.info("Sending %s notification to %d users", notification_type, len(users)) + + entity_name = str(entity) + entity_type = model_class.__name__.lower() + + days_bytes = data.get(b"days_remaining") + days_info = "" + if days_bytes: + days = days_bytes.decode() + days_info = f" ({days} days left)" + + changed_fields_bytes = data.get(b"changed_fields") + changes_description = "" + if changed_fields_bytes: + changed_fields = json.loads(changed_fields_bytes.decode()) + changes_list = [] + for field, values in changed_fields.items(): + old_val = values.get("old") or "empty" + new_val = values.get("new") or "empty" + field_display = field.replace("_", " ").title() + changes_list.append(f"{field_display}: {old_val} → {new_val}") + changes_description = " | ".join(changes_list) + + entity_title = entity.title if hasattr(entity, "title") else entity_name + + titles = { + "snapshot_published": f"New Snapshot Published: {entity_title}", + "chapter_created": f"New Chapter Created: {entity_name}", + "chapter_updated": f"Chapter Updated: {entity_name}", + "event_created": f"New Event Published: {entity_name}", + "event_updated": f"Event Updated: {entity_name}", + "event_deadline_reminder": f"Event Deadline Approaching{days_info}: {entity_name}", + } + entity_messages = { + "snapshot_published": f"Check out the latest OWASP snapshot: {entity_title}", + "chapter_created": f"A new OWASP chapter has been created: {entity_name}", + "chapter_updated": ( + f"The OWASP chapter '{entity_name}' has been updated. " + f"Changes: {changes_description}" + if changes_description + else f"The OWASP chapter '{entity_name}' has been updated." + ), + "event_created": f"A new OWASP event has been published: {entity_name}", + "event_updated": ( + f"The OWASP event '{entity_name}' has been updated. " + f"Changes: {changes_description}" + if changes_description + else f"The OWASP event '{entity_name}' has been updated." + ), + "event_deadline_reminder": ( + f"Reminder: The OWASP event '{entity_name}' " + f"deadline is approaching{days_info}." + ), + } + url_builders = { + "snapshot": lambda e: f"community/snapshots/{e.key}", + "chapter": lambda e: f"chapters/{e.id}", + "event": lambda e: f"events/{e.id}", + } + + title = titles.get(notification_type, f"Notification: {entity_name}") + message = entity_messages.get(notification_type, f"Update for {entity_name}") + + url_builder = url_builders.get(entity_type) + if url_builder: + related_link = f"{settings.SITE_URL}/{url_builder(entity)}" + else: + related_link = f"{settings.SITE_URL}" - raw_dlq_retries = data.get(b"dlq_retries", b"0") - try: - dlq_retries = int(raw_dlq_retries.decode("utf-8")) - except (ValueError, AttributeError): - dlq_retries = 0 - raw_user_id = data.get(b"user_id") - raw_snapshot_id = data.get(b"snapshot_id") - - if not raw_user_id or not raw_snapshot_id: - logger.warning("DLQ message %s missing required fields, removing", msg_id) - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - continue - - user_id = int(raw_user_id.decode("utf-8")) - snapshot_id = int(raw_snapshot_id.decode("utf-8")) - - user = User.objects.get(id=user_id) - snapshot = Snapshot.objects.get(id=snapshot_id) - - self.send_notification(user, snapshot) - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - processed_count += 1 - logger.info( - "Successfully reprocessed notification for user %s", - user.email, + failed_users = [] + + for user in users: + success = self.send_notification_with_retry( + user=user, + title=title, + message=message, + notification_type=notification_type, + related_link=related_link, + ) + if not success: + failed_users.append( + { + "user": user, + "user_id": str(user.id), + "entity_type": entity_type, + "entity_id": str(entity_id), + } ) - except ObjectDoesNotExist: - logger.warning("User or snapshot not found, removing from DLQ") - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - failed_count += 1 - except Exception: - failed_count += 1 - dlq_retries += 1 - - if dlq_retries > self.MAX_DLQ_RETRIES: - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - logger.exception( - "DLQ message %s exceeded max retries (%d). Dropping.", - msg_id, - self.MAX_DLQ_RETRIES, - ) - else: - new_data = {k.decode("utf-8"): v.decode("utf-8") for k, v in data.items()} - new_data["dlq_retries"] = str(dlq_retries) - redis_conn.xadd(self.DLQ_STREAM_KEY, new_data) - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) - logger.warning( - "Failed to reprocess DLQ message %s (attempt %d/%d). Re-queued.", - msg_id, - dlq_retries, - self.MAX_DLQ_RETRIES, - ) + if failed_users: + for failed_user in failed_users: + user_obj = failed_user.get("user") + dlq_message = { + "type": "failed_notification", + "notification_type": notification_type, + "user_id": failed_user["user_id"], + "user_email": user_obj.email if user_obj else "unknown", + "entity_type": entity_type, + "entity_id": str(entity_id), + "entity_name": entity_name, + "title": title, + "message": message, + "related_link": related_link, + "timestamp": str(time.time()), + "dlq_retries": "0", + } + redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_message) - logger.info( - "DLQ processing complete: %d successful, %d failed", - processed_count, - failed_count, - ) + logger.warning("Sent %d failed notifications to DLQ", len(failed_users)) + except model_class.DoesNotExist: + logger.exception("%s matching ID not found.", model_class.__name__) except Exception: - logger.exception("Error processing DLQ") - finally: - with contextlib.suppress(Exception): - lock.release() + logger.exception("Error handling %s event", notification_type) def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_name): """Recover and reprocess stuck messages from PEL.""" diff --git a/backend/apps/owasp/signals/chapter.py b/backend/apps/owasp/signals/chapter.py new file mode 100644 index 0000000000..a5ee0142d4 --- /dev/null +++ b/backend/apps/owasp/signals/chapter.py @@ -0,0 +1,43 @@ +"""Chapter signals.""" + +from django.db.models.signals import post_save, pre_save +from django.dispatch import receiver + +from apps.owasp.models.chapter import Chapter +from apps.owasp.utils.notifications import publish_chapter_notification + +MEANINGFUL_FIELDS = ("name", "country", "region", "suggested_location", "description") + + +@receiver(pre_save, sender=Chapter) +def chapter_pre_save(sender, instance, **kwargs): # noqa: ARG001 + """Store the previous values before saving.""" + if instance.pk: + db_instance = Chapter.objects.filter(pk=instance.pk).values(*MEANINGFUL_FIELDS).first() + if db_instance: + instance._previous_values = { + field: db_instance.get(field) for field in MEANINGFUL_FIELDS + } # noqa: SLF001 + else: + instance._previous_values = {} # noqa: SLF001 + + +@receiver(post_save, sender=Chapter) +def chapter_post_save(sender, instance, created, **kwargs): # noqa: ARG001 + """Signal handler for chapter creation and updates.""" + if created: + publish_chapter_notification(instance, "created") + else: + changed_fields = {} + previous_values = getattr(instance, "_previous_values", {}) + for field in MEANINGFUL_FIELDS: + old_value = previous_values.get(field) + new_value = getattr(instance, field) + if old_value != new_value: + changed_fields[field] = { + "old": str(old_value) if old_value else None, + "new": str(new_value) if new_value else None, + } + + if changed_fields: + publish_chapter_notification(instance, "updated", changed_fields) diff --git a/backend/apps/owasp/signals/event.py b/backend/apps/owasp/signals/event.py new file mode 100644 index 0000000000..2689597acc --- /dev/null +++ b/backend/apps/owasp/signals/event.py @@ -0,0 +1,50 @@ +"""Event signals.""" + +from django.db.models.signals import post_save, pre_save +from django.dispatch import receiver + +from apps.owasp.models.event import Event +from apps.owasp.utils.notifications import publish_event_notification + +MEANINGFUL_FIELDS = ( + "name", + "start_date", + "end_date", + "suggested_location", + "url", + "description", +) + + +@receiver(pre_save, sender=Event) +def event_pre_save(sender, instance, **kwargs): # noqa: ARG001 + """Store the previous values before saving.""" + if instance.pk: + db_instance = Event.objects.filter(pk=instance.pk).values(*MEANINGFUL_FIELDS).first() + if db_instance: + instance._previous_values = { # noqa: SLF001 + field: db_instance.get(field) for field in MEANINGFUL_FIELDS + } + else: + instance._previous_values = {} # noqa: SLF001 + + +@receiver(post_save, sender=Event) +def event_post_save(sender, instance, created, **kwargs): # noqa: ARG001 + """Signal handler for event creation and updates.""" + if created: + publish_event_notification(instance, "created") + else: + changed_fields = {} + previous_values = getattr(instance, "_previous_values", {}) + for field in MEANINGFUL_FIELDS: + old_value = previous_values.get(field) + new_value = getattr(instance, field) + if old_value != new_value: + changed_fields[field] = { + "old": str(old_value) if old_value else None, + "new": str(new_value) if new_value else None, + } + + if changed_fields: + publish_event_notification(instance, "updated", changed_fields=changed_fields) diff --git a/backend/apps/owasp/utils/notifications.py b/backend/apps/owasp/utils/notifications.py index 4afaab36f3..e349dc3297 100644 --- a/backend/apps/owasp/utils/notifications.py +++ b/backend/apps/owasp/utils/notifications.py @@ -1,29 +1,103 @@ """Notification utils.""" +import json import logging from django.utils.timezone import now from django_redis import get_redis_connection +from apps.owasp.models.chapter import Chapter +from apps.owasp.models.event import Event from apps.owasp.models.snapshot import Snapshot logger = logging.getLogger(__name__) +STREAM_KEY = "owasp_notifications" + def publish_snapshot_notification(snapshot: Snapshot) -> None: """Publish a notification for a published snapshot.""" try: redis_conn = get_redis_connection("default") - stream_key = "owasp_notifications" message = { "type": "snapshot_published", "snapshot_id": str(snapshot.id), "timestamp": str(now().timestamp()), } - redis_conn.xadd(stream_key, message) + redis_conn.xadd(STREAM_KEY, message) logger.info("Published snapshot notification for snapshot %s", snapshot.id) except Exception: logger.exception( "Failed to publish snapshot notification for snapshot %s", snapshot.id, ) + + +def publish_chapter_notification( + chapter: Chapter, trigger: str, changed_fields: dict | None = None +) -> None: + """Publish a notification for a chapter creation or update. + + Args: + chapter: The Chapter instance. + trigger: Either "created" or "updated". + changed_fields: Dict of changed fields with old/new values (only for updates). + + """ + msg_type = f"chapter_{trigger}" + try: + redis_conn = get_redis_connection("default") + message = { + "type": msg_type, + "chapter_id": str(chapter.id), + "timestamp": str(now().timestamp()), + } + if changed_fields: + message["changed_fields"] = json.dumps(changed_fields) + + redis_conn.xadd(STREAM_KEY, message) + logger.info("Published %s notification for chapter %s", msg_type, chapter.id) + except Exception: + logger.exception( + "Failed to publish %s notification for chapter %s", + msg_type, + chapter.id, + ) + + +def publish_event_notification( + event: Event, + trigger: str, + days_remaining: int | None = None, + changed_fields: dict | None = None, +) -> None: + """Publish a notification for an event creation, update, or deadline reminder. + + Args: + event: The Event instance. + trigger: Either "created", "updated", or "deadline_reminder". + days_remaining: Days until event (only for deadline_reminder). + changed_fields: Dict of changed fields with old/new values (only for updates). + + """ + msg_type = f"event_{trigger}" + try: + redis_conn = get_redis_connection("default") + message = { + "type": msg_type, + "event_id": str(event.id), + "timestamp": str(now().timestamp()), + } + if days_remaining is not None: + message["days_remaining"] = str(days_remaining) + if changed_fields: + message["changed_fields"] = json.dumps(changed_fields) + + redis_conn.xadd(STREAM_KEY, message) + logger.info("Published %s notification for event %s", msg_type, event.id) + except Exception: + logger.exception( + "Failed to publish %s notification for event %s", + msg_type, + event.id, + ) diff --git a/backend/tests/apps/owasp/management/commands/owasp_check_event_deadlines_test.py b/backend/tests/apps/owasp/management/commands/owasp_check_event_deadlines_test.py new file mode 100644 index 0000000000..e03a9f1636 --- /dev/null +++ b/backend/tests/apps/owasp/management/commands/owasp_check_event_deadlines_test.py @@ -0,0 +1,83 @@ +"""Tests for event deadline check management command.""" + +from datetime import timedelta +from unittest.mock import MagicMock, patch + +from django.utils import timezone + +from apps.owasp.management.commands.owasp_check_event_deadlines import Command + + +class TestCheckEventDeadlines: + """Test owasp_check_event_deadlines management command.""" + + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.publish_event_notification") + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.Event") + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.timezone") + def test_finds_events_at_reminder_days(self, mock_tz, mock_event_cls, mock_publish): + """Test that the command checks for events at 7, 3, and 1 day thresholds.""" + today = timezone.now().date() + mock_tz.now.return_value.date.return_value = today + mock_tz.timedelta = timedelta + + mock_event_cls.objects.filter.return_value = [] + + command = Command() + command.stdout = MagicMock() + command.style = MagicMock() + command.style.SUCCESS = lambda x: x + + command.handle() + + # Should query for 3 different dates (7, 3, 1 days from now) + assert mock_event_cls.objects.filter.call_count == 3 + + expected_dates = [today + timedelta(days=d) for d in (7, 3, 1)] + actual_dates = [ + call.kwargs["start_date"] for call in mock_event_cls.objects.filter.call_args_list + ] + assert actual_dates == expected_dates + + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.publish_event_notification") + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.Event") + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.timezone") + def test_publishes_reminder_for_matching_events(self, mock_tz, mock_event_cls, mock_publish): + """Test that matching events trigger deadline_reminder notifications.""" + today = timezone.now().date() + mock_tz.now.return_value.date.return_value = today + mock_tz.timedelta = timedelta + + mock_event = MagicMock() + mock_event.name = "AppSec Days" + + # Only the 7-day query returns an event + mock_event_cls.objects.filter.side_effect = [[mock_event], [], []] + + command = Command() + command.stdout = MagicMock() + command.style = MagicMock() + command.style.SUCCESS = lambda x: x + + command.handle() + + mock_publish.assert_called_once_with(mock_event, "deadline_reminder", days_remaining=7) + + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.publish_event_notification") + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.Event") + @patch("apps.owasp.management.commands.owasp_check_event_deadlines.timezone") + def test_no_events_found(self, mock_tz, mock_event_cls, mock_publish): + """Test that no notifications are sent when no events match.""" + today = timezone.now().date() + mock_tz.now.return_value.date.return_value = today + mock_tz.timedelta = timedelta + + mock_event_cls.objects.filter.return_value = [] + + command = Command() + command.stdout = MagicMock() + command.style = MagicMock() + command.style.SUCCESS = lambda x: x + + command.handle() + + mock_publish.assert_not_called() diff --git a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py index dc539cee97..91b252d735 100644 --- a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py +++ b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py @@ -1,7 +1,6 @@ from unittest import mock import pytest -from django.core.exceptions import ObjectDoesNotExist from apps.nest.models import User from apps.owasp.management.commands.owasp_run_notification_worker import Command @@ -38,7 +37,13 @@ def test_send_notification_success( mock_settings.SITE_URL = "https://example.com" mock_notification.objects.filter.return_value.exists.return_value = False - command.send_notification(mock_user, mock_snapshot) + command.send_notification( + user=mock_user, + title=f"New Snapshot Published: {mock_snapshot.title}", + message=f"Check out the latest OWASP snapshot: {mock_snapshot.title}", + notification_type="snapshot_published", + related_link=f"https://example.com/community/snapshots/{mock_snapshot.key}", + ) mock_send_mail.assert_called_once() mock_notification.objects.create.assert_called_once_with( @@ -57,86 +62,17 @@ def test_send_notification_idempotency( """Test that notification is skipped if it already exists.""" mock_notification.objects.filter.return_value.exists.return_value = True - command.send_notification(mock_user, mock_snapshot) + command.send_notification( + user=mock_user, + title=f"New Snapshot Published: {mock_snapshot.title}", + message=f"Check out the latest OWASP snapshot: {mock_snapshot.title}", + notification_type="snapshot_published", + related_link=f"https://example.com/community/snapshots/{mock_snapshot.key}", + ) mock_send_mail.assert_not_called() mock_notification.objects.create.assert_not_called() - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Snapshot") - @mock.patch( - "apps.owasp.management.commands.owasp_run_notification_worker.Command.send_notification" - ) - def test_process_dlq_success( - self, - mock_send_notify, - mock_snapshot_model, - mock_user_model, - command, - mock_user, - mock_snapshot, - ): - """Test successful DLQ processing.""" - redis_conn = mock.Mock() - redis_conn.lock.return_value.acquire.return_value = True - - # Mock message data - redis_conn.xrange.return_value = [(b"123-0", {b"user_id": b"123", b"snapshot_id": b"456"})] - - mock_user_model.objects.get.return_value = mock_user - mock_snapshot_model.objects.get.return_value = mock_snapshot - - command.process_dlq(redis_conn) - - mock_send_notify.assert_called_once_with(mock_user, mock_snapshot) - redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") - - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") - def test_process_dlq_missing_object(self, mock_user, command): - """Test DLQ processing handles ObjectDoesNotExist by removing message.""" - redis_conn = mock.Mock() - redis_conn.lock.return_value.acquire.return_value = True - - redis_conn.xrange.return_value = [(b"123-0", {b"user_id": b"999", b"snapshot_id": b"456"})] - - mock_user.objects.get.side_effect = ObjectDoesNotExist - - command.process_dlq(redis_conn) - - redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") - - def test_process_dlq_missing_fields(self, command): - """Test DLQ processing handles messages with missing fields.""" - redis_conn = mock.Mock() - redis_conn.lock.return_value.acquire.return_value = True - - # Missing user_id - redis_conn.xrange.return_value = [(b"123-0", {b"snapshot_id": b"456"})] - - command.process_dlq(redis_conn) - - redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") - - def test_process_dlq_recovery_failure(self, command): - """Test DLQ processing handles recovery_failed messages by removing them.""" - redis_conn = mock.Mock() - redis_conn.lock.return_value.acquire.return_value = True - - redis_conn.xrange.return_value = [ - ( - b"123-0", - { - b"type": b"recovery_failed", - b"message_id": b"original-999", - b"error": b"Something went wrong", - }, - ) - ] - - command.process_dlq(redis_conn) - - redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") - @mock.patch.object(Command, "process_message") def test_recover_pending_messages(self, mock_process, command): """Test recovery of pending messages.""" @@ -162,40 +98,228 @@ def test_recover_pending_messages_failure(self, mock_process, command): assert redis_conn.xadd.call_args[0][0] == command.DLQ_STREAM_KEY redis_conn.xack.assert_called_once() - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") - def test_process_dlq_retry_increment(self, mock_user, command): - """Test that DLQ processing increments retry count and re-adds on failure.""" - redis_conn = mock.Mock() - redis_conn.lock.return_value.acquire.return_value = True - redis_conn.xrange.return_value = [ - (b"123-0", {b"user_id": b"123", b"snapshot_id": b"456", b"dlq_retries": b"1"}) - ] +class TestProcessMessageRouting: + """Test process_message routes new entity message types correctly.""" - mock_user.objects.get.side_effect = Exception("Temp Error") + @pytest.fixture + def command(self): + return Command() - command.process_dlq(redis_conn) + @pytest.mark.parametrize( + ("msg_type", "handler_name"), + [ + ("snapshot_published", "handle_snapshot_published"), + ("chapter_created", "handle_chapter_created"), + ("chapter_updated", "handle_chapter_updated"), + ("event_created", "handle_event_created"), + ("event_updated", "handle_event_updated"), + ("event_deadline_reminder", "handle_event_deadline_reminder"), + ], + ) + def test_routes_to_correct_handler(self, command, msg_type, handler_name): + """Test that each message type routes to the correct handler.""" + data = {b"type": msg_type.encode()} + with mock.patch.object(command, handler_name) as mock_handler: + command.process_message(data) + mock_handler.assert_called_once_with(data) - # Should be deleted and re-added with count 2 - redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") - assert redis_conn.xadd.called - new_msg = redis_conn.xadd.call_args[0][1] - assert new_msg["dlq_retries"] == "2" + def test_unknown_message_type_does_not_raise(self, command): + """Test that unknown message types are handled gracefully.""" + data = {b"type": b"unknown_type"} + command.process_message(data) # Should not raise - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.User") - def test_process_dlq_max_retries_exceeded(self, mock_user, command): - """Test that DLQ processing drops message after max retries.""" - redis_conn = mock.Mock() - redis_conn.lock.return_value.acquire.return_value = True - # Already at max retries (5) - redis_conn.xrange.return_value = [ - (b"123-0", {b"user_id": b"123", b"snapshot_id": b"456", b"dlq_retries": b"5"}) - ] +class TestEntityNotificationHandlers: + """Test entity notification handler methods.""" - mock_user.objects.get.side_effect = Exception("Final Error") + @pytest.fixture + def command(self): + return Command() + + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Subscription") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.ContentType") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Snapshot") + def test_snapshot_published_queries_global_subscribers( + self, mock_snapshot_cls, mock_ct, mock_sub, mock_redis + ): + """Test that snapshot_published queries global subscribers (object_id=0).""" + command = Command() + mock_redis.return_value = mock.MagicMock() + mock_snapshot = mock.MagicMock() + mock_snapshot.title = "Test Snapshot" + mock_snapshot.key = "test-key" + mock_snapshot_cls.objects.get.return_value = mock_snapshot + mock_snapshot_cls.DoesNotExist = Exception + mock_snapshot_cls.__name__ = "Snapshot" + + mock_content_type = mock.MagicMock() + mock_ct.objects.get_for_model.return_value = mock_content_type + + # Mock subscriptions + mock_sub.objects.filter.return_value.select_related.return_value = [] + + data = {b"snapshot_id": b"99"} + + command.handle_snapshot_published(data) + + # Verify subscriptions queried with object_id=0 + mock_sub.objects.filter.assert_called_once_with( + content_type=mock_content_type, object_id=0 + ) + + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Subscription") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.ContentType") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Chapter") + def test_chapter_created_queries_global_subscribers( + self, mock_chapter_cls, mock_ct, mock_sub, mock_redis, command + ): + """Test that chapter_created queries subscribers with object_id=0.""" + mock_redis.return_value = mock.MagicMock() + mock_chapter = mock.MagicMock() + mock_chapter_cls.objects.get.return_value = mock_chapter + mock_chapter_cls.DoesNotExist = Exception + mock_chapter_cls.__name__ = "Chapter" + + mock_content_type = mock.MagicMock() + mock_ct.objects.get_for_model.return_value = mock_content_type + mock_sub.objects.filter.return_value.select_related.return_value = [] + + data = {b"chapter_id": b"1"} + command.handle_chapter_created(data) + + mock_sub.objects.filter.assert_called_once_with( + content_type=mock_content_type, object_id=0 + ) + + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Subscription") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.ContentType") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Chapter") + def test_chapter_updated_queries_specific_subscribers( + self, mock_chapter_cls, mock_ct, mock_sub, mock_redis, command + ): + """Test that chapter_updated queries subscribers with specific object_id.""" + mock_redis.return_value = mock.MagicMock() + mock_chapter = mock.MagicMock() + mock_chapter_cls.objects.get.return_value = mock_chapter + mock_chapter_cls.DoesNotExist = Exception + mock_chapter_cls.__name__ = "Chapter" + + mock_content_type = mock.MagicMock() + mock_ct.objects.get_for_model.return_value = mock_content_type + mock_sub.objects.filter.return_value.select_related.return_value = [] + + data = {b"chapter_id": b"42"} + command.handle_chapter_updated(data) + + mock_sub.objects.filter.assert_called_once_with( + content_type=mock_content_type, object_id=42 + ) - command.process_dlq(redis_conn) + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Subscription") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.ContentType") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Event") + def test_event_created_queries_global_subscribers( + self, mock_event_cls, mock_ct, mock_sub, mock_redis, command + ): + """Test that event_created queries subscribers with object_id=0.""" + mock_redis.return_value = mock.MagicMock() + mock_event = mock.MagicMock() + mock_event_cls.objects.get.return_value = mock_event + mock_event_cls.DoesNotExist = Exception + mock_event_cls.__name__ = "Event" + + mock_content_type = mock.MagicMock() + mock_ct.objects.get_for_model.return_value = mock_content_type + mock_sub.objects.filter.return_value.select_related.return_value = [] + + data = {b"event_id": b"10"} + command.handle_event_created(data) + + mock_sub.objects.filter.assert_called_once_with( + content_type=mock_content_type, object_id=0 + ) - redis_conn.xdel.assert_called_with(command.DLQ_STREAM_KEY, b"123-0") - assert redis_conn.xadd.call_count == 0 + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Subscription") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.ContentType") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Event") + def test_event_deadline_reminder_queries_specific_subscribers( + self, mock_event_cls, mock_ct, mock_sub, mock_redis, command + ): + """Test that event_deadline_reminder queries subscribers with specific object_id.""" + mock_redis.return_value = mock.MagicMock() + mock_event = mock.MagicMock() + mock_event_cls.objects.get.return_value = mock_event + mock_event_cls.DoesNotExist = Exception + mock_event_cls.__name__ = "Event" + + mock_content_type = mock.MagicMock() + mock_ct.objects.get_for_model.return_value = mock_content_type + mock_sub.objects.filter.return_value.select_related.return_value = [] + + data = {b"event_id": b"10"} + command.handle_event_deadline_reminder(data) + + mock_sub.objects.filter.assert_called_once_with( + content_type=mock_content_type, object_id=10 + ) + + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Subscription") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.ContentType") + @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Event") + def test_event_deadline_reminder_includes_days_remaining( + self, mock_event_cls, mock_ct, mock_sub, mock_redis, command + ): + """Test that event_deadline_reminder includes days remaining in title/message.""" + mock_redis.return_value = mock.MagicMock() + mock_event = mock.MagicMock() + mock_event.name = "Test Event" + mock_event_cls.objects.get.return_value = mock_event + mock_event_cls.DoesNotExist = Exception + mock_event_cls.__name__ = "Event" + + mock_content_type = mock.MagicMock() + mock_ct.objects.get_for_model.return_value = mock_content_type + + # Mock a subscriber + mock_user = mock.MagicMock(is_active=True) + mock_sub_obj = mock.MagicMock() + mock_sub_obj.user = mock_user + mock_sub.objects.filter.return_value.select_related.return_value = [mock_sub_obj] + + data = {b"event_id": b"10", b"days_remaining": b"3"} + + with mock.patch.object(command, "send_notification_with_retry") as mock_send: + command.handle_event_deadline_reminder(data) + + mock_send.assert_called_once() + kwargs = mock_send.call_args[1] + assert "(3 days left)" in kwargs["title"] + assert "(3 days left)" in kwargs["message"] + + @mock.patch( + "apps.owasp.management.commands.owasp_run_notification_worker.get_redis_connection" + ) + def test_missing_entity_id_returns_early(self, mock_redis, command): + """Test that messages without entity ID are handled gracefully.""" + mock_redis.return_value = mock.MagicMock() + data = {b"type": b"chapter_created"} + command.handle_chapter_created(data) # Should not raise diff --git a/backend/tests/apps/owasp/signals/__init__.py b/backend/tests/apps/owasp/signals/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/tests/apps/owasp/signals/chapter_test.py b/backend/tests/apps/owasp/signals/chapter_test.py new file mode 100644 index 0000000000..0b8f397bfd --- /dev/null +++ b/backend/tests/apps/owasp/signals/chapter_test.py @@ -0,0 +1,38 @@ +"""Tests for chapter signal handlers.""" + +from unittest.mock import MagicMock, patch + +from apps.owasp.signals.chapter import chapter_post_save + + +class TestChapterSignals: + """Test chapter post_save signal handler.""" + + @patch("apps.owasp.signals.chapter.publish_chapter_notification") + def test_chapter_created_publishes_created_notification(self, mock_publish): + """Test that creating a chapter publishes a 'created' notification.""" + chapter = MagicMock() + chapter_post_save(sender=None, instance=chapter, created=True) + mock_publish.assert_called_once_with(chapter, "created") + + @patch("apps.owasp.signals.chapter.publish_chapter_notification") + def test_chapter_updated_publishes_updated_notification(self, mock_publish): + """Test that updating a chapter publishes an 'updated' notification.""" + chapter = MagicMock() + # Set up previous values that match current values - no changes, no notification + chapter._previous_values = { + "name": "Test Chapter", + "country": "Test Country", + "region": "Test Region", + "suggested_location": "Test Location", + "description": "Test description", + } + # Set current values to be the same - no notification should be sent + chapter.name = "Test Chapter" + chapter.country = "Test Country" + chapter.region = "Test Region" + chapter.suggested_location = "Test Location" + chapter.description = "Test description" + chapter_post_save(sender=None, instance=chapter, created=False) + # No changes, so no notification should be published + mock_publish.assert_not_called() diff --git a/backend/tests/apps/owasp/signals/event_test.py b/backend/tests/apps/owasp/signals/event_test.py new file mode 100644 index 0000000000..c2f04f5fb8 --- /dev/null +++ b/backend/tests/apps/owasp/signals/event_test.py @@ -0,0 +1,40 @@ +"""Tests for event signal handlers.""" + +from unittest.mock import MagicMock, patch + +from apps.owasp.signals.event import event_post_save + + +class TestEventSignals: + """Test event post_save signal handler.""" + + @patch("apps.owasp.signals.event.publish_event_notification") + def test_event_created_publishes_created_notification(self, mock_publish): + """Test that creating an event publishes a 'created' notification.""" + event = MagicMock() + event_post_save(sender=None, instance=event, created=True) + mock_publish.assert_called_once_with(event, "created") + + @patch("apps.owasp.signals.event.publish_event_notification") + def test_event_updated_publishes_updated_notification(self, mock_publish): + """Test that updating an event publishes an 'updated' notification.""" + event = MagicMock() + # Set up previous values that match current values - no changes, no notification + event._previous_values = { + "name": "Test Event", + "start_date": "2026-01-01", + "end_date": "2026-01-02", + "suggested_location": "Test Location", + "url": "http://test.com", + "description": "Test description", + } + # Set current values to be the same - no notification should be sent + event.name = "Test Event" + event.start_date = "2026-01-01" + event.end_date = "2026-01-02" + event.suggested_location = "Test Location" + event.url = "http://test.com" + event.description = "Test description" + event_post_save(sender=None, instance=event, created=False) + # No changes, so no notification should be published + mock_publish.assert_not_called() diff --git a/backend/tests/apps/owasp/utils/notifications_test.py b/backend/tests/apps/owasp/utils/notifications_test.py new file mode 100644 index 0000000000..f19850cedc --- /dev/null +++ b/backend/tests/apps/owasp/utils/notifications_test.py @@ -0,0 +1,132 @@ +"""Tests for notification utils.""" + +from unittest.mock import MagicMock, patch + +from apps.owasp.utils.notifications import ( + publish_chapter_notification, + publish_event_notification, + publish_snapshot_notification, +) + + +class TestPublishSnapshotNotification: + """Test publish_snapshot_notification.""" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_publishes_to_redis_stream(self, mock_redis): + """Test that snapshot notification is published to Redis stream.""" + mock_conn = MagicMock() + mock_redis.return_value = mock_conn + snapshot = MagicMock() + snapshot.id = 1 + + publish_snapshot_notification(snapshot) + + mock_conn.xadd.assert_called_once() + call_args = mock_conn.xadd.call_args + assert call_args[0][0] == "owasp_notifications" + assert call_args[0][1]["type"] == "snapshot_published" + assert call_args[0][1]["snapshot_id"] == "1" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_handles_redis_error(self, mock_redis): + """Test that Redis connection errors are handled gracefully.""" + mock_redis.side_effect = Exception("Redis connection failed") + snapshot = MagicMock() + snapshot.id = 1 + + publish_snapshot_notification(snapshot) # Should not raise + + +class TestPublishChapterNotification: + """Test publish_chapter_notification.""" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_publishes_created_notification(self, mock_redis): + """Test that chapter created notification is published.""" + mock_conn = MagicMock() + mock_redis.return_value = mock_conn + chapter = MagicMock() + chapter.id = 5 + + publish_chapter_notification(chapter, "created") + + mock_conn.xadd.assert_called_once() + call_args = mock_conn.xadd.call_args + assert call_args[0][1]["type"] == "chapter_created" + assert call_args[0][1]["chapter_id"] == "5" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_publishes_updated_notification(self, mock_redis): + """Test that chapter updated notification is published.""" + mock_conn = MagicMock() + mock_redis.return_value = mock_conn + chapter = MagicMock() + chapter.id = 5 + + publish_chapter_notification(chapter, "updated") + + call_args = mock_conn.xadd.call_args + assert call_args[0][1]["type"] == "chapter_updated" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_handles_redis_error(self, mock_redis): + """Test that Redis errors are handled gracefully.""" + mock_redis.side_effect = Exception("Redis down") + chapter = MagicMock() + chapter.id = 5 + + publish_chapter_notification(chapter, "created") # Should not raise + + +class TestPublishEventNotification: + """Test publish_event_notification.""" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_publishes_created_notification(self, mock_redis): + """Test that event created notification is published.""" + mock_conn = MagicMock() + mock_redis.return_value = mock_conn + event = MagicMock() + event.id = 10 + + publish_event_notification(event, "created") + + call_args = mock_conn.xadd.call_args + assert call_args[0][1]["type"] == "event_created" + assert call_args[0][1]["event_id"] == "10" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_publishes_updated_notification(self, mock_redis): + """Test that event updated notification is published.""" + mock_conn = MagicMock() + mock_redis.return_value = mock_conn + event = MagicMock() + event.id = 10 + + publish_event_notification(event, "updated") + + call_args = mock_conn.xadd.call_args + assert call_args[0][1]["type"] == "event_updated" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_publishes_deadline_reminder_notification(self, mock_redis): + """Test that event deadline reminder notification is published.""" + mock_conn = MagicMock() + mock_redis.return_value = mock_conn + event = MagicMock() + event.id = 10 + + publish_event_notification(event, "deadline_reminder") + + call_args = mock_conn.xadd.call_args + assert call_args[0][1]["type"] == "event_deadline_reminder" + + @patch("apps.owasp.utils.notifications.get_redis_connection") + def test_handles_redis_error(self, mock_redis): + """Test that Redis errors are handled gracefully.""" + mock_redis.side_effect = Exception("Redis down") + event = MagicMock() + event.id = 10 + + publish_event_notification(event, "created") # Should not raise diff --git a/cron/production b/cron/production index 9bbff5aed8..94a2acfffe 100644 --- a/cron/production +++ b/cron/production @@ -2,3 +2,4 @@ 17 05 * * * cd /home/production; make sync-data > /var/log/nest/production/sync-data.log 2>&1 17 17 * * * cd /home/production; make owasp-update-project-health-requirements && make owasp-update-project-health-metrics > /var/log/nest/production/update-project-health-metrics 2>&1 22 17 * * * cd /home/production; make owasp-update-project-health-scores > /var/log/nest/production/update-project-health-scores 2>&1 +00 06 * * * cd /home/production; make owasp-check-event-deadlines > /var/log/nest/production/check-event-deadlines.log 2>&1 From 4eadda6d2ee0cd39e16d022e91bb2733712bb647 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Fri, 27 Feb 2026 09:02:55 +0600 Subject: [PATCH 15/22] run make check-test --- .../commands/owasp_check_event_deadlines.py | 8 ++--- .../commands/owasp_notification_dlq.py | 31 +++++++------------ .../commands/owasp_run_notification_worker.py | 3 +- backend/apps/owasp/signals/chapter.py | 4 +-- 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py b/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py index 7279c7d114..56ff89f315 100644 --- a/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py +++ b/backend/apps/owasp/management/commands/owasp_check_event_deadlines.py @@ -29,12 +29,8 @@ def handle(self, *args, **options): events = Event.objects.filter(start_date=target_date) for event in events: - self.stdout.write( - f" Event '{event.name}' starts in {days} days ({target_date})" - ) + self.stdout.write(f" Event '{event.name}' starts in {days} days ({target_date})") publish_event_notification(event, "deadline_reminder", days_remaining=days) total_reminders += 1 - self.stdout.write( - self.style.SUCCESS(f"Queued {total_reminders} deadline reminder(s).") - ) + self.stdout.write(self.style.SUCCESS(f"Queued {total_reminders} deadline reminder(s).")) diff --git a/backend/apps/owasp/management/commands/owasp_notification_dlq.py b/backend/apps/owasp/management/commands/owasp_notification_dlq.py index c798a3f57e..55e4c793db 100644 --- a/backend/apps/owasp/management/commands/owasp_notification_dlq.py +++ b/backend/apps/owasp/management/commands/owasp_notification_dlq.py @@ -1,9 +1,7 @@ """Management command to manage notification DLQ.""" import sys -import time -from django.conf import settings from django.core.mail import send_mail from django.core.management.base import BaseCommand from django_redis import get_redis_connection @@ -76,7 +74,8 @@ def list_dlq(self, redis_conn): retries = self._get_value(data, b"dlq_retries", "0") self.stdout.write( - f"{msg_id_str:<20} | {user_email:<25} | {notif_type:<18} | {entity_name:<15} | {retries:<8}" + f"{msg_id_str:<20} | {user_email:<25} | " + f"{notif_type:<18} | {entity_name:<15} | {retries:<8}" ) self.stdout.write("=" * 100) @@ -87,18 +86,10 @@ def retry_dlq(self, redis_conn, message_id, all_messages): if all_messages: messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") else: - messages = ( - [ - ( - message_id.encode(), - redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id), - ) - ] - if redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id) - else [] - ) + result = redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id) + messages = result or [] - if not messages or (not all_messages and not messages[0][1]): + if not messages: self.stdout.write(self.style.ERROR("Message(s) not found")) return @@ -113,13 +104,15 @@ def retry_dlq(self, redis_conn, message_id, all_messages): user_email = self._get_value(data, b"user_email") title = self._get_value(data, b"title") message = self._get_value(data, b"message") - notif_type = self._get_value(data, b"notification_type") related_link = self._get_value(data, b"related_link") if user_email and title and message: + full_message = ( + f"{message}\n\nView: {related_link}" if related_link else message + ) send_mail( subject=title, - message=message, + message=full_message, from_email="noreply@owasp.org", recipient_list=[user_email], fail_silently=False, @@ -131,7 +124,7 @@ def retry_dlq(self, redis_conn, message_id, all_messages): self.stdout.write(self.style.WARNING(f"Skipped (missing data): {msg_id}")) error_count += 1 - except Exception as e: + except Exception as e: # noqa: BLE001 error_count += 1 retries = int(self._get_value(data, b"dlq_retries", "0")) new_retries = str(retries + 1) @@ -154,7 +147,7 @@ def remove_dlq(self, redis_conn, message_id, all_messages): if all_messages: messages = redis_conn.xrange(self.DLQ_STREAM_KEY, "-", "+") else: - messages = [(message_id.encode(), None)] + messages = redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id) if not messages: self.stdout.write(self.style.ERROR("No messages found")) @@ -169,7 +162,7 @@ def remove_dlq(self, redis_conn, message_id, all_messages): self.stdout.write(self.style.SUCCESS(f"\nRemoved {count} message(s) from DLQ")) def _get_value(self, data, key, default=None): - """Helper to get value from message data.""" + """Get value from message data.""" value = data.get(key.encode() if isinstance(key, str) else key) if value: return value.decode("utf-8") diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 4490c11545..3bba9b0285 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -164,9 +164,10 @@ def send_notification(self, *, user, title, message, notification_type, related_ logger.info("Already notified %s for %s, skipping", user.email, notification_type) return + full_message = f"{message}\n\nView: {related_link}" if related_link else message send_mail( subject=title, - message=message, + message=full_message, from_email="noreply@owasp.org", recipient_list=[user.email], fail_silently=False, diff --git a/backend/apps/owasp/signals/chapter.py b/backend/apps/owasp/signals/chapter.py index a5ee0142d4..c34d1b5917 100644 --- a/backend/apps/owasp/signals/chapter.py +++ b/backend/apps/owasp/signals/chapter.py @@ -15,9 +15,9 @@ def chapter_pre_save(sender, instance, **kwargs): # noqa: ARG001 if instance.pk: db_instance = Chapter.objects.filter(pk=instance.pk).values(*MEANINGFUL_FIELDS).first() if db_instance: - instance._previous_values = { + instance._previous_values = { # noqa: SLF001 field: db_instance.get(field) for field in MEANINGFUL_FIELDS - } # noqa: SLF001 + } else: instance._previous_values = {} # noqa: SLF001 From fbc4663f52ee06c53ce53858258e728ee0999c19 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sun, 5 Apr 2026 08:08:26 +0600 Subject: [PATCH 16/22] fix: route failed messages to DLQ immediately and preserve raw data - Main loop now sends failed messages directly to DLQ with full original data instead of only logging and leaving them stranded in PEL - Recovery DLQ entries now include original stream payload for observability and future retries --- .../commands/owasp_run_notification_worker.py | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 3bba9b0285..4c39fcb9b3 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -64,8 +64,27 @@ def handle(self, *args, **options): # Explicitly acknowledge the message redis_conn.xack(stream_key, group_name, message_id) logger.info("Message processed successfully.") - except Exception: + except Exception as exc: logger.exception("Error processing message %s", message_id) + try: + dlq_entry = { + k.decode(): v.decode() for k, v in data.items() + } + dlq_entry.update({ + "type": "processing_failed", + "original_message_id": message_id.decode() + if isinstance(message_id, bytes) + else str(message_id), + "error": str(exc), + "dlq_retries": "0", + }) + redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_entry) + # ACK so it doesn't stay stranded in PEL + redis_conn.xack(stream_key, group_name, message_id) + except Exception: + logger.exception( + "Failed to send stranded message %s to DLQ", message_id + ) except Exception as e: if "NOGROUP" in str(e): @@ -416,14 +435,18 @@ def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_ self.stdout.write(f"Successfully recovered message {message_id}") except Exception as exc: logger.exception("Failed to recover message %s", message_id) - redis_conn.xadd( - self.DLQ_STREAM_KEY, - { - "type": "recovery_failed", - "message_id": str(message_id), - "error": str(exc), - }, - ) + dlq_entry = { + k.decode(): v.decode() for k, v in data.items() + } + dlq_entry.update({ + "type": "recovery_failed", + "original_message_id": message_id.decode() + if isinstance(message_id, bytes) + else str(message_id), + "error": str(exc), + "dlq_retries": "0", + }) + redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_entry) redis_conn.xack(stream_key, group_name, message_id) else: self.stdout.write("No stuck messages found.") From 2de837a394085409d9c6bb626d270a1f18e50e59 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Sun, 5 Apr 2026 09:52:31 +0600 Subject: [PATCH 17/22] fix: re-raise exceptions in entity notification handler - Re-raise DoesNotExist and generic Exception in _handle_entity_notification so failures propagate to the main loop DLQ routing instead of being silently swallowed and ACKed --- .../commands/owasp_run_notification_worker.py | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 4c39fcb9b3..14fb13674a 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -67,17 +67,17 @@ def handle(self, *args, **options): except Exception as exc: logger.exception("Error processing message %s", message_id) try: - dlq_entry = { - k.decode(): v.decode() for k, v in data.items() - } - dlq_entry.update({ - "type": "processing_failed", - "original_message_id": message_id.decode() - if isinstance(message_id, bytes) - else str(message_id), - "error": str(exc), - "dlq_retries": "0", - }) + dlq_entry = {k.decode(): v.decode() for k, v in data.items()} + dlq_entry.update( + { + "type": "processing_failed", + "original_message_id": message_id.decode() + if isinstance(message_id, bytes) + else str(message_id), + "error": str(exc), + "dlq_retries": "0", + } + ) redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_entry) # ACK so it doesn't stay stranded in PEL redis_conn.xack(stream_key, group_name, message_id) @@ -410,8 +410,10 @@ def _handle_entity_notification( except model_class.DoesNotExist: logger.exception("%s matching ID not found.", model_class.__name__) + raise except Exception: logger.exception("Error handling %s event", notification_type) + raise def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_name): """Recover and reprocess stuck messages from PEL.""" @@ -435,17 +437,17 @@ def recover_pending_messages(self, redis_conn, stream_key, group_name, consumer_ self.stdout.write(f"Successfully recovered message {message_id}") except Exception as exc: logger.exception("Failed to recover message %s", message_id) - dlq_entry = { - k.decode(): v.decode() for k, v in data.items() - } - dlq_entry.update({ - "type": "recovery_failed", - "original_message_id": message_id.decode() - if isinstance(message_id, bytes) - else str(message_id), - "error": str(exc), - "dlq_retries": "0", - }) + dlq_entry = {k.decode(): v.decode() for k, v in data.items()} + dlq_entry.update( + { + "type": "recovery_failed", + "original_message_id": message_id.decode() + if isinstance(message_id, bytes) + else str(message_id), + "error": str(exc), + "dlq_retries": "0", + } + ) redis_conn.xadd(self.DLQ_STREAM_KEY, dlq_entry) redis_conn.xack(stream_key, group_name, message_id) else: From f74a9d47c6086b7860a211599cf976c92b9d4903 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Mon, 6 Apr 2026 09:57:31 +0600 Subject: [PATCH 18/22] refactor: use shared notification utility for worker and DLQ idempotency - Extracted send_notification business logic out of the worker class and moved it to utils/notifications.py to serve as a shared utility - Updated DLQ retry command to use the shared send_notification utility so that manually retried messages go through the exact same deduplication checks and Notification model persistence as the worker - Updated worker and related unit tests to use the new shared utilit sonarcloud security warning for hardcoded http:// URLs in event test --- .../commands/owasp_notification_dlq.py | 45 ++++++++++++++----- .../commands/owasp_run_notification_worker.py | 35 ++------------- backend/apps/owasp/utils/notifications.py | 32 +++++++++++++ .../owasp_run_notification_worker_test.py | 18 ++++---- .../tests/apps/owasp/signals/event_test.py | 4 +- 5 files changed, 81 insertions(+), 53 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_notification_dlq.py b/backend/apps/owasp/management/commands/owasp_notification_dlq.py index 55e4c793db..a268c582d9 100644 --- a/backend/apps/owasp/management/commands/owasp_notification_dlq.py +++ b/backend/apps/owasp/management/commands/owasp_notification_dlq.py @@ -6,6 +6,9 @@ from django.core.management.base import BaseCommand from django_redis import get_redis_connection +from apps.nest.models import User +from apps.owasp.utils.notifications import send_notification + class Command(BaseCommand): """Manage notification DLQ manually.""" @@ -106,17 +109,39 @@ def retry_dlq(self, redis_conn, message_id, all_messages): message = self._get_value(data, b"message") related_link = self._get_value(data, b"related_link") + user_id = self._get_value(data, b"user_id") + notification_type = self._get_value(data, b"notification_type") + if user_email and title and message: - full_message = ( - f"{message}\n\nView: {related_link}" if related_link else message - ) - send_mail( - subject=title, - message=full_message, - from_email="noreply@owasp.org", - recipient_list=[user_email], - fail_silently=False, - ) + if user_id and notification_type: + try: + user = User.objects.get(id=int(user_id)) + send_notification( + user=user, + title=title, + message=message, + notification_type=notification_type, + related_link=related_link or "", + ) + except User.DoesNotExist: + self.stdout.write( + self.style.WARNING(f"User {user_id} not found: {msg_id}") + ) + error_count += 1 + continue + else: + # Fallback for old DLQ format + full_message = ( + f"{message}\n\nView: {related_link}" if related_link else message + ) + send_mail( + subject=title, + message=full_message, + from_email="noreply@owasp.org", + recipient_list=[user_email], + fail_silently=False, + ) + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) success_count += 1 self.stdout.write(f"Retried: {msg_id} -> {user_email}") diff --git a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py index 14fb13674a..07d6254f3d 100644 --- a/backend/apps/owasp/management/commands/owasp_run_notification_worker.py +++ b/backend/apps/owasp/management/commands/owasp_run_notification_worker.py @@ -8,14 +8,14 @@ from django.conf import settings from django.contrib.contenttypes.models import ContentType -from django.core.mail import send_mail from django.core.management.base import BaseCommand from django_redis import get_redis_connection from apps.owasp.models.chapter import Chapter from apps.owasp.models.event import Event -from apps.owasp.models.notification import Notification, Subscription +from apps.owasp.models.notification import Subscription from apps.owasp.models.snapshot import Snapshot +from apps.owasp.utils.notifications import send_notification logger = logging.getLogger(__name__) @@ -133,7 +133,7 @@ def send_notification_with_retry( while retry_count <= self.MAX_RETRIES: try: - self.send_notification( + send_notification( user=user, title=title, message=message, @@ -172,35 +172,6 @@ def send_notification_with_retry( return False - def send_notification(self, *, user, title, message, notification_type, related_link): - """Send notification to user.""" - if Notification.objects.filter( - recipient_id=user.id, - type=notification_type, - related_link=related_link, - message=message, - ).exists(): - logger.info("Already notified %s for %s, skipping", user.email, notification_type) - return - - full_message = f"{message}\n\nView: {related_link}" if related_link else message - send_mail( - subject=title, - message=full_message, - from_email="noreply@owasp.org", - recipient_list=[user.email], - fail_silently=False, - ) - logger.info("Sent %s email to %s", notification_type, user.email) - - Notification.objects.create( - recipient=user, - type=notification_type, - title=title, - message=message, - related_link=related_link, - ) - def handle_snapshot_published(self, data): """Handle snapshot published event.""" self._handle_entity_notification( diff --git a/backend/apps/owasp/utils/notifications.py b/backend/apps/owasp/utils/notifications.py index e349dc3297..633f5b2725 100644 --- a/backend/apps/owasp/utils/notifications.py +++ b/backend/apps/owasp/utils/notifications.py @@ -3,11 +3,13 @@ import json import logging +from django.core.mail import send_mail from django.utils.timezone import now from django_redis import get_redis_connection from apps.owasp.models.chapter import Chapter from apps.owasp.models.event import Event +from apps.owasp.models.notification import Notification from apps.owasp.models.snapshot import Snapshot logger = logging.getLogger(__name__) @@ -101,3 +103,33 @@ def publish_event_notification( msg_type, event.id, ) + + +def send_notification(*, user, title, message, notification_type, related_link): + """Send notification to user and persist to DB.""" + if Notification.objects.filter( + recipient_id=user.id, + type=notification_type, + related_link=related_link, + message=message, + ).exists(): + logger.info("Already notified %s for %s, skipping", user.email, notification_type) + return + + full_message = f"{message}\n\nView: {related_link}" if related_link else message + send_mail( + subject=title, + message=full_message, + from_email="noreply@owasp.org", + recipient_list=[user.email], + fail_silently=False, + ) + logger.info("Sent %s email to %s", notification_type, user.email) + + Notification.objects.create( + recipient=user, + type=notification_type, + title=title, + message=message, + related_link=related_link, + ) diff --git a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py index 91b252d735..e3827da83e 100644 --- a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py +++ b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py @@ -27,17 +27,16 @@ def mock_snapshot(self): snapshot.key = "2025-02" return snapshot - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.send_mail") - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Notification") - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.settings") + @mock.patch("apps.owasp.utils.notifications.send_mail") + @mock.patch("apps.owasp.utils.notifications.Notification") def test_send_notification_success( - self, mock_settings, mock_notification, mock_send_mail, command, mock_user, mock_snapshot + self, mock_notification, mock_send_mail, command, mock_user, mock_snapshot ): """Test successful notification sending.""" - mock_settings.SITE_URL = "https://example.com" mock_notification.objects.filter.return_value.exists.return_value = False - command.send_notification( + from apps.owasp.utils.notifications import send_notification + send_notification( user=mock_user, title=f"New Snapshot Published: {mock_snapshot.title}", message=f"Check out the latest OWASP snapshot: {mock_snapshot.title}", @@ -54,15 +53,16 @@ def test_send_notification_success( related_link=f"https://example.com/community/snapshots/{mock_snapshot.key}", ) - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.send_mail") - @mock.patch("apps.owasp.management.commands.owasp_run_notification_worker.Notification") + @mock.patch("apps.owasp.utils.notifications.send_mail") + @mock.patch("apps.owasp.utils.notifications.Notification") def test_send_notification_idempotency( self, mock_notification, mock_send_mail, command, mock_user, mock_snapshot ): """Test that notification is skipped if it already exists.""" mock_notification.objects.filter.return_value.exists.return_value = True - command.send_notification( + from apps.owasp.utils.notifications import send_notification + send_notification( user=mock_user, title=f"New Snapshot Published: {mock_snapshot.title}", message=f"Check out the latest OWASP snapshot: {mock_snapshot.title}", diff --git a/backend/tests/apps/owasp/signals/event_test.py b/backend/tests/apps/owasp/signals/event_test.py index c2f04f5fb8..8e68c88944 100644 --- a/backend/tests/apps/owasp/signals/event_test.py +++ b/backend/tests/apps/owasp/signals/event_test.py @@ -25,7 +25,7 @@ def test_event_updated_publishes_updated_notification(self, mock_publish): "start_date": "2026-01-01", "end_date": "2026-01-02", "suggested_location": "Test Location", - "url": "http://test.com", + "url": "https://test.com", "description": "Test description", } # Set current values to be the same - no notification should be sent @@ -33,7 +33,7 @@ def test_event_updated_publishes_updated_notification(self, mock_publish): event.start_date = "2026-01-01" event.end_date = "2026-01-02" event.suggested_location = "Test Location" - event.url = "http://test.com" + event.url = "https://test.com" event.description = "Test description" event_post_save(sender=None, instance=event, created=False) # No changes, so no notification should be published From 5ca42241d390db668fe27887ee11902f184b3b4b Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Tue, 7 Apr 2026 08:40:50 +0600 Subject: [PATCH 19/22] fix: address feedback for signal robustness and test coverage - Wrapped all post_save publishers (snapshot, chapter, event) in transaction.on_commit() to prevent phantom Redis messages during rollbacks - Fixed falsy dictionary parsing in signals so empty strings aren't converted to None - Updated 0073 migration to use PositiveBigIntegerField for GenericForeignKey object_id to prevent overflow with large integer keys - Added missing days_remaining payload assertion in deadline reminder test - Fixed chapter and event signal tests to properly cover the meaningful-update publish path instead of just the no-change skip path - Added transaction.on_commit mock decorators to test classes to pass Pytest strict database-access rules --- .../0073_notification_subscription.py | 2 +- backend/apps/owasp/signals/chapter.py | 11 +++-- backend/apps/owasp/signals/event.py | 13 ++++-- backend/apps/owasp/signals/snapshot.py | 3 +- .../owasp_run_notification_worker_test.py | 2 + .../tests/apps/owasp/signals/chapter_test.py | 37 +++++++++++++++-- .../tests/apps/owasp/signals/event_test.py | 41 +++++++++++++++++-- .../apps/owasp/utils/notifications_test.py | 3 +- 8 files changed, 95 insertions(+), 17 deletions(-) diff --git a/backend/apps/owasp/migrations/0073_notification_subscription.py b/backend/apps/owasp/migrations/0073_notification_subscription.py index fa5f403f68..f86ac760a1 100644 --- a/backend/apps/owasp/migrations/0073_notification_subscription.py +++ b/backend/apps/owasp/migrations/0073_notification_subscription.py @@ -51,7 +51,7 @@ class Migration(migrations.Migration): auto_created=True, primary_key=True, serialize=False, verbose_name="ID" ), ), - ("object_id", models.PositiveIntegerField()), + ("object_id", models.PositiveBigIntegerField()), ("created_at", models.DateTimeField(auto_now_add=True)), ( "content_type", diff --git a/backend/apps/owasp/signals/chapter.py b/backend/apps/owasp/signals/chapter.py index c34d1b5917..f8973fac92 100644 --- a/backend/apps/owasp/signals/chapter.py +++ b/backend/apps/owasp/signals/chapter.py @@ -1,5 +1,6 @@ """Chapter signals.""" +from django.db import transaction from django.db.models.signals import post_save, pre_save from django.dispatch import receiver @@ -26,7 +27,7 @@ def chapter_pre_save(sender, instance, **kwargs): # noqa: ARG001 def chapter_post_save(sender, instance, created, **kwargs): # noqa: ARG001 """Signal handler for chapter creation and updates.""" if created: - publish_chapter_notification(instance, "created") + transaction.on_commit(lambda: publish_chapter_notification(instance, "created")) else: changed_fields = {} previous_values = getattr(instance, "_previous_values", {}) @@ -35,9 +36,11 @@ def chapter_post_save(sender, instance, created, **kwargs): # noqa: ARG001 new_value = getattr(instance, field) if old_value != new_value: changed_fields[field] = { - "old": str(old_value) if old_value else None, - "new": str(new_value) if new_value else None, + "old": str(old_value) if old_value is not None else None, + "new": str(new_value) if new_value is not None else None, } if changed_fields: - publish_chapter_notification(instance, "updated", changed_fields) + transaction.on_commit( + lambda: publish_chapter_notification(instance, "updated", changed_fields) + ) diff --git a/backend/apps/owasp/signals/event.py b/backend/apps/owasp/signals/event.py index 2689597acc..582f49fa42 100644 --- a/backend/apps/owasp/signals/event.py +++ b/backend/apps/owasp/signals/event.py @@ -1,5 +1,6 @@ """Event signals.""" +from django.db import transaction from django.db.models.signals import post_save, pre_save from django.dispatch import receiver @@ -33,7 +34,7 @@ def event_pre_save(sender, instance, **kwargs): # noqa: ARG001 def event_post_save(sender, instance, created, **kwargs): # noqa: ARG001 """Signal handler for event creation and updates.""" if created: - publish_event_notification(instance, "created") + transaction.on_commit(lambda: publish_event_notification(instance, "created")) else: changed_fields = {} previous_values = getattr(instance, "_previous_values", {}) @@ -42,9 +43,13 @@ def event_post_save(sender, instance, created, **kwargs): # noqa: ARG001 new_value = getattr(instance, field) if old_value != new_value: changed_fields[field] = { - "old": str(old_value) if old_value else None, - "new": str(new_value) if new_value else None, + "old": str(old_value) if old_value is not None else None, + "new": str(new_value) if new_value is not None else None, } if changed_fields: - publish_event_notification(instance, "updated", changed_fields=changed_fields) + transaction.on_commit( + lambda: publish_event_notification( + instance, "updated", changed_fields=changed_fields + ) + ) diff --git a/backend/apps/owasp/signals/snapshot.py b/backend/apps/owasp/signals/snapshot.py index f8b283dd28..9430432d10 100644 --- a/backend/apps/owasp/signals/snapshot.py +++ b/backend/apps/owasp/signals/snapshot.py @@ -1,5 +1,6 @@ """Snapshot signals.""" +from django.db import transaction from django.db.models.signals import post_save, pre_save from django.dispatch import receiver @@ -24,4 +25,4 @@ def snapshot_published(sender, instance, created, **kwargs): # noqa: ARG001 if instance.status == Snapshot.Status.COMPLETED and ( created or instance._previous_status != Snapshot.Status.COMPLETED # noqa: SLF001 ): - publish_snapshot_notification(instance) + transaction.on_commit(lambda: publish_snapshot_notification(instance)) diff --git a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py index e3827da83e..4e33b20f7a 100644 --- a/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py +++ b/backend/tests/apps/owasp/management/commands/owasp_run_notification_worker_test.py @@ -36,6 +36,7 @@ def test_send_notification_success( mock_notification.objects.filter.return_value.exists.return_value = False from apps.owasp.utils.notifications import send_notification + send_notification( user=mock_user, title=f"New Snapshot Published: {mock_snapshot.title}", @@ -62,6 +63,7 @@ def test_send_notification_idempotency( mock_notification.objects.filter.return_value.exists.return_value = True from apps.owasp.utils.notifications import send_notification + send_notification( user=mock_user, title=f"New Snapshot Published: {mock_snapshot.title}", diff --git a/backend/tests/apps/owasp/signals/chapter_test.py b/backend/tests/apps/owasp/signals/chapter_test.py index 0b8f397bfd..289954cd3b 100644 --- a/backend/tests/apps/owasp/signals/chapter_test.py +++ b/backend/tests/apps/owasp/signals/chapter_test.py @@ -8,16 +8,18 @@ class TestChapterSignals: """Test chapter post_save signal handler.""" + @patch("apps.owasp.signals.chapter.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.chapter.publish_chapter_notification") - def test_chapter_created_publishes_created_notification(self, mock_publish): + def test_chapter_created_publishes_created_notification(self, mock_publish, mock_on_commit): """Test that creating a chapter publishes a 'created' notification.""" chapter = MagicMock() chapter_post_save(sender=None, instance=chapter, created=True) mock_publish.assert_called_once_with(chapter, "created") + @patch("apps.owasp.signals.chapter.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.chapter.publish_chapter_notification") - def test_chapter_updated_publishes_updated_notification(self, mock_publish): - """Test that updating a chapter publishes an 'updated' notification.""" + def test_chapter_updated_no_changes_skips_notification(self, mock_publish, mock_on_commit): + """Test that updating a chapter with no changes skips notification.""" chapter = MagicMock() # Set up previous values that match current values - no changes, no notification chapter._previous_values = { @@ -36,3 +38,32 @@ def test_chapter_updated_publishes_updated_notification(self, mock_publish): chapter_post_save(sender=None, instance=chapter, created=False) # No changes, so no notification should be published mock_publish.assert_not_called() + + @patch("apps.owasp.signals.chapter.transaction.on_commit", side_effect=lambda f: f()) + @patch("apps.owasp.signals.chapter.publish_chapter_notification") + def test_chapter_updated_publishes_updated_notification(self, mock_publish, mock_on_commit): + """Test that updating a chapter publishes an 'updated' notification.""" + chapter = MagicMock() + chapter._previous_values = { + "name": "Test Chapter", + "country": "Test Country", + "region": "Test Region", + "suggested_location": "Test Location", + "description": "Test description", + } + # Change a meaningful field + chapter.name = "New Test Chapter Name" + chapter.country = "Test Country" + chapter.region = "Test Region" + chapter.suggested_location = "Test Location" + chapter.description = "Test description" + + chapter_post_save(sender=None, instance=chapter, created=False) + + expected_changed_fields = { + "name": { + "old": "Test Chapter", + "new": "New Test Chapter Name", + } + } + mock_publish.assert_called_once_with(chapter, "updated", expected_changed_fields) diff --git a/backend/tests/apps/owasp/signals/event_test.py b/backend/tests/apps/owasp/signals/event_test.py index 8e68c88944..21f797bb33 100644 --- a/backend/tests/apps/owasp/signals/event_test.py +++ b/backend/tests/apps/owasp/signals/event_test.py @@ -8,16 +8,18 @@ class TestEventSignals: """Test event post_save signal handler.""" + @patch("apps.owasp.signals.event.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.event.publish_event_notification") - def test_event_created_publishes_created_notification(self, mock_publish): + def test_event_created_publishes_created_notification(self, mock_publish, mock_on_commit): """Test that creating an event publishes a 'created' notification.""" event = MagicMock() event_post_save(sender=None, instance=event, created=True) mock_publish.assert_called_once_with(event, "created") + @patch("apps.owasp.signals.event.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.event.publish_event_notification") - def test_event_updated_publishes_updated_notification(self, mock_publish): - """Test that updating an event publishes an 'updated' notification.""" + def test_event_updated_no_changes_skips_notification(self, mock_publish, mock_on_commit): + """Test that updating an event with no meaningful changes skips notification.""" event = MagicMock() # Set up previous values that match current values - no changes, no notification event._previous_values = { @@ -38,3 +40,36 @@ def test_event_updated_publishes_updated_notification(self, mock_publish): event_post_save(sender=None, instance=event, created=False) # No changes, so no notification should be published mock_publish.assert_not_called() + + @patch("apps.owasp.signals.event.transaction.on_commit", side_effect=lambda f: f()) + @patch("apps.owasp.signals.event.publish_event_notification") + def test_event_updated_publishes_updated_notification(self, mock_publish, mock_on_commit): + """Test that updating an event with meaningful changes publishes a notification.""" + event = MagicMock() + event._previous_values = { + "name": "Test Event", + "start_date": "2026-01-01", + "end_date": "2026-01-02", + "suggested_location": "Test Location", + "url": "https://test.com", + "description": "Test description", + } + # Change a meaningful field + event.name = "New Test Event Name" + event.start_date = "2026-01-01" + event.end_date = "2026-01-02" + event.suggested_location = "Test Location" + event.url = "https://test.com" + event.description = "Test description" + + event_post_save(sender=None, instance=event, created=False) + + expected_changed_fields = { + "name": { + "old": "Test Event", + "new": "New Test Event Name", + } + } + mock_publish.assert_called_once_with( + event, "updated", changed_fields=expected_changed_fields + ) diff --git a/backend/tests/apps/owasp/utils/notifications_test.py b/backend/tests/apps/owasp/utils/notifications_test.py index f19850cedc..391df7b595 100644 --- a/backend/tests/apps/owasp/utils/notifications_test.py +++ b/backend/tests/apps/owasp/utils/notifications_test.py @@ -117,10 +117,11 @@ def test_publishes_deadline_reminder_notification(self, mock_redis): event = MagicMock() event.id = 10 - publish_event_notification(event, "deadline_reminder") + publish_event_notification(event, "deadline_reminder", days_remaining="5") call_args = mock_conn.xadd.call_args assert call_args[0][1]["type"] == "event_deadline_reminder" + assert call_args[0][1]["days_remaining"] == "5" @patch("apps.owasp.utils.notifications.get_redis_connection") def test_handles_redis_error(self, mock_redis): From beb6da7f4ea20a734aefffcf7e78f6c7eb061531 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Tue, 7 Apr 2026 11:25:06 +0600 Subject: [PATCH 20/22] fix: refine signal reliability and DLQ error handling - Patched lambda late-binding bug in all signals by using default argument binding (inst=instance) to capture immutable snapshots - Hardened DLQ retry logic to explicitly delete messages for missing users, preventing infinite 'Zombie' retry loops in Redis - Added explicit mock_on_commit assertions to chapter and event tests to ensure notifications are always scheduled within transactions - Standardized deadline reminder tests to pass integer days_remaining while verifying stringified Redis payloads --- .../owasp/management/commands/owasp_notification_dlq.py | 1 + backend/apps/owasp/signals/chapter.py | 6 ++++-- backend/apps/owasp/signals/event.py | 6 +++--- backend/apps/owasp/signals/snapshot.py | 2 +- backend/tests/apps/owasp/signals/chapter_test.py | 3 +++ backend/tests/apps/owasp/signals/event_test.py | 3 +++ backend/tests/apps/owasp/utils/notifications_test.py | 2 +- 7 files changed, 16 insertions(+), 7 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_notification_dlq.py b/backend/apps/owasp/management/commands/owasp_notification_dlq.py index a268c582d9..4f35f8fd26 100644 --- a/backend/apps/owasp/management/commands/owasp_notification_dlq.py +++ b/backend/apps/owasp/management/commands/owasp_notification_dlq.py @@ -128,6 +128,7 @@ def retry_dlq(self, redis_conn, message_id, all_messages): self.style.WARNING(f"User {user_id} not found: {msg_id}") ) error_count += 1 + redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) continue else: # Fallback for old DLQ format diff --git a/backend/apps/owasp/signals/chapter.py b/backend/apps/owasp/signals/chapter.py index f8973fac92..5069b75c7f 100644 --- a/backend/apps/owasp/signals/chapter.py +++ b/backend/apps/owasp/signals/chapter.py @@ -27,7 +27,7 @@ def chapter_pre_save(sender, instance, **kwargs): # noqa: ARG001 def chapter_post_save(sender, instance, created, **kwargs): # noqa: ARG001 """Signal handler for chapter creation and updates.""" if created: - transaction.on_commit(lambda: publish_chapter_notification(instance, "created")) + transaction.on_commit(lambda inst=instance: publish_chapter_notification(inst, "created")) else: changed_fields = {} previous_values = getattr(instance, "_previous_values", {}) @@ -42,5 +42,7 @@ def chapter_post_save(sender, instance, created, **kwargs): # noqa: ARG001 if changed_fields: transaction.on_commit( - lambda: publish_chapter_notification(instance, "updated", changed_fields) + lambda inst=instance, cf=changed_fields: publish_chapter_notification( + inst, "updated", cf + ) ) diff --git a/backend/apps/owasp/signals/event.py b/backend/apps/owasp/signals/event.py index 582f49fa42..dbacfa91ef 100644 --- a/backend/apps/owasp/signals/event.py +++ b/backend/apps/owasp/signals/event.py @@ -34,7 +34,7 @@ def event_pre_save(sender, instance, **kwargs): # noqa: ARG001 def event_post_save(sender, instance, created, **kwargs): # noqa: ARG001 """Signal handler for event creation and updates.""" if created: - transaction.on_commit(lambda: publish_event_notification(instance, "created")) + transaction.on_commit(lambda inst=instance: publish_event_notification(inst, "created")) else: changed_fields = {} previous_values = getattr(instance, "_previous_values", {}) @@ -49,7 +49,7 @@ def event_post_save(sender, instance, created, **kwargs): # noqa: ARG001 if changed_fields: transaction.on_commit( - lambda: publish_event_notification( - instance, "updated", changed_fields=changed_fields + lambda inst=instance, cf=changed_fields: publish_event_notification( + inst, "updated", changed_fields=cf ) ) diff --git a/backend/apps/owasp/signals/snapshot.py b/backend/apps/owasp/signals/snapshot.py index 9430432d10..69d49dc95b 100644 --- a/backend/apps/owasp/signals/snapshot.py +++ b/backend/apps/owasp/signals/snapshot.py @@ -25,4 +25,4 @@ def snapshot_published(sender, instance, created, **kwargs): # noqa: ARG001 if instance.status == Snapshot.Status.COMPLETED and ( created or instance._previous_status != Snapshot.Status.COMPLETED # noqa: SLF001 ): - transaction.on_commit(lambda: publish_snapshot_notification(instance)) + transaction.on_commit(lambda inst=instance: publish_snapshot_notification(inst)) diff --git a/backend/tests/apps/owasp/signals/chapter_test.py b/backend/tests/apps/owasp/signals/chapter_test.py index 289954cd3b..1edfe754ea 100644 --- a/backend/tests/apps/owasp/signals/chapter_test.py +++ b/backend/tests/apps/owasp/signals/chapter_test.py @@ -15,6 +15,7 @@ def test_chapter_created_publishes_created_notification(self, mock_publish, mock chapter = MagicMock() chapter_post_save(sender=None, instance=chapter, created=True) mock_publish.assert_called_once_with(chapter, "created") + mock_on_commit.assert_called_once() @patch("apps.owasp.signals.chapter.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.chapter.publish_chapter_notification") @@ -38,6 +39,7 @@ def test_chapter_updated_no_changes_skips_notification(self, mock_publish, mock_ chapter_post_save(sender=None, instance=chapter, created=False) # No changes, so no notification should be published mock_publish.assert_not_called() + mock_on_commit.assert_not_called() @patch("apps.owasp.signals.chapter.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.chapter.publish_chapter_notification") @@ -67,3 +69,4 @@ def test_chapter_updated_publishes_updated_notification(self, mock_publish, mock } } mock_publish.assert_called_once_with(chapter, "updated", expected_changed_fields) + mock_on_commit.assert_called_once() diff --git a/backend/tests/apps/owasp/signals/event_test.py b/backend/tests/apps/owasp/signals/event_test.py index 21f797bb33..f45b949eab 100644 --- a/backend/tests/apps/owasp/signals/event_test.py +++ b/backend/tests/apps/owasp/signals/event_test.py @@ -15,6 +15,7 @@ def test_event_created_publishes_created_notification(self, mock_publish, mock_o event = MagicMock() event_post_save(sender=None, instance=event, created=True) mock_publish.assert_called_once_with(event, "created") + mock_on_commit.assert_called_once() @patch("apps.owasp.signals.event.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.event.publish_event_notification") @@ -40,6 +41,7 @@ def test_event_updated_no_changes_skips_notification(self, mock_publish, mock_on event_post_save(sender=None, instance=event, created=False) # No changes, so no notification should be published mock_publish.assert_not_called() + mock_on_commit.assert_not_called() @patch("apps.owasp.signals.event.transaction.on_commit", side_effect=lambda f: f()) @patch("apps.owasp.signals.event.publish_event_notification") @@ -73,3 +75,4 @@ def test_event_updated_publishes_updated_notification(self, mock_publish, mock_o mock_publish.assert_called_once_with( event, "updated", changed_fields=expected_changed_fields ) + mock_on_commit.assert_called_once() diff --git a/backend/tests/apps/owasp/utils/notifications_test.py b/backend/tests/apps/owasp/utils/notifications_test.py index 391df7b595..2028616c76 100644 --- a/backend/tests/apps/owasp/utils/notifications_test.py +++ b/backend/tests/apps/owasp/utils/notifications_test.py @@ -117,7 +117,7 @@ def test_publishes_deadline_reminder_notification(self, mock_redis): event = MagicMock() event.id = 10 - publish_event_notification(event, "deadline_reminder", days_remaining="5") + publish_event_notification(event, "deadline_reminder", days_remaining=5) call_args = mock_conn.xadd.call_args assert call_args[0][1]["type"] == "event_deadline_reminder" From 2f11ec0d2a088ecf664e0fb5f973a0090604eec7 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Wed, 8 Apr 2026 08:34:16 +0600 Subject: [PATCH 21/22] fix: harden DLQ command safety and signal execution reliability - Standardized DLQ command to use Django's CommandError for idiomatic error reporting and correct terminal exit codes - Patched DLQ command arguments with mutually exclusive groups to prevent accidental data loss from overlapping --id and --all flags - Improved DLQ logging to distinguish between retriable email failures and system-level worker failures requiring manual admin review - Implemented early-binding snapshots (inst=instance) for all signal lambdas to fix the Python late-binding closure bug in transactions - Strengthened test suite with mandatory on_commit mock assertions and resolved payload type mismatches in deadline reminder tests --- .../commands/owasp_notification_dlq.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_notification_dlq.py b/backend/apps/owasp/management/commands/owasp_notification_dlq.py index 4f35f8fd26..9fdd38fbab 100644 --- a/backend/apps/owasp/management/commands/owasp_notification_dlq.py +++ b/backend/apps/owasp/management/commands/owasp_notification_dlq.py @@ -1,9 +1,7 @@ """Management command to manage notification DLQ.""" -import sys - from django.core.mail import send_mail -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from django_redis import get_redis_connection from apps.nest.models import User @@ -24,12 +22,13 @@ def add_arguments(self, parser): choices=["list", "retry", "remove"], help="Action to perform: list, retry, or remove", ) - parser.add_argument( + group = parser.add_mutually_exclusive_group() + group.add_argument( "--id", type=str, help="Specific message ID to act on (required for retry/remove unless --all is used)", ) - parser.add_argument( + group.add_argument( "--all", action="store_true", help="Apply action to all messages", @@ -46,13 +45,11 @@ def handle(self, *args, **options): self.list_dlq(redis_conn) elif action == "retry": if not message_id and not all_messages: - self.stdout.write(self.style.ERROR("Error: --id or --all is required for retry")) - sys.exit(1) + raise CommandError("Error: --id or --all is required for retry") self.retry_dlq(redis_conn, message_id, all_messages) elif action == "remove": if not message_id and not all_messages: - self.stdout.write(self.style.ERROR("Error: --id or --all is required for remove")) - sys.exit(1) + raise CommandError("Error: --id or --all is required for remove") self.remove_dlq(redis_conn, message_id, all_messages) def list_dlq(self, redis_conn): @@ -93,8 +90,7 @@ def retry_dlq(self, redis_conn, message_id, all_messages): messages = result or [] if not messages: - self.stdout.write(self.style.ERROR("Message(s) not found")) - return + raise CommandError("Message(s) not found") success_count = 0 error_count = 0 @@ -128,7 +124,6 @@ def retry_dlq(self, redis_conn, message_id, all_messages): self.style.WARNING(f"User {user_id} not found: {msg_id}") ) error_count += 1 - redis_conn.xdel(self.DLQ_STREAM_KEY, msg_id) continue else: # Fallback for old DLQ format @@ -147,7 +142,16 @@ def retry_dlq(self, redis_conn, message_id, all_messages): success_count += 1 self.stdout.write(f"Retried: {msg_id} -> {user_email}") else: - self.stdout.write(self.style.WARNING(f"Skipped (missing data): {msg_id}")) + msg_type = self._get_value(data, b"type") + if msg_type in ["processing_failed", "recovery_failed"]: + self.stdout.write( + self.style.WARNING( + f"Skipped: {msg_id} is a system processing failure. " + "Requires manual admin review." + ) + ) + else: + self.stdout.write(self.style.WARNING(f"Skipped (missing data): {msg_id}")) error_count += 1 except Exception as e: # noqa: BLE001 @@ -176,8 +180,7 @@ def remove_dlq(self, redis_conn, message_id, all_messages): messages = redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id) if not messages: - self.stdout.write(self.style.ERROR("No messages found")) - return + raise CommandError("No messages found") count = 0 for msg_id, _ in messages: From aba550e949a691543d9f7c4638e249c28dfd5777 Mon Sep 17 00:00:00 2001 From: shofikul islam Date: Wed, 8 Apr 2026 09:21:49 +0600 Subject: [PATCH 22/22] fix: resolve Ruff linting errors in DLQ management command - Fixed TRY003 and EM101 errors in owasp_notification_dlq.py by assigning exception string literals to variables before raising CommandError - Ensured compliance with project linting rules for better error traceability and cleaner exception handling --- .../management/commands/owasp_notification_dlq.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/backend/apps/owasp/management/commands/owasp_notification_dlq.py b/backend/apps/owasp/management/commands/owasp_notification_dlq.py index 9fdd38fbab..25fcb24d82 100644 --- a/backend/apps/owasp/management/commands/owasp_notification_dlq.py +++ b/backend/apps/owasp/management/commands/owasp_notification_dlq.py @@ -45,11 +45,13 @@ def handle(self, *args, **options): self.list_dlq(redis_conn) elif action == "retry": if not message_id and not all_messages: - raise CommandError("Error: --id or --all is required for retry") + msg = "Error: --id or --all is required for retry" + raise CommandError(msg) self.retry_dlq(redis_conn, message_id, all_messages) elif action == "remove": if not message_id and not all_messages: - raise CommandError("Error: --id or --all is required for remove") + msg = "Error: --id or --all is required for remove" + raise CommandError(msg) self.remove_dlq(redis_conn, message_id, all_messages) def list_dlq(self, redis_conn): @@ -90,7 +92,8 @@ def retry_dlq(self, redis_conn, message_id, all_messages): messages = result or [] if not messages: - raise CommandError("Message(s) not found") + msg = "Message(s) not found" + raise CommandError(msg) success_count = 0 error_count = 0 @@ -180,7 +183,8 @@ def remove_dlq(self, redis_conn, message_id, all_messages): messages = redis_conn.xrange(self.DLQ_STREAM_KEY, message_id, message_id) if not messages: - raise CommandError("No messages found") + msg = "No messages found" + raise CommandError(msg) count = 0 for msg_id, _ in messages: