mirror of
https://github.com/lingble/chatwoot.git
synced 2025-10-29 18:22:53 +00:00
feat: use redis set with nx to prevent race conditions
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user