Refactor ActionView::Path

* Decouple from ActionController and ActionMailer
  * Bring back localization support.
  * Prepare to decouple templates from the filesystem.
  * Prepare to decouple localization from ActionView
  * Fix ActionMailer to take advantage of ActionView::Path

actionmailer/lib/action_mailer/base.rb

@@ -465,48 +465,48 @@ module ActionMailer #:nodoc:
     def create!(method_name, *parameters) #:nodoc:
       initialize_defaults(method_name)
       __send__(method_name, *parameters)
-
+      
       # If an explicit, textual body has not been set, we check assumptions.
       unless String === @body
         # First, we look to see if there are any likely templates that match,
         # which include the content-type in their file name (i.e.,
         # "the_template_file.text.html.erb", etc.). Only do this if parts
         # have not already been specified manually.
-        if @parts.empty?
-          Dir.glob("#{template_path}/#{@template}.*").each do |path|
-            template = template_root.find_by_parts("#{mailer_name}/#{File.basename(path)}")
-
-            # Skip unless template has a multipart format
-            next unless template && template.multipart?
-
+        # if @parts.empty?
+          template_root.find_all_by_parts(@template, {}, template_path).each do |template|
             @parts << Part.new(
-              :content_type => template.content_type,
+              :content_type => Mime::Type.lookup_by_extension(template.content_type || "text").to_s,
               :disposition => "inline",
               :charset => charset,
               :body => render_template(template, @body)
             )
           end
-          unless @parts.empty?
+          
+          if @parts.size > 1
             @content_type = "multipart/alternative" if @content_type !~ /^multipart/
             @parts = sort_parts(@parts, @implicit_parts_order)
           end
-        end
-
+        # end
+        
         # Then, if there were such templates, we check to see if we ought to
         # 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 ||= template_root.find_by_parts("#{mailer_name}/#{@template}")
-        @body = render_message(@template, @body) if template_exists
+        # ====
+        # TODO: Revisit this
+        # template_exists = @parts.empty?
+        # template_exists ||= template_root.find_by_parts("#{mailer_name}/#{@template}")
+        # @body = render_message(@template, @body) if template_exists
 
         # 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
         # that create_mail doesn't try to render it in addition to the parts).
-        if !@parts.empty? && String === @body
-          @parts.unshift Part.new(:charset => charset, :body => @body)
-          @body = nil
-        end
+        # ====
+        # TODO: Revisit this
+        # if !@parts.empty? && String === @body
+        #   @parts.unshift Part.new(:charset => charset, :body => @body)
+        #   @body = nil
+        # end
       end
 
       # If this is a multipart e-mail add the mime_version if it is not
@@ -580,7 +580,7 @@ module ActionMailer #:nodoc:
           
           if file
             prefix = mailer_name unless file =~ /\//
