diff options
107 files changed, 1511 insertions, 589 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d545c59afc..aeb961decc 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,28 @@ +* Migrating to keyword arguments syntax in `ActionController::TestCase` and + `ActionDispatch::Integration` HTTP request methods + + New syntax example: + + post :create, params: { y: x }, session: { a: 'b' } + get :view, params: { id: 1 } + get :view, params: { id: 1 }, format: :json + + *Kir Shatrov* + +* Preserve default url options when generating URLs + + Fixes an issue that would cause default_url_options to be lost when + generating URLs with fewer positional arguments than parameters in the + route definition. + + *Tekin Suleyman* + +* Deprecate *_via_redirect integration test methods. + + Use `follow_redirect!` manually after the request call for the same behavior. + + *Aditya Kapoor* + * Add `ActionController::Renderer` to render arbitrary templates outside controller actions. diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index a219d35b25..20afcee537 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -106,11 +106,11 @@ module ActionController end def auth_scheme(request) - request.authorization.split(' ', 2).first + request.authorization.to_s.split(' ', 2).first end def auth_param(request) - request.authorization.split(' ', 2).second + request.authorization.to_s.split(' ', 2).second end def encode_credentials(user_name, password) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index d30615fade..15e587bcea 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -494,55 +494,61 @@ module ActionController # Simulate a GET request with the given parameters. # # - +action+: The controller action to call. - # - +parameters+: The HTTP parameters that you want to pass. This may - # be +nil+, a hash, or a string that is appropriately encoded + # - +params+: The hash with HTTP parameters that you want to pass. This may be +nil+. + # - +body+: The request body with a string that is appropriately encoded # (<tt>application/x-www-form-urlencoded</tt> or <tt>multipart/form-data</tt>). # - +session+: A hash of parameters to store in the session. This may be +nil+. # - +flash+: A hash of parameters to store in the flash. This may be +nil+. # # You can also simulate POST, PATCH, PUT, DELETE, and HEAD requests with # +post+, +patch+, +put+, +delete+, and +head+. + # Example sending parameters, session and setting a flash message: + # + # get :show, + # params: { id: 7 }, + # session: { user_id: 1 }, + # flash: { notice: 'This is flash message' } # # Note that the request method is not verified. The different methods are # available to make the tests more expressive. def get(action, *args) - process(action, "GET", *args) + process_with_kwargs("GET", action, *args) end # Simulate a POST request with the given parameters and set/volley the response. # See +get+ for more details. def post(action, *args) - process(action, "POST", *args) + process_with_kwargs("POST", action, *args) end # Simulate a PATCH request with the given parameters and set/volley the response. # See +get+ for more details. def patch(action, *args) - process(action, "PATCH", *args) + process_with_kwargs("PATCH", action, *args) end # Simulate a PUT request with the given parameters and set/volley the response. # See +get+ for more details. def put(action, *args) - process(action, "PUT", *args) + process_with_kwargs("PUT", action, *args) end # Simulate a DELETE request with the given parameters and set/volley the response. # See +get+ for more details. def delete(action, *args) - process(action, "DELETE", *args) + process_with_kwargs("DELETE", action, *args) end # Simulate a HEAD request with the given parameters and set/volley the response. # See +get+ for more details. def head(action, *args) - process(action, "HEAD", *args) + process_with_kwargs("HEAD", action, *args) end - def xml_http_request(request_method, action, parameters = nil, session = nil, flash = nil) + def xml_http_request(*args) @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') - __send__(request_method, action, parameters, session, flash).tap do + __send__(*args).tap do @request.env.delete 'HTTP_X_REQUESTED_WITH' @request.env.delete 'HTTP_ACCEPT' end @@ -566,41 +572,68 @@ module ActionController # parameters and set/volley the response. # # - +action+: The controller action to call. - # - +http_method+: Request method used to send the http request. Possible values - # are +GET+, +POST+, +PATCH+, +PUT+, +DELETE+, +HEAD+. Defaults to +GET+. - # - +parameters+: The HTTP parameters. This may be +nil+, a hash, or a - # string that is appropriately encoded (+application/x-www-form-urlencoded+ - # or +multipart/form-data+). + # - +method+: Request method used to send the HTTP request. Possible values + # are +GET+, +POST+, +PATCH+, +PUT+, +DELETE+, +HEAD+. Defaults to +GET+. Can be a symbol. + # - +params+: The hash with HTTP parameters that you want to pass. This may be +nil+. + # - +body+: The request body with a string that is appropriately encoded + # (<tt>application/x-www-form-urlencoded</tt> or <tt>multipart/form-data</tt>). # - +session+: A hash of parameters to store in the session. This may be +nil+. # - +flash+: A hash of parameters to store in the flash. This may be +nil+. # # Example calling +create+ action and sending two params: # - # process :create, 'POST', user: { name: 'Gaurish Sharma', email: 'user@example.com' } - # - # Example sending parameters, +nil+ session and setting a flash message: - # - # process :view, 'GET', { id: 7 }, nil, { notice: 'This is flash message' } + # process :create, + # method: 'POST', + # params: { + # user: { name: 'Gaurish Sharma', email: 'user@example.com' } + # }, + # session: { user_id: 1 }, + # flash: { notice: 'This is flash message' } # # To simulate +GET+, +POST+, +PATCH+, +PUT+, +DELETE+ and +HEAD+ requests # prefer using #get, #post, #patch, #put, #delete and #head methods # respectively which will make tests more expressive. # # Note that the request method is not verified. - def process(action, http_method = 'GET', *args) + def process(action, *args) check_required_ivars - if args.first.is_a?(String) && http_method != 'HEAD' - @request.env['RAW_POST_DATA'] = args.shift + if kwarg_request?(*args) + parameters, session, body, flash, http_method, format = args[0].values_at(:params, :session, :body, :flash, :method, :format) + else + http_method, parameters, session, flash = args + format = nil + + if parameters.is_a?(String) && http_method != 'HEAD' + body = parameters + parameters = nil + end + + if parameters.present? || session.present? || flash.present? + non_kwarg_request_warning + end + end + + if body.present? + @request.env['RAW_POST_DATA'] = body + end + + if http_method.present? + http_method = http_method.to_s.upcase + else + http_method = "GET" end - parameters, session, flash = args parameters ||= {} # Ensure that numbers and symbols passed as params are converted to # proper params, as is the case when engaging rack. parameters = paramify_values(parameters) if html_format?(parameters) + if format.present? + parameters[:format] = format + end + @html_document = nil unless @controller.respond_to?(:recycle!) @@ -693,6 +726,38 @@ module ActionController private + def process_with_kwargs(http_method, action, *args) + if kwarg_request?(*args) + args.first.merge!(method: http_method) + process(action, *args) + else + non_kwarg_request_warning if args.present? + + args = args.unshift(http_method) + process(action, *args) + end + end + + REQUEST_KWARGS = %i(params session flash method body) + def kwarg_request?(*args) + args[0].respond_to?(:keys) && ( + (args[0].key?(:format) && args[0].keys.size == 1) || + args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } + ) + end + + def non_kwarg_request_warning + ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) + ActionController::TestCase HTTP request methods will accept only + keyword arguments in future Rails versions. + + Examples: + + get :show, params: { id: 1 }, session: { user_id: 1 } + process :update, http_method: :post, params: { id: 1 } + MSG + end + def document_root_element html_document.root end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index b4c861d306..2b24b896db 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -264,9 +264,10 @@ module ActionDispatch path_params -= controller_options.keys path_params -= result.keys end - path_params.each { |param| - result[param] = inner_options.fetch(param) { args.shift } - } + path_params -= inner_options.keys + path_params.take(args.size).each do |param| + result[param] = args.shift + end end result.merge!(inner_options) @@ -419,7 +420,14 @@ module ActionDispatch # Rails.application.routes.url_helpers.url_for(args) @_routes = routes class << self - delegate :url_for, :optimize_routes_generation?, to: '@_routes' + def url_for(options) + @_routes.url_for(options) + end + + def optimize_routes_generation? + @_routes.optimize_routes_generation? + end + attr_reader :_routes def url_options; {}; end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index f0e2c5becc..db6233840a 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -12,12 +12,14 @@ module ActionDispatch # # - +path+: The URI (as a String) on which you want to perform a GET # request. - # - +parameters+: The HTTP parameters that you want to pass. This may + # - +params+: The HTTP parameters that you want to pass. This may # be +nil+, # a Hash, or a String that is appropriately encoded # (<tt>application/x-www-form-urlencoded</tt> or # <tt>multipart/form-data</tt>). - # - +headers_or_env+: Additional headers to pass, as a Hash. The headers will be + # - +headers+: Additional headers to pass, as a Hash. The headers will be + # merged into the Rack env hash. + # - +env+: Additional env to pass, as a Hash. The headers will be # merged into the Rack env hash. # # This method returns a Response object, which one can use to @@ -28,38 +30,43 @@ module ActionDispatch # # You can also perform POST, PATCH, PUT, DELETE, and HEAD requests with # +#post+, +#patch+, +#put+, +#delete+, and +#head+. - def get(path, parameters = nil, headers_or_env = nil) - process :get, path, parameters, headers_or_env + # + # Example: + # + # get '/feed', params: { since: 201501011400 } + # post '/profile', headers: { "X-Test-Header" => "testvalue" } + def get(path, *args) + process_with_kwargs(:get, path, *args) end # Performs a POST request with the given parameters. See +#get+ for more # details. - def post(path, parameters = nil, headers_or_env = nil) - process :post, path, parameters, headers_or_env + def post(path, *args) + process_with_kwargs(:post, path, *args) end # Performs a PATCH request with the given parameters. See +#get+ for more # details. - def patch(path, parameters = nil, headers_or_env = nil) - process :patch, path, parameters, headers_or_env + def patch(path, *args) + process_with_kwargs(:patch, path, *args) end # Performs a PUT request with the given parameters. See +#get+ for more # details. - def put(path, parameters = nil, headers_or_env = nil) - process :put, path, parameters, headers_or_env + def put(path, *args) + process_with_kwargs(:put, path, *args) end # Performs a DELETE request with the given parameters. See +#get+ for # more details. - def delete(path, parameters = nil, headers_or_env = nil) - process :delete, path, parameters, headers_or_env + def delete(path, *args) + process_with_kwargs(:delete, path, *args) end # Performs a HEAD request with the given parameters. See +#get+ for more # details. - def head(path, parameters = nil, headers_or_env = nil) - process :head, path, parameters, headers_or_env + def head(path, *args) + process_with_kwargs(:head, path, *args) end # Performs an XMLHttpRequest request with the given parameters, mirroring @@ -68,11 +75,28 @@ module ActionDispatch # The request_method is +:get+, +:post+, +:patch+, +:put+, +:delete+ or # +:head+; the parameters are +nil+, a hash, or a url-encoded or multipart # string; the headers are a hash. - def xml_http_request(request_method, path, parameters = nil, headers_or_env = nil) - headers_or_env ||= {} - headers_or_env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - headers_or_env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') - process(request_method, path, parameters, headers_or_env) + # + # Example: + # + # xhr :get, '/feed', params: { since: 201501011400 } + def xml_http_request(request_method, path, *args) + if kwarg_request?(*args) + params, headers, env = args.first.values_at(:params, :headers, :env) + else + params = args[0] + headers = args[1] + env = {} + + if params.present? || headers.present? + non_kwarg_request_warning + 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) end alias xhr :xml_http_request @@ -89,40 +113,52 @@ module ActionDispatch # redirect. Note that the redirects are followed until the response is # not a redirect--this means you may run into an infinite loop if your # redirect loops back to itself. - def request_via_redirect(http_method, path, parameters = nil, headers_or_env = nil) - process(http_method, path, parameters, headers_or_env) + # + # Example: + # + # request_via_redirect :post, '/welcome', + # params: { ref_id: 14 }, + # headers: { "X-Test-Header" => "testvalue" } + def request_via_redirect(http_method, path, *args) + process_with_kwargs(http_method, path, *args) + follow_redirect! while redirect? status end # Performs a GET request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def get_via_redirect(path, parameters = nil, headers_or_env = nil) - request_via_redirect(:get, path, parameters, headers_or_env) + def get_via_redirect(path, *args) + ActiveSupport::Deprecation.warn('`get_via_redirect` is deprecated and will be removed in the next version of Rails. Please use follow_redirect! manually after the request call for the same behavior.') + request_via_redirect(:get, path, *args) end # Performs a POST request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def post_via_redirect(path, parameters = nil, headers_or_env = nil) - request_via_redirect(:post, path, parameters, headers_or_env) + def post_via_redirect(path, *args) + ActiveSupport::Deprecation.warn('`post_via_redirect` is deprecated and will be removed in the next version of Rails. Please use follow_redirect! manually after the request call for the same behavior.') + request_via_redirect(:post, path, *args) end # Performs a PATCH request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def patch_via_redirect(path, parameters = nil, headers_or_env = nil) - request_via_redirect(:patch, path, parameters, headers_or_env) + def patch_via_redirect(path, *args) + ActiveSupport::Deprecation.warn('`patch_via_redirect` is deprecated and will be removed in the next version of Rails. Please use follow_redirect! manually after the request call for the same behavior.') + request_via_redirect(:patch, path, *args) end # Performs a PUT request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def put_via_redirect(path, parameters = nil, headers_or_env = nil) - request_via_redirect(:put, path, parameters, headers_or_env) + def put_via_redirect(path, *args) + ActiveSupport::Deprecation.warn('`put_via_redirect` is deprecated and will be removed in the next version of Rails. Please use follow_redirect! manually after the request call for the same behavior.') + request_via_redirect(:put, path, *args) end # Performs a DELETE request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def delete_via_redirect(path, parameters = nil, headers_or_env = nil) - request_via_redirect(:delete, path, parameters, headers_or_env) + def delete_via_redirect(path, *args) + ActiveSupport::Deprecation.warn('`delete_via_redirect` is deprecated and will be removed in the next version of Rails. Please use follow_redirect! manually after the request call for the same behavior.') + request_via_redirect(:delete, path, *args) end end @@ -261,8 +297,39 @@ module ActionDispatch @_mock_session ||= Rack::MockSession.new(@app, host) end + def process_with_kwargs(http_method, path, *args) + if kwarg_request?(*args) + process(http_method, path, *args) + else + non_kwarg_request_warning if args.present? + process(http_method, path, { params: args[0], headers: args[1] }) + end + end + + REQUEST_KWARGS = %i(params headers env) + def kwarg_request?(*args) + args[0].respond_to?(:keys) && args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } + end + + def non_kwarg_request_warning + ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) + ActionDispatch::Integration::TestCase HTTP request methods will accept only + keyword arguments in future Rails versions. + + Examples: + + get '/profile', + params: { id: 1 }, + headers: { 'X-Extra-Header' => '123' }, + env: { 'action_dispatch.custom' => 'custom' } + + xhr :post, '/profile', + params: { id: 1 } + MSG + end + # Performs the actual request. - def process(method, path, parameters = nil, headers_or_env = nil) + def process(method, path, params: nil, headers: nil, env: nil) if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -272,9 +339,9 @@ module ActionDispatch hostname, port = host.split(':') - env = { + request_env = { :method => method, - :params => parameters, + :params => params, "SERVER_NAME" => hostname, "SERVER_PORT" => port || (https? ? "443" : "80"), @@ -287,14 +354,19 @@ module ActionDispatch "CONTENT_TYPE" => "application/x-www-form-urlencoded", "HTTP_ACCEPT" => accept } - # this modifies the passed env directly - Http::Headers.new(env).merge!(headers_or_env || {}) + # this modifies the passed request_env directly + if headers.present? + Http::Headers.new(request_env).merge!(headers) + end + if env.present? + Http::Headers.new(request_env).merge!(env) + end session = Rack::Test::Session.new(_mock_session) # NOTE: rack-test v0.5 doesn't build a default uri correctly # Make sure requested path is always a full uri - session.request(build_full_uri(path, env), env) + session.request(build_full_uri(path, request_env), request_env) @request_count += 1 @request = ActionDispatch::Request.new(session.last_request.env) diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index d0dfbfbc74..21586e2193 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -378,7 +378,9 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end def test_render_based_on_parameters - process :render_based_on_parameters, "GET", "name" => "David" + process :render_based_on_parameters, + method: "GET", + params: { name: "David" } assert_equal "Mr. David", @response.body end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 001493afc0..f7ad8e5158 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -27,6 +27,16 @@ class DefaultUrlOptionsController < ActionController::Base end end +class OptionalDefaultUrlOptionsController < ActionController::Base + def show + render nothing: true + end + + def default_url_options + { format: 'atom', id: 'default-id' } + end +end + class UrlOptionsController < ActionController::Base def from_view render :inline => "<%= #{params[:route]} %>" @@ -168,7 +178,7 @@ class UrlOptionsTest < ActionController::TestCase get ':controller/:action' end - get :from_view, :route => "from_view_url" + get :from_view, params: { route: "from_view_url" } assert_equal 'http://www.override.com/from_view', @response.body assert_equal 'http://www.override.com/from_view', @controller.send(:from_view_url) @@ -202,7 +212,7 @@ class DefaultUrlOptionsTest < ActionController::TestCase get ':controller/:action' end - get :from_view, :route => "from_view_url" + get :from_view, params: { route: "from_view_url" } assert_equal 'http://www.override.com/from_view?locale=en', @response.body assert_equal 'http://www.override.com/from_view?locale=en', @controller.send(:from_view_url) @@ -219,7 +229,7 @@ class DefaultUrlOptionsTest < ActionController::TestCase get ':controller/:action' end - get :from_view, :route => "description_path(1)" + get :from_view, params: { route: "description_path(1)" } assert_equal '/en/descriptions/1', @response.body assert_equal '/en/descriptions', @controller.send(:descriptions_path) @@ -234,7 +244,18 @@ class DefaultUrlOptionsTest < ActionController::TestCase assert_equal '/en/descriptions/1.xml', @controller.send(:description_path, 1, :format => "xml") end end +end +class OptionalDefaultUrlOptionsControllerTest < ActionController::TestCase + def test_default_url_options_override_missing_positional_arguments + with_routing do |set| + set.draw do + get "/things/:id(.:format)" => 'things#show', :as => :thing + end + assert_equal '/things/1.atom', thing_path('1') + assert_equal '/things/default-id.atom', thing_path + end + end end class EmptyUrlOptionsTest < ActionController::TestCase diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index c0e6a2ebd1..4760ec1698 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -210,7 +210,7 @@ CACHED end def test_skipping_fragment_cache_digesting - get :fragment_cached_without_digest, :format => "html" + get :fragment_cached_without_digest, format: "html" assert_response :success expected_body = "<body>\n<p>ERB</p>\n</body>\n" @@ -244,7 +244,7 @@ CACHED end def test_html_formatted_fragment_caching - get :formatted_fragment_cached, :format => "html" + get :formatted_fragment_cached, format: "html" assert_response :success expected_body = "<body>\n<p>ERB</p>\n</body>\n" @@ -255,7 +255,7 @@ CACHED end def test_xml_formatted_fragment_caching - get :formatted_fragment_cached, :format => "xml" + get :formatted_fragment_cached, format: "xml" assert_response :success expected_body = "<body>\n <p>Builder</p>\n</body>\n" @@ -269,7 +269,7 @@ CACHED def test_fragment_caching_with_variant @request.variant = :phone - get :formatted_fragment_cached_with_variant, :format => "html" + get :formatted_fragment_cached_with_variant, format: "html" assert_response :success expected_body = "<body>\n<p>PHONE</p>\n</body>\n" diff --git a/actionpack/test/controller/default_url_options_with_before_action_test.rb b/actionpack/test/controller/default_url_options_with_before_action_test.rb index 656fd0431e..230f40d7ad 100644 --- a/actionpack/test/controller/default_url_options_with_before_action_test.rb +++ b/actionpack/test/controller/default_url_options_with_before_action_test.rb @@ -1,6 +1,5 @@ require 'abstract_unit' - class ControllerWithBeforeActionAndDefaultUrlOptions < ActionController::Base before_action { I18n.locale = params[:locale] } @@ -23,7 +22,7 @@ class ControllerWithBeforeActionAndDefaultUrlOptionsTest < ActionController::Tes # This test has its roots in issue #1872 test "should redirect with correct locale :de" do - get :redirect, :locale => "de" + get :redirect, params: { locale: "de" } assert_redirected_to "/controller_with_before_action_and_default_url_options/target?locale=de" end end diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 3720a920d0..0ff0a1ef61 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -288,16 +288,16 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest def test_setting_flash_does_not_raise_in_following_requests with_test_route_set do env = { 'action_dispatch.request.flash_hash' => ActionDispatch::Flash::FlashHash.new } - get '/set_flash', nil, env - get '/set_flash', nil, env + get '/set_flash', env: env + get '/set_flash', env: env end end def test_setting_flash_now_does_not_raise_in_following_requests with_test_route_set do env = { 'action_dispatch.request.flash_hash' => ActionDispatch::Flash::FlashHash.new } - get '/set_flash_now', nil, env - get '/set_flash_now', nil, env + get '/set_flash_now', env: env + get '/set_flash_now', env: env end end @@ -312,9 +312,11 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest private # Overwrite get to send SessionSecret in env hash - def get(path, parameters = nil, env = {}) - env["action_dispatch.key_generator"] ||= Generator - super + def get(path, *args) + args[0] ||= {} + args[0][:env] ||= {} + args[0][:env]["action_dispatch.key_generator"] ||= Generator + super(path, *args) end def with_test_route_set diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 00d4612ac9..04222745d9 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -100,7 +100,7 @@ class ForceSSLControllerLevelTest < ActionController::TestCase end def test_banana_redirects_to_https_with_extra_params - get :banana, :token => "secret" + get :banana, params: { token: "secret" } assert_response 301 assert_equal "https://test.host/force_ssl_controller_level/banana?token=secret", redirect_to_url end @@ -273,7 +273,7 @@ class ForceSSLFormatTest < ActionController::TestCase get '/foo', :to => 'force_ssl_controller_level#banana' end - get :banana, :format => :json + get :banana, format: :json assert_response 301 assert_equal 'https://test.host/foo.json', redirect_to_url end @@ -294,7 +294,7 @@ class ForceSSLOptionalSegmentsTest < ActionController::TestCase end @request.env['PATH_INFO'] = '/en/foo' - get :banana, :locale => 'en' + get :banana, params: { locale: 'en' } assert_equal 'en', @controller.params[:locale] assert_response 301 assert_equal 'https://test.host/en/foo', redirect_to_url diff --git a/actionpack/test/controller/http_basic_authentication_test.rb b/actionpack/test/controller/http_basic_authentication_test.rb index 9052fc6962..20962a90cb 100644 --- a/actionpack/test/controller/http_basic_authentication_test.rb +++ b/actionpack/test/controller/http_basic_authentication_test.rb @@ -83,6 +83,13 @@ class HttpBasicAuthenticationTest < ActionController::TestCase assert_response :unauthorized assert_equal "HTTP Basic: Access denied.\n", @response.body, "Authentication didn't fail for request header #{header} and long credentials" end + + test "unsuccessful authentication with #{header.downcase} and no credentials" do + get :show + + assert_response :unauthorized + assert_equal "HTTP Basic: Access denied.\n", @response.body, "Authentication didn't fail for request header #{header} and no credentials" + end end def test_encode_credentials_has_no_newline diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 5c0651bd73..a5b246e7ad 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -32,149 +32,317 @@ class SessionTest < ActiveSupport::TestCase def test_request_via_redirect_uses_given_method path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue"} - @session.expects(:process).with(:put, path, args, headers) + @session.expects(:process).with(:put, path, params: args, headers: headers) @session.stubs(:redirect?).returns(false) - @session.request_via_redirect(:put, path, args, headers) + @session.request_via_redirect(:put, path, params: args, headers: headers) + end + + def test_deprecated_request_via_redirect_uses_given_method + path = "/somepath"; args = { id: '1' }; headers = { "X-Test-Header" => "testvalue" } + @session.expects(:process).with(:put, path, params: args, headers: headers) + @session.stubs(:redirect?).returns(false) + assert_deprecated { @session.request_via_redirect(:put, path, args, headers) } end def test_request_via_redirect_follows_redirects path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue"} @session.stubs(:redirect?).returns(true, true, false) @session.expects(:follow_redirect!).times(2) - @session.request_via_redirect(:get, path, args, headers) + @session.request_via_redirect(:get, path, params: args, headers: headers) end def test_request_via_redirect_returns_status path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue"} @session.stubs(:redirect?).returns(false) @session.stubs(:status).returns(200) - assert_equal 200, @session.request_via_redirect(:get, path, args, headers) + assert_equal 200, @session.request_via_redirect(:get, path, params: args, headers: headers) end def test_get_via_redirect path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue" } + @session.expects(:request_via_redirect).with(:get, path, params: args, headers: headers) + @session.get_via_redirect(path, params: args, headers: headers) + end + + def test_deprecated_get_via_redirect + path = "/somepath"; args = { id: '1' }; headers = { "X-Test-Header" => "testvalue" } @session.expects(:request_via_redirect).with(:get, path, args, headers) - @session.get_via_redirect(path, args, headers) + + assert_deprecated do + @session.get_via_redirect(path, args, headers) + end end def test_post_via_redirect path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue" } + @session.expects(:request_via_redirect).with(:post, path, params: args, headers: headers) + @session.post_via_redirect(path, params: args, headers: headers) + end + + def test_deprecated_post_via_redirect + path = "/somepath"; args = { id: '1' }; headers = { "X-Test-Header" => "testvalue" } @session.expects(:request_via_redirect).with(:post, path, args, headers) - @session.post_via_redirect(path, args, headers) + + assert_deprecated do + @session.post_via_redirect(path, args, headers) + end end def test_patch_via_redirect path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue" } + @session.expects(:request_via_redirect).with(:patch, path, params: args, headers: headers) + @session.patch_via_redirect(path, params: args, headers: headers) + end + + def test_deprecated_patch_via_redirect + path = "/somepath"; args = { id: '1' }; headers = { "X-Test-Header" => "testvalue" } @session.expects(:request_via_redirect).with(:patch, path, args, headers) - @session.patch_via_redirect(path, args, headers) + + assert_deprecated do + @session.patch_via_redirect(path, args, headers) + end end def test_put_via_redirect path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue" } + @session.expects(:request_via_redirect).with(:put, path, params: args, headers: headers) + @session.put_via_redirect(path, params: args, headers: headers) + end + + def test_deprecated_put_via_redirect + path = "/somepath"; args = { id: '1' }; headers = { "X-Test-Header" => "testvalue" } @session.expects(:request_via_redirect).with(:put, path, args, headers) - @session.put_via_redirect(path, args, headers) + + assert_deprecated do + @session.put_via_redirect(path, args, headers) + end end def test_delete_via_redirect path = "/somepath"; args = {:id => '1'}; headers = {"X-Test-Header" => "testvalue" } + @session.expects(:request_via_redirect).with(:delete, path, params: args, headers: headers) + @session.delete_via_redirect(path, params: args, headers: headers) + end + + def test_deprecated_delete_via_redirect + path = "/somepath"; args = { id: '1' }; headers = { "X-Test-Header" => "testvalue" } @session.expects(:request_via_redirect).with(:delete, path, args, headers) - @session.delete_via_redirect(path, args, headers) + + assert_deprecated do + @session.delete_via_redirect(path, args, headers) + end end def test_get - path = "/index"; params = "blah"; headers = {:location => 'blah'} - @session.expects(:process).with(:get,path,params,headers) - @session.get(path,params,headers) + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:get, path, params: params, headers: headers) + @session.get(path, params: params, headers: headers) + end + + def test_get_with_env_and_headers + path = "/index"; params = "blah"; headers = { location: 'blah' }; env = { 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest' } + @session.expects(:process).with(:get, path, params: params, headers: headers, env: env) + @session.get(path, params: params, headers: headers, env: env) + end + + def test_deprecated_get + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:get, path, params: params, headers: headers) + assert_deprecated { + @session.get(path, params, headers) + } end def test_post - path = "/index"; params = "blah"; headers = {:location => 'blah'} - @session.expects(:process).with(:post,path,params,headers) - @session.post(path,params,headers) + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:post, path, params: params, headers: headers) + assert_deprecated { + @session.post(path, params, headers) + } + end + + def test_deprecated_post + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:post, path, params: params, headers: headers) + @session.post(path, params: params, headers: headers) end def test_patch - path = "/index"; params = "blah"; headers = {:location => 'blah'} - @session.expects(:process).with(:patch,path,params,headers) - @session.patch(path,params,headers) + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:patch, path, params: params, headers: headers) + @session.patch(path, params: params, headers: headers) + end + + def test_deprecated_patch + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:patch, path, params: params, headers: headers) + assert_deprecated { + @session.patch(path, params, headers) + } end def test_put - path = "/index"; params = "blah"; headers = {:location => 'blah'} - @session.expects(:process).with(:put,path,params,headers) - @session.put(path,params,headers) + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:put, path, params: params, headers: headers) + @session.put(path, params: params, headers: headers) + end + + def test_deprecated_put + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:put, path, params: params, headers: headers) + assert_deprecated { + @session.put(path, params, headers) + } end def test_delete - path = "/index"; params = "blah"; headers = {:location => 'blah'} - @session.expects(:process).with(:delete,path,params,headers) - @session.delete(path,params,headers) + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:delete, path, params: params, headers: headers) + assert_deprecated { + @session.delete(path,params,headers) + } + end + + def test_deprecated_delete + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:delete, path, params: params, headers: headers) + @session.delete(path, params: params, headers: headers) end def test_head - path = "/index"; params = "blah"; headers = {:location => 'blah'} - @session.expects(:process).with(:head,path,params,headers) - @session.head(path,params,headers) + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:head, path, params: params, headers: headers) + @session.head(path, params: params, headers: headers) + end + + def deprecated_test_head + path = "/index"; params = "blah"; headers = { location: 'blah' } + @session.expects(:process).with(:head, path, params: params, headers: headers) + assert_deprecated { + @session.head(path, params, headers) + } end def test_xml_http_request_get - path = "/index"; params = "blah"; headers = {:location => 'blah'} + 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) + 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,headers_after_xhr) - @session.xml_http_request(:get,path,params,headers) + @session.expects(:process).with(:get, path, params: params, headers: headers_after_xhr) + assert_deprecated { + @session.xml_http_request(:get,path,params,headers) + } end def test_xml_http_request_post - path = "/index"; params = "blah"; headers = {:location => 'blah'} + 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) + 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,headers_after_xhr) - @session.xml_http_request(:post,path,params,headers) + @session.expects(:process).with(:post, path, params: params, headers: headers_after_xhr) + assert_deprecated { @session.xml_http_request(:post,path,params,headers) } end def test_xml_http_request_patch - path = "/index"; params = "blah"; headers = {:location => 'blah'} + 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,headers_after_xhr) - @session.xml_http_request(:patch,path,params,headers) + @session.expects(:process).with(:patch, path, params: params, headers: headers_after_xhr) + @session.xml_http_request(:patch, path, params: params, headers: headers) + 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) } end def test_xml_http_request_put - path = "/index"; params = "blah"; headers = {:location => 'blah'} + 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,headers_after_xhr) - @session.xml_http_request(:put,path,params,headers) + @session.expects(:process).with(:put, path, params: params, headers: headers_after_xhr) + @session.xml_http_request(:put, path, params: params, headers: headers) + 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) } end def test_xml_http_request_delete - path = "/index"; params = "blah"; headers = {:location => 'blah'} + 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) + 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,headers_after_xhr) - @session.xml_http_request(:delete,path,params,headers) + @session.expects(:process).with(:delete, path, params: params, headers: headers_after_xhr) + assert_deprecated { @session.xml_http_request(:delete, path, params, headers) } end def test_xml_http_request_head - path = "/index"; params = "blah"; headers = {:location => 'blah'} + 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,headers_after_xhr) - @session.xml_http_request(:head,path,params,headers) + @session.expects(:process).with(:head, path, params: params, headers: headers_after_xhr) + @session.xml_http_request(:head, path, params: params, headers: headers) + 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) } end def test_xml_http_request_override_accept @@ -182,8 +350,8 @@ class SessionTest < ActiveSupport::TestCase headers_after_xhr = headers.merge( "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" ) - @session.expects(:process).with(:post,path,params,headers_after_xhr) - @session.xml_http_request(:post,path,params,headers) + @session.expects(:process).with(:post, path, params: params, headers: headers_after_xhr) + @session.xml_http_request(:post, path, params: params, headers: headers) end end @@ -423,7 +591,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest def test_get_with_parameters with_test_route_set do - get '/get_with_params', :foo => "bar" + get '/get_with_params', params: { foo: "bar" } assert_equal '/get_with_params', request.env["PATH_INFO"] assert_equal '/get_with_params', request.path_info assert_equal 'foo=bar', request.env["QUERY_STRING"] @@ -588,14 +756,22 @@ class MetalIntegrationTest < ActionDispatch::IntegrationTest end def test_pass_headers - get "/success", {}, "Referer" => "http://www.example.com/foo", "Host" => "http://nohost.com" + get "/success", headers: {"Referer" => "http://www.example.com/foo", "Host" => "http://nohost.com"} assert_equal "http://nohost.com", @request.env["HTTP_HOST"] assert_equal "http://www.example.com/foo", @request.env["HTTP_REFERER"] end + def test_pass_headers_and_env + get "/success", headers: { "X-Test-Header" => "value" }, env: { "HTTP_REFERER" => "http://test.com/", "HTTP_HOST" => "http://test.com" } + + assert_equal "http://test.com", @request.env["HTTP_HOST"] + assert_equal "http://test.com/", @request.env["HTTP_REFERER"] + assert_equal "value", @request.env["HTTP_X_TEST_HEADER"] + end + def test_pass_env - get "/success", {}, "HTTP_REFERER" => "http://test.com/", "HTTP_HOST" => "http://test.com" + get "/success", env: { "HTTP_REFERER" => "http://test.com/", "HTTP_HOST" => "http://test.com" } assert_equal "http://test.com", @request.env["HTTP_HOST"] assert_equal "http://test.com/", @request.env["HTTP_REFERER"] @@ -685,7 +861,7 @@ class ApplicationIntegrationTest < ActionDispatch::IntegrationTest test "process do not modify the env passed as argument" do env = { :SERVER_NAME => 'server', 'action_dispatch.custom' => 'custom' } old_env = env.dup - get '/foo', nil, env + get '/foo', env: env assert_equal old_env, env end end @@ -715,7 +891,7 @@ class EnvironmentFilterIntegrationTest < ActionDispatch::IntegrationTest end test "filters rack request form vars" do - post "/post", :username => 'cjolly', :password => 'secret' + post "/post", params: { username: 'cjolly', password: 'secret' } assert_equal 'cjolly', request.filtered_parameters['username'] assert_equal '[FILTERED]', request.filtered_parameters['password'] diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 864c6ee130..03a4ad7823 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -140,7 +140,7 @@ class ACLogSubscriberTest < ActionController::TestCase end def test_process_action_with_parameters - get :show, :id => '10' + get :show, params: { id: '10' } wait assert_equal 3, logs.size @@ -148,8 +148,8 @@ class ACLogSubscriberTest < ActionController::TestCase end def test_multiple_process_with_parameters - get :show, :id => '10' - get :show, :id => '20' + get :show, params: { id: '10' } + get :show, params: { id: '20' } wait @@ -160,7 +160,7 @@ class ACLogSubscriberTest < ActionController::TestCase def test_process_action_with_wrapped_parameters @request.env['CONTENT_TYPE'] = 'application/json' - post :show, :id => '10', :name => 'jose' + post :show, params: { id: '10', name: 'jose' } wait assert_equal 3, logs.size @@ -186,7 +186,9 @@ class ACLogSubscriberTest < ActionController::TestCase def test_process_action_with_filter_parameters @request.env["action_dispatch.parameter_filter"] = [:lifo, :amount] - get :show, :lifo => 'Pratik', :amount => '420', :step => '1' + get :show, params: { + lifo: 'Pratik', amount: '420', step: '1' + } wait params = logs[1] diff --git a/actionpack/test/controller/mime/accept_format_test.rb b/actionpack/test/controller/mime/accept_format_test.rb index 811c507af2..e20c08da4e 100644 --- a/actionpack/test/controller/mime/accept_format_test.rb +++ b/actionpack/test/controller/mime/accept_format_test.rb @@ -11,7 +11,7 @@ end class StarStarMimeControllerTest < ActionController::TestCase def test_javascript_with_format @request.accept = "text/javascript" - get :index, :format => 'js' + get :index, format: 'js' assert_match "function addition(a,b){ return a+b; }", @response.body end @@ -86,7 +86,7 @@ class MimeControllerLayoutsTest < ActionController::TestCase end def test_non_navigational_format_with_no_template_fallbacks_to_html_template_with_no_layout - get :index, :format => :js + get :index, format: :js assert_equal "Hello Firefox", @response.body end end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 66d2fd7716..8bd32f9584 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -338,10 +338,10 @@ class RespondToControllerTest < ActionController::TestCase xhr :get, :json_or_yaml assert_equal 'JSON', @response.body - get :json_or_yaml, :format => 'json' + get :json_or_yaml, format: 'json' assert_equal 'JSON', @response.body - get :json_or_yaml, :format => 'yaml' + get :json_or_yaml, format: 'yaml' assert_equal 'YAML', @response.body { 'YAML' => %w(text/yaml), @@ -474,7 +474,7 @@ class RespondToControllerTest < ActionController::TestCase end def test_handle_any_any_parameter_format - get :handle_any_any, {:format=>'html'} + get :handle_any_any, format: 'html' assert_equal 'HTML', @response.body end @@ -497,7 +497,7 @@ class RespondToControllerTest < ActionController::TestCase end def test_handle_any_any_unkown_format - get :handle_any_any, { format: 'php' } + get :handle_any_any, format: 'php' assert_equal 'Whatever you ask for, I got it', @response.body end @@ -530,13 +530,13 @@ class RespondToControllerTest < ActionController::TestCase end def test_custom_constant - get :custom_constant_handling, :format => "mobile" + get :custom_constant_handling, format: "mobile" assert_equal "text/x-mobile", @response.content_type assert_equal "Mobile", @response.body end def test_custom_constant_handling_without_block - get :custom_constant_handling_without_block, :format => "mobile" + get :custom_constant_handling_without_block, format: "mobile" assert_equal "text/x-mobile", @response.content_type assert_equal "Mobile", @response.body end @@ -545,13 +545,13 @@ class RespondToControllerTest < ActionController::TestCase get :html_xml_or_rss assert_equal "HTML", @response.body - get :html_xml_or_rss, :format => "html" + get :html_xml_or_rss, format: "html" assert_equal "HTML", @response.body - get :html_xml_or_rss, :format => "xml" + get :html_xml_or_rss, format: "xml" assert_equal "XML", @response.body - get :html_xml_or_rss, :format => "rss" + get :html_xml_or_rss, format: "rss" assert_equal "RSS", @response.body end @@ -559,12 +559,12 @@ class RespondToControllerTest < ActionController::TestCase get :forced_xml assert_equal "XML", @response.body - get :forced_xml, :format => "html" + get :forced_xml, format: "html" assert_equal "XML", @response.body end def test_extension_synonyms - get :html_xml_or_rss, :format => "xhtml" + get :html_xml_or_rss, format: "xhtml" assert_equal "HTML", @response.body end @@ -581,7 +581,7 @@ class RespondToControllerTest < ActionController::TestCase get :using_defaults assert_equal "using_defaults - #{[:html]}", @response.body - get :using_defaults, :format => "xml" + get :using_defaults, format: "xml" assert_equal "using_defaults - #{[:xml]}", @response.body end @@ -589,7 +589,7 @@ class RespondToControllerTest < ActionController::TestCase get :iphone_with_html_response_type assert_equal '<html><div id="html">Hello future from Firefox!</div></html>', @response.body - get :iphone_with_html_response_type, :format => "iphone" + get :iphone_with_html_response_type, format: "iphone" assert_equal "text/html", @response.content_type assert_equal '<html><div id="iphone">Hello iPhone future from iPhone!</div></html>', @response.body end @@ -603,7 +603,7 @@ class RespondToControllerTest < ActionController::TestCase def test_invalid_format assert_raises(ActionController::UnknownFormat) do - get :using_defaults, :format => "invalidformat" + get :using_defaults, format: "invalidformat" end end diff --git a/actionpack/test/controller/new_base/content_negotiation_test.rb b/actionpack/test/controller/new_base/content_negotiation_test.rb index 5fd5946619..c8166280fc 100644 --- a/actionpack/test/controller/new_base/content_negotiation_test.rb +++ b/actionpack/test/controller/new_base/content_negotiation_test.rb @@ -15,12 +15,12 @@ module ContentNegotiation class TestContentNegotiation < Rack::TestCase test "A */* Accept header will return HTML" do - get "/content_negotiation/basic/hello", {}, "HTTP_ACCEPT" => "*/*" + get "/content_negotiation/basic/hello", headers: { "HTTP_ACCEPT" => "*/*" } assert_body "Hello world */*!" end test "Not all mimes are converted to symbol" do - get "/content_negotiation/basic/all", {}, "HTTP_ACCEPT" => "text/plain, mime/another" + get "/content_negotiation/basic/all", headers: { "HTTP_ACCEPT" => "text/plain, mime/another" } assert_body '[:text, "mime/another"]' end end diff --git a/actionpack/test/controller/new_base/content_type_test.rb b/actionpack/test/controller/new_base/content_type_test.rb index 9b57641e75..88453988dd 100644 --- a/actionpack/test/controller/new_base/content_type_test.rb +++ b/actionpack/test/controller/new_base/content_type_test.rb @@ -76,7 +76,7 @@ module ContentType end test "sets Content-Type as application/xml when rendering *.xml.erb" do - get "/content_type/implied/i_am_xml_erb", "format" => "xml" + get "/content_type/implied/i_am_xml_erb", params: { "format" => "xml" } assert_header "Content-Type", "application/xml; charset=utf-8" end @@ -88,7 +88,7 @@ module ContentType end test "sets Content-Type as application/xml when rendering *.xml.builder" do - get "/content_type/implied/i_am_xml_builder", "format" => "xml" + get "/content_type/implied/i_am_xml_builder", params: { "format" => "xml" } assert_header "Content-Type", "application/xml; charset=utf-8" end diff --git a/actionpack/test/controller/new_base/render_action_test.rb b/actionpack/test/controller/new_base/render_action_test.rb index 475bf9d3c9..3bf1dd0ede 100644 --- a/actionpack/test/controller/new_base/render_action_test.rb +++ b/actionpack/test/controller/new_base/render_action_test.rb @@ -88,7 +88,7 @@ module RenderAction test "rendering with layout => true" do assert_raise(ArgumentError) do - get "/render_action/basic/hello_world_with_layout", {}, "action_dispatch.show_exceptions" => false + get "/render_action/basic/hello_world_with_layout", headers: { "action_dispatch.show_exceptions" => false } end end @@ -108,7 +108,7 @@ module RenderAction test "rendering with layout => 'greetings'" do assert_raise(ActionView::MissingTemplate) do - get "/render_action/basic/hello_world_with_custom_layout", {}, "action_dispatch.show_exceptions" => false + get "/render_action/basic/hello_world_with_custom_layout", headers: { "action_dispatch.show_exceptions" => false } end end end diff --git a/actionpack/test/controller/new_base/render_layout_test.rb b/actionpack/test/controller/new_base/render_layout_test.rb index 4ac40ca405..7ab3777026 100644 --- a/actionpack/test/controller/new_base/render_layout_test.rb +++ b/actionpack/test/controller/new_base/render_layout_test.rb @@ -86,7 +86,7 @@ module ControllerLayouts XML_INSTRUCT = %Q(<?xml version="1.0" encoding="UTF-8"?>\n) test "if XML is selected, an HTML template is not also selected" do - get :index, :format => "xml" + get :index, params: { format: "xml" } assert_response XML_INSTRUCT end @@ -96,7 +96,7 @@ module ControllerLayouts end test "a layout for JS is ignored even if explicitly provided for HTML" do - get :explicit, { :format => "js" } + get :explicit, params: { format: "js" } assert_response "alert('foo');" end end @@ -120,7 +120,7 @@ module ControllerLayouts testing ControllerLayouts::FalseLayoutMethodController test "access false layout returned by a method/proc" do - get :index, :format => "js" + get :index, params: { format: "js" } assert_response "alert('foo');" end end diff --git a/actionpack/test/controller/new_base/render_streaming_test.rb b/actionpack/test/controller/new_base/render_streaming_test.rb index 4c9126ca8c..9ea056194a 100644 --- a/actionpack/test/controller/new_base/render_streaming_test.rb +++ b/actionpack/test/controller/new_base/render_streaming_test.rb @@ -97,7 +97,7 @@ module RenderStreaming end test "do not stream on HTTP/1.0" do - get "/render_streaming/basic/hello_world", nil, "HTTP_VERSION" => "HTTP/1.0" + get "/render_streaming/basic/hello_world", headers: { "HTTP_VERSION" => "HTTP/1.0" } assert_body "Hello world, I'm here!" assert_status 200 assert_equal "22", headers["Content-Length"] diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb index 42a86b1d0d..19fef718e7 100644 --- a/actionpack/test/controller/new_base/render_template_test.rb +++ b/actionpack/test/controller/new_base/render_template_test.rb @@ -111,7 +111,7 @@ module RenderTemplate end test "rendering a builder template" do - get :builder_template, "format" => "xml" + get :builder_template, params: { "format" => "xml" } assert_response "<html>\n <p>Hello</p>\n</html>\n" end @@ -126,7 +126,7 @@ module RenderTemplate assert_body "Hello <strong>this is also raw</strong> in an html template" assert_status 200 - get :with_implicit_raw, format: 'text' + get :with_implicit_raw, params: { format: 'text' } assert_body "Hello <strong>this is also raw</strong> in a text template" assert_status 200 diff --git a/actionpack/test/controller/new_base/render_test.rb b/actionpack/test/controller/new_base/render_test.rb index 5635e16234..11a19ab783 100644 --- a/actionpack/test/controller/new_base/render_test.rb +++ b/actionpack/test/controller/new_base/render_test.rb @@ -74,7 +74,7 @@ module Render end assert_raises(AbstractController::DoubleRenderError) do - get "/render/double_render", {}, "action_dispatch.show_exceptions" => false + get "/render/double_render", headers: { "action_dispatch.show_exceptions" => false } end end end @@ -84,13 +84,13 @@ module Render # Only public methods on actual controllers are callable actions test "raises an exception when a method of Object is called" do assert_raises(AbstractController::ActionNotFound) do - get "/render/blank_render/clone", {}, "action_dispatch.show_exceptions" => false + get "/render/blank_render/clone", headers: { "action_dispatch.show_exceptions" => false } end end test "raises an exception when a private method is called" do assert_raises(AbstractController::ActionNotFound) do - get "/render/blank_render/secretz", {}, "action_dispatch.show_exceptions" => false + get "/render/blank_render/secretz", headers: { "action_dispatch.show_exceptions" => false } end end end diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index 645ecae220..8bf016d060 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -40,7 +40,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_filtered_parameters with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_equal @request.filtered_parameters, { 'controller' => 'params_wrapper_test/users', 'action' => 'parse', 'username' => 'sikachu', 'user' => { 'username' => 'sikachu' } } end end @@ -48,7 +48,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_derived_name_from_controller with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_parameters({ 'username' => 'sikachu', 'user' => { 'username' => 'sikachu' }}) end end @@ -58,7 +58,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters :person @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_parameters({ 'username' => 'sikachu', 'person' => { 'username' => 'sikachu' }}) end end @@ -68,7 +68,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters Person @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_parameters({ 'username' => 'sikachu', 'person' => { 'username' => 'sikachu' }}) end end @@ -78,7 +78,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters :include => :username @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) end end @@ -88,7 +88,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters :exclude => :title @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) end end @@ -98,7 +98,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters :person, :include => :username @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }}) end end @@ -106,7 +106,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_not_enabled_format with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/xml' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer' }) end end @@ -115,7 +115,7 @@ class ParamsWrapperTest < ActionController::TestCase with_default_wrapper_options do UsersController.wrap_parameters false @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer' }) end end @@ -125,7 +125,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters :format => :xml @request.env['CONTENT_TYPE'] = 'application/xml' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu', 'title' => 'Developer' }}) end end @@ -133,7 +133,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_not_wrap_reserved_parameters with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'authenticity_token' => 'pwned', '_method' => 'put', 'utf8' => '☃', 'username' => 'sikachu' } + post :parse, params: { 'authenticity_token' => 'pwned', '_method' => 'put', 'utf8' => '☃', 'username' => 'sikachu' } assert_parameters({ 'authenticity_token' => 'pwned', '_method' => 'put', 'utf8' => '☃', 'username' => 'sikachu', 'user' => { 'username' => 'sikachu' }}) end end @@ -141,7 +141,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_no_double_wrap_if_key_exists with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'user' => { 'username' => 'sikachu' }} + post :parse, params: { 'user' => { 'username' => 'sikachu' }} assert_parameters({ 'user' => { 'username' => 'sikachu' }}) end end @@ -149,7 +149,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_nested_params with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'person' => { 'username' => 'sikachu' }} + post :parse, params: { 'person' => { 'username' => 'sikachu' }} assert_parameters({ 'person' => { 'username' => 'sikachu' }, 'user' => {'person' => { 'username' => 'sikachu' }}}) end end @@ -160,7 +160,7 @@ class ParamsWrapperTest < ActionController::TestCase with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) end end @@ -173,7 +173,7 @@ class ParamsWrapperTest < ActionController::TestCase UsersController.wrap_parameters Person @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }}) end end @@ -184,7 +184,7 @@ class ParamsWrapperTest < ActionController::TestCase with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu', 'title' => 'Developer' }}) end end @@ -192,7 +192,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_preserves_query_string_params with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - get :parse, { 'user' => { 'username' => 'nixon' } } + get :parse, params: { 'user' => { 'username' => 'nixon' } } assert_parameters( {'user' => { 'username' => 'nixon' } } ) @@ -202,7 +202,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_empty_parameter_set with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, {} + post :parse, params: {} assert_parameters( {'user' => { } } ) @@ -249,7 +249,7 @@ class NamespacedParamsWrapperTest < ActionController::TestCase def test_derived_name_from_controller with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_parameters({'username' => 'sikachu', 'user' => { 'username' => 'sikachu' }}) end end @@ -259,7 +259,7 @@ class NamespacedParamsWrapperTest < ActionController::TestCase begin with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) end ensure @@ -272,7 +272,7 @@ class NamespacedParamsWrapperTest < ActionController::TestCase begin with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'title' => 'Developer' }}) end ensure @@ -299,7 +299,7 @@ class AnonymousControllerParamsWrapperTest < ActionController::TestCase def test_does_not_implicitly_wrap_params with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_parameters({ 'username' => 'sikachu' }) end end @@ -308,7 +308,7 @@ class AnonymousControllerParamsWrapperTest < ActionController::TestCase with_default_wrapper_options do @controller.class.wrap_parameters(:name => "guest") @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu' } + post :parse, params: { 'username' => 'sikachu' } assert_parameters({ 'username' => 'sikachu', 'guest' => { 'username' => 'sikachu' }}) end end @@ -344,7 +344,7 @@ class IrregularInflectionParamsWrapperTest < ActionController::TestCase with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, { 'username' => 'sikachu', 'test_attr' => 'test_value' } + post :parse, params: { 'username' => 'sikachu', 'test_attr' => 'test_value' } assert_parameters({ 'username' => 'sikachu', 'test_attr' => 'test_value', 'paramswrappernews_item' => { 'test_attr' => 'test_value' }}) end end diff --git a/actionpack/test/controller/permitted_params_test.rb b/actionpack/test/controller/permitted_params_test.rb index f46249d712..7136fafae5 100644 --- a/actionpack/test/controller/permitted_params_test.rb +++ b/actionpack/test/controller/permitted_params_test.rb @@ -14,12 +14,12 @@ class ActionControllerPermittedParamsTest < ActionController::TestCase tests PeopleController test "parameters are forbidden" do - post :create, { person: { name: "Mjallo!" } } + post :create, params: { person: { name: "Mjallo!" } } assert_equal "forbidden", response.body end test "parameters can be permitted and are then not forbidden" do - post :create_with_permit, { person: { name: "Mjallo!" } } + post :create_with_permit, params: { person: { name: "Mjallo!" } } assert_equal "permitted", response.body end end diff --git a/actionpack/test/controller/render_js_test.rb b/actionpack/test/controller/render_js_test.rb index d550422a2f..d482df195f 100644 --- a/actionpack/test/controller/render_js_test.rb +++ b/actionpack/test/controller/render_js_test.rb @@ -28,7 +28,7 @@ class RenderJSTest < ActionController::TestCase end def test_should_render_js_partial - xhr :get, :show_partial, :format => 'js' + xhr :get, :show_partial, format: 'js' assert_equal 'partial js', @response.body end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 108a35f59f..1c3f436cfa 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -462,21 +462,21 @@ class HeadRenderTest < ActionController::TestCase end def test_head_with_symbolic_status - get :head_with_symbolic_status, :status => "ok" + get :head_with_symbolic_status, params: { status: "ok" } assert_equal 200, @response.status assert_response :ok - get :head_with_symbolic_status, :status => "not_found" + get :head_with_symbolic_status, params: { status: "not_found" } assert_equal 404, @response.status assert_response :not_found - get :head_with_symbolic_status, :status => "no_content" + get :head_with_symbolic_status, params: { status: "no_content" } assert_equal 204, @response.status assert !@response.headers.include?('Content-Length') assert_response :no_content Rack::Utils::SYMBOL_TO_STATUS_CODE.each do |status, code| - get :head_with_symbolic_status, :status => status.to_s + get :head_with_symbolic_status, params: { status: status.to_s } assert_equal code, @response.response_code assert_response status end @@ -484,7 +484,7 @@ class HeadRenderTest < ActionController::TestCase def test_head_with_integer_status Rack::Utils::HTTP_STATUS_CODES.each do |code, message| - get :head_with_integer_status, :status => code.to_s + get :head_with_integer_status, params: { status: code.to_s } assert_equal message, @response.message end end @@ -498,7 +498,7 @@ class HeadRenderTest < ActionController::TestCase end def test_head_with_string_status - get :head_with_string_status, :status => "404 Eat Dirt" + get :head_with_string_status, params: { status: "404 Eat Dirt" } assert_equal 404, @response.response_code assert_equal "Not Found", @response.message assert_response :not_found diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 4f280c4bec..7a91577b17 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -81,7 +81,7 @@ class RenderXmlTest < ActionController::TestCase end def test_should_render_formatted_xml_erb_template - get :formatted_xml_erb, :format => :xml + get :formatted_xml_erb, format: :xml assert_equal '<test>passed formatted xml erb</test>', @response.body end @@ -91,7 +91,7 @@ class RenderXmlTest < ActionController::TestCase end def test_should_use_implicit_content_type - get :implicit_content_type, :format => 'atom' + get :implicit_content_type, format: 'atom' assert_equal Mime::ATOM, @response.content_type end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index ea2d35c3f8..8a0eed5a87 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -266,19 +266,19 @@ module RequestForgeryProtectionTests end def test_should_allow_post_with_token - assert_not_blocked { post :index, :custom_authenticity_token => @token } + assert_not_blocked { post :index, params: { custom_authenticity_token: @token } } end def test_should_allow_patch_with_token - assert_not_blocked { patch :index, :custom_authenticity_token => @token } + assert_not_blocked { patch :index, params: { custom_authenticity_token: @token } } end def test_should_allow_put_with_token - assert_not_blocked { put :index, :custom_authenticity_token => @token } + assert_not_blocked { put :index, params: { custom_authenticity_token: @token } } end def test_should_allow_delete_with_token - assert_not_blocked { delete :index, :custom_authenticity_token => @token } + assert_not_blocked { delete :index, params: { custom_authenticity_token: @token } } end def test_should_allow_post_with_token_in_header @@ -341,7 +341,7 @@ module RequestForgeryProtectionTests 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 { xhr :get, :same_origin_js, format: 'js'} assert_cross_origin_not_blocked do @request.accept = 'text/javascript' xhr :get, :negotiate_same_origin @@ -350,11 +350,11 @@ module RequestForgeryProtectionTests # Allow non-GET requests since GET is all a remote <script> tag can muster. def test_should_allow_non_get_js_without_xhr_header - assert_cross_origin_not_blocked { post :same_origin_js, custom_authenticity_token: @token } - assert_cross_origin_not_blocked { post :same_origin_js, format: 'js', custom_authenticity_token: @token } + assert_cross_origin_not_blocked { post :same_origin_js, params: { custom_authenticity_token: @token } } + assert_cross_origin_not_blocked { post :same_origin_js, params: { format: 'js', custom_authenticity_token: @token } } assert_cross_origin_not_blocked do @request.accept = 'text/javascript' - post :negotiate_same_origin, custom_authenticity_token: @token + post :negotiate_same_origin, params: { custom_authenticity_token: @token} end end @@ -543,7 +543,7 @@ class CustomAuthenticityParamControllerTest < ActionController::TestCase @controller.stubs(:valid_authenticity_token?).returns(:true) begin - post :index, :custom_token_name => 'foobar' + post :index, params: { custom_token_name: 'foobar' } assert_equal 0, @logger.logged(:warn).size ensure ActionController::Base.logger = @old_logger @@ -554,7 +554,7 @@ class CustomAuthenticityParamControllerTest < ActionController::TestCase ActionController::Base.logger = @logger begin - post :index, :custom_token_name => 'bazqux' + post :index, params: { custom_token_name: 'bazqux' } assert_equal 1, @logger.logged(:warn).size ensure ActionController::Base.logger = @old_logger diff --git a/actionpack/test/controller/required_params_test.rb b/actionpack/test/controller/required_params_test.rb index 6803dbbb62..a901e56332 100644 --- a/actionpack/test/controller/required_params_test.rb +++ b/actionpack/test/controller/required_params_test.rb @@ -12,21 +12,21 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase test "missing required parameters will raise exception" do assert_raise ActionController::ParameterMissing do - post :create, { magazine: { name: "Mjallo!" } } + post :create, params: { magazine: { name: "Mjallo!" } } end assert_raise ActionController::ParameterMissing do - post :create, { book: { title: "Mjallo!" } } + post :create, params: { book: { title: "Mjallo!" } } end end test "required parameters that are present will not raise" do - post :create, { book: { name: "Mjallo!" } } + post :create, params: { book: { name: "Mjallo!" } } assert_response :ok end test "required parameters with false value will not raise" do - post :create, { book: { name: false } } + post :create, params: { book: { name: false } } assert_response :ok end end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 0e15883f43..05806177e3 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -1187,7 +1187,7 @@ class ResourcesTest < ActionController::TestCase @controller.singleton_class.send(:include, @routes.url_helpers) @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new - get :index, options[:options] + get :index, params: options[:options] options[:options].delete :action path = "#{options[:as] || controller_name}" @@ -1257,7 +1257,7 @@ class ResourcesTest < ActionController::TestCase @controller.singleton_class.send(:include, @routes.url_helpers) @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new - get :show, options[:options] + get :show, params: options[:options] options[:options].delete :action full_path = "/#{options[:path_prefix]}#{options[:as] || singleton_name}" diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index f7eba1ef43..fba5ebba15 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -58,13 +58,13 @@ module ShowExceptions class ShowExceptionsOverriddenTest < ActionDispatch::IntegrationTest test 'show error page' do @app = ShowExceptionsOverriddenController.action(:boom) - get '/', {'detailed' => '0'} + get '/', params: { 'detailed' => '0' } assert_equal "500 error fixture\n", body end test 'show diagnostics message' do @app = ShowExceptionsOverriddenController.action(:boom) - get '/', {'detailed' => '1'} + get '/', params: { 'detailed' => '1' } assert_match(/boom/, body) end end @@ -72,7 +72,7 @@ module ShowExceptions class ShowExceptionsFormatsTest < ActionDispatch::IntegrationTest def test_render_json_exception @app = ShowExceptionsOverriddenController.action(:boom) - get "/", {}, 'HTTP_ACCEPT' => 'application/json' + get "/", headers: { 'HTTP_ACCEPT' => 'application/json' } assert_response :internal_server_error assert_equal 'application/json', response.content_type.to_s assert_equal({ :status => '500', :error => 'Internal Server Error' }.to_json, response.body) @@ -80,7 +80,7 @@ module ShowExceptions def test_render_xml_exception @app = ShowExceptionsOverriddenController.action(:boom) - get "/", {}, 'HTTP_ACCEPT' => 'application/xml' + get "/", headers: { 'HTTP_ACCEPT' => 'application/xml' } assert_response :internal_server_error assert_equal 'application/xml', response.content_type.to_s assert_equal({ :status => '500', :error => 'Internal Server Error' }.to_xml, response.body) @@ -88,7 +88,7 @@ module ShowExceptions def test_render_fallback_exception @app = ShowExceptionsOverriddenController.action(:boom) - get "/", {}, 'HTTP_ACCEPT' => 'text/csv' + get "/", headers: { 'HTTP_ACCEPT' => 'text/csv' } assert_response :internal_server_error assert_equal 'text/html', response.content_type.to_s end @@ -101,7 +101,7 @@ module ShowExceptions @app.instance_variable_set(:@exceptions_app, nil) $stderr = StringIO.new - get '/', {}, 'HTTP_ACCEPT' => 'text/json' + get '/', headers: { 'HTTP_ACCEPT' => 'text/json' } assert_response :internal_server_error assert_equal 'text/plain', response.content_type.to_s ensure diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 2e1f21c645..901db63e39 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -6,57 +6,57 @@ require 'rails/engine' class TestCaseTest < ActionController::TestCase class TestController < ActionController::Base def no_op - render :text => 'dummy' + render text: 'dummy' end def set_flash flash["test"] = ">#{flash["test"]}<" - render :text => 'ignore me' + render text: 'ignore me' end def set_flash_now flash.now["test_now"] = ">#{flash["test_now"]}<" - render :text => 'ignore me' + render text: 'ignore me' end def set_session session['string'] = 'A wonder' session[:symbol] = 'it works' - render :text => 'Success' + render text: 'Success' end def reset_the_session reset_session - render :text => 'ignore me' + render text: 'ignore me' end def render_raw_post raise ActiveSupport::TestCase::Assertion, "#raw_post is blank" if request.raw_post.blank? - render :text => request.raw_post + render text: request.raw_post end def render_body - render :text => request.body.read + render text: request.body.read end def test_params - render :text => params.inspect + render text: params.inspect end def test_uri - render :text => request.fullpath + render text: request.fullpath end def test_format - render :text => request.format + render text: request.format end def test_query_string - render :text => request.query_string + render text: request.query_string end def test_protocol - render :text => request.protocol + render text: request.protocol end def test_headers @@ -64,7 +64,7 @@ class TestCaseTest < ActionController::TestCase end def test_html_output - render :text => <<HTML + render text: <<HTML <html> <body> <a href="/"><img src="/images/button.png" /></a> @@ -86,7 +86,7 @@ HTML def test_xml_output response.content_type = "application/xml" - render :text => <<XML + render text: <<XML <?xml version="1.0" encoding="UTF-8"?> <root> <area>area is an empty tag in HTML, raising an error if not in xml mode</area> @@ -95,15 +95,15 @@ XML end def test_only_one_param - render :text => (params[:left] && params[:right]) ? "EEP, Both here!" : "OK" + render text: (params[:left] && params[:right]) ? "EEP, Both here!" : "OK" end def test_remote_addr - render :text => (request.remote_addr || "not specified") + render text: (request.remote_addr || "not specified") end def test_file_upload - render :text => params[:file].size + render text: params[:file].size end def test_send_file @@ -111,26 +111,26 @@ XML end def redirect_to_same_controller - redirect_to :controller => 'test', :action => 'test_uri', :id => 5 + redirect_to controller: 'test', action: 'test_uri', id: 5 end def redirect_to_different_controller - redirect_to :controller => 'fail', :id => 5 + redirect_to controller: 'fail', id: 5 end def create - head :created, :location => 'created resource' + head :created, location: 'created resource' end def delete_cookie cookies.delete("foo") - render :nothing => true + render nothing: true end def test_assigns @foo = "foo" - @foo_hash = {:foo => :bar} - render :nothing => true + @foo_hash = { foo: :bar } + render nothing: true end def test_without_body @@ -144,7 +144,7 @@ XML private def generate_url(opts) - url_for(opts.merge(:action => "test_uri")) + url_for(opts.merge(action: "test_uri")) end end @@ -164,7 +164,7 @@ XML class ViewAssignsController < ActionController::Base def test_assigns @foo = "foo" - render :nothing => true + render nothing: true end def view_assigns @@ -209,32 +209,50 @@ XML end def test_raw_post_handling - params = Hash[:page, {:name => 'page name'}, 'some key', 123] - post :render_raw_post, params.dup + params = Hash[:page, { name: 'page name' }, 'some key', 123] + post :render_raw_post, params: params.dup assert_equal params.to_query, @response.body end def test_body_stream - params = Hash[:page, { :name => 'page name' }, 'some key', 123] + params = Hash[:page, { name: 'page name' }, 'some key', 123] - post :render_body, params.dup + post :render_body, params: params.dup + + assert_equal params.to_query, @response.body + end + + def test_deprecated_body_stream + params = Hash[:page, { name: 'page name' }, 'some key', 123] + + assert_deprecated { post :render_body, params.dup } assert_equal params.to_query, @response.body end def test_document_body_and_params_with_post - post :test_params, :id => 1 - assert_equal("{\"id\"=>\"1\", \"controller\"=>\"test_case_test/test\", \"action\"=>\"test_params\"}", @response.body) + post :test_params, params: { id: 1 } + assert_equal(%({"id"=>"1", "controller"=>"test_case_test/test", "action"=>"test_params"}), @response.body) end def test_document_body_with_post - post :render_body, "document body" + post :render_body, body: "document body" + assert_equal "document body", @response.body + end + + def test_deprecated_document_body_with_post + assert_deprecated { post :render_body, "document body" } assert_equal "document body", @response.body end def test_document_body_with_put - put :render_body, "document body" + put :render_body, body: "document body" + assert_equal "document body", @response.body + end + + def test_deprecated_document_body_with_put + assert_deprecated { put :render_body, "document body" } assert_equal "document body", @response.body end @@ -243,22 +261,32 @@ XML assert_equal 200, @response.status end - def test_head_params_as_string - assert_raise(NoMethodError) { head :test_params, "document body", :id => 10 } - end - def test_process_without_flash process :set_flash assert_equal '><', flash['test'] end + def test_deprecated_process_with_flash + assert_deprecated { process :set_flash, "GET", nil, nil, { "test" => "value" } } + assert_equal '>value<', flash['test'] + end + def test_process_with_flash - process :set_flash, "GET", nil, nil, { "test" => "value" } + process :set_flash, + method: "GET", + flash: { "test" => "value" } assert_equal '>value<', flash['test'] end + def test_deprecated_process_with_flash_now + assert_deprecated { process :set_flash_now, "GET", nil, nil, { "test_now" => "value_now" } } + assert_equal '>value_now<', flash['test_now'] + end + def test_process_with_flash_now - process :set_flash_now, "GET", nil, nil, { "test_now" => "value_now" } + process :set_flash_now, + method: "GET", + flash: { "test_now" => "value_now" } assert_equal '>value_now<', flash['test_now'] end @@ -271,22 +299,48 @@ XML end def test_process_with_session_arg - process :no_op, "GET", nil, { 'string' => 'value1', :symbol => 'value2' } + assert_deprecated { process :no_op, "GET", nil, { 'string' => 'value1', symbol: 'value2' } } assert_equal 'value1', session['string'] assert_equal 'value1', session[:string] assert_equal 'value2', session['symbol'] assert_equal 'value2', session[:symbol] end + def test_process_with_session_kwarg + process :no_op, method: "GET", session: { 'string' => 'value1', symbol: 'value2' } + assert_equal 'value1', session['string'] + assert_equal 'value1', session[:string] + assert_equal 'value2', session['symbol'] + assert_equal 'value2', session[:symbol] + end + + def test_deprecated_process_merges_session_arg + session[:foo] = 'bar' + assert_deprecated { + get :no_op, nil, { bar: 'baz' } + } + assert_equal 'bar', session[:foo] + assert_equal 'baz', session[:bar] + end + def test_process_merges_session_arg session[:foo] = 'bar' - get :no_op, nil, { :bar => 'baz' } + get :no_op, session: { bar: 'baz' } assert_equal 'bar', session[:foo] assert_equal 'baz', session[:bar] end + def test_deprecated_merged_session_arg_is_retained_across_requests + assert_deprecated { + get :no_op, nil, { foo: 'bar' } + } + assert_equal 'bar', session[:foo] + get :no_op + assert_equal 'bar', session[:foo] + end + def test_merged_session_arg_is_retained_across_requests - get :no_op, nil, { :foo => 'bar' } + get :no_op, session: { foo: 'bar' } assert_equal 'bar', session[:foo] get :no_op assert_equal 'bar', session[:foo] @@ -294,7 +348,7 @@ XML def test_process_overwrites_existing_session_arg session[:foo] = 'bar' - get :no_op, nil, { :foo => 'baz' } + get :no_op, session: { foo: 'baz' } assert_equal 'baz', session[:foo] end @@ -321,19 +375,40 @@ XML assert_equal "/test_case_test/test/test_uri", @response.body end + def test_process_with_symbol_method + process :test_uri, method: :get + assert_equal "/test_case_test/test/test_uri", @response.body + end + + def test_deprecated_process_with_request_uri_with_params + assert_deprecated { process :test_uri, "GET", id: 7 } + assert_equal "/test_case_test/test/test_uri/7", @response.body + end + def test_process_with_request_uri_with_params - process :test_uri, "GET", :id => 7 + process :test_uri, + method: "GET", + params: { id: 7 } + assert_equal "/test_case_test/test/test_uri/7", @response.body end + def test_deprecated_process_with_request_uri_with_params_with_explicit_uri + @request.env['PATH_INFO'] = "/explicit/uri" + assert_deprecated { process :test_uri, "GET", id: 7 } + assert_equal "/explicit/uri", @response.body + end + def test_process_with_request_uri_with_params_with_explicit_uri @request.env['PATH_INFO'] = "/explicit/uri" - process :test_uri, "GET", :id => 7 + process :test_uri, method: "GET", params: { id: 7 } assert_equal "/explicit/uri", @response.body end def test_process_with_query_string - process :test_query_string, "GET", :q => 'test' + process :test_query_string, + method: "GET", + params: { q: 'test' } assert_equal "q=test", @response.body end @@ -345,9 +420,9 @@ XML end def test_multiple_calls - process :test_only_one_param, "GET", :left => true + process :test_only_one_param, method: "GET", params: { left: true } assert_equal "OK", @response.body - process :test_only_one_param, "GET", :right => true + process :test_only_one_param, method: "GET", params: { right: true } assert_equal "OK", @response.body end @@ -362,7 +437,7 @@ XML assert_equal "foo", assigns["foo"] # but the assigned variable should not have its own keys stringified - expected_hash = { :foo => :bar } + expected_hash = { foo: :bar } assert_equal expected_hash, assigns(:foo_hash) end @@ -390,21 +465,21 @@ XML end def test_assert_generates - assert_generates 'controller/action/5', :controller => 'controller', :action => 'action', :id => '5' - assert_generates 'controller/action/7', {:id => "7"}, {:controller => "controller", :action => "action"} - assert_generates 'controller/action/5', {:controller => "controller", :action => "action", :id => "5", :name => "bob"}, {}, {:name => "bob"} - assert_generates 'controller/action/7', {:id => "7", :name => "bob"}, {:controller => "controller", :action => "action"}, {:name => "bob"} - assert_generates 'controller/action/7', {:id => "7"}, {:controller => "controller", :action => "action", :name => "bob"}, {} + assert_generates 'controller/action/5', controller: 'controller', action: 'action', id: '5' + assert_generates 'controller/action/7', { id: "7" }, { controller: "controller", action: "action" } + assert_generates 'controller/action/5', { controller: "controller", action: "action", id: "5", name: "bob" }, {}, { name: "bob" } + assert_generates 'controller/action/7', { id: "7", name: "bob" }, { controller: "controller", action: "action" }, { name: "bob" } + assert_generates 'controller/action/7', { id: "7" }, { controller: "controller", action: "action", name: "bob" }, {} end def test_assert_routing - assert_routing 'content', :controller => 'content', :action => 'index' + assert_routing 'content', controller: 'content', action: 'index' end def test_assert_routing_with_method with_routing do |set| set.draw { resources(:content) } - assert_routing({ :method => 'post', :path => 'content' }, { :controller => 'content', :action => 'create' }) + assert_routing({ method: 'post', path: 'content' }, { controller: 'content', action: 'create' }) end end @@ -416,29 +491,75 @@ XML end end - assert_routing 'admin/user', :controller => 'admin/user', :action => 'index' + assert_routing 'admin/user', controller: 'admin/user', action: 'index' end end def test_assert_routing_with_glob with_routing do |set| set.draw { get('*path' => "pages#show") } - assert_routing('/company/about', { :controller => 'pages', :action => 'show', :path => 'company/about' }) + assert_routing('/company/about', { controller: 'pages', action: 'show', path: 'company/about' }) end end + def test_deprecated_params_passing + assert_deprecated { + get :test_params, page: { name: "Page name", month: '4', year: '2004', day: '6' } + } + parsed_params = eval(@response.body) + assert_equal( + { + 'controller' => 'test_case_test/test', 'action' => 'test_params', + 'page' => { 'name' => "Page name", 'month' => '4', 'year' => '2004', 'day' => '6' } + }, + parsed_params + ) + end + def test_params_passing - get :test_params, :page => {:name => "Page name", :month => '4', :year => '2004', :day => '6'} + get :test_params, params: { + page: { + name: "Page name", + month: '4', + year: '2004', + day: '6' + } + } + parsed_params = eval(@response.body) + assert_equal( + { + 'controller' => 'test_case_test/test', 'action' => 'test_params', + 'page' => { 'name' => "Page name", 'month' => '4', 'year' => '2004', 'day' => '6' } + }, + parsed_params + ) + end + + def test_kwarg_params_passing_with_session_and_flash + get :test_params, params: { + page: { + name: "Page name", + month: '4', + year: '2004', + day: '6' + } + }, session: { 'foo' => 'bar' }, flash: { notice: 'created' } + parsed_params = eval(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', 'page' => {'name' => "Page name", 'month' => '4', 'year' => '2004', 'day' => '6'}}, parsed_params ) + + assert_equal 'bar', session[:foo] + assert_equal 'created', flash[:notice] end def test_params_passing_with_fixnums - get :test_params, :page => {:name => "Page name", :month => 4, :year => 2004, :day => 6} + get :test_params, params: { + page: { name: "Page name", month: 4, year: 2004, day: 6 } + } parsed_params = eval(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', @@ -448,7 +569,7 @@ XML end def test_params_passing_with_fixnums_when_not_html_request - get :test_params, :format => 'json', :count => 999 + get :test_params, params: { format: 'json', count: 999 } parsed_params = eval(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', @@ -458,7 +579,17 @@ XML end def test_params_passing_path_parameter_is_string_when_not_html_request - get :test_params, :format => 'json', :id => 1 + get :test_params, params: { format: 'json', id: 1 } + parsed_params = eval(@response.body) + assert_equal( + {'controller' => 'test_case_test/test', 'action' => 'test_params', + 'format' => 'json', 'id' => '1' }, + parsed_params + ) + end + + def test_deprecated_params_passing_path_parameter_is_string_when_not_html_request + assert_deprecated { get :test_params, format: 'json', id: 1 } parsed_params = eval(@response.body) assert_equal( {'controller' => 'test_case_test/test', 'action' => 'test_params', @@ -469,7 +600,9 @@ XML def test_params_passing_with_frozen_values assert_nothing_raised do - get :test_params, :frozen => 'icy'.freeze, :frozens => ['icy'.freeze].freeze, :deepfreeze => { :frozen => 'icy'.freeze }.freeze + get :test_params, params: { + frozen: 'icy'.freeze, frozens: ['icy'.freeze].freeze, deepfreeze: { frozen: 'icy'.freeze }.freeze + } end parsed_params = eval(@response.body) assert_equal( @@ -480,8 +613,8 @@ XML end def test_params_passing_doesnt_modify_in_place - page = {:name => "Page name", :month => 4, :year => 2004, :day => 6} - get :test_params, :page => page + page = { name: "Page name", month: 4, year: 2004, day: 6 } + get :test_params, params: { page: page } assert_equal 2004, page[:year] end @@ -504,25 +637,32 @@ XML end def test_id_converted_to_string - get :test_params, :id => 20, :foo => Object.new + get :test_params, params: { + id: 20, foo: Object.new + } + assert_kind_of String, @request.path_parameters[:id] + end + + def test_deprecared_id_converted_to_string + assert_deprecated { get :test_params, id: 20, foo: Object.new} assert_kind_of String, @request.path_parameters[:id] end def test_array_path_parameter_handled_properly with_routing do |set| set.draw do - get 'file/*path', :to => 'test_case_test/test#test_params' + get 'file/*path', to: 'test_case_test/test#test_params' get ':controller/:action' end - get :test_params, :path => ['hello', 'world'] + get :test_params, params: { path: ['hello', 'world'] } assert_equal ['hello', 'world'], @request.path_parameters[:path] assert_equal 'hello/world', @request.path_parameters[:path].to_param end end def test_assert_realistic_path_parameters - get :test_params, :id => 20, :foo => Object.new + get :test_params, params: { id: 20, foo: Object.new } # All elements of path_parameters should use Symbol keys @request.path_parameters.each_key do |key| @@ -559,14 +699,37 @@ XML assert_nil @request.env['HTTP_ACCEPT'] end + def test_xhr_with_params + 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_session + 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" + 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_header_properly_reset_after_get_request get :test_params @request.recycle! assert_nil @request.instance_variable_get("@request_method") end + def test_deprecated_params_reset_between_post_requests + assert_deprecated { post :no_op, foo: "bar" } + assert_equal "bar", @request.params[:foo] + + post :no_op + assert @request.params[:foo].blank? + end + def test_params_reset_between_post_requests - post :no_op, :foo => "bar" + post :no_op, params: { foo: "bar" } assert_equal "bar", @request.params[:foo] post :no_op @@ -574,15 +737,15 @@ XML end def test_filtered_parameters_reset_between_requests - get :no_op, :foo => "bar" + get :no_op, params: { foo: "bar" } assert_equal "bar", @request.filtered_parameters[:foo] - get :no_op, :foo => "baz" + get :no_op, params: { foo: "baz" } assert_equal "baz", @request.filtered_parameters[:foo] end def test_path_params_reset_between_request - get :test_params, :id => "foo" + get :test_params, params: { id: "foo" } assert_equal "foo", @request.path_parameters[:id] get :test_params @@ -603,19 +766,38 @@ XML end def test_request_format - get :test_format, :format => 'html' + get :test_format, params: { format: 'html' } + assert_equal 'text/html', @response.body + + get :test_format, params: { format: 'json' } + assert_equal 'application/json', @response.body + + get :test_format, params: { format: 'xml' } + assert_equal 'application/xml', @response.body + + get :test_format assert_equal 'text/html', @response.body + end - get :test_format, :format => 'json' + def test_request_format_kwarg + get :test_format, format: 'html' + assert_equal 'text/html', @response.body + + get :test_format, format: 'json' assert_equal 'application/json', @response.body - get :test_format, :format => 'xml' + get :test_format, format: 'xml' assert_equal 'application/xml', @response.body get :test_format assert_equal 'text/html', @response.body end + def test_request_format_kwarg_overrides_params + get :test_format, format: 'json', params: { format: 'html' } + assert_equal 'application/json', @response.body + end + def test_should_have_knowledge_of_client_side_cookie_state_even_if_they_are_not_set cookies['foo'] = 'bar' get :no_op @@ -700,7 +882,10 @@ XML end def test_fixture_file_upload - post :test_file_upload, :file => fixture_file_upload(FILES_DIR + "/mona_lisa.jpg", "image/jpg") + post :test_file_upload, + params: { + file: fixture_file_upload(FILES_DIR + "/mona_lisa.jpg", "image/jpg") + } assert_equal '159528', @response.body end @@ -716,10 +901,21 @@ XML assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read end + def test_deprecated_action_dispatch_uploaded_file_upload + filename = 'mona_lisa.jpg' + path = "#{FILES_DIR}/#{filename}" + assert_deprecated { + post :test_file_upload, file: ActionDispatch::Http::UploadedFile.new(filename: path, type: "image/jpg", tempfile: File.open(path)) + } + assert_equal '159528', @response.body + end + def test_action_dispatch_uploaded_file_upload filename = 'mona_lisa.jpg' path = "#{FILES_DIR}/#{filename}" - post :test_file_upload, :file => ActionDispatch::Http::UploadedFile.new(:filename => path, :type => "image/jpg", :tempfile => File.open(path)) + post :test_file_upload, params: { + file: ActionDispatch::Http::UploadedFile.new(filename: path, type: "image/jpg", tempfile: File.open(path)) + } assert_equal '159528', @response.body end @@ -753,7 +949,7 @@ module EngineControllerTests class BarController < ActionController::Base def index - render :text => 'bar' + render text: 'bar' end end @@ -830,7 +1026,7 @@ class NamedRoutesControllerTest < ActionController::TestCase with_routing do |set| set.draw { resources :contents } assert_equal 'http://test.host/contents/new', new_content_url - assert_equal 'http://test.host/contents/1', content_url(:id => 1) + assert_equal 'http://test.host/contents/1', content_url(id: 1) end end end @@ -839,7 +1035,7 @@ class AnonymousControllerTest < ActionController::TestCase def setup @controller = Class.new(ActionController::Base) do def index - render :text => params[:controller] + render text: params[:controller] end end.new @@ -860,29 +1056,29 @@ class RoutingDefaultsTest < ActionController::TestCase def setup @controller = Class.new(ActionController::Base) do def post - render :text => request.fullpath + render text: request.fullpath end def project - render :text => request.fullpath + render text: request.fullpath end end.new @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do - get '/posts/:id', :to => 'anonymous#post', :bucket_type => 'post' - get '/projects/:id', :to => 'anonymous#project', :defaults => { :bucket_type => 'project' } + get '/posts/:id', to: 'anonymous#post', bucket_type: 'post' + get '/projects/:id', to: 'anonymous#project', defaults: { bucket_type: 'project' } end end end def test_route_option_can_be_passed_via_process - get :post, :id => 1, :bucket_type => 'post' + get :post, params: { id: 1, bucket_type: 'post'} assert_equal '/posts/1', @response.body end def test_route_default_is_not_required_for_building_request_uri - get :project, :id => 2 + get :project, params: { id: 2 } assert_equal '/projects/2', @response.body end end diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 2b109ff19e..21fa670bb6 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -35,7 +35,9 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_post_json with_test_route_set do - post "/", '{"entry":{"summary":"content..."}}', 'CONTENT_TYPE' => 'application/json' + post "/", + params: '{"entry":{"summary":"content..."}}', + headers: { 'CONTENT_TYPE' => 'application/json' } assert_equal 'entry', @controller.response.body assert @controller.params.has_key?(:entry) @@ -45,7 +47,9 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_put_json with_test_route_set do - put "/", '{"entry":{"summary":"content..."}}', 'CONTENT_TYPE' => 'application/json' + put "/", + params: '{"entry":{"summary":"content..."}}', + headers: { 'CONTENT_TYPE' => 'application/json' } assert_equal 'entry', @controller.response.body assert @controller.params.has_key?(:entry) @@ -56,8 +60,9 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_register_and_use_json_simple with_test_route_set do with_params_parsers Mime::JSON => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do - post "/", '{"request":{"summary":"content...","title":"JSON"}}', - 'CONTENT_TYPE' => 'application/json' + post "/", + params: '{"request":{"summary":"content...","title":"JSON"}}', + headers: { 'CONTENT_TYPE' => 'application/json' } assert_equal 'summary, title', @controller.response.body assert @controller.params.has_key?(:summary) @@ -70,14 +75,16 @@ class WebServiceTest < ActionDispatch::IntegrationTest def test_use_json_with_empty_request with_test_route_set do - assert_nothing_raised { post "/", "", 'CONTENT_TYPE' => 'application/json' } + assert_nothing_raised { post "/", headers: { 'CONTENT_TYPE' => 'application/json' } } assert_equal '', @controller.response.body end end def test_dasherized_keys_as_json with_test_route_set do - post "/?full=1", '{"first-key":{"sub-key":"..."}}', 'CONTENT_TYPE' => 'application/json' + post "/?full=1", + params: '{"first-key":{"sub-key":"..."}}', + headers: { 'CONTENT_TYPE' => 'application/json' } assert_equal 'action, controller, first-key(sub-key), full', @controller.response.body assert_equal "...", @controller.params['first-key']['sub-key'] end @@ -87,7 +94,9 @@ class WebServiceTest < ActionDispatch::IntegrationTest with_test_route_set do with_params_parsers Mime::JSON => Proc.new { |data| raise Interrupt } do assert_raises(Interrupt) do - post "/", '{"title":"JSON"}}', 'CONTENT_TYPE' => 'application/json' + post "/", + params: '{"title":"JSON"}}', + headers: { 'CONTENT_TYPE' => 'application/json' } end end end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 1e5ed60b09..7921f05688 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -86,21 +86,21 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test 'skip diagnosis if not showing detailed exceptions' do @app = ProductionApp assert_raise RuntimeError do - get "/", {}, {'action_dispatch.show_exceptions' => true} + get "/", headers: { 'action_dispatch.show_exceptions' => true } end end test 'skip diagnosis if not showing exceptions' do @app = DevelopmentApp assert_raise RuntimeError do - get "/", {}, {'action_dispatch.show_exceptions' => false} + get "/", headers: { 'action_dispatch.show_exceptions' => false } end end test 'raise an exception on cascade pass' do @app = ProductionApp assert_raise ActionController::RoutingError do - get "/pass", {}, {'action_dispatch.show_exceptions' => true} + get "/pass", headers: { 'action_dispatch.show_exceptions' => true } end end @@ -108,14 +108,14 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest boomer = Boomer.new(false) @app = ActionDispatch::DebugExceptions.new(boomer) assert_raise ActionController::RoutingError do - get "/pass", {}, {'action_dispatch.show_exceptions' => true} + get "/pass", headers: { 'action_dispatch.show_exceptions' => true } end assert boomer.closed, "Expected to close the response body" end test 'displays routes in a table when a RoutingError occurs' do @app = DevelopmentApp - get "/pass", {}, {'action_dispatch.show_exceptions' => true} + get "/pass", headers: { 'action_dispatch.show_exceptions' => true } routing_table = body[/route_table.*<.table>/m] assert_match '/:controller(/:action)(.:format)', routing_table assert_match ':controller#:action', routing_table @@ -125,7 +125,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test 'displays request and response info when a RoutingError occurs' do @app = DevelopmentApp - get "/pass", {}, {'action_dispatch.show_exceptions' => true} + get "/pass", headers: { 'action_dispatch.show_exceptions' => true } assert_select 'h2', /Request/ assert_select 'h2', /Response/ @@ -134,27 +134,27 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test "rescue with diagnostics message" do @app = DevelopmentApp - get "/", {}, {'action_dispatch.show_exceptions' => true} + get "/", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 assert_match(/puke/, body) - get "/not_found", {}, {'action_dispatch.show_exceptions' => true} + get "/not_found", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 assert_match(/#{AbstractController::ActionNotFound.name}/, body) - get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} + get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true } assert_response 405 assert_match(/ActionController::MethodNotAllowed/, body) - get "/unknown_http_method", {}, {'action_dispatch.show_exceptions' => true} + get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true } assert_response 405 assert_match(/ActionController::UnknownHttpMethod/, body) - get "/bad_request", {}, {'action_dispatch.show_exceptions' => true} + get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true } assert_response 400 assert_match(/ActionController::BadRequest/, body) - get "/parameter_missing", {}, {'action_dispatch.show_exceptions' => true} + get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true } assert_response 400 assert_match(/ActionController::ParameterMissing/, body) end @@ -163,38 +163,38 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest @app = DevelopmentApp xhr_request_env = {'action_dispatch.show_exceptions' => true, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest'} - get "/", {}, xhr_request_env + get "/", headers: xhr_request_env assert_response 500 assert_no_match(/<header>/, body) assert_no_match(/<body>/, body) assert_equal "text/plain", response.content_type assert_match(/RuntimeError\npuke/, body) - get "/not_found", {}, xhr_request_env + get "/not_found", headers: xhr_request_env assert_response 404 assert_no_match(/<body>/, body) assert_equal "text/plain", response.content_type assert_match(/#{AbstractController::ActionNotFound.name}/, body) - get "/method_not_allowed", {}, xhr_request_env + get "/method_not_allowed", headers: xhr_request_env assert_response 405 assert_no_match(/<body>/, body) assert_equal "text/plain", response.content_type assert_match(/ActionController::MethodNotAllowed/, body) - get "/unknown_http_method", {}, xhr_request_env + get "/unknown_http_method", headers: xhr_request_env assert_response 405 assert_no_match(/<body>/, body) assert_equal "text/plain", response.content_type assert_match(/ActionController::UnknownHttpMethod/, body) - get "/bad_request", {}, xhr_request_env + get "/bad_request", headers: xhr_request_env assert_response 400 assert_no_match(/<body>/, body) assert_equal "text/plain", response.content_type assert_match(/ActionController::BadRequest/, body) - get "/parameter_missing", {}, xhr_request_env + get "/parameter_missing", headers: xhr_request_env assert_response 400 assert_no_match(/<body>/, body) assert_equal "text/plain", response.content_type @@ -204,8 +204,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test "does not show filtered parameters" do @app = DevelopmentApp - get "/", {"foo"=>"bar"}, {'action_dispatch.show_exceptions' => true, - 'action_dispatch.parameter_filter' => [:foo]} + get "/", params: { "foo"=>"bar" }, headers: { 'action_dispatch.show_exceptions' => true, + 'action_dispatch.parameter_filter' => [:foo] } assert_response 500 assert_match(""foo"=>"[FILTERED]"", body) end @@ -213,7 +213,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test "show registered original exception for wrapped exceptions" do @app = DevelopmentApp - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + get "/not_found_original_exception", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 assert_match(/AbstractController::ActionNotFound/, body) end @@ -221,7 +221,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test "named urls missing keys raise 500 level error" do @app = DevelopmentApp - get "/missing_keys", {}, {'action_dispatch.show_exceptions' => true} + get "/missing_keys", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 assert_match(/ActionController::UrlGenerationError/, body) @@ -229,7 +229,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test "show the controller name in the diagnostics template when controller name is present" do @app = DevelopmentApp - get("/runtime_error", {}, { + get("/runtime_error", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.request.parameters' => { 'action' => 'show', @@ -252,7 +252,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest } } - get("/runtime_error", {}, { + get("/runtime_error", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.request.parameters' => { 'action' => 'show', @@ -267,21 +267,21 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test "sets the HTTP charset parameter" do @app = DevelopmentApp - get "/", {}, {'action_dispatch.show_exceptions' => true} + get "/", headers: { 'action_dispatch.show_exceptions' => true } assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end test 'uses logger from env' do @app = DevelopmentApp output = StringIO.new - get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} + get "/", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output) } assert_match(/puke/, output.rewind && output.read) end test 'uses backtrace cleaner from env' do @app = DevelopmentApp cleaner = stub(:clean => ['passed backtrace cleaner']) - get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner} + get "/", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner } assert_match(/passed backtrace cleaner/, body) end @@ -294,14 +294,14 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest 'action_dispatch.logger' => Logger.new(output), 'action_dispatch.backtrace_cleaner' => backtrace_cleaner} - get "/", {}, env + get "/", headers: env assert_operator((output.rewind && output.read).lines.count, :>, 10) end test 'display backtrace when error type is SyntaxError' do @app = DevelopmentApp - get '/original_syntax_error', {}, {'action_dispatch.backtrace_cleaner' => ActiveSupport::BacktraceCleaner.new} + get '/original_syntax_error', headers: { 'action_dispatch.backtrace_cleaner' => ActiveSupport::BacktraceCleaner.new } assert_response 500 assert_select '#Application-Trace' do @@ -312,7 +312,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test 'display backtrace on template missing errors' do @app = DevelopmentApp - get "/missing_template", nil, {} + get "/missing_template" assert_select "header h1", /Template is missing/ @@ -328,7 +328,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test 'display backtrace when error type is SyntaxError wrapped by ActionView::Template::Error' do @app = DevelopmentApp - get '/syntax_error_into_view', {}, {'action_dispatch.backtrace_cleaner' => ActiveSupport::BacktraceCleaner.new} + get '/syntax_error_into_view', headers: { 'action_dispatch.backtrace_cleaner' => ActiveSupport::BacktraceCleaner.new } assert_response 500 assert_select '#Application-Trace' do @@ -344,7 +344,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } end - get '/framework_raises', {}, {'action_dispatch.backtrace_cleaner' => cleaner} + get '/framework_raises', headers: { 'action_dispatch.backtrace_cleaner' => cleaner } # Assert correct error assert_response 500 diff --git a/actionpack/test/dispatch/mount_test.rb b/actionpack/test/dispatch/mount_test.rb index d5a4d8ee11..6a439be2b5 100644 --- a/actionpack/test/dispatch/mount_test.rb +++ b/actionpack/test/dispatch/mount_test.rb @@ -64,7 +64,7 @@ class TestRoutingMount < ActionDispatch::IntegrationTest end def test_mounting_works_with_nested_script_name - get "/foo/sprockets/omg", {}, 'SCRIPT_NAME' => '/foo', 'PATH_INFO' => '/sprockets/omg' + get "/foo/sprockets/omg", headers: { 'SCRIPT_NAME' => '/foo', 'PATH_INFO' => '/sprockets/omg' } assert_equal "/foo/sprockets -- /omg", response.body end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index b765a13fa1..d77341bc64 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -56,7 +56,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest with_test_routing do output = StringIO.new json = "[\"person]\": {\"name\": \"David\"}}" - post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => ActiveSupport::Logger.new(output)} + post "/parse", params: json, headers: { 'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => ActiveSupport::Logger.new(output) } assert_response :bad_request output.rewind && err = output.read assert err =~ /Error occurred while parsing request parameters/ @@ -79,7 +79,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest test 'raw_post is not empty for JSON request' do with_test_routing do - post '/parse', '{"posts": [{"title": "Post Title"}]}', 'CONTENT_TYPE' => 'application/json' + post '/parse', params: '{"posts": [{"title": "Post Title"}]}', headers: { 'CONTENT_TYPE' => 'application/json' } assert_equal '{"posts": [{"title": "Post Title"}]}', request.raw_post end end @@ -87,7 +87,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest private def assert_parses(expected, actual, headers = {}) with_test_routing do - post "/parse", actual, headers + post "/parse", params: actual, headers: headers assert_response :ok assert_equal(expected, TestController.last_request_parameters) end @@ -146,7 +146,7 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest private def assert_parses(expected, actual, headers = {}) with_test_routing(UsersController) do - post "/parse", actual, headers + post "/parse", params: actual, headers: headers assert_response :ok assert_equal(expected, UsersController.last_request_parameters) assert_equal(expected.merge({"action" => "parse"}), UsersController.last_parameters) diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 926472163e..6d1b267ad2 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -37,7 +37,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest end test "parse single utf8 parameter" do - assert_equal({ 'Iñtërnâtiônà lizætiøn_name' => 'Iñtërnâtiônà lizætiøn_value'}, + assert_equal({ 'Iñtërnâtiônà lizætiøn_name' => 'Iñtërnâtiônà lizætiøn_value'}, parse_multipart('single_utf8_param'), "request.request_parameters") assert_equal( 'Iñtërnâtiônà lizætiøn_value', @@ -45,8 +45,8 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest end test "parse bracketed utf8 parameter" do - assert_equal({ 'Iñtërnâtiônà lizætiøn_name' => { - 'Iñtërnâtiônà lizætiøn_nested_name' => 'Iñtërnâtiônà lizætiøn_value'} }, + assert_equal({ 'Iñtërnâtiônà lizætiøn_name' => { + 'Iñtërnâtiônà lizætiøn_nested_name' => 'Iñtërnâtiônà lizætiøn_value'} }, parse_multipart('bracketed_utf8_param'), "request.request_parameters") assert_equal( {'Iñtërnâtiônà lizætiøn_nested_name' => 'Iñtërnâtiônà lizætiøn_value'}, @@ -134,13 +134,13 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest with_test_routing do fixture = FIXTURE_PATH + "/mona_lisa.jpg" params = { :uploaded_data => fixture_file_upload(fixture, "image/jpg") } - post '/read', params + post '/read', params: params end end test "uploads and reads file" do with_test_routing do - post '/read', :uploaded_data => fixture_file_upload(FIXTURE_PATH + "/hello.txt", "text/plain") + post '/read', params: { uploaded_data: fixture_file_upload(FIXTURE_PATH + "/hello.txt", "text/plain") } assert_equal "File: Hello", response.body end end @@ -152,7 +152,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest get ':action', controller: 'multipart_params_parsing_test/test' end headers = { "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x" } - get "/parse", {}, headers + get "/parse", headers: headers assert_response :ok end end @@ -169,7 +169,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest def parse_multipart(name) with_test_routing do headers = fixture(name) - post "/parse", headers.delete("rack.input"), headers + post "/parse", params: headers.delete("rack.input"), headers: headers assert_response :ok TestController.last_request_parameters end diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index 50daafbb54..bc6716525e 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -147,7 +147,7 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest get ':action', :to => ::QueryStringParsingTest::TestController end - get "/parse", nil, "QUERY_STRING" => "foo[]=bar&foo[4]=bar" + get "/parse", headers: { "QUERY_STRING" => "foo[]=bar&foo[4]=bar" } assert_response :bad_request end end @@ -162,8 +162,7 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest middleware.use(EarlyParse) end - - get "/parse", actual + get "/parse", params: actual assert_response :ok assert_equal(expected, ::QueryStringParsingTest::TestController.last_query_parameters) end diff --git a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb index 1de05cbf09..365edf849a 100644 --- a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb @@ -131,7 +131,7 @@ class UrlEncodedParamsParsingTest < ActionDispatch::IntegrationTest test "ambiguous params returns a bad request" do with_test_routing do - post "/parse", "foo[]=bar&foo[4]=bar" + post "/parse", params: "foo[]=bar&foo[4]=bar" assert_response :bad_request end end @@ -148,7 +148,7 @@ class UrlEncodedParamsParsingTest < ActionDispatch::IntegrationTest def assert_parses(expected, actual) with_test_routing do - post "/parse", actual + post "/parse", params: actual assert_response :ok assert_equal expected, TestController.last_request_parameters assert_utf8 TestController.last_request_parameters diff --git a/actionpack/test/dispatch/request_id_test.rb b/actionpack/test/dispatch/request_id_test.rb index a8050b4fab..a307483509 100644 --- a/actionpack/test/dispatch/request_id_test.rb +++ b/actionpack/test/dispatch/request_id_test.rb @@ -41,7 +41,7 @@ class RequestIdResponseTest < ActionDispatch::IntegrationTest test "request id given on request is passed all the way to the response" do with_test_route_set do - get '/', {}, 'HTTP_X_REQUEST_ID' => 'X' * 500 + get '/', headers: { 'HTTP_X_REQUEST_ID' => 'X' * 500 } assert_equal "X" * 255, @response.headers["X-Request-Id"] end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 3373705326..3a95a9025f 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -362,22 +362,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get 'admin/passwords' => "queenbee#passwords", :constraints => ::TestRoutingMapper::IpRestrictor end - get '/admin', {}, {'REMOTE_ADDR' => '192.168.1.100'} + get '/admin', headers: { 'REMOTE_ADDR' => '192.168.1.100' } assert_equal 'queenbee#index', @response.body - get '/admin', {}, {'REMOTE_ADDR' => '10.0.0.100'} + get '/admin', headers: { 'REMOTE_ADDR' => '10.0.0.100' } assert_equal 'pass', @response.headers['X-Cascade'] - get '/admin/accounts', {}, {'REMOTE_ADDR' => '192.168.1.100'} + get '/admin/accounts', headers: { 'REMOTE_ADDR' => '192.168.1.100' } assert_equal 'queenbee#accounts', @response.body - get '/admin/accounts', {}, {'REMOTE_ADDR' => '10.0.0.100'} + get '/admin/accounts', headers: { 'REMOTE_ADDR' => '10.0.0.100' } assert_equal 'pass', @response.headers['X-Cascade'] - get '/admin/passwords', {}, {'REMOTE_ADDR' => '192.168.1.100'} + get '/admin/passwords', headers: { 'REMOTE_ADDR' => '192.168.1.100' } assert_equal 'queenbee#passwords', @response.body - get '/admin/passwords', {}, {'REMOTE_ADDR' => '10.0.0.100'} + get '/admin/passwords', headers: { 'REMOTE_ADDR' => '10.0.0.100' } assert_equal 'pass', @response.headers['X-Cascade'] end @@ -1683,9 +1683,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/products/0001/images/0001' assert_equal 'images#show', @response.body - get '/dashboard', {}, {'REMOTE_ADDR' => '10.0.0.100'} + get '/dashboard', headers: { 'REMOTE_ADDR' => '10.0.0.100' } assert_equal 'pass', @response.headers['X-Cascade'] - get '/dashboard', {}, {'REMOTE_ADDR' => '192.168.1.100'} + get '/dashboard', headers: { 'REMOTE_ADDR' => '192.168.1.100' } assert_equal 'dashboards#show', @response.body end @@ -3573,12 +3573,12 @@ class TestAltApp < ActionDispatch::IntegrationTest end def test_alt_request_with_matched_header - get "/", {}, "HTTP_X_HEADER" => "HEADER" + get "/", headers: { "HTTP_X_HEADER" => "HEADER" } assert_equal "XHeader", @response.body end def test_alt_request_with_unmatched_header - get "/", {}, "HTTP_X_HEADER" => "NON_MATCH" + get "/", headers: { "HTTP_X_HEADER" => "NON_MATCH" } assert_equal "Alternative App", @response.body end end @@ -3773,7 +3773,7 @@ class TestHttpMethods < ActionDispatch::IntegrationTest (RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC4791 + RFC5789).each do |method| test "request method #{method.underscore} can be matched" do - get '/', nil, 'REQUEST_METHOD' => method + get '/', headers: { 'REQUEST_METHOD' => method } assert_equal method, @response.body end end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index c5cd24d06e..2194efa503 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -125,7 +125,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_does_set_secure_cookies_over_https with_test_route_set(:secure => true) do - get '/set_session_value', nil, 'HTTPS' => 'on' + get '/set_session_value', headers: { 'HTTPS' => 'on' } assert_response :success assert_equal "_myapp_session=#{response.body}; path=/; secure; HttpOnly", headers['Set-Cookie'] @@ -331,9 +331,11 @@ class CookieStoreTest < ActionDispatch::IntegrationTest private # Overwrite get to send SessionSecret in env hash - def get(path, parameters = nil, env = {}) - env["action_dispatch.key_generator"] ||= Generator - super + def get(path, *args) + args[0] ||= {} + args[0][:headers] ||= {} + args[0][:headers]["action_dispatch.key_generator"] ||= Generator + super(path, *args) end def with_test_route_set(options = {}) diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index f7a06cfed4..fbd82945cc 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -172,7 +172,7 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest reset! - get '/set_session_value', :_session_id => session_id + get '/set_session_value', params: { _session_id: session_id } assert_response :success assert_not_equal session_id, cookies['_session_id'] end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 323fbc285e..72eaa916bc 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -27,30 +27,30 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "skip exceptions app if not showing exceptions" do @app = ProductionApp assert_raise RuntimeError do - get "/", {}, {'action_dispatch.show_exceptions' => false} + get "/", headers: { 'action_dispatch.show_exceptions' => false } end end test "rescue with error page" do @app = ProductionApp - get "/", {}, {'action_dispatch.show_exceptions' => true} + get "/", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 assert_equal "500 error fixture\n", body - get "/bad_params", {}, {'action_dispatch.show_exceptions' => true} + get "/bad_params", headers: { 'action_dispatch.show_exceptions' => true } assert_response 400 assert_equal "400 error fixture\n", body - get "/not_found", {}, {'action_dispatch.show_exceptions' => true} + get "/not_found", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 assert_equal "404 error fixture\n", body - get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} + get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true } assert_response 405 assert_equal "", body - get "/unknown_http_method", {}, {'action_dispatch.show_exceptions' => true} + get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true } assert_response 405 assert_equal "", body end @@ -61,11 +61,11 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest begin @app = ProductionApp - get "/", {}, {'action_dispatch.show_exceptions' => true} + get "/", headers: { 'action_dispatch.show_exceptions' => true } assert_response 500 assert_equal "500 localized error fixture\n", body - get "/not_found", {}, {'action_dispatch.show_exceptions' => true} + get "/not_found", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 assert_equal "404 error fixture\n", body ensure @@ -76,14 +76,14 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "sets the HTTP charset parameter" do @app = ProductionApp - get "/", {}, {'action_dispatch.show_exceptions' => true} + get "/", headers: { 'action_dispatch.show_exceptions' => true } assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end test "show registered original exception for wrapped exceptions" do @app = ProductionApp - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + get "/not_found_original_exception", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 assert_match(/404 error/, body) end @@ -97,7 +97,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) - get "/not_found_original_exception", {}, {'action_dispatch.show_exceptions' => true} + get "/not_found_original_exception", headers: { 'action_dispatch.show_exceptions' => true } assert_response 404 assert_equal "YOU FAILED BRO", body end @@ -108,7 +108,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) - get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} + get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true } assert_response 405 assert_equal "", body end diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index c3598c5e8e..7ced41bc2e 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -20,7 +20,7 @@ class SSLTest < ActionDispatch::IntegrationTest end def test_allows_https_proxy_header_url - get "http://example.org/", {}, 'HTTP_X_FORWARDED_PROTO' => "https" + get "http://example.org/", headers: { 'HTTP_X_FORWARDED_PROTO' => "https" } assert_response :success end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index 8f79e7bf9a..ce1e1d0a6a 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -39,12 +39,12 @@ module TestUrlGeneration end test "the request's SCRIPT_NAME takes precedence over the route" do - get "/foo", {}, 'SCRIPT_NAME' => "/new", 'action_dispatch.routes' => Routes + get "/foo", headers: { 'SCRIPT_NAME' => "/new", 'action_dispatch.routes' => Routes } assert_equal "/new/foo", response.body end test "the request's SCRIPT_NAME wraps the mounted app's" do - get '/new/bar/foo', {}, 'SCRIPT_NAME' => '/new', 'PATH_INFO' => '/bar/foo', 'action_dispatch.routes' => Routes + get '/new/bar/foo', headers: { 'SCRIPT_NAME' => '/new', 'PATH_INFO' => '/bar/foo', 'action_dispatch.routes' => Routes } assert_equal "/new/bar/foo", response.body end diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index d18656e398..c544e8a10f 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -175,9 +175,10 @@ module ActiveJob original_enqueued_jobs = enqueued_jobs.dup clear_enqueued_jobs args.assert_valid_keys(:job, :args, :at, :queue) + serialized_args = serialize_args_for_assertion(args) yield matching_job = enqueued_jobs.any? do |job| - args.all? { |key, value| value == job[key] } + serialized_args.all? { |key, value| value == job[key] } end assert matching_job, "No enqueued job found with #{args}" ensure @@ -195,9 +196,10 @@ module ActiveJob original_performed_jobs = performed_jobs.dup clear_performed_jobs args.assert_valid_keys(:job, :args, :at, :queue) + serialized_args = serialize_args_for_assertion(args) perform_enqueued_jobs { yield } matching_job = performed_jobs.any? do |job| - args.all? { |key, value| value == job[key] } + serialized_args.all? { |key, value| value == job[key] } end assert matching_job, "No performed job found with #{args}" ensure @@ -239,6 +241,14 @@ module ActiveJob enqueued_jobs.size end end + + def serialize_args_for_assertion(args) + serialized_args = args.dup + if job_args = serialized_args.delete(:args) + serialized_args[:args] = ActiveJob::Arguments.serialize(job_args) + end + serialized_args + end end end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index f34638e7d8..0a23ae33c4 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/date' require 'jobs/hello_job' require 'jobs/logging_job' require 'jobs/nested_job' +require 'models/person' class EnqueuedJobsTest < ActiveJob::TestCase def test_assert_enqueued_jobs @@ -175,6 +176,25 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end end + + def test_assert_enqueued_job_with_global_id_args + ricardo = Person.new(9) + assert_enqueued_with(job: HelloJob, args: [ricardo]) do + HelloJob.perform_later(ricardo) + end + end + + def test_assert_enqueued_job_failure_with_global_id_args + ricardo = Person.new(9) + wilma = Person.new(11) + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_enqueued_with(job: HelloJob, args: [wilma]) do + HelloJob.perform_later(ricardo) + end + end + + assert_equal "No enqueued job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message + end end class PerformedJobsTest < ActiveJob::TestCase @@ -282,4 +302,23 @@ class PerformedJobsTest < ActiveJob::TestCase end end end + + def test_assert_performed_job_with_global_id_args + ricardo = Person.new(9) + assert_performed_with(job: HelloJob, args: [ricardo]) do + HelloJob.perform_later(ricardo) + end + end + + def test_assert_performed_job_failure_with_global_id_args + ricardo = Person.new(9) + wilma = Person.new(11) + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, args: [wilma]) do + HelloJob.perform_later(ricardo) + end + end + + assert_equal "No performed job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 22b1c8e95b..3bd1fe32b4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,27 @@ +* Fixed ActiveRecord::Relation#group method when argument is SQL reserved key word: + + SplitTest.group(:key).count + Property.group(:value).count + + *Bogdan Gusiev* + +* Added the `#or` method on ActiveRecord::Relation, allowing use of the OR + operator to combine WHERE or HAVING clauses. + + Example: + + Post.where('id = 1').or(Post.where('id = 2')) + # => SELECT * FROM posts WHERE (id = 1) OR (id = 2) + + *Sean Griffin*, *Matthew Draper*, *Gael Muller*, *Olivier El Mekki* + +* Don't define autosave association callbacks twice from + `accepts_nested_attributes_for`. + + Fixes #18704. + + *Sean Griffin* + * Integer types will no longer raise a `RangeError` when assigning an attribute, but will instead raise when going to the database. diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index f1e784d771..4fcbc98c62 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -143,7 +143,7 @@ module ActiveRecord stmt.from scope.klass.arel_table stmt.wheres = arel.constraints - count = scope.klass.connection.delete(stmt, 'SQL', scope.bind_values) + count = scope.klass.connection.delete(stmt, 'SQL', scope.bound_attributes) end when :nullify count = scope.update_all(source_reflection.foreign_key => nil) diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 3a184cee37..a6ad09a38a 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -75,7 +75,7 @@ module ActiveRecord column = klass.columns_hash[reflection.type.to_s] substitute = klass.connection.substitute_at(column) - binds << Attribute.with_cast_value(column.name, value, klass.type_for_attribute(column.name)) + binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name)) constraint = constraint.and table[reflection.type].eq substitute end diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index eadc17035e..48b50d9017 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -88,6 +88,11 @@ module ActiveRecord value_before_type_cast == other.value_before_type_cast && type == other.type end + alias eql? == + + def hash + [self.class, name, value_before_type_cast, type].hash + end protected diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 787d07c4c2..6022d0102b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -286,10 +286,10 @@ module ActiveRecord columns = schema_cache.columns_hash(table_name) binds = fixture.map do |name, value| - [columns[name], value] + Relation::QueryAttribute.new(name, value, columns[name].cast_type) end key_list = fixture.keys.map { |name| quote_column_name(name) } - value_list = prepare_binds_for_database(binds).map do |_, value| + value_list = prepare_binds_for_database(binds).map do |value| begin quote(value) rescue TypeError @@ -381,7 +381,7 @@ module ActiveRecord def binds_from_relation(relation, binds) if relation.is_a?(Relation) && binds.empty? - relation, binds = relation.arel, relation.bind_values + relation, binds = relation.arel, relation.bound_attributes end [relation, binds] end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index c18caa2a2f..62123cdc1f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -97,13 +97,7 @@ module ActiveRecord end def prepare_binds_for_database(binds) # :nodoc: - binds.map do |column, value| - if column - column_name = column.name - value = column.cast_type.type_cast_for_database(value) - end - [column_name, value] - end + binds.map(&:value_for_database) end private diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index c941c9f1eb..436552d300 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -114,7 +114,7 @@ module ActiveRecord class BindCollector < Arel::Collectors::Bind def compile(bvs, conn) casted_binds = conn.prepare_binds_for_database(bvs) - super(casted_binds.map { |_, value| conn.quote(value) }) + super(casted_binds.map { |value| conn.quote(value) }) end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 23d8389abb..16f50fc594 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -395,11 +395,9 @@ module ActiveRecord def exec_stmt(sql, name, binds) cache = {} - type_casted_binds = binds.map { |col, val| - [col, type_cast(val, col)] - } + type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, type_casted_binds) do + log(sql, name, binds) do if binds.empty? stmt = @connection.prepare(sql) else @@ -410,7 +408,7 @@ module ActiveRecord end begin - stmt.execute(*type_casted_binds.map { |_, val| val }) + stmt.execute(*type_casted_binds) rescue Mysql::Error => e # Older versions of MySQL leave the prepared statement in a bad # place when an error occurs. To support older MySQL versions, we diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index f4f9747359..d789069f0b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -609,12 +609,10 @@ module ActiveRecord def exec_cache(sql, name, binds) stmt_key = prepare_statement(sql) - type_casted_binds = binds.map { |col, val| - [col, type_cast(val, col)] - } + type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, type_casted_binds, stmt_key) do - @connection.exec_prepared(stmt_key, type_casted_binds.map { |_, val| val }) + log(sql, name, binds, stmt_key) do + @connection.exec_prepared(stmt_key, type_casted_binds) end rescue ActiveRecord::StatementInvalid => e pgerror = e.original_exception diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 52dce6291a..02a3b65934 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -50,16 +50,6 @@ module ActiveRecord end end - class SQLite3String < Type::String # :nodoc: - def type_cast_for_database(value) - if value.is_a?(::String) && value.encoding == Encoding::ASCII_8BIT - value.encode(Encoding::UTF_8, undef: :replace) - else - super - end - end - end - # The SQLite3 adapter works SQLite 3.6.16 or newer # with the sqlite3-ruby drivers (available as gem from https://rubygems.org/gems/sqlite3). # @@ -239,6 +229,12 @@ module ActiveRecord case value when BigDecimal value.to_f + when String + if value.encoding == Encoding::ASCII_8BIT + super(value.encode(Encoding::UTF_8)) + else + super + end else super end @@ -280,11 +276,9 @@ module ActiveRecord end def exec_query(sql, name = nil, binds = []) - type_casted_binds = binds.map { |col, val| - [col, type_cast(val, col)] - } + type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) } - log(sql, name, type_casted_binds) do + log(sql, name, binds) do # Don't cache statements if they are not prepared if without_prepared_statement?(binds) stmt = @connection.prepare(sql) @@ -302,7 +296,7 @@ module ActiveRecord stmt = cache[:stmt] cols = cache[:cols] ||= stmt.columns stmt.reset! - stmt.bind_params type_casted_binds.map { |_, val| val } + stmt.bind_params type_casted_binds end ActiveRecord::Result.new(cols, stmt.to_a) @@ -498,7 +492,6 @@ module ActiveRecord def initialize_type_map(m) super m.register_type(/binary/i, SQLite3Binary.new) - register_class_with_limit m, %r(char)i, SQLite3String end def table_structure(table_name) diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index a5c7279db9..6b26d7be78 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -20,18 +20,14 @@ module ActiveRecord @odd = false end - def render_bind(column, value) - if column - if column.binary? && value - # This specifically deals with the PG adapter that casts bytea columns into a Hash. - value = value[:value] if value.is_a?(Hash) - value = "<#{value.bytesize} bytes of binary data>" - end - - [column.name, value] + def render_bind(attribute) + value = if attribute.type.binary? && attribute.value + "<#{attribute.value.bytesize} bytes of binary data>" else - [nil, value] + attribute.value_for_database end + + [attribute.name, value] end def sql(event) @@ -47,9 +43,7 @@ module ActiveRecord binds = nil unless (payload[:binds] || []).empty? - binds = " " + payload[:binds].map { |col,v| - render_bind(col, v) - }.inspect + binds = " " + payload[:binds].map { |attr| render_bind(attr) }.inspect end if odd? diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 846e1162a9..919bbfa649 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -312,7 +312,6 @@ module ActiveRecord attr_names.each do |association_name| if reflection = _reflect_on_association(association_name) reflection.autosave = true - add_autosave_association_callbacks(reflection) nested_attributes_options = self.nested_attributes_options.dup nested_attributes_options[association_name.to_sym] = options diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index b406da14dc..63ca29305d 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -75,5 +75,9 @@ module ActiveRecord def exists?(_id = false) false end + + def or(other) + other.spawn + end end end diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 91c9a0db99..4e597590e9 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -7,7 +7,7 @@ module ActiveRecord delegate :find_by, :find_by!, to: :all delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, to: :all delegate :find_each, :find_in_batches, to: :all - delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, + delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :or, :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :create_with, :uniq, :distinct, :references, :none, :unscope, to: :all delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 07e27fe59e..8073eb99dd 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -81,7 +81,7 @@ module ActiveRecord end relation = scope.where(@klass.primary_key => (id_was || id)) - bvs = binds + relation.bind_values + bvs = binds + relation.bound_attributes um = relation .arel .compile_update(substitutes, @klass.primary_key) @@ -95,11 +95,11 @@ module ActiveRecord def substitute_values(values) # :nodoc: binds = values.map do |arel_attr, value| - [@klass.columns_hash[arel_attr.name], value] + QueryAttribute.new(arel_attr.name, value, klass.type_for_attribute(arel_attr.name)) end - substitutes = values.each_with_index.map do |(arel_attr, _), i| - [arel_attr, @klass.connection.substitute_at(binds[i][0])] + substitutes = values.map do |(arel_attr, _)| + [arel_attr, connection.substitute_at(klass.columns_hash[arel_attr.name])] end [substitutes, binds] @@ -342,7 +342,7 @@ module ActiveRecord stmt.wheres = arel.constraints end - @klass.connection.update stmt, 'SQL', bind_values + @klass.connection.update stmt, 'SQL', bound_attributes end # Updates an object (or multiple objects) and saves it to the database, if validations pass. @@ -488,7 +488,7 @@ module ActiveRecord stmt.wheres = arel.constraints end - affected = @klass.connection.delete(stmt, 'SQL', bind_values) + affected = @klass.connection.delete(stmt, 'SQL', bound_attributes) reset affected @@ -558,9 +558,9 @@ module ActiveRecord find_with_associations { |rel| relation = rel } end - binds = relation.bind_values + binds = relation.bound_attributes binds = connection.prepare_binds_for_database(binds) - binds.map! { |_, value| connection.quote(value) } + binds.map! { |value| connection.quote(value) } collect = visitor.accept(relation.arel.ast, Arel::Collectors::Bind.new) collect.substitute_binds(binds).join end @@ -634,7 +634,7 @@ module ActiveRecord private def exec_queries - @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values) + @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bound_attributes) preload = preload_values preload += includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 724bba5f87..c3c4d7f1ce 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -166,7 +166,7 @@ module ActiveRecord relation.select_values = column_names.map { |cn| columns_hash.key?(cn) ? arel_table[cn] : cn } - result = klass.connection.select_all(relation.arel, nil, bind_values) + result = klass.connection.select_all(relation.arel, nil, bound_attributes) result.cast_values(klass.column_types) end end @@ -244,7 +244,7 @@ module ActiveRecord query_builder = relation.arel end - result = @klass.connection.select_all(query_builder, nil, bind_values) + result = @klass.connection.select_all(query_builder, nil, bound_attributes) row = result.first value = row && row.values.first column = result.column_types.fetch(column_alias) do @@ -300,11 +300,11 @@ module ActiveRecord relation.group_values = group relation.select_values = select_values - calculated_data = @klass.connection.select_all(relation, nil, relation.bind_values) + calculated_data = @klass.connection.select_all(relation, nil, relation.bound_attributes) if association key_ids = calculated_data.collect { |row| row[group_aliases.first] } - key_records = association.klass.base_class.find(key_ids) + key_records = association.klass.base_class.where(association.klass.base_class.primary_key => key_ids) key_records = Hash[key_records.map { |r| [r.id, r] }] end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 77bf1217e6..fb47d915ff 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -311,7 +311,7 @@ module ActiveRecord end end - connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false + connection.select_value(relation, "#{name} Exists", relation.bound_attributes) ? true : false end # This method is called whenever no records are found with either a single @@ -365,7 +365,7 @@ module ActiveRecord [] else arel = relation.arel - rows = connection.select_all(arel, 'SQL', relation.bind_values) + rows = connection.select_all(arel, 'SQL', relation.bound_attributes) join_dependency.instantiate(rows, aliases) end end @@ -410,7 +410,7 @@ module ActiveRecord relation = relation.except(:select).select(values).distinct! arel = relation.arel - id_rows = @klass.connection.select_all(arel, 'SQL', relation.bind_values) + id_rows = @klass.connection.select_all(arel, 'SQL', relation.bound_attributes) id_rows.map {|row| row[primary_key]} end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 7b605db88c..43e9afe853 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -108,7 +108,7 @@ module ActiveRecord else if can_be_bound?(column_name, value) result[column_name] = Arel::Nodes::BindParam.new - binds << Attribute.with_cast_value(column_name.to_s, value, table.type(column_name)) + binds << Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name)) end end end diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb new file mode 100644 index 0000000000..e69319b4de --- /dev/null +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -0,0 +1,19 @@ +require 'active_record/attribute' + +module ActiveRecord + class Relation + class QueryAttribute < Attribute + def type_cast(value) + value + end + + def value_for_database + @value_for_database ||= super + end + + def with_cast_value(value) + QueryAttribute.new(name, value, type) + end + end + end +end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index bc5f126a23..7514401072 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1,4 +1,5 @@ require "active_record/relation/from_clause" +require "active_record/relation/query_attribute" require "active_record/relation/where_clause" require "active_record/relation/where_clause_factory" require 'active_model/forbidden_attributes_protection' @@ -96,16 +97,6 @@ module ActiveRecord from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds end - def bind_values - # convert to old style - bound_attributes.map do |attribute| - if attribute.name - column = ConnectionAdapters::Column.new(attribute.name, nil, attribute.type) - end - [column, attribute.value_before_type_cast] - end - end - def create_with_value # :nodoc: @values[:create_with] || {} end @@ -591,6 +582,37 @@ module ActiveRecord unscope(where: conditions.keys).where(conditions) end + # Returns a new relation, which is the logical union of this relation and the one passed as an + # argument. + # + # The two relations must be structurally compatible: they must be scoping the same model, and + # they must differ only by +where+ (if no +group+ has been defined) or +having+ (if a +group+ is + # present). Neither relation may have a +limit+, +offset+, or +uniq+ set. + # + # Post.where("id = 1").or(Post.where("id = 2")) + # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'id = 2')) + # + def or(other) + spawn.or!(other) + end + + def or!(other) # :nodoc: + unless structurally_compatible_for_or?(other) + raise ArgumentError, 'Relation passed to #or must be structurally compatible' + end + + self.where_clause = self.where_clause.or(other.where_clause) + self.having_clause = self.having_clause.or(other.having_clause) + + self + end + + private def structurally_compatible_for_or?(other) # :nodoc: + Relation::SINGLE_VALUE_METHODS.all? { |m| send("#{m}_value") == other.send("#{m}_value") } && + (Relation::MULTI_VALUE_METHODS - [:extending]).all? { |m| send("#{m}_values") == other.send("#{m}_values") } && + (Relation::CLAUSE_METHODS - [:having, :where]).all? { |m| send("#{m}_clause") != other.send("#{m}_clause") } + end + # Allows to specify a HAVING clause. Note that you can't use HAVING # without also specifying a GROUP clause. # @@ -865,11 +887,11 @@ module ActiveRecord arel.having(having_clause.ast) unless having_clause.empty? arel.take(connection.sanitize_limit(limit_value)) if limit_value arel.skip(offset_value.to_i) if offset_value - arel.group(*group_values.uniq.reject(&:blank?)) unless group_values.empty? + arel.group(*arel_columns(group_values.uniq.reject(&:blank?))) unless group_values.empty? build_order(arel) - build_select(arel, select_values.uniq) + build_select(arel) arel.distinct(distinct_value) arel.from(build_from) unless from_clause.empty? @@ -968,22 +990,24 @@ module ActiveRecord .map { |join| table.create_string_join(Arel.sql(join)) } end - def build_select(arel, selects) - if !selects.empty? - expanded_select = selects.map do |field| - if (Symbol === field || String === field) && columns_hash.key?(field.to_s) - arel_table[field] - else - field - end - end - - arel.project(*expanded_select) + def build_select(arel) + if select_values.any? + arel.project(*arel_columns(select_values.uniq)) else arel.project(@klass.arel_table[Arel.star]) end end + def arel_columns(columns) + columns.map do |field| + if (Symbol === field || String === field) && columns_hash.key?(field.to_s) + arel_table[field] + else + field + end + end + end + def reverse_sql_order(order_query) order_query = ["#{quoted_table_name}.#{quoted_primary_key} ASC"] if order_query.empty? diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index ae5667dfd6..f9b9e640ec 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -31,6 +31,19 @@ module ActiveRecord ) end + def or(other) + if empty? + self + elsif other.empty? + other + else + WhereClause.new( + [ast.or(other.ast)], + binds + other.binds + ) + end + end + def to_h(table_name = nil) equalities = predicates.grep(Arel::Nodes::Equality) if table_name diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index 3047a81ec4..95986c820c 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -48,7 +48,7 @@ module ActiveRecord def sql_for(binds, connection) val = @values.dup binds = connection.prepare_binds_for_database(binds) - @indexes.each { |i| val[i] = connection.quote(binds.shift.last) } + @indexes.each { |i| val[i] = connection.quote(binds.shift) } val.join end end @@ -67,21 +67,21 @@ module ActiveRecord end class BindMap # :nodoc: - def initialize(bind_values) + def initialize(bound_attributes) @indexes = [] - @bind_values = bind_values + @bound_attributes = bound_attributes - bind_values.each_with_index do |(_, value), i| - if Substitute === value + bound_attributes.each_with_index do |attr, i| + if Substitute === attr.value @indexes << i end end end def bind(values) - bvs = @bind_values.map(&:dup) - @indexes.each_with_index { |offset,i| bvs[offset][1] = values[i] } - bvs + bas = @bound_attributes.dup + @indexes.each_with_index { |offset,i| bas[offset] = bas[offset].with_cast_value(values[i]) } + bas end end @@ -89,7 +89,7 @@ module ActiveRecord def self.create(connection, block = Proc.new) relation = block.call Params.new - bind_map = BindMap.new relation.bind_values + bind_map = BindMap.new relation.bound_attributes query_builder = connection.cacheable_query relation.arel new query_builder, bind_map end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 99e3d7021d..ecf1368a91 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -193,7 +193,7 @@ module ActiveRecord author = Author.create!(name: 'john') Post.create!(author: author, title: 'foo', body: 'bar') query = author.posts.where(title: 'foo').select(:title) - assert_equal({"title" => "foo"}, @connection.select_one(query.arel, nil, query.bind_values)) + assert_equal({"title" => "foo"}, @connection.select_one(query.arel, nil, query.bound_attributes)) assert_equal({"title" => "foo"}, @connection.select_one(query)) assert @connection.select_all(query).is_a?(ActiveRecord::Result) assert_equal "foo", @connection.select_value(query) @@ -203,7 +203,7 @@ module ActiveRecord def test_select_methods_passing_a_relation Post.create!(title: 'foo', body: 'bar') query = Post.where(title: 'foo').select(:title) - assert_equal({"title" => "foo"}, @connection.select_one(query.arel, nil, query.bind_values)) + assert_equal({"title" => "foo"}, @connection.select_one(query.arel, nil, query.bound_attributes)) assert_equal({"title" => "foo"}, @connection.select_one(query)) assert @connection.select_all(query).is_a?(ActiveRecord::Result) assert_equal "foo", @connection.select_value(query) diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb index ce01b16362..4762ef43b5 100644 --- a/activerecord/test/cases/adapters/mysql/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql/connection_test.rb @@ -94,7 +94,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase with_example_table do @connection.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = ?', nil, [[nil, 1]]) + 'SELECT id, data FROM ex WHERE id = ?', nil, [ActiveRecord::Relation::QueryAttribute.new("id", 1, ActiveRecord::Type::Value.new)]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length @@ -106,10 +106,10 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_exec_typecasts_bind_vals with_example_table do @connection.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') - column = @connection.columns('ex').find { |col| col.name == 'id' } + bind = ActiveRecord::Relation::QueryAttribute.new("id", "1-fuu", ActiveRecord::Type::Integer.new) result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = ?', nil, [[column, '1-fuu']]) + 'SELECT id, data FROM ex WHERE id = ?', nil, [bind]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index 85db8f4614..1e6511620a 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -129,10 +129,10 @@ module ActiveRecord private def insert(ctx, data, table='ex') - binds = data.map { |name, value| - [ctx.columns(table).find { |x| x.name == name }, value] + binds = data.map { |name, value| + Relation::QueryAttribute.new(name, value, Type::Value.new) } - columns = binds.map(&:first).map(&:name) + columns = binds.map(&:name) sql = "INSERT INTO #{table} (#{columns.join(", ")}) VALUES (#{(['?'] * columns.length).join(', ')})" diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index ab7fd3c6d5..7bf8d12eae 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -126,11 +126,11 @@ module ActiveRecord end def test_statement_key_is_logged - bindval = 1 - @connection.exec_query('SELECT $1::integer', 'SQL', [[nil, bindval]]) + bind = Relation::QueryAttribute.new(nil, 1, Type::Value.new) + @connection.exec_query('SELECT $1::integer', 'SQL', [bind]) name = @subscriber.payloads.last[:statement_name] assert name - res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(#{bindval})") + res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(1)") plan = res.column_types['QUERY PLAN'].type_cast_from_database res.rows.first.first assert_operator plan.length, :>, 0 end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 6bb2b26cd5..15c2f48ede 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -284,7 +284,7 @@ module ActiveRecord string = @connection.quote('foo') @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = $1', nil, [[nil, 1]]) + 'SELECT id, data FROM ex WHERE id = $1', nil, [bind_param(1)]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length @@ -298,9 +298,9 @@ module ActiveRecord string = @connection.quote('foo') @connection.exec_query("INSERT INTO ex (id, data) VALUES (1, #{string})") - column = @connection.columns('ex').find { |col| col.name == 'id' } + bind = ActiveRecord::Relation::QueryAttribute.new("id", "1-fuu", ActiveRecord::Type::Integer.new) result = @connection.exec_query( - 'SELECT id, data FROM ex WHERE id = $1', nil, [[column, '1-fuu']]) + 'SELECT id, data FROM ex WHERE id = $1', nil, [bind]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length @@ -437,10 +437,10 @@ module ActiveRecord private def insert(ctx, data) - binds = data.map { |name, value| - [ctx.columns('ex').find { |x| x.name == name }, value] + binds = data.map { |name, value| + bind_param(value, name) } - columns = binds.map(&:first).map(&:name) + columns = binds.map(&:name) bind_subs = columns.length.times.map { |x| "$#{x + 1}" } @@ -457,6 +457,10 @@ module ActiveRecord def connection_without_insert_returning ActiveRecord::Base.postgresql_connection(ActiveRecord::Base.configurations['arunit'].merge(:insert_returning => false)) end + + def bind_param(value, name = nil) + ActiveRecord::Relation::QueryAttribute.new(name, value, ActiveRecord::Type::Value.new) + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb index 99c26c4bf7..6937145439 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_authorization_test.rb @@ -55,7 +55,7 @@ class SchemaAuthorizationTest < ActiveRecord::TestCase set_session_auth USERS.each do |u| set_session_auth u - assert_equal u, @connection.exec_query("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [[nil, 1]]).first['name'] + assert_equal u, @connection.exec_query("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [bind_param(1)]).first['name'] set_session_auth end end @@ -67,7 +67,7 @@ class SchemaAuthorizationTest < ActiveRecord::TestCase USERS.each do |u| @connection.clear_cache! set_session_auth u - assert_equal u, @connection.exec_query("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [[nil, 1]]).first['name'] + assert_equal u, @connection.exec_query("SELECT name FROM #{TABLE_NAME} WHERE id = $1", 'SQL', [bind_param(1)]).first['name'] set_session_auth end end @@ -111,4 +111,7 @@ class SchemaAuthorizationTest < ActiveRecord::TestCase @connection.session_auth = auth || 'default' end + def bind_param(value) + ActiveRecord::Relation::QueryAttribute.new(nil, value, ActiveRecord::Type::Value.new) + end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 642b63357f..83e35ad1a1 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -151,10 +151,10 @@ class SchemaTest < ActiveRecord::TestCase def test_schema_change_with_prepared_stmt altered = false - @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]] + @connection.exec_query "select * from developers where id = $1", 'sql', [bind_param(1)] @connection.exec_query "alter table developers add column zomg int", 'sql', [] altered = true - @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]] + @connection.exec_query "select * from developers where id = $1", 'sql', [bind_param(1)] ensure # We are not using DROP COLUMN IF EXISTS because that syntax is only # supported by pg 9.X @@ -435,6 +435,10 @@ class SchemaTest < ActiveRecord::TestCase assert_equal this_index_column, this_index.columns[0] assert_equal this_index_name, this_index.name end + + def bind_param(value) + ActiveRecord::Relation::QueryAttribute.new(nil, value, ActiveRecord::Type::Value.new) + end end class SchemaForeignKeyTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb index 274e358e4a..c1d9b7c273 100644 --- a/activerecord/test/cases/adapters/sqlite3/quoting_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/quoting_test.rb @@ -85,7 +85,7 @@ module ActiveRecord def test_quoting_binary_strings value = "hello".encode('ascii-8bit') - type = SQLite3String.new + type = Type::String.new assert_equal "'hello'", @conn.quote(type.type_cast_for_database(value)) end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 029663e7f4..0527bf8b15 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -83,8 +83,7 @@ module ActiveRecord def test_exec_insert with_example_table do - column = @conn.columns('ex').find { |col| col.name == 'number' } - vals = [[column, 10]] + vals = [Relation::QueryAttribute.new("number", 10, Type::Value.new)] @conn.exec_insert('insert into ex (number) VALUES (?)', 'SQL', vals) result = @conn.exec_query( @@ -157,7 +156,7 @@ module ActiveRecord with_example_table 'id int, data string' do @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') result = @conn.exec_query( - 'SELECT id, data FROM ex WHERE id = ?', nil, [[nil, 1]]) + 'SELECT id, data FROM ex WHERE id = ?', nil, [Relation::QueryAttribute.new(nil, 1, Type::Value.new)]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length @@ -169,10 +168,9 @@ module ActiveRecord def test_exec_query_typecasts_bind_vals with_example_table 'id int, data string' do @conn.exec_query('INSERT INTO ex (id, data) VALUES (1, "foo")') - column = @conn.columns('ex').find { |col| col.name == 'id' } result = @conn.exec_query( - 'SELECT id, data FROM ex WHERE id = ?', nil, [[column, '1-fuu']]) + 'SELECT id, data FROM ex WHERE id = ?', nil, [Relation::QueryAttribute.new("id", "1-fuu", Type::Integer.new)]) assert_equal 1, result.rows.length assert_equal 2, result.columns.length diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index b6ab576d76..68d5a232ba 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -40,7 +40,7 @@ module ActiveRecord def test_binds_are_logged sub = @connection.substitute_at(@pk) - binds = [[@pk, 1]] + binds = [Relation::QueryAttribute.new("id", 1, Type::Value.new)] sql = "select * from topics where id = #{sub.to_sql}" @connection.exec_query(sql, 'SQL', binds) @@ -49,29 +49,17 @@ module ActiveRecord assert_equal binds, message[4][:binds] end - def test_binds_are_logged_after_type_cast - sub = @connection.substitute_at(@pk) - binds = [[@pk, "3"]] - sql = "select * from topics where id = #{sub.to_sql}" - - @connection.exec_query(sql, 'SQL', binds) - - message = @subscriber.calls.find { |args| args[4][:sql] == sql } - assert_equal [[@pk, 3]], message[4][:binds] - end - def test_find_one_uses_binds Topic.find(1) - binds = [[@pk, 1]] - message = @subscriber.calls.find { |args| args[4][:binds] == binds } + message = @subscriber.calls.find { |args| args[4][:binds].any? { |attr| attr.value == 1 } } assert message, 'expected a message with binds' end - def test_logs_bind_vars + def test_logs_bind_vars_after_type_cast payload = { :name => 'SQL', :sql => 'select * from topics where id = ?', - :binds => [[@pk, 10]] + :binds => [Relation::QueryAttribute.new("id", "10", Type::Integer.new)] } event = ActiveSupport::Notifications::Event.new( 'foo', diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 299217214e..f47568f2f5 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -631,4 +631,9 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal({ "has trinket" => part.id }, ShipPart.joins(:trinkets).group("ship_parts.name").sum(:id)) end + + def test_calculation_grouped_by_association_doesnt_error_when_no_records_have_association + Client.update_all(client_of: nil) + assert_equal({ nil => Client.count }, Client.group(:firm).count) + end end diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index 9d25bdd82a..f1d5511bb8 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -28,7 +28,7 @@ if ActiveRecord::Base.connection.supports_explain? assert_match "SELECT", sql if binds.any? assert_equal 1, binds.length - assert_equal "honda", binds.flatten.last + assert_equal "honda", binds.last.value else assert_match 'honda', sql end diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 97c0350911..4192d12ff4 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -63,14 +63,6 @@ class LogSubscriberTest < ActiveRecord::TestCase assert_match(/ruby rails/, logger.debugs.first) end - def test_ignore_binds_payload_with_nil_column - event = Struct.new(:duration, :payload) - - logger = TestDebugLogSubscriber.new - logger.sql(event.new(0, sql: 'hi mom!', binds: [[nil, 1]])) - assert_equal 1, logger.debugs.length - end - def test_basic_query_logging Developer.all.load wait diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 8bc42b8890..a9e93ad1b6 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -1037,4 +1037,14 @@ class TestHasManyAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveR ShipPart.create!(:ship => @ship, :name => "Stern") assert_no_queries { @ship.valid? } end + + test "circular references do not perform unnecessary queries" do + ship = Ship.new(name: "The Black Rock") + part = ship.parts.build(name: "Stern") + ship.treasures.build(looter: part) + + assert_queries 3 do + ship.save! + end + end end diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index fe0854b7b4..0a2e874e4f 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -82,10 +82,10 @@ class RelationMergingTest < ActiveRecord::TestCase left = Post.where(title: "omg").where(comments_count: 1) right = Post.where(title: "wtf").where(title: "bbq") - expected = [left.bind_values[1]] + right.bind_values + expected = [left.bound_attributes[1]] + right.bound_attributes merged = left.merge(right) - assert_equal expected, merged.bind_values + assert_equal expected, merged.bound_attributes assert !merged.to_sql.include?("omg") assert merged.to_sql.include?("wtf") assert merged.to_sql.include?("bbq") diff --git a/activerecord/test/cases/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb new file mode 100644 index 0000000000..2006fc9611 --- /dev/null +++ b/activerecord/test/cases/relation/or_test.rb @@ -0,0 +1,84 @@ +require "cases/helper" +require 'models/post' + +module ActiveRecord + class OrTest < ActiveRecord::TestCase + fixtures :posts + + def test_or_with_relation + expected = Post.where('id = 1 or id = 2').to_a + assert_equal expected, Post.where('id = 1').or(Post.where('id = 2')).to_a + end + + def test_or_identity + expected = Post.where('id = 1').to_a + assert_equal expected, Post.where('id = 1').or(Post.where('id = 1')).to_a + end + + def test_or_with_null_left + expected = Post.where('id = 1').to_a + assert_equal expected, Post.none.or(Post.where('id = 1')).to_a + end + + def test_or_with_null_right + expected = Post.where('id = 1').to_a + assert_equal expected, Post.where('id = 1').or(Post.none).to_a + end + + def test_or_with_bind_params + assert_equal Post.find([1, 2]), Post.where(id: 1).or(Post.where(id: 2)).to_a + end + + def test_or_with_null_both + expected = Post.none.to_a + assert_equal expected, Post.none.or(Post.none).to_a + end + + def test_or_without_left_where + expected = Post.all + assert_equal expected, Post.or(Post.where('id = 1')).to_a + end + + def test_or_without_right_where + expected = Post.all + assert_equal expected, Post.where('id = 1').or(Post.all).to_a + end + + def test_or_preserves_other_querying_methods + expected = Post.where('id = 1 or id = 2 or id = 3').order('body asc').to_a + partial = Post.order('body asc') + assert_equal expected, partial.where('id = 1').or(partial.where(:id => [2, 3])).to_a + assert_equal expected, Post.order('body asc').where('id = 1').or(Post.order('body asc').where(:id => [2, 3])).to_a + end + + def test_or_with_incompatible_relations + assert_raises ArgumentError do + Post.order('body asc').where('id = 1').or(Post.order('id desc').where(:id => [2, 3])).to_a + end + end + + def test_or_when_grouping + groups = Post.where('id < 10').group('body').select('body, COUNT(*) AS c') + expected = groups.having("COUNT(*) > 1 OR body like 'Such%'").to_a.map {|o| [o.body, o.c] } + assert_equal expected, groups.having('COUNT(*) > 1').or(groups.having("body like 'Such%'")).to_a.map {|o| [o.body, o.c] } + end + + def test_or_with_named_scope + expected = Post.where("id = 1 or body LIKE '\%a\%'").to_a + assert_equal expected, Post.where('id = 1').or(Post.containing_the_letter_a) + end + + def test_or_inside_named_scope + expected = Post.where("body LIKE '\%a\%' OR title LIKE ?", "%'%").order('id DESC').to_a + assert_equal expected, Post.order(id: :desc).typographically_interesting + end + + def test_or_on_loaded_relation + expected = Post.where('id = 1 or id = 2').to_a + p = Post.where('id = 1') + p.load + assert_equal p.loaded?, true + assert_equal expected, p.or(Post.where('id = 2')).to_a + end + end +end diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb index db18980e0b..c20ed94d90 100644 --- a/activerecord/test/cases/relation/where_clause_test.rb +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -145,6 +145,26 @@ class ActiveRecord::Relation assert_equal where_clause.ast, where_clause_with_empty.ast end + test "or joins the two clauses using OR" do + where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)]) + other_clause = WhereClause.new([table["name"].eq(bind_param)], [attribute("name", "Sean")]) + expected_ast = + Arel::Nodes::Grouping.new( + Arel::Nodes::Or.new(table["id"].eq(bind_param), table["name"].eq(bind_param)) + ) + expected_binds = where_clause.binds + other_clause.binds + + assert_equal expected_ast.to_sql, where_clause.or(other_clause).ast.to_sql + assert_equal expected_binds, where_clause.or(other_clause).binds + end + + test "or returns an empty where clause when either side is empty" do + where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)]) + + assert_equal WhereClause.empty, where_clause.or(WhereClause.empty) + assert_equal WhereClause.empty, WhereClause.empty.or(where_clause) + end + private def table diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 1e39a9bd28..5c5ab499a0 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -16,6 +16,7 @@ require 'models/engine' require 'models/tyre' require 'models/minivan' require 'models/aircraft' +require "models/possession" class RelationTest < ActiveRecord::TestCase @@ -1482,6 +1483,10 @@ class RelationTest < ActiveRecord::TestCase assert_equal [post], having_then_where end + def test_grouping_by_column_with_reserved_name + assert_equal [], Possession.select(:where).group(:where).to_a + end + def test_references_triggers_eager_loading scope = Post.includes(:comments) assert !scope.eager_loading? @@ -1758,7 +1763,7 @@ class RelationTest < ActiveRecord::TestCase end def test_merging_keeps_lhs_bind_parameters - binds = [ActiveRecord::Attribute.with_cast_value("id", 20, Post.type_for_attribute("id"))] + binds = [ActiveRecord::Relation::QueryAttribute.new("id", 20, Post.type_for_attribute("id"))] right = Post.where(id: 20) left = Post.where(id: 10) diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 7b637c9e3f..052b1c9690 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -18,6 +18,7 @@ class Post < ActiveRecord::Base end scope :containing_the_letter_a, -> { where("body LIKE '%a%'") } + scope :titled_with_an_apostrophe, -> { where("title LIKE '%''%'") } scope :ranked_by_comments, -> { order("comments_count DESC") } scope :limit_by, lambda {|l| limit(l) } @@ -43,6 +44,8 @@ class Post < ActiveRecord::Base scope :tagged_with, ->(id) { joins(:taggings).where(taggings: { tag_id: id }) } scope :tagged_with_comment, ->(comment) { joins(:taggings).where(taggings: { comment: comment }) } + scope :typographically_interesting, -> { containing_the_letter_a.or(titled_with_an_apostrophe) } + has_many :comments do def find_most_recent order("id DESC").first diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index c2f6d492d8..312caef604 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -4,6 +4,7 @@ class Ship < ActiveRecord::Base belongs_to :pirate belongs_to :update_only_pirate, :class_name => 'Pirate' has_many :parts, :class_name => 'ShipPart' + has_many :treasures accepts_nested_attributes_for :parts, :allow_destroy => true accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc(&:empty?) diff --git a/activerecord/test/models/treasure.rb b/activerecord/test/models/treasure.rb index a69d3fd3df..ffc65466d5 100644 --- a/activerecord/test/models/treasure.rb +++ b/activerecord/test/models/treasure.rb @@ -1,6 +1,7 @@ class Treasure < ActiveRecord::Base has_and_belongs_to_many :parrots belongs_to :looter, :polymorphic => true + belongs_to :ship has_many :price_estimates, :as => :estimate_of has_and_belongs_to_many :rich_people, join_table: 'peoples_treasures', validate: false diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 21b23d8e0c..1add85cc05 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -772,6 +772,7 @@ ActiveRecord::Schema.define do t.column :type, :string t.column :looter_id, :integer t.column :looter_type, :string + t.belongs_to :ship end create_table :tyres, force: true do |t| diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 2caf0b27ee..367e857d27 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,15 @@ +* Add `file_fixture` to `ActiveSupport::TestCase`. + It provides a simple mechanism to access sample files in your test cases. + + By default file fixtures are stored in `test/fixtures/files`. This can be + configured per test-case using the `file_fixture_path` class attribute. + + *Yves Senn* + +* Return value of yielded block in `File.atomic_write`. + + *Ian Ker-Seymer* + * Duplicate frozen array when assigning it to a HashWithIndifferentAccess so that it doesn't raise a `RuntimeError` when calling `map!` on it in `convert_value`. diff --git a/activesupport/lib/active_support/core_ext/file/atomic.rb b/activesupport/lib/active_support/core_ext/file/atomic.rb index 38374af388..fad6fa8d9d 100644 --- a/activesupport/lib/active_support/core_ext/file/atomic.rb +++ b/activesupport/lib/active_support/core_ext/file/atomic.rb @@ -20,7 +20,7 @@ class File temp_file = Tempfile.new(basename(file_name), temp_dir) temp_file.binmode - yield temp_file + return_val = yield temp_file temp_file.close if File.exist?(file_name) @@ -40,6 +40,9 @@ class File chown(old_stat.uid, old_stat.gid, file_name) # This operation will affect filesystem ACL's chmod(old_stat.mode, file_name) + + # Make sure we return the result of the yielded block + return_val rescue Errno::EPERM, Errno::EACCES # Changing file ownership failed, moving on. end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index e03e7c30d8..664cc15a29 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -373,7 +373,7 @@ module ActiveSupport #:nodoc: # Is the provided constant path defined? def qualified_const_defined?(path) - Object.qualified_const_defined?(path.sub(/^::/, ''), false) + Object.const_defined?(path, false) end # Given +path+, a filesystem path to a ruby file, return an array of @@ -607,7 +607,7 @@ module ActiveSupport #:nodoc: def autoloaded?(desc) return false if desc.is_a?(Module) && desc.anonymous? name = to_constant_name desc - return false unless qualified_const_defined? name + return false unless qualified_const_defined?(name) return autoloaded_constants.include?(name) end diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index c7d6c62129..739823bd56 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -8,6 +8,7 @@ require 'active_support/testing/declarative' require 'active_support/testing/isolation' require 'active_support/testing/constant_lookup' require 'active_support/testing/time_helpers' +require 'active_support/testing/file_fixtures' require 'active_support/core_ext/kernel/reporting' module ActiveSupport @@ -55,6 +56,7 @@ module ActiveSupport include ActiveSupport::Testing::Assertions include ActiveSupport::Testing::Deprecation include ActiveSupport::Testing::TimeHelpers + include ActiveSupport::Testing::FileFixtures extend ActiveSupport::Testing::Declarative # test/unit backwards compatibility methods diff --git a/activesupport/lib/active_support/testing/file_fixtures.rb b/activesupport/lib/active_support/testing/file_fixtures.rb new file mode 100644 index 0000000000..4c6a0801b8 --- /dev/null +++ b/activesupport/lib/active_support/testing/file_fixtures.rb @@ -0,0 +1,34 @@ +module ActiveSupport + module Testing + # Adds simple access to sample files called file fixtures. + # File fixtures are normal files stored in + # <tt>ActiveSupport::TestCase.file_fixture_path</tt>. + # + # File fixtures are represented as +Pathname+ objects. + # This makes it easy to extract specific information: + # + # file_fixture("example.txt").read # get the file's content + # file_fixture("example.mp3").size # get the file size + module FileFixtures + extend ActiveSupport::Concern + + included do + class_attribute :file_fixture_path, instance_writer: false + end + + # Returns a +Pathname+ to the fixture file named +fixture_name+. + # + # Raises ArgumentError if +fixture_name+ can't be found. + def file_fixture(fixture_name) + path = Pathname.new(File.join(file_fixture_path, fixture_name)) + + if path.exist? + path + else + msg = "the directory '%s' does not contain a file named '%s'" + raise ArgumentError, msg % [file_fixture_path, fixture_name] + end + end + end + end +end diff --git a/activesupport/test/core_ext/file_test.rb b/activesupport/test/core_ext/file_test.rb index 2c04e9687c..cde0132b97 100644 --- a/activesupport/test/core_ext/file_test.rb +++ b/activesupport/test/core_ext/file_test.rb @@ -57,6 +57,16 @@ class AtomicWriteTest < ActiveSupport::TestCase File.unlink(file_name) rescue nil end + def test_atomic_write_returns_result_from_yielded_block + block_return_value = File.atomic_write(file_name, Dir.pwd) do |file| + "Hello world!" + end + + assert_equal "Hello world!", block_return_value + ensure + File.unlink(file_name) rescue nil + end + private def file_name "atomic.file" diff --git a/activesupport/test/file_fixtures/sample.txt b/activesupport/test/file_fixtures/sample.txt new file mode 100644 index 0000000000..0fa80e7383 --- /dev/null +++ b/activesupport/test/file_fixtures/sample.txt @@ -0,0 +1 @@ +sample file fixture diff --git a/activesupport/test/testing/file_fixtures_test.rb b/activesupport/test/testing/file_fixtures_test.rb new file mode 100644 index 0000000000..91b8a9071c --- /dev/null +++ b/activesupport/test/testing/file_fixtures_test.rb @@ -0,0 +1,28 @@ +require 'abstract_unit' + +class FileFixturesTest < ActiveSupport::TestCase + self.file_fixture_path = File.expand_path("../../file_fixtures", __FILE__) + + test "#file_fixture returns Pathname to file fixture" do + path = file_fixture("sample.txt") + assert_kind_of Pathname, path + assert_match %r{activesupport/test/file_fixtures/sample.txt$}, path.to_s + end + + test "raises an exception when the fixture file does not exist" do + e = assert_raises(ArgumentError) do + file_fixture("nope") + end + assert_match(/^the directory '[^']+test\/file_fixtures' does not contain a file named 'nope'$/, e.message) + end +end + +class FileFixturesPathnameDirectoryTest < ActiveSupport::TestCase + self.file_fixture_path = Pathname.new(File.expand_path("../../file_fixtures", __FILE__)) + + test "#file_fixture_path returns Pathname to file fixture" do + path = file_fixture("sample.txt") + assert_kind_of Pathname, path + assert_match %r{activesupport/test/file_fixtures/sample.txt$}, path.to_s + end +end diff --git a/guides/source/testing.md b/guides/source/testing.md index fa55c09c64..a083b3f981 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -480,21 +480,28 @@ In the `test_should_get_index` test, Rails simulates a request on the action cal The `get` method kicks off the web request and populates the results into the response. It accepts 4 arguments: -* The action of the controller you are requesting. This can be in the form of a string or a symbol. -* An optional hash of request parameters to pass into the action (eg. query string parameters or article variables). -* An optional hash of session variables to pass along with the request. -* An optional hash of flash values. +* The action of the controller you are requesting. + This can be in the form of a string or a symbol. + +* `params`: option with a hash of request parameters to pass into the action + (e.g. query string parameters or article variables). + +* `session`: option with a hash of session variables to pass along with the request. + +* `flash`: option with a hash of flash values. + +All the keyword arguments are optional. Example: Calling the `:show` action, passing an `id` of 12 as the `params` and setting a `user_id` of 5 in the session: ```ruby -get(:show, {'id' => "12"}, {'user_id' => 5}) +get(:show, params: { 'id' => "12" }, session: { 'user_id' => 5 }) ``` Another example: Calling the `:view` action, passing an `id` of 12 as the `params`, this time with no session, but with a flash message. ```ruby -get(:view, {'id' => '12'}, nil, {'message' => 'booya!'}) +get(:view, params: { 'id' => '12' }, flash: { 'message' => 'booya!' }) ``` NOTE: If you try running `test_should_create_article` test from `articles_controller_test.rb` it will fail on account of the newly added model level validation and rightly so. @@ -504,7 +511,7 @@ Let us modify `test_should_create_article` test in `articles_controller_test.rb` ```ruby test "should create article" do assert_difference('Article.count') do - post :create, article: {title: 'Some title'} + post :create, params: { article: { title: 'Some title' } } end assert_redirected_to article_path(assigns(:article)) @@ -534,7 +541,7 @@ NOTE: Functional tests do not verify whether the specified request type is accep ```ruby test "ajax request responds with no layout" do - xhr :get, :show, id: articles(:first).id + xhr :get, :show, params: { id: articles(:first).id } assert_template :index assert_template layout: nil @@ -638,7 +645,7 @@ Let's start by adding this assertion to our `test_should_create_article` test: ```ruby test "should create article" do assert_difference('Article.count') do - post :create, article: {title: 'Some title'} + post :create, params: { article: { title: 'Some title' } } end assert_redirected_to article_path(assigns(:article)) @@ -708,7 +715,7 @@ Let's write a test for the `:show` action: ```ruby test "should show article" do article = articles(:one) - get :show, id: article.id + get :show, params: { id: article.id } assert_response :success end ``` @@ -721,7 +728,7 @@ How about deleting an existing Article? test "should destroy article" do article = articles(:one) assert_difference('Article.count', -1) do - delete :destroy, id: article.id + delete :destroy, params: { id: article.id } end assert_redirected_to articles_path @@ -733,7 +740,7 @@ We can also add a test for updating an existing Article. ```ruby test "should update article" do article = articles(:one) - patch :update, id: article.id, article: {title: "updated"} + patch :update, params: { id: article.id, article: { title: "updated" } } assert_redirected_to article_path(assigns(:article)) end ``` @@ -759,20 +766,20 @@ class ArticlesControllerTest < ActionController::TestCase test "should show article" do # Reuse the @article instance variable from setup - get :show, id: @article.id + get :show, params: { id: @article.id } assert_response :success end test "should destroy article" do assert_difference('Article.count', -1) do - delete :destroy, id: @article.id + delete :destroy, params: { id: @article.id } end assert_redirected_to articles_path end test "should update article" do - patch :update, id: @article.id, article: {title: "updated"} + patch :update, params: { id: @article.id, article: { title: "updated" } } assert_redirected_to article_path(assigns(:article)) end end @@ -1026,7 +1033,8 @@ test "can create an article" do assert_response :success assert_template "articles/new", partial: "articles/_form" - post "/articles", article: {title: "can create", body: "article successfully."} + post "/articles", + params: { article: { title: "can create", body: "article successfully." } } assert_response :redirect follow_redirect! assert_response :success @@ -1042,7 +1050,8 @@ We start by calling the `:new` action on our Articles controller. This response After this we make a post request to the `:create` action of our Articles controller: ```ruby -post "/articles", article: {title: "can create", body: "article successfully."} +post "/articles", + params: { article: { title: "can create", body: "article successfully." } } assert_response :redirect follow_redirect! ``` @@ -1147,7 +1156,7 @@ require 'test_helper' class UserControllerTest < ActionController::TestCase test "invite friend" do assert_difference 'ActionMailer::Base.deliveries.size', +1 do - post :invite_friend, email: 'friend@example.com' + post :invite_friend, params: { email: 'friend@example.com' } end invite_email = ActionMailer::Base.deliveries.last diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 22b94f70ed..b6e6642f11 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -125,6 +125,7 @@ module Rails def test empty_directory_with_keep_file 'test/fixtures' + empty_directory_with_keep_file 'test/fixtures/files' empty_directory_with_keep_file 'test/controllers' empty_directory_with_keep_file 'test/mailers' empty_directory_with_keep_file 'test/models' diff --git a/railties/lib/rails/generators/rails/model/USAGE b/railties/lib/rails/generators/rails/model/USAGE index b8f7845896..11daa5c3cb 100644 --- a/railties/lib/rails/generators/rails/model/USAGE +++ b/railties/lib/rails/generators/rails/model/USAGE @@ -22,7 +22,7 @@ Description: If you pass a namespaced model name (e.g. admin/account or Admin::Account) then the generator will create a module with a table_name_prefix method - to prefix the model's table name with the module name (e.g. admin_account) + to prefix the model's table name with the module name (e.g. admin_accounts) Available field types: diff --git a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb index bf3da1fc4d..0852ffce9a 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/test/test_helper.rb @@ -20,4 +20,6 @@ Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].each { |f| require f } # Load fixtures from the engine if ActiveSupport::TestCase.respond_to?(:fixture_path=) ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__) + ActiveSupport::TestCase.file_fixture_path = ActiveSupport::TestCase.fixture_path + "files" + ActiveSupport::TestCase.fixtures :all end diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index c837fadb40..40a1915b54 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -21,6 +21,7 @@ if defined?(ActiveRecord::Base) class ActiveSupport::TestCase include ActiveRecord::TestFixtures self.fixture_path = "#{Rails.root}/test/fixtures/" + self.file_fixture_path = self.fixture_path + "files" end ActionDispatch::IntegrationTest.fixture_path = ActiveSupport::TestCase.fixture_path diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index e418bb282e..689173f184 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -33,6 +33,7 @@ DEFAULT_APP_FILES = %w( log test/test_helper.rb test/fixtures + test/fixtures/files test/controllers test/models test/helpers |