diff options
Diffstat (limited to 'actionpack')
79 files changed, 1645 insertions, 524 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a7a4aabc98..c5b679207a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,328 +1 @@ -* Remove deprecated `.to_prepare`, `.to_cleanup`, `.prepare!` and `.cleanup!` from `ActionDispatch::Reloader`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::Callbacks.to_prepare` and `ActionDispatch::Callbacks.to_cleanup`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionController::Metal.call`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionController::Metal#env`. - - *Rafael Mendonça França* - -* Make `with_routing` test helper work when testing controllers inheriting from `ActionController::API` - - *Julia López* - -* Use accept header in integration tests with `as: :json` - - Instead of appending the `format` to the request path, Rails will figure - out the format from the header instead. - - This allows devs to use `:as` on routes that don't have a format. - - Fixes #27144. - - *Kasper Timm Hansen* - -* Reset a new session directly after its creation in `ActionDispatch::IntegrationTest#open_session`. - - Fixes #22742. - - *Tawan Sierek* - -* Fixes incorrect output from rails routes when using singular resources. - - Fixes #26606. - - *Erick Reyna* - -* Fixes multiple calls to `logger.fatal` instead of a single call, - for every line in an exception backtrace, when printing trace - from `DebugExceptions` middleware. - - Fixes #26134. - - *Vipul A M* - -* Add support for arbitrary hashes in strong parameters: - - ```ruby - params.permit(preferences: {}) - ``` - - *Xavier Noria* - -* Add `ActionController::Parameters#merge!`, which behaves the same as `Hash#merge!`. - - *Yuji Yaginuma* - -* Allow keys not found in `RACK_KEY_TRANSLATION` for setting the environment when rendering - arbitrary templates. - - *Sammy Larbi* - -* Remove deprecated support to non-keyword arguments in `ActionDispatch::IntegrationTest#process`, - `#get`, `#post`, `#patch`, `#put`, `#delete`, and `#head`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::IntegrationTest#*_via_redirect`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::IntegrationTest#xml_http_request`. - - *Rafael Mendonça França* - -* Remove deprecated support for passing `:path` and route path as strings in `ActionDispatch::Routing::Mapper#match`. - - *Rafael Mendonça França* - -* Remove deprecated support for passing path as `nil` in `ActionDispatch::Routing::Mapper#match`. - - *Rafael Mendonça França* - -* Remove deprecated `cache_control` argument from `ActionDispatch::Static#initialize`. - - *Rafael Mendonça França* - -* Remove deprecated support to passing strings or symbols to the middleware stack. - - *Rafael Mendonça França* - -* Change HSTS subdomain to true. - - *Rafael Mendonça França* - -* Remove deprecated `host` and `port` ssl options. - - *Rafael Mendonça França* - -* Remove deprecated `const_error` argument in - `ActionDispatch::Session::SessionRestoreError#initialize`. - - *Rafael Mendonça França* - -* Remove deprecated `#original_exception` in `ActionDispatch::Session::SessionRestoreError`. - - *Rafael Mendonça França* - -* Deprecate `ActionDispatch::ParamsParser::ParseError` in favor of - `ActionDispatch::Http::Parameters::ParseError`. - - *Rafael Mendonça França* - -* Remove deprecated `ActionDispatch::ParamsParser`. - - *Rafael Mendonça França* - -* Remove deprecated `original_exception` and `message` arguments in - `ActionDispatch::ParamsParser::ParseError#initialize`. - - *Rafael Mendonça França* - -* Remove deprecated `#original_exception` in `ActionDispatch::ParamsParser::ParseError`. - - *Rafael Mendonça França* - -* Remove deprecated access to mime types through constants. - - *Rafael Mendonça França* - -* Remove deprecated support to non-keyword arguments in `ActionController::TestCase#process`, - `#get`, `#post`, `#patch`, `#put`, `#delete`, and `#head`. - - *Rafael Mendonça França* - -* Remove deprecated `xml_http_request` and `xhr` methods in `ActionController::TestCase`. - - *Rafael Mendonça França* - -* Remove deprecated methods in `ActionController::Parameters`. - - *Rafael Mendonça França* - -* Remove deprecated support to comparing a `ActionController::Parameters` - with a `Hash`. - - *Rafael Mendonça França* - -* Remove deprecated support to `:text` in `render`. - - *Rafael Mendonça França* - -* Remove deprecated support to `:nothing` in `render`. - - *Rafael Mendonça França* - -* Remove deprecated support to `:back` in `redirect_to`. - - *Rafael Mendonça França* - -* Remove deprecated support to passing status as option `head`. - - *Rafael Mendonça França* - -* Remove deprecated support to passing original exception to `ActionController::BadRequest` - and the `ActionController::BadRequest#original_exception` method. - - *Rafael Mendonça França* - -* Remove deprecated methods `skip_action_callback`, `skip_filter`, `before_filter`, - `prepend_before_filter`, `skip_before_filter`, `append_before_filter`, `around_filter` - `prepend_around_filter`, `skip_around_filter`, `append_around_filter`, `after_filter`, - `prepend_after_filter`, `skip_after_filter` and `append_after_filter`. - - *Rafael Mendonça França* - -* Show an "unmatched constraints" error when params fail to match constraints - on a matched route, rather than a "missing keys" error. - - Fixes #26470. - - *Chris Carter* - -* Fix adding implicitly rendered template digests to ETags. - - Fixes a case when modifying an implicitly rendered template for a - controller action using `fresh_when` or `stale?` would not result in a new - `ETag` value. - - *Javan Makhmali* - -* Make `fixture_file_upload` work in integration tests. - - *Yuji Yaginuma* - -* Add `to_param` to `ActionController::Parameters` deprecations. - - In the future `ActionController::Parameters` are discouraged from being used - in URLs without explicit whitelisting. Go through `to_h` to use `to_param`. - - *Kir Shatrov* - -* Fix nested multiple roots - - The PR #20940 enabled the use of multiple roots with different constraints - at the top level but unfortunately didn't work when those roots were inside - a namespace and also broke the use of root inside a namespace after a top - level root was defined because the check for the existence of the named route - used the global :root name and not the namespaced name. - - This is fixed by using the name_for_action method to expand the :root name to - the full namespaced name. We can pass nil for the second argument as we're not - dealing with resource definitions so don't need to handle the cases for edit - and new routes. - - Fixes #26148. - - *Ryo Hashimoto*, *Andrew White* - -* Include the content of the flash in the auto-generated etag. This solves the following problem: - - 1. POST /messages - 2. redirect_to messages_url, notice: 'Message was created' - 3. GET /messages/1 - 4. GET /messages - - Step 4 would before still include the flash message, even though it's no longer relevant, - because the etag cache was recorded with the flash in place and didn't change when it was gone. - - *DHH* - -* SSL: Changes redirect behavior for all non-GET and non-HEAD requests - (like POST/PUT/PATCH etc) to `http://` resources to redirect to `https://` - with a [307 status code](http://tools.ietf.org/html/rfc7231#section-6.4.7) instead of [301 status code](http://tools.ietf.org/html/rfc7231#section-6.4.2). - - 307 status code instructs the HTTP clients to preserve the original - request method while redirecting. It has been part of HTTP RFC since - 1999 and is implemented/recognized by most (if not all) user agents. - - # Before - POST http://example.com/articles (i.e. ArticlesContoller#create) - redirects to - GET https://example.com/articles (i.e. ArticlesContoller#index) - - # After - POST http://example.com/articles (i.e. ArticlesContoller#create) - redirects to - POST https://example.com/articles (i.e. ArticlesContoller#create) - - *Chirag Singhal* - -* Add `:as` option to `ActionController:TestCase#process` and related methods. - - Specifying `as: mime_type` allows the `CONTENT_TYPE` header to be specified - in controller tests without manually doing this through `@request.headers['CONTENT_TYPE']`. - - *Everest Stefan Munro-Zeisberger* - -* Show cache hits and misses when rendering partials. - - Partials using the `cache` helper will show whether a render hit or missed - the cache: - - ``` - Rendered messages/_message.html.erb in 1.2 ms [cache hit] - Rendered recordings/threads/_thread.html.erb in 1.5 ms [cache miss] - ``` - - This removes the need for the old fragment cache logging: - - ``` - Read fragment views/v1/2914079/v1/2914079/recordings/70182313-20160225015037000000/d0bdf2974e1ef6d31685c3b392ad0b74 (0.6ms) - Rendered messages/_message.html.erb in 1.2 ms [cache hit] - Write fragment views/v1/2914079/v1/2914079/recordings/70182313-20160225015037000000/3b4e249ac9d168c617e32e84b99218b5 (1.1ms) - Rendered recordings/threads/_thread.html.erb in 1.5 ms [cache miss] - ``` - - Though that full output can be reenabled with - `config.action_controller.enable_fragment_cache_logging = true`. - - *Stan Lo* - -* Don't override the `Accept` header in integration tests when called with `xhr: true`. - - Fixes #25859. - - *David Chen* - -* Fix `defaults` option for root route. - - A regression from some refactoring for the 5.0 release, this change - fixes the use of `defaults` (default parameters) in the `root` routing method. - - *Chris Arcand* - -* Check `request.path_parameters` encoding at the point they're set. - - Check for any non-UTF8 characters in path parameters at the point they're - set in `env`. Previously they were checked for when used to get a controller - class, but this meant routes that went directly to a Rack app, or skipped - controller instantiation for some other reason, had to defend against - non-UTF8 characters themselves. - - *Grey Baker* - -* Don't raise `ActionController::UnknownHttpMethod` from `ActionDispatch::Static`. - - Pass `Rack::Request` objects to `ActionDispatch::FileHandler` to avoid it - raising `ActionController::UnknownHttpMethod`. If an unknown method is - passed, it should pass exception higher in the stack instead, once we've had a - chance to define exception handling behaviour. - - *Grey Baker* - -* Handle `Rack::QueryParser` errors in `ActionDispatch::ExceptionWrapper`. - - Updated `ActionDispatch::ExceptionWrapper` to handle the Rack 2.0 namespace - for `ParameterTypeError` and `InvalidParameterError` errors. - - *Grey Baker* - -Please check [5-0-stable](https://github.com/rails/rails/blob/5-0-stable/actionpack/CHANGELOG.md) for previous changes. +Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index 5cd8d77ddb..0d1af0d0bd 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -81,10 +81,9 @@ module ActionController # end # end # - # Quite straightforward. Make sure to check the modules included in - # <tt>ActionController::Base</tt> if you want to use any other - # functionality that is not provided by <tt>ActionController::API</tt> - # out of the box. + # Make sure to check the modules included in <tt>ActionController::Base</tt> + # if you want to use any other functionality that is not provided + # by <tt>ActionController::API</tt> out of the box. class API < Metal abstract! diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index ca8066cd82..b420e00c78 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -8,7 +8,7 @@ module ActionController # on the controller, which will automatically be made accessible to the web-server through \Rails Routes. # # By default, only the ApplicationController in a \Rails application inherits from <tt>ActionController::Base</tt>. All other - # controllers in turn inherit from ApplicationController. This gives you one class to configure things such as + # controllers inherit from ApplicationController. This gives you one class to configure things such as # request forgery protection and filtering of sensitive request parameters. # # A sample controller could look like this: @@ -30,7 +30,7 @@ module ActionController # # Unlike index, the create action will not render a template. After performing its main purpose (creating a # new post), it initiates a redirect instead. This redirect works by returning an external - # "302 Moved" HTTP response that takes the user to the index action. + # <tt>302 Moved</tt> HTTP response that takes the user to the index action. # # These two methods represent the two basic action archetypes used in Action Controllers: Get-and-show and do-and-redirect. # Most actions are variations on these themes. @@ -59,7 +59,7 @@ module ActionController # <input type="text" name="post[name]" value="david"> # <input type="text" name="post[address]" value="hyacintvej"> # - # A request stemming from a form holding these inputs will include <tt>{ "post" => { "name" => "david", "address" => "hyacintvej" } }</tt>. + # A request coming from a form holding these inputs will include <tt>{ "post" => { "name" => "david", "address" => "hyacintvej" } }</tt>. # If the address input had been named <tt>post[address][street]</tt>, the <tt>params</tt> would have included # <tt>{ "post" => { "address" => { "street" => "hyacintvej" } } }</tt>. There's no limit to the depth of the nesting. # @@ -74,7 +74,7 @@ module ActionController # # session[:person] = Person.authenticate(user_name, password) # - # And retrieved again through the same hash: + # You can retrieve it again through the same hash: # # Hello #{session[:person]} # @@ -261,6 +261,12 @@ module ActionController PROTECTED_IVARS end + def self.make_response!(request) + ActionDispatch::Response.create.tap do |res| + res.request = request + end + end + ActiveSupport.run_load_hooks(:action_controller, self) end end diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index a9a8508abc..954265ad97 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -38,7 +38,7 @@ module ActionController end def instrument_name - "action_controller" + "action_controller".freeze end end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 337718afc0..246644dcbd 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -129,7 +129,7 @@ module ActionController end def self.make_response!(request) - ActionDispatch::Response.create.tap do |res| + ActionDispatch::Response.new.tap do |res| res.request = request end end @@ -138,7 +138,7 @@ module ActionController false end - # Delegates to the class' <tt>controller_name</tt> + # Delegates to the class' <tt>controller_name</tt>. def controller_name self.class.controller_name end @@ -244,7 +244,7 @@ module ActionController end end - # Direct dispatch to the controller. Instantiates the controller, then + # Direct dispatch to the controller. Instantiates the controller, then # executes the action named +name+. def self.dispatch(name, req, res) if middleware_stack.any? diff --git a/actionpack/lib/action_controller/metal/etag_with_flash.rb b/actionpack/lib/action_controller/metal/etag_with_flash.rb index 474d75f02e..7bd338bd7c 100644 --- a/actionpack/lib/action_controller/metal/etag_with_flash.rb +++ b/actionpack/lib/action_controller/metal/etag_with_flash.rb @@ -1,9 +1,9 @@ module ActionController # When you're using the flash, it's generally used as a conditional on the view. # This means the content of the view depends on the flash. Which in turn means - # that the etag for a response should be computed with the content of the flash + # that the ETag for a response should be computed with the content of the flash # in mind. This does that by including the content of the flash as a component - # in the etag that's generated for a response. + # in the ETag that's generated for a response. module EtagWithFlash extend ActiveSupport::Concern diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 9d43e752ac..73e67573ca 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -2,17 +2,17 @@ require "active_support/core_ext/hash/except" require "active_support/core_ext/hash/slice" module ActionController - # This module provides a method which will redirect the browser to use HTTPS - # protocol. This will ensure that user's sensitive information will be + # This module provides a method which will redirect the browser to use the secured HTTPS + # protocol. This will ensure that users' sensitive information will be # transferred safely over the internet. You _should_ always force the browser # to use HTTPS when you're transferring sensitive information such as # user authentication, account information, or credit card information. # # Note that if you are really concerned about your application security, # you might consider using +config.force_ssl+ in your config file instead. - # That will ensure all the data transferred via HTTPS protocol and prevent - # the user from getting their session hijacked when accessing the site over - # unsecured HTTP protocol. + # That will ensure all the data is transferred via HTTPS, and will + # prevent the user from getting their session hijacked when accessing the + # site over unsecured HTTP protocol. module ForceSSL extend ActiveSupport::Concern include AbstractController::Callbacks @@ -23,7 +23,7 @@ module ActionController module ClassMethods # Force the request to this particular controller or specified actions to be - # under HTTPS protocol. + # through the HTTPS protocol. # # If you need to disable this for any reason (e.g. development) then you can use # an +:if+ or +:unless+ condition. @@ -71,7 +71,7 @@ module ActionController # Redirect the existing request to use the HTTPS protocol. # # ==== Parameters - # * <tt>host_or_options</tt> - Either a host name or any of the url & + # * <tt>host_or_options</tt> - Either a host name or any of the url and # redirect options available to the <tt>force_ssl</tt> method. def force_ssl_redirect(host_or_options = nil) unless request.ssl? diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 0575360068..d8bc895265 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -445,7 +445,7 @@ module ActionController end end - # Parses the token and options out of the token authorization header. + # Parses the token and options out of the token Authorization header. # The value for the Authorization header is expected to have the prefix # <tt>"Token"</tt> or <tt>"Bearer"</tt>. If the header looks like this: # Authorization: Token token="abc", nonce="def" diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index dde924e682..eeb27f99f4 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -2,11 +2,11 @@ module ActionController # Handles implicit rendering for a controller action that does not # explicitly respond with +render+, +respond_to+, +redirect+, or +head+. # - # For API controllers, the implicit response is always 204 No Content. + # For API controllers, the implicit response is always <tt>204 No Content</tt>. # # For all other controllers, we use these heuristics to decide whether to # render a template, raise an error for a missing template, or respond with - # 204 No Content: + # <tt>204 No Content</tt>: # # First, if we DO find a template, it's rendered. Template lookup accounts # for the action name, locales, format, variant, template handlers, and more @@ -23,7 +23,7 @@ module ActionController # <tt>ActionView::UnknownFormat</tt> with an explanation. # # Finally, if we DON'T find a template AND the request isn't a browser page - # load, then we implicitly respond with 204 No Content. + # load, then we implicitly respond with <tt>204 No Content</tt>. module ImplicitRender # :stopdoc: include BasicImplicitRender diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 924686218f..2485d27cec 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -3,7 +3,7 @@ require "abstract_controller/logger" module ActionController # Adds instrumentation to several ends in ActionController::Base. It also provides - # some hooks related with process_action, this allows an ORM like Active Record + # some hooks related with process_action. This allows an ORM like Active Record # and/or DataMapper to plug in ActionController and show related information. # # Check ActiveRecord::Railties::ControllerRuntime for an example. diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index fed99e6c82..a607ee2309 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -239,8 +239,8 @@ module ActionController error = nil # This processes the action in a child thread. It lets us return the - # response code and headers back up the rack stack, and still process - # the body in parallel with sending data to the client + # response code and headers back up the Rack stack, and still process + # the body in parallel with sending data to the client. new_controller_thread { ActiveSupport::Dependencies.interlock.running do t2 = Thread.current @@ -278,9 +278,9 @@ module ActionController raise error if error end - # Spawn a new thread to serve up the controller in. This is to get + # Spawn a new thread to serve up the controller in. This is to get # around the fact that Rack isn't based around IOs and we need to use - # a thread to stream data from the response bodies. Nobody should call + # a thread to stream data from the response bodies. Nobody should call # this method except in Rails internals. Seriously! def new_controller_thread # :nodoc: Thread.new { diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index f6aabcb102..96bd548268 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -181,8 +181,8 @@ module ActionController #:nodoc: # # request.variant = [:tablet, :phone] # - # which will work similarly to formats and MIME types negotiation. If there will be no - # +:tablet+ variant declared, +:phone+ variant will be picked: + # This will work similarly to formats and MIME types negotiation. If there + # is no +:tablet+ variant declared, the +:phone+ variant will be used: # # respond_to do |format| # format.html.none diff --git a/actionpack/lib/action_controller/metal/parameter_encoding.rb b/actionpack/lib/action_controller/metal/parameter_encoding.rb index 962532ff09..ecc691619e 100644 --- a/actionpack/lib/action_controller/metal/parameter_encoding.rb +++ b/actionpack/lib/action_controller/metal/parameter_encoding.rb @@ -39,7 +39,7 @@ module ActionController # end # # The show action in the above controller would have all parameter values - # encoded as ASCII-8BIT. This is useful in the case where an application + # encoded as ASCII-8BIT. This is useful in the case where an application # must handle data but encoding of the data is unknown, like file system data. def skip_parameter_encoding(action) @_parameter_encodings[action.to_s] = true diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 7fc898f034..a89fc1678b 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -105,7 +105,11 @@ module ActionController unless super || exclude if m.respond_to?(:attribute_names) && m.attribute_names.any? - self.include = m.attribute_names + if m.respond_to?(:stored_attributes) && !m.stored_attributes.empty? + self.include = m.attribute_names + m.stored_attributes.values.flatten.map(&:to_s) + else + self.include = m.attribute_names + end end end end @@ -213,7 +217,7 @@ module ActionController end # Sets the default wrapper key or model which will be used to determine - # wrapper key and attribute names. Will be called automatically when the + # wrapper key and attribute names. Called automatically when the # module is inherited. def inherited(klass) if klass._wrapper_options.format.any? @@ -225,7 +229,7 @@ module ActionController end end - # Performs parameters wrapping upon the request. Will be called automatically + # Performs parameters wrapping upon the request. Called automatically # by the metal call stack. def process_action(*args) if _wrapper_enabled? @@ -238,11 +242,11 @@ module ActionController wrapped_keys = request.request_parameters.keys wrapped_filtered_hash = _wrap_parameters request.filtered_parameters.slice(*wrapped_keys) - # This will make the wrapped hash accessible from controller and view + # This will make the wrapped hash accessible from controller and view. request.parameters.merge! wrapped_hash request.request_parameters.merge! wrapped_hash - # This will display the wrapped hash in the log file + # This will display the wrapped hash in the log file. request.filtered_parameters.merge! wrapped_filtered_hash end super diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 4dfcf4da28..fdfe82f96b 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -22,7 +22,7 @@ module ActionController # redirect_to posts_url # redirect_to proc { edit_post_url(@post) } # - # The redirection happens as a "302 Found" header unless otherwise specified using the <tt>:status</tt> option: + # The redirection happens as a <tt>302 Found</tt> header unless otherwise specified using the <tt>:status</tt> option: # # redirect_to post_url(@post), status: :found # redirect_to action: 'atom', status: :moved_permanently @@ -36,7 +36,7 @@ module ActionController # If you are using XHR requests other than GET or POST and redirecting after the # request then some browsers will follow the redirect using the original request # method. This may lead to undesirable behavior such as a double DELETE. To work - # around this you can return a <tt>303 See Other</tt> status code which will be + # around this you can return a <tt>303 See Other</tt> status code which will be # followed using a GET request. # # redirect_to posts_url, status: :see_other @@ -50,13 +50,16 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # + # Statements after +redirect_to+ in our controller get executed, so +redirect_to+ doesn't stop the execution of the function. + # To terminate the execution of the function immediately after the +redirect_to+, use return. + # redirect_to post_url(@post) and return def redirect_to(options = {}, response_status = {}) raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body self.status = _extract_redirect_to_status(options, response_status) self.location = _compute_redirect_to_location(request, options) - self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" + self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>" end # Redirects the browser to the page that issued the request (the referrer) diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 6b17719381..67f207afc2 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -36,7 +36,7 @@ module ActionController super end - # Overwrite render_to_string because body can now be set to a rack body. + # Overwrite render_to_string because body can now be set to a Rack body. def render_to_string(*) result = super if result.respond_to?(:each) diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index e8965a6561..d9a8b9c12d 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -262,9 +262,9 @@ module ActionController #:nodoc: # Returns true or false if a request is verified. Checks: # - # * Is it a GET or HEAD request? Gets should be safe and idempotent + # * Is it a GET or HEAD request? GETs should be safe and idempotent # * Does the form_authenticity_token match the given token value from the params? - # * Does the X-CSRF-Token header match the form_authenticity_token + # * Does the X-CSRF-Token header match the form_authenticity_token? def verified_request? # :doc: !protect_against_forgery? || request.get? || request.head? || (valid_request_origin? && any_authenticity_token_valid?) @@ -327,7 +327,7 @@ module ActionController #:nodoc: if masked_token.length == AUTHENTICITY_TOKEN_LENGTH # This is actually an unmasked token. This is expected if # you have just upgraded to masked tokens, but should stop - # happening shortly after installing this gem + # happening shortly after installing this gem. compare_with_real_token masked_token, session elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2 @@ -336,13 +336,13 @@ module ActionController #:nodoc: compare_with_real_token(csrf_token, session) || valid_per_form_csrf_token?(csrf_token, session) else - false # Token is malformed + false # Token is malformed. end end def unmask_token(masked_token) # :doc: # Split the token into the one-time pad and the encrypted - # value and decrypt it + # value and decrypt it. one_time_pad = masked_token[0...AUTHENTICITY_TOKEN_LENGTH] encrypted_csrf_token = masked_token[AUTHENTICITY_TOKEN_LENGTH..-1] xor_byte_strings(one_time_pad, encrypted_csrf_token) diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 2d99e4045b..25757938f5 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -10,7 +10,7 @@ module ActionController #:nodoc: # exceptions must be shown. This method is only called when # consider_all_requests_local is false. By default, it returns # false, but someone may set it to `request.local?` so local - # requests in production still shows the detailed exception pages. + # requests in production still show the detailed exception pages. def show_detailed_exceptions? false end diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 877a08b222..58cf60ad2a 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -3,7 +3,7 @@ require "rack/chunked" module ActionController #:nodoc: # Allows views to be streamed back to the client as they are rendered. # - # The default way Rails renders views is by first rendering the template + # By default, Rails renders views by first rendering the template # and then the layout. The response is sent to the client after the whole # template is rendered, all queries are made, and the layout is processed. # diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index d304dcf468..baf15570d5 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -112,6 +112,77 @@ module ActionController cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false + ## + # :method: as_json + # + # :call-seq: + # as_json(options=nil) + # + # Returns a hash that can be used as the JSON representation for the parameters. + + ## + # :method: empty? + # + # :call-seq: + # empty?() + # + # Returns true if the parameters have no key/value pairs. + + ## + # :method: has_key? + # + # :call-seq: + # has_key?(key) + # + # Returns true if the given key is present in the parameters. + + ## + # :method: has_value? + # + # :call-seq: + # has_value?(value) + # + # Returns true if the given value is present for some key in the parameters. + + ## + # :method: include? + # + # :call-seq: + # include?(key) + # + # Returns true if the given key is present in the parameters. + + ## + # :method: key? + # + # :call-seq: + # key?(key) + # + # Returns true if the given key is present in the parameters. + + ## + # :method: keys + # + # :call-seq: + # keys() + # + # Returns a new array of the keys of the parameters. + + ## + # :method: value? + # + # :call-seq: + # value?(value) + # + # Returns true if the given value is present for some key in the parameters. + + ## + # :method: values + # + # :call-seq: + # values() + # + # Returns a new array of the values of the parameters. delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :as_json, to: :@parameters @@ -191,7 +262,7 @@ module ActionController alias_method :to_unsafe_hash, :to_unsafe_h # Convert all hashes in values into parameters, then yield each pair in - # the same way as <tt>Hash#each_pair</tt> + # the same way as <tt>Hash#each_pair</tt>. def each_pair(&block) @parameters.each_pair do |key, value| yield key, convert_hashes_to_parameters(key, value) @@ -339,7 +410,7 @@ module ActionController # # params.permit(preferences: {}) # - # but be careful because this opens the door to arbitrary input. In this + # Be careful because this opens the door to arbitrary input. In this # case, +permit+ ensures values in the returned structure are permitted # scalars and filters out anything else. # @@ -575,20 +646,37 @@ module ActionController end # Returns a new <tt>ActionController::Parameters</tt> with all keys from - # +other_hash+ merges into current hash. + # +other_hash+ merged into current hash. def merge(other_hash) new_instance_with_inherited_permitted_status( @parameters.merge(other_hash.to_h) ) end - # Returns current <tt>ActionController::Parameters</tt> instance which - # +other_hash+ merges into current hash. + # Returns current <tt>ActionController::Parameters</tt> instance with + # +other_hash+ merged into current hash. def merge!(other_hash) @parameters.merge!(other_hash.to_h) self end + # Returns a new <tt>ActionController::Parameters</tt> with all keys from + # current hash merged into +other_hash+. + def reverse_merge(other_hash) + new_instance_with_inherited_permitted_status( + other_hash.to_h.merge(@parameters) + ) + end + alias_method :with_defaults, :reverse_merge + + # Returns current <tt>ActionController::Parameters</tt> instance with + # current hash merged into +other_hash+. + def reverse_merge!(other_hash) + @parameters.merge!(other_hash.to_h) { |key, left, right| left } + self + end + alias_method :with_defaults!, :reverse_merge! + # This is required by ActiveModel attribute assignment, so that user can # pass +Parameters+ to a mass assignment methods in a model. It should not # matter as we are using +HashWithIndifferentAccess+ internally. @@ -629,7 +717,7 @@ module ActionController undef_method :to_param - # Returns duplicate of object including all parameters + # Returns duplicate of object including all parameters. def deep_dup self.class.new(@parameters.deep_dup).tap do |duplicate| duplicate.permitted = @permitted @@ -849,7 +937,7 @@ module ActionController # whitelisted. # # In addition, parameters can be marked as required and flow through a - # predefined raise/rescue flow to end up as a 400 Bad Request with no + # predefined raise/rescue flow to end up as a <tt>400 Bad Request</tt> with no # effort. # # class PeopleController < ActionController::Base @@ -862,7 +950,7 @@ module ActionController # end # # # This will pass with flying colors as long as there's a person key in the - # # parameters, otherwise it'll raise an ActionController::MissingParameter + # # parameters, otherwise it'll raise an ActionController::ParameterMissing # # exception, which will get caught by ActionController::Base and turned # # into a 400 Bad Request reply. # def update @@ -873,7 +961,7 @@ module ActionController # # private # # Using a private method to encapsulate the permissible parameters is - # # just a good pattern since you'll be able to reuse the same permit + # # a good pattern since you'll be able to reuse the same permit # # list between create and update. Also, you can specialize this method # # with per-user checking of permissible attributes. # def person_params diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index 9f3cc099d6..21ed5b4ec8 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -3,7 +3,7 @@ module ActionController # the <tt>_routes</tt> method. Otherwise, an exception will be raised. # # In addition to <tt>AbstractController::UrlFor</tt>, this module accesses the HTTP layer to define - # url options like the +host+. In order to do so, this module requires the host class + # URL options like the +host+. In order to do so, this module requires the host class # to implement +env+ which needs to be Rack-compatible and +request+ # which is either an instance of +ActionDispatch::Request+ or an object # that responds to the +host+, +optional_port+, +protocol+ and diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index a7cdfe6a98..fadfc8de60 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -42,7 +42,7 @@ module ActionController options.javascripts_dir ||= paths["public/javascripts"].first options.stylesheets_dir ||= paths["public/stylesheets"].first - # Ensure readers methods get compiled + # Ensure readers methods get compiled. options.asset_host ||= app.config.asset_host options.relative_url_root ||= app.config.relative_url_root diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index acb400cd15..cbb719d8b2 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -5,7 +5,7 @@ module ActionController # without requirement of being in controller actions. # # You get a concrete renderer class by invoking ActionController::Base#renderer. - # For example, + # For example: # # ApplicationController.renderer # @@ -18,7 +18,7 @@ module ActionController # ApplicationController.render template: '...' # # #render allows you to use the same options that you can use when rendering in a controller. - # For example, + # For example: # # FooController.render :action, locals: { ... }, assigns: { ... } # @@ -56,7 +56,7 @@ module ActionController # Create a new renderer for the same controller but with new defaults. def with_defaults(defaults) - self.class.new controller, env, self.defaults.merge(defaults) + self.class.new controller, @env, self.defaults.merge(defaults) end # Accepts a custom Rack environment to render templates in. @@ -85,6 +85,7 @@ module ActionController def normalize_keys(env) new_env = {} env.each_pair { |k, v| new_env[rack_key_for(k)] = rack_value_for(k, v) } + new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http" new_env end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 7229c67f30..bc42d50205 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -13,10 +13,10 @@ module ActionController end module Live - # Disable controller / rendering threads in tests. User tests can access + # Disable controller / rendering threads in tests. User tests can access # the database on the main thread, so they could open a txn, then the # controller thread will open a new connection and try to access data - # that's only visible to the main thread's txn. This is the problem in #23483 + # that's only visible to the main thread's txn. This is the problem in #23483. remove_method :new_controller_thread def new_controller_thread # :nodoc: yield @@ -35,7 +35,7 @@ module ActionController attr_reader :controller_class - # Create a new test request with default `env` values + # Create a new test request with default `env` values. def self.create(controller_class) env = {} env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application @@ -131,7 +131,7 @@ module ActionController include Rack::Test::Utils def should_multipart?(params) - # FIXME: lifted from Rack-Test. We should push this separation upstream + # FIXME: lifted from Rack-Test. We should push this separation upstream. multipart = false query = lambda { |value| case value @@ -300,7 +300,7 @@ module ActionController # assert_equal "Dave", cookies[:name] # makes sure that a cookie called :name was set as "Dave" # assert flash.empty? # makes sure that there's nothing in the flash # - # On top of the collections, you have the complete url that a given action redirected to available in <tt>redirect_to_url</tt>. + # On top of the collections, you have the complete URL that a given action redirected to available in <tt>redirect_to_url</tt>. # # For redirects within the same controller, you can even call follow_redirect and the redirect will be followed, triggering another # action call which can then be asserted against. @@ -534,7 +534,6 @@ module ActionController @request.delete_header "HTTP_ACCEPT" end @request.query_string = "" - @request.env.delete "PATH_INFO" @response.sent! end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 028177ace2..303790e96d 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -97,6 +97,8 @@ module ActionDispatch autoload :TestResponse autoload :AssertionResponse end + + autoload :SystemTestCase, "action_dispatch/system_test_case" end autoload :Mime, "action_dispatch/http/mime_type" diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index c4fe3a5c09..19f89edbc1 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -135,9 +135,7 @@ module ActionDispatch } end - # Receives an array of mimes and return the first user sent mime that - # matches the order array. - # + # Returns the first MIME type that matches the provided array of MIME types. def negotiate_mime(order) formats.each do |priority| if priority == Mime::ALL diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 1583a8f87f..0cf010f59e 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -46,7 +46,7 @@ module Mime end end - # Encapsulates the notion of a mime type. Can be used at render time, for example, with: + # Encapsulates the notion of a MIME type. Can be used at render time, for example, with: # # class PostsController < ActionController::Base # def show @@ -64,7 +64,7 @@ module Mime @register_callbacks = [] - # A simple helper class used in parsing the accept header + # A simple helper class used in parsing the accept header. class AcceptItem #:nodoc: attr_accessor :index, :name, :q alias :to_s :name @@ -72,7 +72,7 @@ module Mime def initialize(index, name, q = nil) @index = index @name = name - q ||= 0.0 if @name == "*/*".freeze # default wildcard match to end of list + q ||= 0.0 if @name == "*/*".freeze # Default wildcard match to end of list. @q = ((q || 1.0).to_f * 100).to_i end @@ -90,22 +90,22 @@ module Mime text_xml_idx = find_item_by_name list, "text/xml" app_xml_idx = find_item_by_name list, Mime[:xml].to_s - # Take care of the broken text/xml entry by renaming or deleting it + # Take care of the broken text/xml entry by renaming or deleting it. if text_xml_idx && app_xml_idx app_xml = list[app_xml_idx] text_xml = list[text_xml_idx] - app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two - if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list + app_xml.q = [text_xml.q, app_xml.q].max # Set the q value to the max of the two. + if app_xml_idx > text_xml_idx # Make sure app_xml is ahead of text_xml in the list. list[app_xml_idx], list[text_xml_idx] = text_xml, app_xml app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx end - list.delete_at(text_xml_idx) # delete text_xml from the list + list.delete_at(text_xml_idx) # Delete text_xml from the list. elsif text_xml_idx list[text_xml_idx].name = Mime[:xml].to_s end - # Look for more specific XML-based types and sort them ahead of app/xml + # Look for more specific XML-based types and sort them ahead of app/xml. if app_xml_idx app_xml = list[app_xml_idx] idx = app_xml_idx @@ -147,7 +147,7 @@ module Mime EXTENSION_LOOKUP[extension.to_s] end - # Registers an alias that's not used on mime type lookup, but can be referenced directly. Especially useful for + # Registers an alias that's not used on MIME type lookup, but can be referenced directly. Especially useful for # rendering different HTML versions depending on the user agent, like an iPhone. def register_alias(string, symbol, extension_synonyms = []) register(string, symbol, [], extension_synonyms, true) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 8f21eca440..79a2ef965b 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -13,7 +13,7 @@ module ActionDispatch } # Raised when raw data from the request cannot be parsed by the parser - # defined for request's content mime type. + # defined for request's content MIME type. class ParseError < StandardError def initialize super($!.message) @@ -30,9 +30,9 @@ module ActionDispatch end module ClassMethods - # Configure the parameter parser for a given mime type. + # Configure the parameter parser for a given MIME type. # - # It accepts a hash where the key is the symbol of the mime type + # It accepts a hash where the key is the symbol of the MIME type # and the value is a proc. # # original_parsers = ActionDispatch::Request.parameter_parsers @@ -100,7 +100,7 @@ module ActionDispatch begin strategy.call(raw_post) - rescue # JSON or Ruby code block errors + rescue # JSON or Ruby code block errors. my_logger = logger || ActiveSupport::Logger.new($stderr) my_logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{raw_post}" @@ -115,6 +115,7 @@ module ActionDispatch end module ParamsParser - ParseError = ActiveSupport::Deprecation::DeprecatedConstantProxy.new("ActionDispatch::ParamsParser::ParseError", "ActionDispatch::Http::Parameters::ParseError") + include ActiveSupport::Deprecation::DeprecatedConstantAccessor + deprecate_constant "ParseError", "ActionDispatch::Http::Parameters::ParseError" end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 19fa42ce12..6d42404a98 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -114,7 +114,7 @@ module ActionDispatch HTTP_METHOD_LOOKUP = {} - # Populate the HTTP method lookup cache + # Populate the HTTP method lookup cache. HTTP_METHODS.each { |method| HTTP_METHOD_LOOKUP[method] = method.underscore.to_sym } @@ -165,12 +165,12 @@ module ActionDispatch def show_exceptions? # :nodoc: # We're treating `nil` as "unset", and we want the default setting to be - # `true`. This logic should be extracted to `env_config` and calculated + # `true`. This logic should be extracted to `env_config` and calculated # once. !(get_header("action_dispatch.show_exceptions".freeze) == false) end - # Returns a symbol form of the #request_method + # Returns a symbol form of the #request_method. def request_method_symbol HTTP_METHOD_LOOKUP[request_method] end @@ -182,7 +182,7 @@ module ActionDispatch @method ||= check_method(get_header("rack.methodoverride.original_method") || get_header("REQUEST_METHOD")) end - # Returns a symbol form of the #method + # Returns a symbol form of the #method. def method_symbol HTTP_METHOD_LOOKUP[method] end @@ -267,7 +267,7 @@ module ActionDispatch # (which sets the action_dispatch.request_id environment variable). # # This unique ID is useful for tracing a request from end-to-end as part of logging or debugging. - # This relies on the rack variable set by the ActionDispatch::RequestId middleware. + # This relies on the Rack variable set by the ActionDispatch::RequestId middleware. def request_id get_header ACTION_DISPATCH_REQUEST_ID end @@ -339,7 +339,7 @@ module ActionDispatch Session::Options.set self, options end - # Override Rack's GET method to support indifferent access + # Override Rack's GET method to support indifferent access. def GET fetch_header("action_dispatch.request.query_parameters") do |k| rack_query_params = super || {} @@ -352,7 +352,7 @@ module ActionDispatch end alias :query_parameters :GET - # Override Rack's POST method to support indifferent access + # Override Rack's POST method to support indifferent access. def POST fetch_header("action_dispatch.request.request_parameters") do pr = parse_formatted_parameters(params_parsers) do |params| diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index dc159596c4..2ee52eeb48 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -85,7 +85,7 @@ module ActionDispatch # :nodoc: cattr_accessor(:default_headers) include Rack::Response::Helpers - # Aliasing these off because AD::Http::Cache::Response defines them + # Aliasing these off because AD::Http::Cache::Response defines them. alias :_cache_control :cache_control alias :_cache_control= :cache_control= @@ -142,7 +142,7 @@ module ActionDispatch # :nodoc: private def each_chunk(&block) - @buf.each(&block) # extract into own method + @buf.each(&block) end end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index a6937d54ff..b6be48a48b 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -101,10 +101,8 @@ module ActionDispatch end def add_trailing_slash(path) - # includes querysting if path.include?("?") path.sub!(/\?/, '/\&') - # does not have a .format elsif !path.include?(".") path.sub!(/[^\/]\z|\A\z/, '\&/') end diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index f3b8e82d32..326f4e52f9 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -15,7 +15,7 @@ module ActionDispatch def generate(name, options, path_parameters, parameterize = nil) constraints = path_parameters.merge(options) - missing_keys = nil # need for variable scope + missing_keys = nil match_route(name, constraints) do |route| parameterized_parts = extract_parameterized_parts(route, options, path_parameters, parameterize) diff --git a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb index beb9f1ef3b..e1ac2c873e 100644 --- a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb +++ b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb @@ -109,7 +109,6 @@ module ActionDispatch svg = to_svg javascripts = [states, fsm_js] - # Annoying hack warnings fun_routes = fun_routes stylesheets = stylesheets svg = svg diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 927fd369c4..7bc15aa6b3 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -89,8 +89,15 @@ module ActionDispatch end end + # Needed for `rails routes`. Picks up succinctly defined requirements + # for a route, for example route + # + # get 'photo/:id', :controller => 'photos', :action => 'show', + # :id => /[A-Z]\d{5}/ + # + # will have {:controller=>"photos", :action=>"show", :id=>/[A-Z]\d{5}/} + # as requirements. def requirements - # needed for rails `rails routes` @defaults.merge(path.requirements).delete_if { |_, v| /.+?/ == v } diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index d641642338..ffcd875b4d 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -5,7 +5,7 @@ module ActionDispatch # Normalizes URI path. # # Strips off trailing slash and ensures there is a leading slash. - # Also converts downcase url encoded string to uppercase. + # Also converts downcase URL encoded string to uppercase. # # normalize_path("/foo") # => "/foo" # normalize_path("/foo/") # => "/foo" diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 1c50192867..335797f4b9 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -154,7 +154,7 @@ module ActionDispatch end end - # Loop through the requirements AST + # Loop through the requirements AST. class Each < FunctionalVisitor # :nodoc: def visit(node, block) block.call(node) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index c61cb3fd68..e565c03a8a 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -160,7 +160,7 @@ module ActionDispatch # Raised when storing more than 4K of session data. CookieOverflow = Class.new StandardError - # Include in a cookie jar to allow chaining, e.g. cookies.permanent.signed + # Include in a cookie jar to allow chaining, e.g. cookies.permanent.signed. module ChainedCookieJars # Returns a jar that'll automatically set the assigned cookies to have an expiration date 20 years from now. Example: # @@ -345,16 +345,16 @@ module ActionDispatch options[:path] ||= "/" if options[:domain] == :all || options[:domain] == "all" - # if there is a provided tld length then we use it otherwise default domain regexp + # If there is a provided tld length then we use it otherwise default domain regexp. domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP - # if host is not ip and matches domain regexp + # If host is not ip and matches domain regexp. # (ip confirms to domain regexp so we explicitly check for ip) options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) ".#{$&}" end elsif options[:domain].is_a? Array - # if host matches one of the supplied domains without a dot in front of it + # If host matches one of the supplied domains without a dot in front of it. options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } end end @@ -404,7 +404,7 @@ module ActionDispatch @delete_cookies[name.to_s] == options end - # Removes all cookies on the client machine by calling <tt>delete</tt> for each cookie + # Removes all cookies on the client machine by calling <tt>delete</tt> for each cookie. def clear(options = {}) @cookies.each_key { |k| delete(k, options) } end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index cbe2f4be4d..6b29ce63ba 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -65,7 +65,7 @@ module ActionDispatch self.flash = flash_hash.dup end - if (!session.respond_to?(:loaded?) || session.loaded?) && # (reset_session uses {}, which doesn't implement #loaded?) + if (!session.respond_to?(:loaded?) || session.loaded?) && # reset_session uses {}, which doesn't implement #loaded? session.key?("flash") && session["flash"].nil? session.delete("flash") end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 8bae5bfeff..53d5a4918c 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -157,13 +157,13 @@ module ActionDispatch def ips_from(header) # :doc: return [] unless header - # Split the comma-separated list into an array of strings + # Split the comma-separated list into an array of strings. ips = header.strip.split(/[,\s]+/) ips.select do |ip| begin - # Only return IPs that are valid according to the IPAddr#new method + # Only return IPs that are valid according to the IPAddr#new method. range = IPAddr.new(ip).to_range - # we want to make sure nobody is sneaking a netmask in + # We want to make sure nobody is sneaking a netmask in. range.begin == range.end rescue ArgumentError nil diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index d9f018c8ac..21ccf5a097 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -53,7 +53,7 @@ module ActionDispatch rescue ArgumentError => argument_error if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} begin - # Note that the regexp does not allow $1 to end with a ':' + # Note that the regexp does not allow $1 to end with a ':'. $1.constantize rescue LoadError, NameError raise ActionDispatch::Session::SessionRestoreError diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 90f26a1c33..5a99714ec2 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -8,7 +8,7 @@ module ActionDispatch # The exceptions app should be passed as parameter on initialization # of ShowExceptions. Every time there is an exception, ShowExceptions will # store the exception in env["action_dispatch.exception"], rewrite the - # PATH_INFO to the exception status code and call the rack app. + # PATH_INFO to the exception status code and call the Rack app. # # If the application returns a "X-Cascade" pass response, this middleware # will send an empty response as result with the correct status code. diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb index 429ea7057c..2d21ae63f5 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb @@ -60,7 +60,7 @@ <%= link_to "Path", "#", 'data-route-helper' => '_path', title: "Returns a relative path (without the http or domain)" %> / <%= link_to "Url", "#", 'data-route-helper' => '_url', - title: "Returns an absolute url (with the http and domain)" %> + title: "Returns an absolute URL (with the http and domain)" %> </th> <th><%# HTTP Verb %> </th> @@ -93,7 +93,7 @@ } } - // get JSON from url and invoke callback with result + // get JSON from URL and invoke callback with result function getJSON(url, success) { var xhr = new XMLHttpRequest(); xhr.open('GET', url); diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index a2a80f39fc..74ba6466cf 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -7,10 +7,10 @@ module ActionDispatch ENV_SESSION_KEY = Rack::RACK_SESSION # :nodoc: ENV_SESSION_OPTIONS_KEY = Rack::RACK_SESSION_OPTIONS # :nodoc: - # Singleton object used to determine if an optional param wasn't specified + # Singleton object used to determine if an optional param wasn't specified. Unspecified = Object.new - # Creates a session hash, merging the properties of the previous session if any + # Creates a session hash, merging the properties of the previous session if any. def self.create(store, req, default_options) session_was = find req session = Request::Session.new(store, req) @@ -63,7 +63,7 @@ module ActionDispatch @req = req @delegate = {} @loaded = false - @exists = nil # we haven't checked yet + @exists = nil # We haven't checked yet. end def id @@ -79,7 +79,7 @@ module ActionDispatch options = self.options || {} @by.send(:delete_session, @req, options.id(@req), options) - # Load the new sid to be written with the response + # Load the new sid to be written with the response. @loaded = false load_for_write! end diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index 01bc871e5f..3615e4b1d8 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -40,7 +40,6 @@ module ActionDispatch class ParamEncoder # :nodoc: # Convert nested Hash to HashWithIndifferentAccess. - # def self.normalize_encode_params(params) case params when Array @@ -63,7 +62,7 @@ module ActionDispatch end end - # Remove nils from the params hash + # Remove nils from the params hash. class NoNilParamEncoder < ParamEncoder # :nodoc: def self.handle_array(params) list = super diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index c554ce98bc..60d4789a63 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -120,7 +120,7 @@ module ActionDispatch # controller :blog do # get 'blog/show' => :list # get 'blog/delete' => :delete - # get 'blog/edit' => :edit + # get 'blog/edit' => :edit # end # # # provides named routes for show, delete, and edit diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index b91ffb8419..9aa4b92df2 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -196,7 +196,7 @@ module ActionDispatch @buffer << @view.render(partial: "routes/route", collection: routes) end - # the header is part of the HTML page, so we don't construct it here. + # The header is part of the HTML page, so we don't construct it here. def header(routes) end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 8d9f70e3c6..8ad17504ae 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -17,9 +17,9 @@ module ActionDispatch CALL = ->(app, req) { app.call req.env } def initialize(app, constraints, strategy) - # Unwrap Constraints objects. I don't actually think it's possible + # Unwrap Constraints objects. I don't actually think it's possible # to pass a Constraints object to this constructor, but there were - # multiple places that kept testing children of this object. I + # multiple places that kept testing children of this object. I # *think* they were just being defensive, but I have no idea. if app.is_a?(self.class) constraints += app.constraints @@ -218,7 +218,7 @@ module ActionDispatch private def add_wildcard_options(options, formatted, path_ast) # Add a constraint for wildcard route to make it non-greedy and match the - # optional format part of the route by default + # optional format part of the route by default. if formatted != false path_ast.grep(Journey::Nodes::Star).each_with_object({}) { |node, hash| hash[node.name.to_sym] ||= /.+?/ @@ -396,7 +396,7 @@ module ActionDispatch end module Base - # Matches a url pattern to one or more routes. + # Matches a URL pattern to one or more routes. # # You should not use the +match+ method in your router # without specifying an HTTP method. @@ -406,7 +406,7 @@ module ActionDispatch # # sets :controller, :action and :id in params # match ':controller/:action/:id', via: [:get, :post] # - # Note that +:controller+, +:action+ and +:id+ are interpreted as url + # Note that +:controller+, +:action+ and +:id+ are interpreted as URL # query parameters and thus available through +params+ in an action. # # If you want to expose your action to GET, use +get+ in the router: @@ -455,7 +455,7 @@ module ActionDispatch # # === Options # - # Any options not seen here are passed on as params with the url. + # Any options not seen here are passed on as params with the URL. # # [:controller] # The route's controller. @@ -660,7 +660,7 @@ module ActionDispatch else prefix_options = options.slice(*_route.segment_keys) prefix_options[:relative_url_root] = "".freeze - # we must actually delete prefix segment keys to avoid passing them to next url_for + # We must actually delete prefix segment keys to avoid passing them to next url_for. _route.segment_keys.each { |k| options.delete(k) } _routes.url_helpers.send("#{name}_path", prefix_options) end @@ -1238,7 +1238,7 @@ module ActionDispatch # # resource :profile # - # creates six different routes in your application, all mapping to + # This creates six different routes in your application, all mapping to # the +Profiles+ controller (note that the controller is named after # the plural): # @@ -1323,14 +1323,14 @@ module ActionDispatch # # resources :posts, path_names: { new: "brand_new" } # - # The above example will now change /posts/new to /posts/brand_new + # The above example will now change /posts/new to /posts/brand_new. # # [:path] # Allows you to change the path prefix for the resource. # # resources :posts, path: 'postings' # - # The resource and all segments will now route to /postings instead of /posts + # The resource and all segments will now route to /postings instead of /posts. # # [:only] # Only generate routes for the given actions. @@ -1525,7 +1525,7 @@ module ActionDispatch end end - # See ActionDispatch::Routing::Mapper::Scoping#namespace + # See ActionDispatch::Routing::Mapper::Scoping#namespace. def namespace(path, options = {}) if resource_scope? nested { super } @@ -1545,7 +1545,7 @@ module ActionDispatch !parent_resource.singleton? && @scope[:shallow] end - # Matches a url pattern to one or more routes. + # Matches a URL pattern to one or more routes. # For more information, see match[rdoc-ref:Base#match]. # # match 'path' => 'controller#action', via: patch @@ -1904,7 +1904,7 @@ module ActionDispatch ast = Journey::Parser.parse path mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) - @set.add_route(mapping, ast, as, anchor) + @set.add_route(mapping, as) end def match_root_route(options) @@ -2003,7 +2003,7 @@ module ActionDispatch # concerns :commentable # end # - # concerns also work in any routes helper that you want to use: + # Concerns also work in any routes helper that you want to use: # # namespace :posts do # concerns :commentable @@ -2020,6 +2020,120 @@ module ActionDispatch end end + module CustomUrls + # Define custom url helpers that will be added to the application's + # routes. This allows you to override and/or replace the default behavior + # of routing helpers, e.g: + # + # direct :homepage do + # "http://www.rubyonrails.org" + # end + # + # direct :commentable do |model| + # [ model, anchor: model.dom_id ] + # end + # + # direct :main do + # { controller: "pages", action: "index", subdomain: "www" } + # end + # + # The return value from the block passed to `direct` must be a valid set of + # arguments for `url_for` which will actually build the URL string. This can + # be one of the following: + # + # * A string, which is treated as a generated URL + # * A hash, e.g. { controller: "pages", action: "index" } + # * An array, which is passed to `polymorphic_url` + # * An Active Model instance + # * An Active Model class + # + # NOTE: Other URL helpers can be called in the block but be careful not to invoke + # your custom URL helper again otherwise it will result in a stack overflow error. + # + # You can also specify default options that will be passed through to + # your URL helper definition, e.g: + # + # direct :browse, page: 1, size: 10 do |options| + # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] + # end + # + # In this instance the `params` object comes from the context in which the the + # block is executed, e.g. generating a URL inside a controller action or a view. + # If the block is executed where there isn't a params object such as this: + # + # Rails.application.routes.url_helpers.browse_path + # + # then it will raise a `NameError`. Because of this you need to be aware of the + # context in which you will use your custom URL helper when defining it. + # + # NOTE: The `direct` method can't be used inside of a scope block such as + # `namespace` or `scope` and will raise an error if it detects that it is. + def direct(name, options = {}, &block) + unless @scope.root? + raise RuntimeError, "The direct method can't be used inside a routes scope block" + end + + @set.add_url_helper(name, options, &block) + end + + # Define custom polymorphic mappings of models to URLs. This alters the + # behavior of `polymorphic_url` and consequently the behavior of + # `link_to` and `form_for` when passed a model instance, e.g: + # + # resource :basket + # + # resolve "Basket" do + # [:basket] + # end + # + # This will now generate "/basket" when a `Basket` instance is passed to + # `link_to` or `form_for` instead of the standard "/baskets/:id". + # + # NOTE: This custom behavior only applies to simple polymorphic URLs where + # a single model instance is passed and not more complicated forms, e.g: + # + # # config/routes.rb + # resource :profile + # namespace :admin do + # resources :users + # end + # + # resolve("User") { [:profile] } + # + # # app/views/application/_menu.html.erb + # link_to "Profile", @current_user + # link_to "Profile", [:admin, @current_user] + # + # The first `link_to` will generate "/profile" but the second will generate + # the standard polymorphic URL of "/admin/users/1". + # + # You can pass options to a polymorphic mapping - the arity for the block + # needs to be two as the instance is passed as the first argument, e.g: + # + # resolve "Basket", anchor: "items" do |basket, options| + # [:basket, options] + # end + # + # This generates the URL "/basket#items" because when the last item in an + # array passed to `polymorphic_url` is a hash then it's treated as options + # to the URL helper that gets called. + # + # NOTE: The `resolve` method can't be used inside of a scope block such as + # `namespace` or `scope` and will raise an error if it detects that it is. + def resolve(*args, &block) + unless @scope.root? + raise RuntimeError, "The resolve method can't be used inside a routes scope block" + end + + options = args.extract_options! + args = args.flatten(1) + + args.each do |klass| + @set.add_polymorphic_mapping(klass, options, &block) + end + end + end + class Scope # :nodoc: OPTIONS = [:path, :shallow_path, :as, :shallow_prefix, :module, :controller, :action, :path_names, :constraints, @@ -2040,6 +2154,14 @@ module ActionDispatch scope_level == :nested end + def null? + @hash.nil? && @parent.nil? + end + + def root? + @parent.null? + end + def resources? scope_level == :resources end @@ -2113,6 +2235,7 @@ module ActionDispatch include Scoping include Concerns include Resources + include CustomUrls end end end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 432b9bf4c1..e89ea8b21d 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -40,7 +40,7 @@ module ActionDispatch # # Example usage: # - # edit_polymorphic_path(@post) # => "/posts/1/edit" + # edit_polymorphic_path(@post) # => "/posts/1/edit" # polymorphic_path(@post, format: :pdf) # => "/posts/1.pdf" # # == Usage with mounted engines @@ -103,6 +103,10 @@ module ActionDispatch return polymorphic_url record, options end + if mapping = polymorphic_mapping(record_or_hash_or_array) + return mapping.call(self, [record_or_hash_or_array, options], false) + end + opts = options.dup action = opts.delete :action type = opts.delete(:routing_type) || :url @@ -123,6 +127,10 @@ module ActionDispatch return polymorphic_path record, options end + if mapping = polymorphic_mapping(record_or_hash_or_array) + return mapping.call(self, [record_or_hash_or_array, options], true) + end + opts = options.dup action = opts.delete :action type = :path @@ -156,6 +164,14 @@ module ActionDispatch polymorphic_path(record_or_hash, options.merge(action: action)) end + def polymorphic_mapping(record) + if record.respond_to?(:to_model) + _routes.polymorphic_mappings[record.to_model.model_name.name] + else + _routes.polymorphic_mappings[record.class.name] + end + end + class HelperMethodBuilder # :nodoc: CACHE = { "path" => {}, "url" => {} } @@ -255,9 +271,13 @@ module ActionDispatch [named_route, args] end - def handle_model_call(target, model) - method, args = handle_model model - target.send(method, *args) + def handle_model_call(target, record) + if mapping = polymorphic_mapping(target, record) + mapping.call(target, [record], suffix == "path") + else + method, args = handle_model(record) + target.send(method, *args) + end end def handle_list(list) @@ -303,6 +323,14 @@ module ActionDispatch private + def polymorphic_mapping(target, record) + if record.respond_to?(:to_model) + target._routes.polymorphic_mappings[record.to_model.model_name.name] + else + target._routes.polymorphic_mappings[record.class.name] + end + end + def get_method_for_class(klass) name = @key_strategy.call klass.model_name get_method_for_string name diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index dabc045007..3bcb341758 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -36,6 +36,8 @@ module ActionDispatch uri.host ||= req.host uri.port ||= req.port unless req.standard_port? + req.commit_flash + body = %(<html><body>You are being <a href="#{ERB::Util.unwrapped_html_escape(uri.to_s)}">redirected</a>.</body></html>) headers = { @@ -144,7 +146,7 @@ module ActionDispatch # # get 'docs/:article', to: redirect('/wiki/%{article}') # - # Note that if you return a path without a leading slash then the url is prefixed with the + # Note that if you return a path without a leading slash then the URL is prefixed with the # current SCRIPT_NAME environment variable. This is typically '/' but may be different in # a mounted engine or where the application is deployed to a subdirectory of a website. # @@ -163,7 +165,7 @@ module ActionDispatch # Note that the +do end+ syntax for the redirect block wouldn't work, as Ruby would pass # the block to +get+ instead of +redirect+. Use <tt>{ ... }</tt> instead. # - # The options version of redirect allows you to supply only the parts of the url which need + # The options version of redirect allows you to supply only the parts of the URL which need # to change, it also supports interpolation of the path similar to the first example. # # get 'stores/:name', to: redirect(subdomain: 'stores', path: '/%{name}') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5b873aeab7..129e90037e 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -73,6 +73,7 @@ module ActionDispatch @routes = {} @path_helpers = Set.new @url_helpers = Set.new + @custom_helpers = Set.new @url_helpers_module = Module.new @path_helpers_module = Module.new end @@ -88,16 +89,30 @@ module ActionDispatch def clear! @path_helpers.each do |helper| - @path_helpers_module.send :undef_method, helper + @path_helpers_module.send :remove_method, helper end @url_helpers.each do |helper| - @url_helpers_module.send :undef_method, helper + @url_helpers_module.send :remove_method, helper + end + + @custom_helpers.each do |helper| + path_name = :"#{helper}_path" + url_name = :"#{helper}_url" + + if @path_helpers_module.method_defined?(path_name) + @path_helpers_module.send :remove_method, path_name + end + + if @url_helpers_module.method_defined?(url_name) + @url_helpers_module.send :remove_method, url_name + end end @routes.clear @path_helpers.clear @url_helpers.clear + @custom_helpers.clear end def add(name, route) @@ -143,6 +158,23 @@ module ActionDispatch routes.length end + def add_url_helper(name, defaults, &block) + @custom_helpers << name + helper = CustomUrlHelper.new(name, defaults, &block) + + @path_helpers_module.module_eval do + define_method(:"#{name}_path") do |*args| + helper.call(self, args, true) + end + end + + @url_helpers_module.module_eval do + define_method(:"#{name}_url") do |*args| + helper.call(self, args, false) + end + end + end + class UrlHelper def self.create(route, options, route_name, url_strategy) if optimize_helper?(route) @@ -263,7 +295,7 @@ module ActionDispatch end private - # Create a url helper allowing ordered parameters to be associated + # Create a URL helper allowing ordered parameters to be associated # with corresponding dynamic segments, so you can do: # # foo_url(bar, baz, bang) @@ -305,7 +337,7 @@ module ActionDispatch attr_accessor :formatter, :set, :named_routes, :default_scope, :router attr_accessor :disable_clear_and_finalize, :resources_path_names attr_accessor :default_url_options - attr_reader :env_key + attr_reader :env_key, :polymorphic_mappings alias :routes :set @@ -347,6 +379,7 @@ module ActionDispatch @set = Journey::Routes.new @router = Journey::Router.new @set @formatter = Journey::Formatter.new self + @polymorphic_mappings = {} end def eager_load! @@ -408,6 +441,7 @@ module ActionDispatch named_routes.clear set.clear formatter.clear + @polymorphic_mappings.clear @prepend.each { |blk| eval_block(blk) } end @@ -452,17 +486,50 @@ module ActionDispatch # Define url_for in the singleton level so one can do: # Rails.application.routes.url_helpers.url_for(args) - @_routes = routes + proxy_class = Class.new do + include UrlFor + include routes.named_routes.path_helpers_module + include routes.named_routes.url_helpers_module + + attr_reader :_routes + + def initialize(routes) + @_routes = routes + end + + def optimize_routes_generation? + @_routes.optimize_routes_generation? + end + end + + @_proxy = proxy_class.new(routes) + class << self def url_for(options) - @_routes.url_for(options) + @_proxy.url_for(options) + end + + def full_url_for(options) + @_proxy.full_url_for(options) + end + + def route_for(name, *args) + @_proxy.route_for(name, *args) end def optimize_routes_generation? - @_routes.optimize_routes_generation? + @_proxy.optimize_routes_generation? end - attr_reader :_routes + def polymorphic_url(record_or_hash_or_array, options = {}) + @_proxy.polymorphic_url(record_or_hash_or_array, options) + end + + def polymorphic_path(record_or_hash_or_array, options = {}) + @_proxy.polymorphic_path(record_or_hash_or_array, options) + end + + def _routes; @_proxy._routes; end def url_options; {}; end end @@ -506,7 +573,7 @@ module ActionDispatch routes.empty? end - def add_route(mapping, path_ast, name, anchor) + def add_route(mapping, name) raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i) if name && named_routes[name] @@ -523,20 +590,58 @@ module ActionDispatch if route.segment_keys.include?(:controller) ActiveSupport::Deprecation.warn(<<-MSG.squish) Using a dynamic :controller segment in a route is deprecated and - will be removed in Rails 5.1. + will be removed in Rails 5.2. MSG end if route.segment_keys.include?(:action) ActiveSupport::Deprecation.warn(<<-MSG.squish) Using a dynamic :action segment in a route is deprecated and - will be removed in Rails 5.1. + will be removed in Rails 5.2. MSG end route end + def add_polymorphic_mapping(klass, options, &block) + @polymorphic_mappings[klass] = CustomUrlHelper.new(klass, options, &block) + end + + def add_url_helper(name, options, &block) + named_routes.add_url_helper(name, options, &block) + end + + class CustomUrlHelper + attr_reader :name, :defaults, :block + + def initialize(name, defaults, &block) + @name = name + @defaults = defaults + @block = block + end + + def call(t, args, only_path = false) + options = args.extract_options! + url = t.full_url_for(eval_block(t, args, options)) + + if only_path + "/" + url.partition(%r{(?<!/)/(?!/)}).last + else + url + end + end + + private + def eval_block(t, args, options) + t.instance_exec(*args, merge_defaults(options), &block) + end + + def merge_defaults(options) + defaults ? defaults.merge(options) : options + end + end + class Generator PARAMETERIZE = lambda do |name, value| if name == :controller @@ -751,8 +856,7 @@ module ActionDispatch params[key] = URI.parser.unescape(value) end end - old_params = req.path_parameters - req.path_parameters = old_params.merge params + req.path_parameters = params app = route.app if app.matches?(req) && app.dispatcher? begin diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 3e564f13d8..008216cc80 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -113,10 +113,10 @@ module ActionDispatch default_url_options end - # Generate a url based on the options provided, default_url_options and the + # Generate a URL based on the options provided, default_url_options and the # routes defined in routes.rb. The following options are supported: # - # * <tt>:only_path</tt> - If true, the relative url is returned. Defaults to +false+. + # * <tt>:only_path</tt> - If true, the relative URL is returned. Defaults to +false+. # * <tt>:protocol</tt> - The protocol to connect to. Defaults to 'http'. # * <tt>:host</tt> - Specifies the host the link should be targeted at. # If <tt>:only_path</tt> is false, this option must be @@ -164,6 +164,10 @@ module ActionDispatch # implicitly used by +url_for+ can always be overwritten like shown on the # last +url_for+ calls. def url_for(options = nil) + full_url_for(options) + end + + def full_url_for(options = nil) # :nodoc: case options when nil _routes.url_for(url_options.symbolize_keys) @@ -192,6 +196,10 @@ module ActionDispatch end end + def route_for(name, *args) # :nodoc: + public_send(:"#{name}_url", *args) + end + protected def optimize_routes_generation? diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb new file mode 100644 index 0000000000..98fdb36c91 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -0,0 +1,125 @@ +require "capybara/dsl" +require "capybara/minitest" +require "action_controller" +require "action_dispatch/system_testing/driver" +require "action_dispatch/system_testing/server" +require "action_dispatch/system_testing/test_helpers/screenshot_helper" +require "action_dispatch/system_testing/test_helpers/setup_and_teardown" + +module ActionDispatch + # = System Testing + # + # System tests let you test applications in the browser. Because system + # tests use a real browser experience, you can test all of your JavaScript + # easily from your test suite. + # + # To create a system test in your application, extend your test class + # from <tt>ApplicationSystemTestCase</tt>. System tests use Capybara as a + # base and allow you to configure the settings through your + # <tt>application_system_test_case.rb</tt> file that is generated with a new + # application or scaffold. + # + # Here is an example system test: + # + # require 'application_system_test_case' + # + # class Users::CreateTest < ApplicationSystemTestCase + # test "adding a new user" do + # visit users_path + # click_on 'New User' + # + # fill_in 'Name', with: 'Arya' + # click_on 'Create User' + # + # assert_text 'Arya' + # end + # end + # + # When generating an application or scaffold, an +application_system_test_case.rb+ + # file will also be generated containing the base class for system testing. + # This is where you can change the driver, add Capybara settings, and other + # configuration for your system tests. + # + # require "test_helper" + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :selenium, using: :chrome, screen_size: [1400, 1400] + # end + # + # By default, <tt>ActionDispatch::SystemTestCase</tt> is driven by the + # Selenium driver, with the Chrome browser, and a browser size of 1400x1400. + # + # Changing the driver configuration options is easy. Let's say you want to use + # the Firefox browser instead of Chrome. In your +application_system_test_case.rb+ + # file add the following: + # + # require "test_helper" + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :selenium, using: :firefox + # end + # + # +driven_by+ has a required argument for the driver name. The keyword + # arguments are +:using+ for the browser and +:screen_size+ to change the + # size of the browser screen. These two options are not applicable for + # headless drivers and will be silently ignored if passed. + # + # To use a headless driver, like Poltergeist, update your Gemfile to use + # Poltergeist instead of Selenium and then declare the driver name in the + # +application_system_test_case.rb+ file. In this case you would leave out the +:using+ + # option because the driver is headless. + # + # require "test_helper" + # require "capybara/poltergeist" + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :poltergeist + # end + # + # Because <tt>ActionDispatch::SystemTestCase</tt> is a shim between Capybara + # and Rails, any driver that is supported by Capybara is supported by system + # tests as long as you include the required gems and files. + class SystemTestCase < IntegrationTest + include Capybara::DSL + include Capybara::Minitest::Assertions + include SystemTesting::TestHelpers::SetupAndTeardown + include SystemTesting::TestHelpers::ScreenshotHelper + + def initialize(*) # :nodoc: + super + self.class.driver.use + end + + def self.start_application # :nodoc: + Capybara.app = Rack::Builder.new do + map "/" do + run Rails.application + end + end + + SystemTesting::Server.new.run + end + + class_attribute :driver, instance_accessor: false + + # System Test configuration options + # + # The default settings are Selenium, using Chrome, with a screen size + # of 1400x1400. + # + # Examples: + # + # driven_by :poltergeist + # + # driven_by :selenium, using: :firefox + # + # driven_by :selenium, screen_size: [800, 800] + def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}) + self.driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size, options: options) + end + + driven_by :selenium + end + + SystemTestCase.start_application +end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb new file mode 100644 index 0000000000..5cf17883f7 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -0,0 +1,34 @@ +module ActionDispatch + module SystemTesting + class Driver # :nodoc: + def initialize(name, **options) + @name = name + @browser = options[:using] + @screen_size = options[:screen_size] + @options = options[:options] + end + + def use + register if selenium? + setup + end + + private + def selenium? + @name == :selenium + end + + def register + Capybara.register_driver @name do |app| + Capybara::Selenium::Driver.new(app, { browser: @browser }.merge(@options)).tap do |driver| + driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) + end + end + end + + def setup + Capybara.current_driver = @name + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb new file mode 100644 index 0000000000..4a214ef713 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -0,0 +1,32 @@ +require "rack/handler/puma" + +module ActionDispatch + module SystemTesting + class Server # :nodoc: + def run + register + setup + end + + private + def register + Capybara.register_server :rails_puma do |app, port, host| + Rack::Handler::Puma.run(app, Port: port, Threads: "0:1") + end + end + + def setup + set_server + set_port + end + + def set_server + Capybara.server = :rails_puma + end + + def set_port + Capybara.always_include_port = true + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb new file mode 100644 index 0000000000..859d68e475 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -0,0 +1,94 @@ +module ActionDispatch + module SystemTesting + module TestHelpers + # Screenshot helper for system testing. + module ScreenshotHelper + # Takes a screenshot of the current page in the browser. + # + # +take_screenshot+ can be used at any point in your system tests to take + # a screenshot of the current state. This can be useful for debugging or + # automating visual testing. + # + # The screenshot will be displayed in your console, if supported. + # + # You can set the +RAILS_SYSTEM_TESTING_SCREENSHOT+ environment variable to + # control the output. Possible values are: + # * [+inline+ (default)] display the screenshot in the terminal using the + # iTerm image protocol (http://iterm2.com/documentation-images.html). + # * [+simple+] only display the screenshot path. + # This is the default value if the +CI+ environment variables + # is defined. + # * [+artifact+] display the screenshot in the terminal, using the terminal + # artifact format (http://buildkite.github.io/terminal/inline-images/). + def take_screenshot + save_image + puts display_image + end + + # Takes a screenshot of the current page in the browser if the test + # failed. + # + # +take_failed_screenshot+ is included in <tt>application_system_test_case.rb</tt> + # that is generated with the application. To take screenshots when a test + # fails add +take_failed_screenshot+ to the teardown block before clearing + # sessions. + def take_failed_screenshot + take_screenshot if failed? && supports_screenshot? + end + + private + def image_name + failed? ? "failures_#{method_name}" : method_name + end + + def image_path + "tmp/screenshots/#{image_name}.png" + end + + def save_image + page.save_screenshot(Rails.root.join(image_path)) + end + + def output_type + # Environment variables have priority + output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] || ENV["CAPYBARA_INLINE_SCREENSHOT"] + + # If running in a CI environment, default to simple + output_type ||= "simple" if ENV["CI"] + + # Default + output_type ||= "inline" + + output_type + end + + def display_image + message = "[Screenshot]: #{image_path}\n" + + case output_type + when "artifact" + message << "\e]1338;url=artifact://#{image_path}\a\n" + when "inline" + name = inline_base64(File.basename(image_path)) + image = inline_base64(File.read(image_path)) + message << "\e]1337;File=name=#{name};height=400px;inline=1:#{image}\a\n" + end + + message + end + + def inline_base64(path) + Base64.encode64(path).gsub("\n", "") + end + + def failed? + !passed? && !skipped? + end + + def supports_screenshot? + Capybara.current_driver != :rack_test + end + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb new file mode 100644 index 0000000000..187ba2cc5f --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb @@ -0,0 +1,20 @@ +module ActionDispatch + module SystemTesting + module TestHelpers + module SetupAndTeardown # :nodoc: + DEFAULT_HOST = "127.0.0.1" + + def before_setup + host! DEFAULT_HOST + super + end + + def after_teardown + take_failed_screenshot + Capybara.reset_sessions! + super + end + end + end + end +end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 817737341c..1baf979ac9 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -45,7 +45,7 @@ module ActionDispatch # # Asserts that the redirection was to the named route login_url # assert_redirected_to login_url # - # # Asserts that the redirection was to the url for @customer + # # Asserts that the redirection was to the URL for @customer # assert_redirected_to @customer # # # Asserts that the redirection matches the regular expression diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 37c1ca02b6..8645df4370 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -18,8 +18,8 @@ module ActionDispatch # assert_recognizes({controller: 'items', action: 'create'}, {path: 'items', method: :post}) # # You can also pass in +extras+ with a hash containing URL parameters that would normally be in the query string. This can be used - # to assert that values in the query string will end up in the params hash correctly. To test query strings you must use the - # extras argument, appending the query string on the path directly will not work. For example: + # to assert that values in the query string will end up in the params hash correctly. To test query strings you must use the extras + # argument because appending the query string on the path directly will not work. For example: # # # Asserts that a path of '/items/list/1?view=print' returns the correct options # assert_recognizes({controller: 'items', action: 'list', id: '1', view: 'print'}, 'items/list/1', { view: "print" }) @@ -132,8 +132,7 @@ module ActionDispatch end # A helper to make it easier to test different route configurations. - # This method temporarily replaces @routes - # with a new RouteSet instance. + # This method temporarily replaces @routes with a new RouteSet instance. # # The new instance is yielded to the passed block. Typically the block # will create some routes using <tt>set.draw { match ... }</tt>: @@ -186,7 +185,6 @@ module ActionDispatch method = :get end - # Assume given controller request = ActionController::TestRequest.create @controller.class if path =~ %r{://} diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 5fa0b727ab..2e2db98ad6 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -192,11 +192,10 @@ module ActionDispatch # HTTP methods in integration tests. +#process+ is only required when using a # request method that doesn't have a method defined in the integration tests. # - # This method returns a Response object, which one can use to - # inspect the details of the response. Furthermore, if this method was - # called from an ActionDispatch::IntegrationTest object, then that - # object's <tt>@response</tt> instance variable will point to the same - # response object. + # This method returns the response status, after performing the request. + # Furthermore, if this method was called from an ActionDispatch::IntegrationTest object, + # then that object's <tt>@response</tt> instance variable will point to a Response object + # which one can use to inspect the details of the response. # # Example: # process :get, '/author', params: { since: 201501011400 } @@ -247,7 +246,7 @@ module ActionDispatch wrapped_headers["HTTP_ACCEPT"] ||= [Mime[:js], Mime[:html], Mime[:xml], "text/xml", "*/*"].join(", ") end - # this modifies the passed request_env directly + # This modifies the passed request_env directly. if wrapped_headers.present? Http::Headers.from_hash(request_env).merge!(wrapped_headers) end @@ -258,7 +257,7 @@ module ActionDispatch 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 + # Make sure requested path is always a full URI. session.request(build_full_uri(path, request_env), request_env) @request_count += 1 @@ -325,8 +324,8 @@ module ActionDispatch def create_session(app) klass = APP_SESSIONS[app] ||= Class.new(Integration::Session) { - # If the app is a Rails app, make url_helpers available on the session - # This makes app.url_for and app.foo_path available in the console + # If the app is a Rails app, make url_helpers available on the session. + # This makes app.url_for and app.foo_path available in the console. if app.respond_to?(:routes) include app.routes.url_helpers include app.routes.mounted_helpers @@ -572,7 +571,7 @@ module ActionDispatch # end # # assert_response :success - # assert_equal({ id: Arcticle.last.id, title: "Ahoy!" }, response.parsed_body) + # assert_equal({ id: Article.last.id, title: "Ahoy!" }, response.parsed_body) # end # end # diff --git a/actionpack/lib/action_dispatch/testing/test_request.rb b/actionpack/lib/action_dispatch/testing/test_request.rb index 91b25ec155..ec949c869b 100644 --- a/actionpack/lib/action_dispatch/testing/test_request.rb +++ b/actionpack/lib/action_dispatch/testing/test_request.rb @@ -9,7 +9,7 @@ module ActionDispatch "HTTP_USER_AGENT" => "Rails Testing", ) - # Create a new test request with default `env` values + # Create a new test request with default `env` values. def self.create(env = {}) env = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application env["rack.request.cookie_hash"] ||= {}.with_indifferent_access diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index d8f86630b1..fddc3033d5 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -6,7 +6,7 @@ module ActionPack module VERSION MAJOR = 5 - MINOR = 1 + MINOR = 2 TINY = 0 PRE = "alpha" diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 459b0d6c54..4185ce1a1f 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -439,3 +439,11 @@ class ActiveSupport::TestCase skip message if defined?(JRUBY_VERSION) end end + +class DrivenByRackTest < ActionDispatch::SystemTestCase + driven_by :rack_test +end + +class DrivenBySeleniumWithChrome < ActionDispatch::SystemTestCase + driven_by :selenium, using: :chrome +end diff --git a/actionpack/test/controller/api/with_helpers_test.rb b/actionpack/test/controller/api/with_helpers_test.rb new file mode 100644 index 0000000000..06db949153 --- /dev/null +++ b/actionpack/test/controller/api/with_helpers_test.rb @@ -0,0 +1,42 @@ +require "abstract_unit" + +module ApiWithHelper + def my_helper + "helper" + end +end + +class WithHelpersController < ActionController::API + include ActionController::Helpers + helper ApiWithHelper + + def with_helpers + render plain: self.class.helpers.my_helper + end +end + +class SubclassWithHelpersController < WithHelpersController + def with_helpers + render plain: self.class.helpers.my_helper + end +end + +class WithHelpersTest < ActionController::TestCase + tests WithHelpersController + + def test_with_helpers + get :with_helpers + + assert_equal "helper", response.body + end +end + +class SubclassWithHelpersTest < ActionController::TestCase + tests WithHelpersController + + def test_with_helpers + get :with_helpers + + assert_equal "helper", response.body + end +end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 42a5157010..4e969fac07 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -11,6 +11,12 @@ end class EmptyController < ActionController::Base end +class SimpleController < ActionController::Base + def hello + self.response_body = "hello" + end +end + class NonEmptyController < ActionController::Base def public_action head :ok @@ -118,6 +124,27 @@ class ControllerInstanceTests < ActiveSupport::TestCase controller = klass.new assert_equal "examples", controller.controller_path end + + def test_response_has_default_headers + original_default_headers = ActionDispatch::Response.default_headers + + ActionDispatch::Response.default_headers = { + "X-Frame-Options" => "DENY", + "X-Content-Type-Options" => "nosniff", + "X-XSS-Protection" => "1;" + } + + response_headers = SimpleController.action("hello").call( + "REQUEST_METHOD" => "GET", + "rack.input" => -> {} + )[1] + + assert response_headers.key?("X-Frame-Options") + assert response_headers.key?("X-Content-Type-Options") + assert response_headers.key?("X-XSS-Protection") + ensure + ActionDispatch::Response.default_headers = original_default_headers + end end class PerformActionTest < ActionController::TestCase diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index e76628b936..581081dd07 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -1,4 +1,5 @@ require "abstract_unit" +require "timeout" require "concurrent/atomic/count_down_latch" Thread.abort_on_exception = true diff --git a/actionpack/test/controller/metal_test.rb b/actionpack/test/controller/metal_test.rb new file mode 100644 index 0000000000..e16452ed6f --- /dev/null +++ b/actionpack/test/controller/metal_test.rb @@ -0,0 +1,30 @@ +require "abstract_unit" + +class MetalControllerInstanceTests < ActiveSupport::TestCase + class SimpleController < ActionController::Metal + def hello + self.response_body = "hello" + end + end + + def test_response_has_default_headers + original_default_headers = ActionDispatch::Response.default_headers + + ActionDispatch::Response.default_headers = { + "X-Frame-Options" => "DENY", + "X-Content-Type-Options" => "nosniff", + "X-XSS-Protection" => "1;" + } + + response_headers = SimpleController.action("hello").call( + "REQUEST_METHOD" => "GET", + "rack.input" => -> {} + )[1] + + refute response_headers.key?("X-Frame-Options") + refute response_headers.key?("X-Content-Type-Options") + refute response_headers.key?("X-XSS-Protection") + ensure + ActionDispatch::Response.default_headers = original_default_headers + end +end diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 2893eb7b91..7725c25e22 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -53,6 +53,15 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } end + test "empty? returns true when params contains no key/value pairs" do + params = ActionController::Parameters.new + assert params.empty? + end + + test "empty? returns false when any params are present" do + refute @params.empty? + end + test "except retains permitted status" do @params.permit! assert @params.except(:person).permitted? @@ -75,6 +84,45 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params[:person].fetch(:name).permitted? end + test "has_key? returns true if the given key is present in the params" do + assert @params.has_key?(:person) + end + + test "has_key? returns false if the given key is not present in the params" do + refute @params.has_key?(:address) + end + + test "has_value? returns true if the given value is present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + assert params.has_value?("Chicago") + end + + test "has_value? returns false if the given value is not present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + refute params.has_value?("New York") + end + + test "include? returns true if the given key is present in the params" do + assert @params.include?(:person) + end + + test "include? returns false if the given key is not present in the params" do + refute @params.include?(:address) + end + + test "key? returns true if the given key is present in the params" do + assert @params.key?(:person) + end + + test "key? returns false if the given key is not present in the params" do + refute @params.key?(:address) + end + + test "keys returns an array of the keys of the params" do + assert_equal ["person"], @params.keys + assert_equal ["age", "name", "addresses"], @params[:person].keys + end + test "reject retains permitted status" do assert_not @params.reject { |k| k == "person" }.permitted? end @@ -120,6 +168,21 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params.transform_values { |v| v }.permitted? end + test "value? returns true if the given value is present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + assert params.value?("Chicago") + end + + test "value? returns false if the given value is not present in the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + refute params.value?("New York") + end + + test "values returns an array of the values of the params" do + params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") + assert_equal ["Chicago", "Illinois"], params.values + end + test "values_at retains permitted status" do @params.permit! assert @params.values_at(:person).first.permitted? diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 8920914af1..3e067314d6 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -302,6 +302,47 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "32", @params[:person][:age] end + test "#reverse_merge with parameters" do + default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! + merged_params = @params.reverse_merge(default_params) + + assert_equal "1234", merged_params[:id] + refute_predicate merged_params[:person], :empty? + end + + test "#with_defaults is an alias of reverse_merge" do + default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! + merged_params = @params.with_defaults(default_params) + + assert_equal "1234", merged_params[:id] + refute_predicate merged_params[:person], :empty? + end + + test "not permitted is sticky beyond reverse_merge" do + refute_predicate @params.reverse_merge(a: "b"), :permitted? + end + + test "permitted is sticky beyond reverse_merge" do + @params.permit! + assert_predicate @params.reverse_merge(a: "b"), :permitted? + end + + test "#reverse_merge! with parameters" do + default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! + @params.reverse_merge!(default_params) + + assert_equal "1234", @params[:id] + refute_predicate @params[:person], :empty? + end + + test "#with_defaults! is an alias of reverse_merge!" do + default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! + @params.with_defaults!(default_params) + + assert_equal "1234", @params[:id] + refute_predicate @params[:person], :empty? + end + test "modifying the parameters" do @params[:person][:hometown] = "Chicago" @params[:person][:family] = { brother: "Jonas" } diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index faa57c4559..2a41d57b26 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -32,6 +32,10 @@ class ParamsWrapperTest < ActionController::TestCase def self.attribute_names [] end + + def self.stored_attributes + { settings: [:color, :size] } + end end class Person @@ -62,6 +66,17 @@ class ParamsWrapperTest < ActionController::TestCase end end + def test_store_accessors_wrapped + assert_called(User, :attribute_names, times: 2, returns: ["username"]) do + with_default_wrapper_options do + @request.env["CONTENT_TYPE"] = "application/json" + post :parse, params: { "username" => "sikachu", "color" => "blue", "size" => "large" } + assert_parameters("username" => "sikachu", "color" => "blue", "size" => "large", + "user" => { "username" => "sikachu", "color" => "blue", "size" => "large" }) + end + end + end + def test_specify_wrapper_name with_default_wrapper_options do UsersController.wrap_parameters :person diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index e4e968dfdb..f06a1f4d23 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -21,8 +21,8 @@ end class RedirectController < ActionController::Base # empty method not used anywhere to ensure methods like # `status` and `location` aren't called on `redirect_to` calls - def status; render plain: "called status"; end - def location; render plain: "called location"; end + def status; raise "Should not be called!"; end + def location; raise "Should not be called!"; end def simple_redirect redirect_to action: "hello_world" diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index 866600b935..052c974d68 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -19,6 +19,16 @@ class RendererTest < ActiveSupport::TestCase assert_equal controller, renderer.controller end + test "creating with new defaults" do + renderer = ApplicationController.renderer + + new_defaults = { https: true } + new_renderer = renderer.with_defaults(new_defaults).new + content = new_renderer.render(inline: "<%= request.ssl? %>") + + assert_equal "true", content + end + test "rendering with a class renderer" do renderer = ApplicationController.renderer content = renderer.render template: "ruby_template" @@ -103,6 +113,20 @@ class RendererTest < ActiveSupport::TestCase assert_equal "true", content end + test "return valid asset url with defaults" do + renderer = ApplicationController.renderer + content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" + + assert_equal "http://example.org/asset.jpg", content + end + + test "return valid asset url when https is true" do + renderer = ApplicationController.renderer.new https: true + content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" + + assert_equal "https://example.org/asset.jpg", content + end + private def render @render ||= ApplicationController.renderer.method(:render) diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index e084e45373..3a4307b64b 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -134,7 +134,7 @@ XML end def create - head :created, location: "created resource" + head :created, location: "/resource" end def render_cookie @@ -679,6 +679,11 @@ XML assert_equal "baz", @request.filtered_parameters[:foo] end + def test_path_is_kept_after_the_request + get :test_params, params: { id: "foo" } + assert_equal "/test_case_test/test/test_params/foo", @request.path + end + def test_path_params_reset_between_request get :test_params, params: { id: "foo" } assert_equal "foo", @request.path_parameters[:id] @@ -728,20 +733,6 @@ XML assert_equal "text/html", @response.body end - def test_request_path_info_and_format_reset - get :test_format, format: "json" - assert_equal "application/json", @response.body - - get :test_uri, format: "json" - assert_equal "/test_case_test/test/test_uri.json", @response.body - - get :test_format - assert_equal "text/html", @response.body - - get :test_uri - assert_equal "/test_case_test/test/test_uri", @response.body - end - def test_request_format_kwarg_overrides_params get :test_format, format: "json", params: { format: "html" } assert_equal "application/json", @response.body @@ -893,12 +884,12 @@ XML assert_response :created # Redirect url doesn't care that it wasn't a :redirect response. - assert_equal "created resource", @response.redirect_url + assert_equal "/resource", @response.redirect_url assert_equal @response.redirect_url, redirect_to_url # Must be a :redirect response. assert_raise(ActiveSupport::TestCase::Assertion) do - assert_redirected_to "created resource" + assert_redirected_to "/resource" end end diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb new file mode 100644 index 0000000000..cb5ca5888b --- /dev/null +++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb @@ -0,0 +1,325 @@ +require "abstract_unit" + +class TestCustomUrlHelpers < ActionDispatch::IntegrationTest + class Linkable + attr_reader :id + + def self.name + super.demodulize + end + + def initialize(id) + @id = id + end + + def linkable_type + self.class.name.underscore + end + end + + class Category < Linkable; end + class Collection < Linkable; end + class Product < Linkable; end + class Manufacturer < Linkable; end + + class Model + extend ActiveModel::Naming + include ActiveModel::Conversion + + attr_reader :id + + def initialize(id = nil) + @id = id + end + + remove_method :model_name + def model_name + @_model_name ||= ActiveModel::Name.new(self.class, nil, self.class.name.demodulize) + end + + def persisted? + false + end + end + + class Basket < Model; end + class User < Model; end + class Video < Model; end + + class Article + attr_reader :id + + def self.name + "Article" + end + + def initialize(id) + @id = id + end + end + + class Page + attr_reader :id + + def self.name + super.demodulize + end + + def initialize(id) + @id = id + end + end + + class CategoryPage < Page; end + class ProductPage < Page; end + + Routes = ActionDispatch::Routing::RouteSet.new + Routes.draw do + default_url_options host: "www.example.com" + + root to: "pages#index" + get "/basket", to: "basket#show", as: :basket + get "/posts/:id", to: "posts#show", as: :post + get "/profile", to: "users#profile", as: :profile + get "/media/:id", to: "media#show", as: :media + get "/pages/:id", to: "pages#show", as: :page + + resources :categories, :collections, :products, :manufacturers + + namespace :admin do + get "/dashboard", to: "dashboard#index" + end + + direct(:website) { "http://www.rubyonrails.org" } + direct("string") { "http://www.rubyonrails.org" } + direct(:helper) { basket_url } + direct(:linkable) { |linkable| [:"#{linkable.linkable_type}", { id: linkable.id }] } + direct(:nested) { |linkable| route_for(:linkable, linkable) } + direct(:params) { |params| params } + direct(:symbol) { :basket } + direct(:hash) { { controller: "basket", action: "show" } } + direct(:array) { [:admin, :dashboard] } + direct(:options) { |options| [:products, options] } + direct(:defaults, size: 10) { |options| [:products, options] } + + direct(:browse, page: 1, size: 10) do |options| + [:products, options.merge(params.permit(:page, :size).to_h.symbolize_keys)] + end + + resolve("Article") { |article| [:post, { id: article.id }] } + resolve("Basket") { |basket| [:basket] } + resolve("Manufacturer") { |manufacturer| route_for(:linkable, manufacturer) } + resolve("User", anchor: "details") { |user, options| [:profile, options] } + resolve("Video") { |video| [:media, { id: video.id }] } + resolve(%w[Page CategoryPage ProductPage]) { |page| [:page, { id: page.id }] } + end + + APP = build_app Routes + + def app + APP + end + + include Routes.url_helpers + + def setup + @category = Category.new("1") + @collection = Collection.new("2") + @product = Product.new("3") + @manufacturer = Manufacturer.new("apple") + @basket = Basket.new + @user = User.new + @video = Video.new("4") + @article = Article.new("5") + @page = Page.new("6") + @category_page = CategoryPage.new("7") + @product_page = ProductPage.new("8") + @path_params = { "controller" => "pages", "action" => "index" } + @unsafe_params = ActionController::Parameters.new(@path_params) + @safe_params = ActionController::Parameters.new(@path_params).permit(:controller, :action) + end + + def params + ActionController::Parameters.new(page: 2, size: 25) + end + + def test_direct_paths + assert_equal "/", website_path + assert_equal "/", Routes.url_helpers.website_path + + assert_equal "/", string_path + assert_equal "/", Routes.url_helpers.string_path + + assert_equal "/basket", helper_path + assert_equal "/basket", Routes.url_helpers.helper_path + + assert_equal "/categories/1", linkable_path(@category) + assert_equal "/categories/1", Routes.url_helpers.linkable_path(@category) + assert_equal "/collections/2", linkable_path(@collection) + assert_equal "/collections/2", Routes.url_helpers.linkable_path(@collection) + assert_equal "/products/3", linkable_path(@product) + assert_equal "/products/3", Routes.url_helpers.linkable_path(@product) + + assert_equal "/categories/1", nested_path(@category) + assert_equal "/categories/1", Routes.url_helpers.nested_path(@category) + + assert_equal "/", params_path(@safe_params) + assert_equal "/", Routes.url_helpers.params_path(@safe_params) + assert_raises(ArgumentError) { params_path(@unsafe_params) } + assert_raises(ArgumentError) { Routes.url_helpers.params_path(@unsafe_params) } + + assert_equal "/basket", symbol_path + assert_equal "/basket", Routes.url_helpers.symbol_path + assert_equal "/basket", hash_path + assert_equal "/basket", Routes.url_helpers.hash_path + assert_equal "/admin/dashboard", array_path + assert_equal "/admin/dashboard", Routes.url_helpers.array_path + + assert_equal "/products?page=2", options_path(page: 2) + assert_equal "/products?page=2", Routes.url_helpers.options_path(page: 2) + assert_equal "/products?size=10", defaults_path + assert_equal "/products?size=10", Routes.url_helpers.defaults_path + assert_equal "/products?size=20", defaults_path(size: 20) + assert_equal "/products?size=20", Routes.url_helpers.defaults_path(size: 20) + + assert_equal "/products?page=2&size=25", browse_path + assert_raises(NameError) { Routes.url_helpers.browse_path } + end + + def test_direct_urls + assert_equal "http://www.rubyonrails.org", website_url + assert_equal "http://www.rubyonrails.org", Routes.url_helpers.website_url + + assert_equal "http://www.rubyonrails.org", string_url + assert_equal "http://www.rubyonrails.org", Routes.url_helpers.string_url + + assert_equal "http://www.example.com/basket", helper_url + assert_equal "http://www.example.com/basket", Routes.url_helpers.helper_url + + assert_equal "http://www.example.com/categories/1", linkable_url(@category) + assert_equal "http://www.example.com/categories/1", Routes.url_helpers.linkable_url(@category) + assert_equal "http://www.example.com/collections/2", linkable_url(@collection) + assert_equal "http://www.example.com/collections/2", Routes.url_helpers.linkable_url(@collection) + assert_equal "http://www.example.com/products/3", linkable_url(@product) + assert_equal "http://www.example.com/products/3", Routes.url_helpers.linkable_url(@product) + + assert_equal "http://www.example.com/categories/1", nested_url(@category) + assert_equal "http://www.example.com/categories/1", Routes.url_helpers.nested_url(@category) + + assert_equal "http://www.example.com/", params_url(@safe_params) + assert_equal "http://www.example.com/", Routes.url_helpers.params_url(@safe_params) + assert_raises(ArgumentError) { params_url(@unsafe_params) } + assert_raises(ArgumentError) { Routes.url_helpers.params_url(@unsafe_params) } + + assert_equal "http://www.example.com/basket", symbol_url + assert_equal "http://www.example.com/basket", Routes.url_helpers.symbol_url + assert_equal "http://www.example.com/basket", hash_url + assert_equal "http://www.example.com/basket", Routes.url_helpers.hash_url + assert_equal "http://www.example.com/admin/dashboard", array_url + assert_equal "http://www.example.com/admin/dashboard", Routes.url_helpers.array_url + + assert_equal "http://www.example.com/products?page=2", options_url(page: 2) + assert_equal "http://www.example.com/products?page=2", Routes.url_helpers.options_url(page: 2) + assert_equal "http://www.example.com/products?size=10", defaults_url + assert_equal "http://www.example.com/products?size=10", Routes.url_helpers.defaults_url + assert_equal "http://www.example.com/products?size=20", defaults_url(size: 20) + assert_equal "http://www.example.com/products?size=20", Routes.url_helpers.defaults_url(size: 20) + + assert_equal "http://www.example.com/products?page=2&size=25", browse_url + assert_raises(NameError) { Routes.url_helpers.browse_url } + end + + def test_resolve_paths + assert_equal "/basket", polymorphic_path(@basket) + assert_equal "/basket", Routes.url_helpers.polymorphic_path(@basket) + + assert_equal "/profile#details", polymorphic_path(@user) + assert_equal "/profile#details", Routes.url_helpers.polymorphic_path(@user) + + assert_equal "/profile#password", polymorphic_path(@user, anchor: "password") + assert_equal "/profile#password", Routes.url_helpers.polymorphic_path(@user, anchor: "password") + + assert_equal "/media/4", polymorphic_path(@video) + assert_equal "/media/4", Routes.url_helpers.polymorphic_path(@video) + assert_equal "/media/4", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @video) + + assert_equal "/posts/5", polymorphic_path(@article) + assert_equal "/posts/5", Routes.url_helpers.polymorphic_path(@article) + assert_equal "/posts/5", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @article) + + assert_equal "/pages/6", polymorphic_path(@page) + assert_equal "/pages/6", Routes.url_helpers.polymorphic_path(@page) + assert_equal "/pages/6", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @page) + + assert_equal "/pages/7", polymorphic_path(@category_page) + assert_equal "/pages/7", Routes.url_helpers.polymorphic_path(@category_page) + assert_equal "/pages/7", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @category_page) + + assert_equal "/pages/8", polymorphic_path(@product_page) + assert_equal "/pages/8", Routes.url_helpers.polymorphic_path(@product_page) + assert_equal "/pages/8", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @product_page) + + assert_equal "/manufacturers/apple", polymorphic_path(@manufacturer) + assert_equal "/manufacturers/apple", Routes.url_helpers.polymorphic_path(@manufacturer) + end + + def test_resolve_urls + assert_equal "http://www.example.com/basket", polymorphic_url(@basket) + assert_equal "http://www.example.com/basket", Routes.url_helpers.polymorphic_url(@basket) + assert_equal "http://www.example.com/basket", polymorphic_url(@basket) + assert_equal "http://www.example.com/basket", Routes.url_helpers.polymorphic_url(@basket) + + assert_equal "http://www.example.com/profile#details", polymorphic_url(@user) + assert_equal "http://www.example.com/profile#details", Routes.url_helpers.polymorphic_url(@user) + + assert_equal "http://www.example.com/profile#password", polymorphic_url(@user, anchor: "password") + assert_equal "http://www.example.com/profile#password", Routes.url_helpers.polymorphic_url(@user, anchor: "password") + + assert_equal "http://www.example.com/media/4", polymorphic_url(@video) + assert_equal "http://www.example.com/media/4", Routes.url_helpers.polymorphic_url(@video) + assert_equal "http://www.example.com/media/4", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @video) + + assert_equal "http://www.example.com/posts/5", polymorphic_url(@article) + assert_equal "http://www.example.com/posts/5", Routes.url_helpers.polymorphic_url(@article) + assert_equal "http://www.example.com/posts/5", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @article) + + assert_equal "http://www.example.com/pages/6", polymorphic_url(@page) + assert_equal "http://www.example.com/pages/6", Routes.url_helpers.polymorphic_url(@page) + assert_equal "http://www.example.com/pages/6", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @page) + + assert_equal "http://www.example.com/pages/7", polymorphic_url(@category_page) + assert_equal "http://www.example.com/pages/7", Routes.url_helpers.polymorphic_url(@category_page) + assert_equal "http://www.example.com/pages/7", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @category_page) + + assert_equal "http://www.example.com/pages/8", polymorphic_url(@product_page) + assert_equal "http://www.example.com/pages/8", Routes.url_helpers.polymorphic_url(@product_page) + assert_equal "http://www.example.com/pages/8", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @product_page) + + assert_equal "http://www.example.com/manufacturers/apple", polymorphic_url(@manufacturer) + assert_equal "http://www.example.com/manufacturers/apple", Routes.url_helpers.polymorphic_url(@manufacturer) + end + + def test_defining_direct_inside_a_scope_raises_runtime_error + routes = ActionDispatch::Routing::RouteSet.new + + assert_raises RuntimeError do + routes.draw do + namespace :admin do + direct(:rubyonrails) { "http://www.rubyonrails.org" } + end + end + end + end + + def test_defining_resolve_inside_a_scope_raises_runtime_error + routes = ActionDispatch::Routing::RouteSet.new + + assert_raises RuntimeError do + routes.draw do + namespace :admin do + resolve("User") { "/profile" } + end + end + end + end +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 53758a4fbc..64818e6ca1 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4913,3 +4913,113 @@ class TestInternalRoutingParams < ActionDispatch::IntegrationTest ) end end + +class FlashRedirectTest < ActionDispatch::IntegrationTest + SessionKey = "_myapp_session" + Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") + + class KeyGeneratorMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.key_generator"] ||= Generator + @app.call(env) + end + end + + class FooController < ActionController::Base + def bar + render plain: (flash[:foo] || "foo") + end + end + + Routes = ActionDispatch::Routing::RouteSet.new + Routes.draw do + get "/foo", to: redirect { |params, req| req.flash[:foo] = "bar"; "/bar" } + get "/bar", to: "flash_redirect_test/foo#bar" + end + + APP = build_app Routes do |middleware| + middleware.use KeyGeneratorMiddleware + middleware.use ActionDispatch::Session::CookieStore, key: SessionKey + middleware.use ActionDispatch::Flash + middleware.delete ActionDispatch::ShowExceptions + end + + def app + APP + end + + include Routes.url_helpers + + def test_block_redirect_commits_flash + get "/foo", env: { "action_dispatch.key_generator" => Generator } + assert_response :redirect + + follow_redirect! + assert_equal "bar", response.body + end +end + +class TestRecognizePath < ActionDispatch::IntegrationTest + class PageConstraint + attr_reader :key, :pattern + + def initialize(key, pattern) + @key = key + @pattern = pattern + end + + def matches?(request) + request.path_parameters[key] =~ pattern + end + end + + stub_controllers do |routes| + Routes = routes + routes.draw do + get "/hash/:foo", to: "pages#show", constraints: { foo: /foo/ } + get "/hash/:bar", to: "pages#show", constraints: { bar: /bar/ } + + get "/proc/:foo", to: "pages#show", constraints: proc { |r| r.path_parameters[:foo] =~ /foo/ } + get "/proc/:bar", to: "pages#show", constraints: proc { |r| r.path_parameters[:bar] =~ /bar/ } + + get "/class/:foo", to: "pages#show", constraints: PageConstraint.new(:foo, /foo/) + get "/class/:bar", to: "pages#show", constraints: PageConstraint.new(:bar, /bar/) + end + end + + APP = build_app Routes + def app + APP + end + + def test_hash_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/hash/bar") + + assert_equal expected_params, actual_params + end + + def test_proc_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/proc/bar") + + assert_equal expected_params, actual_params + end + + def test_class_constraints_dont_leak_between_routes + expected_params = { controller: "pages", action: "show", bar: "bar" } + actual_params = recognize_path("/class/bar") + + assert_equal expected_params, actual_params + end + + private + + def recognize_path(*args) + Routes.recognize_path(*args) + end +end diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index bd8318f5f6..3082d1072b 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -224,7 +224,7 @@ module StaticTests def assert_gzip(file_name, response) expected = File.read("#{FIXTURE_LOAD_PATH}/#{public_path}" + file_name) - actual = Zlib::GzipReader.new(StringIO.new(response.body)).read + actual = ActiveSupport::Gzip.decompress(response.body) assert_equal expected, actual end diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb new file mode 100644 index 0000000000..814e1d707b --- /dev/null +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -0,0 +1,21 @@ +require "abstract_unit" +require "action_dispatch/system_testing/driver" + +class DriverTest < ActiveSupport::TestCase + test "initializing the driver" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium) + assert_equal :selenium, driver.instance_variable_get(:@name) + end + + test "initializing the driver with a browser" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) + assert_equal :selenium, driver.instance_variable_get(:@name) + assert_equal :chrome, driver.instance_variable_get(:@browser) + assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) + assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) + end + + test "selenium? returns false if driver is poltergeist" do + assert_not ActionDispatch::SystemTesting::Driver.new(:poltergeist).send(:selenium?) + end +end diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb new file mode 100644 index 0000000000..a83818fd80 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -0,0 +1,41 @@ +require "abstract_unit" +require "action_dispatch/system_testing/test_helpers/screenshot_helper" +require "capybara/dsl" + +class ScreenshotHelperTest < ActiveSupport::TestCase + test "image path is saved in tmp directory" do + new_test = DrivenBySeleniumWithChrome.new("x") + + assert_equal "tmp/screenshots/x.png", new_test.send(:image_path) + end + + test "image path includes failures text if test did not pass" do + new_test = DrivenBySeleniumWithChrome.new("x") + + new_test.stub :passed?, false do + assert_equal "tmp/screenshots/failures_x.png", new_test.send(:image_path) + end + end + + test "image path does not include failures text if test skipped" do + new_test = DrivenBySeleniumWithChrome.new("x") + + new_test.stub :passed?, false do + new_test.stub :skipped?, true do + assert_equal "tmp/screenshots/x.png", new_test.send(:image_path) + end + end + end +end + +class RackTestScreenshotsTest < DrivenByRackTest + test "rack_test driver does not support screenshot" do + assert_not self.send(:supports_screenshot?) + end +end + +class SeleniumScreenshotsTest < DrivenBySeleniumWithChrome + test "selenium driver supports screenshot" do + assert self.send(:supports_screenshot?) + end +end diff --git a/actionpack/test/dispatch/system_testing/server_test.rb b/actionpack/test/dispatch/system_testing/server_test.rb new file mode 100644 index 0000000000..10412d6367 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/server_test.rb @@ -0,0 +1,17 @@ +require "abstract_unit" +require "capybara/dsl" +require "action_dispatch/system_testing/server" + +class ServerTest < ActiveSupport::TestCase + setup do + ActionDispatch::SystemTesting::Server.new.run + end + + test "initializing the server port" do + assert_includes Capybara.servers, :rails_puma + end + + test "port is always included" do + assert Capybara.always_include_port, "expected Capybara.always_include_port to be true" + end +end diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb new file mode 100644 index 0000000000..33d98f924f --- /dev/null +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -0,0 +1,21 @@ +require "abstract_unit" + +class SetDriverToRackTestTest < DrivenByRackTest + test "uses rack_test" do + assert_equal :rack_test, Capybara.current_driver + end +end + +class OverrideSeleniumSubclassToRackTestTest < DrivenBySeleniumWithChrome + driven_by :rack_test + + test "uses rack_test" do + assert_equal :rack_test, Capybara.current_driver + end +end + +class SetDriverToSeleniumTest < DrivenBySeleniumWithChrome + test "uses selenium" do + assert_equal :selenium, Capybara.current_driver + end +end |