Refactor delivery methods.

5 files changed, 113 insertions, 165 deletions

diff --git a/actionmailer/lib/action_mailer.rb b/actionmailer/lib/action_mailer.rb
index 66b07c39f4..f1c94e9e69 100644
--- a/actionmailer/lib/action_mailer.rb
+++ b/actionmailer/lib/action_mailer.rb
@@ -32,6 +32,7 @@ module ActionMailer
 
   autoload :AdvAttrAccessor
   autoload :Base
+  autoload :DeliveryMethods
   autoload :DeprecatedBody
   autoload :MailHelper
   autoload :Quoting
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 5ece35e69b..be6d93316f 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -264,6 +264,8 @@ module ActionMailer #:nodoc:
     helper  ActionMailer::MailHelper
     include ActionMailer::DeprecatedBody
 
+    include ActionMailer::DeliveryMethods
+
     private_class_method :new #:nodoc:
 
     @@raise_delivery_errors = true
@@ -354,9 +356,6 @@ module ActionMailer #:nodoc:
     # Alias controller_path to mailer_name so render :partial in views work.
     alias :controller_path :mailer_name
 
-    superclass_delegating_accessor :delivery_method
-    self.delivery_method = :smtp
-
     class << self
 
       def mailer_name
@@ -364,38 +363,10 @@ module ActionMailer #:nodoc:
       end
       attr_writer :mailer_name
 
-      # Mail uses the same defaults as Rails, except for the file delivery method
-      # save location so we just add this here.
-      def delivery_settings
-        @@delivery_settings ||= begin
-          hash = Hash.new { |h,k| h[k] = {} }
-          hash[:file] = {
-            :location => defined?(Rails.root) ? "#{Rails.root}/tmp/mails" : "#{Dir.tmpdir}/mails"
-          }
-
-          hash[:smtp] = { 
-            :address    => "localhost",
-            :port       => 25,
-            :domain     => 'localhost.localdomain',
-            :user_name  => nil,
-            :password   => nil,
-            :authentication       => nil,
-            :enable_starttls_auto => true
-          }
-
-          hash[:sendmail] = {
-            :location   => '/usr/sbin/sendmail',
-            :arguments  => '-i -t'
-          }
-
-          hash
-        end
-      end
-
       alias :controller_path :mailer_name
 
       def respond_to?(method_symbol, include_private = false) #:nodoc:
-        matches_dynamic_method?(method_symbol) || matches_settings_method?(method_symbol) || super
+        matches_dynamic_method?(method_symbol) || super
       end
 
       def method_missing(method_symbol, *parameters) #:nodoc:
@@ -406,13 +377,6 @@ module ActionMailer #:nodoc:
             when 'new'     then nil
             else super
           end
-        elsif match = matches_settings_method?(method_symbol)
-          # TODO Deprecation warning
-          if match[2]
-            delivery_settings[match[1].to_sym] = parameters[0]
-          else
-            delivery_settings[match[1].to_sym]
-          end
         else
           super
         end
@@ -447,11 +411,13 @@ module ActionMailer #:nodoc:
         raise "no mail object available for delivery!" unless mail
 
         begin
-          ActiveSupport::Notifications.instrument("action_mailer.deliver",
-            :mailer => self.name) do |payload|
+          ActiveSupport::Notifications.instrument("action_mailer.deliver", :mailer => self.name) do |payload|
             set_payload_for_mail(payload, mail)
-            
-            mail.delivery_method delivery_method, get_delivery_settings(delivery_method)
+
+            # TODO Move me to the instance
+            mail.delivery_method delivery_methods[delivery_method],
+                                 delivery_settings[delivery_method]
+
             if @@perform_deliveries
               mail.deliver! 
               self.deliveries << mail
@@ -486,25 +452,12 @@ module ActionMailer #:nodoc:
 
       private
 
-        def get_delivery_settings(method) #:nodoc:
-          delivery_settings[method]
-        end
-
-        def matches_settings_method?(method_name) #:nodoc:
-          /(\w+)_settings(=)?$/.match(method_name.to_s)
-        end
-
         def matches_dynamic_method?(method_name) #:nodoc:
           method_name = method_name.to_s
           /^(create|deliver)_([_a-z]\w*)/.match(method_name) || /^(new)$/.match(method_name)
         end
     end
 
-    # Configure delivery method. Check ActionMailer::DeliveryMethod for more
-    # instructions.
-    superclass_delegating_reader :delivery_method
-    self.delivery_method = :smtp
-
     # Add a part to a multipart message, with the given content-type. The
     # part itself is yielded to the block so that other properties (charset,
     # body, headers, etc.) can be set on it.
