mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Ensure Action Mailbox processes an email only once when received multiple times
This also adds a new column, message_checksum, to the action_mailbox_inbound_emails table for storing SHA1 digest of the email source. Additionally, it makes generating the missing message id deterministic and adds a unique index on message_checksum and message_id to detect duplicate emails.
This commit is contained in:
parent
2dee59fed1
commit
5cd733a334
6 changed files with 44 additions and 15 deletions
|
@ -9,30 +9,30 @@
|
||||||
module ActionMailbox::InboundEmail::MessageId
|
module ActionMailbox::InboundEmail::MessageId
|
||||||
extend ActiveSupport::Concern
|
extend ActiveSupport::Concern
|
||||||
|
|
||||||
included do
|
|
||||||
before_save :generate_missing_message_id
|
|
||||||
end
|
|
||||||
|
|
||||||
class_methods do
|
class_methods do
|
||||||
# Create a new +InboundEmail+ from the raw +source+ of the email, which be uploaded as a Active Storage
|
# Create a new +InboundEmail+ from the raw +source+ of the email, which be uploaded as a Active Storage
|
||||||
# attachment called +raw_email+. Before the upload, extract the Message-ID from the +source+ and set
|
# attachment called +raw_email+. Before the upload, extract the Message-ID from the +source+ and set
|
||||||
# it as an attribute on the new +InboundEmail+.
|
# it as an attribute on the new +InboundEmail+.
|
||||||
def create_and_extract_message_id!(source, **options)
|
def create_and_extract_message_id!(source, **options)
|
||||||
create! options.merge(message_id: extract_message_id(source)) do |inbound_email|
|
message_checksum = Digest::SHA1.hexdigest(source)
|
||||||
|
message_id = extract_message_id(source) || generate_missing_message_id(message_checksum)
|
||||||
|
|
||||||
|
create! options.merge(message_id: message_id, message_checksum: message_checksum) do |inbound_email|
|
||||||
inbound_email.raw_email.attach io: StringIO.new(source), filename: "message.eml", content_type: "message/rfc822"
|
inbound_email.raw_email.attach io: StringIO.new(source), filename: "message.eml", content_type: "message/rfc822"
|
||||||
end
|
end
|
||||||
|
rescue ActiveRecord::RecordNotUnique
|
||||||
|
nil
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
def extract_message_id(source)
|
def extract_message_id(source)
|
||||||
Mail.from_source(source).message_id rescue nil
|
Mail.from_source(source).message_id rescue nil
|
||||||
end
|
end
|
||||||
end
|
|
||||||
|
|
||||||
private
|
def generate_missing_message_id(message_checksum)
|
||||||
def generate_missing_message_id
|
Mail::MessageIdField.new("<#{message_checksum}@#{::Socket.gethostname}.mail>").message_id.tap do |message_id|
|
||||||
self.message_id ||= Mail::MessageIdField.new.message_id.tap do |message_id|
|
logger.warn "Message-ID couldn't be parsed or is missing. Generated a new Message-ID: #{message_id}"
|
||||||
logger.warn "Message-ID couldn't be parsed or is missing. Generated a new Message-ID: #{message_id}"
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -2,10 +2,13 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0]
|
||||||
def change
|
def change
|
||||||
create_table :action_mailbox_inbound_emails do |t|
|
create_table :action_mailbox_inbound_emails do |t|
|
||||||
t.integer :status, default: 0, null: false
|
t.integer :status, default: 0, null: false
|
||||||
t.string :message_id
|
t.string :message_id, null: false
|
||||||
|
t.string :message_checksum, null: false
|
||||||
|
|
||||||
t.datetime :created_at, precision: 6, null: false
|
t.datetime :created_at, precision: 6, null: false
|
||||||
t.datetime :updated_at, precision: 6, null: false
|
t.datetime :updated_at, precision: 6, null: false
|
||||||
|
|
||||||
|
t.index [ :message_id, :message_checksum ], unique: true
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -2,10 +2,13 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0]
|
||||||
def change
|
def change
|
||||||
create_table :action_mailbox_inbound_emails do |t|
|
create_table :action_mailbox_inbound_emails do |t|
|
||||||
t.integer :status, default: 0, null: false
|
t.integer :status, default: 0, null: false
|
||||||
t.string :message_id
|
t.string :message_id, null: false
|
||||||
|
t.string :message_checksum, null: false
|
||||||
|
|
||||||
t.datetime :created_at, precision: 6
|
t.datetime :created_at, precision: 6
|
||||||
t.datetime :updated_at, precision: 6
|
t.datetime :updated_at, precision: 6
|
||||||
|
|
||||||
|
t.index [ :message_id, :message_checksum ], unique: true
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -15,8 +15,10 @@ ActiveRecord::Schema.define(version: 2018_02_12_164506) do
|
||||||
create_table "action_mailbox_inbound_emails", force: :cascade do |t|
|
create_table "action_mailbox_inbound_emails", force: :cascade do |t|
|
||||||
t.integer "status", default: 0, null: false
|
t.integer "status", default: 0, null: false
|
||||||
t.string "message_id"
|
t.string "message_id"
|
||||||
|
t.string "message_checksum"
|
||||||
t.datetime "created_at", precision: 6
|
t.datetime "created_at", precision: 6
|
||||||
t.datetime "updated_at", precision: 6
|
t.datetime "updated_at", precision: 6
|
||||||
|
t.index ["message_id", "message_checksum"], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true
|
||||||
end
|
end
|
||||||
|
|
||||||
create_table "active_storage_attachments", force: :cascade do |t|
|
create_table "active_storage_attachments", force: :cascade do |t|
|
||||||
|
|
|
@ -11,5 +11,27 @@ module ActionMailbox
|
||||||
test "source returns the contents of the raw email" do
|
test "source returns the contents of the raw email" do
|
||||||
assert_equal file_fixture("welcome.eml").read, create_inbound_email_from_fixture("welcome.eml").source
|
assert_equal file_fixture("welcome.eml").read, create_inbound_email_from_fixture("welcome.eml").source
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "email with message id is processed only once when received multiple times" do
|
||||||
|
mail = Mail.from_source(file_fixture("welcome.eml").read)
|
||||||
|
assert mail.message_id
|
||||||
|
|
||||||
|
inbound_email_1 = create_inbound_email_from_source(mail.to_s)
|
||||||
|
assert inbound_email_1
|
||||||
|
|
||||||
|
inbound_email_2 = create_inbound_email_from_source(mail.to_s)
|
||||||
|
assert_nil inbound_email_2
|
||||||
|
end
|
||||||
|
|
||||||
|
test "email with missing message id is processed only once when received multiple times" do
|
||||||
|
mail = Mail.from_source("Date: Fri, 28 Sep 2018 11:08:55 -0700\r\nTo: a@example.com\r\nMime-Version: 1.0\r\nContent-Type: text/plain\r\nContent-Transfer-Encoding: 7bit\r\n\r\nHello!")
|
||||||
|
assert_nil mail.message_id
|
||||||
|
|
||||||
|
inbound_email_1 = create_inbound_email_from_source(mail.to_s)
|
||||||
|
assert inbound_email_1
|
||||||
|
|
||||||
|
inbound_email_2 = create_inbound_email_from_source(mail.to_s)
|
||||||
|
assert_nil inbound_email_2
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -15,11 +15,10 @@ end
|
||||||
class ActionMailbox::Base::RoutingTest < ActiveSupport::TestCase
|
class ActionMailbox::Base::RoutingTest < ActiveSupport::TestCase
|
||||||
setup do
|
setup do
|
||||||
$processed = false
|
$processed = false
|
||||||
@inbound_email = create_inbound_email_from_fixture("welcome.eml")
|
|
||||||
end
|
end
|
||||||
|
|
||||||
test "string routing" do
|
test "string routing" do
|
||||||
ApplicationMailbox.route @inbound_email
|
ApplicationMailbox.route create_inbound_email_from_fixture("welcome.eml")
|
||||||
assert_equal "Discussion: Let's debate these attachments", $processed
|
assert_equal "Discussion: Let's debate these attachments", $processed
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue