Merge pull request #22120 from hnatt/refactor-actionmailer

Refactor ActionMailer::Base

actionmailer/lib/action_mailer/base.rb

@@ -464,30 +464,26 @@ def register_interceptors(*interceptors)
       # Either a class, string or symbol can be passed in as the Observer.
       # If a string or symbol is passed in it will be camelized and constantized.
       def register_observer(observer)
-        delivery_observer = case observer
-          when String, Symbol
-            observer.to_s.camelize.constantize
-          else
-            observer
-          end
-
-        Mail.register_observer(delivery_observer)
+        Mail.register_observer(observer_class_for(observer))
       end
 
       # Register an Interceptor which will be called before mail is sent.
       # Either a class, string or symbol can be passed in as the Interceptor.
       # If a string or symbol is passed in it will be camelized and constantized.
       def register_interceptor(interceptor)
-        delivery_interceptor = case interceptor
-          when String, Symbol
-            interceptor.to_s.camelize.constantize
-          else
-            interceptor
-          end
-
-        Mail.register_interceptor(delivery_interceptor)
+        Mail.register_interceptor(observer_class_for(interceptor))
       end
 
+      def observer_class_for(value)
+        case value
+        when String, Symbol
+          value.to_s.camelize.constantize
+        else
+          value
+        end
+      end
+      private :observer_class_for
+
       # Returns the name of current mailer. This method is also being used as a path for a view lookup.
       # If this is an anonymous mailer, this method will return +anonymous+ instead.
       def mailer_name
@@ -796,49 +792,57 @@ def _raise_error
     #   end
     #
     def mail(headers = {}, &block)
-      return @_message if @_mail_was_called && headers.blank? && !block
-
-      m = @_message
+      return message if @_mail_was_called && headers.blank? && !block
 
       # At the beginning, do not consider class default for content_type
       content_type = headers[:content_type]
 
-      # Call all the procs (if any)
-      default_values = {}
-      self.class.default.each do |k,v|
-        default_values[k] = v.is_a?(Proc) ? instance_eval(&v) : v
-      end
-
-      # Handle defaults
-      headers = headers.reverse_merge(default_values)
-      headers[:subject] ||= default_i18n_subject
+      headers = apply_defaults(headers)
 
       # Apply charset at the beginning so all fields are properly quoted
-      m.charset = charset = headers[:charset]
+      message.charset = charset = headers[:charset]
 
       # Set configure delivery behavior
       wrap_delivery_behavior!(headers.delete(:delivery_method), headers.delete(:delivery_method_options))
 
-      # Assign all headers except parts_order, content_type, body, template_name, and template_path
-      assignable = headers.except(:parts_order, :content_type, :body, :template_name, :template_path)
-      assignable.each { |k, v| m[k] = v }
+      assign_headers_to_message(message, headers)
 
       # Render the templates and blocks
       responses = collect_responses(headers, &block)
       @_mail_was_called = true
 
-      create_parts_from_responses(m, responses)
+      create_parts_from_responses(message, responses)
 
       # Setup content type, reapply charset and handle parts order
-      m.content_type = set_content_type(m, content_type, headers[:content_type])
-      m.charset      = charset
+      message.content_type = set_content_type(message, content_type, headers[:content_type])
+      message.charset      = charset
 
-      if m.multipart?
-        m.body.set_sort_order(headers[:parts_order])
-        m.body.sort_parts!
+      if message.multipart?
+        message.body.set_sort_order(headers[:parts_order])
+        message.body.sort_parts!
       end
 
-      m
+      message
+    end
+
+  private
+
+    def apply_defaults(headers)
+      default_values = self.class.default.map do |key, value|
+        [
+          key,
+          value.is_a?(Proc) ? instance_eval(&value) : value
+        ]
+      end.to_h
+
+      headers_with_defaults = headers.reverse_merge(default_values)
+      headers_with_defaults[:subject] ||= default_i18n_subject
+      headers_with_defaults
+    end
+
+    def assign_headers_to_message(message, headers)
+      assignable = headers.except(:parts_order, :content_type, :body, :template_name, :template_path)
+      assignable.each { |k, v| message[k] = v }
     end
 
   protected
@@ -880,33 +884,33 @@ def default_i18n_subject(interpolations = {})
     end
 
     def collect_responses(headers) #:nodoc:
-      responses = []
-
       if block_given?
         collector = ActionMailer::Collector.new(lookup_context) { render(action_name) }
         yield(collector)
-        responses = collector.responses
+        collector.responses
       elsif headers[:body]
-        responses << {
+        [{
           body: headers.delete(:body),
           content_type: self.class.default[:content_type] || "text/plain"
-        }
+        }]
       else
-        templates_path = headers.delete(:template_path) || self.class.mailer_name
-        templates_name = headers.delete(:template_name) || action_name
+        collect_responses_from_templates(headers)
+      end
+    end
 
-        each_template(Array(templates_path), templates_name) do |template|
-          self.formats = template.formats
+    def collect_responses_from_templates(headers)
+      templates_path = headers[:template_path] || self.class.mailer_name
+      templates_name = headers[:template_name] || action_name
 
-          responses << {
-            body: render(template: template),
-            content_type: template.type.to_s
-          }
-        end
+      each_template(Array(templates_path), templates_name).map do |template|
+        self.formats = template.formats
+        {
+          body: render(template: template),
+          content_type: template.type.to_s
+        }
       end
-
-      responses
     end
+    private :collect_responses_from_templates
 
     def each_template(paths, name, &block) #:nodoc:
       templates = lookup_context.find_all(name, paths)