Refactor email notifier

lib/exception_notifier/email_notifier.rb

@@ -1,16 +1,27 @@
-require 'active_support/core_ext/hash/reverse_merge'
 require 'active_support/core_ext/time'
 require 'action_mailer'
 require 'action_dispatch'
 require 'pp'
 
 module ExceptionNotifier
   class EmailNotifier < BaseNotifier
-    attr_accessor(:sender_address, :exception_recipients,
-                  :pre_callback, :post_callback,
-                  :email_prefix, :email_format, :sections, :background_sections,
-                  :verbose_subject, :normalize_subject, :include_controller_and_action_names_in_subject,
-                  :delivery_method, :mailer_settings, :email_headers, :mailer_parent, :template_path, :deliver_with)
+    DEFAULT_OPTIONS = {
+      sender_address: %("Exception Notifier" <exception.notifier@example.com>),
+      exception_recipients: [],
+      email_prefix: '[ERROR] ',
+      email_format: :text,
+      sections: %w[request session environment backtrace],
+      background_sections: %w[backtrace data],
+      verbose_subject: true,
+      normalize_subject: false,
+      include_controller_and_action_names_in_subject: true,
+      delivery_method: nil,
+      mailer_settings: nil,
+      email_headers: {},
+      mailer_parent: 'ActionMailer::Base',
+      template_path: 'exception_notifier',
+      deliver_with: nil
+    }.freeze
 
     module Mailer
       class MissingController
@@ -29,7 +40,10 @@ def exception_notification(env, exception, options = {}, default_options = {})
 
             @env        = env
             @exception  = exception
-            @options    = options.reverse_merge(env['exception_notifier.options'] || {}).reverse_merge(default_options)
+
+            env_options = env['exception_notifier.options'] || {}
+            @options    = default_options.merge(env_options).merge(options)
+
             @kontroller = env['action_controller.instance'] || MissingController.new
             @request    = ActionDispatch::Request.new(env)
             @backtrace  = exception.backtrace ? clean_backtrace(exception) : []
@@ -45,7 +59,7 @@ def background_exception_notification(exception, options = {}, default_options =
             load_custom_views
 
             @exception = exception
-            @options   = options.reverse_merge(default_options).symbolize_keys
+            @options   = default_options.merge(options).symbolize_keys
             @backtrace = exception.backtrace || []
             @timestamp = Time.current
             @sections  = @options[:background_sections]
@@ -135,88 +149,48 @@ def maybe_call(maybe_proc)
 
     def initialize(options)
       super
+
       delivery_method = (options[:delivery_method] || :smtp)
       mailer_settings_key = "#{delivery_method}_settings".to_sym
       options[:mailer_settings] = options.delete(mailer_settings_key)
 
-      merged_opts = options.reverse_merge(EmailNotifier.default_options)
-      filtered_opts = merged_opts.select do |k, _v|
-        %i[
-          sender_address exception_recipients pre_callback
-          post_callback email_prefix email_format
-          sections background_sections verbose_subject normalize_subject
-          include_controller_and_action_names_in_subject delivery_method mailer_settings
-          email_headers mailer_parent template_path deliver_with
-        ].include?(k)
-      end
-
-      filtered_opts.each { |k, v| send("#{k}=", v) }
-    end
-
-    def options
-      @options ||= {}.tap do |opts|
-        instance_variables.each { |var| opts[var[1..-1].to_sym] = instance_variable_get(var) }
-      end
-    end
-
-    def mailer
-      @mailer ||= Class.new(mailer_parent.constantize).tap do |mailer|
-        mailer.extend(EmailNotifier::Mailer)
-        mailer.mailer_name = template_path
-      end
+      @base_options = DEFAULT_OPTIONS.merge(options)
     end
 
     def call(exception, options = {})
       message = create_email(exception, options)
 
-      # FIXME: use `if Gem::Version.new(ActionMailer::VERSION::STRING) < Gem::Version.new('4.1')`
-      if deliver_with == :default
-        if message.respond_to?(:deliver_now)
-          message.deliver_now
-        else
-          message.deliver
-        end
-      else
-        message.send(deliver_with)
-      end
+      message.send(base_options[:deliver_with] || default_deliver_with(message))
     end
 
     def create_email(exception, options = {})
       env = options[:env]
-      default_options = self.options
-      if env.nil?
-        send_notice(exception, options, nil, default_options) do |_, default_opts|
+
+      send_notice(exception, options, nil, base_options) do |_, default_opts|
+        if env.nil?
           mailer.background_exception_notification(exception, options, default_opts)
-        end
-      else
-        send_notice(exception, options, nil, default_options) do |_, default_opts|
+        else
           mailer.exception_notification(env, exception, options, default_opts)
         end
       end
     end
 
-    def self.default_options
-      {
-        sender_address: %("Exception Notifier" <exception.notifier@example.com>),
-        exception_recipients: [],
-        email_prefix: '[ERROR] ',
-        email_format: :text,
-        sections: %w[request session environment backtrace],
-        background_sections: %w[backtrace data],
-        verbose_subject: true,
-        normalize_subject: false,
-        include_controller_and_action_names_in_subject: true,
-        delivery_method: nil,
-        mailer_settings: nil,
-        email_headers: {},
-        mailer_parent: 'ActionMailer::Base',
-        template_path: 'exception_notifier',
-        deliver_with: :default
-      }
-    end
-
     def self.normalize_digits(string)
       string.gsub(/[0-9]+/, 'N')
     end
+
+    private
+
+    def mailer
+      @mailer ||= Class.new(base_options[:mailer_parent].constantize).tap do |mailer|
+        mailer.extend(EmailNotifier::Mailer)
+        mailer.mailer_name = base_options[:template_path]
+      end
+    end
+
+    def default_deliver_with(message)
+      # FIXME: use `if Gem::Version.new(ActionMailer::VERSION::STRING) < Gem::Version.new('4.1')`
+      message.respond_to?(:deliver_now) ? :deliver_now : :deliver
+    end
   end
 end

test/exception_notifier/email_notifier_test.rb

@@ -16,25 +16,23 @@ class EmailNotifierTest < ActiveSupport::TestCase
       email_headers: { 'X-Custom-Header' => 'foobar' },
       sections: %w[new_section request session environment backtrace],
       background_sections: %w[new_bkg_section backtrace data],
-      pre_callback: proc { |_opts, _notifier, _backtrace, _message, message_opts| message_opts[:pre_callback_called] = 1 },
-      post_callback: proc { |_opts, _notifier, _backtrace, _message, message_opts| message_opts[:post_callback_called] = 1 },
+      pre_callback: proc { |_opts, _notifier, _backtrace, _message, _message_opts| @pre_callback_called = true },
+      post_callback: proc { |_opts, _notifier, _backtrace, _message, _message_opts| @post_callback_called = true },
       smtp_settings: {
         user_name: 'Dummy user_name',
         password: 'Dummy password'
       }
     )
 
