Make AP test suite green once again and speed up performance in layouts lookup for some cases.

4 files changed, 39 insertions, 45 deletions

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