From d90fa6a83754d02d0cc8be6853fac4cf16052cc1 Mon Sep 17 00:00:00 2001
From: Shivam Mishra <scm.mymail@gmail.com>
Date: Wed, 15 Oct 2025 13:45:03 +0530
Subject: [PATCH] feat: use redis set with `nx` to prevent race conditions

---
 .../messages/send_email_notification_service.rb     |  9 ++++-----
 .../send_email_notification_service_spec.rb         | 13 ++++++-------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/app/services/messages/send_email_notification_service.rb b/app/services/messages/send_email_notification_service.rb
index 44dca4144..f1db920d0 100644
--- a/app/services/messages/send_email_notification_service.rb
+++ b/app/services/messages/send_email_notification_service.rb
@@ -7,12 +7,11 @@ class Messages::SendEmailNotificationService
     conversation = message.conversation
     conversation_mail_key = format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: conversation.id)
 
-    # will set a redis key for the conversation so that we don't need to send email for every new message
-    # last few messages coupled together is sent every 2 minutes rather than one email for each message
-    # if redis key exists there is an unprocessed job that will take care of delivering the email
-    return if Redis::Alfred.get(conversation_mail_key).present?
+    # Atomically set redis key to prevent duplicate email workers
+    # Only the first message in a 2-minute window will successfully set the key and enqueue the worker
+    # Subsequent messages will fail to set the key (returns false) and skip enqueueing
+    return unless Redis::Alfred.set(conversation_mail_key, message.id, nx: true, ex: 2.minutes.to_i)
 
-    Redis::Alfred.setex(conversation_mail_key, message.id)
     ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, message.id)
   end
 
diff --git a/spec/services/messages/send_email_notification_service_spec.rb b/spec/services/messages/send_email_notification_service_spec.rb
index 3de863252..232a8f53c 100644
--- a/spec/services/messages/send_email_notification_service_spec.rb
+++ b/spec/services/messages/send_email_notification_service_spec.rb
@@ -13,8 +13,7 @@ describe Messages::SendEmailNotificationService do
 
       before do
         conversation.contact.update!(email: 'test@example.com')
-        allow(Redis::Alfred).to receive(:get).and_return(nil)
-        allow(Redis::Alfred).to receive(:setex)
+        allow(Redis::Alfred).to receive(:set).and_return(true)
         allow(ConversationReplyEmailWorker).to receive(:perform_in)
       end
 
@@ -28,17 +27,17 @@ describe Messages::SendEmailNotificationService do
         )
       end
 
-      it 'sets redis key to prevent duplicate emails' do
+      it 'atomically sets redis key to prevent duplicate emails' do
         expected_key = format(Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: conversation.id)
 
         service.perform
 
-        expect(Redis::Alfred).to have_received(:setex).with(expected_key, message.id)
+        expect(Redis::Alfred).to have_received(:set).with(expected_key, message.id, nx: true, ex: 2.minutes.to_i)
       end
 
       context 'when redis key already exists' do
         before do
-          allow(Redis::Alfred).to receive(:get).and_return('existing_key')
+          allow(Redis::Alfred).to receive(:set).and_return(false)
         end
 
         it 'does not schedule worker' do
@@ -47,10 +46,10 @@ describe Messages::SendEmailNotificationService do
           expect(ConversationReplyEmailWorker).not_to have_received(:perform_in)
         end
 
-        it 'does not set redis key' do
+        it 'attempts atomic set once' do
           service.perform
 
-          expect(Redis::Alfred).not_to have_received(:setex)
+          expect(Redis::Alfred).to have_received(:set).once
         end
       end
     end