refactor: use a strategy based approach to find conversation

2025-10-30 15:31:15 +05:30
parent 21457368d7
commit 841d9f756b
6 changed files with 169 additions and 121 deletions
@@ -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+<conversation-uuid>@<mailer-domain.com>
-  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
-  # <conversation/#{@conversation.uuid}/messages/#{@messages&.last&.id}@#{@account.inbound_email_domain}>
-  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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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