Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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.
  • Loading branch information...
commit c88ee76928a85cc34318d0442b38da4c850b7030 1 parent ea544e9
Yves Senn authored February 21, 2013
14  actionpack/CHANGELOG.md
Source Rendered
... ...
@@ -1,5 +1,19 @@
1 1
 ## Rails 4.0.0 (unreleased) ##
2 2
 
  3
+*   Determine the controller#action from only the matched path when using the
  4
+    shorthand syntax. Previously the complete path was used, which led
  5
+    to problems with nesting (scopes and namespaces).
  6
+    Fixes #7554.
  7
+
  8
+    Example:
  9
+
  10
+        # this will route to questions#new
  11
+        scope ':locale' do
  12
+          get 'questions/new'
  13
+        end
  14
+
  15
+    *Yves Senn*
  16
+
3 17
 *   Remove support for parsing XML parameters from request. If you still want to parse XML
4 18
     parameters, please install `actionpack-xml_parser' gem.
5 19
 
26  actionpack/lib/action_dispatch/routing/mapper.rb
@@ -50,7 +50,6 @@ def constraint_args(constraint, request)
50 50
       class Mapping #:nodoc:
51 51
         IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix, :format]
52 52
         ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
53  
-        SHORTHAND_REGEX = %r{/[\w/]+$}
54 53
         WILDCARD_PATH = %r{\*([^/\)]+)\)?$}
55 54
 
56 55
         attr_reader :scope, :path, :options, :requirements, :conditions, :defaults
@@ -111,17 +110,7 @@ def normalize_options!
111 110
               @options[:controller] ||= /.+?/
112 111
             end
113 112
 
114  
-            if using_match_shorthand?(path_without_format, @options)
115  
-              to_shorthand    = @options[:to].blank?
116  
-              @options[:to] ||= path_without_format.gsub(/\(.*\)/, "")[1..-1].sub(%r{/([^/]*)$}, '#\1')
117  
-            end
118  
-
119  
-            @options.merge!(default_controller_and_action(to_shorthand))
120  
-          end
121  
-
122  
-          # match "account/overview"
123  
-          def using_match_shorthand?(path, options)
124  
-            path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX
  113
+            @options.merge!(default_controller_and_action)
125 114
           end
126 115
 
127 116
           def normalize_format!
@@ -214,7 +203,7 @@ def app
214 203
             Constraints.new(endpoint, blocks, @set.request_class)
215 204
           end
216 205
 
217  
-          def default_controller_and_action(to_shorthand=nil)
  206
+          def default_controller_and_action
218 207
             if to.respond_to?(:call)
219 208
               { }
220 209
             else
@@ -227,7 +216,7 @@ def default_controller_and_action(to_shorthand=nil)
227 216
               controller ||= default_controller
228 217
               action     ||= default_action
229 218
 
230  
-              unless controller.is_a?(Regexp) || to_shorthand
  219
+              unless controller.is_a?(Regexp)
231 220
                 controller = [@scope[:module], controller].compact.join("/").presence
232 221
               end
233 222
 
@@ -1383,6 +1372,11 @@ def match(path, *rest)
1383 1372
             paths = [path] + rest
1384 1373
           end
1385 1374
 
  1375
+          path_without_format = path.to_s.sub(/\(\.:format\)$/, '')
  1376
+          if using_match_shorthand?(path_without_format, options)
  1377
+            options[:to] ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1')
  1378
+          end
  1379
+
1386 1380
           options[:anchor] = true unless options.key?(:anchor)
1387 1381
 
1388 1382
           if options[:on] && !VALID_ON_OPTIONS.include?(options[:on])
@@ -1393,6 +1387,10 @@ def match(path, *rest)
1393 1387
           self
1394 1388
         end
1395 1389
 
  1390
+        def using_match_shorthand?(path, options)
  1391
+          path && (options[:to] || options[:action]).nil? && path =~ %r{/[\w/]+$}
  1392
+        end
  1393
+
1396 1394
         def decomposed_match(path, options) # :nodoc:
1397 1395
           if on = options.delete(:on)
1398 1396
             send(on) { decomposed_match(path, options) }
27  actionpack/test/dispatch/routing_test.rb
@@ -1146,6 +1146,33 @@ def test_match_shorthand_inside_namespace_with_controller
1146 1146
     assert_equal 'api/products#list', @response.body
1147 1147
   end
1148 1148
 
  1149
+  def test_match_shorthand_inside_scope_with_variables_with_controller
  1150
+    draw do
  1151
+      scope ':locale' do
  1152
+        match 'questions/new', via: [:get]
  1153
+      end
  1154
+    end
  1155
+
  1156
+    get '/de/questions/new'
  1157
+    assert_equal 'questions#new', @response.body
  1158
+    assert_equal 'de', @request.params[:locale]
  1159
+  end
  1160
+
  1161
+  def test_match_shorthand_inside_nested_namespaces_and_scopes_with_controller
  1162
+    draw do
  1163
+      namespace :api do
  1164
+        namespace :v3 do
  1165
+          scope ':locale' do
  1166
+            get "products/list"
  1167
+          end
  1168
+        end
  1169
+      end
  1170
+    end
  1171
+
  1172
+    get '/api/v3/en/products/list'
  1173
+    assert_equal 'api/v3/products#list', @response.body
  1174
+  end
  1175
+
1149 1176
   def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper
1150 1177
     draw do
1151 1178
       resources :replies do

0 notes on commit c88ee76

Please sign in to comment.
Something went wrong with that request. Please try again.