@@ -534,20 +487,6 @@ module ActionMailer #:nodoc:
       part(params, &block)
     end
 
-    # Allow you to set assigns for your template:
-    #
-    #   body :greetings => "Hi"
-    #
-    # Will make @greetings available in the template to be rendered.
-    def body(object=nil)
-      returning(super) do # Run deprecation hooks
-        if object.is_a?(Hash)
-          @assigns_set = true
-          object.each { |k, v| instance_variable_set(:"@#{k}", v) }
-        end
-      end
-    end
-
     # 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"
@@ -591,6 +530,8 @@ module ActionMailer #:nodoc:
       #   render_message "special_message"
       #   render_message :template => "special_message"
       #   render_message :inline => "<%= 'Hi!' %>"
+      #
+      # TODO Deprecate me
       def render_message(object)
         case object
         when String
diff --git a/actionmailer/lib/action_mailer/delivery_methods.rb b/actionmailer/lib/action_mailer/delivery_methods.rb
new file mode 100644
index 0000000000..c8c4148353
--- /dev/null
+++ b/actionmailer/lib/action_mailer/delivery_methods.rb
@@ -0,0 +1,71 @@
+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"
+
+      add_delivery_method :sendmail, Mail::Sendmail,
+        :location   => '/usr/sbin/sendmail',
+        :arguments  => '-i -t'
+
+      add_delivery_method :test, Mail::TestMailer
+
+      superclass_delegating_reader :delivery_method
+      self.delivery_method = :smtp
+    end
+
+    module ClassMethods
+      def delivery_settings
+        @@delivery_settings ||= Hash.new { |h,k| h[k] = {} }
+      end
+
+      def delivery_methods
+        @@delivery_methods ||= {}
+      end
+
+      def delivery_method=(method)
+        raise ArgumentError, "Unknown delivery method #{method.inspect}" unless delivery_methods[method]
+        @delivery_method = method
+      end
+
+      def add_delivery_method(symbol, klass, default_options={})
+        self.delivery_methods[symbol]  = klass
+        self.delivery_settings[symbol] = default_options
+      end
+
+      def respond_to?(method_symbol, include_private = false) #:nodoc:
+        matches_settings_method?(method_symbol) || super
+      end
+
+    protected
+
+      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
+        else
+          super
+        end
+      end
+
+      def matches_settings_method?(method_name) #:nodoc:
+        /(#{delivery_methods.keys.join('|')})_settings(=)?$/.match(method_name.to_s)
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/actionmailer/lib/action_mailer/deprecated_body.rb b/actionmailer/lib/action_mailer/deprecated_body.rb
index 5379b33a54..c82610014f 100644
--- a/actionmailer/lib/action_mailer/deprecated_body.rb
+++ b/actionmailer/lib/action_mailer/deprecated_body.rb
@@ -1,6 +1,6 @@
 module ActionMailer
   # TODO Remove this module all together in a next release. Ensure that super
-  # hooks and @assigns_set in ActionMailer::Base are removed as well.
+  # hooks in ActionMailer::Base are removed as well.
   module DeprecatedBody
     extend ActionMailer::AdvAttrAccessor
 
@@ -22,12 +22,14 @@ module ActionMailer
     end
 
     def create_parts
-      if String === @body && !defined?(@assigns_set)
+      if String === @body
         ActiveSupport::Deprecation.warn('body(String) is deprecated. To set the body with a text ' <<
                                         'call render(:text => "body")', caller[0,10])
         self.response_body = @body
-      elsif self.response_body
-        @body = self.response_body
+      elsif @body.is_a?(Hash) && !@body.empty?
+        ActiveSupport::Deprecation.warn('body(Hash) is deprecated. Use instance variables to define ' <<
+                                        'assigns in your view', caller[0,10])
+        @body.each { |k, v| instance_variable_set(:"@#{k}", v) }
       end
     end
 
diff --git a/actionmailer/test/delivery_method_test.rb b/actionmailer/test/delivery_method_test.rb
index fb43086423..1e7408d6d6 100644
--- a/actionmailer/test/delivery_method_test.rb
+++ b/actionmailer/test/delivery_method_test.rb
@@ -1,41 +1,14 @@
 require 'abstract_unit'
 require 'mail'
 
-class DefaultDeliveryMethodMailer < ActionMailer::Base
+class MyCustomDelivery
 end
 
-class NonDefaultDeliveryMethodMailer < ActionMailer::Base
-  self.delivery_method = :sendmail
-end
-
-class FileDeliveryMethodMailer < ActionMailer::Base
-  self.delivery_method = :file
-end
-
-class CustomDeliveryMethod
-
-  def initialize(values)
-    @custom_deliveries = []
-  end
-
-  attr_accessor :custom_deliveries
-  
-  attr_accessor :settings
-  
-  def deliver!(mail)
-    self.custom_deliveries << mail
-  end
-end
-
-class CustomerDeliveryMailer < ActionMailer::Base
-  self.delivery_method = CustomDeliveryMethod
-end
-
-class ActionMailerBase_delivery_method_Test < Test::Unit::TestCase
+class DefaultsDeliveryMethodsTest < ActionMailer::TestCase
   def setup
     set_delivery_method :smtp
   end
-  
+
   def teardown
     restore_delivery_method
   end
@@ -54,87 +27,47 @@ class ActionMailerBase_delivery_method_Test < Test::Unit::TestCase
                  :enable_starttls_auto => true }
     assert_equal settings, ActionMailer::Base.smtp_settings
   end
