From 4f51f01ff9125579f977295a99c593b7951068dd Mon Sep 17 00:00:00 2001 From: Steve Hull Date: Mon, 8 Nov 2010 00:53:18 -0800 Subject: [PATCH] Making caching work with content_for (in action caches and fragment caches) --- actionpack/lib/abstract_controller/base.rb | 3 +- actionpack/lib/abstract_controller/rendering.rb | 4 +- .../lib/action_controller/caching/actions.rb | 13 +---- .../lib/action_controller/caching/fragments.rb | 21 ++++++-- actionpack/lib/action_view/helpers/cache_helper.rb | 13 ++++- .../lib/action_view/helpers/capture_helper.rb | 15 ++++++ actionpack/test/controller/caching_test.rb | 53 +++++++++++++++++-- .../action_caching_test/with_content_for.html.erb | 2 + .../fragment_cached_with_content_for.html.erb | 6 ++ .../test/fixtures/layouts/talk_from_action.erb | 2 +- actionpack/test/template/capture_helper_test.rb | 29 +++++++++++ 11 files changed, 135 insertions(+), 26 deletions(-) create mode 100644 actionpack/test/fixtures/action_caching_test/with_content_for.html.erb create mode 100644 actionpack/test/fixtures/functional_caching/fragment_cached_with_content_for.html.erb diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index f83eade..c272ce1 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -9,9 +9,10 @@ module AbstractController # AbstractController::Base is a low-level API. Nobody should be # using it directly, and subclasses (like ActionController::Base) are # expected to provide their own +render+ method, since rendering means - # different things depending on the context. + # different things depending on the context. class Base attr_internal :response_body + attr_internal :content_to_cache attr_internal :action_name attr_internal :formats diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 475785a..67cae47 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -111,7 +111,9 @@ module AbstractController # Find and renders a template based on the options given. # :api: private def _render_template(options) #:nodoc: - view_context.render(options) + rendered_body = view_context.render(options) + self.content_to_cache = view_context.content_to_cache.merge(:layout => rendered_body) + return rendered_body end # The prefix used in render "foo" shortcuts. diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index d69d96b..26f6adc 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -88,15 +88,6 @@ module ActionController #:nodoc: end end - def _save_fragment(name, options) - return unless caching_allowed? - - content = response_body - content = content.join if content.is_a?(Array) - - write_fragment(name, content, options) - end - protected def expire_action(options = {}) return unless cache_configured? @@ -130,7 +121,9 @@ module ActionController #:nodoc: controller.action_has_layout = false unless @cache_layout yield controller.action_has_layout = true - body = controller._save_fragment(cache_path.path, @store_options) + + for_cache = @cache_layout ? Array(controller.response_body).join : controller.content_to_cache + body = controller.write_fragment(cache_path.path, for_cache, @store_options) if controller.caching_allowed? end body = controller.render_to_string(:text => body, :layout => true) unless @cache_layout diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 37c155b..7e16b0f 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -36,14 +36,15 @@ module ActionController #:nodoc: # Writes content to the location signified by key (see expire_fragment for acceptable formats) def write_fragment(key, content, options = nil) - return content unless cache_configured? + return_value = content.is_a?(Hash) ? content[:layout] : content + return return_value unless cache_configured? key = fragment_cache_key(key) instrument_fragment_cache :write_fragment, key do content = content.html_safe.to_str if content.respond_to?(:html_safe) cache_store.write(key, content, options) end - content + return_value end # Reads a cached fragment from the location signified by key (see expire_fragment for acceptable formats) @@ -53,6 +54,14 @@ module ActionController #:nodoc: key = fragment_cache_key(key) instrument_fragment_cache :read_fragment, key do result = cache_store.read(key, options) + + if result.is_a?(Hash) + result = result.dup if result.frozen? + fragment = result.delete(:layout) + result.each {|k,v| view_context.content_for(k,v) } + result = fragment + end + result.respond_to?(:html_safe) ? result.html_safe : result end end @@ -86,14 +95,18 @@ module ActionController #:nodoc: # method (or delete_matched, for Regexp keys.) def expire_fragment(key, options = nil) return unless cache_configured? - key = fragment_cache_key(key) unless key.is_a?(Regexp) - message = nil + + unless key.is_a?(Regexp) + key = fragment_cache_key(key) + key_for_content_for = fragment_cache_key(key) + end instrument_fragment_cache :expire_fragment, key do if key.is_a?(Regexp) cache_store.delete_matched(key, options) else cache_store.delete(key, options) + cache_store.delete(key_for_content_for, options) end end end diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index f544a9d..ea8851d 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -52,9 +52,16 @@ module ActionView # VIEW TODO: Make #capture usable outside of ERB # This dance is needed because Builder can't use capture pos = output_buffer.length - yield - fragment = output_buffer.slice!(pos..-1) - controller.write_fragment(name, fragment, options) + + # This particular dance is to determine what + # content_for was added/appended by this block + hash_to_cache = nil + cache_with_content_for do + yield + fragment = output_buffer.slice!(pos..-1) + hash_to_cache = {:layout => fragment}.merge(content_to_cache) + end + controller.write_fragment(name, hash_to_cache, options) end end end diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index c88bd1e..d992e44 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -136,6 +136,7 @@ module ActionView def content_for(name, content = nil, &block) content = capture(&block) if block_given? @_content_for[name] << content if content + @_subset_content_for[name] << content if content && @_subset_content_for @_content_for[name] unless content end @@ -175,6 +176,20 @@ module ActionView self.output_buffer = old_buffer end + # Added to support implementation of action caching + def cache_with_content_for #:nodoc:# + @_subset_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } + yield + ensure + @_subset_content_for = nil + end + + # Added to support implementation of action caching + def content_to_cache #:nodoc:# + cache_this = @_subset_content_for || @_content_for.except(:layout) + cache_this.dup.tap {|h| h.default = nil } + end + # Add the output buffer to the response body and start a new one. def flush_output_buffer #:nodoc: if output_buffer && !output_buffer.empty? diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 914ae56..c83728e 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -164,6 +164,7 @@ class ActionCachingTestController < CachingController caches_action :show, :cache_path => 'http://test.host/custom/show' caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" } caches_action :with_layout + caches_action :with_content_for, :layout => false caches_action :layout_false, :layout => false caches_action :record_not_found, :four_oh_four, :simple_runtime_error @@ -174,6 +175,11 @@ class ActionCachingTestController < CachingController render :text => @cache_this end + def with_content_for + @first_time = MockTime.now.to_f.to_s + @second_time = MockTime.now.to_f.to_s + end + def redirected redirect_to :action => 'index' end @@ -323,6 +329,20 @@ class ActionCacheTest < ActionController::TestCase assert_equal cached_time, body end + def test_action_cache_layout_false_and_content_for + get :with_content_for + first_time = assigns(:first_time) + second_time = assigns(:second_time) + assert @response.body.include?(first_time) + assert @response.body.include?(second_time) + assert fragment_exist?('hostname.com/action_caching_test/with_content_for') + reset! + + get :with_content_for + assert @response.body.include?(first_time) + assert @response.body.include?(second_time) + end + def test_action_cache_conditional_options @request.env['HTTP_ACCEPT'] = 'application/json' get :index @@ -722,14 +742,35 @@ This bit's fragment cached 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')[:layout] + end + + def test_fragment_caching_with_content_for + get :fragment_cached_with_content_for + assert_response :success + expected_body = <<-CACHED +Hello +This bit's fragment cached +CACHED + assert_equal expected_body, @response.body + + assert_equal "This bit's fragment cached\n", @store.read('views/test.host/functional_caching/fragment_cached_with_content_for')[:layout] + assert_equal ": Enhanced", @store.read('views/test.host/functional_caching/fragment_cached_with_content_for')[:title] + assert_equal "Fancy caching", @store.read('views/test.host/functional_caching/fragment_cached_with_content_for')[:widget] + setup + + get :fragment_cached_with_content_for + assert_response :success + assert_equal expected_body, @response.body + expected_hash = {:title => ": Enhanced", :widget => "Fancy caching", :layout => "This bit's fragment cached\n"} + assert_equal expected_hash, @store.read('views/test.host/functional_caching/fragment_cached_with_content_for') 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')[:layout] end def test_render_inline_before_fragment_caching @@ -737,14 +778,14 @@ 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')[:layout]) end def test_fragment_caching_in_rjs_partials xhr :get, :js_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/js_fragment_cached_with_partial')) + assert_match("Old fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial')[:layout]) end def test_html_formatted_fragment_caching @@ -754,7 +795,7 @@ CACHED 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')[:layout] end def test_xml_formatted_fragment_caching @@ -764,6 +805,6 @@ CACHED 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')[:layout] end end diff --git a/actionpack/test/fixtures/action_caching_test/with_content_for.html.erb b/actionpack/test/fixtures/action_caching_test/with_content_for.html.erb new file mode 100644 index 0000000..7dc0798 --- /dev/null +++ b/actionpack/test/fixtures/action_caching_test/with_content_for.html.erb @@ -0,0 +1,2 @@ +<%= @first_time %><% content_for :second_time, @second_time %> +<%= content_for :second_time %> diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached_with_content_for.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached_with_content_for.html.erb new file mode 100644 index 0000000..3776f8f --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_with_content_for.html.erb @@ -0,0 +1,6 @@ +Hello<%- content_for :title, "My Awesome Title!" -%> +<%- cache do -%> + <%- content_for :title, ": Enhanced" -%> + <%- content_for :widget, "Fancy caching" -%> +This bit's fragment cached +<%- end -%> diff --git a/actionpack/test/fixtures/layouts/talk_from_action.erb b/actionpack/test/fixtures/layouts/talk_from_action.erb index bf53fdb..935a3f0 100644 --- a/actionpack/test/fixtures/layouts/talk_from_action.erb +++ b/actionpack/test/fixtures/layouts/talk_from_action.erb @@ -1,2 +1,2 @@ <%= @title || yield(:title) %> -<%= yield -%> \ No newline at end of file +<%= yield -%> diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb index 0305048..574567b 100644 --- a/actionpack/test/template/capture_helper_test.rb +++ b/actionpack/test/template/capture_helper_test.rb @@ -117,6 +117,35 @@ class CaptureHelperTest < ActionView::TestCase end end + def test_content_to_cache_without_subset_only_with_layout + content_for :layout, 'body text' + assert_equal Hash.new, content_to_cache + end + + def test_content_to_cache_without_subset + content_for :foo, 'body text' + assert_equal @_content_for, content_to_cache + assert_not_equal content_to_cache.object_id, @_content_for + end + + def test_content_to_cache_with_subset + @_content_for = {:fizz => "bang"} + @_subset_content_for = {:foo => "bar"} + assert content_to_cache == {:foo => "bar"} + end + + def test_cache_with_content_for + content_for(:uncached, "content") + + cache_with_content_for do + assert_not_nil @_subset_content_for + content_for(:foo, "bar") + expected_hash = {:foo => "bar"} + assert_equal expected_hash, content_to_cache + end + assert_nil @_subset_content_for + end + def alt_encoding(output_buffer) output_buffer.encoding == Encoding::US_ASCII ? Encoding::UTF_8 : Encoding::US_ASCII end -- 1.7.3.2