From 47bc138fc1d9ddafab8e4cf9cac8865f2092c003 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 11 Mar 2010 17:32:26 -0800 Subject: Write strings to fragment cache, not outputbuffers --- actionpack/lib/action_controller/caching/fragments.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 00a7f034d3..bb5ff95a67 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -41,7 +41,9 @@ module ActionController #:nodoc: else pos = buffer.length block.call - write_fragment(name, buffer[pos..-1], options) + content = buffer[pos..-1] + content = content.as_str if content.respond_to?(:as_str) + write_fragment(name, content, options) end else block.call -- cgit v1.2.3 From 16572fd46e189d80c6be7d499e687fe129053a2c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 14 Mar 2010 18:55:13 -0700 Subject: read_ and write_fragment cache preserve html safety yet cache strings only --- actionpack/lib/action_controller/caching/fragments.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index bb5ff95a67..841e64ecaf 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -41,9 +41,7 @@ module ActionController #:nodoc: else pos = buffer.length block.call - content = buffer[pos..-1] - content = content.as_str if content.respond_to?(:as_str) - write_fragment(name, content, options) + write_fragment(name, buffer[pos..-1], options) end else block.call @@ -53,9 +51,10 @@ 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? - key = fragment_cache_key(key) + key = fragment_cache_key(key) instrument_fragment_cache :write_fragment, key do + content = content.html_safe.as_str if content.respond_to?(:html_safe) cache_store.write(key, content, options) end content @@ -64,10 +63,11 @@ module ActionController #:nodoc: # Reads a cached fragment from the location signified by key (see expire_fragment for acceptable formats) def read_fragment(key, options = nil) return unless cache_configured? - key = fragment_cache_key(key) + key = fragment_cache_key(key) instrument_fragment_cache :read_fragment, key do - cache_store.read(key, options) + result = cache_store.read(key, options) + result.respond_to?(:html_safe) ? result.html_safe : result end end -- cgit v1.2.3 From c937da9e2fc14f74fb11d1ce605479c033ca29ee Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 11:17:18 -0700 Subject: to_str works here --- actionpack/lib/action_controller/caching/fragments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 841e64ecaf..89787727bd 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -54,7 +54,7 @@ module ActionController #:nodoc: key = fragment_cache_key(key) instrument_fragment_cache :write_fragment, key do - content = content.html_safe.as_str if content.respond_to?(:html_safe) + content = content.html_safe.to_str if content.respond_to?(:html_safe) cache_store.write(key, content, options) end content -- cgit v1.2.3 From 9de83050d3a4b260d4aeb5d09ec4eb64f913ba64 Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Mon, 15 Mar 2010 11:58:05 -0700 Subject: Add deprecation notices for <% %>. * The approach is to compile <% %> into a method call that checks whether the value returned from a block is a String. If it is, it concats to the buffer and prints a deprecation warning. * <%= %> uses exactly the same logic to compile the template, which first checks to see whether it's compiling a block. * This should have no impact on other uses of block in templates. For instance, in <% [1,2,3].each do |i| %><%= i %><% end %>, the call to each returns an Array, not a String, so the result is not concatenated * In two cases (#capture and #cache), a String can be returned that should *never* be concatenated. We have temporarily created a String subclass called NonConcattingString which behaves (and is serialized) identically to String, but is not concatenated by the code that handles deprecated <% %> block helpers. Once we remove support for <% %> block helpers, we can remove NonConcattingString. --- actionpack/lib/action_controller/caching/fragments.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 89787727bd..a07fe2b255 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -34,17 +34,23 @@ module ActionController #:nodoc: ActiveSupport::Cache.expand_cache_key(key.is_a?(Hash) ? url_for(key).split("://").last : key, :views) end - def fragment_for(buffer, name = {}, options = nil, &block) #:nodoc: + def fragment_for(name = {}, options = nil, &block) #:nodoc: if perform_caching if fragment_exist?(name, options) - buffer.safe_concat(read_fragment(name, options)) + read_fragment(name, options) else + # VIEW TODO: Make #capture usable outside of ERB + # This dance is needed because Builder can't use capture + buffer = view_context.output_buffer pos = buffer.length - block.call - write_fragment(name, buffer[pos..-1], options) + yield + fragment = buffer[pos..-1] + write_fragment(name, fragment, options) + ActionView::NonConcattingString.new(fragment) end else - block.call + ret = yield + ActiveSupport::SafeBuffer.new(ret) if ret.is_a?(String) end end -- cgit v1.2.3 From 748c78ffc8b11a608745290e7d68bdf63720cfab Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Mar 2010 23:48:32 -0700 Subject: RJS may cache an array --- actionpack/lib/action_controller/caching/fragments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index a07fe2b255..19bf3ddd3b 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -46,7 +46,7 @@ module ActionController #:nodoc: yield fragment = buffer[pos..-1] write_fragment(name, fragment, options) - ActionView::NonConcattingString.new(fragment) + fragment.is_a?(String) ? ActionView::NonConcattingString.new(fragment) : fragment end else ret = yield -- cgit v1.2.3 From c61ed70b00c93bdf42c7538a334f07e58c60bc4e Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Tue, 16 Mar 2010 11:43:04 -0700 Subject: Some more tweaks on <% %>. * The cache helper is now semantically "mark this region for caching" * As a result, <% x = cache do %> no longer works --- actionpack/lib/action_controller/caching/fragments.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 19bf3ddd3b..8a10bdfe23 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -44,9 +44,8 @@ module ActionController #:nodoc: buffer = view_context.output_buffer pos = buffer.length yield - fragment = buffer[pos..-1] + fragment = buffer.slice!(pos..-1) write_fragment(name, fragment, options) - fragment.is_a?(String) ? ActionView::NonConcattingString.new(fragment) : fragment end else ret = yield -- cgit v1.2.3 From 3deb60e6b492b5b944e7e6ead3531ad4e412f56d Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Thu, 18 Mar 2010 13:45:50 -0700 Subject: @layout is a confusing name... use @cache_layout --- actionpack/lib/action_controller/caching/actions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 35111a4b92..3e5240a0ef 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -112,7 +112,7 @@ module ActionController #:nodoc: class ActionCacheFilter #:nodoc: def initialize(options, &block) - @cache_path, @store_options, @layout = + @cache_path, @store_options, @cache_layout = options.values_at(:cache_path, :store_options, :layout) end @@ -126,10 +126,10 @@ module ActionController #:nodoc: cache_path = ActionCachePath.new(controller, path_options || {}) if cache = controller.read_fragment(cache_path.path, @store_options) - controller._render_cache_fragment(cache, cache_path.extension, @layout == false) + controller._render_cache_fragment(cache, cache_path.extension, @cache_layout == false) else yield - controller._save_fragment(cache_path.path, @layout == false, @store_options) + controller._save_fragment(cache_path.path, @cache_layout == false, @store_options) end end end -- cgit v1.2.3 From 523d0f3700f5bb68cdd3d549eaad63d8a88c2aef Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Thu, 18 Mar 2010 14:55:13 -0700 Subject: Remove caching's dependency on view_context. Also, make it so that the layout is always rendered the same way (so that layout dependencies on the action actually being rendered aren't masked on the first render) --- .../lib/action_controller/caching/actions.rb | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 3e5240a0ef..e89bf64026 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -80,6 +80,7 @@ module ActionController #:nodoc: def caches_action(*actions) return unless cache_configured? options = actions.extract_options! + options[:layout] = true unless options.key?(:layout) filter_options = options.extract!(:if, :unless).merge(:only => actions) cache_options = options.extract!(:layout, :cache_path).merge(:store_options => options) @@ -87,14 +88,10 @@ module ActionController #:nodoc: end end - def _render_cache_fragment(cache, extension, layout) - render :text => cache, :layout => layout, :content_type => Mime[extension || :html] - end - - def _save_fragment(name, layout, options) + def _save_fragment(name, options) return unless caching_allowed? - content = layout ? view_context.content_for(:layout) : response_body + content = response_body write_fragment(name, content, options) end @@ -125,12 +122,19 @@ module ActionController #:nodoc: cache_path = ActionCachePath.new(controller, path_options || {}) - if cache = controller.read_fragment(cache_path.path, @store_options) - controller._render_cache_fragment(cache, cache_path.extension, @cache_layout == false) - else + body = controller.read_fragment(cache_path.path, @store_options) + + unless body + controller.action_has_layout = false unless @cache_layout yield - controller._save_fragment(cache_path.path, @cache_layout == false, @store_options) + controller.action_has_layout = true + body = controller._save_fragment(cache_path.path, @store_options) end + + body = controller.render_to_string(:text => cache, :layout => true) unless @cache_layout + + controller.response_body = body + controller.content_type = Mime[cache_path.extension || :html] end end -- cgit v1.2.3 From 71c9337f45f9c5461cbc6ddf6cab764ad0f82c3b Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Thu, 18 Mar 2010 15:52:43 -0700 Subject: All tests pass without memoizing view_context --- actionpack/lib/action_controller/caching/fragments.rb | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 8a10bdfe23..473a2fe214 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -34,25 +34,6 @@ module ActionController #:nodoc: ActiveSupport::Cache.expand_cache_key(key.is_a?(Hash) ? url_for(key).split("://").last : key, :views) end - def fragment_for(name = {}, options = nil, &block) #:nodoc: - if perform_caching - if fragment_exist?(name, options) - read_fragment(name, options) - else - # VIEW TODO: Make #capture usable outside of ERB - # This dance is needed because Builder can't use capture - buffer = view_context.output_buffer - pos = buffer.length - yield - fragment = buffer.slice!(pos..-1) - write_fragment(name, fragment, options) - end - else - ret = yield - ActiveSupport::SafeBuffer.new(ret) if ret.is_a?(String) - end - end - # 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? -- cgit v1.2.3 From f868c2afa9190604015b6c5fd3bcc58371f03570 Mon Sep 17 00:00:00 2001 From: wycats Date: Fri, 19 Mar 2010 18:56:06 -0700 Subject: response_body is an Array in 1.9, so an Array was being pushed onto the cache --- actionpack/lib/action_controller/caching/actions.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'actionpack/lib/action_controller/caching') diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index e89bf64026..43ddf6435a 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -92,6 +92,8 @@ module ActionController #:nodoc: return unless caching_allowed? content = response_body + content = content.join if content.is_a?(Array) + write_fragment(name, content, options) end -- cgit v1.2.3