diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 7a617108d1230..1be093b7555e4 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -87,7 +87,7 @@ class Mapping # :nodoc: attr_reader :path, :requirements, :defaults, :to, :default_controller, :default_action, :required_defaults, :ast, :scope_options - def self.build(scope, set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) + def self.build(scope, set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, internal, options) scope_params = { blocks: scope[:blocks] || [], constraints: scope[:constraints] || {}, @@ -98,7 +98,7 @@ def self.build(scope, set, ast, controller, default_action, to, via, formatted, new set: set, ast: ast, controller: controller, default_action: default_action, to: to, formatted: formatted, via: via, options_constraints: options_constraints, - anchor: anchor, scope_params: scope_params, options: scope_params[:options].merge(options) + anchor: anchor, scope_params: scope_params, internal: internal, options: scope_params[:options].merge(options) end def self.check_via(via) @@ -129,7 +129,7 @@ def self.optional_format?(path, format) format != false && !path.match?(OPTIONAL_FORMAT_REGEX) end - def initialize(set:, ast:, controller:, default_action:, to:, formatted:, via:, options_constraints:, anchor:, scope_params:, options:) + def initialize(set:, ast:, controller:, default_action:, to:, formatted:, via:, options_constraints:, anchor:, scope_params:, internal:, options:) @defaults = scope_params[:defaults] @set = set @to = intern(to) @@ -137,7 +137,7 @@ def initialize(set:, ast:, controller:, default_action:, to:, formatted:, via:, @default_action = intern(default_action) @anchor = anchor @via = via - @internal = options.delete(:internal) + @internal = internal @scope_options = scope_params[:options] ast = Journey::Ast.new(ast, formatted) @@ -607,17 +607,32 @@ def match(path, options = nil) # # This will generate the `exciting_path` and `exciting_url` helpers which can be # used to navigate to this mounted app. - def mount(app, options = nil) - if options - path = options.delete(:at) - elsif Hash === app - options = app - app, path = options.find { |k, _| k.respond_to?(:call) } - options.delete(app) if app + def mount(app = nil, deprecated_options = nil, as: DEFAULT, via: nil, at: nil, defaults: nil, constraints: nil, anchor: false, format: false, path: nil, internal: nil, **mapping, &block) + if deprecated_options.is_a?(Hash) + as = assign_deprecated_option(deprecated_options, :as, :mount) if deprecated_options.key?(:as) + via ||= assign_deprecated_option(deprecated_options, :via, :mount) + at ||= assign_deprecated_option(deprecated_options, :at, :mount) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :mount) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :mount) + anchor = assign_deprecated_option(deprecated_options, :anchor, :mount) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :mount) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :mount) + internal ||= assign_deprecated_option(deprecated_options, :internal, :mount) + assign_deprecated_options(deprecated_options, mapping, :mount) + end + + path_or_action = at + + if app.nil? + hash_app, hash_path = mapping.find { |key, _| key.respond_to?(:call) } + mapping.delete(hash_app) if hash_app + + app ||= hash_app + path_or_action ||= hash_path end raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call) - raise ArgumentError, <<~MSG unless path + raise ArgumentError, <<~MSG unless path_or_action Must be called with mount point mount SomeRackApp, at: "some_route" @@ -626,12 +641,12 @@ def mount(app, options = nil) MSG rails_app = rails_app? app - options[:as] ||= app_name(app, rails_app) + as = app_name(app, rails_app) if as == DEFAULT - target_as = name_for_action(options[:as], path) - options[:via] ||= :all + target_as = name_for_action(as, path_or_action) + via ||= :all - match(path, { to: app, anchor: false, format: false }.merge(options)) + match(path_or_action, to: app, as:, via:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, &block) define_generate_prefix(app, target_as) if rails_app self @@ -643,7 +658,7 @@ def default_url_options=(options) alias_method :default_url_options, :default_url_options= def with_default_scope(scope, &block) - scope(scope) do + scope(**scope) do instance_exec(&block) end end @@ -654,6 +669,24 @@ def has_named_route?(name) end private + def assign_deprecated_option(deprecated_options, key, method_name) + if (deprecated_value = deprecated_options.delete(key)) + ActionDispatch.deprecator.warn(<<~MSG.squish) + #{method_name} received a hash argument #{key}. Please use a keyword instead. + MSG + deprecated_value + end + end + + def assign_deprecated_options(deprecated_options, options, method_name) + deprecated_options.each do |key, value| + ActionDispatch.deprecator.warn(<<~MSG.squish) + #{method_name} received a hash argument #{key}. Please use a keyword instead. + MSG + options[key] = value + end + end + def rails_app?(app) app.is_a?(Class) && app < Rails::Railtie end @@ -707,48 +740,144 @@ module HttpHelpers # [match](rdoc-ref:Base#match) # # get 'bacon', to: 'food#bacon' - def get(*args, &block) - map_method(:get, args, &block) + def get(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: nil, format: nil, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :get) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :get) + controller ||= assign_deprecated_option(deprecated_options, :controller, :get) + action ||= assign_deprecated_option(deprecated_options, :action, :get) + on ||= assign_deprecated_option(deprecated_options, :on, :get) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :get) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :get) + anchor = assign_deprecated_option(deprecated_options, :anchor, :get) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :get) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :get) + internal ||= assign_deprecated_option(deprecated_options, :internal, :get) + assign_deprecated_options(deprecated_options, mapping, :get) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: :get, &block) + self end # Define a route that only recognizes HTTP POST. For supported arguments, see # [match](rdoc-ref:Base#match) # # post 'bacon', to: 'food#bacon' - def post(*args, &block) - map_method(:post, args, &block) + def post(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: nil, format: nil, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :post) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :post) + controller ||= assign_deprecated_option(deprecated_options, :controller, :post) + action ||= assign_deprecated_option(deprecated_options, :action, :post) + on ||= assign_deprecated_option(deprecated_options, :on, :post) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :post) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :post) + anchor = assign_deprecated_option(deprecated_options, :anchor, :post) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :post) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :post) + internal ||= assign_deprecated_option(deprecated_options, :internal, :post) + assign_deprecated_options(deprecated_options, mapping, :post) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: :post, &block) + self end # Define a route that only recognizes HTTP PATCH. For supported arguments, see # [match](rdoc-ref:Base#match) # # patch 'bacon', to: 'food#bacon' - def patch(*args, &block) - map_method(:patch, args, &block) + def patch(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: nil, format: nil, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :patch) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :patch) + controller ||= assign_deprecated_option(deprecated_options, :controller, :patch) + action ||= assign_deprecated_option(deprecated_options, :action, :patch) + on ||= assign_deprecated_option(deprecated_options, :on, :patch) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :patch) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :patch) + anchor = assign_deprecated_option(deprecated_options, :anchor, :patch) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :patch) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :patch) + internal ||= assign_deprecated_option(deprecated_options, :internal, :patch) + assign_deprecated_options(deprecated_options, mapping, :patch) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: :patch, &block) + self end # Define a route that only recognizes HTTP PUT. For supported arguments, see # [match](rdoc-ref:Base#match) # # put 'bacon', to: 'food#bacon' - def put(*args, &block) - map_method(:put, args, &block) + def put(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: nil, format: nil, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :put) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :put) + controller ||= assign_deprecated_option(deprecated_options, :controller, :put) + action ||= assign_deprecated_option(deprecated_options, :action, :put) + on ||= assign_deprecated_option(deprecated_options, :on, :put) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :put) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :put) + anchor = assign_deprecated_option(deprecated_options, :anchor, :put) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :put) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :put) + internal ||= assign_deprecated_option(deprecated_options, :internal, :put) + assign_deprecated_options(deprecated_options, mapping, :put) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: :put, &block) + self end # Define a route that only recognizes HTTP DELETE. For supported arguments, see # [match](rdoc-ref:Base#match) # # delete 'broccoli', to: 'food#broccoli' - def delete(*args, &block) - map_method(:delete, args, &block) + def delete(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: nil, format: nil, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :delete) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :delete) + controller ||= assign_deprecated_option(deprecated_options, :controller, :delete) + action ||= assign_deprecated_option(deprecated_options, :action, :delete) + on ||= assign_deprecated_option(deprecated_options, :on, :delete) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :delete) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :delete) + anchor = assign_deprecated_option(deprecated_options, :anchor, :delete) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :delete) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :delete) + internal ||= assign_deprecated_option(deprecated_options, :internal, :delete) + assign_deprecated_options(deprecated_options, mapping, :delete) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: :delete, &block) + self end # Define a route that only recognizes HTTP OPTIONS. For supported arguments, see # [match](rdoc-ref:Base#match) # # options 'carrots', to: 'food#carrots' - def options(*args, &block) - map_method(:options, args, &block) + def options(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: false, format: false, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :options) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :options) + controller ||= assign_deprecated_option(deprecated_options, :controller, :options) + action ||= assign_deprecated_option(deprecated_options, :action, :options) + on ||= assign_deprecated_option(deprecated_options, :on, :options) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :options) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :options) + anchor = assign_deprecated_option(deprecated_options, :anchor, :options) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :options) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :options) + internal ||= assign_deprecated_option(deprecated_options, :internal, :options) + assign_deprecated_options(deprecated_options, mapping, :options) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: :options, &block) + self end # Define a route that recognizes HTTP CONNECT (and GET) requests. More @@ -757,17 +886,25 @@ def options(*args, &block) # see [match](rdoc-ref:Base#match) # # connect 'live', to: 'live#index' - def connect(*args, &block) - map_method([:get, :connect], args, &block) + def connect(*path_or_actions, as: DEFAULT, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: false, format: false, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :connect) if deprecated_options.key?(:as) + to ||= assign_deprecated_option(deprecated_options, :to, :connect) + controller ||= assign_deprecated_option(deprecated_options, :controller, :connect) + action ||= assign_deprecated_option(deprecated_options, :action, :connect) + on ||= assign_deprecated_option(deprecated_options, :on, :connect) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :connect) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :connect) + anchor = assign_deprecated_option(deprecated_options, :anchor, :connect) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :connect) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :connect) + internal ||= assign_deprecated_option(deprecated_options, :internal, :connect) + assign_deprecated_options(deprecated_options, mapping, :connect) + end + + match(*path_or_actions, as:, to:, controller:, action:, on:, defaults:, constraints:, anchor:, format:, path:, internal:, **mapping, via: [:get, :connect], &block) + self end - - private - def map_method(method, args, &block) - options = args.extract_options! - options[:via] = method - match(*args, options, &block) - self - end end # You may wish to organize groups of controllers under a namespace. Most @@ -854,8 +991,13 @@ module Scoping # scope as: "sekret" do # resources :posts # end - def scope(*args) - options = args.extract_options!.dup + def scope(*args, only: nil, except: nil, **options) + if args.grep(Hash).any? && (deprecated_options = args.extract_options!) + only ||= assign_deprecated_option(deprecated_options, :only, :scope) + only ||= assign_deprecated_option(deprecated_options, :except, :scope) + assign_deprecated_options(deprecated_options, options, :scope) + end + scope = {} options[:path] = args.flatten.join("/") if args.any? @@ -876,9 +1018,8 @@ def scope(*args) block, options[:constraints] = options[:constraints], {} end - if options.key?(:only) || options.key?(:except) - scope[:action_options] = { only: options.delete(:only), - except: options.delete(:except) } + if only || except + scope[:action_options] = { only:, except: } end if options.key? :anchor @@ -958,18 +1099,24 @@ def controller(controller) # namespace :admin, as: "sekret" do # resources :posts # end - def namespace(path, options = {}, &block) - path = path.to_s - - defaults = { - module: path, - as: options.fetch(:as, path), - shallow_path: options.fetch(:path, path), - shallow_prefix: options.fetch(:as, path) - } + def namespace(name, deprecated_options = nil, as: DEFAULT, path: DEFAULT, shallow_path: DEFAULT, shallow_prefix: DEFAULT, **options, &block) + if deprecated_options.is_a?(Hash) + as = assign_deprecated_option(deprecated_options, :as, :namespace) if deprecated_options.key?(:as) + path ||= assign_deprecated_option(deprecated_options, :path, :namespace) if deprecated_options.key?(:path) + shallow_path ||= assign_deprecated_option(deprecated_options, :shallow_path, :namespace) if deprecated_options.key?(:shallow_path) + shallow_prefix ||= assign_deprecated_option(deprecated_options, :shallow_prefix, :namespace) if deprecated_options.key?(:shallow_prefix) + assign_deprecated_options(deprecated_options, options, :namespace) + end + + name = name.to_s + options[:module] ||= name + as = name if as == DEFAULT + path = name if path == DEFAULT + shallow_path = path if shallow_path == DEFAULT + shallow_prefix = as if shallow_prefix == DEFAULT - path_scope(options.delete(:path) { path }) do - scope(defaults.merge!(options), &block) + path_scope(path) do + scope(**options, as:, shallow_path:, shallow_prefix:, &block) end end @@ -1174,13 +1321,13 @@ def default_actions(api_only) attr_reader :controller, :path, :param - def initialize(entities, api_only, shallow, options = {}) + def initialize(entities, api_only, shallow, only: nil, except: nil, **options) if options[:param].to_s.include?(":") raise ArgumentError, ":param option can't contain colons" end valid_actions = self.class.default_actions(false) # ignore api_only for this validation - if invalid_actions = invalid_only_except_options(options, valid_actions).presence + if (invalid_actions = invalid_only_except_options(valid_actions, only:, except:).presence) raise ArgumentError, ":only and :except must include only #{valid_actions}, but also included #{invalid_actions}" end @@ -1192,8 +1339,8 @@ def initialize(entities, api_only, shallow, options = {}) @options = options @shallow = shallow @api_only = api_only - @only = options.delete :only - @except = options.delete :except + @only = only + @except = except end def default_actions @@ -1267,8 +1414,8 @@ def shallow? def singleton?; false; end private - def invalid_only_except_options(options, valid_actions) - options.values_at(:only, :except).flatten.compact.uniq.map(&:to_sym) - valid_actions + def invalid_only_except_options(valid_actions, only:, except:) + [only, except].flatten.compact.uniq.map(&:to_sym) - valid_actions end end @@ -1283,7 +1430,7 @@ def default_actions(api_only) end end - def initialize(entities, api_only, shallow, options) + def initialize(entities, api_only, shallow, **options) super @as = nil @controller = (options[:controller] || plural).to_s @@ -1344,19 +1491,22 @@ def resources_path_names(options) # # ### Options # Takes same options as [resources](rdoc-ref:#resources) - def resource(*resources, &block) - options = resources.extract_options!.dup + def resource(*resources, concerns: nil, **options, &block) + if resources.grep(Hash).any? && (deprecated_options = resources.extract_options!) + concerns = assign_deprecated_option(deprecated_options, :concerns, :resource) if deprecated_options.key?(:concerns) + assign_deprecated_options(deprecated_options, options, :resource) + end - if apply_common_behavior_for(:resource, resources, options, &block) + if apply_common_behavior_for(:resource, resources, concerns:, **options, &block) return self end with_scope_level(:resource) do options = apply_action_options :resource, options - resource_scope(SingletonResource.new(resources.pop, api_only?, @scope[:shallow], options)) do + resource_scope(SingletonResource.new(resources.pop, api_only?, @scope[:shallow], **options)) do yield if block_given? - concerns(options[:concerns]) if options[:concerns] + concerns(*concerns) if concerns new do get :new @@ -1514,19 +1664,22 @@ def resource(*resources, &block) # # # resource actions are at /admin/posts. # resources :posts, path: "admin/posts" - def resources(*resources, &block) - options = resources.extract_options!.dup + def resources(*resources, concerns: nil, **options, &block) + if resources.grep(Hash).any? && (deprecated_options = resources.extract_options!) + concerns = assign_deprecated_option(deprecated_options, :concerns, :resources) if deprecated_options.key?(:concerns) + assign_deprecated_options(deprecated_options, options, :resources) + end - if apply_common_behavior_for(:resources, resources, options, &block) + if apply_common_behavior_for(:resources, resources, concerns:, **options, &block) return self end with_scope_level(:resources) do options = apply_action_options :resources, options - resource_scope(Resource.new(resources.pop, api_only?, @scope[:shallow], options)) do + resource_scope(Resource.new(resources.pop, api_only?, @scope[:shallow], **options)) do yield if block_given? - concerns(options[:concerns]) if options[:concerns] + concerns(*concerns) if concerns collection do get :index if parent_resource.actions.include?(:index) @@ -1611,19 +1764,19 @@ def nested(&block) if shallow? && shallow_nesting_depth >= 1 shallow_scope do path_scope(parent_resource.nested_scope) do - scope(nested_options, &block) + scope(**nested_options, &block) end end else path_scope(parent_resource.nested_scope) do - scope(nested_options, &block) + scope(**nested_options, &block) end end end end # See ActionDispatch::Routing::Mapper::Scoping#namespace. - def namespace(path, options = {}) + def namespace(name, deprecated_options = nil, as: DEFAULT, path: DEFAULT, shallow_path: DEFAULT, shallow_prefix: DEFAULT, **options, &block) if resource_scope? nested { super } else @@ -1685,37 +1838,59 @@ def draw(name) # # match 'path', to: 'controller#action', via: :post # match 'path', 'otherpath', on: :member, via: :get - def match(path, *rest, &block) - if rest.empty? && Hash === path - options = path - path, to = options.find { |name, _value| name.is_a?(String) } + def match(*path_or_actions, as: DEFAULT, via: nil, to: nil, controller: nil, action: nil, on: nil, defaults: nil, constraints: nil, anchor: nil, format: nil, path: nil, internal: nil, **mapping, &block) + if path_or_actions.grep(Hash).any? && (deprecated_options = path_or_actions.extract_options!) + as = assign_deprecated_option(deprecated_options, :as, :match) if deprecated_options.key?(:as) + via ||= assign_deprecated_option(deprecated_options, :via, :match) + to ||= assign_deprecated_option(deprecated_options, :to, :match) + controller ||= assign_deprecated_option(deprecated_options, :controller, :match) + action ||= assign_deprecated_option(deprecated_options, :action, :match) + on ||= assign_deprecated_option(deprecated_options, :on, :match) + defaults ||= assign_deprecated_option(deprecated_options, :defaults, :match) + constraints ||= assign_deprecated_option(deprecated_options, :constraints, :match) + anchor = assign_deprecated_option(deprecated_options, :anchor, :match) if deprecated_options.key?(:anchor) + format = assign_deprecated_option(deprecated_options, :format, :match) if deprecated_options.key?(:format) + path ||= assign_deprecated_option(deprecated_options, :path, :match) + internal ||= assign_deprecated_option(deprecated_options, :internal, :match) + assign_deprecated_options(deprecated_options, mapping, :match) + end + + ActionDispatch.deprecator.warn(<<-MSG.squish) if path_or_actions.count > 1 + Mapping a route with multiple paths is deprecated and + will be removed in Rails 8.1. Please use multiple method calls instead. + MSG - raise ArgumentError, "Route path not specified" if path.nil? + if path_or_actions.none? && mapping.any? + hash_path, hash_to = mapping.find { |key, _| key.is_a?(String) } + if hash_path.nil? + raise ArgumentError, "Route path not specified" + else + mapping.delete(hash_path) + end - case to - when Symbol - options[:action] = to - when String - if to.include?("#") - options[:to] = to + if hash_path + path_or_actions.push hash_path + case hash_to + when Symbol + action ||= hash_to + when String + if hash_to.include?("#") + to ||= hash_to + else + controller ||= hash_to + end else - options[:controller] = to + to ||= hash_to end - else - options[:to] = to end - - options.delete(path) - paths = [path] - else - options = rest.pop || {} - paths = [path] + rest end - if options.key?(:defaults) - defaults(options.delete(:defaults)) { map_match(paths, options, &block) } - else - map_match(paths, options, &block) + path_or_actions.each do |path_or_action| + if defaults + defaults(defaults) { map_match(path_or_action, as:, via:, to:, controller:, action:, on:, constraints:, anchor:, format:, path:, internal:, mapping:, &block) } + else + map_match(path_or_action, as:, via:, to:, controller:, action:, on:, constraints:, anchor:, format:, path:, internal:, mapping:, &block) + end end end @@ -1757,22 +1932,21 @@ def parent_resource @scope[:scope_level_resource] end - def apply_common_behavior_for(method, resources, options, &block) + def apply_common_behavior_for(method, resources, shallow: nil, **options, &block) if resources.length > 1 - resources.each { |r| public_send(method, r, options, &block) } + resources.each { |r| public_send(method, r, shallow:, **options, &block) } return true end - if options[:shallow] - options.delete(:shallow) - shallow do - public_send(method, resources.pop, options, &block) + if shallow + self.shallow do + public_send(method, resources.pop, **options, &block) end return true end if resource_scope? - nested { public_send(method, resources.pop, options, &block) } + nested { public_send(method, resources.pop, shallow:, **options, &block) } return true end @@ -1781,9 +1955,9 @@ def apply_common_behavior_for(method, resources, options, &block) end scope_options = options.slice!(*RESOURCE_OPTIONS) - unless scope_options.empty? - scope(scope_options) do - public_send(method, resources.pop, options, &block) + if !scope_options.empty? || !shallow.nil? + scope(**scope_options, shallow:) do + public_send(method, resources.pop, **options, &block) end return true end @@ -1874,9 +2048,10 @@ def canonical_action?(action) end def shallow_scope - scope = { as: @scope[:shallow_prefix], - path: @scope[:shallow_path] } - @scope = @scope.new scope + @scope = @scope.new( + as: @scope[:shallow_prefix], + path: @scope[:shallow_path], + ) yield ensure @@ -1898,7 +2073,7 @@ def action_path(name) end def prefix_name_for_action(as, action) - if as + if as && as != DEFAULT prefix = as elsif !canonical_action?(action) prefix = action @@ -1914,7 +2089,7 @@ def name_for_action(as, action) name_prefix = @scope[:as] if parent_resource - return nil unless as || action + return nil unless as != DEFAULT || action collection_name = parent_resource.collection_name member_name = parent_resource.member_name @@ -1927,7 +2102,7 @@ def name_for_action(as, action) # If a name was not explicitly given, we check if it is valid and return nil in # case it isn't. Otherwise, we pass the invalid name forward so the underlying # router engine treats it and raises an exception. - if as.nil? + if as == DEFAULT candidate unless !candidate.match?(/\A[_a-z]/i) || has_named_route?(candidate) else candidate @@ -1958,47 +2133,35 @@ def path_scope(path) @scope = @scope.parent end - def map_match(paths, options) - ActionDispatch.deprecator.warn(<<-MSG.squish) if paths.count > 1 - Mapping a route with multiple paths is deprecated and - will be removed in Rails 8.1. Please use multiple method calls instead. - MSG - - if (on = options[:on]) && !VALID_ON_OPTIONS.include?(on) + def map_match(path_or_action, constraints: nil, anchor: nil, format: nil, path: nil, as: DEFAULT, via: nil, to: nil, controller: nil, action: nil, on: nil, internal: nil, mapping: nil) + if on && !VALID_ON_OPTIONS.include?(on) raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end if @scope[:to] - options[:to] ||= @scope[:to] + to ||= @scope[:to] end if @scope[:controller] && @scope[:action] - options[:to] ||= "#{@scope[:controller]}##{@scope[:action]}" + to ||= "#{@scope[:controller]}##{@scope[:action]}" end - controller = options.delete(:controller) || @scope[:controller] - option_path = options.delete :path - to = options.delete :to - via = Mapping.check_via Array(options.delete(:via) { - @scope[:via] - }) - formatted = options.delete(:format) { @scope[:format] } - anchor = options.delete(:anchor) { true } - options_constraints = options.delete(:constraints) || {} - - path_types = paths.group_by(&:class) - (path_types[String] || []).each do |_path| - route_options = options.dup - if _path && option_path + controller ||= @scope[:controller] + via = Mapping.check_via Array(via || @scope[:via]) + format ||= @scope[:format] if format.nil? + anchor ||= true if anchor.nil? + constraints ||= {} + + case path_or_action + when String + if path_or_action && path raise ArgumentError, "Ambiguous route definition. Both :path and the route path were 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) - end - - (path_types[Symbol] || []).each do |action| - route_options = options.dup - decomposed_match(action, controller, route_options, option_path, to, via, formatted, anchor, options_constraints) + path = path_or_action + to = get_to_from_path(path_or_action, to, action) + decomposed_match(path, controller, as, action, path, to, via, format, anchor, constraints, internal, mapping, on) + when Symbol + decomposed_match(path_or_action, controller, as, action, path, to, via, format, anchor, constraints, internal, mapping, on) end self @@ -2019,28 +2182,28 @@ def using_match_shorthand?(path) %r{^/?[-\w]+/[-\w/]+$}.match?(path) end - def decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) - if on = options.delete(:on) - send(on) { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) } + def decomposed_match(path, controller, as, action, _path, to, via, formatted, anchor, options_constraints, internal, options_mapping, on = nil) + if on + send(on) { decomposed_match(path, controller, as, action, _path, to, via, formatted, anchor, options_constraints, internal, options_mapping) } else case @scope.scope_level when :resources - nested { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) } + nested { decomposed_match(path, controller, as, action, _path, to, via, formatted, anchor, options_constraints, internal, options_mapping) } when :resource - member { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) } + member { decomposed_match(path, controller, as, action, _path, to, via, formatted, anchor, options_constraints, internal, options_mapping) } else - add_route(path, controller, options, _path, to, via, formatted, anchor, options_constraints) + add_route(path, controller, as, action, _path, to, via, formatted, anchor, options_constraints, internal, options_mapping) end end end - def add_route(action, controller, options, _path, to, via, formatted, anchor, options_constraints) + def add_route(action, controller, as, options_action, _path, to, via, formatted, anchor, options_constraints, internal, options_mapping) path = path_for_action(action, _path) raise ArgumentError, "path is required" if path.blank? action = action.to_s - default_action = options.delete(:action) || @scope[:action] + default_action = options_action || @scope[:action] if /^[\w\-\/]+$/.match?(action) default_action ||= action.tr("-", "_") unless action.include?("/") @@ -2048,22 +2211,16 @@ def add_route(action, controller, options, _path, to, via, formatted, anchor, op action = nil end - as = if !options.fetch(:as, true) # if it's set to nil or false - options.delete(:as) - else - name_for_action(options.delete(:as), action) - end - + as = name_for_action(as, action) if as path = Mapping.normalize_path URI::RFC2396_PARSER.escape(path), formatted - ast = Journey::Parser.parse path + ast = Journey::Parser.parse path - mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) + mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, internal, options_mapping) @set.add_route(mapping, as) end def match_root_route(options) - args = ["/", { as: :root, via: :get }.merge(options)] - match(*args) + match("/", as: :root, via: :get, **options) end end @@ -2158,8 +2315,7 @@ def concern(name, callable = nil, &block) # namespace :posts do # concerns :commentable # end - def concerns(*args) - options = args.extract_options! + def concerns(*args, **options) args.flatten.each do |name| if concern = @concerns[name] concern.call(self, options) @@ -2374,6 +2530,8 @@ def each ROOT = Scope.new({}, nil) end + DEFAULT = Object.new # :nodoc: + def initialize(set) # :nodoc: @set = set @draw_paths = set.draw_paths diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 07540535467b4..89b56655e20a5 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -76,7 +76,7 @@ def test_multiple_default_restful_routes def test_multiple_resources_with_options expected_options = { controller: "threads", action: "index" } - with_restful_routing :messages, :comments, expected_options.slice(:controller) do + with_restful_routing :messages, :comments, controller: "threads" do assert_recognizes(expected_options, path: "comments") assert_recognizes(expected_options, path: "messages") end @@ -1176,17 +1176,15 @@ def test_invalid_except_option_for_singleton_resource end private - def with_restful_routing(*args) - options = args.extract_options! + def with_restful_routing(*args, **options) collection_methods = options.delete(:collection) member_methods = options.delete(:member) path_prefix = options.delete(:path_prefix) - args.push(options) with_routing do |set| set.draw do scope(path_prefix || "") do - resources(*args) do + resources(*args, **options) do if collection_methods collection do collection_methods.each do |name, method| diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 27ac24831888b..707d2e29618de 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -93,7 +93,7 @@ def test_mapping_requirements options = {} scope = Mapper::Scope.new({}) ast = Journey::Parser.parse "/store/:name(*rest)" - m = Mapper::Mapping.build(scope, FakeSet.new, ast, "foo", "bar", nil, [:get], nil, {}, true, options) + m = Mapper::Mapping.build(scope, FakeSet.new, ast, "foo", "bar", nil, [:get], nil, {}, true, nil, options) assert_equal(/.+?/m, m.requirements[:rest]) end @@ -225,6 +225,63 @@ def test_scope_does_not_destructively_mutate_default_options end end end + + def test_deprecated_hash + fakeset = FakeSet.new + mapper = Mapper.new fakeset + + assert_deprecated(ActionDispatch.deprecator) do + mapper.get "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.post "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.put "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.patch "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.delete "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.options "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.connect "/foo", { to: "home#index" } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.match "/foo", { to: "home#index", via: :get } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.mount(lambda { |env| [200, {}, [""]] }, { at: "/" }) + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.scope("/hello", { only: :get }) { } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.namespace(:admin, { module: "sekret" }) { } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.resource(:user, { only: :show }) { } + end + + assert_deprecated(ActionDispatch.deprecator) do + mapper.resources(:users, { only: :show }) { } + end + end end end end diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 1ff236aee3c67..3c80772f9faf2 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -7,14 +7,14 @@ class ReviewsController < ResourcesController; end class RoutingConcernsTest < ActionDispatch::IntegrationTest class Reviewable def self.call(mapper, options = {}) - mapper.resources :reviews, options + mapper.resources :reviews, **options end end Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| app.draw do concern :commentable do |options| - resources :comments, options + resources :comments, **options end concern :image_attachable do diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 869b8d0cfd1e3..488ef423c5ce9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -988,13 +988,13 @@ def test_resources_for_uncountable_names def test_resource_does_not_modify_passed_options options = { id: /.+?/, format: /json|xml/ } - draw { resource :user, options } + draw { resource :user, **options } assert_equal({ id: /.+?/, format: /json|xml/ }, options) end def test_resources_does_not_modify_passed_options options = { id: /.+?/, format: /json|xml/ } - draw { resources :users, options } + draw { resources :users, **options } assert_equal({ id: /.+?/, format: /json|xml/ }, options) end @@ -2107,32 +2107,10 @@ def test_resource_merges_options_from_scope end end - assert_equal "/account", account_path assert_raise(NoMethodError) { new_account_path } get "/account/new" assert_equal 404, status - - get "/account" - assert_equal 200, status - end - - def test_resource_merges_options_from_scope_hash - draw do - scope_options = { only: :show } - scope scope_options do - resource :account - end - end - - assert_equal "/account", account_path - assert_raise(NoMethodError) { new_account_path } - - get "/account/new" - assert_equal 404, status - - get "/account" - assert_equal 200, status end def test_resources_merges_options_from_scope diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 4dfff85c33380..7c475d52ec0c5 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -219,7 +219,7 @@ def test_nil_path_parts_are_ignored def test_generate_slash params = [ [:controller, "tasks"], [:action, "show"] ] - get "/", Hash[params] + get "/", **Hash[params] path, _ = _generate(nil, Hash[params], {}) assert_equal "/", path @@ -498,15 +498,15 @@ def _generate(route_name, options, recall) [uri.path, params] end - def get(*args) + def get(...) ActionDispatch.deprecator.silence do - mapper.get(*args) + mapper.get(...) end end - def match(*args) + def match(...) ActionDispatch.deprecator.silence do - mapper.match(*args) + mapper.match(...) end end