-            template = view_paths.find_by_parts(file, formats, prefix)
+            template = view_paths.find_by_parts(file, {:formats => formats}, prefix)
           end
 
           layout = _pick_layout(layout, 
@@ -611,7 +611,7 @@ module ActionMailer #:nodoc:
       end
 
       def template_path
-        "#{template_root}/#{mailer_name}"
+        "#{mailer_name}"
       end
 
       def initialize_template_class(assigns)
@@ -622,7 +622,7 @@ module ActionMailer #:nodoc:
 
       def sort_parts(parts, order = [])
         order = order.collect { |s| s.downcase }
-
+        
         parts = parts.sort do |a, b|
           a_ct = a.content_type.downcase
           b_ct = b.content_type.downcase
@@ -663,10 +663,13 @@ module ActionMailer #:nodoc:
         headers.each { |k, v| m[k] = v }
 
         real_content_type, ctype_attrs = parse_content_type
-
+        
         if @parts.empty?
           m.set_content_type(real_content_type, nil, ctype_attrs)
           m.body = normalize_new_lines(body)
+        elsif @parts.size == 1 && @parts.first.parts.empty?
+          m.set_content_type(real_content_type, nil, ctype_attrs)
+          m.body = normalize_new_lines(@parts.first.body)
         else
           if String === body
             part = TMail::Mail.new

actionmailer/lib/action_mailer/vendor/tmail-1.2.3/tmail/mail.rb

@@ -518,6 +518,7 @@ module TMail
 
     def parse_body( f = nil )
       return if @body_parsed
+      
       if f
         parse_body_0 f
       else

actionmailer/test/fixtures/test_mailer/rxml_template.builder

@@ -1,2 +0,0 @@
-xml.instruct!
-xml.test

actionmailer/test/mail_service_test.rb

@@ -338,6 +338,7 @@ class ActionMailerTest < Test::Unit::TestCase
 
   def test_nested_parts_with_body
     created = nil
+    TestMailer.create_nested_multipart_with_body(@recipient)
     assert_nothing_raised { created = TestMailer.create_nested_multipart_with_body(@recipient)}
     assert_equal 1,created.parts.size
     assert_equal 2,created.parts.first.parts.size
@@ -351,8 +352,8 @@ class ActionMailerTest < Test::Unit::TestCase
 
   def test_attachment_with_custom_header
     created = nil
-    assert_nothing_raised { created = TestMailer.create_attachment_with_custom_header(@recipient)}
-    assert_equal "<test@test.com>", created.parts[1].header['content-id'].to_s
+    assert_nothing_raised { created = TestMailer.create_attachment_with_custom_header(@recipient) }
+    assert created.parts.any? { |p| p.header['content-id'].to_s == "<test@test.com>" }
   end
 
   def test_signed_up
@@ -824,7 +825,7 @@ EOF
     assert_equal 3, mail.parts.length
     assert_equal "1.0", mail.mime_version
     assert_equal "multipart/alternative", mail.content_type
-    assert_equal "text/yaml", mail.parts[0].content_type
+    assert_equal "application/x-yaml", mail.parts[0].content_type
     assert_equal "utf-8", mail.parts[0].sub_header("content-type", "charset")
     assert_equal "text/plain", mail.parts[1].content_type
     assert_equal "utf-8", mail.parts[1].sub_header("content-type", "charset")
@@ -835,11 +836,11 @@ EOF
   def test_implicitly_multipart_messages_with_custom_order
     assert ActionView::Template.template_handler_extensions.include?("bak"), "bak extension was not registered"
 
-    mail = TestMailer.create_implicitly_multipart_example(@recipient, nil, ["text/yaml", "text/plain"])
+    mail = TestMailer.create_implicitly_multipart_example(@recipient, nil, ["application/x-yaml", "text/plain"])
     assert_equal 3, mail.parts.length
     assert_equal "text/html", mail.parts[0].content_type
     assert_equal "text/plain", mail.parts[1].content_type
-    assert_equal "text/yaml", mail.parts[2].content_type
+    assert_equal "application/x-yaml", mail.parts[2].content_type
   end
 
   def test_implicitly_multipart_messages_with_charset
@@ -938,8 +939,7 @@ EOF
   def test_multipart_with_template_path_with_dots
     mail = FunkyPathMailer.create_multipart_with_template_path_with_dots(@recipient)
     assert_equal 2, mail.parts.length
-    assert_equal 'text/plain', mail.parts[0].content_type
-    assert_equal 'utf-8', mail.parts[0].charset
+    assert mail.parts.any? {|part| part.content_type == "text/plain" && part.charset == "utf-8"}
   end
 
   def test_custom_content_type_attributes

actionpack/lib/action_controller/abstract/layouts.rb

@@ -38,7 +38,7 @@ module AbstractController
         else
           self.class_eval %{
             def _layout
-              if view_paths.find_by_parts?("#{_implied_layout_name}", formats, "layouts")
+              if view_paths.find_by_parts?("#{_implied_layout_name}", {:formats => formats}, "layouts")
                 "#{_implied_layout_name}"
               else
                 super
@@ -62,7 +62,7 @@ module AbstractController
         raise ArgumentError, "String, false, or nil expected; you passed #{name.inspect}"
       end
       
-      name && view_paths.find_by_parts(name, formats, "layouts")
+      name && view_paths.find_by_parts(name, {:formats => formats}, "layouts")
     end
     
     def _default_layout(require_layout = false)

actionpack/lib/action_controller/abstract/renderer.rb

@@ -29,7 +29,7 @@ module AbstractController
     def render_to_body(options = {})
       name = options[:_template_name] || action_name
       
-      template = options[:_template] || view_paths.find_by_parts(name.to_s, formats, options[:_prefix])
+      template = options[:_template] || view_paths.find_by_parts(name.to_s, {:formats => formats}, options[:_prefix])
       _render_template(template, options)
     end
 

actionpack/lib/action_controller/base/layout.rb

@@ -182,7 +182,7 @@ module ActionController #:nodoc:
       def memoized_find_layout(layout, formats) #:nodoc:
         return layout if layout.nil? || layout.respond_to?(:render)
         prefix = layout.to_s =~ /layouts\// ? nil : "layouts"
-        view_paths.find_by_parts(layout.to_s, formats, prefix)
+        view_paths.find_by_parts(layout.to_s, {:formats => formats}, prefix)
       end
 
       def find_layout(*args)

actionpack/lib/action_controller/base/render.rb

@@ -374,8 +374,13 @@ module ActionController
         render_for_file(name.sub(/^\//, ''), [layout, true], options)
       end
     end
-  
+
+    # ==== Arguments
+    # parts<Array[String, Array[Symbol*], String, Boolean]>::
+    #     Example: ["show", [:html, :xml], "users", false]
     def render_for_parts(parts, layout, options = {})
+      parts[1] = {:formats => parts[1], :locales => [I18n.locale]}
+      
       tmp = view_paths.find_by_parts(*parts)
       
       layout = _pick_layout(*layout) unless tmp.exempt_from_layout?

actionpack/lib/action_view/paths.rb

@@ -33,18 +33,18 @@ module ActionView #:nodoc:
       super(*objs.map { |obj| self.class.type_cast(obj) })
     end
 
-    def find_by_parts(path, extension = nil, prefix = nil, partial = false)
+    def find_by_parts(path, details = {}, prefix = nil, partial = false)
       template_path = path.sub(/^\//, '')
 
       each do |load_path|
-        if template = load_path.find_by_parts(template_path, extension, prefix, partial)
+        if template = load_path.find_by_parts(template_path, details, prefix, partial)
           return template
         end
       end
 
       Template.new(path, self)
     rescue ActionView::MissingTemplate => e
-      extension ||= []
+      extension = details[:formats] || []
       raise ActionView::MissingTemplate.new(self, "#{prefix}/#{path}.{#{extension.join(",")}}")
     end
     

actionpack/lib/action_view/render/partials.rb

@@ -187,6 +187,7 @@ module ActionView
           path = ActionController::RecordIdentifier.partial_path(object, controller_path)
         end
         _, _, prefix, object = parts = partial_parts(path, options)
+        parts[1] = {:formats => parts[1]}
         template = find_by_parts(*parts)
         _render_partial_object(template, options, (object unless object == true))
       end
@@ -225,7 +226,7 @@ module ActionView
       def _render_partial_with_layout(layout, options)
         if layout
           prefix = controller && !layout.include?("/") ? controller.controller_path : nil
-          layout = find_by_parts(layout, formats, prefix, true)
+          layout = find_by_parts(layout, {:formats => formats}, prefix, true)
         end
         content = _render_partial(options)
         return _render_content_with_layout(content, layout, options[:locals])
@@ -253,7 +254,7 @@ module ActionView
       def _render_partial_with_layout(layout, options)
         if layout
           prefix = controller && !layout.include?("/") ? controller.controller_path : nil
-          layout = find_by_parts(layout, formats, prefix, true)
+          layout = find_by_parts(layout, {:formats => formats}, prefix, true)
         end
         content = _render_partial(options)
         return _render_content_with_layout(content, layout, options[:locals])
@@ -321,7 +322,7 @@ module ActionView
 
       def _pick_partial_template(partial_path) #:nodoc:
         prefix = controller_path unless partial_path.include?('/')
-        find_by_parts(partial_path, formats, prefix, true)
+        find_by_parts(partial_path, {:formats => formats}, prefix, true)
       end
       memoize :_pick_partial_template
   end

actionpack/lib/action_view/render/rendering.rb

@@ -23,10 +23,10 @@ module ActionView
         return _render_partial_with_layout(layout, options) if options.key?(:partial)
         return _render_partial_with_block(layout, block, options) if block_given?
     
-        layout = find_by_parts(layout, formats) if layout
+        layout = find_by_parts(layout, {:formats => formats}) if layout
     
         if file = options[:file]
-          template = find_by_parts(file, formats)
+          template = find_by_parts(file, {:formats => formats})
           _render_template_with_layout(template, layout, :locals => options[:locals])
         elsif inline = options[:inline]
           _render_inline(inline, layout, options)

actionpack/lib/action_view/template/handlers.rb

@@ -16,6 +16,10 @@ module ActionView #:nodoc:
 
     @@template_handlers = {}
     @@default_template_handlers = nil
+    
+    def self.extensions
+      @@template_handlers.keys
+    end
 
     # Register a class that knows how to handle template files with the given
     # extension. This can be used to implement new template types.

actionpack/lib/action_view/template/path.rb

@@ -1,13 +1,81 @@
 module ActionView
   class Template
+  # Abstract super class
     class Path
-      attr_reader :path, :paths
-      delegate :hash, :inspect, :to => :path
-      
       def initialize(options)
-        @cache = options[:cache]
+        @cache  = options[:cache]
+        @cached = {}
       end
-
+    
+      # Normalizes the arguments and passes it on to find_template
+      def find_by_parts(*args)
+        find_all_by_parts(*args).first
+      end
+      
+      def find_all_by_parts(name, details = {}, prefix = nil, partial = nil)
+        details[:locales] = [I18n.locale]
+        name = name.to_s.gsub(handler_matcher, '').split("/")
+        find_templates(name.pop, details, [prefix, *name].compact.join("/"), partial)
+      end
+    
+    private
+      
+      # This is what child classes implement. No defaults are needed
+      # because Path guarentees that the arguments are present and
+      # normalized.
+      def find_templates(name, details, prefix, partial)
+        raise NotImplementedError
+      end
+  
+      # TODO: Refactor this to abstract out the file system
+      def initialize_template(file)
+        t = Template.new(file.split("#{self}/").last, self)
+        t.load!
+        t
+      end
+      
+      def valid_handlers
+        @valid_handlers ||= TemplateHandlers.extensions
+      end
+      
+      def handler_matcher
+        @handler_matcher ||= begin
+          e = valid_handlers.join('|')
+          /\.(?:#{e})$/
+        end
+      end
+      
+      def handler_glob
+        e = TemplateHandlers.extensions.join(',')
+        ".{#{e}}"
+      end
+      
+      def formats_glob
+        @formats_glob ||= begin
+          formats = Mime::SET.map { |m| m.symbol }
+          '{' + formats.map { |l| ".#{l}," }.join + '}'
+        end
+      end
+      
+      def cached(key)
+        return yield unless @cache
+        return @cached[key] if @cached.key?(key)
+        @cached[key] = yield
+      end
+    end
+  
+    class FileSystemPath < Path
+    
+      def initialize(path, options = {})
+        raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
+        super(options)
+        @path = path
+      end
+    
+      # TODO: This is the currently needed API. Make this suck less
+      # ==== <suck>
+      attr_reader :path
+    
       def to_s
         if defined?(RAILS_ROOT)
           path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '')
@@ -27,61 +95,36 @@ module ActionView
       def eql?(path)
         to_str == path.to_str
       end
-
-      def find_by_parts(name, extensions = nil, prefix = nil, partial = nil)
-        path = prefix ? "#{prefix}/" : ""
+      # ==== </suck>
     
-        name = name.to_s.split("/")
-        name[-1] = "_#{name[-1]}" if partial
-    
-        path << name.join("/")
-
-        template = nil
-
-        Array(extensions).each do |extension|
-          extensioned_path = extension ? "#{path}.#{extension}" : path
-          break if (template = find_template(extensioned_path))
+      def find_templates(name, details, prefix, partial)
+        if glob = parts_to_glob(name, details, prefix, partial)
+          cached(glob) do
+            Dir[glob].map do |path|
+              initialize_template(path) unless File.directory?(path)
+            end.compact
+          end
         end
-        template || find_template(path)
       end
+    
+    private
   
-    private
-      def create_template(file)
-        Template.new(file.split("#{self}/").last, self)
-      end
-    end
-
-    class FileSystemPath < Path
-      def initialize(path, options = {})
-        raise ArgumentError, "path already is a Path class" if path.is_a?(Path)        
-        
-        super(options)
-        @path, @paths = path, {}
-        
-        # **/*/** is a hax for symlinked directories
-        load_templates("#{@path}/{**/*,**}/**") if @cache
-      end
-
-    private
-    
-      def load_template(template)
-        template.load!
-        template.accessible_paths.each do |path|
-          @paths[path] = template
-        end
-      end
-    
-      def find_template(path)
-        load_templates("#{@path}/#{path}{,.*}") unless @cache
-        @paths[path]
-      end    
-    
-      def load_templates(glob)
-        Dir[glob].each do |file|
-          load_template(create_template(file)) unless File.directory?(file)
-        end
-      end
+      def parts_to_glob(name, details, prefix, partial)
+        path = ""
+        path << "#{prefix}/" unless prefix.empty?
+        path << (partial ? "_#{name}" : name)
       
+        extensions = ""
+        [:locales, :formats].each do |k|
+          extensions << if exts = details[k]
+            '{' + exts.map {|e| ".#{e},"}.join + '}'
+          else
+            k == :formats ? formats_glob : ''
+          end
+        end
+        
+        "#{@path}/#{path}#{extensions}#{handler_glob}"
+      end
     end
   end
 end

actionpack/lib/action_view/template/template.rb

@@ -101,7 +101,7 @@ module ActionView #:nodoc:
     end    
     
     def content_type
-      format.gsub('.', '/')
+      format && format.gsub('.', '/')
     end    
     
   private

actionpack/test/abstract_controller/abstract_controller_test.rb

@@ -139,10 +139,10 @@ module AbstractController
       private
       def self.layout(formats)
         begin
-          view_paths.find_by_parts(name.underscore, formats, "layouts")
+          view_paths.find_by_parts(name.underscore, {:formats => formats}t, "layouts")
         rescue ActionView::MissingTemplate
           begin
-            view_paths.find_by_parts("application", formats, "layouts")
+            view_paths.find_by_parts("application", {:formats => formats}, "layouts")
           rescue ActionView::MissingTemplate
           end
         end

actionpack/test/template/render_test.rb

@@ -29,23 +29,19 @@ module RenderTestCases
   end
 
   def test_render_file_with_localization
-    pending do
-      begin
-        old_locale = I18n.locale
-        I18n.locale = :da
-        assert_equal "Hey verden", @view.render(:file => "test/hello_world")
-      ensure
-        I18n.locale = old_locale
-      end
+    begin
+      old_locale = I18n.locale
+      I18n.locale = :da
+      assert_equal "Hey verden", @view.render(:file => "test/hello_world")
+    ensure
+      I18n.locale = old_locale
     end
   end
 
   def test_render_file_with_dashed_locale
     old_locale = I18n.locale
-    pending do
-      I18n.locale = :"pt-BR"
-      assert_equal "Ola mundo", @view.render(:file => "test/hello_world")
-    end
+    I18n.locale = :"pt-BR"
+    assert_equal "Ola mundo", @view.render(:file => "test/hello_world")
   ensure
     I18n.locale = old_locale
   end