diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index e9738f6a86a76291a70729b7bb8b07f87f79e723..109f2752f62df33f10074a6cd8b4e876320a0de5 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 1cf6937578d4ffe21054695863d8bdacb4905eac..68a23221639cd836bd951afb66a4611eb93b96c7 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 0000000000000000000000000000000000000000..cfa864cdd4a0136f9799bbd382ace6a90bc8df31 --- /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 39518268dfc777188c09239497e6e4ae3d1a3689..59e1015976110d522f305f7501fb165ffb3bbdfc 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/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 %> # All the topics on this project # <%= 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 %> + # All the topics on this project + # <%= 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 diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 0efba5b77f8580a070c4ae111fd39970d83272d0..2c3511f6a0f024d4dc9cfc141282e42d4d43c609 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/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 "

ERB

", @store.read('views/test.host/functional_caching/formatted_fragment_cached') + assert_equal "

ERB

", + @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 "

Builder

\n", @store.read('views/test.host/functional_caching/formatted_fragment_cached') + assert_equal "

Builder

\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 0000000000000000000000000000000000000000..f172e749daa5dab1d5bb2921e004defe07aa7321 --- /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 0000000000000000000000000000000000000000..c28646a2831fef53bdb401d3687d951d74a4c25c --- /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 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 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 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 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 0000000000000000000000000000000000000000..406a0fb84815faacafac23bc73d7a1acc8ac5e8c --- /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 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 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 0000000000000000000000000000000000000000..1937b652e4d8fb31d14a25c01eac6fbebace9195 --- /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 0000000000000000000000000000000000000000..9f73345a9f0c9888e9f7de003b0d7cb85c414c2a --- /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 0000000000000000000000000000000000000000..067ab500f51e0413af50c9869186d4d504303605 --- /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