Improve performance of the rendering stack by freezing formats as a sign that they shouldn't be further modified.

3 files changed, 43 insertions, 19 deletions

diff --git a/actionpack/test/controller/new_base/render_rjs_test.rb b/actionpack/test/controller/new_base/render_rjs_test.rb
index 7c8f1cb869..b602b9f8e9 100644
--- a/actionpack/test/controller/new_base/render_rjs_test.rb
+++ b/actionpack/test/controller/new_base/render_rjs_test.rb
@@ -5,7 +5,7 @@ module RenderRjs
     self.view_paths = [ActionView::FixtureResolver.new(
       "render_rjs/basic/index.js.rjs"          => "page[:customer].replace_html render(:partial => 'customer')",
       "render_rjs/basic/index_html.js.rjs"     => "page[:customer].replace_html :partial => 'customer'",
-      "render_rjs/basic/index_no_js.js.rjs"    => "page[:developer].replace_html render(:partial => 'developer')",
+      "render_rjs/basic/index_no_js.js.erb"    => "<%= render(:partial => 'developer') %>",
       "render_rjs/basic/_customer.js.erb"      => "JS Partial",
       "render_rjs/basic/_customer.html.erb"    => "HTML Partial",
       "render_rjs/basic/_developer.html.erb"   => "HTML Partial",
@@ -18,6 +18,12 @@ module RenderRjs
       render
     end
 
+    def index_respond_to
+      respond_to do |format|
+        format.js { render :action => "index_no_js" }
+      end
+    end
+
     def index_locale
       self.locale = :da
     end
@@ -41,7 +47,12 @@ module RenderRjs
 
     test "rendering a partial in an RJS template should pick the HTML one if no JS is available" do
       get :index_no_js, "format" => "js"
-      assert_response("$(\"developer\").update(\"HTML Partial\");")
+      assert_response("HTML Partial")
+    end
+
+    test "rendering a partial in an RJS template should pick the HTML one if no JS is available on respond_to" do
+      get :index_respond_to, "format" => "js"
+      assert_response("HTML Partial")
     end
 
     test "replacing an element with a partial in an RJS template should pick the HTML template over the JS one" do
diff --git a/actionpack/test/lib/fixture_template.rb b/actionpack/test/lib/fixture_template.rb
index 74c0918a14..b49ccd39ca 100644
--- a/actionpack/test/lib/fixture_template.rb
+++ b/actionpack/test/lib/fixture_template.rb
@@ -9,7 +9,7 @@ module ActionView #:nodoc:
 
   private
 
-    def query(path, exts)
+    def query(path, exts, formats)
       query = Regexp.escape(path)
       exts.each do |ext|
         query << '(' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
@@ -17,7 +17,7 @@ module ActionView #:nodoc:
 
       templates = []
       @hash.select { |k,v| k =~ /^#{query}$/ }.each do |path, source|
-        handler, format = extract_handler_and_format(path)
+        handler, format = extract_handler_and_format(path, formats)
         templates << Template.new(source, path, handler,
           :virtual_path => path, :format => format)
       end
diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb
index 37526957bc..df1aa2edb2 100644
--- a/actionpack/test/template/lookup_context_test.rb
+++ b/actionpack/test/template/lookup_context_test.rb
@@ -22,34 +22,35 @@ class LookupContextTest < ActiveSupport::TestCase
   end
 
   test "normalizes details on initialization" do
-    formats = Mime::SET
-    locale  = [I18n.locale]
-    assert_equal Hash[:formats => formats, :locale => locale], @lookup_context.details
-  end
-
-  test "allows me to set details" do
-    @lookup_context.details = { :formats => [:html], :locale => :pt }
-    assert_equal Hash[:formats => [:html], :locale => [:pt]], @lookup_context.details
+    assert_equal Mime::SET, @lookup_context.formats
+    assert_equal :en, @lookup_context.locale
   end
 
-  test "does not allow details to be modified in place" do
-    assert @lookup_context.details.frozen?
+  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
-    formats = Mime::SET
-    locale  = [I18n.locale]
-    assert_equal Hash[:formats => formats, :locale => locale], @lookup_context.details
+    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?
   end
 
   test "allows me to change some details to execute an specific block of code" do
     formats = Mime::SET
     @lookup_context.update_details(:locale => :pt) do
-      assert_equal Hash[:formats => formats, :locale => [:pt]], @lookup_context.details
+      assert_equal formats, @lookup_context.formats
+      assert_equal :pt, @lookup_context.locale
     end
-    assert_equal Hash[:formats => formats, :locale => [:en]], @lookup_context.details
+    assert_equal formats, @lookup_context.formats
+    assert_equal :en, @lookup_context.locale
   end
 
   test "provides getters and setters for formats" do
@@ -62,6 +63,11 @@ class LookupContextTest < ActiveSupport::TestCase
     assert_equal Mime::SET, @lookup_context.formats
   end
 
+  test "adds :html fallback to :js formats" do
+    @lookup_context.formats = [:js]
+    assert_equal [:js, :html], @lookup_context.formats
+  end
+
   test "provides getters and setters for locale" do
     @lookup_context.locale = :pt
     assert_equal :pt, @lookup_context.locale
@@ -94,6 +100,13 @@ class LookupContextTest < ActiveSupport::TestCase
     assert_equal "Hey verden", template.source
   end
 
+  test "found templates respects given formats if one cannot be found from template or handler" do
+    ActionView::Template::Handlers::ERB.expects(:default_format).returns(nil)
+    @lookup_context.formats = [:text]
+    template = @lookup_context.find("hello_world", "test")
+    assert_equal [:text], template.formats
+  end
+
   test "adds fallbacks to view paths when required" do
     assert_equal 1, @lookup_context.view_paths.size