From a09e99259c688a839bd5b4635da6f119dd1e0e41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 27 Mar 2010 20:51:25 +0100 Subject: [PATCH] Ensure details are frozen after @details_keys lookup. The implementation waits to freeze until the last required moment, to avoid duping hashes. --- actionmailer/lib/action_mailer/base.rb | 2 +- actionmailer/lib/action_mailer/collector.rb | 3 +-- .../action_controller/metal/mime_responds.rb | 3 +-- actionpack/lib/action_view/lookup_context.rb | 21 +++++++++++++++---- .../lib/action_view/render/rendering.rb | 11 +--------- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 1dc2d92c4318a..0519783d02e39 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -591,7 +591,7 @@ def collect_responses_and_parts_order(headers) #:nodoc: responses, parts_order = [], nil if block_given? - collector = ActionMailer::Collector.new(self) { render(action_name) } + collector = ActionMailer::Collector.new(lookup_context) { render(action_name) } yield(collector) parts_order = collector.responses.map { |r| r[:content_type] } responses = collector.responses diff --git a/actionmailer/lib/action_mailer/collector.rb b/actionmailer/lib/action_mailer/collector.rb index bd4f76bbb67cb..d03e085e83a98 100644 --- a/actionmailer/lib/action_mailer/collector.rb +++ b/actionmailer/lib/action_mailer/collector.rb @@ -22,8 +22,7 @@ def any(*args, &block) def custom(mime, options={}) options.reverse_merge!(:content_type => mime.to_s) - @context.formats = [mime.to_sym] - @context.formats.freeze + @context.freeze_formats([mime.to_sym]) options[:body] = block_given? ? yield : @default_render.call @responses << options end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index fa69ab2038b72..4f384d1ec5bed 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -262,8 +262,7 @@ def retrieve_response_from_mimes(mimes=nil, &block) if format = request.negotiate_mime(collector.order) self.content_type ||= format.to_s - self.formats = [format.to_sym] - self.formats.freeze + lookup_context.freeze_formats([format.to_sym]) collector.response_for(format) else head :not_acceptable diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 10f1911e31030..9b59aac0eb6f3 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -23,9 +23,12 @@ def #{name} def #{name}=(value) value = Array.wrap(value.presence || _#{name}_defaults) - @details_key = nil unless value == @details[:#{name}] - # Always set the value to handle frozen arrays - @details[:#{name}] = value + + if value != @details[:#{name}] + @details_key = nil + @details = @details.dup if @details.frozen? + @details[:#{name}] = value.freeze + end end METHOD end @@ -45,7 +48,7 @@ class DetailsKey #:nodoc: @details_keys = Hash.new def self.get(details) - @details_keys[details] ||= new + @details_keys[details.freeze] ||= new end def initialize @@ -55,6 +58,7 @@ def initialize def initialize(view_paths, details = {}) @details, @details_key = { :handlers => default_handlers }, nil + @frozen_formats = false self.view_paths = view_paths self.update_details(details, true) end @@ -127,6 +131,15 @@ def details_key #:nodoc: @details_key ||= DetailsKey.get(@details) end + # Freeze the current formats in the lookup context. By freezing them, you are guaranteeing + # that next template lookups are not going to modify the formats. The controller can also + # use this, to ensure that formats won't be further modified (as it does in respond_to blocks). + def freeze_formats(formats, unless_frozen=false) #:nodoc: + return if unless_frozen && @frozen_formats + self.formats = formats + @frozen_formats = true + end + # Overload formats= to reject [:"*/*"] values. def formats=(value) value = nil if value == [:"*/*"] diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index a8f745afd1541..492326964a965 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -21,7 +21,7 @@ def render(options = {}, locals = {}, &block) _render_partial(options) else template = _determine_template(options) - _freeze_formats(template.formats) + lookup_context.freeze_formats(template.formats, true) _render_template(template, options[:layout], options) end when :update @@ -62,14 +62,5 @@ def _render_template(template, layout = nil, options = {}) #:nodoc: content end end - - # Freeze the current formats in the lookup context. By freezing them, you are guaranteeing - # that next template lookups are not going to modify the formats. The controller can also - # use this, to ensure that formats won't be further modified (as it does in respond_to blocks). - def _freeze_formats(formats) #:nodoc: - return if self.formats.frozen? - self.formats = formats - self.formats.freeze - end end end