Refactor ActionView::Resolver

.gitignore

@@ -1,3 +1,4 @@
+.DS_Store
 debug.log
 doc/rdoc
 activemodel/doc
@@ -13,6 +14,7 @@ actionpack/pkg
 activemodel/test/fixtures/fixture_database.sqlite3
 actionmailer/pkg
 activesupport/pkg
+actionpack/test/tmp
 activesupport/test/fixtures/isolation_test
 railties/pkg
 railties/test/500.html

actionmailer/lib/action_mailer/base.rb

@@ -489,7 +489,7 @@ 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?
-          template_root.find_all_by_parts(@template, {}, template_path).each do |template|
+          template_root.find_all(@template, {}, template_path).each do |template|
             @parts << Part.new(
               :content_type => template.mime_type ? template.mime_type.to_s : "text/plain",
               :disposition => "inline",

actionpack/lib/action_controller/legacy/layout.rb

@@ -200,7 +200,7 @@ def find_layout(*args)
       end
 
       def layout_list #:nodoc:
-        Array(view_paths).sum([]) { |path| Dir["#{path.to_str}/layouts/**/*"] }
+        Array(view_paths).sum([]) { |path| Dir["#{path}/layouts/**/*"] }
       end
       memoize :layout_list
 

actionpack/lib/action_view.rb

@@ -40,6 +40,7 @@ def self.load_all!
   autoload :MissingTemplate,   'action_view/base'
   autoload :Partials,          'action_view/render/partials'
   autoload :Resolver,          'action_view/template/resolver'
+  autoload :PathResolver,      'action_view/template/resolver'
   autoload :PathSet,           'action_view/paths'
   autoload :Rendering,         'action_view/render/rendering'
   autoload :Renderable,        'action_view/template/renderable'

actionpack/lib/action_view/template/resolver.rb

@@ -1,9 +1,28 @@
 require "pathname"
+require "active_support/core_ext/class"
 require "action_view/template/template"
 
 module ActionView
   # Abstract superclass
   class Resolver
+
+    class_inheritable_accessor(:registered_details)
+    self.registered_details = {}
+
+    def self.register_detail(name, options = {})
+      registered_details[name] = lambda do |val|
+        val ||= yield
+        val |= [nil] unless options[:allow_nil] == false
+        val
+      end
+    end
+
+    register_detail(:locale)  { [I18n.locale] }
+    register_detail(:formats) { Mime::SET.symbols }
+    register_detail(:handlers, :allow_nil => false) do
+      TemplateHandlers.extensions
+    end
+
     def initialize(options = {})
       @cache  = options[:cache]
       @cached = {}
@@ -11,15 +30,18 @@ def initialize(options = {})
 
     # Normalizes the arguments and passes it on to find_template
     def find(*args)
-      find_all_by_parts(*args).first
+      find_all(*args).first
     end
-    
+
-    def find_all_by_parts(name, details = {}, prefix = nil, partial = nil)
+    def find_all(name, details = {}, prefix = nil, partial = nil)
-      details[:locales] = [I18n.locale]
+      details = normalize_details(details)
-      name = name.to_s.gsub(handler_matcher, '').split("/")
+      name, prefix = normalize_name(name, prefix)
-      find_templates(name.pop, details, [prefix, *name].compact.join("/"), partial)
+
+      cached([name, details, prefix, partial]) do
+        find_templates(name, details, prefix, partial)
+      end
     end
-  
+
   private
 
     # This is what child classes implement. No defaults are needed
@@ -28,29 +50,24 @@ def find_all_by_parts(name, details = {}, prefix = nil, partial = nil)
     def find_templates(name, details, prefix, partial)
       raise NotImplementedError
     end
-    
+
-    def valid_handlers
+    def normalize_details(details)
-      @valid_handlers ||= TemplateHandlers.extensions
+      details = details.dup
+      registered_details.each do |k, v|
+        details[k] = v.call(details[k])
+      end
+      details
     end
 
-    def handler_matcher
+    # Support legacy foo.erb names even though we now ignore .erb
-      @handler_matcher ||= begin
+    # as well as incorrectly putting part of the path in the template
-        e = valid_handlers.join('|')
+    # name instead of the prefix.
-        /\.(?:#{e})$/
+    def normalize_name(name, prefix)
-      end
+      handlers = TemplateHandlers.extensions.join('|')
-    end
+      name = name.to_s.gsub(/\.(?:#{handlers})$/, '')
 
-    def handler_glob
+      parts = name.split('/')
-      @handler_glob ||= begin
+      return parts.pop, [prefix, *parts].compact.join("/")
-        e = TemplateHandlers.extensions.map{|h| ".#{h}"}.join(",")
-        "{#{e}}"
-      end
-    end
-    
-    def formats_glob
-      @formats_glob ||= begin
-        '{' + Mime::SET.symbols.map { |l| ".#{l}," }.join + '}'
-      end
     end
 
     def cached(key)
@@ -60,67 +77,49 @@ def cached(key)
     end
   end
 
-  class FileSystemResolver < Resolver
+  class PathResolver < Resolver
 
-    def self.cached_glob
+    EXTENSION_ORDER = [:locale, :formats, :handlers]
-      @@cached_glob ||= {}
-    end
-
-    def initialize(path, options = {})
-      raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver)
-      super(options)
-      @path = Pathname.new(path).expand_path
-    end
 
     def to_s
       @path.to_s
     end
     alias to_path to_s
 
-    def find_templates(name, details, prefix, partial, root = "#{@path}/")
+    def find_templates(name, details, prefix, partial)
-      if glob = details_to_glob(name, details, prefix, partial, root)
+      path = build_path(name, details, prefix, partial)
-        cached(glob) do
+      query(path, EXTENSION_ORDER.map { |ext| details[ext] })
-          Dir[glob].map do |path|
-            next if File.directory?(path)
-            source = File.read(path)
-            identifier = Pathname.new(path).expand_path.to_s
-
-            Template.new(source, identifier, *path_to_details(path))
-          end.compact
-        end
-      end
     end
-  
+
   private
 
-    # :api: plugin
+    def build_path(name, details, prefix, partial)
-    def details_to_glob(name, details, prefix, partial, root)
+      path = ""
-      self.class.cached_glob[[name, prefix, partial, details, root]] ||= begin
+      path << "#{prefix}/" unless prefix.empty?
-        path = ""
+      path << (partial ? "_#{name}" : name)
-        path << "#{prefix}/" unless prefix.empty?
+      path
-        path << (partial ? "_#{name}" : name)
-
-        extensions = ""
-        [:locales, :formats].each do |k|
-          # TODO: OMG NO
-          if details[k] == [:"*/*"]
-            extensions << formats_glob if k == :formats
-          elsif exts = details[k]
-            extensions << '{' + exts.map {|e| ".#{e},"}.join + '}'
-          else
-            extensions << formats_glob if k == :formats
-          end
-        end
-
-        "#{root}#{path}#{extensions}#{handler_glob}"
-      end
     end
 
-    # TODO: fix me
+    def query(path, exts)
-    # :api: plugin
+      query = "#{@path}/#{path}"
+      exts.each do |ext|
+        query << '{' << ext.map {|e| e && ".#{e}" }.join(',') << '}'
+      end
+
+      Dir[query].map do |path|
+        next if File.directory?(path)
+        source = File.read(path)
+        identifier = Pathname.new(path).expand_path.to_s
+
+        Template.new(source, identifier, *path_to_details(path))
+      end.compact
+    end
+
+    # # TODO: fix me
+    # # :api: plugin
     def path_to_details(path)
       # [:erb, :format => :html, :locale => :en, :partial => true/false]
-      if m = path.match(%r'/(_)?[\w-]+(\.[\w-]+)*\.(\w+)$')
+      if m = path.match(%r'(?:^|/)(_)?[\w-]+(\.[\w-]+)*\.(\w+)$')
         partial = m[1] == '_'
         details = (m[2]||"").split('.').reject { |e| e.empty? }
         handler = Template.handler_class_for_extension(m[3])
@@ -133,13 +132,32 @@ def path_to_details(path)
     end
   end
 
-  class FileSystemResolverWithFallback < FileSystemResolver
+  class FileSystemResolver < PathResolver
+    def initialize(path, options = {})
+      raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver)
+      super(options)
+      @path = Pathname.new(path).expand_path
+    end
+  end
 
-    def find_templates(name, details, prefix, partial)
+  # OMG HAX
-      templates = super
+  # TODO: remove hax
-      return super(name, details, prefix, partial, '') if templates.empty?
+  class FileSystemResolverWithFallback < Resolver
-      templates
+    def initialize(path, options = {})
+      super(options)
+      @paths = [FileSystemResolver.new(path, options), FileSystemResolver.new("", options), FileSystemResolver.new("/", options)]
     end
 
+    def find_templates(*args)
+      @paths.each do |p|
+        template = p.find_templates(*args)
+        return template unless template.empty?
+      end
+      []
+    end
+
+    def to_s
+      @paths.first.to_s
+    end
   end
 end

actionpack/test/lib/fixture_template.rb

@@ -1,70 +1,28 @@
 module ActionView #:nodoc:
-  class FixtureResolver < Resolver
+  class FixtureResolver < PathResolver
     def initialize(hash = {}, options = {})
       super(options)
       @hash = hash
     end
 
-    def find_templates(name, details, prefix, partial)
-      if regexp = details_to_regexp(name, details, prefix, partial)
-        cached(regexp) do
-          templates = []
-          @hash.select { |k,v| k =~ regexp }.each do |path, source|
-            templates << Template.new(source, path, *path_to_details(path))
-          end
-          templates.sort_by {|t| -t.details.values.compact.size }
-        end
-      end
-    end
-
   private
 
-    def formats_regexp
+    def or_extensions(array)
-      @formats_regexp ||= begin
+      "(?:" << array.map {|e| e && Regexp.escape(".#{e}")}.join("|") << ")"
-        formats = Mime::SET.symbols
-        '(?:' + formats.map { |l| "\\.#{Regexp.escape(l.to_s)}" }.join('|') + ')?'
-      end
     end
 
-    def handler_regexp
+    def query(path, exts)
-      e = TemplateHandlers.extensions.map{|h| "\\.#{Regexp.escape(h.to_s)}"}.join("|")
+      query = Regexp.escape(path)
-      "(?:#{e})"
+      exts.each do |ext|
-    end
+        query << '(?:' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << ')'
-
-    def details_to_regexp(name, details, prefix, partial)
-      path = ""
-      path << "#{prefix}/" unless prefix.empty?
-      path << (partial ? "_#{name}" : name)
-
-      extensions = ""
-      [:locales, :formats].each do |k|
-        # TODO: OMG NO
-        if details[k] == [:"*/*"]
-          extensions << formats_regexp if k == :formats
-        elsif exts = details[k]
-          extensions << '(?:' + exts.map {|e| "\\.#{Regexp.escape(e.to_s)}"}.join('|') + ')?'
-        else
-          extensions << formats_regexp if k == :formats
-        end
       end
 
-      %r'^#{Regexp.escape(path)}#{extensions}#{handler_regexp}$'
+      templates = []
-    end
+      @hash.select { |k,v| k =~ /^#{query}$/ }.each do |path, source|
-
+        templates << Template.new(source, path, *path_to_details(path))
-    # TODO: fix me
-    # :api: plugin
-    def path_to_details(path)
-      # [:erb, :format => :html, :locale => :en, :partial => true/false]
-      if m = path.match(%r'(_)?[\w-]+((?:\.[\w-]+)*)\.(\w+)$')
-        partial = m[1] == '_'
-        details = (m[2]||"").split('.').reject { |e| e.empty? }
-        handler = Template.handler_class_for_extension(m[3])
-
-        format  = Mime[details.last] && details.pop.to_sym
-        locale  = details.last && details.pop.to_sym
-
-        return handler, :format => format, :locale => locale, :partial => partial
       end
+      templates.sort_by {|t| -t.details.values.compact.size }
     end
+
   end
 end

actionpack/test/new_base/render_file_test.rb

@@ -3,7 +3,7 @@
 module RenderFile
   
   class BasicController < ActionController::Base
-    self.view_paths = "."
+      self.view_paths = File.dirname(__FILE__)
     
     def index
       render :file => File.join(File.dirname(__FILE__), *%w[.. fixtures test hello_world])

actionpack/test/template/compiled_templates_test.rb

@@ -14,6 +14,9 @@ def test_template_gets_recompiled_when_using_different_keys_in_local_assigns
     assert_equal "two", render(:file => "test/render_file_with_locals_and_default.erb", :locals => { :secret => "two" })
   end
 
+  # This is broken in 1.8.6 (not supported in Rails 3.0) because the cache uses a Hash
+  # key. Since Ruby 1.8.6 implements Hash#hash using the hash's object_id, it will never
+  # successfully get a cache hit here.
   def test_template_changes_are_not_reflected_with_cached_templates
     assert_equal "Hello world!", render(:file => "test/hello_world.erb")
     modify_template "test/hello_world.erb", "Goodbye world!" do