From 3dc110a1fb7607df2704d99e0227e9270d42e9d3 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 15 Dec 2023 09:39:27 +0000 Subject: [PATCH 1/2] WIP Integrate ActionMailbox Integrate ActionMailbox into `RequestMailer.receive` for inbound email handling. This maintains existing functionality but lays the groundwork for future enhancements. Added an `origin` column to the ActionMailbox::InboundEmail table. This change was necessary to preserve the functionality of checking the mail `source`, avoiding a naming clash with the `source` method. Benefits of using ActionMailbox include: 1. Enhanced email routing capabilities, enabling specialized processing like the Excel hidden data spreadsheet analyzer. 2. Refactoring opportunities for `RequestMailer#receive`, particularly for spam detection, duplicate email handling, and initial request assessment. 3. Clear separation of concerns between Mailers (for sending) and Mailboxes (for receiving). 4. Improved email processing efficiency through ActionMailbox, facilitating background job handling and potential simplification of mail ingress. 5. Provides a solution for re-users to receive emails without needing their own mail server setup. WIP --- app/mailboxes/application_mailbox.rb | 3 +++ app/mailboxes/request_mailbox.rb | 11 +++++++++++ app/mailers/request_mailer.rb | 6 ++++-- config/application.rb | 4 +++- config/sidekiq.yml-example | 2 ++ config/storage.yml-example | 19 +++++++++++++++++++ ...te_action_mailbox_tables.action_mailbox.rb | 14 ++++++++++++++ ...origin_to_action_mailbox_inbound_emails.rb | 5 +++++ doc/CHANGES.md | 9 +++++++++ spec/mailboxes/request_mailbox_spec.rb | 5 +++++ 10 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 app/mailboxes/application_mailbox.rb create mode 100644 app/mailboxes/request_mailbox.rb create mode 100644 db/migrate/20231214125336_create_action_mailbox_tables.action_mailbox.rb create mode 100644 db/migrate/20231214135151_add_origin_to_action_mailbox_inbound_emails.rb create mode 100644 spec/mailboxes/request_mailbox_spec.rb diff --git a/app/mailboxes/application_mailbox.rb b/app/mailboxes/application_mailbox.rb new file mode 100644 index 0000000000..0cb0768fe4 --- /dev/null +++ b/app/mailboxes/application_mailbox.rb @@ -0,0 +1,3 @@ +class ApplicationMailbox < ActionMailbox::Base + routing all: :request +end diff --git a/app/mailboxes/request_mailbox.rb b/app/mailboxes/request_mailbox.rb new file mode 100644 index 0000000000..30fd8de86c --- /dev/null +++ b/app/mailboxes/request_mailbox.rb @@ -0,0 +1,11 @@ +class RequestMailbox < ApplicationMailbox + def process + mail = MailHandler.mail_from_raw_email(inbound_email.source) + + RequestMailer.new.receive( + mail, + inbound_email.source, + inbound_email.origin&.to_sym + ) + end +end diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index 712bb4362d..9f27e88121 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -210,8 +210,10 @@ def self.receive(raw_email, source = :mailin) unless logger.nil? logger.debug "Received mail from #{source}:\n #{raw_email}" end - mail = MailHandler.mail_from_raw_email(raw_email) - new.receive(mail, raw_email, source) + + ActionMailbox::InboundEmail.create_and_extract_message_id!( + raw_email, origin: source + ) end # Find which info requests the email is for diff --git a/config/application.rb b/config/application.rb index b1dee98822..7779a515d0 100644 --- a/config/application.rb +++ b/config/application.rb @@ -8,7 +8,7 @@ require "active_storage/engine" require "action_controller/railtie" require "action_mailer/railtie" -# require "action_mailbox/engine" +require "action_mailbox/engine" require "action_text/engine" require "action_view/railtie" # require "action_cable/engine" @@ -101,5 +101,7 @@ class Application < Rails::Application # Allow the generation of full URLs in emails config.action_mailer.default_url_options = { host: AlaveteliConfiguration.domain } + + config.action_mailbox.storage_service = :inbound_emails end end diff --git a/config/sidekiq.yml-example b/config/sidekiq.yml-example index 10d5d514ae..7af4156d4b 100644 --- a/config/sidekiq.yml-example +++ b/config/sidekiq.yml-example @@ -8,6 +8,8 @@ production: - default - xapian - low + - action_mailbox_routing + - action_mailbox_incineration :limits: xapian: 1 diff --git a/config/storage.yml-example b/config/storage.yml-example index fee9833c2f..c8831a1334 100644 --- a/config/storage.yml-example +++ b/config/storage.yml-example @@ -20,6 +20,25 @@ # project: '' # bucket: '' +## Inbound Emails ## + +inbound_emails_disk: &inbound_emails_disk + service: Disk + root: <%= Rails.root.join('storage/inbound_emails') %> + +inbound_emails_production: &inbound_emails_production + <<: *inbound_emails_disk + +inbound_emails_development: &inbound_emails_development + <<: *inbound_emails_disk + +inbound_emails_test: &inbound_emails_test + service: Disk + root: <%= Rails.root.join('tmp/storage/inbound_emails') %> + +inbound_emails: + <<: *inbound_emails_<%= Rails.env %> + ## Raw Emails ## raw_emails_disk: &raw_emails_disk diff --git a/db/migrate/20231214125336_create_action_mailbox_tables.action_mailbox.rb b/db/migrate/20231214125336_create_action_mailbox_tables.action_mailbox.rb new file mode 100644 index 0000000000..2dd4279d9b --- /dev/null +++ b/db/migrate/20231214125336_create_action_mailbox_tables.action_mailbox.rb @@ -0,0 +1,14 @@ +# This migration comes from action_mailbox (originally 20180917164000) +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, null: false + t.string :message_checksum, null: false + + t.timestamps + + t.index [ :message_id, :message_checksum ], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true + end + end +end diff --git a/db/migrate/20231214135151_add_origin_to_action_mailbox_inbound_emails.rb b/db/migrate/20231214135151_add_origin_to_action_mailbox_inbound_emails.rb new file mode 100644 index 0000000000..068f237c5b --- /dev/null +++ b/db/migrate/20231214135151_add_origin_to_action_mailbox_inbound_emails.rb @@ -0,0 +1,5 @@ +class AddOriginToActionMailboxInboundEmails < ActiveRecord::Migration[7.0] + def change + add_column :action_mailbox_inbound_emails, :origin, :string + end +end diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 015684d991..07db344ad2 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -2,6 +2,7 @@ ## Highlighted Features +* Integrate ActionMailbox for better inbound email processing (Graeme Porteous) * Add basic Citation searching in admin UI (Gareth Rees) * Improve citations admin to allow title and description updates (Graeme Porteous) @@ -109,6 +110,14 @@ DAEMON=puma.service`. For detailed instructions, refer to [the documentation](https://alaveteli.org/docs/installing/cron_and_daemons/#puma). +* _Required:_ Please update your `config/storage.yml` file to include a + production configuration for `inbound_emails`. See + `config/storage.yml-example` as an example. + +* _Required:_ Please update your `config/sidekiq.yml` file to include the + `action_mailbox_routing` and `action_mailbox_incineration` queues. See + `config/sidekiq.yml-example` as an example. + * _Optional:_ Bodies with not many requests will automatically get tagged `not_many_requests` as they are updated. If you want to automatically tag them all in one go, run the following from the app root directory: diff --git a/spec/mailboxes/request_mailbox_spec.rb b/spec/mailboxes/request_mailbox_spec.rb new file mode 100644 index 0000000000..a780a0f05a --- /dev/null +++ b/spec/mailboxes/request_mailbox_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +RSpec.describe RequestMailbox, type: :mailbox do + pending "add some examples to (or delete) #{__FILE__}" +end From a1a8efc1ab2cf3294813390c12d45466a79baef5 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 23 Oct 2024 21:08:26 +0100 Subject: [PATCH 2/2] fixup! WIP Integrate ActionMailbox --- spec/script/mailin_spec.rb | 7 ------- spec/support/email_helpers.rb | 5 ++++- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/spec/script/mailin_spec.rb b/spec/script/mailin_spec.rb index c76a11d973..be4309766d 100644 --- a/spec/script/mailin_spec.rb +++ b/spec/script/mailin_spec.rb @@ -24,11 +24,4 @@ def mailin_test(email_filename) expect(r.status).to eq(0) expect(r.out).to eq("") end - - # Destroy the incoming message so that it doesn't affect other tests - after do - ir = info_requests(:other_request) - incoming_message = ir.incoming_messages[0] - incoming_message.destroy - end end diff --git a/spec/support/email_helpers.rb b/spec/support/email_helpers.rb index 9f7eab508c..92440736b2 100644 --- a/spec/support/email_helpers.rb +++ b/spec/support/email_helpers.rb @@ -11,7 +11,10 @@ def receive_incoming_mail(email_name_or_string, **kargs) content = load_file_fixture(email_name_or_string) || email_name_or_string content = gsub_addresses(content.dup, **kargs) content = ::Mail::Utilities.binary_unsafe_to_crlf(content) - RequestMailer.receive(content) + + ActionMailbox::InboundEmail.create_and_extract_message_id!( + content, origin: :mailin, status: :processing + )&.route end def get_fixture_mail(filename, email_to = nil, email_from = nil)