From 841d9f756bf9d370fb9685e9f75dba32948640e8 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 30 Oct 2025 15:31:15 +0530 Subject: [PATCH] refactor: use a strategy based approach to find conversation --- app/mailboxes/reply_mailbox.rb | 127 +----------------- .../mailbox/conversation_finder_chain.rb | 28 ++++ .../base_strategy.rb | 13 ++ .../in_reply_to_strategy.rb | 40 ++++++ .../receiver_uuid_strategy.rb | 26 ++++ .../references_strategy.rb | 56 ++++++++ 6 files changed, 169 insertions(+), 121 deletions(-) create mode 100644 app/services/mailbox/conversation_finder_chain.rb create mode 100644 app/services/mailbox/conversation_finder_strategies/base_strategy.rb create mode 100644 app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb create mode 100644 app/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy.rb create mode 100644 app/services/mailbox/conversation_finder_strategies/references_strategy.rb diff --git a/app/mailboxes/reply_mailbox.rb b/app/mailboxes/reply_mailbox.rb index 7acd441ba..cddea684e 100644 --- a/app/mailboxes/reply_mailbox.rb +++ b/app/mailboxes/reply_mailbox.rb @@ -1,12 +1,7 @@ class ReplyMailbox < ApplicationMailbox - attr_accessor :conversation_uuid, :processed_mail + attr_accessor :conversation, :processed_mail - # Last part is the regex for the UUID - # Eg: email should be something like : reply+6bdc3f4d-0bec-4515-a284-5d916fdde489@domain.com - EMAIL_PART_PATTERN = /^reply\+([0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12})$/i - - before_processing :conversation_uuid_from_to_address, - :find_relative_conversation + before_processing :find_conversation def process return if @conversation.blank? @@ -18,122 +13,12 @@ class ReplyMailbox < ApplicationMailbox private - def find_relative_conversation - if @conversation_uuid - find_conversation_with_uuid - elsif mail.in_reply_to.present? - find_conversation_with_in_reply_to - elsif mail.references.present? - find_conversation_with_references - end - end + def find_conversation + @conversation = Mailbox::ConversationFinderChain.new(mail).find - def conversation_uuid_from_to_address - @mail = MailPresenter.new(mail) + return unless @conversation.nil? - return if @mail.mail_receiver.blank? - - @mail.mail_receiver.each do |email| - username = email.split('@')[0] - match_result = username.match(ApplicationMailbox::REPLY_EMAIL_UUID_PATTERN) - if match_result - @conversation_uuid = match_result.captures - break - end - end - @conversation_uuid - end - - # find conversation uuid from below pattern - # reply+@ - def find_conversation_with_uuid - @conversation = Conversation.find_by(uuid: conversation_uuid) - validate_resource @conversation - end - - def find_conversation_by_uuid(match_result) - @conversation_uuid = match_result.captures[0] - - find_conversation_with_uuid - end - - def find_conversation_by_message_id(in_reply_to) - @message = Message.find_by(source_id: in_reply_to) - @conversation = @message.conversation if @message.present? - @conversation_uuid = @conversation.uuid if @conversation.present? - end - - # find conversation uuid from below pattern - # - def find_conversation_with_in_reply_to - match_result = nil - in_reply_to_addresses = mail.in_reply_to - in_reply_to_addresses = [in_reply_to_addresses] if in_reply_to_addresses.is_a?(String) - in_reply_to_addresses.each do |in_reply_to| - match_result = in_reply_to.match(::ApplicationMailbox::CONVERSATION_MESSAGE_ID_PATTERN) - break if match_result - end - find_by_in_reply_to_addresses(match_result, in_reply_to_addresses) - end - - def find_by_in_reply_to_addresses(match_result, in_reply_to_addresses) - find_conversation_by_uuid(match_result) if match_result - find_conversation_by_message_id(in_reply_to_addresses) if @conversation.blank? - end - - # Find conversation from References header as fallback - # Extract conversation UUID from any reference that matches our patterns - def find_conversation_with_references - references_addresses = mail.references - references_addresses = [references_addresses] if references_addresses.is_a?(String) - - references_addresses.each do |reference| - conversation = find_conversation_from_reference(reference) - next unless conversation && conversation_belongs_to_channel?(conversation) - - @conversation = conversation - @conversation_uuid = conversation.uuid - break - end - end - - def find_conversation_from_reference(reference) - # Try message-specific pattern: conversation/{uuid}/messages/{id}@domain - message_match = reference.match(::ApplicationMailbox::CONVERSATION_MESSAGE_ID_PATTERN) - if message_match - uuid = message_match.captures[0] - conversation = Conversation.find_by(uuid: uuid) - return conversation if conversation.present? - end - - # Try conversation fallback pattern: account/{id}/conversation/{uuid}@domain - fallback_match = reference.match(::ApplicationMailbox::CONVERSATION_FALLBACK_ID_PATTERN) - if fallback_match - uuid = fallback_match.captures[1] - conversation = Conversation.find_by(uuid: uuid) - return conversation if conversation.present? - end - - # Try finding by message source_id - message = Message.find_by(source_id: reference) - message&.conversation - end - - def conversation_belongs_to_channel?(conversation) - return true unless conversation - - # Get the channel from the email's To/CC addresses - channel = EmailChannelFinder.new(mail).perform - return false unless channel - - # Check if the conversation's inbox matches the channel - conversation.inbox.channel_id == channel.id - end - - def validate_resource(resource) - Rails.logger.error "[App::Mailboxes::ReplyMailbox] Email conversation with uuid: #{conversation_uuid} not found" if resource.nil? - - resource + Rails.logger.error "[ReplyMailbox] No conversation found for email #{mail.message_id}" end def decorate_mail diff --git a/app/services/mailbox/conversation_finder_chain.rb b/app/services/mailbox/conversation_finder_chain.rb new file mode 100644 index 000000000..552734b17 --- /dev/null +++ b/app/services/mailbox/conversation_finder_chain.rb @@ -0,0 +1,28 @@ +class Mailbox::ConversationFinderChain + DEFAULT_STRATEGIES = [ + Mailbox::ConversationFinderStrategies::ReceiverUuidStrategy, + Mailbox::ConversationFinderStrategies::InReplyToStrategy, + Mailbox::ConversationFinderStrategies::ReferencesStrategy + ].freeze + + def initialize(mail, strategies: DEFAULT_STRATEGIES) + @mail = mail + @strategies = strategies + end + + def find + @strategies.each do |strategy_class| + conversation = strategy_class.new(@mail).find + + next unless conversation + + strategy_name = strategy_class.name.demodulize.underscore + Rails.logger.info "Conversation found via #{strategy_name} strategy" + return conversation + end + + # No strategy matched + Rails.logger.info 'No conversation found via any strategy' + nil + end +end diff --git a/app/services/mailbox/conversation_finder_strategies/base_strategy.rb b/app/services/mailbox/conversation_finder_strategies/base_strategy.rb new file mode 100644 index 000000000..c1e738805 --- /dev/null +++ b/app/services/mailbox/conversation_finder_strategies/base_strategy.rb @@ -0,0 +1,13 @@ +class Mailbox::ConversationFinderStrategies::BaseStrategy + attr_reader :mail + + def initialize(mail) + @mail = mail + end + + # Returns Conversation or nil + # Subclasses must implement this method + def find + raise NotImplementedError, "#{self.class} must implement #find" + end +end diff --git a/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb b/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb new file mode 100644 index 000000000..6bfe5f7eb --- /dev/null +++ b/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb @@ -0,0 +1,40 @@ +class Mailbox::ConversationFinderStrategies::InReplyToStrategy < Mailbox::ConversationFinderStrategies::BaseStrategy + # Patterns from ApplicationMailbox + MESSAGE_PATTERN = %r{conversation/([a-zA-Z0-9-]+)/messages/(\d+)@} + FALLBACK_PATTERN = %r{account/(\d+)/conversation/([a-zA-Z0-9-]+)@} + + def find + return nil if mail.in_reply_to.blank? + + in_reply_to_addresses = Array.wrap(mail.in_reply_to) + + in_reply_to_addresses.each do |in_reply_to| + # Try extracting UUID from patterns + uuid = extract_uuid_from_patterns(in_reply_to) + if uuid + conversation = Conversation.find_by(uuid: uuid) + return conversation if conversation + end + + # Try finding by message source_id + message = Message.find_by(source_id: in_reply_to) + return message.conversation if message&.conversation + end + + nil + end + + private + + def extract_uuid_from_patterns(message_id) + # Try message-specific pattern first + match = MESSAGE_PATTERN.match(message_id) + return match[1] if match + + # Try conversation fallback pattern + match = FALLBACK_PATTERN.match(message_id) + return match[2] if match + + nil + end +end diff --git a/app/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy.rb b/app/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy.rb new file mode 100644 index 000000000..39b155cf8 --- /dev/null +++ b/app/services/mailbox/conversation_finder_strategies/receiver_uuid_strategy.rb @@ -0,0 +1,26 @@ +class Mailbox::ConversationFinderStrategies::ReceiverUuidStrategy < Mailbox::ConversationFinderStrategies::BaseStrategy + # Pattern from ApplicationMailbox::REPLY_EMAIL_UUID_PATTERN + UUID_PATTERN = /^reply\+([0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12})$/i + + def find + uuid = extract_uuid_from_receivers + return nil unless uuid + + Conversation.find_by(uuid: uuid) + end + + private + + def extract_uuid_from_receivers + mail_presenter = MailPresenter.new(mail) + return nil if mail_presenter.mail_receiver.blank? + + mail_presenter.mail_receiver.each do |email| + username = email.split('@').first + match = username.match(UUID_PATTERN) + return match[1] if match + end + + nil + end +end diff --git a/app/services/mailbox/conversation_finder_strategies/references_strategy.rb b/app/services/mailbox/conversation_finder_strategies/references_strategy.rb new file mode 100644 index 000000000..d9fdad485 --- /dev/null +++ b/app/services/mailbox/conversation_finder_strategies/references_strategy.rb @@ -0,0 +1,56 @@ +class Mailbox::ConversationFinderStrategies::ReferencesStrategy < Mailbox::ConversationFinderStrategies::BaseStrategy + # Patterns from ApplicationMailbox + MESSAGE_PATTERN = %r{conversation/([a-zA-Z0-9-]+)/messages/(\d+)@} + FALLBACK_PATTERN = %r{account/(\d+)/conversation/([a-zA-Z0-9-]+)@} + + def find + return nil if mail.references.blank? + + references = Array.wrap(mail.references) + + references.each do |reference| + conversation = find_conversation_from_reference(reference) + next unless conversation && conversation_belongs_to_channel?(conversation) + + return conversation + end + + nil + end + + private + + def find_conversation_from_reference(reference) + # Try extracting UUID from patterns + uuid = extract_uuid_from_patterns(reference) + if uuid + conversation = Conversation.find_by(uuid: uuid) + return conversation if conversation + end + + # Try finding by message source_id + message = Message.find_by(source_id: reference) + message&.conversation + end + + def extract_uuid_from_patterns(message_id) + # Try message-specific pattern first + match = MESSAGE_PATTERN.match(message_id) + return match[1] if match + + # Try conversation fallback pattern + match = FALLBACK_PATTERN.match(message_id) + return match[2] if match + + nil + end + + def conversation_belongs_to_channel?(conversation) + # Get the channel from the email's To/CC addresses + channel = EmailChannelFinder.new(mail).perform + return false unless channel + + # Check if the conversation's inbox matches the channel + conversation.inbox.channel_id == channel.id + end +end