Skip to content

Commit

Permalink
Fix GET JSON integration test request to use method override
Browse files Browse the repository at this point in the history
When a `GET` request is sent `as: :json` in an integration test the test
should use Rack's method override to change to a post request so the
paramters are included in the postdata. Otherwise it will not encode the
parameters correctly for the integration test.

Because integration test sets up it's own middleware,
`Rack::MethodOverride` needs to be included in the integration tests as
well.

`headers ||= {}` was moved so that headers are never nil. They should
default to a hash.

Fixes #26033

[Eileen M. Uchitelle & Aaron Patterson]
  • Loading branch information
eileencodes committed Aug 5, 2016
1 parent 70f2f98 commit af1680f
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
7 changes: 6 additions & 1 deletion actionpack/lib/action_dispatch/testing/integration.rb
Expand Up @@ -327,6 +327,12 @@ def non_kwarg_request_warning
# Performs the actual request. # Performs the actual request.
def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil) def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil)
request_encoder = RequestEncoder.encoder(as) request_encoder = RequestEncoder.encoder(as)
headers ||= {}

if method == :get && as == :json && params
headers['X-Http-Method-Override'] = 'GET'
method = :post
end


if path =~ %r{://} if path =~ %r{://}
path = build_expanded_path(path, request_encoder) do |location| path = build_expanded_path(path, request_encoder) do |location|
Expand Down Expand Up @@ -361,7 +367,6 @@ def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: n
} }


if xhr if xhr
headers ||= {}
headers['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' headers['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'
headers['HTTP_ACCEPT'] ||= [Mime[:js], Mime[:html], Mime[:xml], 'text/xml', '*/*'].join(', ') headers['HTTP_ACCEPT'] ||= [Mime[:js], Mime[:html], Mime[:xml], 'text/xml', '*/*'].join(', ')
end end
Expand Down
1 change: 1 addition & 0 deletions actionpack/test/abstract_unit.rb
Expand Up @@ -104,6 +104,7 @@ def self.build_app(routes = nil)
middleware.use ActionDispatch::Callbacks middleware.use ActionDispatch::Callbacks
middleware.use ActionDispatch::Cookies middleware.use ActionDispatch::Cookies
middleware.use ActionDispatch::Flash middleware.use ActionDispatch::Flash
middleware.use Rack::MethodOverride
middleware.use Rack::Head middleware.use Rack::Head
yield(middleware) if block_given? yield(middleware) if block_given?
end end
Expand Down
16 changes: 16 additions & 0 deletions actionpack/test/controller/integration_test.rb
Expand Up @@ -1238,6 +1238,22 @@ def test_get_parameters_with_as_option
end end
end end


def test_get_request_with_json_uses_method_override_and_sends_a_post_request
with_routing do |routes|
routes.draw do
ActiveSupport::Deprecation.silence do
get ':action' => FooController
end
end

get '/foos_json', params: { foo: 'heyo' }, as: :json

assert_equal 'POST', request.method
assert_equal 'GET', request.headers['X-Http-Method-Override']
assert_equal({ 'foo' => 'heyo' }, response.parsed_body)
end
end

private private
def post_to_foos(as:) def post_to_foos(as:)
with_routing do |routes| with_routing do |routes|
Expand Down

2 comments on commit af1680f

@kaspth
Copy link
Contributor

@kaspth kaspth commented on af1680f Aug 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileencodes @tenderlove heads up: since this is a bug in the 5.0 series I backported this to 5-0-stable as well: 922a066.

@eileencodes
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @kaspth 👍

Please sign in to comment.