Clean up view path cruft and split path implementations into Template::Path and Template::EagerPath

actionmailer/test/abstract_unit.rb

@@ -17,7 +17,6 @@
 
 FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures')
 ActionMailer::Base.template_root = FIXTURE_LOAD_PATH
-ActionMailer::Base.template_root.load
 
 class MockSMTP
   def self.deliveries

actionpack/lib/action_controller/base.rb

@@ -869,7 +869,7 @@ def render(options = nil, extra_options = {}, &block) #:doc:
         validate_render_arguments(options, extra_options, block_given?)
 
         if options.nil?
-          options = { :template => default_template.filename, :layout => true }
+          options = { :template => default_template, :layout => true }
         elsif options == :update
           options = extra_options.merge({ :update => true })
         elsif options.is_a?(String) || options.is_a?(Symbol)
@@ -1125,7 +1125,7 @@ def stale?(options)
       end
 
       # Sets the etag, last_modified, or both on the response and renders a
-      # "304 Not Modified" response if the request is already fresh. 
+      # "304 Not Modified" response if the request is already fresh.
       #
       # Example:
       #
@@ -1133,8 +1133,8 @@ def stale?(options)
       #     @article = Article.find(params[:id])
       #     fresh_when(:etag => @article, :last_modified => @article.created_at.utc)
       #   end
-      # 
-      # This will render the show template if the request isn't sending a matching etag or 
+      #
+      # This will render the show template if the request isn't sending a matching etag or
       # If-Modified-Since header and just a "304 Not Modified" response if there's a match.
       def fresh_when(options)
         options.assert_valid_keys(:etag, :last_modified)
@@ -1239,7 +1239,7 @@ def log_processing
           log_processing_for_parameters
         end
       end
-      
+
       def log_processing_for_request_id
         request_id = "\n\nProcessing #{self.class.name}\##{action_name} "
         request_id << "to #{params[:format]} " if params[:format]
@@ -1251,7 +1251,7 @@ def log_processing_for_request_id
       def log_processing_for_parameters
         parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup
         parameters = parameters.except!(:controller, :action, :format, :_method)
-        
+
         logger.info "  Parameters: #{parameters.inspect}" unless parameters.empty?
       end
 

actionpack/lib/action_controller/rescue.rb

@@ -38,8 +38,8 @@ module Rescue
       'ActionView::TemplateError'         => 'template_error'
     }
 
-    RESCUES_TEMPLATE_PATH = ActionView::PathSet::Path.new(
-      File.join(File.dirname(__FILE__), "templates"), true)
+    RESCUES_TEMPLATE_PATH = ActionView::Template::EagerPath.new(
+      File.join(File.dirname(__FILE__), "templates"))
 
     def self.included(base) #:nodoc:
       base.cattr_accessor :rescue_responses

actionpack/lib/action_view/base.rb

@@ -225,7 +225,7 @@ def view_paths=(paths)
     end
 
     # Returns the result of a render that's dictated by the options hash. The primary options are:
-    # 
+    #
     # * <tt>:partial</tt> - See ActionView::Partials.
     # * <tt>:update</tt> - Calls update_page with the block given.
     # * <tt>:file</tt> - Renders an explicit template file (this used to be the old default), add :locals to pass in those.

actionpack/lib/action_view/inline_template.rb

@@ -12,7 +12,7 @@ def initialize(source, type = nil)
 
     private
       # Always recompile inline templates
-      def recompile?(local_assigns)
+      def recompile?
         true
       end
   end

actionpack/lib/action_view/paths.rb

@@ -2,7 +2,7 @@ module ActionView #:nodoc:
   class PathSet < Array #:nodoc:
     def self.type_cast(obj)
       if obj.is_a?(String)
-        Path.new(obj)
+        Template::EagerPath.new(obj)
       else
         obj
       end
@@ -32,111 +32,6 @@ def unshift(*objs)
       super(*objs.map { |obj| self.class.type_cast(obj) })
     end
 
