Ticket #10800: refactor_action_view_io_caching.2.patch

a/actionpack/lib/action_controller/base.rb

@@ -5 +5 @@
-require 'action_controller/resources'
-require 'action_controller/url_rewriter'
-require 'action_controller/status_codes'
+require 'action_view/template_finder'
-require 'drb'
-require 'set'
-
@@ -428 +429 @@
-
-      def view_paths=(value)
-        @view_paths = value
+        ActionView::TemplateFinder.process_view_paths(value)
-      end
-
-      # Adds a view_path to the front of the view_paths array.
@@ -440 +442 @@
-      def prepend_view_path(path)
-        @view_paths = superclass.view_paths.dup if @view_paths.nil?
-        view_paths.unshift(*path)
+        ActionView::TemplateFinder.process_view_paths(path)
-      end
-      
-      # Adds a view_path to the end of the view_paths array.
@@ -452 +455 @@
-      def append_view_path(path)
-        @view_paths = superclass.view_paths.dup if @view_paths.nil?
-        view_paths.push(*path)
+        ActionView::TemplateFinder.process_view_paths(path)
-      end
-      
-      # Replace sensitive parameter data from the request log.
@@ -642 +646 @@
-      
-      # View load paths for controller.
-      def view_paths
-
+finder.
-      end
-    
-      def view_paths=(value)
-view_paths = value
+finder.view_paths = value  # Mutex needed
-      end
-
-      # Adds a view_path to the front of the view_paths array.
@@ -656 +660 @@
-      #   self.prepend_view_path(["views/default", "views/custom"])
-      #
-      def prepend_view_path(path)
-prepend_view_path(path)
+finder.prepend_view_path(path)  # Mutex needed
-      end
-      
-      # Adds a view_path to the end of the view_paths array.
@@ -666 +670 @@
-      #   self.append_view_path(["views/default", "views/custom"])
-      #
-      def append_view_path(path)
-@template.append_view_path(path)
+  @template.finder.append_view_path(path)  # Mutex needed
-      end
-
-    protected
@@ -1249 +1253 @@
-      end
-
-      def template_exists?(template_name = default_template_name)
-
+nder.fi
-      end
-
-      def template_public?(template_name = default_template_name)
@@ -1257 +1261 @@
-      end
-
-      def template_exempt_from_layout?(template_name = default_template_name)
-
+finder.
-        name_with_extension = !template_name.include?('.') && extension ? "#{template_name}.#{extension}" : template_name
-        @@exempt_from_layout.any? { |ext| name_with_extension =~ ext }
-      end

a/actionpack/lib/action_controller/dispatcher.rb

@@ -153 +153 @@
-
-      if unprepared || force
-        run_callbacks :prepare
+        ActionView::TemplateFinder.reload! unless ActionView::Base.cache_template_loading
-        self.unprepared = false
-      end
-    end

a/actionpack/lib/action_controller/layout.rb

