Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Add automatic template digests to all CacheHelper#cache calls (origin…

…ally spiked in the cache_digests plugin) *DHH*
  • Loading branch information...
commit 502d5e24e28b3634910495d0fb71cb20b1426aee 1 parent 3da10e3
@dhh dhh authored
View
2  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.
View
1  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
View
104 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
View
102 actionpack/lib/action_view/helpers/cache_helper.rb
@@ -13,10 +13,9 @@ module CacheHelper
# 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 CacheHelper
# 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 @@ def fragment_for(name = {}, options = nil, &block) #:nodoc:
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
View
21 actionpack/test/controller/caching_test.rb
@@ -854,14 +854,17 @@ def test_fragment_caching
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 @@ def test_render_inline_before_fragment_caching
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 @@ def test_html_formatted_fragment_caching
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 @@ def test_xml_formatted_fragment_caching
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
View
1  actionpack/test/fixtures/digestor/comments/_comment.html.erb
@@ -0,0 +1 @@
+Great story, bro!
View
1  actionpack/test/fixtures/digestor/comments/_comments.html.erb
@@ -0,0 +1 @@
+<%= render partial: "comments/comment", collection: commentable.comments %>
View
0  actionpack/test/fixtures/digestor/events/_event.html.erb
No changes.
View
0  actionpack/test/fixtures/digestor/messages/_header.html.erb
No changes.
View
1  actionpack/test/fixtures/digestor/messages/_message.html.erb
@@ -0,0 +1 @@
+THIS BE WHERE THEM MESSAGE GO, YO!
View
0  actionpack/test/fixtures/digestor/messages/actions/_move.html.erb
No changes.
View
2  actionpack/test/fixtures/digestor/messages/index.html.erb
@@ -0,0 +1,2 @@
+<%= render @messages %>
+<%= render @events %>
View
9 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" %>
View
128 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
Please sign in to comment.
Something went wrong with that request. Please try again.