Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix controller test not resetting @_url_options

Commit 4f2cd3e introduced a bug by reordering the call to
`@controller.recycle!` above the call to `build_request_uri`. The
impact of this was that the `@_url_options` cache ends up not being
reset between building a request URI (occurring within the test
controller) and the firing of the actual request.

We encountered this bug because we had the following setup:

  class MinimumReproducibleController < ActionController::Base
    before_filter { @param = 'param' }

    def index
      render text: url_for(params)
    end

    def default_url_options
      { custom_opt: @param }
    end
  end

  def test_index
    get :index # builds url, then fires actual request
  end

The first step in  `get :index` in the test suite would populate the
@_url_options cache. The subsequent call to `url_for` inside of the
controller action would then utilize the uncleared cache, thus never
calling the now-updated default_url_options.

This commit fixes this bug calling recycle! twice, and removes a call
to set response_body, which should no longer be needed since we're
recycling the request object explicitly.
  • Loading branch information...
commit a351149e805910cd980bee1558e56e61c4a82db2 1 parent 3225898
Tony Wooster twooster authored twooster committed
5 actionpack/CHANGELOG.md
View
@@ -1 +1,6 @@
+* Fix URL generation in controller tests with request-dependent
+ `default_url_options` methods.
+
+ *Tony Wooster*
+
Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionpack/CHANGELOG.md) for previous changes.
1  actionpack/lib/action_controller/metal/testing.rb
View
@@ -17,7 +17,6 @@ def set_response!(request)
def recycle!
@_url_options = nil
- self.response_body = nil
self.formats = nil
self.params = nil
end
1  actionpack/lib/action_controller/test_case.rb
View
@@ -568,6 +568,7 @@ def process(action, http_method = 'GET', *args)
name = @request.parameters[:action]
+ @controller.recycle!
@controller.process(name)
if cookies = @request.env['action_dispatch.cookies']
23 actionpack/test/controller/test_case_test.rb
View
@@ -163,6 +163,29 @@ def view_assigns
end
end
+ class DefaultUrlOptionsCachingController < ActionController::Base
+ before_filter { @dynamic_opt = 'opt' }
+
+ def test_url_options_reset
+ render text: url_for(params)
+ end
+
+ def default_url_options
+ if defined?(@dynamic_opt)
+ super.merge dynamic_opt: @dynamic_opt
+ else
+ super
+ end
+ end
+ end
+
+ def test_url_options_reset
+ @controller = DefaultUrlOptionsCachingController.new
+ get :test_url_options_reset
+ assert_nil @request.params['dynamic_opt']
+ assert_match(/dynamic_opt=opt/, @response.body)
+ end
+
def test_raw_post_handling
params = Hash[:page, {:name => 'page name'}, 'some key', 123]
post :render_raw_post, params.dup
Please sign in to comment.
Something went wrong with that request. Please try again.