-    class Path #:nodoc:
-      attr_reader :path, :paths
-      delegate :hash, :inspect, :to => :path
-
-      def initialize(path, load = false)
-        raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
-        @path = path.freeze
-        reload! if load
-      end
-
-      def to_s
-        if defined?(RAILS_ROOT)
-          path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '')
-        else
-          path.to_s
-        end
-      end
-
-      def to_str
-        path.to_str
-      end
-
-      def ==(path)
-        to_str == path.to_str
-      end
-
-      def eql?(path)
-        to_str == path.to_str
-      end
-
-      # Returns a ActionView::Template object for the given path string. The
-      # input path should be relative to the view path directory,
-      # +hello/index.html.erb+. This method also has a special exception to
-      # match partial file names without a handler extension. So
-      # +hello/index.html+ will match the first template it finds with a
-      # known template extension, +hello/index.html.erb+. Template extensions
-      # should not be confused with format extensions +html+, +js+, +xml+,
-      # etc. A format must be supplied to match a formated file. +hello/index+
-      # will never match +hello/index.html.erb+.
-      #
-      # This method also has two different implementations, one that is "lazy"
-      # and makes file system calls every time and the other is cached,
-      # "eager" which looks up the template in an in memory index. The "lazy"
-      # version is designed for development where you want to automatically
-      # find new templates between requests. The "eager" version is designed
-      # for production mode and it is much faster but requires more time
-      # upfront to build the file index.
-      def [](path)
-        if loaded?
-          @paths[path]
-        else
-          Dir.glob("#{@path}/#{path}*").each do |file|
-            template = create_template(file)
-            if template.accessible_paths.include?(path)
-              return template
-            end
-          end
-          nil
-        end
-      end
-
-      def loaded?
-        @loaded ? true : false
-      end
-
-      def load
-        reload! unless loaded?
-        self
-      end
-
-      # Rebuild load path directory cache
-      def reload!
-        @paths = {}
-
-        templates_in_path do |template|
-          template.load!
-          template.accessible_paths.each do |path|
-            @paths[path] = template
-          end
-        end
-
-        @paths.freeze
-        @loaded = true
-      end
-
-      private
-        def templates_in_path
-          (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file|
-            yield create_template(file) unless File.directory?(file)
-          end
-        end
-
-        def create_template(file)
-          Template.new(file.split("#{self}/").last, self)
-        end
-    end
-
-    def load
-      each { |path| path.load }
-    end
-
-    def reload!
-      each { |path| path.reload! }
-    end
-
     def find_template(original_template_path, format = nil)
       return original_template_path if original_template_path.respond_to?(:render)
       template_path = original_template_path.sub(/^\//, '')

actionpack/lib/action_view/renderable.rb

@@ -60,7 +60,7 @@ def method_name(local_assigns)
       def compile(local_assigns)
         render_symbol = method_name(local_assigns)
 
-        if recompile?(render_symbol)
+        if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile?
           compile!(render_symbol, local_assigns)
         end
       end
@@ -89,11 +89,8 @@ def #{render_symbol}(local_assigns)
         end
       end
 
-      # Method to check whether template compilation is necessary.
-      # The template will be compiled if the file has not been compiled yet, or
-      # if local_assigns has a new key, which isn't supported by the compiled code yet.
-      def recompile?(symbol)
-        !Base::CompiledTemplates.method_defined?(symbol) || !loaded?
+      def recompile?
+        false
       end
   end
 end

actionpack/lib/action_view/template.rb

 module ActionView #:nodoc:
   class Template
+    class Path
+      attr_reader :path, :paths
+      delegate :hash, :inspect, :to => :path
+
+      def initialize(path)
+        raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
+        @path = path.freeze
+      end
+
+      def to_s
+        if defined?(RAILS_ROOT)
+          path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '')
+        else
+          path.to_s
+        end
+      end
+
+      def to_str
+        path.to_str
+      end
+
+      def ==(path)
+        to_str == path.to_str
+      end
+
+      def eql?(path)
+        to_str == path.to_str
+      end
+
+      # Returns a ActionView::Template object for the given path string. The
+      # input path should be relative to the view path directory,
+      # +hello/index.html.erb+. This method also has a special exception to
+      # match partial file names without a handler extension. So
+      # +hello/index.html+ will match the first template it finds with a
+      # known template extension, +hello/index.html.erb+. Template extensions
+      # should not be confused with format extensions +html+, +js+, +xml+,
+      # etc. A format must be supplied to match a formated file. +hello/index+
+      # will never match +hello/index.html.erb+.
+      def [](path)
+        templates_in_path do |template|
+          if template.accessible_paths.include?(path)
+            return template
+          end
+        end
+        nil
+      end
+
+      private
+        def templates_in_path
+          (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file|
+            yield create_template(file) unless File.directory?(file)
+          end
+        end
+
+        def create_template(file)
+          Template.new(file.split("#{self}/").last, self)
+        end
+    end
+
+    class EagerPath < Path
+      def initialize(path)
+        super
+
+        @paths = {}
+        templates_in_path do |template|
+          template.load!
+          template.accessible_paths.each do |path|
+            @paths[path] = template
+          end
+        end
+        @paths.freeze
+      end
+
+      def [](path)
+        @paths[path]
+      end
+    end
+
     extend TemplateHandlers
     extend ActiveSupport::Memoizable
     include Renderable
@@ -115,13 +193,12 @@ def stale?
       File.mtime(filename) > mtime
     end
 
-    def loaded?
-      @loaded
+    def recompile?
+      !@cached
     end
 
     def load!
-      @loaded = true
-      compile({})
+      @cached = true
       freeze
     end
 

actionpack/test/abstract_unit.rb

@@ -34,7 +34,6 @@
 
 FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures')
 ActionController::Base.view_paths = FIXTURE_LOAD_PATH
-ActionController::Base.view_paths.load
 
 def uses_mocha(test_name)
   yield

actionpack/test/template/compiled_templates_test.rb

@@ -31,7 +31,7 @@ def test_compiled_template_will_not_be_recompiled_when_rendered_with_identical_l
     end
 
     def test_compiled_template_will_always_be_recompiled_when_template_is_not_cached
-      ActionView::Template.any_instance.expects(:loaded?).times(3).returns(false)
+      ActionView::Template.any_instance.expects(:recompile?).times(3).returns(true)
       assert_equal 0, @compiled_templates.instance_methods.size
       assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
       ActionView::Template.any_instance.expects(:compile!).times(3)
@@ -62,13 +62,14 @@ def render(*args)
 
       def render_with_cache(*args)
         view_paths = ActionController::Base.view_paths
-        assert view_paths.first.loaded?
+        assert_equal ActionView::Template::EagerPath, view_paths.first.class
         ActionView::Base.new(view_paths, {}).render(*args)
       end
 
       def render_without_cache(*args)
-        view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH)
-        assert !view_paths.first.loaded?
+        path = ActionView::Template::Path.new(FIXTURE_LOAD_PATH)
+        view_paths = ActionView::Base.process_view_paths(path)
+        assert_equal ActionView::Template::Path, view_paths.first.class
         ActionView::Base.new(view_paths, {}).render(*args)
       end
 

actionpack/test/template/render_test.rb

@@ -197,7 +197,7 @@ class CachedViewRenderTest < Test::Unit::TestCase
   # Ensure view path cache is primed
   def setup
     view_paths = ActionController::Base.view_paths
-    assert view_paths.first.loaded?
+    assert_equal ActionView::Template::EagerPath, view_paths.first.class
     setup_view(view_paths)
   end
 end
@@ -208,8 +208,9 @@ class LazyViewRenderTest < Test::Unit::TestCase
   # Test the same thing as above, but make sure the view path
   # is not eager loaded
   def setup
-    view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH)
-    assert !view_paths.first.loaded?
+    path = ActionView::Template::Path.new(FIXTURE_LOAD_PATH)
+    view_paths = ActionView::Base.process_view_paths(path)
+    assert_equal ActionView::Template::Path, view_paths.first.class
     setup_view(view_paths)
   end
 end

railties/lib/initializer.rb

@@ -370,8 +370,9 @@ def load_observers
     def load_view_paths
       if configuration.frameworks.include?(:action_view)
         if configuration.cache_classes
-          ActionController::Base.view_paths.load if configuration.frameworks.include?(:action_controller)
-          ActionMailer::Base.template_root.load if configuration.frameworks.include?(:action_mailer)
+          view_path = ActionView::Template::EagerPath.new(configuration.view_path)
+          ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller)
+          ActionMailer::Base.template_root = view_path if configuration.frameworks.include?(:action_mailer)
         end
       end
     end
@@ -473,7 +474,7 @@ def initialize_framework_logging
     # set to use Configuration#view_path.
     def initialize_framework_views
       if configuration.frameworks.include?(:action_view)
-        view_path = ActionView::PathSet::Path.new(configuration.view_path, false)
+        view_path = ActionView::Template::Path.new(configuration.view_path)
         ActionMailer::Base.template_root ||= view_path if configuration.frameworks.include?(:action_mailer)
         ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller) && ActionController::Base.view_paths.empty?
       end