From b19999f3a7c39fc1a959cc3c39575014d0fd2835 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Sun, 1 Feb 2015 16:07:42 +0300 Subject: Migrating xhr methods to keyword arguments syntax in `ActionController::TestCase` and `ActionDispatch::Integration` Old syntax: `xhr :get, :create, params: { id: 1 }` New syntax example: `get :create, params: { id: 1 }, xhr: true` --- actionpack/CHANGELOG.md | 13 ++ actionpack/lib/action_controller/test_case.rb | 21 ++- .../lib/action_dispatch/testing/integration.rb | 24 ++-- actionpack/test/controller/integration_test.rb | 155 ++++++++++----------- actionpack/test/controller/mime/respond_to_test.rb | 20 +-- actionpack/test/controller/render_js_test.rb | 4 +- actionpack/test/controller/render_json_test.rb | 4 +- .../controller/request_forgery_protection_test.rb | 14 +- actionpack/test/controller/test_case_test.rb | 21 ++- 9 files changed, 159 insertions(+), 117 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index aeb961decc..89395f5b52 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,16 @@ +* Migrating xhr methods to keyword arguments syntax + in `ActionController::TestCase` and `ActionDispatch::Integration` + + Old syntax: + + xhr :get, :create, params: { id: 1 } + + New syntax example: + + get :create, params: { id: 1 }, xhr: true + + *Kir Shatrov* + * Migrating to keyword arguments syntax in `ActionController::TestCase` and `ActionDispatch::Integration` HTTP request methods diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 15e587bcea..a15a4bfab1 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -546,8 +546,13 @@ module ActionController end def xml_http_request(*args) + ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) + xhr and xml_http_request methods are deprecated in favor of + `get :index, xhr: true` and `post :create, xhr: true` + MSG + @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') __send__(*args).tap do @request.env.delete 'HTTP_X_REQUESTED_WITH' @request.env.delete 'HTTP_ACCEPT' @@ -599,7 +604,7 @@ module ActionController check_required_ivars if kwarg_request?(*args) - parameters, session, body, flash, http_method, format = args[0].values_at(:params, :session, :body, :flash, :method, :format) + parameters, session, body, flash, http_method, format, xhr = args[0].values_at(:params, :session, :body, :flash, :method, :format, :xhr) else http_method, parameters, session, flash = args format = nil @@ -655,6 +660,11 @@ module ActionController @request.session.update(session) if session @request.flash.update(flash || {}) + if xhr + @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' + @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + end + @controller.request = @request @controller.response = @response @@ -678,6 +688,11 @@ module ActionController @request.session['flash'] = flash_value end + if xhr + @request.env.delete 'HTTP_X_REQUESTED_WITH' + @request.env.delete 'HTTP_ACCEPT' + end + @response end @@ -738,7 +753,7 @@ module ActionController end end - REQUEST_KWARGS = %i(params session flash method body) + REQUEST_KWARGS = %i(params session flash method body xhr) def kwarg_request?(*args) args[0].respond_to?(:keys) && ( (args[0].key?(:format) && args[0].keys.size == 1) || diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index db6233840a..876a980ff1 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -92,11 +92,12 @@ module ActionDispatch end end - headers ||= {} - headers.merge!(env) if env - headers['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - headers['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') - process(request_method, path, params: params, headers: headers) + ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) + xhr and xml_http_request methods are deprecated in favor of + `get "/posts", xhr: true` and `post "/posts/1", xhr: true` + MSG + + process(request_method, path, params: params, headers: headers, xhr: true) end alias xhr :xml_http_request @@ -306,7 +307,7 @@ module ActionDispatch end end - REQUEST_KWARGS = %i(params headers env) + REQUEST_KWARGS = %i(params headers env xhr) def kwarg_request?(*args) args[0].respond_to?(:keys) && args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } end @@ -329,7 +330,7 @@ module ActionDispatch end # Performs the actual request. - def process(method, path, params: nil, headers: nil, env: nil) + def process(method, path, params: nil, headers: nil, env: nil, xhr: false) if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -354,6 +355,13 @@ module ActionDispatch "CONTENT_TYPE" => "application/x-www-form-urlencoded", "HTTP_ACCEPT" => accept } + + if xhr + headers ||= {} + headers['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' + headers['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + end + # this modifies the passed request_env directly if headers.present? Http::Headers.new(request_env).merge!(headers) @@ -377,7 +385,7 @@ module ActionDispatch @controller = session.last_request.env['action_controller.instance'] - return response.status + response.status end def build_full_uri(path, env) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 2139cf2661..ec9f50ca2b 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -195,133 +195,112 @@ class SessionTest < ActiveSupport::TestCase def test_xml_http_request_get path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:get, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:get, path, params: params, headers: headers) + @session.expects(:process).with(:get, path, params: params, headers: headers, xhr: true) + @session.get(path, params: params, headers: headers, xhr: true) end def test_deprecated_xml_http_request_get path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:get, path, params: params, headers: headers_after_xhr) - assert_deprecated { - @session.xml_http_request(:get,path,params,headers) + @session.expects(:process).with(:get, path, params: params, headers: headers, xhr: true) + @session.get(path, params: params, headers: headers, xhr: true) + end + + def test_deprecated_args_xml_http_request_get + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:get, path, params: params, headers: headers, xhr: true) + assert_deprecated(/xml_http_request/) { + @session.xml_http_request(:get, path, params, headers) } end def test_xml_http_request_post path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:post, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:post, path, params: params, headers: headers) + @session.expects(:process).with(:post, path, params: params, headers: headers, xhr: true) + @session.post(path, params: params, headers: headers, xhr: true) end def test_deprecated_xml_http_request_post path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:post, path, params: params, headers: headers_after_xhr) - assert_deprecated { @session.xml_http_request(:post,path,params,headers) } + @session.expects(:process).with(:post, path, params: params, headers: headers, xhr: true) + @session.post(path, params: params, headers: headers, xhr: true) + end + + def test_deprecated_args_xml_http_request_post + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:post, path, params: params, headers: headers, xhr: true) + assert_deprecated(/xml_http_request/) { @session.xml_http_request(:post,path,params,headers) } end def test_xml_http_request_patch path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:patch, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:patch, path, params: params, headers: headers) + @session.expects(:process).with(:patch, path, params: params, headers: headers, xhr: true) + @session.patch(path, params: params, headers: headers, xhr: true) end def test_deprecated_xml_http_request_patch path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:patch, path, params: params, headers: headers_after_xhr) - assert_deprecated { @session.xml_http_request(:patch,path,params,headers) } + @session.expects(:process).with(:patch, path, params: params, headers: headers, xhr: true) + @session.patch(path, params: params, headers: headers, xhr: true) + end + + def test_deprecated_args_xml_http_request_patch + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:patch, path, params: params, headers: headers, xhr: true) + assert_deprecated(/xml_http_request/) { @session.xml_http_request(:patch,path,params,headers) } end def test_xml_http_request_put path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:put, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:put, path, params: params, headers: headers) + @session.expects(:process).with(:put, path, params: params, headers: headers, xhr: true) + @session.put(path, params: params, headers: headers, xhr: true) end def test_deprecated_xml_http_request_put path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:put, path, params: params, headers: headers_after_xhr) - assert_deprecated { @session.xml_http_request(:put, path, params, headers) } + @session.expects(:process).with(:put, path, params: params, headers: headers, xhr: true) + @session.put(path, params: params, headers: headers, xhr: true) + end + + def test_deprecated_args_xml_http_request_put + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:put, path, params: params, headers: headers, xhr: true) + assert_deprecated(/xml_http_request/) { @session.xml_http_request(:put, path, params, headers) } end def test_xml_http_request_delete path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:delete, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:delete, path, params: params, headers: headers) + @session.expects(:process).with(:delete, path, params: params, headers: headers, xhr: true) + @session.delete(path, params: params, headers: headers, xhr: true) end def test_deprecated_xml_http_request_delete path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:delete, path, params: params, headers: headers_after_xhr) - assert_deprecated { @session.xml_http_request(:delete, path, params, headers) } + @session.expects(:process).with(:delete, path, params: params, headers: headers, xhr: true) + assert_deprecated { @session.xml_http_request(:delete, path, params: params, headers: headers) } + end + + def test_deprecated_args_xml_http_request_delete + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:delete, path, params: params, headers: headers, xhr: true) + assert_deprecated(/xml_http_request/) { @session.xml_http_request(:delete, path, params, headers) } end def test_xml_http_request_head path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:head, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:head, path, params: params, headers: headers) + @session.expects(:process).with(:head, path, params: params, headers: headers, xhr: true) + @session.head(path, params: params, headers: headers, xhr: true) end def test_deprecated_xml_http_request_head path = "/index"; params = "blah"; headers = { location: 'blah' } - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => "text/javascript, text/html, application/xml, text/xml, */*" - ) - @session.expects(:process).with(:head, path, params: params, headers: headers_after_xhr) - assert_deprecated { @session.xml_http_request(:head, path, params, headers) } + @session.expects(:process).with(:head, path, params: params, headers: headers, xhr: true) + assert_deprecated(/xml_http_request/) { @session.xml_http_request(:head, path, params: params, headers: headers) } end - def test_xml_http_request_override_accept - path = "/index"; params = "blah"; headers = {:location => 'blah', "HTTP_ACCEPT" => "application/xml"} - headers_after_xhr = headers.merge( - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" - ) - @session.expects(:process).with(:post, path, params: params, headers: headers_after_xhr) - @session.xml_http_request(:post, path, params: params, headers: headers) + def test_deprecated_args_xml_http_request_head + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:head, path, params: params, headers: headers, xhr: true) + assert_deprecated { @session.xml_http_request(:head, path, params, headers) } end end @@ -526,7 +505,19 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest def test_xml_http_request_get with_test_route_set do - xhr :get, '/get' + get '/get', xhr: true + assert_equal 200, status + assert_equal "OK", status_message + assert_response 200 + assert_response :success + assert_response :ok + assert_equal "JS OK", response.body + end + end + + def test_deprecated_xml_http_request_get + with_test_route_set do + assert_deprecated { xhr :get, '/get' } assert_equal 200, status assert_equal "OK", status_message assert_response 200 @@ -538,7 +529,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest def test_request_with_bad_format with_test_route_set do - xhr :get, '/get.php' + get '/get.php', xhr: true assert_equal 406, status assert_response 406 assert_response :not_acceptable diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 8bd32f9584..1f5f66dc80 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -310,17 +310,17 @@ class RespondToControllerTest < ActionController::TestCase def test_js_or_html @request.accept = "text/javascript, text/html" - xhr :get, :js_or_html + get :js_or_html, xhr: true assert_equal 'JS', @response.body @request.accept = "text/javascript, text/html" - xhr :get, :html_or_xml + get :html_or_xml, xhr: true assert_equal 'HTML', @response.body @request.accept = "text/javascript, text/html" assert_raises(ActionController::UnknownFormat) do - xhr :get, :just_xml + get :just_xml, xhr: true end end @@ -335,7 +335,7 @@ class RespondToControllerTest < ActionController::TestCase end def test_json_or_yaml - xhr :get, :json_or_yaml + get :json_or_yaml, xhr: true assert_equal 'JSON', @response.body get :json_or_yaml, format: 'json' @@ -357,13 +357,13 @@ class RespondToControllerTest < ActionController::TestCase def test_js_or_anything @request.accept = "text/javascript, */*" - xhr :get, :js_or_html + get :js_or_html, xhr: true assert_equal 'JS', @response.body - xhr :get, :html_or_xml + get :html_or_xml, xhr: true assert_equal 'HTML', @response.body - xhr :get, :just_xml + get :just_xml, xhr: true assert_equal 'XML', @response.body end @@ -408,14 +408,14 @@ class RespondToControllerTest < ActionController::TestCase def test_with_atom_content_type @request.accept = "" @request.env["CONTENT_TYPE"] = "application/atom+xml" - xhr :get, :made_for_content_type + get :made_for_content_type, xhr: true assert_equal "ATOM", @response.body end def test_with_rss_content_type @request.accept = "" @request.env["CONTENT_TYPE"] = "application/rss+xml" - xhr :get, :made_for_content_type + get :made_for_content_type, xhr: true assert_equal "RSS", @response.body end @@ -525,7 +525,7 @@ class RespondToControllerTest < ActionController::TestCase end def test_xhr - xhr :get, :js_or_html + get :js_or_html, xhr: true assert_equal 'JS', @response.body end diff --git a/actionpack/test/controller/render_js_test.rb b/actionpack/test/controller/render_js_test.rb index d482df195f..6b661de064 100644 --- a/actionpack/test/controller/render_js_test.rb +++ b/actionpack/test/controller/render_js_test.rb @@ -22,13 +22,13 @@ class RenderJSTest < ActionController::TestCase tests TestController def test_render_vanilla_js - xhr :get, :render_vanilla_js_hello + get :render_vanilla_js_hello, xhr: true assert_equal "alert('hello')", @response.body assert_equal "text/javascript", @response.content_type end def test_should_render_js_partial - xhr :get, :show_partial, format: 'js' + get :show_partial, format: 'js', xhr: true assert_equal 'partial js', @response.body end end diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index ada978aa11..b1ad16bc55 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -100,13 +100,13 @@ class RenderJsonTest < ActionController::TestCase end def test_render_json_with_callback - xhr :get, :render_json_hello_world_with_callback + get :render_json_hello_world_with_callback, xhr: true assert_equal '/**/alert({"hello":"world"})', @response.body assert_equal 'text/javascript', @response.content_type end def test_render_json_with_custom_content_type - xhr :get, :render_json_with_custom_content_type + get :render_json_with_custom_content_type, xhr: true assert_equal '{"hello":"world"}', @response.body assert_equal 'text/javascript', @response.content_type end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 8a0eed5a87..88155bb404 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -262,7 +262,7 @@ module RequestForgeryProtectionTests end def test_should_not_allow_xhr_post_without_token - assert_blocked { xhr :post, :index } + assert_blocked { post :index, xhr: true } end def test_should_allow_post_with_token @@ -340,11 +340,11 @@ module RequestForgeryProtectionTests get :negotiate_same_origin end - assert_cross_origin_not_blocked { xhr :get, :same_origin_js } - assert_cross_origin_not_blocked { xhr :get, :same_origin_js, format: 'js'} + assert_cross_origin_not_blocked { get :same_origin_js, xhr: true } + assert_cross_origin_not_blocked { get :same_origin_js, xhr: true, format: 'js'} assert_cross_origin_not_blocked do @request.accept = 'text/javascript' - xhr :get, :negotiate_same_origin + get :negotiate_same_origin, xhr: true end end @@ -366,11 +366,11 @@ module RequestForgeryProtectionTests get :negotiate_cross_origin end - assert_cross_origin_not_blocked { xhr :get, :cross_origin_js } - assert_cross_origin_not_blocked { xhr :get, :cross_origin_js, format: 'js' } + assert_cross_origin_not_blocked { get :cross_origin_js, xhr: true } + assert_cross_origin_not_blocked { get :cross_origin_js, xhr: true, format: 'js' } assert_cross_origin_not_blocked do @request.accept = 'text/javascript' - xhr :get, :negotiate_cross_origin + get :negotiate_cross_origin, xhr: true end end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 901db63e39..02a757ad75 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -694,19 +694,34 @@ XML end def test_header_properly_reset_after_remote_http_request - xhr :get, :test_params + get :test_params, xhr: true assert_nil @request.env['HTTP_X_REQUESTED_WITH'] assert_nil @request.env['HTTP_ACCEPT'] end + def test_deprecated_xhr_with_params + assert_deprecated { xhr :get, :test_params, params: { id: 1 } } + + assert_equal(%({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}), @response.body) + end + def test_xhr_with_params - xhr :get, :test_params, params: { id: 1 } + get :test_params, params: { id: 1 }, xhr: true assert_equal(%({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}), @response.body) end def test_xhr_with_session - xhr :get, :set_session + get :set_session, xhr: true + + assert_equal 'A wonder', session['string'], "A value stored in the session should be available by string key" + assert_equal 'A wonder', session[:string], "Test session hash should allow indifferent access" + assert_equal 'it works', session['symbol'], "Test session hash should allow indifferent access" + assert_equal 'it works', session[:symbol], "Test session hash should allow indifferent access" + end + + def test_deprecated_xhr_with_session + assert_deprecated { xhr :get, :set_session } assert_equal 'A wonder', session['string'], "A value stored in the session should be available by string key" assert_equal 'A wonder', session[:string], "Test session hash should allow indifferent access" -- cgit v1.2.3