From 0ff1c3ab8201452e6538b65c52e9343298ea11d0 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 30 Oct 2025 15:47:32 +0530 Subject: [PATCH] feat: add specs for strategies --- .../mailbox/conversation_finder_spec.rb | 123 +++++++++++ .../in_reply_to_strategy_spec.rb | 118 ++++++++++ .../receiver_uuid_strategy_spec.rb | 99 +++++++++ .../references_strategy_spec.rb | 208 ++++++++++++++++++ 4 files changed, 548 insertions(+) create mode 100644 spec/services/mailbox/conversation_finder_spec.rb create mode 100644 spec/services/mailbox/conversation_finder_strategies/in_reply_to_strategy_spec.rb create mode 100644 spec/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy_spec.rb create mode 100644 spec/services/mailbox/conversation_finder_strategies/references_strategy_spec.rb diff --git a/spec/services/mailbox/conversation_finder_spec.rb b/spec/services/mailbox/conversation_finder_spec.rb new file mode 100644 index 000000000..174462b2a --- /dev/null +++ b/spec/services/mailbox/conversation_finder_spec.rb @@ -0,0 +1,123 @@ +require 'rails_helper' + +RSpec.describe Mailbox::ConversationFinder do + let(:account) { create(:account) } + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:conversation) { create(:conversation, inbox: email_channel.inbox, account: account) } + let(:mail) { Mail.new } + + describe '#find' do + context 'when receiver uuid strategy finds conversation' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'reply+12345678-1234-1234-1234-123456789012@example.com' + end + + it 'returns the conversation' do + finder = described_class.new(mail) + expect(finder.find).to eq(conversation) + end + + it 'logs which strategy succeeded' do + allow(Rails.logger).to receive(:info) + finder = described_class.new(mail) + finder.find + + expect(Rails.logger).to have_received(:info).with('Conversation found via receiver_uuid_strategy strategy') + end + end + + context 'when in_reply_to strategy finds conversation' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.in_reply_to = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'returns the conversation' do + finder = described_class.new(mail) + expect(finder.find).to eq(conversation) + end + + it 'logs which strategy succeeded' do + allow(Rails.logger).to receive(:info) + finder = described_class.new(mail) + finder.find + + expect(Rails.logger).to have_received(:info).with('Conversation found via in_reply_to_strategy strategy') + end + end + + context 'when references strategy finds conversation' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'test@example.com' + mail.references = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'returns the conversation' do + finder = described_class.new(mail) + expect(finder.find).to eq(conversation) + end + + it 'logs which strategy succeeded' do + allow(Rails.logger).to receive(:info) + finder = described_class.new(mail) + finder.find + + expect(Rails.logger).to have_received(:info).with('Conversation found via references_strategy strategy') + end + end + + context 'when no strategy finds conversation' do + it 'returns nil' do + finder = described_class.new(mail) + expect(finder.find).to be_nil + end + + it 'logs that no conversation was found' do + allow(Rails.logger).to receive(:info) + finder = described_class.new(mail) + finder.find + + expect(Rails.logger).to have_received(:info).with('No conversation found via any strategy') + end + end + + context 'with custom strategies' do + let(:custom_strategy_class) do + Class.new(Mailbox::ConversationFinderStrategies::BaseStrategy) do + def find + # Always return nil for testing + nil + end + end + end + + it 'uses provided strategies instead of defaults' do + finder = described_class.new(mail, strategies: [custom_strategy_class]) + expect(finder.find).to be_nil + end + end + + context 'strategy execution order' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + + # Set up mail so all strategies could match + mail.to = 'reply+12345678-1234-1234-1234-123456789012@example.com' + mail.in_reply_to = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + mail.references = 'conversation/12345678-1234-1234-1234-123456789012/messages/456@example.com' + end + + it 'returns conversation from first matching strategy' do + allow(Rails.logger).to receive(:info) + finder = described_class.new(mail) + result = finder.find + + expect(result).to eq(conversation) + # Should only log the first strategy that succeeded (ReceiverUuidStrategy) + expect(Rails.logger).to have_received(:info).once.with('Conversation found via receiver_uuid_strategy strategy') + end + end + end +end diff --git a/spec/services/mailbox/conversation_finder_strategies/in_reply_to_strategy_spec.rb b/spec/services/mailbox/conversation_finder_strategies/in_reply_to_strategy_spec.rb new file mode 100644 index 000000000..3fca8c3c9 --- /dev/null +++ b/spec/services/mailbox/conversation_finder_strategies/in_reply_to_strategy_spec.rb @@ -0,0 +1,118 @@ +require 'rails_helper' + +RSpec.describe Mailbox::ConversationFinderStrategies::InReplyToStrategy do + let(:account) { create(:account) } + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:conversation) { create(:conversation, inbox: email_channel.inbox, account: account) } + let(:mail) { Mail.new } + + describe '#find' do + context 'when in_reply_to has message-specific pattern' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.in_reply_to = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'extracts UUID and returns conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when in_reply_to has conversation fallback pattern' do + before do + conversation.update!(uuid: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee') + mail.in_reply_to = "account/#{account.id}/conversation/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee@example.com" + end + + it 'extracts UUID and returns conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when in_reply_to matches message source_id' do + let(:message) do + conversation.messages.create!( + source_id: 'original-message-id@example.com', + account_id: account.id, + message_type: 'outgoing', + inbox_id: email_channel.inbox.id, + content: 'Original message' + ) + end + + before do + message # Create the message + mail.in_reply_to = 'original-message-id@example.com' + end + + it 'finds conversation from message source_id' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when in_reply_to has multiple values' do + let(:message) do + conversation.messages.create!( + source_id: 'message-123@example.com', + account_id: account.id, + message_type: 'outgoing', + inbox_id: email_channel.inbox.id, + content: 'Test message' + ) + end + + before do + message # Create the message + mail.in_reply_to = ['some-other-id@example.com', 'message-123@example.com'] + end + + it 'finds conversation from any in_reply_to value' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when in_reply_to is blank' do + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when in_reply_to does not match any pattern or source_id' do + before do + mail.in_reply_to = 'random-message-id@gmail.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when UUID exists but conversation does not' do + before do + mail.in_reply_to = 'conversation/99999999-9999-9999-9999-999999999999/messages/123@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'with malformed in_reply_to pattern' do + before do + mail.in_reply_to = 'conversation/not-a-uuid/messages/123@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + end +end diff --git a/spec/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy_spec.rb b/spec/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy_spec.rb new file mode 100644 index 000000000..d45f6f4fe --- /dev/null +++ b/spec/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy_spec.rb @@ -0,0 +1,99 @@ +require 'rails_helper' + +RSpec.describe Mailbox::ConversationFinderStrategies::ReceiverUuidStrategy do + let(:account) { create(:account) } + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:conversation) { create(:conversation, inbox: email_channel.inbox, account: account) } + let(:mail) { Mail.new } + + describe '#find' do + context 'when mail has valid reply+uuid format' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'reply+12345678-1234-1234-1234-123456789012@example.com' + end + + it 'returns the conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when mail has uppercase UUID' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'reply+12345678-1234-1234-1234-123456789012@EXAMPLE.COM' + end + + it 'returns the conversation (case-insensitive matching)' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when mail has multiple recipients with valid UUID' do + before do + conversation.update!(uuid: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee') + mail.to = ['other@example.com', 'reply+aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee@example.com'] + end + + it 'extracts UUID from any recipient' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when UUID does not exist in database' do + before do + mail.to = 'reply+99999999-9999-9999-9999-999999999999@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when mail has no recipients' do + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when mail recipient has malformed UUID' do + before do + mail.to = 'reply+not-a-valid-uuid@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when mail recipient has no reply+ prefix' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'test+12345678-1234-1234-1234-123456789012@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when mail recipient has additional text after UUID' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'reply+12345678-1234-1234-1234-123456789012-extra@example.com' + end + + it 'returns nil (UUID must be exact)' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + end +end diff --git a/spec/services/mailbox/conversation_finder_strategies/references_strategy_spec.rb b/spec/services/mailbox/conversation_finder_strategies/references_strategy_spec.rb new file mode 100644 index 000000000..6ef61a58f --- /dev/null +++ b/spec/services/mailbox/conversation_finder_strategies/references_strategy_spec.rb @@ -0,0 +1,208 @@ +require 'rails_helper' + +RSpec.describe Mailbox::ConversationFinderStrategies::ReferencesStrategy do + let(:account) { create(:account) } + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:conversation) { create(:conversation, inbox: email_channel.inbox, account: account) } + let(:mail) { Mail.new } + + describe '#find' do + context 'when references has message-specific pattern' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'test@example.com' + mail.references = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'extracts UUID and returns conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when references has conversation fallback pattern' do + before do + conversation.update!(uuid: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee') + mail.to = 'test@example.com' + mail.references = "account/#{account.id}/conversation/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee@example.com" + end + + it 'extracts UUID and returns conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when references matches message source_id' do + let(:message) do + conversation.messages.create!( + source_id: 'original-message-id@example.com', + account_id: account.id, + message_type: 'outgoing', + inbox_id: email_channel.inbox.id, + content: 'Original message' + ) + end + + before do + message # Create the message + mail.to = 'test@example.com' + mail.references = 'original-message-id@example.com' + end + + it 'finds conversation from message source_id' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when references has multiple values' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'test@example.com' + mail.references = [ + 'some-random-message@gmail.com', + 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com', + 'another-message@outlook.com' + ] + end + + it 'finds conversation from any reference' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when references is blank' do + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when references does not match any pattern or source_id' do + before do + mail.references = 'random-message-id@gmail.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'with channel validation' do + context 'when conversation belongs to the correct channel' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'test@example.com' + mail.references = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'returns the conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + + context 'when conversation belongs to a different channel' do + let(:other_email_channel) { create(:channel_email, email: 'other@example.com', account: account) } + let(:other_conversation) do + create( + :conversation, + inbox: other_email_channel.inbox, + account: account + ) + end + + before do + other_conversation.update!(uuid: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee') + # Mail is addressed to test@example.com but references conversation from other@example.com + mail.to = 'test@example.com' + mail.references = 'conversation/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee/messages/456@example.com' + end + + it 'returns nil (prevents cross-channel hijacking)' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when channel cannot be determined from mail' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = 'unknown@example.com' # Email not associated with any channel + mail.references = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when mail has multiple recipients including correct channel' do + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + mail.to = ['other@example.com', 'test@example.com'] + mail.references = 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' + end + + it 'finds the correct channel and returns conversation' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + end + + context 'when UUID exists but conversation does not' do + before do + mail.to = 'test@example.com' + mail.references = 'conversation/99999999-9999-9999-9999-999999999999/messages/123@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'with malformed references pattern' do + before do + mail.references = 'conversation/not-a-uuid/messages/123@example.com' + end + + it 'returns nil' do + strategy = described_class.new(mail) + expect(strategy.find).to be_nil + end + end + + context 'when first reference fails channel validation but second succeeds' do + let(:other_email_channel) { create(:channel_email, email: 'other@example.com', account: account) } + let(:other_conversation) do + create( + :conversation, + inbox: other_email_channel.inbox, + account: account + ) + end + + before do + conversation.update!(uuid: '12345678-1234-1234-1234-123456789012') + other_conversation.update!(uuid: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee') + + mail.to = 'test@example.com' + mail.references = [ + 'conversation/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee/messages/456@example.com', # Wrong channel + 'conversation/12345678-1234-1234-1234-123456789012/messages/123@example.com' # Correct channel + ] + end + + it 'skips invalid reference and returns conversation from valid reference' do + strategy = described_class.new(mail) + expect(strategy.find).to eq(conversation) + end + end + end +end