diff options
author | Sean Griffin <sean@seantheprogrammer.com> | 2017-07-17 07:10:27 -0600 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-07-17 07:10:27 -0600 |
commit | 404eceba8c8f0951407c2508567f6abefec76e4c (patch) | |
tree | cb8c690084083b6aee4a70ad29c24c0050f88bed /actionpack/lib/action_dispatch/routing | |
parent | e505bc8acd4448030fd4d2f85796bdb61d45a5b1 (diff) | |
parent | fbade51248ea48db87703ba7418badbd3ed85e36 (diff) | |
download | rails-404eceba8c8f0951407c2508567f6abefec76e4c.tar.gz rails-404eceba8c8f0951407c2508567f6abefec76e4c.tar.bz2 rails-404eceba8c8f0951407c2508567f6abefec76e4c.zip |
Merge branch 'master' into make-reverse-merge-bang-order-consistent
Diffstat (limited to 'actionpack/lib/action_dispatch/routing')
7 files changed, 129 insertions, 126 deletions
diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index b91ffb8419..9aa4b92df2 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -196,7 +196,7 @@ module ActionDispatch @buffer << @view.render(partial: "routes/route", collection: routes) end - # the header is part of the HTML page, so we don't construct it here. + # The header is part of the HTML page, so we don't construct it here. def header(routes) end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 6e06c70dc2..eaa0e12b67 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1,9 +1,11 @@ +# frozen_string_literal: true + 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" +require_relative "redirection" +require_relative "endpoint" module ActionDispatch module Routing @@ -17,9 +19,9 @@ module ActionDispatch CALL = ->(app, req) { app.call req.env } def initialize(app, constraints, strategy) - # Unwrap Constraints objects. I don't actually think it's possible + # Unwrap Constraints objects. I don't actually think it's possible # to pass a Constraints object to this constructor, but there were - # multiple places that kept testing children of this object. I + # multiple places that kept testing children of this object. I # *think* they were just being defensive, but I have no idea. if app.is_a?(self.class) constraints += app.constraints @@ -54,6 +56,7 @@ module ActionDispatch class Mapping #:nodoc: ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} + OPTIONAL_FORMAT_REGEX = %r{(?:\(\.:format\)+|\.:format|/)\Z} attr_reader :requirements, :defaults attr_reader :to, :default_controller, :default_action @@ -93,7 +96,7 @@ module ActionDispatch end def self.optional_format?(path, format) - format != false && !path.include?(":format") && !path.end_with?("/") + format != false && path !~ OPTIONAL_FORMAT_REGEX end def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, blocks, via, options_constraints, anchor, options) @@ -218,7 +221,7 @@ module ActionDispatch private def add_wildcard_options(options, formatted, path_ast) # Add a constraint for wildcard route to make it non-greedy and match the - # optional format part of the route by default + # optional format part of the route by default. if formatted != false path_ast.grep(Journey::Nodes::Star).each_with_object({}) { |node, hash| hash[node.name.to_sym] ||= /.+?/ @@ -305,7 +308,7 @@ module ActionDispatch def check_controller_and_action(path_params, controller, action) hash = check_part(:controller, controller, path_params, {}) do |part| translate_controller(part) { - message = "'#{part}' is not a supported controller name. This can lead to potential routing problems." + message = "'#{part}' is not a supported controller name. This can lead to potential routing problems.".dup message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" raise ArgumentError, message @@ -396,7 +399,7 @@ module ActionDispatch end module Base - # Matches a url pattern to one or more routes. + # Matches a URL pattern to one or more routes. # # You should not use the +match+ method in your router # without specifying an HTTP method. @@ -406,7 +409,7 @@ module ActionDispatch # # sets :controller, :action and :id in params # match ':controller/:action/:id', via: [:get, :post] # - # Note that +:controller+, +:action+ and +:id+ are interpreted as url + # Note that +:controller+, +:action+ and +:id+ are interpreted as URL # query parameters and thus available through +params+ in an action. # # If you want to expose your action to GET, use +get+ in the router: @@ -455,7 +458,7 @@ module ActionDispatch # # === Options # - # Any options not seen here are passed on as params with the url. + # Any options not seen here are passed on as params with the URL. # # [:controller] # The route's controller. @@ -651,18 +654,25 @@ module ActionDispatch def define_generate_prefix(app, name) _route = @set.named_routes.get name _routes = @set - app.routes.define_mounted_helper(name) + + script_namer = ->(options) do + prefix_options = options.slice(*_route.segment_keys) + prefix_options[:relative_url_root] = "".freeze + # We must actually delete prefix segment keys to avoid passing them to next url_for. + _route.segment_keys.each { |k| options.delete(k) } + _routes.url_helpers.send("#{name}_path", prefix_options) + end + + app.routes.define_mounted_helper(name, script_namer) + app.routes.extend Module.new { def optimize_routes_generation?; false; end + define_method :find_script_name do |options| if options.key? :script_name super(options) else - prefix_options = options.slice(*_route.segment_keys) - prefix_options[:relative_url_root] = "".freeze - # we must actually delete prefix segment keys to avoid passing them to next url_for - _route.segment_keys.each { |k| options.delete(k) } - _routes.url_helpers.send("#{name}_path", prefix_options) + script_namer.call(options) end end } @@ -1238,7 +1248,7 @@ module ActionDispatch # # resource :profile # - # creates six different routes in your application, all mapping to + # This creates six different routes in your application, all mapping to # the +Profiles+ controller (note that the controller is named after # the plural): # @@ -1323,14 +1333,14 @@ module ActionDispatch # # resources :posts, path_names: { new: "brand_new" } # - # The above example will now change /posts/new to /posts/brand_new + # The above example will now change /posts/new to /posts/brand_new. # # [:path] # Allows you to change the path prefix for the resource. # # resources :posts, path: 'postings' # - # The resource and all segments will now route to /postings instead of /posts + # The resource and all segments will now route to /postings instead of /posts. # # [:only] # Only generate routes for the given actions. @@ -1525,7 +1535,7 @@ module ActionDispatch end end - # See ActionDispatch::Routing::Mapper::Scoping#namespace + # See ActionDispatch::Routing::Mapper::Scoping#namespace. def namespace(path, options = {}) if resource_scope? nested { super } @@ -1545,7 +1555,7 @@ module ActionDispatch !parent_resource.singleton? && @scope[:shallow] end - # Matches a url pattern to one or more routes. + # Matches a URL pattern to one or more routes. # For more information, see match[rdoc-ref:Base#match]. # # match 'path' => 'controller#action', via: patch @@ -1836,7 +1846,7 @@ module ActionDispatch path_types.fetch(String, []).each do |_path| route_options = options.dup if _path && option_path - raise ArgumentError, "Ambigous route definition. Both :path and the route path where specified as strings." + raise ArgumentError, "Ambiguous route definition. Both :path and the route path where specified as strings." end to = get_to_from_path(_path, to, route_options[:action]) decomposed_match(_path, controller, route_options, _path, to, via, formatted, anchor, options_constraints) @@ -2003,7 +2013,7 @@ module ActionDispatch # concerns :commentable # end # - # concerns also work in any routes helper that you want to use: + # Concerns also work in any routes helper that you want to use: # # namespace :posts do # concerns :commentable @@ -2037,37 +2047,37 @@ module ActionDispatch # { 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 + # 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 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 + # 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: + # your URL helper definition, e.g: # # direct :browse, page: 1, size: 10 do |options| - # [ :products, options.merge(params.permit(:page, :size)) ] + # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] # end # - # In this instance the `params` object comes from the context in which the the - # block is executed, e.g. generating a url inside a controller action or a view. + # In this instance the +params+ object comes from the context in which the the + # block is executed, e.g. generating a URL inside a controller action or a view. # If the block is executed where there isn't a params object such as this: # # Rails.application.routes.url_helpers.browse_path # - # then it will raise a `NameError`. Because of this you need to be aware of the - # context in which you will use your custom url helper when defining it. + # then it will raise a +NameError+. Because of this you need to be aware of the + # context in which you will use your custom URL helper when defining it. # - # NOTE: The `direct` method can't be used inside of a scope block such as - # `namespace` or `scope` and will raise an error if it detects that it is. + # 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" @@ -2076,9 +2086,9 @@ module ActionDispatch @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: + # 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 # @@ -2086,10 +2096,10 @@ module ActionDispatch # [: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". + # 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 + # 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 @@ -2104,8 +2114,8 @@ module ActionDispatch # 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". + # 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: @@ -2114,12 +2124,12 @@ module ActionDispatch # [: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. + # 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. + # 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" diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 984ded1ff5..e89ea8b21d 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -40,7 +40,7 @@ module ActionDispatch # # Example usage: # - # edit_polymorphic_path(@post) # => "/posts/1/edit" + # edit_polymorphic_path(@post) # => "/posts/1/edit" # polymorphic_path(@post, format: :pdf) # => "/posts/1.pdf" # # == Usage with mounted engines @@ -104,7 +104,7 @@ module ActionDispatch end if mapping = polymorphic_mapping(record_or_hash_or_array) - return mapping.call(self, [record_or_hash_or_array, options]) + return mapping.call(self, [record_or_hash_or_array, options], false) end opts = options.dup @@ -128,7 +128,7 @@ module ActionDispatch end if mapping = polymorphic_mapping(record_or_hash_or_array) - return mapping.call(self, [record_or_hash_or_array, options], only_path: true) + return mapping.call(self, [record_or_hash_or_array, options], true) end opts = options.dup @@ -273,7 +273,7 @@ module ActionDispatch def handle_model_call(target, record) if mapping = polymorphic_mapping(target, record) - mapping.call(target, [record], only_path: suffix == "path") + mapping.call(target, [record], suffix == "path") else method, args = handle_model(record) target.send(method, *args) diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index e8f47b8640..6396b5031d 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -1,9 +1,9 @@ -require "action_dispatch/http/request" +require_relative "../http/request" require "active_support/core_ext/uri" require "active_support/core_ext/array/extract_options" require "rack/utils" require "action_controller/metal/exceptions" -require "action_dispatch/routing/endpoint" +require_relative "endpoint" module ActionDispatch module Routing @@ -146,7 +146,7 @@ module ActionDispatch # # get 'docs/:article', to: redirect('/wiki/%{article}') # - # Note that if you return a path without a leading slash then the url is prefixed with the + # Note that if you return a path without a leading slash then the URL is prefixed with the # current SCRIPT_NAME environment variable. This is typically '/' but may be different in # a mounted engine or where the application is deployed to a subdirectory of a website. # @@ -165,7 +165,7 @@ module ActionDispatch # Note that the +do end+ syntax for the redirect block wouldn't work, as Ruby would pass # the block to +get+ instead of +redirect+. Use <tt>{ ... }</tt> instead. # - # The options version of redirect allows you to supply only the parts of the url which need + # The options version of redirect allows you to supply only the parts of the URL which need # to change, it also supports interpolation of the path similar to the first example. # # get 'stores/:name', to: redirect(subdomain: 'stores', path: '/%{name}') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c4719f8a71..357eaec572 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,11 +1,13 @@ -require "action_dispatch/journey" +# frozen_string_literal: true + +require_relative "../journey" require "active_support/core_ext/object/to_query" require "active_support/core_ext/hash/slice" require "active_support/core_ext/module/remove_method" require "active_support/core_ext/array/extract_options" require "action_controller/metal/exceptions" -require "action_dispatch/http/request" -require "action_dispatch/routing/endpoint" +require_relative "../http/request" +require_relative "endpoint" module ActionDispatch module Routing @@ -73,7 +75,6 @@ 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 @@ -96,23 +97,9 @@ module ActionDispatch @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) @@ -158,21 +145,29 @@ module ActionDispatch routes.length end + # Given a +name+, defines name_path and name_url helpers. + # Used by 'direct', 'resolve', and 'polymorphic' route helpers. def add_url_helper(name, defaults, &block) - @custom_helpers << name helper = CustomUrlHelper.new(name, defaults, &block) + path_name = :"#{name}_path" + url_name = :"#{name}_url" @path_helpers_module.module_eval do - define_method(:"#{name}_path") do |*args| - helper.call(self, args, only_path: true) + define_method(path_name) do |*args| + helper.call(self, args, true) end end @url_helpers_module.module_eval do - define_method(:"#{name}_url") do |*args| - helper.call(self, args) + define_method(url_name) do |*args| + helper.call(self, args, false) end end + + @path_helpers << path_name + @url_helpers << url_name + + self end class UrlHelper @@ -240,7 +235,7 @@ module ActionDispatch missing_keys << missing_key } constraints = Hash[@route.requirements.merge(params).sort_by { |k, v| k.to_s }] - message = "No route matches #{constraints.inspect}" + message = "No route matches #{constraints.inspect}".dup message << ", missing required keys: #{missing_keys.sort.inspect}" raise ActionController::UrlGenerationError, message @@ -279,6 +274,8 @@ module ActionDispatch if args.size < path_params_size path_params -= controller_options.keys path_params -= result.keys + else + path_params = path_params.dup end inner_options.each_key do |key| path_params.delete(key) @@ -295,7 +292,7 @@ module ActionDispatch end private - # Create a url helper allowing ordered parameters to be associated + # Create a URL helper allowing ordered parameters to be associated # with corresponding dynamic segments, so you can do: # # foo_url(bar, baz, bang) @@ -318,11 +315,7 @@ module ActionDispatch when Hash args.pop when ActionController::Parameters - if last.permitted? - args.pop.to_h - else - raise ArgumentError, ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE - end + args.pop.to_h end helper.call self, args, options end @@ -458,7 +451,7 @@ module ActionDispatch MountedHelpers end - def define_mounted_helper(name) + def define_mounted_helper(name, script_namer = nil) return if MountedHelpers.method_defined?(name) routes = self @@ -466,7 +459,7 @@ module ActionDispatch MountedHelpers.class_eval do define_method "_#{name}" do - RoutesProxy.new(routes, _routes_context, helpers) + RoutesProxy.new(routes, _routes_context, helpers, script_namer) end end @@ -509,6 +502,14 @@ module ActionDispatch @_proxy.url_for(options) end + def full_url_for(options) + @_proxy.full_url_for(options) + end + + def route_for(name, *args) + @_proxy.route_for(name, *args) + end + def optimize_routes_generation? @_proxy.optimize_routes_generation? end @@ -613,26 +614,14 @@ module ActionDispatch @block = block end - def call(t, args, outer_options = {}) + def call(t, args, only_path = false) 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))) + url = t.full_url_for(eval_block(t, args, options)) + + if only_path + "/" + url.partition(%r{(?<!/)/(?!/)}).last else - t.url_for([url_options, outer_options]) + url end end @@ -860,8 +849,7 @@ module ActionDispatch params[key] = URI.parser.unescape(value) end end - old_params = req.path_parameters - req.path_parameters = old_params.merge params + req.path_parameters = params app = route.app if app.matches?(req) && app.dispatcher? begin diff --git a/actionpack/lib/action_dispatch/routing/routes_proxy.rb b/actionpack/lib/action_dispatch/routing/routes_proxy.rb index ee847eaeed..7a6c2e95d8 100644 --- a/actionpack/lib/action_dispatch/routing/routes_proxy.rb +++ b/actionpack/lib/action_dispatch/routing/routes_proxy.rb @@ -8,9 +8,10 @@ module ActionDispatch attr_accessor :scope, :routes alias :_routes :routes - def initialize(routes, scope, helpers) + def initialize(routes, scope, helpers, script_namer = nil) @routes, @scope = routes, scope @helpers = helpers + @script_namer = script_namer end def url_options @@ -19,7 +20,8 @@ module ActionDispatch end end - def respond_to_missing?(method, include_private = false) + private + def respond_to_missing?(method, _) super || @helpers.respond_to?(method) end @@ -28,11 +30,13 @@ module ActionDispatch self.class.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def #{method}(*args) options = args.extract_options! - args << url_options.merge((options || {}).symbolize_keys) + options = url_options.merge((options || {}).symbolize_keys) + options.reverse_merge!(script_name: @script_namer.call(options)) if @script_namer + args << options @helpers.#{method}(*args) end RUBY - send(method, *args) + public_send(method, *args) else super end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 3e564f13d8..a9bdefa775 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -113,10 +113,10 @@ module ActionDispatch default_url_options end - # Generate a url based on the options provided, default_url_options and the + # Generate a URL based on the options provided, default_url_options and the # routes defined in routes.rb. The following options are supported: # - # * <tt>:only_path</tt> - If true, the relative url is returned. Defaults to +false+. + # * <tt>:only_path</tt> - If true, the relative URL is returned. Defaults to +false+. # * <tt>:protocol</tt> - The protocol to connect to. Defaults to 'http'. # * <tt>:host</tt> - Specifies the host the link should be targeted at. # If <tt>:only_path</tt> is false, this option must be @@ -164,20 +164,17 @@ module ActionDispatch # implicitly used by +url_for+ can always be overwritten like shown on the # last +url_for+ calls. def url_for(options = nil) + full_url_for(options) + end + + def full_url_for(options = nil) # :nodoc: case options when nil _routes.url_for(url_options.symbolize_keys) - when Hash - route_name = options.delete :use_route - _routes.url_for(options.symbolize_keys.reverse_merge!(url_options), - route_name) - when ActionController::Parameters - unless options.permitted? - raise ArgumentError.new(ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE) - end + when Hash, ActionController::Parameters route_name = options.delete :use_route - _routes.url_for(options.to_h.symbolize_keys. - reverse_merge!(url_options), route_name) + merged_url_options = options.to_h.symbolize_keys.reverse_merge!(url_options) + _routes.url_for(merged_url_options, route_name) when String options when Symbol @@ -192,6 +189,10 @@ module ActionDispatch end end + def route_for(name, *args) # :nodoc: + public_send(:"#{name}_url", *args) + end + protected def optimize_routes_generation? |