@@ -208 +208 @@
-          conditions.inject({}) {|hash, (key, value)| hash.merge(key => [value].flatten.map {|action| action.to_s})}
-        end
-        
-        def layout_directory_exists_cache
-          @@layout_directory_exists_cache ||= Hash.new do |h, dirname|
-            h[dirname] = File.directory? dirname
-          end
-        end
-        
-        def default_layout_with_format(format, layout)
-          list = layout_list
-          if list.grep(%r{layouts/#{layout}\.#{format}(\.[a-z][0-9a-z]*)+$}).empty?
@@ -313 +307 @@
-        end
-      end
-      
-      # Does a layout directory for this class exist?
-      # we cache this info in a class level hash
-      def layout_directory?(layout_name)
-
-
-
-
+
-      end
-  end
-end

a/actionpack/lib/action_view.rb

@@ -26 +26 @@
-require 'action_view/template_handlers/erb'
-require 'action_view/template_handlers/rjs'
-
+require 'action_view/template_finder'
+
-require 'action_view/base'
-require 'action_view/partials'
-require 'action_view/template_error'

a/actionpack/lib/action_view/base.rb

@@ -150 +150 @@
-  class Base
-    include ERB::Util
-
-
+, :finder
-    attr_accessor :base_path, :assigns, :template_extension
-, :view_paths
+
-
-    attr_reader :logger, :response, :headers
-    attr_internal :cookies, :flash, :headers, :params, :request, :response, :session
@@ -204 +204 @@
-    @@template_args = {}
-    # Count the number of inline templates
-    @@inline_template_count = 0
-    # Maps template paths without extension to their file extension returned by pick_template_extension.
-    # If for a given path, path.ext1 and path.ext2 exist on the file system, the order of extensions
-    # used by pick_template_extension determines whether ext1 or ext2 will be stored.
-    @@cached_template_extension = {}
-    # Maps template paths / extensions to 
-    @@cached_base_paths = {}
-
-    # Cache public asset paths
-    cattr_reader :computed_public_paths
@@ -241 +235 @@
-    # return the rendered template as a string.
-    def self.register_template_handler(extension, klass)
-      @@template_handlers[extension.to_sym] = klass
+      TemplateFinder.update_extension_cache_for(extension.to_s)
-    end
-
-    def self.template_handler_extensions
-_extensions ||= @@template_handler
+
-    end
-
-    def self.register_default_template_handler(extension, klass)
@@ -265 +260 @@
-    register_template_handler :rxml, TemplateHandlers::Builder
-
-    def initialize(view_paths = [], assigns_for_first_render = {}, controller = nil)#:nodoc:
-      @view_paths = view_paths.respond_to?(:find) ? view_paths.dup : [*view_paths].compact
-      @assigns = assigns_for_first_render
-      @assigns_added = nil
-      @controller = controller
-
+
+
-    end
-
-    # Renders the template present at <tt>template_path</tt>. If <tt>use_full_path</tt> is set to true, 
@@ -290 +285 @@
-      end
-      
-      @first_render ||= template_path
-
+@finder.
-      if use_full_path
-        if template_extension
-full_template_path
+@finder.template_exists?
-        else
-
+@finder.
-          unless template_extension
-
+@finder.
-          end
-full_template_path
+@finder.template_exists?
-          template_extension = template_extension.gsub(/^.+\./, '') # strip off any formats
-        end
-      else
@@ -309 +304 @@
-      template_source = nil # Don't read the source until we know that it is required
-
-      if template_file_name.blank?
-
+@finder.
-      end
-
-      begin
@@ -319 +314 @@
-          e.sub_template_of(template_file_name)
-          raise e
-        else
-
+
+
-        end
-      end
-    end
@@ -371 +367 @@
-      end
-    end
-
-    # Gets the full template path with base path for the given template_path and extension.
-    #
-    #   full_template_path('users/show', 'html.erb')
-    #   # => '~/rails/app/views/users/show.html.erb
-    #
-    def full_template_path(template_path, extension)
-      if @@cache_template_extensions
-        (@@cached_base_paths[template_path] ||= {})[extension.to_s] ||= find_full_template_path(template_path, extension)
-      else
-        find_full_template_path(template_path, extension)
-      end
-    end
-
-    # Gets the extension for an existing template with the given template_path.
-    # Returns the format with the extension if that template exists.
-    #
-    #   pick_template_extension('users/show')
-    #   # => 'html.erb'
-    #
-    #   pick_template_extension('users/legacy')
-    #   # => "rhtml"
-    #
-    def pick_template_extension(template_path)#:nodoc:
-      if @@cache_template_extensions
-        (@@cached_template_extension[template_path] ||= {})[template_format] ||= find_template_extension_for(template_path)
-      else
-        find_template_extension_for(template_path)
-      end
-    end
-
-    def file_exists?(template_path)#:nodoc:
-      template_file_name, template_file_extension = path_and_extension(template_path)
-      if template_file_extension
-        template_exists?(template_file_name, template_file_extension)
-      else
-        template_exists?(template_file_name, pick_template_extension(template_path))
-      end
-    end
-
-    # Returns true is the file may be rendered implicitly.
-    def file_public?(template_path)#:nodoc:
-      template_path.split('/').last[0,1] != '_'
@@ -422 +379 @@
-      @template_format = format.blank? ? :html : format.to_sym
-    end
-
-    # Adds a view_path to the front of the view_paths array.
-    # This change affects the current request only.
-    #
-    #   @template.prepend_view_path("views/default")
-    #   @template.prepend_view_path(["views/default", "views/custom"])
-    #
-    def prepend_view_path(path)
-      @view_paths.unshift(*path)
-    end
-    
-    # Adds a view_path to the end of the view_paths array.
-    # This change affects the current request only.
-    #
-    #   @template.append_view_path("views/default")
-    #   @template.append_view_path(["views/default", "views/custom"])
-    #
-    def append_view_path(path)
-      @view_paths.push(*path)
-    end
-
-    private
-      def wrap_content_for_layout(content)
-        original_content_for_layout = @content_for_layout
-        @content_for_layout = content
-        returning(yield) { @content_for_layout = original_content_for_layout }
-      end
-  
-      def find_full_template_path(template_path, extension)
-        file_name = "#{template_path}.#{extension}"
-        base_path = find_base_path_for(file_name)
-        base_path.blank? ? "" : "#{base_path}/#{file_name}"
-      end
-
-      # Asserts the existence of a template.
-      def template_exists?(template_path, extension)
-        file_path = full_template_path(template_path, extension)
-        !file_path.blank? && @@method_names.has_key?(file_path) || File.exist?(file_path)
-      end
-
-      # Splits the path and extension from the given template_path and returns as an array.
-      def path_and_extension(template_path)
-        template_path_without_extension = template_path.sub(/\.(\w+)$/, '')
-        [ template_path_without_extension, $1 ]
-      end
-      
-      # Returns the view path that contains the given relative template path.
-      def find_base_path_for(template_file_name)
-        view_paths.find { |p| File.file?(File.join(p, template_file_name)) }
-      end
-
-      # Returns the view path that the full path resides in.
-      def extract_base_path_from(full_path)
-        view_paths.find { |p| full_path[0..p.size - 1] == p }
-      end
-
-      # Determines the template's file extension, such as rhtml, rxml, or rjs.
-      def find_template_extension_for(template_path)
-        find_template_extension_from_handler(template_path, true) ||
-        find_template_extension_from_handler(template_path) ||
-        find_template_extension_from_first_render()
-      end
-
-      def find_template_extension_from_handler(template_path, formatted = nil)
-        checked_template_path = formatted ? "#{template_path}.#{template_format}" : template_path
-
-        self.class.template_handler_extensions.each do |extension|
-          if template_exists?(checked_template_path, extension)
-            return formatted ? "#{template_format}.#{extension}" : extension.to_s
-          end
-        end
-        nil
-      end
-      
-      # Determine the template extension from the <tt>@first_render</tt> filename
-      def find_template_extension_from_first_render
-        File.basename(@first_render.to_s)[/^[^.]+\.(.+)$/, 1]
-      end
-
-      # This method reads a template file.
-      def read_template_file(template_path, extension)
@@ -603 +489 @@
-            logger.debug "Backtrace: #{e.backtrace.join("\n")}"
-          end
-
-
+
+
-        end
-
-        @@compile_time[render_symbol] = Time.now

a/actionpack/lib/action_view/helpers/cache_helper.rb

@@ -32 +32 @@
-      #      <i>Topics listed alphabetically</i>
-      #    <% end %>
-      def cache(name = {}, options = nil, &block)
-find_template_extension_for
+@finder.pick_template_extension
-        handler = Base.handler_class_for_extension(template_extension)
-        handler.new(@controller).cache_fragment(block, name, options)
-      end

/dev/null

@@ +1 @@
+module ActionView #:nodoc:
+  class TemplateFinder #:nodoc:
+    
+    class InvalidViewPath < StandardError #:nodoc:
+    end
+    
+    cattr_reader :processed_view_paths
+    @@processed_view_paths = Hash.new {|hash, key| hash[key] = []}
+    
+    cattr_reader :file_extension_cache
+    @@file_extension_cache = Hash.new {|hash, key| 
+      hash[key] = Hash.new {|hash, key| hash[key] = []}
+    }
+    
+    class << self #:nodoc:
+      
+      # This method is not thread safe. Mutex should be used whenever this is accessed from an instance method
+      def process_view_paths(*view_paths)
+        view_paths.flatten.compact.each do |dir|
+          next if @@processed_view_paths.has_key?(dir)
+          
+          @@processed_view_paths[dir] = []
+          Dir.glob("#{dir}/**/*").each do |file|
+            unless File.directory?(file)
+              @@processed_view_paths[dir] << file.split(dir).last.sub(/^\//, '')
+              
+              # Build extension cache
+              extension = file.split(".").last
+              if template_handler_extensions.include?(extension)
+                key = file.split(dir).last.sub(/^\//, '').sub(/\.(\w+)$/, '')
+                @@file_extension_cache[dir][key] << extension
+              end
+            end
+          end
+        end
+      end
+      
+      def update_extension_cache_for(extension)
+        @@processed_view_paths.keys.each do |dir|
+          Dir.glob("#{dir}/**/*.#{extension}").each do |file|
+            key = file.split(dir).last.sub(/^\//, '').sub(/\.(\w+)$/, '')
+            @@file_extension_cache[dir][key] << extension
+          end
+        end
+      end
+      
+      def template_handler_extensions
+        ActionView::Base.template_handler_extensions
+      end
+      
+      def reload!
+        view_paths = @@processed_view_paths.keys
+        
+        @@processed_view_paths = Hash.new {|hash, key| hash[key] = []}
+        @@file_extension_cache = Hash.new {|hash, key| 
+          hash[key] = Hash.new {|hash, key| hash[key] = []}
+        }
+        
+        process_view_paths(view_paths)
+      end
+    end
+    
+    attr_accessor :view_paths
+    
+    def initialize(*args)
+      @template = args.shift
+      
+      @view_paths = args.flatten
+      @view_paths = @view_paths.respond_to?(:find) ? @view_paths.dup : [*@view_paths].compact
+      check_view_paths(@view_paths)
+    end
+    
+    def prepend_view_path(path)
+      @view_paths.unshift(*path)
+      
+      self.class.process_view_paths(path)
+    end
+    
+    def append_view_path(path)
+      @view_paths.push(*path)
+      
+      self.class.process_view_paths(path)
+    end
+    
+    def view_paths=(path)
+      @view_paths = path
+      self.class.process_view_paths(path)
+    end
+    
+    def template_exists?(template_path, extension)
+      file_name = "#{template_path}.#{extension}"
+      base_path = find_base_path_for(file_name)
+      base_path.blank? ? false : "#{base_path}/#{file_name}"
+    end
+    
+    def file_exists?(template_path)
+      template_file_name, template_file_extension = path_and_extension(template_path)
+      if template_file_extension
+        template_exists?(template_file_name, template_file_extension)
+      else
+        template_exists?(template_file_name, pick_template_extension(template_path))
+      end
+    end
+
+    def find_base_path_for(template_file_name)
+      @view_paths.find { |path| processed_view_paths[path].include?(template_file_name) }
+    end
+    
+    # Returns the view path that the full path resides in.
+    def extract_base_path_from(full_path)
+      @view_paths.find { |p| full_path[0..p.size - 1] == p }
+    end
+    
+    # Gets the extension for an existing template with the given template_path.
+    # Returns the format with the extension if that template exists.
+    #
+    #   pick_template_extension('users/show')
+    #   # => 'html.erb'
+    #
+    #   pick_template_extension('users/legacy')
+    #   # => "rhtml"
+    #
+    def pick_template_extension(template_path)
+      find_template_extension_from_handler(template_path, true) ||
+      find_template_extension_from_handler(template_path) ||
+      find_template_extension_from_first_render()
+    end
+    
+    def find_template_extension_from_handler(template_path, formatted = nil)
+      checked_template_path = formatted ? "#{template_path}.#{@template.template_format}" : template_path
+      
+      view_paths.each do |path|
+        if file_extension_cache[path].has_key?(checked_template_path)
+          extension = file_extension_cache[path][checked_template_path].first
+          return formatted ? "#{@template.template_format}.#{extension}" : extension.to_s
+        end
+      end
+      nil
+    end
+    
+    # Splits the path and extension from the given template_path and returns as an array.
+    def path_and_extension(template_path)
+      template_path_without_extension = template_path.sub(/\.(\w+)$/, '')
+      [ template_path_without_extension, $1 ]
+    end
+    
+    # Determine the template extension from the <tt>@first_render</tt> filename
+    def find_template_extension_from_first_render
+      File.basename(@template.first_render.to_s)[/^[^.]+\.(.+)$/, 1]
+    end
+    
+    private
+    
+    def check_view_paths(view_paths)
+      view_paths.each do |path| 
+        raise(InvalidViewPath, "Unprocessed view path found in #{view_paths.inspect}") unless processed_view_paths.has_key?(path)
+      end
+    end
+    
+  end
+end

a/actionpack/test/controller/mime_responds_test.rb

@@ -437 +437 @@
-end
-
-class AbstractPostController < ActionController::Base
-
-
-
-
-
+
-end
-
-# For testing layouts which are set automatically

a/actionpack/test/controller/render_test.rb

@@ -199 +199 @@
-end
-
-TestController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
-
+ 
-
-class RenderTest < Test::Unit::TestCase
-  def setup

a/actionpack/test/controller/view_paths_test.rb

@@ -16 +16 @@
-    def hello_world_at_request_time() render(:action => 'hello_world') end
-    private
-    def add_view_path
-self.class.view_paths.unshift
+prepend_view_path
-    end
-  end
-  
@@ -27 +27 @@
-  
-  def setup
-    TestController.view_paths = nil
-    ActionView::Base.cache_template_extensions = false
-
-    @request  = ActionController::TestRequest.new
-    @response = ActionController::TestResponse.new
@@ -45 +44 @@
-  
-  def teardown
-    ActiveSupport::Deprecation.behavior = @old_behavior
-    ActionView::Base.cache_template_extensions = true
-  end
-  
-  def test_template_load_path_was_set_correctly
@@ -99 +97 @@
-  end
-  
-  def test_view_paths_override
-view_paths.unshift
+prepend_view_path
-    get :hello_world
-    assert_response :success
-    assert_equal "Hello overridden world!", @response.body

/dev/null

@@ +1 @@
+require 'abstract_unit'
+
+class TemplateFinderTest < Test::Unit::TestCase
+  
+  LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures')
+
+  def setup
+    ActionView::TemplateFinder.process_view_paths(LOAD_PATH_ROOT)
+    ActionView::Base::register_template_handler :mab, Class.new(ActionView::TemplateHandler)
+    @template = ActionView::Base.new
+    @finder = ActionView::TemplateFinder.new(@template, LOAD_PATH_ROOT)
+  end
+  
+  def test_should_raise_exception_for_unprocessed_view_path
+    assert_raises ActionView::TemplateFinder::InvalidViewPath do
+      ActionView::TemplateFinder.new(@template, File.dirname(__FILE__))
+    end
+  end
+  
+  def test_should_cache_file_extension_properly
+    assert_equal ["builder", "erb", "rhtml", "rjs", "rxml", "mab"].sort, 
+                 ActionView::TemplateFinder.file_extension_cache[LOAD_PATH_ROOT].values.flatten.uniq.sort
+                 
+    assert_equal Dir.glob("#{LOAD_PATH_ROOT}/**/*.{erb,rjs,rjs,builder,rxml,mab}").size,
+                 ActionView::TemplateFinder.file_extension_cache[LOAD_PATH_ROOT].keys.size
+  end
+  
+  def test_should_cache_dir_content_properly
+    assert ActionView::TemplateFinder.processed_view_paths[LOAD_PATH_ROOT]
+    assert_equal Dir.glob("#{LOAD_PATH_ROOT}/**/*").find_all {|f| !File.directory?(f) }.size,
+                 ActionView::TemplateFinder.processed_view_paths[LOAD_PATH_ROOT].size
+  end
+  
+  uses_mocha 'Template finder tests' do
+    
+    def test_should_update_extension_cache_when_template_handler_is_registered
+      ActionView::TemplateFinder.expects(:update_extension_cache_for).with("funky")
+      ActionView::Base::register_template_handler :funky, Class.new(ActionView::TemplateHandler)
+    end
+    
+  end
+  
+end

a/railties/environments/development.rb

@@ -12 +12 @@
-config.action_controller.consider_all_requests_local = true
-config.action_view.debug_rjs                         = true
-config.action_controller.perform_caching             = false
-config.action_view.cache_template_extensions         = false
-
-# Don't care if the mailer can't send
-config.action_mailer.raise_delivery_errors = false