Some refactoring.

4 files changed, 164 insertions, 164 deletions

diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index e7eb6bffcd..b881611cfb 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -263,33 +263,42 @@ module ActionMailer #:nodoc:
     include AbstractController::UrlFor
 
     helper  ActionMailer::MailHelper
+
     include ActionMailer::DeprecatedApi
+    extend  ActionMailer::DeliveryMethods
+
+    add_delivery_method :smtp, Mail::SMTP,
+      :address              => "localhost",
+      :port                 => 25,
+      :domain               => 'localhost.localdomain',
+      :user_name            => nil,
+      :password             => nil,
+      :authentication       => nil,
+      :enable_starttls_auto => true
+
+    add_delivery_method :file, Mail::FileDelivery,
+      :location => defined?(Rails.root) ? "#{Rails.root}/tmp/mails" : "#{Dir.tmpdir}/mails"
+
+    add_delivery_method :sendmail, Mail::Sendmail,
+      :location   => '/usr/sbin/sendmail',
+      :arguments  => '-i -t'
 
-    include ActionMailer::DeliveryMethods
+    add_delivery_method :test, Mail::TestMailer
+
+    superclass_delegating_reader :delivery_method
+    self.delivery_method = :smtp
 
     private_class_method :new #:nodoc:
 
-    @@raise_delivery_errors = true
     cattr_accessor :raise_delivery_errors
+    @@raise_delivery_errors = true
 
-    @@perform_deliveries = true
     cattr_accessor :perform_deliveries
-
-    # Provides a list of emails that have been delivered by Mail
-    def self.deliveries
-      Mail.deliveries
-    end
-
-    # Allows you to over write the default deliveries store from an array to some
-    # other object.  If you just want to clear the store, call Mail.deliveries.clear.
-    def self.deliveries=(val)
-      Mail.deliveries = val
-    end
+    @@perform_deliveries = true
 
     extlib_inheritable_accessor :default_charset
     self.default_charset = "utf-8"
 
-    # TODO This should be used when calling render
     extlib_inheritable_accessor :default_content_type
     self.default_content_type = "text/plain"
 
@@ -305,24 +314,9 @@ module ActionMailer #:nodoc:
     extlib_inheritable_accessor :default_implicit_parts_order
     self.default_implicit_parts_order = [ "text/plain", "text/enriched", "text/html" ]
 
-    # Expose the internal Mail message
-    # TODO: Make this an _internal ivar? 
-    attr_reader :message
-    
-    def headers(args=nil)
-      if args
-        ActiveSupport::Deprecation.warn "headers(Hash) is deprecated, please do headers[key] = value instead", caller
-        @headers = args
-      else
-        @message
-      end
-    end
-
-    def attachments
-      @message.attachments
-    end
-
     class << self
+      # Provides a list of emails that have been delivered by Mail
+      delegate :deliveries, :deliveries=, :to => Mail
 
       def mailer_name
         @mailer_name ||= name.underscore
@@ -359,6 +353,7 @@ module ActionMailer #:nodoc:
         self.view_paths = ActionView::Base.process_view_paths(root)
       end
 
+      # TODO The delivery should happen inside the instrument block
       def delivered_email(mail)
         ActiveSupport::Notifications.instrument("action_mailer.deliver", :mailer => self.name) do |payload|
           self.set_payload_for_mail(payload, mail)
@@ -377,66 +372,63 @@ module ActionMailer #:nodoc:
       end
     end
 
+    attr_internal :message
+
     # Instantiate a new mailer object. If +method_name+ is not +nil+, the mailer
     # will be initialized according to the named method. If not, the mailer will
     # remain uninitialized (useful when you only need to invoke the "receive"
     # method, for instance).
     def initialize(method_name=nil, *args)
       super()
-      @message = Mail.new
+      @_message = Mail.new
       process(method_name, *args) if method_name
     end
 
