diff options
author | Yves Senn <yves.senn@gmail.com> | 2013-02-21 17:43:49 +0100 |
---|---|---|
committer | Yves Senn <yves.senn@gmail.com> | 2013-02-21 17:44:31 +0100 |
commit | c88ee76928a85cc34318d0442b38da4c850b7030 (patch) | |
tree | 100c61dcc5b70741ddf9238d558de38650eaf0a2 | |
parent | ea544e9b481d669fa0a0863b872b6e9034806d07 (diff) | |
download | rails-c88ee76928a85cc34318d0442b38da4c850b7030.tar.gz rails-c88ee76928a85cc34318d0442b38da4c850b7030.tar.bz2 rails-c88ee76928a85cc34318d0442b38da4c850b7030.zip |
determine the match shorthand target early.
Closes #7554.
This patch determines the `controller#action` directly
in the `match` method when the shorthand syntax is used.
this prevents problems with namespaces and scopes.
-rw-r--r-- | actionpack/CHANGELOG.md | 14 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 26 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 27 |
3 files changed, 53 insertions, 14 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1a0060c911..c9d3b1c55c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,19 @@ ## Rails 4.0.0 (unreleased) ## +* Determine the controller#action from only the matched path when using the + shorthand syntax. Previously the complete path was used, which led + to problems with nesting (scopes and namespaces). + Fixes #7554. + + Example: + + # this will route to questions#new + scope ':locale' do + get 'questions/new' + end + + *Yves Senn* + * Remove support for parsing XML parameters from request. If you still want to parse XML parameters, please install `actionpack-xml_parser' gem. diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0a41ed0fcf..a8e225d61c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -50,7 +50,6 @@ module ActionDispatch class Mapping #:nodoc: IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix, :format] ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} - SHORTHAND_REGEX = %r{/[\w/]+$} WILDCARD_PATH = %r{\*([^/\)]+)\)?$} attr_reader :scope, :path, :options, :requirements, :conditions, :defaults @@ -111,17 +110,7 @@ module ActionDispatch @options[:controller] ||= /.+?/ end - if using_match_shorthand?(path_without_format, @options) - to_shorthand = @options[:to].blank? - @options[:to] ||= path_without_format.gsub(/\(.*\)/, "")[1..-1].sub(%r{/([^/]*)$}, '#\1') - end - - @options.merge!(default_controller_and_action(to_shorthand)) - end - - # match "account/overview" - def using_match_shorthand?(path, options) - path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX + @options.merge!(default_controller_and_action) end def normalize_format! @@ -214,7 +203,7 @@ module ActionDispatch Constraints.new(endpoint, blocks, @set.request_class) end - def default_controller_and_action(to_shorthand=nil) + def default_controller_and_action if to.respond_to?(:call) { } else @@ -227,7 +216,7 @@ module ActionDispatch controller ||= default_controller action ||= default_action - unless controller.is_a?(Regexp) || to_shorthand + unless controller.is_a?(Regexp) controller = [@scope[:module], controller].compact.join("/").presence end @@ -1383,6 +1372,11 @@ module ActionDispatch paths = [path] + rest end + path_without_format = path.to_s.sub(/\(\.:format\)$/, '') + if using_match_shorthand?(path_without_format, options) + options[:to] ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1') + end + options[:anchor] = true unless options.key?(:anchor) if options[:on] && !VALID_ON_OPTIONS.include?(options[:on]) @@ -1393,6 +1387,10 @@ module ActionDispatch self end + def using_match_shorthand?(path, options) + path && (options[:to] || options[:action]).nil? && path =~ %r{/[\w/]+$} + end + def decomposed_match(path, options) # :nodoc: if on = options.delete(:on) send(on) { decomposed_match(path, options) } diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 143733254b..37ad9ddb6b 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1146,6 +1146,33 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'api/products#list', @response.body end + def test_match_shorthand_inside_scope_with_variables_with_controller + draw do + scope ':locale' do + match 'questions/new', via: [:get] + end + end + + get '/de/questions/new' + assert_equal 'questions#new', @response.body + assert_equal 'de', @request.params[:locale] + end + + def test_match_shorthand_inside_nested_namespaces_and_scopes_with_controller + draw do + namespace :api do + namespace :v3 do + scope ':locale' do + get "products/list" + end + end + end + end + + get '/api/v3/en/products/list' + assert_equal 'api/v3/products#list', @response.body + end + def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper draw do resources :replies do |