From 5273bd97e6746bd5bdc2450ad5a2d60f6a6eb7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 7 Jun 2010 10:13:41 +0200 Subject: Make AP test suite green once again and speed up performance in layouts lookup for some cases. --- actionpack/lib/action_view/base.rb | 2 +- actionpack/lib/action_view/lookup_context.rb | 51 ++++++++++++++++--------- actionpack/lib/action_view/render/layouts.rb | 15 ++------ actionpack/test/template/lookup_context_test.rb | 16 +------- 4 files changed, 39 insertions(+), 45 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 16d7d25279..7dd9dea358 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -184,7 +184,7 @@ module ActionView #:nodoc: attr_internal :captures, :request, :controller, :template, :config delegate :find_template, :template_exists?, :formats, :formats=, :locale, :locale=, - :view_paths, :view_paths=, :with_fallbacks, :update_details, :to => :lookup_context + :view_paths, :view_paths=, :with_fallbacks, :update_details, :with_layout_format, :to => :lookup_context delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, :flash, :action_name, :controller_name, :to => :controller diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 801b08b19d..3aaa5e401c 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -19,6 +19,7 @@ module ActionView def self.register_detail(name, options = {}, &block) self.registered_details << name self.registered_detail_setters << [name, "#{name}="] + Accessors.send :define_method, :"_#{name}_defaults", &block Accessors.module_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{name} @@ -27,12 +28,7 @@ module ActionView def #{name}=(value) value = Array.wrap(value.presence || _#{name}_defaults) - - if value != @details[:#{name}] - @details_key = nil - @details = @details.dup if @details.frozen? - @details[:#{name}] = value.freeze - end + _set_detail(:#{name}, value) if value != @details[:#{name}] end METHOD end @@ -63,8 +59,11 @@ module ActionView def initialize(view_paths, details = {}) @details, @details_key = { :handlers => default_handlers }, nil @frozen_formats, @skip_default_locale = false, false + self.view_paths = view_paths - self.initialize_details(details) + self.registered_detail_setters.each do |key, setter| + send(setter, details[key]) + end end module ViewPaths @@ -177,11 +176,20 @@ module ActionView super(@skip_default_locale ? I18n.locale : _locale_defaults) end - def initialize_details(details) - details = details.dup - - registered_detail_setters.each do |key, setter| - send(setter, details[key]) + # A method which only uses the first format in the formats array for layout lookup. + # This method plays straight with instance variables for performance reasons. + def with_layout_format + if formats.size == 1 + yield + else + old_formats = formats + _set_detail(:formats, formats[0,1]) + + begin + yield + ensure + _set_detail(:formats, formats) + end end end @@ -195,14 +203,21 @@ module ActionView send(setter, new_details[key]) if new_details.key?(key) end - if block_given? - begin - yield - ensure - @details = old_details - end + begin + yield + ensure + @details_key = nil + @details = old_details end end + + protected + + def _set_detail(key, value) + @details_key = nil + @details = @details.dup if @details.frozen? + @details[key] = value.freeze + end end include Accessors diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb index 255c44450d..a9dfc0cced 100644 --- a/actionpack/lib/action_view/render/layouts.rb +++ b/actionpack/lib/action_view/render/layouts.rb @@ -57,15 +57,11 @@ module ActionView # This is the method which actually finds the layout using details in the lookup # context object. If no layout is found, it checkes if at least a layout with # the given name exists across all details before raising the error. - # - # If self.formats contains several formats, just the first one is considered in - # the layout lookup. def find_layout(layout) begin - if formats.size == 1 - _find_layout(layout) - else - update_details(:formats => [self.formats.first]) { _find_layout(layout) } + with_layout_format do + layout =~ /^\// ? + with_fallbacks { find_template(layout) } : find_template(layout) end rescue ActionView::MissingTemplate => e update_details(:formats => nil) do @@ -74,11 +70,6 @@ module ActionView end end - def _find_layout(layout) #:nodoc: - layout =~ /^\// ? - with_fallbacks { find_template(layout) } : find_template(layout) - end - # Contains the logic that actually renders the layout. def _render_layout(layout, locals, &block) #:nodoc: layout.render(self, locals){ |*name| _layout_for(*name, &block) } diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index df1aa2edb2..cc71cb42d0 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -26,18 +26,6 @@ class LookupContextTest < ActiveSupport::TestCase assert_equal :en, @lookup_context.locale end - test "allows me to update details" do - @lookup_context.update_details(:formats => [:html], :locale => :pt) - assert_equal [:html], @lookup_context.formats - assert_equal :pt, @lookup_context.locale - end - - test "allows me to update an specific detail" do - @lookup_context.update_details(:locale => :pt) - assert_equal :pt, I18n.locale - assert_equal :pt, @lookup_context.locale - end - test "allows me to freeze and retrieve frozen formats" do @lookup_context.formats.freeze assert @lookup_context.formats.frozen? @@ -54,7 +42,7 @@ class LookupContextTest < ActiveSupport::TestCase end test "provides getters and setters for formats" do - @lookup_context.formats = :html + @lookup_context.formats = [:html] assert_equal [:html], @lookup_context.formats end @@ -138,7 +126,7 @@ class LookupContextTest < ActiveSupport::TestCase keys << @lookup_context.details_key assert_equal 2, keys.uniq.size - @lookup_context.formats = :html + @lookup_context.formats = [:html] keys << @lookup_context.details_key assert_equal 3, keys.uniq.size -- cgit v1.2.3