From 5cd733a334846669d6435517f7d9913c4a9b1eb1 Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Thu, 17 Jan 2019 11:13:40 -0600
Subject: 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.
---
 ...20180208205311_create_action_mailroom_tables.rb |  5 ++++-
 actionmailbox/test/dummy/db/schema.rb              |  2 ++
 actionmailbox/test/unit/inbound_email_test.rb      | 22 ++++++++++++++++++++++
 actionmailbox/test/unit/mailbox/routing_test.rb    |  3 +--
 4 files changed, 29 insertions(+), 3 deletions(-)

(limited to 'actionmailbox/test')

diff --git a/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailroom_tables.rb b/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailroom_tables.rb
index 0124b4b98f..f587f37095 100644
--- a/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailroom_tables.rb
+++ b/actionmailbox/test/dummy/db/migrate/20180208205311_create_action_mailroom_tables.rb
@@ -2,10 +2,13 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0]
   def change
     create_table :action_mailbox_inbound_emails do |t|
       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 :updated_at, precision: 6
+
+      t.index [ :message_id, :message_checksum ], unique: true
     end
   end
 end
diff --git a/actionmailbox/test/dummy/db/schema.rb b/actionmailbox/test/dummy/db/schema.rb
index 6cfe7de765..4aba92bfc6 100644
--- a/actionmailbox/test/dummy/db/schema.rb
+++ b/actionmailbox/test/dummy/db/schema.rb
@@ -15,8 +15,10 @@ ActiveRecord::Schema.define(version: 2018_02_12_164506) do
   create_table "action_mailbox_inbound_emails", force: :cascade do |t|
     t.integer "status", default: 0, null: false
     t.string "message_id"
+    t.string "message_checksum"
     t.datetime "created_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
 
   create_table "active_storage_attachments", force: :cascade do |t|
diff --git a/actionmailbox/test/unit/inbound_email_test.rb b/actionmailbox/test/unit/inbound_email_test.rb
index 993423406f..76f047eb61 100644
--- a/actionmailbox/test/unit/inbound_email_test.rb
+++ b/actionmailbox/test/unit/inbound_email_test.rb
@@ -11,5 +11,27 @@ module ActionMailbox
     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
     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
diff --git a/actionmailbox/test/unit/mailbox/routing_test.rb b/actionmailbox/test/unit/mailbox/routing_test.rb
index d4dad7eafb..d4ba702ac5 100644
--- a/actionmailbox/test/unit/mailbox/routing_test.rb
+++ b/actionmailbox/test/unit/mailbox/routing_test.rb
@@ -15,11 +15,10 @@ end
 class ActionMailbox::Base::RoutingTest < ActiveSupport::TestCase
   setup do
     $processed = false
-    @inbound_email = create_inbound_email_from_fixture("welcome.eml")
   end
 
   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
   end
 
-- 
cgit v1.2.3