Add automatic template digests to all CacheHelper#cache calls (originally spiked in the cache_digests plugin) *DHH*

14 files changed, 358 insertions, 14 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index e9738f6a86..109f2752f6 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Add automatic template digests to all CacheHelper#cache calls (originally spiked in the cache_digests plugin) *DHH*
+
 *   When building a URL fails, add missing keys provided by Journey. Failed URL
     generation now returns a 500 status instead of a 404.
 
diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb
index 1cf6937578..68a2322163 100644
--- a/actionpack/lib/action_view.rb
+++ b/actionpack/lib/action_view.rb
@@ -33,6 +33,7 @@ module ActionView
     autoload :Base
     autoload :Context
     autoload :CompiledTemplates, "action_view/context"
+    autoload :Digestor
     autoload :Helpers
     autoload :LookupContext
     autoload :PathSet
diff --git a/actionpack/lib/action_view/digestor.rb b/actionpack/lib/action_view/digestor.rb
new file mode 100644
index 0000000000..cfa864cdd4
--- /dev/null
+++ b/actionpack/lib/action_view/digestor.rb
@@ -0,0 +1,104 @@
+require 'active_support/core_ext'
+require 'logger'
+
+module ActionView
+  class Digestor
+    EXPLICIT_DEPENDENCY = /# Template Dependency: ([^ ]+)/
+
+    # Matches:
+    #   render partial: "comments/comment", collection: commentable.comments
+    #   render "comments/comments"
+    #   render 'comments/comments'
+    #   render('comments/comments')
+    #
+    #   render(@topic)         => render("topics/topic")
+    #   render(topics)         => render("topics/topic")
+    #   render(message.topics) => render("topics/topic")
+    RENDER_DEPENDENCY = /
+      render\s?                # render, followed by an optional space
+      \(?                      # start a optional parenthesis for the render call
+      (partial:)?\s?           # naming the partial, used with collection -- 1st capture
+      ([@a-z"'][@a-z_\/\."']+) # the template name itself -- 2nd capture
+    /x
+
+    cattr_accessor(:cache)  { Hash.new }
+    cattr_accessor(:logger, instance_reader: true) { ActionView::Base.logger }
+
+    def self.digest(name, format, finder, options = {})
+      cache["#{name}.#{format}"] ||= new(name, format, finder, options).digest
+    end
+
+    attr_reader :name, :format, :finder, :options
+
+    def initialize(name, format, finder, options = {})
+      @name, @format, @finder, @options = name, format, finder, options
+    end
+
+    def digest
+      Digest::MD5.hexdigest("#{name}.#{format}-#{source}-#{dependency_digest}").tap do |digest|
+        logger.try :info, "Cache digest for #{name}.#{format}: #{digest}"
+      end
+    rescue ActionView::MissingTemplate
+      logger.try :error, "Couldn't find template for digesting: #{name}.#{format}"
+      ''
+    end
+
+    def dependencies
+      render_dependencies + explicit_dependencies
+    rescue ActionView::MissingTemplate
+      [] # File doesn't exist, so no dependencies
+    end
+
+    def nested_dependencies
+      dependencies.collect do |dependency|
+        dependencies = Digestor.new(dependency, format, finder, partial: true).nested_dependencies
+        dependencies.any? ? { dependency => dependencies } : dependency
+      end
+    end
+
+
+    private
+      def logical_name
+        name.gsub(%r|/_|, "/")
+      end
+      
+      def directory
+        name.split("/").first
+      end
+
+      def partial?
+        options[:partial] || name.include?("/_")
+      end
+
+      def source
+        @source ||= finder.find(logical_name, [], partial?, formats: [ format ]).source
+      end
+
+
+      def dependency_digest
+        dependencies.collect do |template_name|
+          Digestor.digest(template_name, format, finder, partial: true)
+        end.join("-")
+      end
+
+      def render_dependencies
+        source.scan(RENDER_DEPENDENCY).
+          collect(&:second).uniq.
+
+          # render(@topic)         => render("topics/topic")
+          # render(topics)         => render("topics/topic")
+          # render(message.topics) => render("topics/topic")
+          collect { |name| name.sub(/\A@?([a-z]+\.)*([a-z_]+)\z/) { "#{$2.pluralize}/#{$2.singularize}" } }.
+
+          # render("headline") => render("message/headline")
+          collect { |name| name.include?("/") ? name : "#{directory}/#{name}" }.
+          
+          # replace quotes from string renders
+          collect { |name| name.gsub(/["']/, "") }
+      end
+
+      def explicit_dependencies
+        source.scan(EXPLICIT_DEPENDENCY).flatten.uniq
+      end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb
index 39518268df..59e1015976 100644
--- a/actionpack/lib/action_view/helpers/cache_helper.rb
+++ b/actionpack/lib/action_view/helpers/cache_helper.rb
@@ -13,10 +13,9 @@ module ActionView
       # kick out old entries. For more on key-based expiration, see:
       # http://37signals.com/svn/posts/3113-how-key-based-cache-expiration-works
       #
-      # When using this method, you list the cache dependencies as part of
-      # the name of the cache, like so:
+      # When using this method, you list the cache dependency as the name of the cache, like so:
       #
-      #   <% cache [ "v1", project ] do %>
+      #   <% cache project do %>
       #     <b>All the topics on this project</b>
       #     <%= render project.topics %>
       #   <% end %>
@@ -24,15 +23,89 @@ module ActionView
       # This approach will assume that when a new topic is added, you'll touch
       # the project. The cache key generated from this call will be something like:
       #
-      #   views/v1/projects/123-20120806214154
-      #            ^class   ^id ^updated_at
+      #   views/projects/123-20120806214154/7a1156131a6928cb0026877f8b749ac9
+      #         ^class   ^id ^updated_at    ^template tree digest
       #
-      # If you update the rendering of topics, you just bump the version to v2.
-      # Otherwise the cache is automatically bumped whenever the project updated_at
-      # is touched.
+      # The cache is thus automatically bumped whenever the project updated_at is touched.
+      #
+      # If your template cache depends on multiple sources (try to avoid this to keep things simple),
+      # you can name all these dependencies as part of an array:
+      #
+      #   <% cache [ project, current_user ] do %>
+      #     <b>All the topics on this project</b>
+      #     <%= render project.topics %>
+      #   <% end %>
+      #
+      # This will include both records as part of the cache key and updating either of them will
+      # expire the cache.
+      #
+      # ==== Template digest
+      #
+      # The template digest that's added to the cache key is computed by taking an md5 of the
+      # contents of the entire template file. This ensures that your caches will automatically
+      # expire when you change the template file.
+      #
+      # Note that the md5 is taken of the entire template file, not just what's within the
+      # cache do/end call. So it's possible that changing something outside of that call will
+      # still expire the cache.
+      #
+      # Additionally, the digestor will automatically look through your template file for
+      # explicit and implicit dependencies, and include those as part of the digest.
+      #
+      # ==== Implicit dependencies
+      #
+      # Most template dependencies can be derived from calls to render in the template itself.
+      # Here are some examples of render calls that Cache Digests knows how to decode:
+      #   
+      #   render partial: "comments/comment", collection: commentable.comments
+      #   render "comments/comments"
+      #   render 'comments/comments'
+      #   render('comments/comments')
+      #   
+      #   render "header" => render("comments/header")
+      #   
+      #   render(@topic)         => render("topics/topic")
+      #   render(topics)         => render("topics/topic")
+      #   render(message.topics) => render("topics/topic")
+      #   
+      # It's not possible to derive all render calls like that, though. Here are a few examples of things that can't be derived:
+      #   
+      #   render group_of_attachments
+      #   render @project.documents.where(published: true).order('created_at')
+      #   
+      # You will have to rewrite those to the explicit form:
+      #   
+      #   render partial: 'attachments/attachment', collection: group_of_attachments
+      #   render partial: 'documents/document', collection: @project.documents.where(published: true).order('created_at')
+      #
+      # === Explicit dependencies
+      # 
+      # Some times you'll have template dependencies that can't be derived at all. This is typically
+      # the case when you have template rendering that happens in helpers. Here's an example:
+      # 
+      #   <%= render_sortable_todolists @project.todolists %>
+      # 
+      # You'll need to use a special comment format to call those out:
+      # 
+      #   <%# Template Dependency: todolists/todolist %>
+      #   <%= render_sortable_todolists @project.todolists %>
+      # 
+      # The pattern used to match these is /# Template Dependency: ([^ ]+)/, so it's important that you type it out just so.
+      # You can only declare one template dependency per line.
+      #
+      # === External dependencies
+      #
+      # If you use a helper method, for example, inside of a cached block and you then update that helper,
+      # you'll have to bump the cache as well. It doesn't really matter how you do it, but the md5 of the template file
+      # must change. One recommendation is to simply be explicit in a comment, like:
+      #
+      #   <%# Helper Dependency Updated: May 6, 2012 at 6pm %>
+      #   <%= some_helper_method(person) %>
+      #
+      # Now all you'll have to do is change that timestamp when the helper method changes.
       def cache(name = {}, options = nil, &block)
         if controller.perform_caching
-          safe_concat(fragment_for(name, options, &block))
+          safe_concat(fragment_for(fragment_name_with_digest(name), options, &block))
         else
           yield
         end
@@ -58,6 +131,17 @@ module ActionView
           controller.write_fragment(name, fragment, options)
         end
       end
+      
+      def fragment_name_with_digest(name)
+        if @virtual_path
+          [
+            *Array(name.is_a?(Hash) ? controller.url_for(name).split("://").last : name),
+            Digestor.digest(@virtual_path, formats.last.to_sym, lookup_context)
+          ]
+        else
+          name
+        end
+      end
     end
   end
 end
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 0efba5b77f..2c3511f6a0 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -854,14 +854,17 @@ Ciao
 CACHED
     assert_equal expected_body, @response.body
 
-    assert_equal "This bit's fragment cached", @store.read('views/test.host/functional_caching/fragment_cached')
+    assert_equal "This bit's fragment cached",
+      @store.read("views/test.host/functional_caching/fragment_cached/#{template_digest("functional_caching/fragment_cached", "html")}")
   end
 
   def test_fragment_caching_in_partials
     get :html_fragment_cached_with_partial
     assert_response :success
     assert_match(/Old fragment caching in a partial/, @response.body)
-    assert_match("Old fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial'))
+    
+    assert_match("Old fragment caching in a partial",
+      @store.read("views/test.host/functional_caching/html_fragment_cached_with_partial/#{template_digest("functional_caching/_partial", "html")}"))
   end
 
   def test_render_inline_before_fragment_caching
@@ -869,7 +872,8 @@ CACHED
     assert_response :success
     assert_match(/Some inline content/, @response.body)
     assert_match(/Some cached content/, @response.body)
-    assert_match("Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached'))
+    assert_match("Some cached content", 
+      @store.read("views/test.host/functional_caching/inline_fragment_cached/#{template_digest("functional_caching/inline_fragment_cached", "html")}"))
   end
 
   def test_html_formatted_fragment_caching
@@ -879,7 +883,8 @@ CACHED
 
     assert_equal expected_body, @response.body
 
-    assert_equal "<p>ERB</p>", @store.read('views/test.host/functional_caching/formatted_fragment_cached')
+    assert_equal "<p>ERB</p>", 
+      @store.read("views/test.host/functional_caching/formatted_fragment_cached/#{template_digest("functional_caching/formatted_fragment_cached", "html")}")
   end
 
   def test_xml_formatted_fragment_caching
@@ -889,8 +894,14 @@ CACHED
 
     assert_equal expected_body, @response.body
 
-    assert_equal "  <p>Builder</p>\n", @store.read('views/test.host/functional_caching/formatted_fragment_cached')
+    assert_equal "  <p>Builder</p>\n",
+      @store.read("views/test.host/functional_caching/formatted_fragment_cached/#{template_digest("functional_caching/formatted_fragment_cached", "xml")}")
   end
+  
+  private
+    def template_digest(name, format)
+      ActionView::Digestor.digest(name, format, @controller.lookup_context)
+    end
 end
 
 class CacheHelperOutputBufferTest < ActionController::TestCase
diff --git a/actionpack/test/fixtures/digestor/comments/_comment.html.erb b/actionpack/test/fixtures/digestor/comments/_comment.html.erb
new file mode 100644
index 0000000000..f172e749da
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/comments/_comment.html.erb
@@ -0,0 +1 @@
+Great story, bro!
diff --git a/actionpack/test/fixtures/digestor/comments/_comments.html.erb b/actionpack/test/fixtures/digestor/comments/_comments.html.erb
new file mode 100644
index 0000000000..c28646a283
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/comments/_comments.html.erb
@@ -0,0 +1 @@
+<%= render partial: "comments/comment", collection: commentable.comments %>
diff --git a/actionpack/test/fixtures/digestor/events/_event.html.erb b/actionpack/test/fixtures/digestor/events/_event.html.erb
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/events/_event.html.erb
diff --git a/actionpack/test/fixtures/digestor/messages/_header.html.erb b/actionpack/test/fixtures/digestor/messages/_header.html.erb
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/messages/_header.html.erb
diff --git a/actionpack/test/fixtures/digestor/messages/_message.html.erb b/actionpack/test/fixtures/digestor/messages/_message.html.erb
new file mode 100644
index 0000000000..406a0fb848
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/messages/_message.html.erb
@@ -0,0 +1 @@
+THIS BE WHERE THEM MESSAGE GO, YO!
\ No newline at end of file
diff --git a/actionpack/test/fixtures/digestor/messages/actions/_move.html.erb b/actionpack/test/fixtures/digestor/messages/actions/_move.html.erb
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/messages/actions/_move.html.erb
diff --git a/actionpack/test/fixtures/digestor/messages/index.html.erb b/actionpack/test/fixtures/digestor/messages/index.html.erb
new file mode 100644
index 0000000000..1937b652e4
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/messages/index.html.erb
@@ -0,0 +1,2 @@
+<%= render @messages %>
+<%= render @events %>
diff --git a/actionpack/test/fixtures/digestor/messages/show.html.erb b/actionpack/test/fixtures/digestor/messages/show.html.erb
new file mode 100644
index 0000000000..9f73345a9f
--- /dev/null
+++ b/actionpack/test/fixtures/digestor/messages/show.html.erb
@@ -0,0 +1,9 @@
+<%# Template Dependency: messages/message %>
+<%= render "header" %>
+<%= render "comments/comments" %>
+
+<%= render "messages/actions/move" %>
+
+<%= render @message.history.events %>
+
+<%# render "something_missing"  %>
\ No newline at end of file
diff --git a/actionpack/test/template/digestor_test.rb b/actionpack/test/template/digestor_test.rb
new file mode 100644
index 0000000000..067ab500f5
--- /dev/null
+++ b/actionpack/test/template/digestor_test.rb
@@ -0,0 +1,128 @@
+require 'abstract_unit'
+require 'fileutils'
+
+class FixtureTemplate
+  attr_reader :source
+  
+  def initialize(template_path)
+    @source = File.read(template_path)
+  rescue Errno::ENOENT
+    raise ActionView::MissingTemplate.new([], "", [], true, [])
+  end
+end
+
+class FixtureFinder
+  FIXTURES_DIR = "#{File.dirname(__FILE__)}/../fixtures/digestor"
+  TMP_DIR      = "#{File.dirname(__FILE__)}/../tmp"
+  
+  def find(logical_name, keys, partial, options)
+    FixtureTemplate.new("#{TMP_DIR}/#{partial ? logical_name.gsub(%r|/([^/]+)$|, '/_\1') : logical_name}.#{options[:formats].first}.erb")
+  end
+end
+
+class TemplateDigestorTest < ActionView::TestCase
+  def setup
+    FileUtils.cp_r FixtureFinder::FIXTURES_DIR, FixtureFinder::TMP_DIR
+  end
+  
+  def teardown
+    FileUtils.rm_r FixtureFinder::TMP_DIR
+    ActionView::Digestor.cache.clear
+  end
+
+  def test_top_level_change_reflected
+    assert_digest_difference("messages/show") do
+      change_template("messages/show")
+    end
+  end
+
+  def test_explicit_dependency
+    assert_digest_difference("messages/show") do
+      change_template("messages/_message")
+    end
+  end
+
+  def test_second_level_dependency
+    assert_digest_difference("messages/show") do
+      change_template("comments/_comments")
+    end
+  end
+
+  def test_second_level_dependency_within_same_directory
+    assert_digest_difference("messages/show") do
+      change_template("messages/_header")
+    end
+  end
+
+  def test_third_level_dependency
+    assert_digest_difference("messages/show") do
+      change_template("comments/_comment")
+    end
+  end
+
+  def test_logging_of_missing_template
+    assert_logged "Couldn't find template for digesting: messages/something_missing.html" do
+      digest("messages/show")
+    end
+  end
+
+  def test_nested_template_directory
+    assert_digest_difference("messages/show") do
+      change_template("messages/actions/_move")
+    end
+  end
+  
+  def test_dont_generate_a_digest_for_missing_templates
+    assert_equal '', digest("nothing/there")
+  end
+
+  def test_collection_dependency
+    assert_digest_difference("messages/index") do
+      change_template("messages/_message")
+    end
+
+    assert_digest_difference("messages/index") do
+      change_template("events/_event")
+    end
+  end
+  
+  def test_collection_derived_from_record_dependency
+    assert_digest_difference("messages/show") do
+      change_template("events/_event")
+    end
+  end
+
+
+  private
+    def assert_logged(message)
+      log = StringIO.new
+      ActionView::Digestor.logger = Logger.new(log)
+
+      yield
+      
+      log.rewind
+      assert_match message, log.read
+      
+      ActionView::Digestor.logger = nil
+    end
+
+    def assert_digest_difference(template_name)
+      previous_digest = digest(template_name)
+      ActionView::Digestor.cache.clear
+
+      yield
+
+      assert previous_digest != digest(template_name), "digest didn't change"
+      ActionView::Digestor.cache.clear
+    end
+  
+    def digest(template_name)
+      ActionView::Digestor.digest(template_name, :html, FixtureFinder.new)
+    end
+    
+    def change_template(template_name)
+      File.open("#{FixtureFinder::TMP_DIR}/#{template_name}.html.erb", "w") do |f|
+        f.write "\nTHIS WAS CHANGED!"
+      end
+    end
+end