-    # TODO: Clean this up and refactor before Rails 3.0 release.
-    # This works for now, but not neat
-    def mail(headers = {})
-      # Guard flag to prevent both the old and the new API from firing
-      # Should be removed when old API is deprecated
-      @mail_was_called = true
+    def headers(args=nil)
+      if args
+        ActiveSupport::Deprecation.warn "headers(Hash) is deprecated, please do headers[key] = value instead", caller[0,2]
+        @headers = args
+      else
+        @_message
+      end
+    end
 
-      m = @message
+    def attachments
+      @_message.attachments
+    end
 
-      m.register_for_delivery_notification(self.class)
+    def mail(headers={}, &block)
+      # Guard flag to prevent both the old and the new API from firing
+      # Should be removed when old API is removed
+      @mail_was_called = true
+      m = @_message
 
       # Give preference to headers and fallback to the ones set in mail
       content_type = headers[:content_type] || m.content_type
       charset      = headers[:charset]      || m.charset      || self.class.default_charset.dup
       mime_version = headers[:mime_version] || m.mime_version || self.class.default_mime_version.dup
 
+      # Set subjects and fields quotings
       headers[:subject] ||= default_subject
-      quote_fields(m, headers, charset)
-
-      sort_order = headers[:parts_order] || self.class.default_implicit_parts_order.dup
-
-      responses = if headers[:body]
-        [ { :body => headers[:body], :content_type => self.class.default_content_type.dup } ]
-      elsif block_given?
-        collector = ActionMailer::Collector.new(self) { render(action_name) }
-        yield(collector)
-        # Collect the sort order of the parts from the collector as Mail will always
-        # sort parts on encode into a "sane" sequence.
-        sort_order = collector.responses.map { |r| r[:content_type] }
-        collector.responses
-      else
-        # TODO Ensure that we don't need to pass I18n.locale as detail
-        templates = self.class.template_root.find_all(action_name, {}, self.class.mailer_name)
-
-        templates.map do |template|
-          { :body => render_to_body(:_template => template),
-            :content_type => template.mime_type.to_s }
-        end
-      end
+      quote_fields!(headers, charset)
 
+      # Render the templates and blocks
+      responses, sort_order = collect_responses_and_sort_order(headers, &block)
       content_type ||= create_parts_from_responses(m, responses, charset)
+
+      # Tidy up content type, charset, mime version and sort order
       m.content_type = content_type
       m.charset      = charset
       m.mime_version = mime_version
+      sort_order     = headers[:parts_order] || sort_order || self.class.default_implicit_parts_order.dup
 
       if m.multipart?
         m.body.set_sort_order(sort_order)
         m.body.sort_parts!
       end
 
+      # Finaly set delivery behavior configured in class
+      wrap_delivery_behavior!(headers[:delivery_method])
       m
     end
 
@@ -448,14 +440,44 @@ module ActionMailer #:nodoc:
     end
 
     # TODO: Move this into Mail
-    def quote_fields(m, headers, charset) #:nodoc:
-      m.subject   ||= quote_if_necessary(headers[:subject], charset)          if headers[:subject]
-      m.to        ||= quote_address_if_necessary(headers[:to], charset)       if headers[:to]
-      m.from      ||= quote_address_if_necessary(headers[:from], charset)     if headers[:from]
-      m.cc        ||= quote_address_if_necessary(headers[:cc], charset)       if headers[:cc]
-      m.bcc       ||= quote_address_if_necessary(headers[:bcc], charset)      if headers[:bcc]
-      m.reply_to  ||= quote_address_if_necessary(headers[:reply_to], charset) if headers[:reply_to]
-      m.date      ||= headers[:date]                                          if headers[:date]
+    def quote_fields!(headers, charset) #:nodoc:
+      m = @_message
+      m.subject  ||= quote_if_necessary(headers[:subject], charset)          if headers[:subject]
+      m.to       ||= quote_address_if_necessary(headers[:to], charset)       if headers[:to]
+      m.from     ||= quote_address_if_necessary(headers[:from], charset)     if headers[:from]
+      m.cc       ||= quote_address_if_necessary(headers[:cc], charset)       if headers[:cc]
+      m.bcc      ||= quote_address_if_necessary(headers[:bcc], charset)      if headers[:bcc]
+      m.reply_to ||= quote_address_if_necessary(headers[:reply_to], charset) if headers[:reply_to]
+      m.date     ||= headers[:date]                                          if headers[:date]
+    end
+
+    def collect_responses_and_sort_order(headers) #:nodoc:
+      responses, sort_order = [], nil
+
+      if block_given?
+        collector = ActionMailer::Collector.new(self) { render(action_name) }
+        yield(collector)
+        sort_order = collector.responses.map { |r| r[:content_type] }
+        responses  = collector.responses
+      elsif headers[:body]
+        responses << {
+          :body => headers[:body],
+          :content_type => self.class.default_content_type.dup
+        }
+      else
+        self.class.template_root.find_all(action_name, {}, self.class.mailer_name).each do |template|
+          responses << {
+            :body => render_to_body(:_template => template),
+            :content_type => template.mime_type.to_s
+          }
+        end
+      end
+
+      [responses, sort_order]
+    end
+
+    def wrap_delivery_behavior!(method=nil) #:nodoc:
+      self.class.wrap_delivery_behavior(@_message, method)
     end
 
     def create_parts_from_responses(m, responses, charset) #:nodoc:
