All tests pass without memoizing view_context

11 files changed, 60 insertions, 41 deletions

diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb
index 16664098e5..eec7e520fa 100644
--- a/actionpack/lib/abstract_controller/rendering.rb
+++ b/actionpack/lib/abstract_controller/rendering.rb
@@ -32,7 +32,6 @@ module AbstractController
   module Rendering
     extend ActiveSupport::Concern
 
-    include AbstractController::Assigns
     include AbstractController::ViewPaths
 
     # Overwrite process to setup I18n proxy.
@@ -53,7 +52,8 @@ module AbstractController
     #
     # Override this method in a module to change the default behavior.
     def view_context
-      @_view_context ||= ActionView::Base.for_controller(self)
+      klass = ActionView::Base.for_controller(self)
+      klass.new(lookup_context, view_assigns, self)
     end
 
     # Normalize arguments, options and then delegates render_to_body and
@@ -82,7 +82,6 @@ module AbstractController
     # Find and renders a template based on the options given.
     # :api: private
     def _render_template(options) #:nodoc:
-      _evaluate_assigns(view_context)
       view_context.render(options)
     end
 
@@ -105,6 +104,17 @@ module AbstractController
 
   private
 
+    # This method should return a hash with assigns.
+    # You can overwrite this configuration per controller.
+    # :api: public
+    def view_assigns
+      hash = {}
+      variables  = instance_variable_names
+      variables -= protected_instance_variables if respond_to?(:protected_instance_variables)
+      variables.each { |name| hash[name.to_s[1..-1]] = instance_variable_get(name) }
+      hash
+    end
+
     # Normalize options by converting render "foo" to render :action => "foo" and
     # render "foo/bar" to render :file => "foo/bar".
     def _normalize_args(action=nil, options={})
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 <tt>content</tt> to the location signified by <tt>key</tt> (see <tt>expire_fragment</tt> for acceptable formats)
       def write_fragment(key, content, options = nil)
         return content unless cache_configured?
diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb
index 08325b468c..d906e1fb5b 100644
--- a/actionpack/lib/action_controller/metal/renderers.rb
+++ b/actionpack/lib/action_controller/metal/renderers.rb
@@ -87,7 +87,7 @@ module ActionController
     end
 
     add :update do |proc, options|
-      _evaluate_assigns(view_context)
+      view_context = self.view_context
       generator = ActionView::Helpers::PrototypeHelper::JavaScriptGenerator.new(view_context, &proc)
       self.content_type  = Mime::JS
       self.response_body = generator.to_s
diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
index eb28cd5107..de833bb3ca 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
@@ -154,15 +154,14 @@ module ActionDispatch
         # TODO: Make this unnecessary
         if @controller
           @controller.singleton_class.send(:include, @router.url_helpers)
-          @controller.class._helper_serial += 1
-          @controller.view_context.singleton_class.send(:include, @router.url_helpers)
+          @controller.class._helper_serial = AbstractController::Helpers.next_serial + 1
         end
         yield @router
       ensure
         @router = old_routes
         if @controller
           @controller = old_controller
-          @controller.class._helper_serial += 1 if @controller
+          @controller.class._helper_serial = AbstractController::Helpers.next_serial + 1 if @controller
         end
       end
 
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 859c5c0863..b82c6a8203 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -250,8 +250,6 @@ module ActionView #:nodoc:
       else
         klass = self
       end
-
-      klass.new(controller.lookup_context, {}, controller)
     end
 
     def initialize(lookup_context = nil, assigns_for_first_render = {}, controller = nil, formats = nil) #:nodoc:
diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb
index f5c2127d3f..a904af56bb 100644
--- a/actionpack/lib/action_view/helpers/cache_helper.rb
+++ b/actionpack/lib/action_view/helpers/cache_helper.rb
@@ -32,9 +32,29 @@ module ActionView
       #      <i>Topics listed alphabetically</i>
       #    <% end %>
       def cache(name = {}, options = nil, &block)
-        safe_concat controller.fragment_for(name, options, &block)
+        safe_concat fragment_for(name, options, &block)
         nil
       end
+
+    private
+      # TODO: Create an object that has caching read/write on it
+      def fragment_for(name = {}, options = nil, &block) #:nodoc:
+        if controller.perform_caching
+          if controller.fragment_exist?(name, options)
+            controller.read_fragment(name, options)
+          else
+            # VIEW TODO: Make #capture usable outside of ERB
+            # This dance is needed because Builder can't use capture
+            pos = output_buffer.length
+            yield
+            fragment = output_buffer.slice!(pos..-1)
+            controller.write_fragment(name, fragment, options)
+          end
+        else
+          ret = yield
+          ActiveSupport::SafeBuffer.new(ret) if ret.is_a?(String)
+        end
+      end
     end
   end
 end
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index f6264c0954..5157454be0 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -616,8 +616,10 @@ class FragmentCachingTest < ActionController::TestCase
     @store.write('views/expensive', 'fragment content')
     fragment_computed = false
 
