From 04cfc8ed9d89c041aa457c3f5f2227ff32787671 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 18 Jan 2022 16:56:30 +0100 Subject: [PATCH 1/3] config: use alternate delivery methods to configure ActionMailer Previously `SENDINBLUE_BALANCING` was used only when `SENDINBLUE_ENABLED` was *disabled* (otherwise only SendInBlue was ever used). This commit: - Ensure that `SENDINBLUE_BALANCING` is used only when SendInBlue is *enabled* (which is more intuitive). - Make it easier to add other delivery methods. --- app/lib/mailtrap/smtp.rb | 4 +++ app/lib/sendinblue/smtp.rb | 4 +++ config/environments/production.rb | 33 +++++++++++-------- .../dynamic_smtp_settings_interceptor.rb | 26 ++++++++------- 4 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 app/lib/mailtrap/smtp.rb create mode 100644 app/lib/sendinblue/smtp.rb diff --git a/app/lib/mailtrap/smtp.rb b/app/lib/mailtrap/smtp.rb new file mode 100644 index 000000000..93b57f412 --- /dev/null +++ b/app/lib/mailtrap/smtp.rb @@ -0,0 +1,4 @@ +module Mailtrap + class Smtp < ::Mail::SMTP + end +end diff --git a/app/lib/sendinblue/smtp.rb b/app/lib/sendinblue/smtp.rb new file mode 100644 index 000000000..cccb4872e --- /dev/null +++ b/app/lib/sendinblue/smtp.rb @@ -0,0 +1,4 @@ +module Sendinblue + class Smtp < ::Mail::SMTP + end +end diff --git a/config/environments/production.rb b/config/environments/production.rb index cdfeaed9b..b006c9ec3 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -1,4 +1,6 @@ require "active_support/core_ext/integer/time" +require Rails.root.join("app/lib/mailtrap/smtp") +require Rails.root.join("app/lib/sendinblue/smtp") Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. @@ -76,8 +78,8 @@ Rails.application.configure do # config.action_mailer.raise_delivery_errors = false if ENV['MAILTRAP_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { + ActionMailer::Base.add_delivery_method :mailtrap, Mailtrap::Smtp + config.action_mailer.mailtrap_settings = { user_name: Rails.application.secrets.mailtrap[:username], password: Rails.application.secrets.mailtrap[:password], address: 'smtp.mailtrap.io', @@ -85,17 +87,22 @@ Rails.application.configure do port: '2525', authentication: :cram_md5 } - elsif ENV['SENDINBLUE_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - user_name: Rails.application.secrets.sendinblue[:username], - password: Rails.application.secrets.sendinblue[:smtp_key], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - else + config.action_mailer.delivery_method = :mailtrap + elsif + if ENV['SENDINBLUE_ENABLED'] == 'enabled' + ActionMailer::Base.add_delivery_method :sendinblue, Sendinblue::Smtp + config.action_mailer.sendinblue_settings = { + user_name: Rails.application.secrets.sendinblue[:username], + password: Rails.application.secrets.sendinblue[:smtp_key], + address: 'smtp-relay.sendinblue.com', + domain: 'smtp-relay.sendinblue.com', + port: '587', + authentication: :cram_md5 + } + end + + # Default delivery method + # (Actual delivery method will be selected at runtime by DynamicSmtpSettingsInterceptor) config.action_mailer.delivery_method = :mailjet end diff --git a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb b/lib/action_mailer/dynamic_smtp_settings_interceptor.rb index 91d6398f8..2aeef5c5e 100644 --- a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb +++ b/lib/action_mailer/dynamic_smtp_settings_interceptor.rb @@ -6,17 +6,21 @@ class DynamicSmtpSettingsInterceptor def self.delivering_email(message) - if ENV['SENDINBLUE_BALANCING'] == 'enabled' - if rand(0..99) < ENV['SENDINBLUE_BALANCING_VALUE'].to_i - message.delivery_method.settings = { - user_name: ENV['SENDINBLUE_USER_NAME'], - password: ENV['SENDINBLUE_SMTP_KEY'], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - end + if balance_to_sendinblue? + ApplicationMailer.wrap_delivery_behavior(message, :sendinblue) + end + # fallback to the default delivery method + end + + private + + def self.balance_to_sendinblue? + if ENV.fetch('SENDINBLUE_ENABLED') != 'enabled' + false + elsif ENV.fetch('SENDINBLUE_BALANCING') == 'enabled' + rand(0..99) < ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i + else + true end end end From 27b42fe8ae9424f1b1f567737960b284698efaf0 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 19 Jan 2022 20:30:01 +0000 Subject: [PATCH 2/3] config: better document `SENDINBLUE_BALANCING` --- config/env.example | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/config/env.example b/config/env.example index d2f9de017..8f3e332f8 100644 --- a/config/env.example +++ b/config/env.example @@ -70,28 +70,33 @@ MATOMO_ENABLED="disabled" MATOMO_ID="" MATOMO_HOST="matomo.organisme.fr" -# SMTP Provider: Send In Blue +# Default SMTP Provider: Mailjet +MAILJET_API_KEY="" +MAILJET_SECRET_KEY="" + +# Alternate SMTP Provider: SendInBlue SENDINBLUE_ENABLED="disabled" -SENDINBLUE_BALANCING="" -SENDINBLUE_BALANCING_VALUE="" SENDINBLUE_CLIENT_KEY="" SENDINBLUE_SMTP_KEY="" SENDINBLUE_USER_NAME="" # SENDINBLUE_LOGIN_URL="https://app.sendinblue.com/account/saml/login/truc" -# SMTP Provider: Mailjet -MAILJET_API_KEY="" -MAILJET_SECRET_KEY="" +# Ratio of emails sent using SendInBlue +# When enabled, N % of emails will be sent using SendInBlue +# (and the others using the default SMTP provider) +SENDINBLUE_BALANCING="disabled" +SENDINBLUE_BALANCING_VALUE="50" + +# Alternate SMTP Provider: Mailtrap (mail catcher for staging environments) +# When enabled, all emails will be sent using this provided +MAILTRAP_ENABLED="disabled" +MAILTRAP_USERNAME="" +MAILTRAP_PASSWORD="" # External service: live chat for admins (specific to démarches-simplifiées.fr) CRISP_ENABLED="disabled" CRISP_CLIENT_KEY="" -# External service: mail catcher for staging environments (specific to démarches-simplifiées.fr) -MAILTRAP_ENABLED="disabled" -MAILTRAP_USERNAME="" -MAILTRAP_PASSWORD="" - # API Entreprise credentials # https://api.gouv.fr/api/api-entreprise.html API_ENTREPRISE_KEY="" From 847abca1226466aabc31eaf795ed92a61d3c7fae Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 26 Jan 2022 10:33:12 +0000 Subject: [PATCH 3/3] config: simplify mailer configuration again Move everything to initializers, and replace the email settings interceptor by a BalancerDeliveryMethod. It has the advantage that it can be configured entirely from the `config/environment.rb` file, without an extra file to look at. --- app/lib/balancer_delivery_method.rb | 38 ++++++++ app/lib/mailtrap/smtp.rb | 4 - app/lib/sendinblue/smtp.rb | 4 - config/environments/development.rb | 39 ++------ config/environments/production.rb | 38 +++----- .../dynamic_smtp_settings_interceptor.rb | 14 --- config/initializers/helo.rb | 17 ++++ config/initializers/mailtrap.rb | 17 ++++ config/initializers/sendinblue.rb | 24 ++++- .../dynamic_smtp_settings_interceptor.rb | 26 ------ spec/lib/balancer_delivery_method_spec.rb | 93 +++++++++++++++++++ 11 files changed, 205 insertions(+), 109 deletions(-) create mode 100644 app/lib/balancer_delivery_method.rb delete mode 100644 app/lib/mailtrap/smtp.rb delete mode 100644 app/lib/sendinblue/smtp.rb delete mode 100644 config/initializers/dynamic_smtp_settings_interceptor.rb create mode 100644 config/initializers/helo.rb create mode 100644 config/initializers/mailtrap.rb delete mode 100644 lib/action_mailer/dynamic_smtp_settings_interceptor.rb create mode 100644 spec/lib/balancer_delivery_method_spec.rb diff --git a/app/lib/balancer_delivery_method.rb b/app/lib/balancer_delivery_method.rb new file mode 100644 index 000000000..85f468ae3 --- /dev/null +++ b/app/lib/balancer_delivery_method.rb @@ -0,0 +1,38 @@ +# A Mail delivery method that randomly balances the actual delivery between different +# methods. +# +# Usage: +# +# ```ruby +# ActionMailer::Base.add_delivery_method :balancer, BalancerDeliveryMethod +# config.action_mailer.balancer_settings = { +# smtp: 25, +# sendmail: 75 +# } +# config.action_mailer.delivery_method = :balancer +# ``` +# +# Be sure to restart your server when you modify this file. +class BalancerDeliveryMethod + # Allows configuring the random number generator used for selecting a delivery method, + # mostly for testing purposes. + mattr_accessor :random, default: Random.new + + def initialize(settings) + @delivery_methods = settings + end + + def deliver!(mail) + balanced_delivery_method = delivery_method(mail) + ApplicationMailer.wrap_delivery_behavior(mail, balanced_delivery_method) + mail.deliver + end + + private + + def delivery_method(mail) + @delivery_methods + .flat_map { |delivery_method, weight| [delivery_method] * weight } + .sample(random: self.class.random) + end +end diff --git a/app/lib/mailtrap/smtp.rb b/app/lib/mailtrap/smtp.rb deleted file mode 100644 index 93b57f412..000000000 --- a/app/lib/mailtrap/smtp.rb +++ /dev/null @@ -1,4 +0,0 @@ -module Mailtrap - class Smtp < ::Mail::SMTP - end -end diff --git a/app/lib/sendinblue/smtp.rb b/app/lib/sendinblue/smtp.rb deleted file mode 100644 index cccb4872e..000000000 --- a/app/lib/sendinblue/smtp.rb +++ /dev/null @@ -1,4 +0,0 @@ -module Sendinblue - class Smtp < ::Mail::SMTP - end -end diff --git a/config/environments/development.rb b/config/environments/development.rb index 2b0f92cbc..e0be95cc4 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -75,40 +75,13 @@ Rails.application.configure do config.assets.raise_runtime_errors = true # Action Mailer settings + config.action_mailer.delivery_method = :letter_opener - if ENV['SENDINBLUE_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - user_name: Rails.application.secrets.sendinblue[:username], - password: Rails.application.secrets.sendinblue[:smtp_key], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - else - # https://usehelo.com - if ENV['HELO_ENABLED'] == 'enabled' - config.action_mailer.delivery_method = :smtp - config.action_mailer.smtp_settings = { - user_name: 'demarches-simplifiees', - password: '', - address: '127.0.0.1', - domain: '127.0.0.1', - port: ENV.fetch('HELO_PORT', '2525'), - authentication: :plain - } - else - config.action_mailer.delivery_method = :letter_opener_web - end - - config.action_mailer.default_url_options = { - host: 'localhost', - port: 3000 - } - - config.action_mailer.asset_host = "http://" + ENV['APP_HOST'] - end + config.action_mailer.default_url_options = { + host: 'localhost', + port: 3000 + } + config.action_mailer.asset_host = "http://" + ENV['APP_HOST'] Rails.application.routes.default_url_options = { host: 'localhost', diff --git a/config/environments/production.rb b/config/environments/production.rb index b006c9ec3..912426727 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -1,6 +1,5 @@ require "active_support/core_ext/integer/time" -require Rails.root.join("app/lib/mailtrap/smtp") -require Rails.root.join("app/lib/sendinblue/smtp") +require Rails.root.join("app/lib/balancer_delivery_method") Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. @@ -78,31 +77,20 @@ Rails.application.configure do # config.action_mailer.raise_delivery_errors = false if ENV['MAILTRAP_ENABLED'] == 'enabled' - ActionMailer::Base.add_delivery_method :mailtrap, Mailtrap::Smtp - config.action_mailer.mailtrap_settings = { - user_name: Rails.application.secrets.mailtrap[:username], - password: Rails.application.secrets.mailtrap[:password], - address: 'smtp.mailtrap.io', - domain: 'smtp.mailtrap.io', - port: '2525', - authentication: :cram_md5 - } config.action_mailer.delivery_method = :mailtrap - elsif - if ENV['SENDINBLUE_ENABLED'] == 'enabled' - ActionMailer::Base.add_delivery_method :sendinblue, Sendinblue::Smtp - config.action_mailer.sendinblue_settings = { - user_name: Rails.application.secrets.sendinblue[:username], - password: Rails.application.secrets.sendinblue[:smtp_key], - address: 'smtp-relay.sendinblue.com', - domain: 'smtp-relay.sendinblue.com', - port: '587', - authentication: :cram_md5 - } - end - # Default delivery method - # (Actual delivery method will be selected at runtime by DynamicSmtpSettingsInterceptor) + elsif ENV['SENDINBLUE_ENABLED'] == 'enabled' && ENV['SENDINBLUE_BALANCING'] == 'enabled' + ActionMailer::Base.add_delivery_method :balancer, BalancerDeliveryMethod + config.action_mailer.balancer_settings = { + sendinblue: ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i, + mailjet: 100 - ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i + } + config.action_mailer.delivery_method = :balancer + + elsif ENV['SENDINBLUE_ENABLED'] == 'enabled' + config.action_mailer.delivery_method = :sendinblue + + else config.action_mailer.delivery_method = :mailjet end diff --git a/config/initializers/dynamic_smtp_settings_interceptor.rb b/config/initializers/dynamic_smtp_settings_interceptor.rb deleted file mode 100644 index b91ee7e19..000000000 --- a/config/initializers/dynamic_smtp_settings_interceptor.rb +++ /dev/null @@ -1,14 +0,0 @@ -# We want to register an interceptor, but we can't make the action idempotent -# (because there's no way to peek at the currently registered interceptors). -# -# To make zeitwerk happy, instead signal that we don't want the -# DynamicSmtpSettingsInterceptor constant to be auto-loaded, by: -# - adding it to a non-autoloaded-path (/lib), -# - requiring it explicitely. -# -# See https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoloading-when-the-application-boots -require 'action_mailer/dynamic_smtp_settings_interceptor' - -ActiveSupport.on_load(:action_mailer) do - ActionMailer::Base.register_interceptor DynamicSmtpSettingsInterceptor -end diff --git a/config/initializers/helo.rb b/config/initializers/helo.rb new file mode 100644 index 000000000..ed119ba81 --- /dev/null +++ b/config/initializers/helo.rb @@ -0,0 +1,17 @@ +if ENV['HELO_ENABLED'] == 'enabled' + ActiveSupport.on_load(:action_mailer) do + module Helo + class SMTP < ::Mail::SMTP; end + end + + ActionMailer::Base.add_delivery_method :helo, Helo::SMTP + ActionMailer::Base.helo_settings = { + user_name: 'demarches-simplifiees', + password: '', + address: '127.0.0.1', + domain: '127.0.0.1', + port: ENV.fetch('HELO_PORT', '2525'), + authentication: :plain + } + end +end diff --git a/config/initializers/mailtrap.rb b/config/initializers/mailtrap.rb new file mode 100644 index 000000000..6d1faa04b --- /dev/null +++ b/config/initializers/mailtrap.rb @@ -0,0 +1,17 @@ +if ENV.fetch('MAILTRAP_ENABLED') == 'enabled' + ActiveSupport.on_load(:action_mailer) do + module Mailtrap + class SMTP < ::Mail::SMTP; end + end + + ActionMailer::Base.add_delivery_method :mailtrap, Mailtrap::SMTP + ActionMailer::Base.mailtrap_settings = { + user_name: Rails.application.secrets.mailtrap[:username], + password: Rails.application.secrets.mailtrap[:password], + address: 'smtp.mailtrap.io', + domain: 'smtp.mailtrap.io', + port: '2525', + authentication: :cram_md5 + } + end +end diff --git a/config/initializers/sendinblue.rb b/config/initializers/sendinblue.rb index d523b2373..206b3905d 100644 --- a/config/initializers/sendinblue.rb +++ b/config/initializers/sendinblue.rb @@ -1,5 +1,23 @@ -require 'sib-api-v3-sdk' +if ENV.fetch('SENDINBLUE_ENABLED') == 'enabled' + require 'sib-api-v3-sdk' -SibApiV3Sdk.configure do |config| - config.api_key['api-key'] = ENV.fetch('SENDINBLUE_API_V3_KEY', '') + ActiveSupport.on_load(:action_mailer) do + module Sendinblue + class SMTP < ::Mail::SMTP; end + end + + ActionMailer::Base.add_delivery_method :sendinblue, Sendinblue::SMTP + ActionMailer::Base.sendinblue_settings = { + user_name: Rails.application.secrets.sendinblue[:username], + password: Rails.application.secrets.sendinblue[:smtp_key], + address: 'smtp-relay.sendinblue.com', + domain: 'smtp-relay.sendinblue.com', + port: '587', + authentication: :cram_md5 + } + end + + SibApiV3Sdk.configure do |config| + config.api_key['api-key'] = Rails.application.secrets.sendinblue[:api_v3_key] + end end diff --git a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb b/lib/action_mailer/dynamic_smtp_settings_interceptor.rb deleted file mode 100644 index 2aeef5c5e..000000000 --- a/lib/action_mailer/dynamic_smtp_settings_interceptor.rb +++ /dev/null @@ -1,26 +0,0 @@ -# Note: this class is instanciated when being added as an interceptor -# during the app initialization. -# -# If you edit this file in development env, you will need to restart -# the app to see the changes. - -class DynamicSmtpSettingsInterceptor - def self.delivering_email(message) - if balance_to_sendinblue? - ApplicationMailer.wrap_delivery_behavior(message, :sendinblue) - end - # fallback to the default delivery method - end - - private - - def self.balance_to_sendinblue? - if ENV.fetch('SENDINBLUE_ENABLED') != 'enabled' - false - elsif ENV.fetch('SENDINBLUE_BALANCING') == 'enabled' - rand(0..99) < ENV.fetch('SENDINBLUE_BALANCING_VALUE').to_i - else - true - end - end -end diff --git a/spec/lib/balancer_delivery_method_spec.rb b/spec/lib/balancer_delivery_method_spec.rb new file mode 100644 index 000000000..b44fbdad8 --- /dev/null +++ b/spec/lib/balancer_delivery_method_spec.rb @@ -0,0 +1,93 @@ +RSpec.describe BalancerDeliveryMethod do + class ExampleMailer < ApplicationMailer + def greet(name) + mail(to: "smtp_to", from: "smtp_from", body: "Hello #{name}") + end + end + + class TestMail + def self.deliveries + @deliveries ||= [] + end + + def self.deliveries=(val) + @deliveries = val + end + + attr_accessor :settings + + def initialize(values) + @settings = values.dup + end + + def deliver!(mail) + Mail::CheckDeliveryParams.check(mail) + self.class.deliveries << mail + end + end + + class MockSmtp < TestMail; end + + class MockSendmail < TestMail; end + + class FixedSequence + def initialize(sequence) + @enumerator = sequence.each + end + + def rand(_) + @enumerator.next + end + end + + before do + ActionMailer::Base.add_delivery_method :mock_smtp, MockSmtp + ActionMailer::Base.add_delivery_method :mock_sendmail, MockSendmail + ActionMailer::Base.add_delivery_method :balancer, BalancerDeliveryMethod + + ExampleMailer.delivery_method = :balancer + end + + context 'when a single delivery method is provided' do + before do + ActionMailer::Base.balancer_settings = { mock_smtp: 10 } + end + + it 'sends emails to the selected delivery method' do + mail = ExampleMailer.greet('Joshua').deliver_now + expect(mail).to have_been_delivered_using(MockSmtp) + end + end + + context 'when multiple delivery methods are provided' do + before do + ActionMailer::Base.balancer_settings = { mock_smtp: 10, mock_sendmail: 5 } + + rng_sequence = [3, 14, 1] + BalancerDeliveryMethod.random = FixedSequence.new(rng_sequence) + end + + after do + BalancerDeliveryMethod.random = Random.new + end + + it 'sends emails randomly, given the provided weights' do + mail1 = ExampleMailer.greet('Lucia').deliver_now + expect(mail1).to have_been_delivered_using(MockSmtp) + + mail2 = ExampleMailer.greet('Damian').deliver_now + expect(mail2).to have_been_delivered_using(MockSendmail) + + mail3 = ExampleMailer.greet('Rahwa').deliver_now + expect(mail3).to have_been_delivered_using(MockSmtp) + end + end + + # Helpers + + def have_been_delivered_using(delivery_class) + satisfy("have been delivered using #{delivery_class}") do |mail| + delivery_class.deliveries.include?(mail) + end + end +end