Resolvers now consider timestamps.

Before this patch, every request in development caused the template
to be compiled, regardless if it was updated in the filesystem or not.
This patch now checks the timestamp and only compiles it again if
any change was done.

While this probably won't show any difference for current setups,
but it will be useful for asset template handlers (like SASS), as
compiling their templates is slower than ERb, Haml, etc.

actionpack/lib/action_view/template.rb

@@ -98,10 +98,9 @@ module ActionView
 
     extend Template::Handlers
 
-    attr_accessor :locals
+    attr_accessor :locals, :formats, :virtual_path
 
-    attr_reader :source, :identifier, :handler, :virtual_path, :formats,
+    attr_reader :source, :identifier, :handler, :original_encoding, :updated_at
-                :original_encoding
 
     # This finalizer is needed (and exactly with a proc inside another proc)
     # otherwise templates leak in development.
@@ -114,15 +113,17 @@ module ActionView
     end
 
     def initialize(source, identifier, handler, details)
-      @source             = source
+      format = details[:format] || (handler.default_format if handler.respond_to?(:default_format))
-      @identifier         = identifier
+
-      @handler            = handler
+      @source            = source
-      @original_encoding  = nil
+      @identifier        = identifier
-      @method_names       = {}
+      @handler           = handler
-      @locals             = details[:locals] || []
+      @compiled          = false
-      @formats            = Array.wrap(details[:format] || :html).map(&:to_sym)
+      @original_encoding = nil
-      @virtual_path       = details[:virtual_path]
+      @locals            = details[:locals] || []
-      @compiled           = false
+      @virtual_path      = details[:virtual_path]
+      @updated_at        = details[:updated_at] || Time.now
+      @formats           = Array.wrap(format).map(&:to_sym)
     end
 
     # Render a template. If the template was not compiled yet, it is done

actionpack/lib/action_view/template/resolver.rb

@@ -16,7 +16,7 @@ module ActionView
 
     # Normalizes the arguments and passes it on to find_template.
     def find_all(name, prefix=nil, partial=false, details={}, locals=[], key=nil)
-      cached(key, prefix, name, partial, locals) do
+      cached(key, [name, prefix, partial], details, locals) do
         find_templates(name, prefix, partial, details)
       end
     end
@@ -35,37 +35,55 @@ module ActionView
     end
 
     # Helpers that builds a path. Useful for building virtual paths.
-    def build_path(name, prefix, partial, details)
+    def build_path(name, prefix, partial)
       path = ""
       path << "#{prefix}/" unless prefix.empty?
       path << (partial ? "_#{name}" : name)
       path
     end
 
-    # Get the handler and format from the given parameters.
+    # Hnadles templates caching. If a key is given and caching is on
-    def retrieve_handler_and_format(handler, format, default_formats=nil)
+    # always check the cache before hitting the resolver. Otherwise,
-      handler  = Template.handler_class_for_extension(handler)
+    # it always hits the resolver but check if the resolver is fresher
-      format   = format && Mime[format]
+    # before returning it.
-      format ||= handler.default_format if handler.respond_to?(:default_format)
+    def cached(key, path_info, details, locals) #:nodoc:
-      format ||= default_formats
+      name, prefix, partial = path_info
-      [handler, format]
-    end
-
-    def cached(key, prefix, name, partial, locals)
       locals = sort_locals(locals)
-      unless key && caching?
+
-        yield.each { |t| t.locals = locals }
+      if key && caching?
+        @cached[key][name][prefix][partial][locals] ||= decorate(yield, path_info, details, locals)
       else
-        @cached[key][prefix][name][partial][locals] ||= yield.each { |t| t.locals = locals }
+        fresh = decorate(yield, path_info, details, locals)
+        return fresh unless key
+
+        scope = @cached[key][name][prefix][partial]
+        cache = scope[locals]
+        mtime = cache && cache.map(&:updated_at).max
+
+        if !mtime || fresh.empty?  || fresh.any? { |t| t.updated_at > mtime }
+          scope[locals] = fresh
+        else
+          cache
+        end
       end
     end
 
