Tidy up ActionMailer rendering logic to take advantage of view path cache...

Tidy up ActionMailer rendering logic to take advantage of view path cache instead of using file system lookups

actionmailer/lib/action_mailer/base.rb

@@ -216,7 +216,7 @@ module ActionMailer #:nodoc:
   #   * <tt>:domain</tt> - If you need to specify a HELO domain, you can do it here.
   #   * <tt>:user_name</tt> - If your mail server requires authentication, set the username in this setting.
   #   * <tt>:password</tt> - If your mail server requires authentication, set the password in this setting.
-  #   * <tt>:authentication</tt> - If your mail server requires authentication, you need to specify the authentication type here. 
+  #   * <tt>:authentication</tt> - If your mail server requires authentication, you need to specify the authentication type here.
   #     This is a symbol and one of <tt>:plain</tt>, <tt>:login</tt>, <tt>:cram_md5</tt>.
   #
   # * <tt>sendmail_settings</tt> - Allows you to override options for the <tt>:sendmail</tt> delivery method.
@@ -233,10 +233,10 @@ module ActionMailer #:nodoc:
   # * <tt>deliveries</tt> - Keeps an array of all the emails sent out through the Action Mailer with <tt>delivery_method :test</tt>. Most useful
   #   for unit and functional testing.
   #
-  # * <tt>default_charset</tt> - The default charset used for the body and to encode the subject. Defaults to UTF-8. You can also 
+  # * <tt>default_charset</tt> - The default charset used for the body and to encode the subject. Defaults to UTF-8. You can also
   #   pick a different charset from inside a method with +charset+.
   # * <tt>default_content_type</tt> - The default content type used for the main part of the message. Defaults to "text/plain". You
-  #   can also pick a different content type from inside a method with +content_type+. 
+  #   can also pick a different content type from inside a method with +content_type+.
   # * <tt>default_mime_version</tt> - The default mime version used for the message. Defaults to <tt>1.0</tt>. You
   #   can also pick a different value from inside a method with +mime_version+.
   # * <tt>default_implicit_parts_order</tt> - When a message is built implicitly (i.e. multiple parts are assembled from templates
@@ -253,9 +253,6 @@ class Base
     class_inheritable_accessor :view_paths
     cattr_accessor :logger
 
