From 4c91c442295fdea93ef1a7c44c28fcd3bf407323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 17 Aug 2016 16:09:34 -0300 Subject: [PATCH] Push :defaults extraction down one level Since e852daa6976cc6b6b28ad0c80a188c06e226df3c only the verb methods where extracting the defaults options. It was merged a fix for the `root` method in 31fbbb7faccba25b2e3b5e10b8fca1468579d629 but `match` was still broken since `:defaults` where not extracted. This was causing routes defined using `match` and having the `:defaults` keys to not be recognized. To fix this it was extracted a new private method with the actual content of `match` and the `:defaults` extracting was moved to `match`. --- .../lib/action_dispatch/routing/mapper.rb | 120 +++++++++--------- actionpack/test/dispatch/routing_test.rb | 18 +++ 2 files changed, 78 insertions(+), 60 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 3f4f920a87..e9173cc264 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -716,11 +716,7 @@ def delete(*args, &block) def map_method(method, args, &block) options = args.extract_options! options[:via] = method - if options.key?(:defaults) - defaults(options.delete(:defaults)) { match(*args, options, &block) } - else - match(*args, options, &block) - end + match(*args, options, &block) self end end @@ -1557,7 +1553,7 @@ def shallow? # match 'path' => 'controller#action', via: patch # match 'path', to: 'controller#action', via: :post # match 'path', 'otherpath', on: :member, via: :get - def match(path, *rest) + def match(path, *rest, &block) if rest.empty? && Hash === path options = path path, to = options.find { |name, _value| name.is_a?(String) } @@ -1588,55 +1584,11 @@ def match(path, *rest) paths = [path] + rest end - if options[:on] && !VALID_ON_OPTIONS.include?(options[:on]) - raise ArgumentError, "Unknown scope #{on.inspect} given to :on" - end - - if @scope[:to] - options[:to] ||= @scope[:to] - end - - if @scope[:controller] && @scope[:action] - options[: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.fetch(String, []).each do |_path| - route_options = options.dup - if _path && option_path - ActiveSupport::Deprecation.warn <<-eowarn -Specifying strings for both :path and the route path is deprecated. Change things like this: - - match #{_path.inspect}, :path => #{option_path.inspect} - -to this: - - match #{option_path.inspect}, :as => #{_path.inspect}, :action => #{path.inspect} - eowarn - route_options[:action] = _path - route_options[:as] = _path - _path = option_path - 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.fetch(Symbol, []).each do |action| - route_options = options.dup - decomposed_match(action, controller, route_options, option_path, to, via, formatted, anchor, options_constraints) + if options.key?(:defaults) + defaults(options.delete(:defaults)) { map_match(paths, options, &block) } + else + map_match(paths, options, &block) end - - self end def get_to_from_path(path, to, action) @@ -1912,6 +1864,7 @@ def set_member_mappings_for_resource def api_only? @set.api_only? end + private def path_scope(path) @@ -1921,16 +1874,63 @@ def path_scope(path) @scope = @scope.parent end + def map_match(paths, options) + if options[:on] && !VALID_ON_OPTIONS.include?(options[:on]) + raise ArgumentError, "Unknown scope #{on.inspect} given to :on" + end + + if @scope[:to] + options[:to] ||= @scope[:to] + end + + if @scope[:controller] && @scope[:action] + options[: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.fetch(String, []).each do |_path| + route_options = options.dup + if _path && option_path + ActiveSupport::Deprecation.warn <<-eowarn +Specifying strings for both :path and the route path is deprecated. Change things like this: + + match #{_path.inspect}, :path => #{option_path.inspect} + +to this: + + match #{option_path.inspect}, :as => #{_path.inspect}, :action => #{_path.inspect} + eowarn + route_options[:action] = _path + route_options[:as] = _path + _path = option_path + 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.fetch(Symbol, []).each do |action| + route_options = options.dup + decomposed_match(action, controller, route_options, option_path, to, via, formatted, anchor, options_constraints) + end + + self + end + def match_root_route(options) name = has_named_route?(:root) ? nil : :root - defaults_option = options.delete(:defaults) args = ["/", { as: name, via: :get }.merge!(options)] - if defaults_option - defaults(defaults_option) { match(*args) } - else - match(*args) - end + match(*args) end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 56be08f54f..ee5c30ef0e 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1759,6 +1759,24 @@ def test_default_integer_params assert_equal 1, @request.params[:page] end + def test_keyed_default_string_params_with_match + draw do + match "/", to: "pages#show", via: :get, defaults: { id: "home" } + end + + get "/" + assert_equal "home", @request.params[:id] + end + + def test_default_string_params_with_match + draw do + match "/", to: "pages#show", via: :get, id: "home" + end + + get "/" + assert_equal "home", @request.params[:id] + end + def test_keyed_default_string_params_with_root draw do root to: "pages#show", defaults: { id: "home" } -- GitLab