-    if :locale.respond_to?("<=>")
+    # Ensures all the resolver information is set in the template.
-      def sort_locals(locals)
+    def decorate(templates, path_info, details, locals) #:nodoc:
+      cached = nil
+      templates.each do |t|
+        t.locals         = locals
+        t.formats        = details[:formats] || [:html] if t.formats.empty?
+        t.virtual_path ||= (cached ||= build_path(*path_info))
+      end
+    end
+
+    if :symbol.respond_to?("<=>")
+      def sort_locals(locals) #:nodoc:
         locals.sort.freeze
       end
     else
-      def sort_locals(locals)
+      def sort_locals(locals) #:nodoc:
         locals = locals.map{ |l| l.to_s }
         locals.sort!
         locals.freeze
@@ -79,7 +97,7 @@ module ActionView
     private
 
     def find_templates(name, prefix, partial, details)
-      path = build_path(name, prefix, partial, details)
+      path = build_path(name, prefix, partial)
       query(path, EXTENSION_ORDER.map { |ext| details[ext] }, details[:formats])
     end
 
@@ -96,17 +114,24 @@ module ActionView
         contents = File.open(p, "rb") {|io| io.read }
 
         Template.new(contents, File.expand_path(p), handler,
-          :virtual_path => path, :format => format)
+          :virtual_path => path, :format => format, :updated_at => mtime(p))
       end
     end
 
+    # Returns the file mtime from the filesystem.
+    def mtime(p)
+      File.stat(p).mtime
+    end
+
     # Extract handler and formats from path. If a format cannot be a found neither
     # from the path, or the handler, we should return the array of formats given
     # to the resolver.
     def extract_handler_and_format(path, default_formats)
       pieces = File.basename(path).split(".")
       pieces.shift
-      retrieve_handler_and_format(pieces.pop, pieces.pop, default_formats)
+      handler = Template.handler_class_for_extension(pieces.pop)
+      format  = pieces.last && Mime[pieces.last]
+      [handler, format]
     end
   end
 

actionpack/lib/action_view/testing/resolvers.rb

@@ -23,11 +23,12 @@ module ActionView #:nodoc:
       query = /^(#{Regexp.escape(path)})#{query}$/
 
       templates = []
-      @hash.each do |_path, source|
+      @hash.each do |_path, array|
+        source, updated_at = array
         next unless _path =~ query
         handler, format = extract_handler_and_format(_path, formats)
         templates << Template.new(source, _path, handler,
-          :virtual_path => $1, :format => format)
+          :virtual_path => $1, :format => format, :updated_at => updated_at)
       end
 
       templates.sort_by {|t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size }

actionpack/test/template/lookup_context_test.rb

@@ -185,3 +185,48 @@ class LookupContextTest < ActiveSupport::TestCase
     assert !ActionView::LookupContext.new(FIXTURE_LOAD_PATH, :cache => false).cache
   end
 end
+
+class LookupContextWithFalseCaching < ActiveSupport::TestCase
+  def setup
+    @resolver = ActionView::FixtureResolver.new("test/_foo.erb" => ["Foo", Time.utc(2000)])
+    @resolver.stubs(:caching?).returns(false)
+    @lookup_context = ActionView::LookupContext.new(@resolver, {})
+  end
+
+  test "templates are always found in the resolver but timestamp is checked before being compiled" do
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Foo", template.source
+
+    # Now we are going to change the template, but it won't change the returned template
+    # since the timestamp is the same.
+    @resolver.hash["test/_foo.erb"][0] = "Bar"
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Foo", template.source
+
+    # Now update the timestamp.
+    @resolver.hash["test/_foo.erb"][1] = Time.now.utc
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Bar", template.source
+  end
+
+  test "if no template was found in the second lookup, give it higher preference" do
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Foo", template.source
+
+    @resolver.hash.clear
+    assert_raise ActionView::MissingTemplate do
+      @lookup_context.find("foo", "test", true)
+    end
+  end
+
+  test "if no template was cached in the first lookup, do not use the cache in the second" do
+    @resolver.hash.clear
+    assert_raise ActionView::MissingTemplate do
+      @lookup_context.find("foo", "test", true)
+    end
+
+    @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)]
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Foo", template.source
+  end
+end