diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 2856c7817..2e53fa7c9 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -140,7 +140,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def destroy authorize @conversation, :destroy? - ::DeleteObjectJob.perform_later(@conversation, Current.user, request.ip) + ::Conversations::DeleteService.new(conversation: @conversation, user: Current.user, ip: request.ip).perform head :ok end diff --git a/app/services/conversations/delete_service.rb b/app/services/conversations/delete_service.rb new file mode 100644 index 000000000..6608cfdcd --- /dev/null +++ b/app/services/conversations/delete_service.rb @@ -0,0 +1,16 @@ +class Conversations::DeleteService + pattr_initialize [:conversation!, :user, :ip] + + def perform + track_deleted_email_messages + ::DeleteObjectJob.perform_later(conversation, user, ip) + end + + private + + def track_deleted_email_messages + return unless conversation.inbox.email? + + Imap::DeletedMessageTracker.new(inbox: conversation.inbox).record(conversation.messages.incoming.pluck(:source_id)) + end +end diff --git a/app/services/imap/base_fetch_email_service.rb b/app/services/imap/base_fetch_email_service.rb index 24dd22589..e55355f3a 100644 --- a/app/services/imap/base_fetch_email_service.rb +++ b/app/services/imap/base_fetch_email_service.rb @@ -38,7 +38,11 @@ class Imap::BaseFetchEmailService end def email_already_present?(channel, message_id) - channel.inbox.messages.find_by(source_id: message_id).present? + channel.inbox.messages.find_by(source_id: message_id).present? || deleted_message_tracker.deleted?(message_id) + end + + def deleted_message_tracker + @deleted_message_tracker ||= Imap::DeletedMessageTracker.new(inbox: channel.inbox) end def fetch_mail_for_channel diff --git a/app/services/imap/deleted_message_tracker.rb b/app/services/imap/deleted_message_tracker.rb new file mode 100644 index 000000000..7892f258b --- /dev/null +++ b/app/services/imap/deleted_message_tracker.rb @@ -0,0 +1,28 @@ +require 'digest' + +class Imap::DeletedMessageTracker + TTL = 2.days.to_i + + pattr_initialize [:inbox!] + + def record(source_ids) + return unless inbox.email? + + keys = source_ids.compact_blank.map { |source_id| redis_key(source_id) } + return if keys.blank? + + Redis::Alfred.pipelined do |pipeline| + keys.each { |key| pipeline.set(key, true, ex: TTL) } + end + end + + def deleted?(source_id) + Redis::Alfred.exists?(redis_key(source_id)) + end + + private + + def redis_key(source_id) + format(Redis::RedisKeys::IMAP_DELETED_MESSAGE, inbox_id: inbox.id, message_id_digest: Digest::SHA256.hexdigest(source_id)) + end +end diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index fff60c342..bc7fb551e 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -2,6 +2,8 @@ module Redis::RedisKeys ## Inbox Keys # Array storing the ordered ids for agent round robin assignment ROUND_ROBIN_AGENTS = 'ROUND_ROBIN_AGENTS:%d'.freeze + # Track recently deleted IMAP messages to prevent them from being synced again + IMAP_DELETED_MESSAGE = 'IMAP_DELETED_MESSAGE::%d::%s'.freeze ## Conversation keys # Detect whether to send an email reply to the conversation diff --git a/spec/services/conversations/delete_service_spec.rb b/spec/services/conversations/delete_service_spec.rb new file mode 100644 index 000000000..2ff14db49 --- /dev/null +++ b/spec/services/conversations/delete_service_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +RSpec.describe Conversations::DeleteService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:ip) { '127.0.0.1' } + let(:service) { described_class.new(conversation: conversation, user: user, ip: ip) } + + context 'when deleting an email conversation' do + let(:inbox) { create(:channel_email, :imap_email, account: account).inbox } + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + let!(:incoming_message) { create(:message, account: account, inbox: inbox, conversation: conversation, source_id: 'incoming@example.com') } + let(:deleted_message_tracker) { instance_double(Imap::DeletedMessageTracker, record: true) } + + before do + allow(Imap::DeletedMessageTracker).to receive(:new).with(inbox: inbox).and_return(deleted_message_tracker) + end + + it 'records incoming message source ids and enqueues the deletion job' do + expect { service.perform }.to have_enqueued_job(DeleteObjectJob).with(conversation, user, ip) + + expect(deleted_message_tracker).to have_received(:record).with([incoming_message.source_id]) + end + end + + context 'when deleting a non-email conversation' do + let(:conversation) { create(:conversation, account: account) } + + it 'enqueues the deletion job without recording message source ids' do + expect(Imap::DeletedMessageTracker).not_to receive(:new) + + expect { service.perform }.to have_enqueued_job(DeleteObjectJob).with(conversation, user, ip) + end + end +end diff --git a/spec/services/imap/fetch_email_service_spec.rb b/spec/services/imap/fetch_email_service_spec.rb index 1f74cf0b7..b88e548d9 100644 --- a/spec/services/imap/fetch_email_service_spec.rb +++ b/spec/services/imap/fetch_email_service_spec.rb @@ -119,6 +119,28 @@ RSpec.describe Imap::FetchEmailService do end end + it 'does not return recently deleted emails' do + travel_to '26.10.2020 10:00'.to_datetime do + email_object = create_inbound_email_from_fixture('only_text.eml') + email_header = Net::IMAP::FetchData.new(1, 'BODY[HEADER]' => eml_content_with_message_id) + redis_key = format(Redis::RedisKeys::IMAP_DELETED_MESSAGE, + inbox_id: imap_email_channel.inbox.id, + message_id_digest: Digest::SHA256.hexdigest(email_object.message_id)) + + Imap::DeletedMessageTracker.new(inbox: imap_email_channel.inbox).record([email_object.message_id]) + allow(imap).to receive(:search).with(%w[SINCE 25-Oct-2020]).and_return([1]) + allow(imap).to receive(:fetch).with([1], 'BODY.PEEK[HEADER]').and_return([email_header]) + allow(imap).to receive(:logout) + + result = described_class.new(channel: imap_email_channel).perform + + expect(result).to be_empty + expect(imap).not_to have_received(:fetch).with(1, 'RFC822') + ensure + Redis::Alfred.delete(redis_key) if redis_key + end + end + it 'does not count emails without message ids toward the sync limit' do travel_to '26.10.2020 10:00'.to_datetime do email_object = create_inbound_email_from_fixture('only_text.eml')