-    cattr_accessor :template_extensions
-    @@template_extensions = ['erb', 'builder', 'rhtml', 'rxml']
-
     @@smtp_settings = {
       :address        => "localhost",
       :port           => 25,
@@ -414,15 +411,10 @@ def deliver(mail)
         new.deliver!(mail)
       end
 
-      # Register a template extension so mailer templates written in a
-      # templating language other than rhtml or rxml are supported.
-      # To use this, include in your template-language plugin's init
-      # code or on a per-application basis, this can be invoked from
-      # <tt>config/environment.rb</tt>:
-      #
-      #   ActionMailer::Base.register_template_extension('haml')
       def register_template_extension(extension)
-        template_extensions << extension
+        ActiveSupport::Deprecation.warn(
+          "ActionMailer::Base.register_template_extension has been deprecated." +
+          "Use ActionView::Base.register_template_extension instead", caller)
       end
 
       def template_root
@@ -455,16 +447,18 @@ def create!(method_name, *parameters) #:nodoc:
         # "the_template_file.text.html.erb", etc.). Only do this if parts
         # have not already been specified manually.
         if @parts.empty?
-          templates = Dir.glob("#{template_path}/#{@template}.*")
-          templates.each do |path|
-            basename = File.basename(path)
-            template_regex = Regexp.new("^([^\\\.]+)\\\.([^\\\.]+\\\.[^\\\.]+)\\\.(" + template_extensions.join('|') + ")$")
-            next unless md = template_regex.match(basename)
-            template_name = basename
-            content_type = md.captures[1].gsub('.', '/')
-            @parts << Part.new(:content_type => content_type,
-              :disposition => "inline", :charset => charset,
-              :body => render_message(template_name, @body))
+          Dir.glob("#{template_path}/#{@template}.*").each do |path|
+            template = template_root["#{mailer_name}/#{File.basename(path)}"]
+
+            # Skip unless template has a multipart format
+            next unless template.multipart?
+
+            @parts << Part.new(
+              :content_type => template.content_type,
+              :disposition => "inline",
+              :charset => charset,
+              :body => render_message(template, @body)
+            )
           end
           unless @parts.empty?
             @content_type = "multipart/alternative"
@@ -476,9 +470,8 @@ def create!(method_name, *parameters) #:nodoc:
         # also render a "normal" template (without the content type). If a
         # normal template exists (or if there were no implicit parts) we render
         # it.
-        template_exists = @parts.empty?
-        template_exists ||= Dir.glob("#{template_path}/#{@template}.*").any? { |i| File.basename(i).split(".").length == 2 }
-        @body = render_message(@template, @body) if template_exists
+        template = template_root["#{mailer_name}/#{@template}"]
+        @body = render_message(@template, @body) if template
 
         # Finally, if there are other message parts and a textual body exists,
         # we shift it onto the front of the parts and set the body to nil (so
@@ -538,7 +531,7 @@ def render_message(method_name, body)
 
       def render(opts)
         body = opts.delete(:body)
-        if opts[:file] && opts[:file] !~ /\//
+        if opts[:file] && (opts[:file] !~ /\// && !opts[:file].respond_to?(:render))
           opts[:file] = "#{mailer_name}/#{opts[:file]}"
         end
         initialize_template_class(body).render(opts)

actionmailer/test/mail_service_test.rb

@@ -219,7 +219,7 @@ def nested_multipart(recipient)
     end
     attachment :content_type => "application/octet-stream",:filename => "test.txt", :body => "test abcdefghijklmnopqstuvwxyz"
   end
-  
+
   def nested_multipart_with_body(recipient)
     recipients   recipient
     subject      "nested multipart with body"
@@ -321,7 +321,7 @@ def test_nested_parts
     assert_nothing_raised { created = TestMailer.create_nested_multipart(@recipient)}
     assert_equal 2,created.parts.size
     assert_equal 2,created.parts.first.parts.size
-    
+
     assert_equal "multipart/mixed", created.content_type
     assert_equal "multipart/alternative", created.parts.first.content_type
     assert_equal "bar", created.parts.first.header['foo'].to_s
@@ -366,7 +366,7 @@ def test_signed_up
     assert_not_nil ActionMailer::Base.deliveries.first
     assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded
   end
-  
+
   def test_custom_template
     expected = new_mail
     expected.to      = @recipient
@@ -382,7 +382,6 @@ def test_custom_template
   end
 
   def test_custom_templating_extension
-    #
     # N.b., custom_templating_extension.text.plain.haml is expected to be in fixtures/test_mailer directory
     expected = new_mail
     expected.to      = @recipient
@@ -390,18 +389,10 @@ def test_custom_templating_extension
     expected.body    = "Hello there, \n\nMr. #{@recipient}"
     expected.from    = "system@loudthinking.com"
     expected.date    = Time.local(2004, 12, 12)
-    
+
     # Stub the render method so no alternative renderers need be present.
     ActionView::Base.any_instance.stubs(:render).returns("Hello there, \n\nMr. #{@recipient}")
-    
-    # If the template is not registered, there should be no parts.
-    created = nil
-    assert_nothing_raised { created = TestMailer.create_custom_templating_extension(@recipient) }
-    assert_not_nil created
-    assert_equal 0, created.parts.length
-    
-    ActionMailer::Base.register_template_extension('haml')
-    
+
     # Now that the template is registered, there should be one part. The text/plain part.
     created = nil
     assert_nothing_raised { created = TestMailer.create_custom_templating_extension(@recipient) }
@@ -428,7 +419,7 @@ def test_cancelled_account
     assert_not_nil ActionMailer::Base.deliveries.first
     assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded
   end
-  
+
   def test_cc_bcc
     expected = new_mail
     expected.to      = @recipient
@@ -550,7 +541,7 @@ def test_perform_deliveries_flag
     TestMailer.deliver_signed_up(@recipient)
     assert_equal 1, ActionMailer::Base.deliveries.size
   end
-  
+
   def test_doesnt_raise_errors_when_raise_delivery_errors_is_false
     ActionMailer::Base.raise_delivery_errors = false
     TestMailer.any_instance.expects(:perform_delivery_test).raises(Exception)
@@ -670,7 +661,7 @@ def test_extended_headers
     assert_not_nil ActionMailer::Base.deliveries.first
     assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded
   end
-  
+
   def test_utf8_body_is_not_quoted
     @recipient = "Foo áëô îü <extended@example.net>"
     expected = new_mail "utf-8"
@@ -760,7 +751,7 @@ def test_multipart_with_mime_version
     mail = TestMailer.create_multipart_with_mime_version(@recipient)
     assert_equal "1.1", mail.mime_version
   end
-  
+
   def test_multipart_with_utf8_subject
     mail = TestMailer.create_multipart_with_utf8_subject(@recipient)
     assert_match(/\nSubject: =\?utf-8\?Q\?Foo_.*?\?=/, mail.encoded)
@@ -825,7 +816,7 @@ def test_implicitly_multipart_messages_with_charset
     mail = TestMailer.create_implicitly_multipart_example(@recipient, 'iso-8859-1')
 
     assert_equal "multipart/alternative", mail.header['content-type'].body
-    
+
     assert_equal 'iso-8859-1', mail.parts[0].sub_header("content-type", "charset")
     assert_equal 'iso-8859-1', mail.parts[1].sub_header("content-type", "charset")
     assert_equal 'iso-8859-1', mail.parts[2].sub_header("content-type", "charset")
@@ -852,7 +843,7 @@ def test_various_newlines_multipart
     assert_equal "line #1\nline #2\nline #3\nline #4\n\n", mail.parts[0].body
     assert_equal "<p>line #1</p>\n<p>line #2</p>\n<p>line #3</p>\n<p>line #4</p>\n\n", mail.parts[1].body
   end
-  
+
   def test_headers_removed_on_smtp_delivery
     ActionMailer::Base.delivery_method = :smtp
     TestMailer.deliver_cc_bcc(@recipient)

actionpack/lib/action_view/base.rb

@@ -300,6 +300,8 @@ def file_exists?(template_path)
     #   # => 'users/legacy.rhtml'
     #
     def pick_template(template_path)
+      return template_path if template_path.respond_to?(:render)
+
       path = template_path.sub(/^\//, '')
       if m = path.match(/(.*)\.(\w+)$/)
         template_file_name, template_file_extension = m[1], m[2]
@@ -343,7 +345,8 @@ def render_file(template_path, use_full_path = nil, local_assigns = {}) #:nodoc:
           ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller)
         end
 
-        if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && !template_path.include?("/")
+        if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) &&
+            template_path.is_a?(String) && !template_path.include?("/")
           raise ActionViewError, <<-END_ERROR
   Due to changes in ActionMailer, you need to provide the mailer_name along with the template name.
 

actionpack/lib/action_view/template.rb

@@ -22,6 +22,14 @@ def format_and_extension
     end
     memoize :format_and_extension
 
+    def multipart?
+      format && format.include?('.')
+    end
+
+    def content_type
+      format.gsub('.', '/')
+    end
+
     def mime_type
       Mime::Type.lookup_by_extension(format) if format
     end
@@ -84,7 +92,7 @@ def find_full_path(path, load_paths)
       #   [base_path, name, format, extension]
       def split(file)
         if m = file.match(/^(.*\/)?([^\.]+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/)
-          if m[5] # Mulipart formats
+          if m[5] # Multipart formats
             [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]]
           elsif m[4] # Single format
             [m[1], m[2], m[3], m[4]]

actionpack/test/controller/assert_select_test.rb

@@ -17,6 +17,8 @@
   end
 end
 
+ActionMailer::Base.template_root = FIXTURE_LOAD_PATH
+
 class AssertSelectTest < Test::Unit::TestCase
   class AssertSelectController < ActionController::Base
     def response_with=(content)
@@ -69,11 +71,10 @@ def setup
     ActionMailer::Base.deliveries = []
   end
 
-
   def teardown
     ActionMailer::Base.deliveries.clear
   end
-  
+
   def assert_failure(message, &block)
     e = assert_raises(AssertionFailedError, &block)
     assert_match(message, e.message) if Regexp === message
@@ -91,7 +92,6 @@ def test_assert_select
     assert_failure(/Expected at least 1 element matching \"p\", found 0/) { assert_select "p" }
   end
 
-
   def test_equality_true_false
     render_html %Q{<div id="1"></div><div id="2"></div>}
     assert_nothing_raised               { assert_select "div" }
@@ -102,7 +102,6 @@ def test_equality_true_false
     assert_nothing_raised               { assert_select "p", false }
   end
 
-
   def test_equality_string_and_regexp
     render_html %Q{<div id="1">foo</div><div id="2">foo</div>}
     assert_nothing_raised               { assert_select "div", "foo" }
@@ -116,7 +115,6 @@ def test_equality_string_and_regexp
     assert_raises(AssertionFailedError) { assert_select "p", :text=>/foobar/ }
   end
 
-
   def test_equality_of_html
     render_html %Q{<p>\n<em>"This is <strong>not</strong> a big problem,"</em> he said.\n</p>}
     text = "\"This is not a big problem,\" he said."
@@ -135,7 +133,6 @@ def test_equality_of_html
     assert_raises(AssertionFailedError) { assert_select "pre", :html=>text }
   end
 
-
   def test_counts
     render_html %Q{<div id="1">foo</div><div id="2">foo</div>}
     assert_nothing_raised               { assert_select "div", 2 }
@@ -166,7 +163,6 @@ def test_counts
     end
   end
 
-
   def test_substitution_values
     render_html %Q{<div id="1">foo</div><div id="2">foo</div>}
     assert_select "div#?", /\d+/ do |elements|
@@ -181,7 +177,6 @@ def test_substitution_values
     end
   end
 
-  
   def test_nested_assert_select
     render_html %Q{<div id="1">foo</div><div id="2">foo</div>}
     assert_select "div" do |elements|
@@ -200,7 +195,7 @@ def test_nested_assert_select
         assert_select "#3", false
       end
     end
-    
+
     assert_failure(/Expected at least 1 element matching \"#4\", found 0\./) do
       assert_select "div" do
         assert_select "#4"
@@ -208,7 +203,6 @@ def test_nested_assert_select
     end
   end
 
-
   def test_assert_select_text_match
     render_html %Q{<div id="1"><span>foo</span></div><div id="2"><span>bar</span></div>}
     assert_select "div" do
@@ -225,7 +219,6 @@ def test_assert_select_text_match
     end
   end
 
-
   # With single result.
   def test_assert_select_from_rjs_with_single_result
     render_rjs do |page|
@@ -255,19 +248,16 @@ def test_assert_select_from_rjs_with_multiple_results
     end
   end
 
-
   #
   # Test css_select.
   #
 
-
   def test_css_select
     render_html %Q{<div id="1"></div><div id="2"></div>}
     assert 2, css_select("div").size
     assert 0, css_select("p").size
   end
 
-
   def test_nested_css_select
     render_html %Q{<div id="1">foo</div><div id="2">foo</div>}
     assert_select "div#?", /\d+/ do |elements|
@@ -286,7 +276,6 @@ def test_nested_css_select
     end
   end
 
-
   # With one result.
   def test_css_select_from_rjs_with_single_result
     render_rjs do |page|
@@ -309,12 +298,10 @@ def test_css_select_from_rjs_with_multiple_results
     assert_equal 1, css_select("#2").size
   end
 
-
   #
   # Test assert_select_rjs.
   #
 
-
   # Test that we can pick up all statements in the result.
   def test_assert_select_rjs_picks_up_all_statements
     render_rjs do |page|
@@ -381,7 +368,6 @@ def test_assert_select_rjs_with_id
     assert_raises(AssertionFailedError) { assert_select_rjs "test4" }
   end
 
-
   def test_assert_select_rjs_for_replace
     render_rjs do |page|
       page.replace "test1", "<div id=\"1\">foo</div>"
@@ -479,7 +465,7 @@ def test_assert_select_rjs_for_show_ignores_block
       end
     end
   end
-  
+
   # Simple hide
   def test_assert_select_rjs_for_hide
     render_rjs do |page|
@@ -500,7 +486,7 @@ def test_assert_select_rjs_for_hide_ignores_block
       end
     end
   end
-  
+
   # Simple toggle
   def test_assert_select_rjs_for_toggle
     render_rjs do |page|
@@ -521,7 +507,7 @@ def test_assert_select_rjs_for_toggle_ignores_block
       end
     end
   end
-  
+
   # Non-positioned insert.
   def test_assert_select_rjs_for_nonpositioned_insert
     render_rjs do |page|
@@ -568,7 +554,7 @@ def test_assert_select_rjs_for_positioned_insert
       assert_select "div", 4
     end
   end
-  
+
   # Simple selection from a single result.
   def test_nested_assert_select_rjs_with_single_result
     render_rjs do |page|
@@ -600,7 +586,6 @@ def test_nested_assert_select_rjs_with_two_results
     end
   end
 
-
   def test_feed_item_encoded
     render_xml <<-EOF
 <rss version="2.0">
@@ -654,7 +639,6 @@ def test_feed_item_encoded
     end
   end
 
-
   #
   # Test assert_select_email
   #
@@ -670,7 +654,6 @@ def test_assert_select_email
     end
   end
 
-
   protected
     def render_html(html)
       @controller.response_with = html