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.

4 files changed, 107 insertions, 35 deletions

diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb
index 04ff752e64..3cc9bb2710 100644
--- a/actionpack/lib/action_view/template.rb
+++ b/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,
-                :original_encoding
+    attr_reader :source, :identifier, :handler, :original_encoding, :updated_at
 
     # 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
-      @identifier         = identifier
-      @handler            = handler
-      @original_encoding  = nil
-      @method_names       = {}
-      @locals             = details[:locals] || []
-      @formats            = Array.wrap(details[:format] || :html).map(&:to_sym)
-      @virtual_path       = details[:virtual_path]
-      @compiled           = false
+      format = details[:format] || (handler.default_format if handler.respond_to?(:default_format))
+
+      @source            = source
+      @identifier        = identifier
+      @handler           = handler
+      @compiled          = false
+      @original_encoding = nil
+      @locals            = details[:locals] || []
+      @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
diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb
index 9ec39b16f0..5c6877a923 100644
--- a/actionpack/lib/action_view/template/resolver.rb
+++ b/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.
-    def retrieve_handler_and_format(handler, format, default_formats=nil)
-      handler  = Template.handler_class_for_extension(handler)
-      format   = format && Mime[format]
-      format ||= handler.default_format if handler.respond_to?(:default_format)
-      format ||= default_formats
-      [handler, format]
-    end
-
-    def cached(key, prefix, name, partial, locals)
+    # Hnadles templates caching. If a key is given and caching is on
+    # always check the cache before hitting the resolver. Otherwise,
+    # it always hits the resolver but check if the resolver is fresher
+    # before returning it.
+    def cached(key, path_info, details, locals) #:nodoc:
+      name, prefix, partial = path_info
       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
+
+    # Ensures all the resolver information is set in the template.
+    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 :locale.respond_to?("<=>")
-      def sort_locals(locals)
+    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
 
diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb
index ec0db48379..55583096e0 100644
--- a/actionpack/lib/action_view/testing/resolvers.rb
+++ b/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 }
diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb
index 55d581e512..23dfc1ba75 100644
--- a/actionpack/test/template/lookup_context_test.rb
+++ b/actionpack/test/template/lookup_context_test.rb
@@ -184,4 +184,49 @@ class LookupContextTest < ActiveSupport::TestCase
   test "can have cache disabled on initialization" do
     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
\ No newline at end of file