diff options
Diffstat (limited to 'actionpack')
68 files changed, 1229 insertions, 331 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a5f6426f21..0167dcbf96 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,72 @@ +* Prefer `remove_method` over `undef_method` when reloading routes + + When `undef_method` is used it prevents access to other implementations of that + url helper in the ancestor chain so use `remove_method` instead to restore access. + + *Andrew White* + +* Add the `resolve` method to the routing DSL + + This new method allows customization of the polymorphic mapping of models: + + ``` ruby + resource :basket + resolve(class: "Basket") { [:basket] } + ``` + + ``` erb + <%= form_for @basket do |form| %> + <!-- basket form --> + <% end %> + ``` + + This generates the correct singular URL for the form instead of the default + resources member url, e.g. `/basket` vs. `/basket/:id`. + + Fixes #1769. + + *Andrew White* + +* Add the `direct` method to the routing DSL + + This new method allows creation of custom url helpers, e.g: + + ``` ruby + direct(:apple) { "http://www.apple.com" } + + >> apple_url + => "http://www.apple.com" + ``` + + This has the advantage of being available everywhere url helpers are available + unlike custom url helpers defined in helper modules, etc. + + *Andrew White* + +* Add `ActionDispatch::SystemTestCase` to Action Pack + + Adds Capybara integration directly into Rails through Action Pack! + + See PR [#26703](https://github.com/rails/rails/pull/26703) + + *Eileen M. Uchitelle* + +* 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* @@ -19,7 +88,7 @@ *Tawan Sierek* -* Fixes incorrect output from rails routes when using singular resources. +* Fixes incorrect output from `rails routes` when using singular resources. Fixes #26606. diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 603c2e9ea7..e7cb6347a2 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -1,4 +1,3 @@ -require "erubis" require "abstract_controller/error" require "active_support/configurable" require "active_support/descendants_tracker" @@ -22,7 +21,6 @@ module AbstractController include ActiveSupport::Configurable extend ActiveSupport::DescendantsTracker - undef_method :not_implemented class << self attr_reader :abstract alias_method :abstract?, :abstract 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 9dab7aeef4..337718afc0 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -118,11 +118,6 @@ module ActionController class Metal < AbstractController::Base abstract! - def env - @_request.env - end - deprecate :env - # Returns the last part of the controller's name, underscored, without the ending # <tt>Controller</tt>. For instance, PostsController returns <tt>posts</tt>. # Namespaces are left out, so Admin::PostsController returns <tt>posts</tt> as well. @@ -232,14 +227,6 @@ module ActionController middleware_stack end - # Makes the controller a Rack endpoint that runs the action in the given - # +env+'s +action_dispatch.request.path_parameters+ key. - def self.call(env) - req = ActionDispatch::Request.new env - action(req.path_parameters[:action]).call(env) - end - class << self; deprecate :call; end - # Returns a Rack endpoint for the given action name. def self.action(name) if middleware_stack.any? diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 4dff23dd85..0c50894bce 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -37,7 +37,7 @@ module ActionController if include_content?(response_code) self.content_type = content_type || (Mime[formats.first] if formats) - self.response.charset = false + response.charset = false end true diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 8615c16c6f..dde924e682 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,5 +1,3 @@ -require "active_support/core_ext/string/strip" - module ActionController # Handles implicit rendering for a controller action that does not # explicitly respond with +render+, +respond_to+, +redirect+, or +head+. diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 30798c1d99..a349841082 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -50,13 +50,13 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # - def redirect_to(options = {}, response_status = {}) #:doc: + 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 4c01891d4c..6b17719381 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -114,7 +114,7 @@ module ActionController self.status = status if status self.content_type = content_type if content_type - self.headers["Location"] = url_for(location) if location + headers["Location"] = url_for(location) if location super end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index acfeca1fcb..d304dcf468 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -150,7 +150,7 @@ module ActionController # permitted flag. def ==(other) if other.respond_to?(:permitted?) - self.permitted? == other.permitted? && self.parameters == other.parameters + permitted? == other.permitted? && parameters == other.parameters else @parameters == other end @@ -398,8 +398,7 @@ module ActionController unpermitted_parameters!(params) if self.class.action_on_unpermitted_parameters - params.permitted = true - params + params.permit! end # Returns a parameter for the given +key+. If not found, @@ -818,7 +817,6 @@ module ActionController # Filter this one out. end end - sanitized.permitted = true end end diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index 3ff80e6a39..acb400cd15 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -60,7 +60,8 @@ module ActionController end # Accepts a custom Rack environment to render templates in. - # It will be merged with ActionController::Renderer.defaults + # It will be merged with the default Rack environment defined by + # +ActionController::Renderer::DEFAULTS+. def initialize(controller, env, defaults) @controller = controller @defaults = defaults diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 441667e556..7229c67f30 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -354,7 +354,7 @@ module ActionController end def controller_class - if current_controller_class = self._controller_class + if current_controller_class = _controller_class current_controller_class else self.controller_class = determine_default_controller_class(name) @@ -534,6 +534,7 @@ 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_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 6b718e3682..1583a8f87f 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -1,7 +1,6 @@ # -*- frozen-string-literal: true -*- require "singleton" -require "active_support/core_ext/module/attribute_accessors" require "active_support/core_ext/string/starts_ends_with" module Mime @@ -298,7 +297,7 @@ module Mime end def respond_to_missing?(method, include_private = false) - method.to_s.ends_with? "?" + (method.to_s.ends_with? "?") || super end end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index ad4aadacf5..8f21eca440 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -22,6 +22,7 @@ module ActionDispatch included do class << self + # Returns the parameter parsers. attr_reader :parameter_parsers end @@ -29,7 +30,16 @@ module ActionDispatch end module ClassMethods - def parameter_parsers=(parsers) # :nodoc: + # Configure the parameter parser for a given 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 + # xml_parser = -> (raw_post) { Hash.from_xml(raw_post) || {} } + # new_parsers = original_parsers.merge(xml: xml_parser) + # ActionDispatch::Request.parameter_parsers = new_parsers + def parameter_parsers=(parsers) @parameter_parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } end end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 516a2af69a..dc159596c4 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -425,7 +425,7 @@ module ActionDispatch # :nodoc: def set_content_type(content_type, charset) type = (content_type || "").dup - type << "; charset=#{charset}" if charset + type << "; charset=#{charset.to_s.downcase}" if charset set_header CONTENT_TYPE, type end diff --git a/actionpack/lib/action_dispatch/journey/parser.rb b/actionpack/lib/action_dispatch/journey/parser.rb index db42b64c4b..e002755bcf 100644 --- a/actionpack/lib/action_dispatch/journey/parser.rb +++ b/actionpack/lib/action_dispatch/journey/parser.rb @@ -1,7 +1,7 @@ # # DO NOT MODIFY!!!! # This file is automatically generated by Racc 1.4.14 -# from Racc grammer file "". +# from Racc grammar file "". # require 'racc/parser.rb' diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 0902b9233e..cf0108ec32 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -31,6 +31,13 @@ module ActionDispatch Visitors::FormatBuilder.new.accept(spec) end + def eager_load! + required_names + offsets + to_regexp + nil + end + def ast @spec.find_all(&:symbol?).each do |node| re = @requirements[node.to_sym] diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index f2ac4818d8..927fd369c4 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -73,6 +73,14 @@ module ActionDispatch @internal = internal end + def eager_load! + path.eager_load! + ast + parts + required_defaults + nil + end + def ast @decorated_ast ||= begin decorated_ast = path.ast diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 084ae9325e..d55e1399e4 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -22,6 +22,13 @@ module ActionDispatch @routes = routes end + def eager_load! + # Eagerly trigger the simulator's initialization so + # it doesn't happen during a request cycle. + simulator + nil + end + def serve(req) find_routes(req).each do |match, parameters, route| set_params = req.path_parameters diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index cda859cba4..1c50192867 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -5,7 +5,7 @@ module ActionDispatch ESCAPE_PATH = ->(value) { Router::Utils.escape_path(value) } ESCAPE_SEGMENT = ->(value) { Router::Utils.escape_segment(value) } - class Parameter < Struct.new(:name, :escaper) + Parameter = Struct.new(:name, :escaper) do def escape(value); escaper.call value; end end diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb index fef246532b..ff129cf96a 100644 --- a/actionpack/lib/action_dispatch/middleware/callbacks.rb +++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb @@ -1,4 +1,3 @@ - module ActionDispatch # Provides callbacks to be executed before and after dispatching the request. class Callbacks @@ -7,17 +6,6 @@ module ActionDispatch define_callbacks :call class << self - def to_prepare(*args, &block) - ActiveSupport::Reloader.to_prepare(*args, &block) - end - - def to_cleanup(*args, &block) - ActiveSupport::Reloader.to_complete(*args, &block) - end - - deprecate to_prepare: "use ActiveSupport::Reloader.to_prepare instead", - to_cleanup: "use ActiveSupport::Reloader.to_complete instead" - def before(*args, &block) set_callback(:call, :before, *args, &block) end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 956c53e813..c61cb3fd68 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -572,7 +572,7 @@ module ActionDispatch super if ActiveSupport::LegacyKeyGenerator === key_generator - raise "You didn't set secrets.secret_key_base, which is required for this cookie jar. " + + raise "You didn't set secrets.secret_key_base, which is required for this cookie jar. " \ "Read the upgrade documentation to learn more about this new config option." end diff --git a/actionpack/lib/action_dispatch/middleware/reloader.rb b/actionpack/lib/action_dispatch/middleware/reloader.rb index 90c64037aa..6d64b1424b 100644 --- a/actionpack/lib/action_dispatch/middleware/reloader.rb +++ b/actionpack/lib/action_dispatch/middleware/reloader.rb @@ -1,54 +1,10 @@ module ActionDispatch - # ActionDispatch::Reloader provides prepare and cleanup callbacks, - # intended to assist with code reloading during development. - # - # Prepare callbacks are run before each request, and cleanup callbacks - # after each request. In this respect they are analogs of ActionDispatch::Callback's - # before and after callbacks. However, cleanup callbacks are not called until the - # request is fully complete -- that is, after #close has been called on - # the response body. This is important for streaming responses such as the - # following: - # - # self.response_body = -> (response, output) do - # # code here which refers to application models - # end - # - # Cleanup callbacks will not be called until after the response_body lambda - # is evaluated, ensuring that it can refer to application models and other - # classes before they are unloaded. + # ActionDispatch::Reloader wraps the request with callbacks provided by ActiveSupport::Reloader + # callbacks, intended to assist with code reloading during development. # # By default, ActionDispatch::Reloader is included in the middleware stack # only in the development environment; specifically, when +config.cache_classes+ - # is false. Callbacks may be registered even when it is not included in the - # middleware stack, but are executed only when <tt>ActionDispatch::Reloader.prepare!</tt> - # or <tt>ActionDispatch::Reloader.cleanup!</tt> are called manually. - # + # is false. class Reloader < Executor - def self.to_prepare(*args, &block) - ActiveSupport::Reloader.to_prepare(*args, &block) - end - - def self.to_cleanup(*args, &block) - ActiveSupport::Reloader.to_complete(*args, &block) - end - - def self.prepare! - default_reloader.prepare! - end - - def self.cleanup! - default_reloader.reload! - end - - class << self - attr_accessor :default_reloader # :nodoc: - - deprecate to_prepare: "use ActiveSupport::Reloader.to_prepare instead", - to_cleanup: "use ActiveSupport::Reloader.to_complete instead", - prepare!: "use Rails.application.reloader.prepare! instead", - cleanup!: "use Rails.application.reloader.reload! instead of cleanup + prepare" - end - - self.default_reloader = ActiveSupport::Reloader end end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 9f1ae80b97..8bae5bfeff 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -131,8 +131,8 @@ module ActionDispatch should_check_ip = @check_ip && client_ips.last && forwarded_ips.last if should_check_ip && !forwarded_ips.include?(client_ips.last) # We don't know which came from the proxy, and which from the user - raise IpSpoofAttackError, "IP spoofing attack?! " + - "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " + + raise IpSpoofAttackError, "IP spoofing attack?! " \ + "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ "HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" end diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 97c937b0b1..d9f018c8ac 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -8,8 +8,8 @@ module ActionDispatch module Session class SessionRestoreError < StandardError #:nodoc: def initialize - super("Session contains objects whose class definition isn't available.\n" + - "Remember to require the classes for all objects kept in the session.\n" + + super("Session contains objects whose class definition isn't available.\n" \ + "Remember to require the classes for all objects kept in the session.\n" \ "(Original exception: #{$!.message} [#{$!.class}])\n") set_backtrace $!.backtrace end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 5c71f0fc48..5d10129d21 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -33,7 +33,7 @@ module ActionDispatch paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] if match = paths.detect { |p| - path = File.join(@root, p.force_encoding("UTF-8".freeze)) + path = File.join(@root, p.force_encoding(Encoding::UTF_8)) begin File.file?(path) && File.readable?(path) rescue SystemCallError diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 48cc91bbfa..16a18a7f25 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -39,8 +39,6 @@ module ActionDispatch config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil? ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie - ActionDispatch::Reloader.default_reloader = app.reloader - ActionDispatch.test_app = app end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 089aa9f78e..073dabd0a8 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1,6 +1,7 @@ require "active_support/core_ext/hash/slice" require "active_support/core_ext/enumerable" require "active_support/core_ext/array/extract_options" +require "active_support/core_ext/regexp" require "action_dispatch/routing/redirection" require "action_dispatch/routing/endpoint" @@ -238,7 +239,7 @@ module ActionDispatch options[:controller] ||= /.+?/ end - if to.respond_to? :call + if to.respond_to?(:action) || to.respond_to?(:call) options else to_endpoint = split_to to @@ -290,16 +291,14 @@ module ActionDispatch end def app(blocks) - if to.is_a?(Class) && to < ActionController::Metal + if to.respond_to?(:action) Routing::RouteSet::StaticDispatcher.new to + elsif to.respond_to?(:call) + Constraints.new(to, blocks, Constraints::CALL) + elsif blocks.any? + Constraints.new(dispatcher(defaults.key?(:controller)), blocks, Constraints::SERVE) else - if to.respond_to?(:call) - Constraints.new(to, blocks, Constraints::CALL) - elsif blocks.any? - Constraints.new(dispatcher(defaults.key?(:controller)), blocks, Constraints::SERVE) - else - dispatcher(defaults.key?(:controller)) - end + dispatcher(defaults.key?(:controller)) end end @@ -2021,6 +2020,111 @@ 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)) ] + # end + # + # 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: + # + # direct class: "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, @@ -2041,6 +2145,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 @@ -2114,6 +2226,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..984ded1ff5 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -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]) + 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], only_path: 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], only_path: 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 4e2318a45e..dabc045007 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -137,6 +137,9 @@ module ActionDispatch # # get "/stories" => redirect("/posts") # + # This will redirect the user, while ignoring certain parts of the request, including query string, etc. + # `/stories`, `/stories?foo=bar`, etc all redirect to `/posts`. + # # You can also use interpolation in the supplied redirect argument: # # get 'docs/:article', to: redirect('/wiki/%{article}') @@ -165,6 +168,11 @@ module ActionDispatch # # get 'stores/:name', to: redirect(subdomain: 'stores', path: '/%{name}') # get 'stores/:name(*all)', to: redirect(subdomain: 'stores', path: '/%{name}%{all}') + # get '/stories', to: redirect(path: '/posts') + # + # This will redirect the user, while changing only the specified parts of the request, + # for example the `path` option in the last example. + # `/stories`, `/stories?foo=bar`, redirect to `/posts` and `/posts?foo=bar` respectively. # # Finally, an object which responds to call can be supplied to redirect, allowing you to reuse # common redirect routes. The call method must accept two arguments, params and request, and return diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5853adb110..2672cd24ed 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, only_path: true) + end + end + + @url_helpers_module.module_eval do + define_method(:"#{name}_url") do |*args| + helper.call(self, args) + end + end + end + class UrlHelper def self.create(route, options, route_name, url_strategy) if optimize_helper?(route) @@ -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,13 @@ module ActionDispatch @set = Journey::Routes.new @router = Journey::Router.new @set @formatter = Journey::Formatter.new self + @polymorphic_mappings = {} + end + + def eager_load! + router.eager_load! + routes.each(&:eager_load!) + nil end def relative_url_root @@ -402,6 +441,7 @@ module ActionDispatch named_routes.clear set.clear formatter.clear + @polymorphic_mappings.clear @prepend.each { |blk| eval_block(blk) } end @@ -446,17 +486,42 @@ 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 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 @@ -517,20 +582,70 @@ 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, outer_options = {}) + options = args.extract_options! + url_options = eval_block(t, args, options) + + case url_options + when String + t.url_for(url_options) + when Hash + t.url_for(url_options.merge(outer_options)) + when ActionController::Parameters + if url_options.permitted? + t.url_for(url_options.to_h.merge(outer_options)) + else + raise ArgumentError, "Generating a URL from non sanitized request parameters is insecure!" + end + when Array + opts = url_options.extract_options! + t.url_for(url_options.push(opts.merge(outer_options))) + else + t.url_for([url_options, outer_options]) + 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 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..99c2be0a35 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -0,0 +1,119 @@ +require "capybara/dsl" +require "action_controller" +require "action_dispatch/system_testing/driver" +require "action_dispatch/system_testing/server" +require "action_dispatch/system_testing/browser" +require "action_dispatch/system_testing/test_helpers/screenshot_helper" +require "action_dispatch/system_testing/test_helpers/setup_and_teardown" + +module ActionDispatch + class SystemTestCase < IntegrationTest + # = 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 are 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. + include Capybara::DSL + include SystemTesting::TestHelpers::SetupAndTeardown + include SystemTesting::TestHelpers::ScreenshotHelper + + def self.start_application # :nodoc: + Capybara.app = Rack::Builder.new do + map "/" do + run Rails.application + end + end + end + + # 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]) + SystemTesting::Driver.new(driver).run + SystemTesting::Server.new.run + SystemTesting::Browser.new(using, screen_size).run if selenium?(driver) + end + + def self.selenium?(driver) # :nodoc: + driver == :selenium + end + end + + SystemTestCase.start_application +end diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb new file mode 100644 index 0000000000..c9a6628516 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -0,0 +1,28 @@ +module ActionDispatch + module SystemTesting + class Browser # :nodoc: + def initialize(name, screen_size) + @name = name + @screen_size = screen_size + end + + def run + register + setup + end + + private + def register + Capybara.register_driver @name do |app| + Capybara::Selenium::Driver.new(app, browser: @name).tap do |driver| + driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) + end + end + end + + def setup + Capybara.default_driver = @name.to_sym + end + end + end +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..7c2ad84e19 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -0,0 +1,18 @@ +module ActionDispatch + module SystemTesting + class Driver # :nodoc: + def initialize(name) + @name = name + end + + def run + register + end + + private + def register + Capybara.default_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..784005cb93 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -0,0 +1,57 @@ +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. + def take_screenshot + save_image + puts "[Screenshot]: #{image_path}" + 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 unless passed? + end + + private + def image_name + passed? ? method_name : "failures_#{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 display_image + if ENV["CAPYBARA_INLINE_SCREENSHOT"] == "artifact" + "\e]1338;url=artifact://#{image_path}\a" + else + name = inline_base64(File.basename(image_path)) + image = inline_base64(File.read(image_path)) + "\e]1337;File=name=#{name};height=400px;inline=1:#{image}\a" + end + end + + def inline_base64(path) + Base64.encode64(path).gsub("\n", "") + 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..491559eedf --- /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 + super + take_failed_screenshot + Capybara.reset_sessions! + end + end + end + end +end diff --git a/actionpack/lib/action_dispatch/testing/assertions.rb b/actionpack/lib/action_dispatch/testing/assertions.rb index b362931ef7..4ea18d671d 100644 --- a/actionpack/lib/action_dispatch/testing/assertions.rb +++ b/actionpack/lib/action_dispatch/testing/assertions.rb @@ -12,7 +12,7 @@ module ActionDispatch include Rails::Dom::Testing::Assertions def html_document - @html_document ||= if @response.content_type.to_s =~ /xml\z/ + @html_document ||= if @response.content_type.to_s.end_with?("xml") Nokogiri::XML::Document.parse(@response.body) else Nokogiri::HTML::Document.parse(@response.body) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 021ffec862..5fa0b727ab 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -2,7 +2,6 @@ require "stringio" require "uri" require "active_support/core_ext/kernel/singleton_class" require "active_support/core_ext/object/try" -require "active_support/core_ext/string/strip" require "rack/test" require "minitest" @@ -145,8 +144,8 @@ module ActionDispatch self.host = DEFAULT_HOST self.remote_addr = "127.0.0.1" - self.accept = "text/xml,application/xml,application/xhtml+xml," + - "text/html;q=0.9,text/plain;q=0.8,image/png," + + self.accept = "text/xml,application/xml,application/xhtml+xml," \ + "text/html;q=0.9,text/plain;q=0.8,image/png," \ "*/*;q=0.5" unless defined? @named_routes_configured diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 11a9092527..459b0d6c54 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -7,8 +7,8 @@ require "active_support/core_ext/kernel/reporting" # These are the normal settings that will be set up by Railties # TODO: Have these tests support other combinations of these values silence_warnings do - Encoding.default_internal = "UTF-8" - Encoding.default_external = "UTF-8" + Encoding.default_internal = Encoding::UTF_8 + Encoding.default_external = Encoding::UTF_8 end require "drb" @@ -259,9 +259,9 @@ module ActionDispatch host = uri_or_host.host unless path path ||= uri_or_host.path - params = { "PATH_INFO" => path, - "REQUEST_METHOD" => method, - "HTTP_HOST" => host } + params = { "PATH_INFO" => path, + "REQUEST_METHOD" => method, + "HTTP_HOST" => host } routes.call(params) end @@ -354,15 +354,6 @@ class CommentsController < ResourcesController; end class AccountsController < ResourcesController; end class ImagesController < ResourcesController; end -# Skips the current run on Rubinius using Minitest::Assertions#skip -def rubinius_skip(message = "") - skip message if RUBY_ENGINE == "rbx" -end -# Skips the current run on JRuby using Minitest::Assertions#skip -def jruby_skip(message = "") - skip message if defined?(JRUBY_VERSION) -end - require "active_support/testing/method_call_assertions" class ForkingExecutor @@ -438,4 +429,13 @@ end class ActiveSupport::TestCase include ActiveSupport::Testing::MethodCallAssertions + + # Skips the current run on Rubinius using Minitest::Assertions#skip + private def rubinius_skip(message = "") + skip message if RUBY_ENGINE == "rbx" + end + # Skips the current run on JRuby using Minitest::Assertions#skip + private def jruby_skip(message = "") + skip message if defined?(JRUBY_VERSION) + end end diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index f9701585a9..5f1463cfa8 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -584,7 +584,7 @@ class FilterTest < ActionController::TestCase assert @controller.instance_variable_get(:@was_audited) end - def test_running_anomolous_yet_valid_condition_actions + def test_running_anomalous_yet_valid_condition_actions test_process(AnomolousYetValidConditionController) assert_equal %w( ensure_login ), @controller.instance_variable_get(:@ran_filter) assert @controller.instance_variable_get(:@ran_class_action) @@ -704,7 +704,7 @@ class FilterTest < ActionController::TestCase def test_prepending_and_appending_around_action test_process(MixedFilterController) - assert_equal " before aroundfilter before procfilter before appended aroundfilter " + + assert_equal " before aroundfilter before procfilter before appended aroundfilter " \ " after appended aroundfilter after procfilter after aroundfilter ", MixedFilterController.execution_log end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 4fae4071b1..57f58fd835 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -269,8 +269,8 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest test "response cookies are added to the cookie jar for the next request" do with_test_route_set do - self.cookies["cookie_1"] = "sugar" - self.cookies["cookie_2"] = "oatmeal" + cookies["cookie_1"] = "sugar" + cookies["cookie_2"] = "oatmeal" get "/cookie_monster" assert_equal "cookie_1=; path=/\ncookie_3=chocolate; path=/", headers["Set-Cookie"] assert_equal({ "cookie_1" => "", "cookie_2" => "oatmeal", "cookie_3" => "chocolate" }, cookies.to_hash) @@ -494,7 +494,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest assert_includes @response.headers, "c" end - def test_accept_not_overriden_when_xhr_true + def test_accept_not_overridden_when_xhr_true with_test_route_set do get "/get", headers: { "Accept" => "application/json" }, xhr: true assert_equal "application/json", request.accept 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/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 818dc119eb..61bd5c80c4 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -519,7 +519,7 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "Whatever you ask for, I got it", @response.body end - def test_handle_any_any_unkown_format + def test_handle_any_any_unknown_format get :handle_any_any, format: "php" assert_equal "Whatever you ask for, I got it", @response.body end diff --git a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb index 5f86901e30..00e591d5a7 100644 --- a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb @@ -140,6 +140,11 @@ class NestedParametersPermitTest < ActiveSupport::TestCase assert_equal "William Shakespeare", permitted[:book][:authors_attributes]["0"][:name] assert_equal "Unattributed Assistant", permitted[:book][:authors_attributes]["1"][:name] + assert_equal( + { "book" => { "authors_attributes" => { "0" => { "name" => "William Shakespeare" }, "1" => { "name" => "Unattributed Assistant" }, "2" => {} } } }, + permitted.to_h + ) + assert_filtered_out permitted[:book][:authors_attributes]["0"], :age_of_death end diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index b62a3d6d7b..8920914af1 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -66,12 +66,20 @@ class ParametersPermitTest < ActiveSupport::TestCase values.each do |value| params = ActionController::Parameters.new(id: value) permitted = params.permit(:id) - assert_equal value, permitted[:id] + if value.nil? + assert_nil permitted[:id] + else + assert_equal value, permitted[:id] + end @struct_fields.each do |sf| params = ActionController::Parameters.new(sf => value) permitted = params.permit(:sf) - assert_equal value, permitted[sf] + if value.nil? + assert_nil permitted[sf] + else + assert_equal value, permitted[sf] + end end end end @@ -187,11 +195,6 @@ class ParametersPermitTest < ActiveSupport::TestCase permitted = params.permit(:username, preferences: {}, hacked: {}) - assert permitted.permitted? - assert permitted[:preferences].permitted? - assert permitted[:preferences][:font].permitted? - assert permitted[:preferences][:dubious].all?(&:permitted?) - assert_equal "fxn", permitted[:username] assert_equal "Marazul", permitted[:preferences][:scheme] assert_equal "Source Code Pro", permitted[:preferences][:font][:name] 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/render_test.rb b/actionpack/test/controller/render_test.rb index 0c0f18f200..3a0a0a8bde 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -313,7 +313,6 @@ class ExpiresInRenderTest < ActionController::TestCase end def test_permitted_dynamic_render_file_hash - skip "FIXME: this test passes on 4-2-stable but not master. Why?" assert File.exist?(File.join(File.dirname(__FILE__), "../../test/abstract_unit.rb")) response = get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } assert_equal File.read(File.join(File.dirname(__FILE__), "../../test/abstract_unit.rb")), diff --git a/actionpack/test/controller/request/test_request_test.rb b/actionpack/test/controller/request/test_request_test.rb index b67ae72c0a..1440db00f6 100644 --- a/actionpack/test/controller/request/test_request_test.rb +++ b/actionpack/test/controller/request/test_request_test.rb @@ -21,16 +21,20 @@ class ActionController::TestRequestTest < ActionController::TestCase @request.get_header("CONTENT_LENGTH")) end - ActionDispatch::Session::AbstractStore::DEFAULT_OPTIONS.each_key do |option| - test "rack default session options #{option} exists in session options and is default" do - assert_equal(ActionDispatch::Session::AbstractStore::DEFAULT_OPTIONS[option], - @request.session_options[option], - "Missing rack session default option #{option} in request.session_options") + ActionDispatch::Session::AbstractStore::DEFAULT_OPTIONS.each_pair do |key, value| + test "rack default session options #{key} exists in session options and is default" do + if value.nil? + assert_nil(@request.session_options[key], + "Missing rack session default option #{key} in request.session_options") + else + assert_equal(value, @request.session_options[key], + "Missing rack session default option #{key} in request.session_options") + end end - test "rack default session options #{option} exists in session options" do - assert(@request.session_options.has_key?(option), - "Missing rack session option #{option} in request.session_options") + test "rack default session options #{key} exists in session options" do + assert(@request.session_options.has_key?(key), + "Missing rack session option #{key} in request.session_options") end end end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 874f9c3c42..891ce0e905 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 @@ -728,6 +728,20 @@ 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 @@ -796,7 +810,6 @@ XML new_content_type = "new content_type" file.content_type = new_content_type assert_equal new_content_type, file.content_type - end def test_fixture_path_is_accessed_from_self_instead_of_active_support_test_case @@ -880,12 +893,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/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 382b4e273d..862dcf01c3 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -26,7 +26,7 @@ module AbstractController action: :index } }.url_helpers - self.default_url_options[:host] = "example.com" + default_url_options[:host] = "example.com" } path = klass.new.fun_path(controller: :articles, diff --git a/actionpack/test/dispatch/callbacks_test.rb b/actionpack/test/dispatch/callbacks_test.rb index 57e21a22c6..29a5dfc0ad 100644 --- a/actionpack/test/dispatch/callbacks_test.rb +++ b/actionpack/test/dispatch/callbacks_test.rb @@ -35,24 +35,6 @@ class DispatcherTest < ActiveSupport::TestCase assert_equal 6, Foo.b end - def test_to_prepare_and_cleanup_delegation - prepared = cleaned = false - assert_deprecated do - ActionDispatch::Callbacks.to_prepare { prepared = true } - ActionDispatch::Callbacks.to_prepare { cleaned = true } - end - - assert_deprecated do - ActionDispatch::Reloader.prepare! - end - assert prepared - - assert_deprecated do - ActionDispatch::Reloader.cleanup! - end - assert cleaned - end - private def dispatch(&block) diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index bb2fc53add..0e093d2188 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -22,8 +22,6 @@ module TestGenerationPrefix end class WithMountedEngine < ActionDispatch::IntegrationTest - include Rack::Test::Methods - class BlogEngine < Rails::Engine routes.draw do get "/posts/:id", to: "inside_engine_generating#show", as: :post @@ -153,114 +151,114 @@ module TestGenerationPrefix # Inside Engine test "[ENGINE] generating engine's url use SCRIPT_NAME from request" do get "/pure-awesomeness/blog/posts/1" - assert_equal "/pure-awesomeness/blog/posts/1", last_response.body + assert_equal "/pure-awesomeness/blog/posts/1", response.body end test "[ENGINE] generating application's url never uses SCRIPT_NAME from request" do get "/pure-awesomeness/blog/url_to_application" - assert_equal "/generate", last_response.body + assert_equal "/generate", response.body end test "[ENGINE] generating engine's url with polymorphic path" do get "/pure-awesomeness/blog/polymorphic_path_for_engine" - assert_equal "/pure-awesomeness/blog/posts/1", last_response.body + assert_equal "/pure-awesomeness/blog/posts/1", response.body end test "[ENGINE] url_helpers from engine have higher priority than application's url_helpers" do get "/awesome/blog/conflicting_url" - assert_equal "engine", last_response.body + assert_equal "engine", response.body end test "[ENGINE] relative path root uses SCRIPT_NAME from request" do get "/awesome/blog/relative_path_root" - verify_redirect "http://example.org/awesome/blog" + verify_redirect "http://www.example.com/awesome/blog" end test "[ENGINE] relative path redirect uses SCRIPT_NAME from request" do get "/awesome/blog/relative_path_redirect" - verify_redirect "http://example.org/awesome/blog/foo" + verify_redirect "http://www.example.com/awesome/blog/foo" end test "[ENGINE] relative option root uses SCRIPT_NAME from request" do get "/awesome/blog/relative_option_root" - verify_redirect "http://example.org/awesome/blog" + verify_redirect "http://www.example.com/awesome/blog" end test "[ENGINE] relative option redirect uses SCRIPT_NAME from request" do get "/awesome/blog/relative_option_redirect" - verify_redirect "http://example.org/awesome/blog/foo" + verify_redirect "http://www.example.com/awesome/blog/foo" end test "[ENGINE] relative custom root uses SCRIPT_NAME from request" do get "/awesome/blog/relative_custom_root" - verify_redirect "http://example.org/awesome/blog" + verify_redirect "http://www.example.com/awesome/blog" end test "[ENGINE] relative custom redirect uses SCRIPT_NAME from request" do get "/awesome/blog/relative_custom_redirect" - verify_redirect "http://example.org/awesome/blog/foo" + verify_redirect "http://www.example.com/awesome/blog/foo" end test "[ENGINE] absolute path root doesn't use SCRIPT_NAME from request" do get "/awesome/blog/absolute_path_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] absolute path redirect doesn't use SCRIPT_NAME from request" do get "/awesome/blog/absolute_path_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] absolute option root doesn't use SCRIPT_NAME from request" do get "/awesome/blog/absolute_option_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] absolute option redirect doesn't use SCRIPT_NAME from request" do get "/awesome/blog/absolute_option_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] absolute custom root doesn't use SCRIPT_NAME from request" do get "/awesome/blog/absolute_custom_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] absolute custom redirect doesn't use SCRIPT_NAME from request" do get "/awesome/blog/absolute_custom_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end # Inside Application test "[APP] generating engine's route includes prefix" do get "/generate" - assert_equal "/awesome/blog/posts/1", last_response.body + assert_equal "/awesome/blog/posts/1", response.body end test "[APP] generating engine's route includes default_url_options[:script_name]" do RailsApplication.routes.default_url_options = { script_name: "/something" } get "/generate" - assert_equal "/something/awesome/blog/posts/1", last_response.body + assert_equal "/something/awesome/blog/posts/1", response.body end test "[APP] generating engine's url with polymorphic path" do get "/polymorphic_path_for_engine" - assert_equal "/awesome/blog/posts/1", last_response.body + assert_equal "/awesome/blog/posts/1", response.body end test "polymorphic_path_for_app" do get "/polymorphic_path_for_app" - assert_equal "/posts/1", last_response.body + assert_equal "/posts/1", response.body end test "[APP] generating engine's url with url_for(@post)" do get "/polymorphic_with_url_for" - assert_equal "http://example.org/awesome/blog/posts/1", last_response.body + assert_equal "http://www.example.com/awesome/blog/posts/1", response.body end test "[APP] instance variable with same name as engine" do get "/ivar_usage" - assert_equal "/awesome/blog/posts/1", last_response.body + assert_equal "/awesome/blog/posts/1", response.body end # Inside any Object @@ -322,9 +320,9 @@ module TestGenerationPrefix private def verify_redirect(url, status = 301) - assert_equal status, last_response.status - assert_equal url, last_response.headers["Location"] - assert_equal expected_redirect_body(url), last_response.body + assert_equal status, response.status + assert_equal url, response.headers["Location"] + assert_equal expected_redirect_body(url), response.body end def expected_redirect_body(url) @@ -333,8 +331,6 @@ module TestGenerationPrefix end class EngineMountedAtRoot < ActionDispatch::IntegrationTest - include Rack::Test::Methods - class BlogEngine def self.routes @routes ||= begin @@ -388,74 +384,74 @@ module TestGenerationPrefix test "generating path inside engine" do get "/posts/1" - assert_equal "/posts/1", last_response.body + assert_equal "/posts/1", response.body end test "[ENGINE] relative path root uses SCRIPT_NAME from request" do get "/relative_path_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] relative path redirect uses SCRIPT_NAME from request" do get "/relative_path_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] relative option root uses SCRIPT_NAME from request" do get "/relative_option_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] relative option redirect uses SCRIPT_NAME from request" do get "/relative_option_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] relative custom root uses SCRIPT_NAME from request" do get "/relative_custom_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] relative custom redirect uses SCRIPT_NAME from request" do get "/relative_custom_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] absolute path root doesn't use SCRIPT_NAME from request" do get "/absolute_path_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] absolute path redirect doesn't use SCRIPT_NAME from request" do get "/absolute_path_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] absolute option root doesn't use SCRIPT_NAME from request" do get "/absolute_option_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] absolute option redirect doesn't use SCRIPT_NAME from request" do get "/absolute_option_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end test "[ENGINE] absolute custom root doesn't use SCRIPT_NAME from request" do get "/absolute_custom_root" - verify_redirect "http://example.org/" + verify_redirect "http://www.example.com/" end test "[ENGINE] absolute custom redirect doesn't use SCRIPT_NAME from request" do get "/absolute_custom_redirect" - verify_redirect "http://example.org/foo" + verify_redirect "http://www.example.com/foo" end private def verify_redirect(url, status = 301) - assert_equal status, last_response.status - assert_equal url, last_response.headers["Location"] - assert_equal expected_redirect_body(url), last_response.body + assert_equal status, response.status + assert_equal url, response.headers["Location"] + assert_equal expected_redirect_body(url), response.body end def expected_redirect_body(url) diff --git a/actionpack/test/dispatch/reloader_test.rb b/actionpack/test/dispatch/reloader_test.rb index db68549b84..9eb78fe059 100644 --- a/actionpack/test/dispatch/reloader_test.rb +++ b/actionpack/test/dispatch/reloader_test.rb @@ -1,32 +1,11 @@ require "abstract_unit" class ReloaderTest < ActiveSupport::TestCase - Reloader = ActionDispatch::Reloader - teardown do ActiveSupport::Reloader.reset_callbacks :prepare ActiveSupport::Reloader.reset_callbacks :complete end - def test_prepare_callbacks - a = b = c = nil - assert_deprecated do - Reloader.to_prepare { |*args| a = b = c = 1 } - Reloader.to_prepare { |*args| b = c = 2 } - Reloader.to_prepare { |*args| c = 3 } - end - - # Ensure to_prepare callbacks are not run when defined - assert_nil a || b || c - - # Run callbacks - call_and_return_body - - assert_equal 1, a - assert_equal 2, b - assert_equal 3, c - end - class MyBody < Array def initialize(&block) @on_close = block @@ -45,6 +24,23 @@ class ReloaderTest < ActiveSupport::TestCase end end + def test_prepare_callbacks + a = b = c = nil + reloader.to_prepare { |*args| a = b = c = 1 } + reloader.to_prepare { |*args| b = c = 2 } + reloader.to_prepare { |*args| c = 3 } + + # Ensure to_prepare callbacks are not run when defined + assert_nil a || b || c + + # Run callbacks + call_and_return_body + + assert_equal 1, a + assert_equal 2, b + assert_equal 3, c + end + def test_returned_body_object_always_responds_to_close body = call_and_return_body assert_respond_to body, :close @@ -62,15 +58,12 @@ class ReloaderTest < ActiveSupport::TestCase def test_condition_specifies_when_to_reload i, j = 0, 0, 0, 0 - assert_deprecated do - Reloader.to_prepare { |*args| i += 1 } - Reloader.to_cleanup { |*args| j += 1 } - end - x = Class.new(ActiveSupport::Reloader) - x.check = lambda { i < 3 } + reloader = reloader(lambda { i < 3 }) + reloader.to_prepare { |*args| i += 1 } + reloader.to_complete { |*args| j += 1 } - app = Reloader.new(lambda { |env| [200, {}, []] }, x) + app = middleware(lambda { |env| [200, {}, []] }, reloader) 5.times do resp = app.call({}) resp[2].close @@ -115,24 +108,20 @@ class ReloaderTest < ActiveSupport::TestCase assert_respond_to body, :bar end - def test_cleanup_callbacks_are_called_when_body_is_closed - cleaned = false - assert_deprecated do - Reloader.to_cleanup { cleaned = true } - end + def test_complete_callbacks_are_called_when_body_is_closed + completed = false + reloader.to_complete { completed = true } body = call_and_return_body - assert !cleaned + assert !completed body.close - assert cleaned + assert completed end def test_prepare_callbacks_arent_called_when_body_is_closed prepared = false - assert_deprecated do - Reloader.to_prepare { prepared = true } - end + reloader.to_prepare { prepared = true } body = call_and_return_body prepared = false @@ -141,45 +130,9 @@ class ReloaderTest < ActiveSupport::TestCase assert !prepared end - def test_manual_reloading - prepared = cleaned = false - assert_deprecated do - Reloader.to_prepare { prepared = true } - Reloader.to_cleanup { cleaned = true } - end - - assert_deprecated do - Reloader.prepare! - end - assert prepared - assert !cleaned - - prepared = cleaned = false - assert_deprecated do - Reloader.cleanup! - end - assert prepared - assert cleaned - end - - def test_prepend_prepare_callback - i = 10 - assert_deprecated do - Reloader.to_prepare { i += 1 } - Reloader.to_prepare(prepend: true) { i = 0 } - end - - assert_deprecated do - Reloader.prepare! - end - assert_equal 1, i - end - - def test_cleanup_callbacks_are_called_on_exceptions - cleaned = false - assert_deprecated do - Reloader.to_cleanup { cleaned = true } - end + def test_complete_callbacks_are_called_on_exceptions + completed = false + reloader.to_complete { completed = true } begin call_and_return_body do @@ -188,16 +141,25 @@ class ReloaderTest < ActiveSupport::TestCase rescue end - assert cleaned + assert completed end private def call_and_return_body(&block) - x = Class.new(ActiveSupport::Reloader) - x.check = lambda { true } + app = middleware(block || proc { [200, {}, "response"] }) + _, _, body = app.call("rack.input" => StringIO.new("")) + body + end + + def middleware(inner_app, reloader = reloader()) + ActionDispatch::Reloader.new(inner_app, reloader) + end - @response ||= "response" - @reloader ||= Reloader.new(block || proc { [200, {}, @response] }, x) - @reloader.call("rack.input" => StringIO.new(""))[2] + def reloader(check = lambda { true }) + @reloader ||= begin + reloader = Class.new(ActiveSupport::Reloader) + reloader.check = check + reloader + end end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index e11b93b4f0..2f9228a62d 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -760,8 +760,8 @@ class RequestMethod < BaseRequestTest end test "post uneffected by local inflections" do - existing_acrnoyms = ActiveSupport::Inflector.inflections.acronyms.dup - existing_acrnoym_regex = ActiveSupport::Inflector.inflections.acronym_regex.dup + existing_acronyms = ActiveSupport::Inflector.inflections.acronyms.dup + existing_acronym_regex = ActiveSupport::Inflector.inflections.acronym_regex.dup begin ActiveSupport::Inflector.inflections do |inflect| inflect.acronym "POS" @@ -774,8 +774,8 @@ class RequestMethod < BaseRequestTest ensure # Reset original acronym set ActiveSupport::Inflector.inflections do |inflect| - inflect.send(:instance_variable_set, "@acronyms", existing_acrnoyms) - inflect.send(:instance_variable_set, "@acronym_regex", existing_acrnoym_regex) + inflect.send(:instance_variable_set, "@acronyms", existing_acronyms) + inflect.send(:instance_variable_set, "@acronym_regex", existing_acronym_regex) end 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..6d230a2557 --- /dev/null +++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb @@ -0,0 +1,291 @@ +require "abstract_unit" + +class TestCustomUrlHelpers < ActionDispatch::IntegrationTest + class Linkable + attr_reader :id + + def initialize(id) + @id = id + end + + def linkable_type + self.class.name.demodulize.underscore + end + end + + class Category < Linkable; end + class Collection < Linkable; end + class Product < 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 + + 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(: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] } + + resolve("Article") { |article| [:post, { id: article.id }] } + resolve("Basket") { |basket| [:basket] } + 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") + @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 test_direct_paths + assert_equal "http://www.rubyonrails.org", website_path + assert_equal "http://www.rubyonrails.org", Routes.url_helpers.website_path + + assert_equal "http://www.rubyonrails.org", string_path + assert_equal "http://www.rubyonrails.org", Routes.url_helpers.string_path + + assert_equal "http://www.example.com/basket", helper_url + assert_equal "http://www.example.com/basket", Routes.url_helpers.helper_url + + 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 "/", 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) + 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/", 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) + 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) + 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) + 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 474d053af6..53758a4fbc 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1603,7 +1603,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get "account/login", to: redirect("/login") end - previous_host, self.host = self.host, "www.example.com:3000" + previous_host, self.host = host, "www.example.com:3000" get "/account/login" verify_redirect "http://www.example.com:3000/login" diff --git a/actionpack/test/dispatch/runner_test.rb b/actionpack/test/dispatch/runner_test.rb index 969933c9ed..b76bf4a320 100644 --- a/actionpack/test/dispatch/runner_test.rb +++ b/actionpack/test/dispatch/runner_test.rb @@ -4,7 +4,6 @@ class RunnerTest < ActiveSupport::TestCase test "runner preserves the setting of integration_session" do runner = Class.new do def before_setup - end end.new diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 2a1053be16..63dfc07c0d 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -175,7 +175,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_doesnt_write_session_cookie_if_session_is_unchanged with_test_route_set do - cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--" + + cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--" \ "fef868465920f415f2c0652d6910d3af288a0367" get "/no_session_access" assert_response :success diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index e29ffa750c..757e26973f 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -102,7 +102,11 @@ class StrictTransportSecurityTest < SSLTest def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }, headers: {}) self.app = build_app ssl_options: { hsts: hsts }, headers: headers get url - assert_equal expected, response.headers["Strict-Transport-Security"] + if expected.nil? + assert_nil response.headers["Strict-Transport-Security"] + else + assert_equal expected, response.headers["Strict-Transport-Security"] + end end test "enabled by default" do @@ -130,7 +134,7 @@ class StrictTransportSecurityTest < SSLTest end test ":expires supports AS::Duration arguments" do - assert_hsts "max-age=31557600; includeSubDomains", hsts: { expires: 1.year } + assert_hsts "max-age=31556952; includeSubDomains", hsts: { expires: 1.year } end test "include subdomains" do diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index d8bc96e3e0..bd8318f5f6 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -163,7 +163,7 @@ module StaticTests assert_equal file_name, env["PATH_INFO"] end - def test_serves_gzip_with_propper_content_type_fallback + def test_serves_gzip_with_proper_content_type_fallback file_name = "/gzip/foo.zoo" response = get(file_name, "HTTP_ACCEPT_ENCODING" => "gzip") assert_gzip file_name, response diff --git a/actionpack/test/dispatch/system_testing/browser_test.rb b/actionpack/test/dispatch/system_testing/browser_test.rb new file mode 100644 index 0000000000..b0ad309492 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/browser_test.rb @@ -0,0 +1,10 @@ +require "abstract_unit" +require "action_dispatch/system_testing/browser" + +class BrowserTest < ActiveSupport::TestCase + test "initializing the browser" do + browser = ActionDispatch::SystemTesting::Browser.new(:chrome, [ 1400, 1400 ]) + assert_equal :chrome, browser.instance_variable_get(:@name) + assert_equal [ 1400, 1400 ], browser.instance_variable_get(:@screen_size) + end +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..f0ebdb38db --- /dev/null +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -0,0 +1,9 @@ +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 +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..8c14f799b0 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -0,0 +1,18 @@ +require "abstract_unit" +require "action_dispatch/system_testing/test_helpers/screenshot_helper" + +class ScreenshotHelperTest < ActiveSupport::TestCase + test "image path is saved in tmp directory" do + new_test = ActionDispatch::SystemTestCase.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 = ActionDispatch::SystemTestCase.new("x") + + new_test.stub :passed?, false do + assert_equal "tmp/screenshots/failures_x.png", new_test.send(:image_path) + end + 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..a384902a14 --- /dev/null +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -0,0 +1,21 @@ +require "abstract_unit" + +class SystemTestCaseTest < ActiveSupport::TestCase + test "driven_by sets Capybara's default driver to poltergeist" do + ActionDispatch::SystemTestCase.driven_by :poltergeist + + assert_equal :poltergeist, Capybara.default_driver + end + + test "driven_by sets Capybara's drivers respectively" do + ActionDispatch::SystemTestCase.driven_by :selenium, using: :chrome + + assert_includes Capybara.drivers, :selenium + assert_includes Capybara.drivers, :chrome + assert_equal :chrome, Capybara.default_driver + end + + test "selenium? returns false if driver is poltergeist" do + assert_not ActionDispatch::SystemTestCase.selenium?(:poltergeist) + end +end diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index d61a8c023a..2c74617944 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -20,7 +20,7 @@ module ActionDispatch "/:controller/*foo/bar" => %r{\A/(#{x})/(.+)/bar\Z}, "/:foo|*bar" => %r{\A/(?:([^/.?]+)|(.+))\Z}, }.each do |path, expected| - define_method(:"test_to_regexp_#{path}") do + define_method(:"test_to_regexp_#{Regexp.escape(path)}") do path = Pattern.build( path, { controller: /.+/ }, @@ -44,7 +44,7 @@ module ActionDispatch "/:controller/*foo/bar" => %r{\A/(#{x})/(.+)/bar}, "/:foo|*bar" => %r{\A/(?:([^/.?]+)|(.+))}, }.each do |path, expected| - define_method(:"test_to_non_anchored_regexp_#{path}") do + define_method(:"test_to_non_anchored_regexp_#{Regexp.escape(path)}") do path = Pattern.build( path, { controller: /.+/ }, @@ -67,7 +67,7 @@ module ActionDispatch "/:controller/*foo" => %w{ controller foo }, "/:controller/*foo/bar" => %w{ controller foo }, }.each do |path, expected| - define_method(:"test_names_#{path}") do + define_method(:"test_names_#{Regexp.escape(path)}") do path = Pattern.build( path, { controller: /.+/ }, diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index 046b4167bb..b768553e7a 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -1,6 +1,6 @@ require "active_model" -class Customer < Struct.new(:name, :id) +Customer = Struct.new(:name, :id) do extend ActiveModel::Naming include ActiveModel::Conversion @@ -28,7 +28,7 @@ class Customer < Struct.new(:name, :id) end end -class Post < Struct.new(:title, :author_name, :body, :secret, :persisted, :written_on, :cost) +Post = Struct.new(:title, :author_name, :body, :secret, :persisted, :written_on, :cost) do extend ActiveModel::Naming include ActiveModel::Conversion extend ActiveModel::Translation |