-    @email_notifier.mailer.append_view_path "#{File.dirname(__FILE__)}/../support/views"
-
     @mail = @email_notifier.call(
       @exception,
       data: { job: 'DivideWorkerJob', payload: '1/0', message: 'My Custom Message' }
     )
   end
 
   test 'should call pre/post_callback if specified' do
-    assert_equal @email_notifier.options[:pre_callback_called], 1
-    assert_equal @email_notifier.options[:post_callback_called], 1
+    assert @pre_callback_called
+    assert @post_callback_called
   end
 
   test 'sends mail with correct content' do
@@ -77,26 +75,6 @@ class EmailNotifierTest < ActiveSupport::TestCase
     assert_equal body, @mail.decode_body
   end
 
-  test 'should have default sections overridden' do
-    %w[new_section request session environment backtrace].each do |section|
-      assert_includes @email_notifier.sections, section
-    end
-  end
-
-  test 'should have default background sections' do
-    %w[new_bkg_section backtrace data].each do |section|
-      assert_includes @email_notifier.background_sections, section
-    end
-  end
-
-  test 'should have mailer_parent by default' do
-    assert_equal @email_notifier.mailer_parent, 'ActionMailer::Base'
-  end
-
-  test 'should have template_path by default' do
-    assert_equal @email_notifier.template_path, 'exception_notifier'
-  end
-
   test 'should normalize multiple digits into one N' do
     assert_equal 'N foo N bar N baz N',
                  ExceptionNotifier::EmailNotifier.normalize_digits('1 foo 12 bar 123 baz 1234')
@@ -107,7 +85,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
       raise ArgumentError
     rescue StandardError => e
       @vowel_exception = e
-      @vowel_mail = @email_notifier.create_email(@vowel_exception)
+      @vowel_mail = @email_notifier.call(@vowel_exception)
     end
 
     assert_includes @vowel_mail.encoded, "An ArgumentError occurred in background at #{Time.current}"
@@ -119,7 +97,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
     rescue StandardError => e
       @ignored_exception = e
       unless ExceptionNotifier.ignored_exceptions.include?(@ignored_exception.class.name)
-        ignored_mail = @email_notifier.create_email(@ignored_exception)
+        ignored_mail = @email_notifier.call(@ignored_exception)
       end
     end
 
@@ -130,11 +108,10 @@ class EmailNotifierTest < ActiveSupport::TestCase
   test 'should encode environment strings' do
     email_notifier = ExceptionNotifier::EmailNotifier.new(
       sender_address: '<dummynotifier@example.com>',
-      exception_recipients: %w[dummyexceptions@example.com],
-      deliver_with: :deliver_now
+      exception_recipients: %w[dummyexceptions@example.com]
     )
 
-    mail = email_notifier.create_email(
+    mail = email_notifier.call(
       @exception,
       env: {
         'REQUEST_METHOD' => 'GET',
@@ -247,8 +224,6 @@ def index; end
       post_callback: proc { |_opts, _notifier, _backtrace, _message, message_opts| message_opts[:post_callback_called] = 1 }
     )
 
-    @email_notifier.mailer.append_view_path "#{File.dirname(__FILE__)}/../support/views"
-
     @controller = HomeController.new
     @controller.process(:index)
 

test/test_helper.rb

@@ -12,3 +12,4 @@
 ExceptionNotifier.testing_mode!
 Time.zone = 'UTC'
 ActionMailer::Base.delivery_method = :test
+ActionMailer::Base.append_view_path "#{File.dirname(__FILE__)}/support/views"