Fix template extension parsing. [#2315 state:resolved] [#2284 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>

2 files changed, 46 insertions, 30 deletions

diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb
index 4497c4ac32..a974f2652b 100644
--- a/actionpack/lib/action_view/template.rb
+++ b/actionpack/lib/action_view/template.rb
@@ -217,46 +217,30 @@ module ActionView #:nodoc:
       end
 
       def valid_locale?(locale)
-        I18n.available_locales.include?(locale.to_sym)
+        locale && I18n.available_locales.include?(locale.to_sym)
       end
 
       # Returns file split into an array
       #   [base_path, name, locale, format, extension]
       def split(file)
         if m = file.to_s.match(/^(.*\/)?([^\.]+)\.(.*)$/)
-          base_path = m[1]
-          name = m[2]
-          extensions = m[3]
-        else
-          return
+          [m[1], m[2], *parse_extensions(m[3])]
         end
+      end
 
-        locale = nil
-        format = nil
-        extension = nil
-
-        if m = extensions.split(".")
-          if valid_locale?(m[0]) && m[1] && valid_extension?(m[2]) # All three
-            locale = m[0]
-            format = m[1]
-            extension = m[2]
-          elsif m[0] && m[1] && valid_extension?(m[2]) # Multipart formats
-            format = "#{m[0]}.#{m[1]}"
-            extension = m[2]
-          elsif valid_locale?(m[0]) && valid_extension?(m[1]) # locale and extension
-            locale = m[0]
-            extension = m[1]
-          elsif valid_extension?(m[1]) # format and extension
-            format = m[0]
-            extension = m[1]
-          elsif valid_extension?(m[0]) # Just extension
-            extension = m[0]
-          else # No extension
-            format = m[0]
-          end
+      # returns parsed extensions as an array
+      #   [locale, format, extension]
+      def parse_extensions(extensions)
+        exts = extensions.split(".")
+
+        if extension = valid_extension?(exts.last) && exts.pop || nil
+          locale = valid_locale?(exts.first) && exts.shift || nil
+          format = exts.join('.') if exts.any? # join('.') is needed for multipart templates
+        else # no extension, just format
+          format = exts.last
         end
 
-        [base_path, name, locale, format, extension]
+        [locale, format, extension]
       end
   end
 end
diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb
new file mode 100644
index 0000000000..7caec7ad9f
--- /dev/null
+++ b/actionpack/test/template/template_test.rb
@@ -0,0 +1,32 @@
+require 'abstract_unit'
+
+class TemplateTest < Test::Unit::TestCase
+  def test_template_path_parsing
+    with_options :base_path => nil, :name => 'abc', :locale => nil, :format => 'html', :extension => 'erb' do |t|
+      t.assert_parses_template_path 'abc.en.html.erb',        :locale => 'en'
+      t.assert_parses_template_path 'abc.en.plain.html.erb',  :locale => 'en', :format => 'plain.html'
+      t.assert_parses_template_path 'abc.html.erb'
+      t.assert_parses_template_path 'abc.plain.html.erb',     :format => 'plain.html'
+      t.assert_parses_template_path 'abc.erb',                :format => nil
+      t.assert_parses_template_path 'abc.html',               :extension => nil
+      
+      t.assert_parses_template_path '_abc.html.erb',          :name => '_abc'
+      
+      t.assert_parses_template_path 'test/abc.html.erb',      :base_path => 'test'
+      t.assert_parses_template_path './test/abc.html.erb',    :base_path => './test'
+      t.assert_parses_template_path '../test/abc.html.erb',   :base_path => '../test'
+      
+      t.assert_parses_template_path 'abc',                    :extension => nil, :format => nil, :name => nil
+      t.assert_parses_template_path 'abc.xxx',                :extension => nil, :format => 'xxx', :name => 'abc'
+      t.assert_parses_template_path 'abc.html.xxx',           :extension => nil, :format => 'xxx', :name => 'abc'
+    end
+  end
+
+  private
+  def assert_parses_template_path(path, parse_results)
+    template = ActionView::Template.new(path, '')
+    parse_results.each_pair do |k, v|
+      assert_block(%Q{Expected template to parse #{k.inspect} from "#{path}" as #{v.inspect}, but got #{template.send(k).inspect}}) { v == template.send(k) }
+    end
+  end
+end