diff --git a/actionmailer/lib/action_mailer/delivery_methods.rb b/actionmailer/lib/action_mailer/delivery_methods.rb
index 5883e446f2..16b84d4118 100644
--- a/actionmailer/lib/action_mailer/delivery_methods.rb
+++ b/actionmailer/lib/action_mailer/delivery_methods.rb
@@ -1,73 +1,63 @@
 module ActionMailer
   # This modules makes a DSL for adding delivery methods to ActionMailer
   module DeliveryMethods
-    extend ActiveSupport::Concern
-
-    included do
-      add_delivery_method :smtp, Mail::SMTP,
-        :address              => "localhost",
-        :port                 => 25,
-        :domain               => 'localhost.localdomain',
-        :user_name            => nil,
-        :password             => nil,
-        :authentication       => nil,
-        :enable_starttls_auto => true
-
-      add_delivery_method :file, Mail::FileDelivery,
-        :location => defined?(Rails.root) ? "#{Rails.root}/tmp/mails" : "#{Dir.tmpdir}/mails"
+    # TODO Make me class inheritable
+    def delivery_settings
+      @@delivery_settings ||= Hash.new { |h,k| h[k] = {} }
+    end
 
-      add_delivery_method :sendmail, Mail::Sendmail,
-        :location   => '/usr/sbin/sendmail',
-        :arguments  => '-i -t'
+    def delivery_methods
+      @@delivery_methods ||= {}
+    end
 
-      add_delivery_method :test, Mail::TestMailer
+    def delivery_method=(method)
+      raise ArgumentError, "Unknown delivery method #{method.inspect}" unless delivery_methods[method]
+      @delivery_method = method
+    end
 
-      superclass_delegating_reader :delivery_method
-      self.delivery_method = :smtp
+    def add_delivery_method(symbol, klass, default_options={})
+      self.delivery_methods[symbol]  = klass
+      self.delivery_settings[symbol] = default_options
     end
 
-    module ClassMethods
-      # TODO Make me class inheritable
-      def delivery_settings
-        @@delivery_settings ||= Hash.new { |h,k| h[k] = {} }
-      end
+    def wrap_delivery_behavior(mail, method=nil)
+      method ||= delivery_method
 
-      def delivery_methods
-        @@delivery_methods ||= {}
-      end
+      mail.register_for_delivery_notification(self)
 
-      def delivery_method=(method)
-        raise ArgumentError, "Unknown delivery method #{method.inspect}" unless delivery_methods[method]
-        @delivery_method = method
+      if method.is_a?(Symbol)
+        mail.delivery_method(delivery_methods[method],
+                             delivery_settings[method])
+      else
+        mail.delivery_method(method)
       end
 
-      def add_delivery_method(symbol, klass, default_options={})
-        self.delivery_methods[symbol]  = klass
-        self.delivery_settings[symbol] = default_options
-      end
+      mail.perform_deliveries    = perform_deliveries
+      mail.raise_delivery_errors = raise_delivery_errors
+    end
 
