Finish cleaning up delivery methods implementation.

4 files changed, 66 insertions, 82 deletions

diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 77b9ac4a97..ad3a9c3c6d 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -253,10 +253,9 @@ module ActionMailer #:nodoc:
   #   and appear last in the mime encoded message. You can also pick a different order from inside a method with
   #   +implicit_parts_order+.
   class Base < AbstractController::Base
+    include DeliveryMethods, Quoting
     abstract!
 
-    include Quoting
-
     include AbstractController::Logger
     include AbstractController::Rendering
     include AbstractController::LocalizedCache
@@ -266,31 +265,9 @@ module ActionMailer #:nodoc:
 
     helper  ActionMailer::MailHelper
 
-    extend  ActionMailer::DeliveryMethods
     include ActionMailer::OldApi
     include ActionMailer::DeprecatedApi
 
-    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
-
     private_class_method :new #:nodoc:
 
     cattr_accessor :raise_delivery_errors
@@ -493,10 +470,6 @@ module ActionMailer #:nodoc:
       [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:
       if responses.size == 1 && !m.has_attachments?
         m.body = responses[0][:body]
diff --git a/actionmailer/lib/action_mailer/delivery_methods.rb b/actionmailer/lib/action_mailer/delivery_methods.rb
index 16b84d4118..38325e512f 100644
--- a/actionmailer/lib/action_mailer/delivery_methods.rb
+++ b/actionmailer/lib/action_mailer/delivery_methods.rb
@@ -1,63 +1,75 @@
+require 'tmpdir'
+
 module ActionMailer
-  # This modules makes a DSL for adding delivery methods to ActionMailer
+  # Provides a DSL for adding delivery methods to ActionMailer.
   module DeliveryMethods
-    # TODO Make me class inheritable
-    def delivery_settings
-      @@delivery_settings ||= Hash.new { |h,k| h[k] = {} }
-    end
+    extend ActiveSupport::Concern
 
-    def delivery_methods
-      @@delivery_methods ||= {}
-    end
+    included do
+      extlib_inheritable_accessor :delivery_methods, :delivery_method,
+                                  :instance_writer => false
 
-    def delivery_method=(method)
-      raise ArgumentError, "Unknown delivery method #{method.inspect}" unless delivery_methods[method]
-      @delivery_method = method
-    end
+      self.delivery_methods = {}
+      self.delivery_method  = :smtp
 
-    def add_delivery_method(symbol, klass, default_options={})
-      self.delivery_methods[symbol]  = klass
-      self.delivery_settings[symbol] = default_options
-    end
+      add_delivery_method :smtp, Mail::SMTP,
+        :address              => "localhost",
+        :port                 => 25,
+        :domain               => 'localhost.localdomain',
+        :user_name            => nil,
+        :password             => nil,
+        :authentication       => nil,
+        :enable_starttls_auto => true
 
-    def wrap_delivery_behavior(mail, method=nil)
-      method ||= delivery_method
+      add_delivery_method :file, Mail::FileDelivery,
+        :location => defined?(Rails.root) ? "#{Rails.root}/tmp/mails" : "#{Dir.tmpdir}/mails"
 
-      mail.register_for_delivery_notification(self)
+      add_delivery_method :sendmail, Mail::Sendmail,
+        :location   => '/usr/sbin/sendmail',
+        :arguments  => '-i -t'
 
-      if method.is_a?(Symbol)
-        mail.delivery_method(delivery_methods[method],
-                             delivery_settings[method])
-      else
-        mail.delivery_method(method)
-      end
-
-      mail.perform_deliveries    = perform_deliveries
-      mail.raise_delivery_errors = raise_delivery_errors
+      add_delivery_method :test, Mail::TestMailer
     end
 
+    module ClassMethods
+      # Adds a new delivery method through the given class using the given symbol
+      # as alias and the default options supplied:
+      #
+      # Example:
+      # 
+      #   add_delivery_method :sendmail, Mail::Sendmail,
+      #     :location   => '/usr/sbin/sendmail',
+      #     :arguments  => '-i -t'
+      # 
+      def add_delivery_method(symbol, klass, default_options={})
+        unless respond_to?(:"#{symbol}_settings")
+          extlib_inheritable_accessor(:"#{symbol}_settings", :instance_writer => false)
+        end
 
-    def respond_to?(method_symbol, include_private = false) #:nodoc:
-      matches_settings_method?(method_symbol) || super
-    end
+        send(:"#{symbol}_settings=", default_options)
+        self.delivery_methods[symbol.to_sym] = klass
+      end
 
-  protected
+      def wrap_delivery_behavior(mail, method=delivery_method) #:nodoc:
+        mail.register_for_delivery_notification(self)
 
-    # 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]
+        if method.is_a?(Symbol)
+          if klass = delivery_methods[method.to_sym]
+            mail.delivery_method(klass, send(:"#{method}_settings"))
+          else
+            raise "Invalid delivery method #{method.inspect}"
+          end
         else
-          delivery_settings[match[1].to_sym]
+          mail.delivery_method(method)
         end
-      else
-        super
+
+        mail.perform_deliveries    = perform_deliveries
+        mail.raise_delivery_errors = raise_delivery_errors
       end
     end
 
-    def matches_settings_method?(method_name) #:nodoc:
-      /(#{delivery_methods.keys.join('|')})_settings(=)?$/.match(method_name.to_s)
+    def wrap_delivery_behavior!(*args) #:nodoc:
+      self.class.wrap_delivery_behavior(message, *args)
     end
   end
 end
\ No newline at end of file
diff --git a/actionmailer/test/delivery_method_test.rb b/actionmailer/test/delivery_methods_test.rb
index 1e7408d6d6..de3d54197d 100644
--- a/actionmailer/test/delivery_method_test.rb
+++ b/actionmailer/test/delivery_methods_test.rb
@@ -47,7 +47,6 @@ class CustomDeliveryMethodsTest < ActionMailer::TestCase
 
   def teardown
     ActionMailer::Base.delivery_methods.delete(:custom)
-    ActionMailer::Base.delivery_settings.delete(:custom)
   end
 
   def test_allow_to_add_a_custom_delivery_method
diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb
index b3bf1b9acd..c0a3f655b9 100644
--- a/actionmailer/test/mail_service_test.rb
+++ b/actionmailer/test/mail_service_test.rb
@@ -511,7 +511,7 @@ class ActionMailerTest < Test::Unit::TestCase
   end
 
   def test_from_without_name_for_smtp
-    ActionMailer::Base.delivery_method = :smtp
+    TestMailer.delivery_method = :smtp
     TestMailer.from_without_name.deliver
 
     mail = MockSMTP.deliveries.first
@@ -522,7 +522,7 @@ class ActionMailerTest < Test::Unit::TestCase
   end
 
   def test_from_with_name_for_smtp
-    ActionMailer::Base.delivery_method = :smtp
+    TestMailer.delivery_method = :smtp
     TestMailer.from_with_name.deliver
 
     mail = MockSMTP.deliveries.first
@@ -659,7 +659,7 @@ class ActionMailerTest < Test::Unit::TestCase
 
   def test_performs_delivery_via_sendmail
     IO.expects(:popen).once.with('/usr/sbin/sendmail -i -t -f "system@loudthinking.com" test@localhost', 'w+')
-    ActionMailer::Base.delivery_method = :sendmail
+    TestMailer.delivery_method = :sendmail
     TestMailer.signed_up(@recipient).deliver
   end
 
@@ -956,7 +956,7 @@ EOF
   end
 
   def test_headers_removed_on_smtp_delivery
-    ActionMailer::Base.delivery_method = :smtp
+    TestMailer.delivery_method = :smtp
     TestMailer.cc_bcc(@recipient).deliver
     assert MockSMTP.deliveries[0][2].include?("root@loudthinking.com")
     assert MockSMTP.deliveries[0][2].include?("nobody@loudthinking.com")
@@ -1053,35 +1053,35 @@ EOF
   end
 
   def test_return_path_with_deliver
-    ActionMailer::Base.delivery_method = :smtp
+    TestMailer.delivery_method = :smtp
     TestMailer.return_path.deliver
     assert_match %r{^Return-Path: <another@somewhere.test>}, MockSMTP.deliveries[0][0]
     assert_equal "another@somewhere.test", MockSMTP.deliveries[0][1].to_s
   end
 
   def test_starttls_is_enabled_if_supported
-    ActionMailer::Base.delivery_settings[:smtp].merge!(:enable_starttls_auto => true)
+    TestMailer.smtp_settings.merge!(:enable_starttls_auto => true)
     MockSMTP.any_instance.expects(:respond_to?).with(:enable_starttls_auto).returns(true)
     MockSMTP.any_instance.expects(:enable_starttls_auto)
-    ActionMailer::Base.delivery_method = :smtp
+    TestMailer.delivery_method = :smtp
     TestMailer.signed_up(@recipient).deliver
   end
 
   def test_starttls_is_disabled_if_not_supported
-    ActionMailer::Base.delivery_settings[:smtp].merge!(:enable_starttls_auto => true)
+    TestMailer.smtp_settings.merge!(:enable_starttls_auto => true)
     MockSMTP.any_instance.expects(:respond_to?).with(:enable_starttls_auto).returns(false)
     MockSMTP.any_instance.expects(:enable_starttls_auto).never
-    ActionMailer::Base.delivery_method = :smtp
+    TestMailer.delivery_method = :smtp
     TestMailer.signed_up(@recipient).deliver
   end
 
   def test_starttls_is_not_enabled
-    TestMailer.delivery_settings[:smtp].merge!(:enable_starttls_auto => false)
+    TestMailer.smtp_settings.merge!(:enable_starttls_auto => false)
     MockSMTP.any_instance.expects(:respond_to?).never
     TestMailer.delivery_method = :smtp
     TestMailer.signed_up(@recipient).deliver
   ensure
-    TestMailer.delivery_settings[:smtp].merge!(:enable_starttls_auto => true)
+    TestMailer.smtp_settings.merge!(:enable_starttls_auto => true)
   end
 end