+    view_context = @controller.view_context
+
     buffer = 'generated till now -> '.html_safe
-    buffer << @controller.fragment_for('expensive') { fragment_computed = true }
+    buffer << view_context.send(:fragment_for, 'expensive') { fragment_computed = true }
 
     assert fragment_computed
     assert_equal 'generated till now -> ', buffer
@@ -627,8 +629,10 @@ class FragmentCachingTest < ActionController::TestCase
     @store.write('views/expensive', 'fragment content')
     fragment_computed = false
 
+    view_context = @controller.view_context
+
     buffer = 'generated till now -> '.html_safe
-    buffer << @controller.fragment_for('expensive') { fragment_computed = true }
+    buffer << view_context.send(:fragment_for, 'expensive') { fragment_computed = true }
 
     assert !fragment_computed
     assert_equal 'generated till now -> fragment content', buffer
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 1d7692f0a8..24817a93d2 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -18,6 +18,13 @@ class TestController < ActionController::Base
 
   layout :determine_layout
 
+  def name
+    nil
+  end
+
+  private :name
+  helper_method :name
+
   def hello_world
   end
 
@@ -418,7 +425,6 @@ class TestController < ActionController::Base
 
   def rendering_with_conflicting_local_vars
     @name = "David"
-    def view_context.name() nil end
     render :action => "potential_conflicts"
   end
 
diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb
index 2216e6b578..bf541c17d3 100644
--- a/actionpack/test/template/capture_helper_test.rb
+++ b/actionpack/test/template/capture_helper_test.rb
@@ -114,7 +114,7 @@ class CaptureHelperTest < ActionView::TestCase
   end
 
   def view_with_controller
-    returning(ActionView::Base.for_controller(TestController.new)) do |view|
+    returning(TestController.new.view_context) do |view|
       view.output_buffer = ActionView::OutputBuffer.new
     end
   end
diff --git a/actionpack/test/template/output_buffer_test.rb b/actionpack/test/template/output_buffer_test.rb
index 9016b74489..bd49a11af1 100644
--- a/actionpack/test/template/output_buffer_test.rb
+++ b/actionpack/test/template/output_buffer_test.rb
@@ -10,6 +10,7 @@ class OutputBufferTest < ActionController::TestCase
   tests TestController
 
   def setup
+    @vc = @controller.view_context
     get :index
     assert_equal ['foo'], body_parts
   end
@@ -25,15 +26,15 @@ class OutputBufferTest < ActionController::TestCase
   end
 
   test 'flushing ignores empty output buffer' do
-    @controller.view_context.output_buffer = ''
-    @controller.view_context.flush_output_buffer
+    @vc.output_buffer = ''
+    @vc.flush_output_buffer
     assert_equal '', output_buffer
     assert_equal ['foo'], body_parts
   end
 
   test 'flushing appends the output buffer to the body parts' do
-    @controller.view_context.output_buffer = 'bar'
-    @controller.view_context.flush_output_buffer
+    @vc.output_buffer = 'bar'
+    @vc.flush_output_buffer
     assert_equal '', output_buffer
     assert_equal ['foo', 'bar'], body_parts
   end
@@ -41,8 +42,8 @@ class OutputBufferTest < ActionController::TestCase
   if '1.9'.respond_to?(:force_encoding)
     test 'flushing preserves output buffer encoding' do
       original_buffer = ' '.force_encoding(Encoding::EUC_JP)
-      @controller.view_context.output_buffer = original_buffer
-      @controller.view_context.flush_output_buffer
+      @vc.output_buffer = original_buffer
+      @vc.flush_output_buffer
       assert_equal ['foo', original_buffer], body_parts
       assert_not_equal original_buffer, output_buffer
       assert_equal Encoding::EUC_JP, output_buffer.encoding
@@ -51,7 +52,7 @@ class OutputBufferTest < ActionController::TestCase
 
   protected
     def output_buffer
-      @controller.view_context.output_buffer
+      @vc.output_buffer
     end
 
     def body_parts
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
index e66202e8fe..e54ebfbf8d 100644
--- a/actionpack/test/template/render_test.rb
+++ b/actionpack/test/template/render_test.rb
@@ -9,7 +9,7 @@ module RenderTestCases
   def setup_view(paths)
     @assigns = { :secret => 'in the sauce' }
     @view = ActionView::Base.new(paths, @assigns)
-    @controller_view = ActionView::Base.for_controller(TestController.new)
+    @controller_view = TestController.new.view_context
 
     # Reload and register danish language for testing
     I18n.reload!