-      def respond_to?(method_symbol, include_private = false) #:nodoc:
-        matches_settings_method?(method_symbol) || super
-      end
 
-    protected
+    def respond_to?(method_symbol, include_private = false) #:nodoc:
+      matches_settings_method?(method_symbol) || super
+    end
 
-      # TODO Get rid of this method missing magic
-      def method_missing(method_symbol, *parameters) #:nodoc:
-        if match = matches_settings_method?(method_symbol)
-          if match[2]
-            delivery_settings[match[1].to_sym] = parameters[0]
-          else
-            delivery_settings[match[1].to_sym]
-          end
+  protected
+
+    # TODO Get rid of this method missing magic
+    def method_missing(method_symbol, *parameters) #:nodoc:
+      if match = matches_settings_method?(method_symbol)
+        if match[2]
+          delivery_settings[match[1].to_sym] = parameters[0]
         else
-          super
+          delivery_settings[match[1].to_sym]
         end
+      else
+        super
       end
+    end
 
-      def matches_settings_method?(method_name) #:nodoc:
-        /(#{delivery_methods.keys.join('|')})_settings(=)?$/.match(method_name.to_s)
-      end
+    def matches_settings_method?(method_name) #:nodoc:
+      /(#{delivery_methods.keys.join('|')})_settings(=)?$/.match(method_name.to_s)
     end
   end
 end
\ No newline at end of file
diff --git a/actionmailer/lib/action_mailer/deprecated_api.rb b/actionmailer/lib/action_mailer/deprecated_api.rb
index a2fa481d0e..f969584a17 100644
--- a/actionmailer/lib/action_mailer/deprecated_api.rb
+++ b/actionmailer/lib/action_mailer/deprecated_api.rb
@@ -71,7 +71,6 @@ module ActionMailer
 
       # Alias controller_path to mailer_name so render :partial in views work.
       alias :controller_path :mailer_name
-      
     end
 
     module ClassMethods
@@ -82,17 +81,14 @@ module ActionMailer
       #   email = MyMailer.create_some_mail(parameters)
       #   email.set_some_obscure_header "frobnicate"
       #   MyMailer.deliver(email)
-      def deliver(mail)
-        return if @mail_was_called
-        raise "no mail object available for delivery!" unless mail
-
-        mail.register_for_delivery_notification(self)
-
-        mail.delivery_method delivery_methods[delivery_method],
-                             delivery_settings[delivery_method]
+      def deliver(mail, show_warning=true)
+        if show_warning
+          ActiveSupport::Deprecation.warn "ActionMailer::Base.deliver is deprecated, just call " <<
+            "deliver in the instance instead", caller
+        end
 
-        mail.raise_delivery_errors = raise_delivery_errors
-        mail.perform_deliveries = perform_deliveries
+        raise "no mail object available for delivery!" unless mail
+        wrap_delivery_behavior(mail)
         mail.deliver
         mail
       end
@@ -122,23 +118,19 @@ module ActionMailer
       end
     end
 
-    def initialize(*)
-      super()
-      @mail_was_called = false
-    end
-
     # Delivers a Mail object. By default, it delivers the cached mail
     # object (from the <tt>create!</tt> method). If no cached mail object exists, and
     # no alternate has been given as the parameter, this will fail.
-    def deliver!(mail = @message)
-      self.class.deliver(mail)
+    def deliver!(mail = @_message)
+      self.class.deliver(mail, false)
     end
+    alias :deliver :deliver!
 
     def render(*args)
       options = args.last.is_a?(Hash) ? args.last : {}
       if options[:body]
-        ActiveSupport::Deprecation.warn(':body in render deprecated. Please call body ' <<
-                                        'with a hash instead', caller[0,1])
+        ActiveSupport::Deprecation.warn(':body in render deprecated. Please use instance ' <<
+                                        'variables as assigns instead', caller[0,1])
 
         body options.delete(:body)
       end
@@ -153,7 +145,7 @@ module ActionMailer
         create_parts
         create_mail
       end
-      @message
+      @_message
     end
 
     # Add a part to a multipart message, with the given content-type. The
@@ -175,6 +167,7 @@ module ActionMailer
     # Add an attachment to a multipart message. This is simply a part with the
     # content-disposition set to "attachment".
     def attachment(params, &block)
+      ActiveSupport::Deprecation.warn "attachment is deprecated, please use the attachments API instead", caller[0,2]
       params = { :content_type => params } if String === params
 
       params[:content] ||= params.delete(:data) || params.delete(:body)
@@ -197,13 +190,9 @@ module ActionMailer
     #   render_message :template => "special_message"
     #   render_message :inline => "<%= 'Hi!' %>"
     #
-    def render_message(object)
-      case object
-      when String
-        render_to_body(:template => object)
-      else
-        render_to_body(object)
-      end
+    def render_message(*args)
+      ActiveSupport::Deprecation.warn "render_message is deprecated, use render instead", caller[0,2]
+      render(*args)
     end
 
   private
@@ -240,14 +229,12 @@ module ActionMailer
     end
     
     def create_mail #:nodoc:
-      m = @message
-
-      m.subject,     = quote_any_if_necessary(charset, subject)
-      m.to, m.from   = quote_any_address_if_necessary(charset, recipients, from)
-      m.bcc          = quote_address_if_necessary(bcc, charset) unless bcc.nil?
-      m.cc           = quote_address_if_necessary(cc, charset) unless cc.nil?
-      m.reply_to     = quote_address_if_necessary(reply_to, charset) unless reply_to.nil?
-      m.mime_version = mime_version unless mime_version.nil?
+      m = @_message
+
+      quote_fields!({:subject => subject, :to => recipients, :from => from,
+                    :bcc => bcc, :cc => cc, :reply_to => reply_to}, charset)
+
+      m.mime_version = mime_version    unless mime_version.nil?
       m.date         = sent_on.to_time rescue sent_on if sent_on
 
       @headers.each { |k, v| m[k] = v }
@@ -274,7 +261,7 @@ module ActionMailer
 
       m.content_transfer_encoding = '8bit' unless m.body.only_us_ascii?
       
-      @message
+      @_message
     end
     
     # Set up the default values for the various instance variables of this
@@ -286,8 +273,9 @@ module ActionMailer
       @implicit_parts_order ||= self.class.default_implicit_parts_order.dup
       @mime_version         ||= self.class.default_mime_version.dup if self.class.default_mime_version
 
-      @mailer_name ||= self.class.mailer_name.dup
-      @template    ||= method_name
+      @mailer_name   ||= self.class.mailer_name.dup
+      @template      ||= method_name
+      @mail_was_called = false
 
       @parts   ||= []
       @headers ||= {}
diff --git a/actionmailer/lib/action_mailer/tmail_compat.rb b/actionmailer/lib/action_mailer/tmail_compat.rb
index d78332c135..c6efdc53b6 100644
--- a/actionmailer/lib/action_mailer/tmail_compat.rb
+++ b/actionmailer/lib/action_mailer/tmail_compat.rb
@@ -3,7 +3,7 @@ module Mail
     
     def set_content_type(*args)
       ActiveSupport::Deprecation.warn('Message#set_content_type is deprecated, please just call ' <<
-                                      'Message#content_type with the same arguments', caller[0,10])
+                                      'Message#content_type with the same arguments', caller[0,2])
       content_type(*args)
     end
     
@@ -11,7 +11,7 @@ module Mail
     def transfer_encoding(value = nil)
       if value
         ActiveSupport::Deprecation.warn('Message#transfer_encoding is deprecated, please call ' <<
-                                        'Message#content_transfer_encoding with the same arguments', caller[0,10])
+                                        'Message#content_transfer_encoding with the same arguments', caller[0,2])
         content_transfer_encoding(value)
       else
         old_transfer_encoding
@@ -20,7 +20,7 @@ module Mail
     
     def original_filename
       ActiveSupport::Deprecation.warn('Message#original_filename is deprecated, ' <<
-                                      'please call Message#filename', caller[0,10])
+                                      'please call Message#filename', caller[0,2])
       filename
     end