-end
-
-class DefaultDeliveryMethodMailer_delivery_method_Test < Test::Unit::TestCase
-  def setup
-    set_delivery_method :smtp
-  end
-  
-  def teardown
-    restore_delivery_method
-  end
-  
-  def test_should_be_the_default_smtp
-    assert_equal :smtp, DefaultDeliveryMethodMailer.delivery_method
-  end
-
-  def test_should_have_default_smtp_delivery_method_settings
-    settings = { :address              => "localhost",
-                 :port                 => 25,
-                 :domain               => 'localhost.localdomain',
-                 :user_name            => nil,
-                 :password             => nil,
-                 :authentication       => nil,
-                 :enable_starttls_auto => true }
-    assert_equal settings, DefaultDeliveryMethodMailer.smtp_settings
-  end
-end
-
-class NonDefaultDeliveryMethodMailer_delivery_method_Test < Test::Unit::TestCase
-  def setup
-    set_delivery_method :smtp
-  end
-  
-  def teardown
-    restore_delivery_method
-  end
 
-  def test_should_be_the_set_delivery_method
-    assert_equal :sendmail, NonDefaultDeliveryMethodMailer.delivery_method
+  def test_should_have_default_file_delivery_method_settings
+    settings = {:location => "#{Dir.tmpdir}/mails"}
+    assert_equal settings, ActionMailer::Base.file_settings
   end
 
   def test_should_have_default_sendmail_delivery_method_settings
     settings = {:location       => '/usr/sbin/sendmail',
                 :arguments      => '-i -t'}
-    assert_equal settings, NonDefaultDeliveryMethodMailer.sendmail_settings
+    assert_equal settings, ActionMailer::Base.sendmail_settings
   end
 end
 
-class FileDeliveryMethodMailer_delivery_method_Test < Test::Unit::TestCase
+class CustomDeliveryMethodsTest < ActionMailer::TestCase
   def setup
-    set_delivery_method :smtp
+    ActionMailer::Base.add_delivery_method :custom, MyCustomDelivery
   end
 
   def teardown
-    restore_delivery_method
-  end
-
-  def test_should_be_the_set_delivery_method
-    assert_equal :file, FileDeliveryMethodMailer.delivery_method
-  end
-
-  def test_should_have_default_file_delivery_method_settings
-    settings = {:location => "#{Dir.tmpdir}/mails"}
-    assert_equal settings, FileDeliveryMethodMailer.file_settings
+    ActionMailer::Base.delivery_methods.delete(:custom)
+    ActionMailer::Base.delivery_settings.delete(:custom)
   end
-end
 
-class CustomDeliveryMethodMailer_delivery_method_Test < Test::Unit::TestCase
-  def setup
-    set_delivery_method :smtp
+  def test_allow_to_add_a_custom_delivery_method
+    ActionMailer::Base.delivery_method = :custom
+    assert_equal :custom, ActionMailer::Base.delivery_method
   end
 
-  def teardown
-    restore_delivery_method
+  def test_allow_to_customize_custom_settings
+    ActionMailer::Base.custom_settings = { :foo => :bar }
+    assert_equal Hash[:foo => :bar], ActionMailer::Base.custom_settings
   end
 
-  def test_should_be_the_set_delivery_method
-    assert_equal CustomDeliveryMethod, CustomerDeliveryMailer.delivery_method
+  def test_respond_to_custom_method_settings
+    assert_respond_to ActionMailer::Base, :custom_settings
+    assert_respond_to ActionMailer::Base, :custom_settings=
   end
 
-  def test_should_have_default_custom_delivery_method_settings
-    settings = {}
-    assert_equal settings, CustomerDeliveryMailer.custom_settings
+  def test_should_not_respond_for_invalid_method_settings
+    assert_raise NoMethodError do
+      ActionMailer::Base.another_settings
+    end
   end
 end