From e135bbacb9316ec6f1cad18548952816150ac927 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 14:50:47 -0700
Subject: add a test for existing mapper functionality

I'm not sure if this is actually used, but I'm adding a test to define
the behavior
---
 actionpack/test/dispatch/routing_test.rb | 12 ++++++++++++
 1 file changed, 12 insertions(+)

(limited to 'actionpack')

diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index c9777ae71f..20e79523f9 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3137,6 +3137,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal '/foo', foo_root_path
   end
 
+  def test_namespace_as_controller
+    draw do
+      namespace :foo do
+        get '/', to: '/bar#index', as: 'root'
+      end
+    end
+
+    get '/foo'
+    assert_equal 'bar#index', @response.body
+    assert_equal '/foo', foo_root_path
+  end
+
   def test_trailing_slash
     draw do
       resources :streams
-- 
cgit v1.2.3


From 1ad50aa37939da41714409bf249dcb208dba3c75 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 14:52:17 -0700
Subject: set defaults at the top so we can avoid the ||= test

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 84a08770f5..68f9225a45 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -240,15 +240,18 @@ module ActionDispatch
             if to.respond_to?(:call)
               { }
             else
-              if to.is_a?(String)
-                controller, action = to.split('#')
-              elsif to.is_a?(Symbol)
+              controller = default_controller
+              action     = default_action
+
+              case to
+              when Symbol
                 action = to.to_s
+              when /#/
+                controller, action = to.split('#')
+              when String
+                controller = to
               end
 
-              controller ||= default_controller
-              action     ||= default_action
-
               if @scope[:module] && !controller.is_a?(Regexp)
                 if controller =~ %r{\A/}
                   controller = controller[1..-1]
-- 
cgit v1.2.3


From ddda5e70f79288f03a873cc8b4dbc6c086f4ad82 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 15:33:32 -0700
Subject: extract controller checks to methods

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 45 ++++++++++++++----------
 1 file changed, 26 insertions(+), 19 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 68f9225a45..e0b725c8f2 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -260,28 +260,11 @@ module ActionDispatch
                 end
               end
 
-              if controller.is_a?(String) && controller =~ %r{\A/}
-                raise ArgumentError, "controller name should not start with a slash"
-              end
-
               controller = controller.to_s unless controller.is_a?(Regexp)
               action     = action.to_s     unless action.is_a?(Regexp)
 
-              if controller.blank? && segment_keys.exclude?(:controller)
-                message = "Missing :controller key on routes definition, please check your routes."
-                raise ArgumentError, message
-              end
-
-              if action.blank? && segment_keys.exclude?(:action)
-                message = "Missing :action key on routes definition, please check your routes."
-                raise ArgumentError, message
-              end
-
-              if controller.is_a?(String) && controller !~ /\A[a-z_0-9\/]*\z/
-                message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
-                message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
-                raise ArgumentError, message
-              end
+              check_action! action
+              check_controller! controller
 
               hash = {}
               hash[:controller] = controller unless controller.blank?
@@ -290,6 +273,30 @@ module ActionDispatch
             end
           end
 
+          def check_action!(action)
+            if action.blank? && segment_keys.exclude?(:action)
+              message = "Missing :action key on routes definition, please check your routes."
+              raise ArgumentError, message
+            end
+          end
+
+          def check_controller!(controller)
+            if controller.is_a?(String) && controller =~ %r{\A/}
+              raise ArgumentError, "controller name should not start with a slash"
+            end
+
+            if controller.blank? && segment_keys.exclude?(:controller)
+              message = "Missing :controller key on routes definition, please check your routes."
+              raise ArgumentError, message
+            end
+
+            if controller.is_a?(String) && controller !~ /\A[a-z_0-9\/]*\z/
+              message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
+              message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
+              raise ArgumentError, message
+            end
+          end
+
           def blocks
             if options[:constraints].present? && !options[:constraints].is_a?(Hash)
               [options[:constraints]]
-- 
cgit v1.2.3


From f0eff10c090a56ea28201341361439dfbc31485b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 15:38:11 -0700
Subject: reduce blank? checks

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index e0b725c8f2..565609542a 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -260,14 +260,19 @@ module ActionDispatch
                 end
               end
 
-              controller = controller.to_s unless controller.is_a?(Regexp)
+              hash = {}
+
               action     = action.to_s     unless action.is_a?(Regexp)
 
+              if controller.is_a? Regexp
+                hash[:controller] = controller
+              else
+                check_controller! controller.to_s
+                hash[:controller] = controller.to_s if controller
+              end
+
               check_action! action
-              check_controller! controller
 
-              hash = {}
-              hash[:controller] = controller unless controller.blank?
               hash[:action]     = action unless action.blank?
               hash
             end
-- 
cgit v1.2.3


From c99d28f1f223e87523559f071eca2d84ac41f14c Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 15:46:00 -0700
Subject: reduce action.blank? calls

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 565609542a..178728ea2a 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -262,8 +262,6 @@ module ActionDispatch
 
               hash = {}
 
-              action     = action.to_s     unless action.is_a?(Regexp)
-
               if controller.is_a? Regexp
                 hash[:controller] = controller
               else
@@ -271,9 +269,13 @@ module ActionDispatch
                 hash[:controller] = controller.to_s if controller
               end
 
-              check_action! action
+              if action.is_a? Regexp
+                hash[:action] = action
+              else
+                check_action! action.to_s
+                hash[:action] = action.to_s if action
+              end
 
-              hash[:action]     = action unless action.blank?
               hash
             end
           end
-- 
cgit v1.2.3


From 353df48a341953b1bf457aa6f871a6054d6f64ba Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 15:50:22 -0700
Subject: fewer blank? calls

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 178728ea2a..6b6e27ceb0 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -265,14 +265,14 @@ module ActionDispatch
               if controller.is_a? Regexp
                 hash[:controller] = controller
               else
-                check_controller! controller.to_s
+                check_controller! controller
                 hash[:controller] = controller.to_s if controller
               end
 
               if action.is_a? Regexp
                 hash[:action] = action
               else
-                check_action! action.to_s
+                check_action! action
                 hash[:action] = action.to_s if action
               end
 
@@ -281,7 +281,7 @@ module ActionDispatch
           end
 
           def check_action!(action)
-            if action.blank? && segment_keys.exclude?(:action)
+            if action.nil? && segment_keys.exclude?(:action)
               message = "Missing :action key on routes definition, please check your routes."
               raise ArgumentError, message
             end
@@ -292,7 +292,7 @@ module ActionDispatch
               raise ArgumentError, "controller name should not start with a slash"
             end
 
-            if controller.blank? && segment_keys.exclude?(:controller)
+            if controller.nil? && segment_keys.exclude?(:controller)
               message = "Missing :controller key on routes definition, please check your routes."
               raise ArgumentError, message
             end
-- 
cgit v1.2.3


From 60ae50507d587e3663794fb8bdd0f967e5368695 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 15:54:43 -0700
Subject: invert logic to remove nil? and exclude? checks (use ruby rather than
 AS when possible

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 6b6e27ceb0..e8dd54b3f0 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -281,22 +281,22 @@ module ActionDispatch
           end
 
           def check_action!(action)
-            if action.nil? && segment_keys.exclude?(:action)
+            unless action || segment_keys.include?(:action)
               message = "Missing :action key on routes definition, please check your routes."
               raise ArgumentError, message
             end
           end
 
           def check_controller!(controller)
-            if controller.is_a?(String) && controller =~ %r{\A/}
-              raise ArgumentError, "controller name should not start with a slash"
-            end
-
-            if controller.nil? && segment_keys.exclude?(:controller)
+            unless controller || segment_keys.include?(:controller)
               message = "Missing :controller key on routes definition, please check your routes."
               raise ArgumentError, message
             end
 
+            if controller.is_a?(String) && controller =~ %r{\A/}
+              raise ArgumentError, "controller name should not start with a slash"
+            end
+
             if controller.is_a?(String) && controller !~ /\A[a-z_0-9\/]*\z/
               message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
               message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
-- 
cgit v1.2.3


From 996e9f568cba400cf82ececbc69ea797b863644b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 15:56:12 -0700
Subject: trade 2 is_a? checks for a nil check

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index e8dd54b3f0..528bcd4189 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -293,11 +293,13 @@ module ActionDispatch
               raise ArgumentError, message
             end
 
-            if controller.is_a?(String) && controller =~ %r{\A/}
+            return unless controller
+
+            if controller =~ %r{\A/}
               raise ArgumentError, "controller name should not start with a slash"
             end
 
-            if controller.is_a?(String) && controller !~ /\A[a-z_0-9\/]*\z/
+            if controller !~ /\A[a-z_0-9\/]*\z/
               message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
               message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
               raise ArgumentError, message
-- 
cgit v1.2.3


From ac9a3a9d641a44f43796b342f76c5b342ae82de6 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:00:40 -0700
Subject: return early if we have a valid controller name

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 528bcd4189..b8aadff5e6 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -294,16 +294,16 @@ module ActionDispatch
             end
 
             return unless controller
+            return if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
 
             if controller =~ %r{\A/}
-              raise ArgumentError, "controller name should not start with a slash"
-            end
-
-            if controller !~ /\A[a-z_0-9\/]*\z/
+              message = "controller name should not start with a slash"
+            else
               message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
               message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
-              raise ArgumentError, message
             end
+
+            raise ArgumentError, message
           end
 
           def blocks
-- 
cgit v1.2.3


From 3c03e7e2ab65a154bf82cbe697a9bddbecc20897 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:14:22 -0700
Subject: swtich to returning early if to responds to call

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 67 +++++++++++-------------
 1 file changed, 32 insertions(+), 35 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index b8aadff5e6..227ceec90f 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -237,47 +237,44 @@ module ActionDispatch
           end
 
           def default_controller_and_action
-            if to.respond_to?(:call)
-              { }
-            else
-              controller = default_controller
-              action     = default_action
-
-              case to
-              when Symbol
-                action = to.to_s
-              when /#/
-                controller, action = to.split('#')
-              when String
-                controller = to
-              end
-
-              if @scope[:module] && !controller.is_a?(Regexp)
-                if controller =~ %r{\A/}
-                  controller = controller[1..-1]
-                else
-                  controller = [@scope[:module], controller].compact.join("/").presence
-                end
-              end
-
-              hash = {}
+            hash = {}
+            return hash if to.respond_to? :call
+
+            controller = default_controller
+            action     = default_action
+
+            case to
+            when Symbol
+              action = to.to_s
+            when /#/
+              controller, action = to.split('#')
+            when String
+              controller = to
+            end
 
-              if controller.is_a? Regexp
-                hash[:controller] = controller
+            if @scope[:module] && !controller.is_a?(Regexp)
+              if controller =~ %r{\A/}
+                controller = controller[1..-1]
               else
-                check_controller! controller
-                hash[:controller] = controller.to_s if controller
+                controller = [@scope[:module], controller].compact.join("/").presence
               end
+            end
 
-              if action.is_a? Regexp
-                hash[:action] = action
-              else
-                check_action! action
-                hash[:action] = action.to_s if action
-              end
+            if controller.is_a? Regexp
+              hash[:controller] = controller
+            else
+              check_controller! controller
+              hash[:controller] = controller.to_s if controller
+            end
 
-              hash
+            if action.is_a? Regexp
+              hash[:action] = action
+            else
+              check_action! action
+              hash[:action] = action.to_s if action
             end
+
+            hash
           end
 
           def check_action!(action)
-- 
cgit v1.2.3


From bc916a7e58544c72516d36274e2709d9094a1f29 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:16:14 -0700
Subject: we don't need the call to presence. that is my present, to you!

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 227ceec90f..80c3b867bc 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -256,7 +256,7 @@ module ActionDispatch
               if controller =~ %r{\A/}
                 controller = controller[1..-1]
               else
-                controller = [@scope[:module], controller].compact.join("/").presence
+                controller = [@scope[:module], controller].compact.join("/")
               end
             end
 
-- 
cgit v1.2.3


From 8d309832df4a32f3bebbbc7ceae800876cef1174 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:23:17 -0700
Subject: extract controller and action parsing to a method

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 45 ++++++++++++++----------
 1 file changed, 26 insertions(+), 19 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 80c3b867bc..aaf5563dce 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -240,25 +240,12 @@ module ActionDispatch
             hash = {}
             return hash if to.respond_to? :call
 
-            controller = default_controller
-            action     = default_action
-
-            case to
-            when Symbol
-              action = to.to_s
-            when /#/
-              controller, action = to.split('#')
-            when String
-              controller = to
-            end
-
-            if @scope[:module] && !controller.is_a?(Regexp)
-              if controller =~ %r{\A/}
-                controller = controller[1..-1]
-              else
-                controller = [@scope[:module], controller].compact.join("/")
-              end
-            end
+            controller, action = get_controller_and_action(
+              default_controller,
+              default_action,
+              to,
+              @scope[:module]
+            )
 
             if controller.is_a? Regexp
               hash[:controller] = controller
@@ -277,6 +264,26 @@ module ActionDispatch
             hash
           end
 
+          def get_controller_and_action(controller, action, to, modyoule)
+            case to
+            when Symbol
+              action = to.to_s
+            when /#/
+              controller, action = to.split('#')
+            when String
+              controller = to
+            end
+
+            if modyoule && !controller.is_a?(Regexp)
+              if controller =~ %r{\A/}
+                controller = controller[1..-1]
+              else
+                controller = [modyoule, controller].compact.join("/")
+              end
+            end
+            [controller, action]
+          end
+
           def check_action!(action)
             unless action || segment_keys.include?(:action)
               message = "Missing :action key on routes definition, please check your routes."
-- 
cgit v1.2.3


From b27a3aff35bf2a7515d4236afe191a7c63481a23 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:28:38 -0700
Subject: only do one nil check against the controller

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index aaf5563dce..5027425d6b 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -250,8 +250,14 @@ module ActionDispatch
             if controller.is_a? Regexp
               hash[:controller] = controller
             else
-              check_controller! controller
-              hash[:controller] = controller.to_s if controller
+              if controller
+                hash[:controller] = check_controller!(controller).to_s
+              else
+                unless segment_keys.include?(:controller)
+                  message = "Missing :controller key on routes definition, please check your routes."
+                  raise ArgumentError, message
+                end
+              end
             end
 
             if action.is_a? Regexp
@@ -292,13 +298,7 @@ module ActionDispatch
           end
 
           def check_controller!(controller)
-            unless controller || segment_keys.include?(:controller)
-              message = "Missing :controller key on routes definition, please check your routes."
-              raise ArgumentError, message
-            end
-
-            return unless controller
-            return if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
+            return controller if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
 
             if controller =~ %r{\A/}
               message = "controller name should not start with a slash"
-- 
cgit v1.2.3


From a729f4050766f97750dd006217a175229b09da34 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:31:40 -0700
Subject: change to case / when on types

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 5027425d6b..72edf6725c 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -247,16 +247,15 @@ module ActionDispatch
               @scope[:module]
             )
 
-            if controller.is_a? Regexp
+            case controller
+            when Regexp
               hash[:controller] = controller
+            when String, Symbol
+              hash[:controller] = check_controller!(controller).to_s
             else
-              if controller
-                hash[:controller] = check_controller!(controller).to_s
-              else
-                unless segment_keys.include?(:controller)
-                  message = "Missing :controller key on routes definition, please check your routes."
-                  raise ArgumentError, message
-                end
+              unless segment_keys.include?(:controller)
+                message = "Missing :controller key on routes definition, please check your routes."
+                raise ArgumentError, message
               end
             end
 
-- 
cgit v1.2.3


From 309ff10d7d895ccb822b426dac3fb0a41e1da193 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:34:36 -0700
Subject: only one nil check on the action variable

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 72edf6725c..5ea694f0ff 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -259,11 +259,16 @@ module ActionDispatch
               end
             end
 
-            if action.is_a? Regexp
+            case action
+            when Regexp
               hash[:action] = action
+            when String, Symbol
+              hash[:action] = action.to_s
             else
-              check_action! action
-              hash[:action] = action.to_s if action
+              unless segment_keys.include?(:action)
+                message = "Missing :action key on routes definition, please check your routes."
+                raise ArgumentError, message
+              end
             end
 
             hash
@@ -289,13 +294,6 @@ module ActionDispatch
             [controller, action]
           end
 
-          def check_action!(action)
-            unless action || segment_keys.include?(:action)
-              message = "Missing :action key on routes definition, please check your routes."
-              raise ArgumentError, message
-            end
-          end
-
           def check_controller!(controller)
             return controller if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
 
-- 
cgit v1.2.3


From 78deb7f1b82f2b8fbe9aee4c0aa7f7557f4ff533 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:49:47 -0700
Subject: translate action / controller to the desired object

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 5ea694f0ff..d7e0d932fb 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -247,11 +247,8 @@ module ActionDispatch
               @scope[:module]
             )
 
-            case controller
-            when Regexp
-              hash[:controller] = controller
-            when String, Symbol
-              hash[:controller] = check_controller!(controller).to_s
+            if controller
+              hash[:controller] = translate_controller controller
             else
               unless segment_keys.include?(:controller)
                 message = "Missing :controller key on routes definition, please check your routes."
@@ -259,11 +256,8 @@ module ActionDispatch
               end
             end
 
-            case action
-            when Regexp
-              hash[:action] = action
-            when String, Symbol
-              hash[:action] = action.to_s
+            if action
+              hash[:action] = translate_action action
             else
               unless segment_keys.include?(:action)
                 message = "Missing :action key on routes definition, please check your routes."
@@ -294,8 +288,13 @@ module ActionDispatch
             [controller, action]
           end
 
-          def check_controller!(controller)
-            return controller if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
+          def translate_action(action)
+            Regexp === action ? action : action.to_s
+          end
+
+          def translate_controller(controller)
+            return controller if Regexp === controller
+            return controller.to_s if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
 
             if controller =~ %r{\A/}
               message = "controller name should not start with a slash"
-- 
cgit v1.2.3


From 89bf31ee565a0f5b1d7138deaa1873223ef22da2 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 16:59:32 -0700
Subject: move nil check to a method that yields to a block if the value is not
 nil

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 33 ++++++++++++------------
 1 file changed, 16 insertions(+), 17 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index d7e0d932fb..58e8ac0201 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -237,8 +237,7 @@ module ActionDispatch
           end
 
           def default_controller_and_action
-            hash = {}
-            return hash if to.respond_to? :call
+            return {} if to.respond_to? :call
 
             controller, action = get_controller_and_action(
               default_controller,
@@ -247,24 +246,28 @@ module ActionDispatch
               @scope[:module]
             )
 
-            if controller
-              hash[:controller] = translate_controller controller
-            else
-              unless segment_keys.include?(:controller)
-                message = "Missing :controller key on routes definition, please check your routes."
-                raise ArgumentError, message
+            hash = check_part(:controller, controller, {}) do
+              translate_controller controller
+            end
+
+            check_part(:action, action, hash) do
+              if Regexp === action
+                action
+              else
+                action.to_s
               end
             end
+          end
 
-            if action
-              hash[:action] = translate_action action
+          def check_part(name, part, hash)
+            if part
+              hash[name] = yield
             else
-              unless segment_keys.include?(:action)
-                message = "Missing :action key on routes definition, please check your routes."
+              unless segment_keys.include?(name)
+                message = "Missing :#{name} key on routes definition, please check your routes."
                 raise ArgumentError, message
               end
             end
-
             hash
           end
 
@@ -288,10 +291,6 @@ module ActionDispatch
             [controller, action]
           end
 
-          def translate_action(action)
-            Regexp === action ? action : action.to_s
-          end
-
           def translate_controller(controller)
             return controller if Regexp === controller
             return controller.to_s if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
-- 
cgit v1.2.3


From 6cabf1d19f90af0360d195df145ca5977704a61b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 17:05:54 -0700
Subject: add a test for controllers without colons

---
 actionpack/test/dispatch/routing_test.rb | 10 ++++++++++
 1 file changed, 10 insertions(+)

(limited to 'actionpack')

diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 20e79523f9..8249403a65 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3558,6 +3558,16 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest
 
     assert_match "'Admin::StorageFiles' is not a supported controller name", e.message
   end
+
+  def test_warn_with_ruby_constant_syntax_no_colons
+    e = assert_raise(ArgumentError) do
+      draw do
+        resources :storage_files, :controller => 'Admin'
+      end
+    end
+
+    assert_match "'Admin' is not a supported controller name", e.message
+  end
 end
 
 class TestDefaultScope < ActionDispatch::IntegrationTest
-- 
cgit v1.2.3


From 3d0dc81a078a738338a49124b7c6636a16319f83 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 17:28:59 -0700
Subject: only error handling between controller and action is the same

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 34 +++++++++++-------------
 1 file changed, 16 insertions(+), 18 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 58e8ac0201..ab2df76856 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -246,22 +246,27 @@ module ActionDispatch
               @scope[:module]
             )
 
-            hash = check_part(:controller, controller, {}) do
-              translate_controller controller
-            end
-
-            check_part(:action, action, hash) do
-              if Regexp === action
-                action
+            hash = check_part(:controller, controller, {}) do |part|
+              if part =~ %r{\A/}
+                message = "controller name should not start with a slash"
               else
-                action.to_s
+                message = "'#{part}' is not a supported controller name. This can lead to potential routing problems."
+                message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
               end
+
+              raise ArgumentError, message
             end
+
+            check_part(:action, action, hash) { |part|
+              # we never get here in the tests, but I believe this block should
+              # be the same as the `controller` block.
+              part.to_s
+            }
           end
 
           def check_part(name, part, hash)
             if part
-              hash[name] = yield
+              hash[name] = translate_part(name, part) { yield(part) }
             else
               unless segment_keys.include?(name)
                 message = "Missing :#{name} key on routes definition, please check your routes."
@@ -291,18 +296,11 @@ module ActionDispatch
             [controller, action]
           end
 
-          def translate_controller(controller)
+          def translate_part(name, controller)
             return controller if Regexp === controller
             return controller.to_s if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
 
-            if controller =~ %r{\A/}
-              message = "controller name should not start with a slash"
-            else
-              message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
-              message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
-            end
-
-            raise ArgumentError, message
+            yield
           end
 
           def blocks
-- 
cgit v1.2.3


From 75bfe647834bc9e46d6400c21cd8ffeb80cffd89 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 17:42:34 -0700
Subject: golf down a bit

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index ab2df76856..132bf4473e 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -239,8 +239,7 @@ module ActionDispatch
           def default_controller_and_action
             return {} if to.respond_to? :call
 
-            controller, action = get_controller_and_action(
-              default_controller,
+            controller, action = get_controller_and_action(default_controller,
               default_action,
               to,
               @scope[:module]
@@ -278,12 +277,9 @@ module ActionDispatch
 
           def get_controller_and_action(controller, action, to, modyoule)
             case to
-            when Symbol
-              action = to.to_s
-            when /#/
-              controller, action = to.split('#')
-            when String
-              controller = to
+            when Symbol then action = to.to_s
+            when /#/    then controller, action = to.split('#')
+            when String then controller = to
             end
 
             if modyoule && !controller.is_a?(Regexp)
-- 
cgit v1.2.3


From d311922c82ace9de7ca588b47a54716835bdca2a Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 18:03:26 -0700
Subject: only validate controllers

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 32 +++++++++++++-----------
 1 file changed, 17 insertions(+), 15 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 132bf4473e..899f0fdc1b 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -127,7 +127,11 @@ module ActionDispatch
               options[:controller] ||= /.+?/
             end
 
-            options.merge!(default_controller_and_action)
+            if to.respond_to? :call
+              options
+            else
+              options.merge!(default_controller_and_action)
+            end
           end
 
           def normalize_requirements!
@@ -237,8 +241,6 @@ module ActionDispatch
           end
 
           def default_controller_and_action
-            return {} if to.respond_to? :call
-
             controller, action = get_controller_and_action(default_controller,
               default_action,
               to,
@@ -246,26 +248,26 @@ module ActionDispatch
             )
 
             hash = check_part(:controller, controller, {}) do |part|
-              if part =~ %r{\A/}
-                message = "controller name should not start with a slash"
-              else
-                message = "'#{part}' is not a supported controller name. This can lead to potential routing problems."
-                message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
-              end
+              translate_controller(part) {
+                if part =~ %r{\A/}
+                  message = "controller name should not start with a slash"
+                else
+                  message = "'#{part}' is not a supported controller name. This can lead to potential routing problems."
+                  message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
+                end
 
-              raise ArgumentError, message
+                raise ArgumentError, message
+              }
             end
 
             check_part(:action, action, hash) { |part|
-              # we never get here in the tests, but I believe this block should
-              # be the same as the `controller` block.
-              part.to_s
+              part.is_a?(Regexp) ? part : part.to_s
             }
           end
 
           def check_part(name, part, hash)
             if part
-              hash[name] = translate_part(name, part) { yield(part) }
+              hash[name] = yield(part)
             else
               unless segment_keys.include?(name)
                 message = "Missing :#{name} key on routes definition, please check your routes."
@@ -292,7 +294,7 @@ module ActionDispatch
             [controller, action]
           end
 
-          def translate_part(name, controller)
+          def translate_controller(controller)
             return controller if Regexp === controller
             return controller.to_s if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/
 
-- 
cgit v1.2.3


From 7e61a327ce0fdb4b98a76d318ca0b5ffee0be7de Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 28 May 2014 18:07:25 -0700
Subject: controllers with slash names are also not supported, so we can reuse
 the message

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 899f0fdc1b..0a1f6afd77 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -249,12 +249,8 @@ module ActionDispatch
 
             hash = check_part(:controller, controller, {}) do |part|
               translate_controller(part) {
-                if part =~ %r{\A/}
-                  message = "controller name should not start with a slash"
-                else
-                  message = "'#{part}' is not a supported controller name. This can lead to potential routing problems."
-                  message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
-                end
+                message = "'#{part}' is not a supported controller name. This can lead to potential routing problems."
+                message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
 
                 raise ArgumentError, message
               }
-- 
cgit v1.2.3


From 8ed1a562c6293c81c894f3fe55fe88610c4f6caa Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 10:55:59 -0700
Subject: "controllers" should be a valid path name

---
 actionpack/lib/action_dispatch/routing/mapper.rb |  2 +-
 actionpack/test/dispatch/routing_test.rb         | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 0a1f6afd77..67e7285dcd 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -117,7 +117,7 @@ module ActionDispatch
               options[$1.to_sym] ||= /.+?/
             end
 
-            if path_without_format.match(':controller')
+            if path_pattern.names.map(&:to_sym).include?(:controller)
               raise ArgumentError, ":controller segment is not allowed within a namespace block" if scope[:module]
 
               # Add a default constraint for :controller path segments that matches namespaced
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 8249403a65..d6477e19bb 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -99,6 +99,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_namespace_without_controller_segment
+    draw do
+      namespace :admin do
+        get 'hello/:controllers/:action'
+      end
+    end
+    get '/admin/hello/foo/new'
+    assert_equal 'foo', @request.params["controllers"]
+  end
+
   def test_session_singleton_resource
     draw do
       resource :session do
-- 
cgit v1.2.3


From b5ea25bc44ceb07f1828b7b7f67bec700d96e216 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 14:44:56 -0700
Subject: pass the parsed parameters through the methods so we don't reparse or
 require caching code

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 72 +++++++++++-------------
 1 file changed, 34 insertions(+), 38 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 8e7951729b..503f70bd5f 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -76,10 +76,11 @@ module ActionDispatch
           @default_controller = options[:controller] || scope[:controller]
           @default_action     = options[:action] || scope[:action]
 
-          @options = normalize_options!(options)
-          normalize_path!
-          normalize_requirements!
-          normalize_conditions!
+          @path = normalize_path! @path, options[:format]
+          path_params = path_params @path
+          @options = normalize_options!(options, path_params)
+          normalize_requirements!(path_params)
+          normalize_conditions!(path_params)
           normalize_defaults!
         end
 
@@ -89,26 +90,24 @@ module ActionDispatch
 
         private
 
-          def normalize_path!
-            raise ArgumentError, "path is required" if @path.blank?
-            @path = Mapper.normalize_path(@path)
+          def normalize_path!(path, format)
+            raise ArgumentError, "path is required" if path.blank?
+            path = Mapper.normalize_path(path)
 
-            if required_format?
-              @path = "#{@path}.:format"
-            elsif optional_format?
-              @path = "#{@path}(.:format)"
+            if format == true
+              "#{path}.:format"
+            elsif optional_format?(path, format)
+              "#{path}(.:format)"
+            else
+              path
             end
           end
 
-          def required_format?
-            options[:format] == true
-          end
-
-          def optional_format?
-            options[:format] != false && !path.include?(':format') && !path.end_with?('/')
+          def optional_format?(path, format)
+            format != false && !path.include?(':format') && !path.end_with?('/')
           end
 
-          def normalize_options!(options)
+          def normalize_options!(options, path_params)
             path_without_format = path.sub(/\(\.:format\)$/, '')
 
             # Add a constraint for wildcard route to make it non-greedy and match the
@@ -117,7 +116,7 @@ module ActionDispatch
               options[$1.to_sym] ||= /.+?/
             end
 
-            if path_pattern.names.map(&:to_sym).include?(:controller)
+            if path_params.include?(:controller)
               raise ArgumentError, ":controller segment is not allowed within a namespace block" if scope[:module]
 
               # Add a default constraint for :controller path segments that matches namespaced
@@ -130,13 +129,13 @@ module ActionDispatch
             if to.respond_to? :call
               options
             else
-              options.merge!(default_controller_and_action)
+              options.merge!(default_controller_and_action(path_params))
             end
           end
 
-          def normalize_requirements!
+          def normalize_requirements!(path_params)
             constraints.each do |key, requirement|
-              next unless segment_keys.include?(key) || key == :controller
+              next unless path_params.include?(key) || key == :controller
               verify_regexp_requirement(requirement) if requirement.is_a?(Regexp)
               @requirements[key] = requirement
             end
@@ -193,18 +192,18 @@ module ActionDispatch
             end
           end
 
-          def normalize_conditions!
+          def normalize_conditions!(path_params)
             @conditions[:path_info] = path
 
             constraints.each do |key, condition|
-              unless segment_keys.include?(key) || key == :controller
+              unless path_params.include?(key) || key == :controller
                 @conditions[key] = condition
               end
             end
 
             required_defaults = []
             options.each do |key, required_default|
-              unless segment_keys.include?(key) || IGNORE_OPTIONS.include?(key) || Regexp === required_default
+              unless path_params.include?(key) || IGNORE_OPTIONS.include?(key) || Regexp === required_default
                 required_defaults << key
               end
             end
@@ -240,14 +239,14 @@ module ActionDispatch
             end
           end
 
-          def default_controller_and_action
+          def default_controller_and_action(path_params)
             controller, action = get_controller_and_action(default_controller,
               default_action,
               to,
               @scope[:module]
             )
 
-            hash = check_part(:controller, controller, {}) do |part|
+            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 << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
@@ -256,16 +255,16 @@ module ActionDispatch
               }
             end
 
-            check_part(:action, action, hash) { |part|
+            check_part(:action, action, path_params, hash) { |part|
               part.is_a?(Regexp) ? part : part.to_s
             }
           end
 
-          def check_part(name, part, hash)
+          def check_part(name, part, path_params, hash)
             if part
               hash[name] = yield(part)
             else
-              unless segment_keys.include?(name)
+              unless path_params.include?(name)
                 message = "Missing :#{name} key on routes definition, please check your routes."
                 raise ArgumentError, message
               end
@@ -317,16 +316,13 @@ module ActionDispatch
             end
           end
 
-          def segment_keys
-            @segment_keys ||= path_pattern.names.map{ |s| s.to_sym }
-          end
-
-          def path_pattern
-            Journey::Path::Pattern.new(strexp)
+          def path_params(path)
+            path_ast(path).grep(Journey::Nodes::Symbol).map { |n| n.name.to_sym }
           end
 
-          def strexp
-            Journey::Router::Strexp.compile(path, requirements, SEPARATORS)
+          def path_ast(path)
+            parser = Journey::Parser.new
+            parser.parse path
           end
 
           def dispatcher
-- 
cgit v1.2.3


From 3a102a58f4d33f9a8c660f617fe1242d7baa9f90 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 14:57:48 -0700
Subject: use a parser to extract the group parts from the path

---
 actionpack/lib/action_dispatch/journey/nodes/node.rb | 4 ++++
 actionpack/lib/action_dispatch/routing/mapper.rb     | 6 +++---
 actionpack/test/dispatch/mapper_test.rb              | 4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb
index 935442ef66..bb01c087bc 100644
--- a/actionpack/lib/action_dispatch/journey/nodes/node.rb
+++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb
@@ -93,6 +93,10 @@ module ActionDispatch
 
       class Star < Unary # :nodoc:
         def type; :STAR; end
+
+        def name
+          left.name.tr '*:', ''
+        end
       end
 
       class Binary < Node # :nodoc:
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 503f70bd5f..ca9f00c92e 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -108,12 +108,12 @@ module ActionDispatch
           end
 
           def normalize_options!(options, path_params)
-            path_without_format = path.sub(/\(\.:format\)$/, '')
+            wildcards = path_ast(path).grep(Journey::Nodes::Star).map(&:name)
 
             # Add a constraint for wildcard route to make it non-greedy and match the
             # optional format part of the route by default
-            if path_without_format.match(WILDCARD_PATH) && options[:format] != false
-              options[$1.to_sym] ||= /.+?/
+            if wildcards.any? && options[:format] != false
+              wildcards.each { |wc| options[wc.to_sym] ||= /.+?/ }
             end
 
             if path_params.include?(:controller)
diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb
index 58457b0c28..e3dcf9b88a 100644
--- a/actionpack/test/dispatch/mapper_test.rb
+++ b/actionpack/test/dispatch/mapper_test.rb
@@ -72,7 +72,7 @@ module ActionDispatch
         mapper = Mapper.new fakeset
         mapper.get '/*path/foo/:bar', :to => 'pages#show'
         assert_equal '/*path/foo/:bar(.:format)', fakeset.conditions.first[:path_info]
-        assert_nil fakeset.requirements.first[:path]
+        assert_equal(/.+?/, fakeset.requirements.first[:path])
       end
 
       def test_map_wildcard_with_multiple_wildcard
@@ -80,7 +80,7 @@ module ActionDispatch
         mapper = Mapper.new fakeset
         mapper.get '/*foo/*bar', :to => 'pages#show'
         assert_equal '/*foo/*bar(.:format)', fakeset.conditions.first[:path_info]
-        assert_nil fakeset.requirements.first[:foo]
+        assert_equal(/.+?/, fakeset.requirements.first[:foo])
         assert_equal(/.+?/, fakeset.requirements.first[:bar])
       end
 
-- 
cgit v1.2.3


From ffbe1b18c23b649c46e8f995cc1577ab6b7ead54 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:01:08 -0700
Subject: reuse the ast we already made

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index ca9f00c92e..63cd2169a5 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -77,8 +77,9 @@ module ActionDispatch
           @default_action     = options[:action] || scope[:action]
 
           @path = normalize_path! @path, options[:format]
-          path_params = path_params @path
-          @options = normalize_options!(options, path_params)
+          ast  = path_ast @path
+          path_params = path_params ast
+          @options = normalize_options!(options, path_params, ast)
           normalize_requirements!(path_params)
           normalize_conditions!(path_params)
           normalize_defaults!
@@ -107,13 +108,13 @@ module ActionDispatch
             format != false && !path.include?(':format') && !path.end_with?('/')
           end
 
-          def normalize_options!(options, path_params)
-            wildcards = path_ast(path).grep(Journey::Nodes::Star).map(&:name)
-
+          def normalize_options!(options, path_params, path_ast)
             # Add a constraint for wildcard route to make it non-greedy and match the
             # optional format part of the route by default
-            if wildcards.any? && options[:format] != false
-              wildcards.each { |wc| options[wc.to_sym] ||= /.+?/ }
+            if options[:format] != false
+              path_ast.grep(Journey::Nodes::Star) do |node|
+                options[node.name.to_sym] ||= /.+?/
+              end
             end
 
             if path_params.include?(:controller)
@@ -316,8 +317,8 @@ module ActionDispatch
             end
           end
 
-          def path_params(path)
-            path_ast(path).grep(Journey::Nodes::Symbol).map { |n| n.name.to_sym }
+          def path_params(ast)
+            ast.grep(Journey::Nodes::Symbol).map { |n| n.name.to_sym }
           end
 
           def path_ast(path)
-- 
cgit v1.2.3


From b3719d34d2f4db55dd622104ff77ac5970c42d89 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:05:22 -0700
Subject: disconnect path from the instance

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 63cd2169a5..1f9333cb65 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -64,11 +64,11 @@ module ActionDispatch
         ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
         WILDCARD_PATH = %r{\*([^/\)]+)\)?$}
 
-        attr_reader :scope, :path, :options, :requirements, :conditions, :defaults
+        attr_reader :scope, :options, :requirements, :conditions, :defaults
         attr_reader :to, :default_controller, :default_action
 
         def initialize(set, scope, path, options)
-          @set, @scope, @path = set, scope, path
+          @set, @scope = set, scope
           @requirements, @conditions, @defaults = {}, {}, {}
 
           options = scope[:options].merge(options) if scope[:options]
@@ -76,12 +76,12 @@ module ActionDispatch
           @default_controller = options[:controller] || scope[:controller]
           @default_action     = options[:action] || scope[:action]
 
-          @path = normalize_path! @path, options[:format]
-          ast  = path_ast @path
+          path = normalize_path! path, options[:format]
+          ast  = path_ast path
           path_params = path_params ast
           @options = normalize_options!(options, path_params, ast)
           normalize_requirements!(path_params)
-          normalize_conditions!(path_params)
+          normalize_conditions!(path_params, path)
           normalize_defaults!
         end
 
@@ -193,7 +193,7 @@ module ActionDispatch
             end
           end
 
-          def normalize_conditions!(path_params)
+          def normalize_conditions!(path_params, path)
             @conditions[:path_info] = path
 
             constraints.each do |key, condition|
-- 
cgit v1.2.3


From 7da98d0a590d74027e6595da8a85ea3b4195c51c Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:08:12 -0700
Subject: remove dead code

---
 actionpack/lib/action_dispatch/routing/mapper.rb | 1 -
 1 file changed, 1 deletion(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 1f9333cb65..38d6d6539a 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -62,7 +62,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}
-        WILDCARD_PATH = %r{\*([^/\)]+)\)?$}
 
         attr_reader :scope, :options, :requirements, :conditions, :defaults
         attr_reader :to, :default_controller, :default_action
-- 
cgit v1.2.3


From eabe504cdfa11b580c614d6bd501eb7cc60f485d Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:16:40 -0700
Subject: ask the strexp for the ast

---
 actionpack/lib/action_dispatch/journey/path/pattern.rb  | 5 ++---
 actionpack/lib/action_dispatch/journey/router/strexp.rb | 5 +++++
 2 files changed, 7 insertions(+), 3 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb
index cb0a02c298..da9d54cf6a 100644
--- a/actionpack/lib/action_dispatch/journey/path/pattern.rb
+++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb
@@ -5,17 +5,16 @@ module ActionDispatch
         attr_reader :spec, :requirements, :anchored
 
         def initialize(strexp)
-          parser = Journey::Parser.new
-
           @anchored = true
 
           case strexp
           when String
+            parser        = Journey::Parser.new
             @spec         = parser.parse(strexp)
             @requirements = {}
             @separators   = "/.?"
           when Router::Strexp
-            @spec         = parser.parse(strexp.path)
+            @spec         = strexp.ast
             @requirements = strexp.requirements
             @separators   = strexp.separators.join
             @anchored     = strexp.anchor
diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb
index f97f1a223e..161b41d6b2 100644
--- a/actionpack/lib/action_dispatch/journey/router/strexp.rb
+++ b/actionpack/lib/action_dispatch/journey/router/strexp.rb
@@ -15,6 +15,11 @@ module ActionDispatch
           @anchor       = anchor
         end
 
+        def ast
+          parser = Journey::Parser.new
+          parser.parse path
+        end
+
         def names
           @path.scan(/:\w+/).map { |s| s.tr(':', '') }
         end
-- 
cgit v1.2.3


From 15ffbedf3bb5907c3774fcc38466c799e92f9a04 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:23:30 -0700
Subject: add an alternate constructor to Strexp that takes a string

---
 .../lib/action_dispatch/journey/router/strexp.rb   | 16 ++++++-----
 .../lib/action_dispatch/routing/route_set.rb       |  2 +-
 actionpack/test/journey/path/pattern_test.rb       | 24 ++++++++--------
 actionpack/test/journey/router/strexp_test.rb      |  4 +--
 actionpack/test/journey/router_test.rb             | 32 +++++++++++-----------
 actionpack/test/journey/routes_test.rb             |  2 +-
 6 files changed, 41 insertions(+), 39 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb
index 161b41d6b2..29196d20bb 100644
--- a/actionpack/lib/action_dispatch/journey/router/strexp.rb
+++ b/actionpack/lib/action_dispatch/journey/router/strexp.rb
@@ -6,20 +6,22 @@ module ActionDispatch
           alias :compile :new
         end
 
-        attr_reader :path, :requirements, :separators, :anchor
+        attr_reader :path, :requirements, :separators, :anchor, :ast
 
-        def initialize(path, requirements, separators, anchor = true)
+        def self.build(path, requirements, separators, anchor = true)
+          parser = Journey::Parser.new
+          ast = parser.parse path
+          new ast, path, requirements, separators, anchor
+        end
+
+        def initialize(ast, path, requirements, separators, anchor = true)
+          @ast          = ast
           @path         = path
           @requirements = requirements
           @separators   = separators
           @anchor       = anchor
         end
 
-        def ast
-          parser = Journey::Parser.new
-          parser.parse path
-        end
-
         def names
           @path.scan(/:\w+/).map { |s| s.tr(':', '') }
         end
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 3ca5abf0fd..66d1805702 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -427,7 +427,7 @@ module ActionDispatch
       end
 
       def build_path(path, requirements, separators, anchor)
-        strexp = Journey::Router::Strexp.new(
+        strexp = Journey::Router::Strexp.build(
             path,
             requirements,
             SEPARATORS,
diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb
index ce02104181..dfb20d2977 100644
--- a/actionpack/test/journey/path/pattern_test.rb
+++ b/actionpack/test/journey/path/pattern_test.rb
@@ -18,7 +18,7 @@ module ActionDispatch
           '/:controller/*foo/bar'        => %r{\A/(#{x})/(.+)/bar\Z},
         }.each do |path, expected|
           define_method(:"test_to_regexp_#{path}") do
-            strexp = Router::Strexp.new(
+            strexp = Router::Strexp.build(
               path,
               { :controller => /.+/ },
               ["/", ".", "?"]
@@ -41,7 +41,7 @@ module ActionDispatch
           '/:controller/*foo/bar'        => %r{\A/(#{x})/(.+)/bar},
         }.each do |path, expected|
           define_method(:"test_to_non_anchored_regexp_#{path}") do
-            strexp = Router::Strexp.new(
+            strexp = Router::Strexp.build(
               path,
               { :controller => /.+/ },
               ["/", ".", "?"],
@@ -65,7 +65,7 @@ module ActionDispatch
           '/:controller/*foo/bar'        => %w{ controller foo },
         }.each do |path, expected|
           define_method(:"test_names_#{path}") do
-            strexp = Router::Strexp.new(
+            strexp = Router::Strexp.build(
               path,
               { :controller => /.+/ },
               ["/", ".", "?"]
@@ -80,7 +80,7 @@ module ActionDispatch
         end
 
         def test_to_regexp_with_extended_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name',
             { :name => /
               #ROFL
@@ -107,7 +107,7 @@ module ActionDispatch
         end
 
         def test_to_regexp_match_non_optional
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/:name',
             { :name => /\d+/ },
             ["/", ".", "?"]
@@ -118,7 +118,7 @@ module ActionDispatch
         end
 
         def test_to_regexp_with_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name',
             { :name => /(tender|love)/ },
             ["/", ".", "?"]
@@ -131,7 +131,7 @@ module ActionDispatch
 
         def test_ast_sets_regular_expressions
           requirements = { :name => /(tender|love)/, :value => /./ }
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name/:value',
             requirements,
             ["/", ".", "?"]
@@ -148,7 +148,7 @@ module ActionDispatch
         end
 
         def test_match_data_with_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name',
             { :name => /(tender|love)/ },
             ["/", ".", "?"]
@@ -160,7 +160,7 @@ module ActionDispatch
         end
 
         def test_match_data_with_multi_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name/:id',
             { :name => /t(((ender|love)))()/ },
             ["/", ".", "?"]
@@ -175,7 +175,7 @@ module ActionDispatch
 
         def test_star_with_custom_re
           z = /\d+/
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/*foo',
             { :foo => z },
             ["/", ".", "?"]
@@ -185,7 +185,7 @@ module ActionDispatch
         end
 
         def test_insensitive_regexp_with_group
-          strexp = Router::Strexp.new(
+          strexp = Router::Strexp.build(
             '/page/:name/aaron',
             { :name => /(tender|love)/i },
             ["/", ".", "?"]
@@ -197,7 +197,7 @@ module ActionDispatch
         end
 
         def test_to_regexp_with_strexp
-          strexp = Router::Strexp.new('/:controller', { }, ["/", ".", "?"])
+          strexp = Router::Strexp.build('/:controller', { }, ["/", ".", "?"])
           path = Pattern.new strexp
           x = %r{\A/([^/.?]+)\Z}
 
diff --git a/actionpack/test/journey/router/strexp_test.rb b/actionpack/test/journey/router/strexp_test.rb
index 7ccdfb7b4d..9cb333757c 100644
--- a/actionpack/test/journey/router/strexp_test.rb
+++ b/actionpack/test/journey/router/strexp_test.rb
@@ -5,7 +5,7 @@ module ActionDispatch
     class Router
       class TestStrexp < ActiveSupport::TestCase
         def test_many_names
-          exp = Strexp.new(
+          exp = Strexp.build(
             "/:controller(/:action(/:id(.:format)))",
             {:controller=>/.+?/},
             ["/", ".", "?"],
@@ -22,7 +22,7 @@ module ActionDispatch
             ":format0"          => %w{ format0 },
             ":format1,:format2" => %w{ format1 format2 },
           }.each do |string, expected|
-            exp = Strexp.new(string, {}, ["/", ".", "?"])
+            exp = Strexp.build(string, {}, ["/", ".", "?"])
             assert_equal expected, exp.names
           end
         end
diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb
index f298808be8..48d1e78b96 100644
--- a/actionpack/test/journey/router_test.rb
+++ b/actionpack/test/journey/router_test.rb
@@ -32,7 +32,7 @@ module ActionDispatch
       def test_dashes
         router = Router.new(routes)
 
-        exp = Router::Strexp.new '/foo-bar-baz', {}, ['/.?']
+        exp = Router::Strexp.build '/foo-bar-baz', {}, ['/.?']
         path  = Path::Pattern.new exp
 
         routes.add_route nil, path, {}, {:id => nil}, {}
@@ -49,7 +49,7 @@ module ActionDispatch
         router = Router.new(routes)
 
         #match the escaped version of /ほげ
-        exp = Router::Strexp.new '/%E3%81%BB%E3%81%92', {}, ['/.?']
+        exp = Router::Strexp.build '/%E3%81%BB%E3%81%92', {}, ['/.?']
         path  = Path::Pattern.new exp
 
         routes.add_route nil, path, {}, {:id => nil}, {}
@@ -68,7 +68,7 @@ module ActionDispatch
 
         requirements = { :hello => /world/ }
 
-        exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?']
+        exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?']
         path  = Path::Pattern.new exp
 
         routes.add_route nil, path, requirements, {:id => nil}, {}
@@ -88,7 +88,7 @@ module ActionDispatch
 
         requirements = { :hello => /mom/ }
 
-        exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?']
+        exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?']
         path  = Path::Pattern.new exp
 
         router.routes.add_route nil, path, requirements, {:id => nil}, {}
@@ -115,7 +115,7 @@ module ActionDispatch
       def test_request_class_overrides_path_info
         router = Router.new(routes)
 
-        exp = Router::Strexp.new '/bar', {}, ['/.?']
+        exp = Router::Strexp.build '/bar', {}, ['/.?']
         path = Path::Pattern.new exp
 
         routes.add_route nil, path, {}, {}, {}
@@ -133,8 +133,8 @@ module ActionDispatch
 
       def test_regexp_first_precedence
         add_routes @router, [
-          Router::Strexp.new("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']),
-          Router::Strexp.new("/whois/:id(.:format)", {}, ['/', '.', '?'])
+          Router::Strexp.build("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']),
+          Router::Strexp.build("/whois/:id(.:format)", {}, ['/', '.', '?'])
         ]
 
         env = rails_env 'PATH_INFO' => '/whois/example.com'
@@ -152,7 +152,7 @@ module ActionDispatch
 
       def test_required_parts_verified_are_anchored
         add_routes @router, [
-          Router::Strexp.new("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false)
+          Router::Strexp.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false)
         ]
 
         assert_raises(ActionController::UrlGenerationError) do
@@ -162,7 +162,7 @@ module ActionDispatch
 
       def test_required_parts_are_verified_when_building
         add_routes @router, [
-          Router::Strexp.new("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
+          Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
         ]
 
         path, _ = @formatter.generate(nil, { :id => '10' }, { })
@@ -175,7 +175,7 @@ module ActionDispatch
 
       def test_only_required_parts_are_verified
         add_routes @router, [
-          Router::Strexp.new("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false)
+          Router::Strexp.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false)
         ]
 
         path, _ = @formatter.generate(nil, { :id => '10' }, { })
@@ -190,7 +190,7 @@ module ActionDispatch
 
       def test_knows_what_parts_are_missing_from_named_route
         route_name = "gorby_thunderhorse"
-        pattern = Router::Strexp.new("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
+        pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
         path = Path::Pattern.new pattern
         @router.routes.add_route nil, path, {}, {}, route_name
 
@@ -213,7 +213,7 @@ module ActionDispatch
         route_set = Routing::RouteSet.new
         mapper = Routing::Mapper.new route_set
 
-        strexp = Router::Strexp.new("/", {}, ['/', '.', '?'], false)
+        strexp = Router::Strexp.build("/", {}, ['/', '.', '?'], false)
         path   = Path::Pattern.new strexp
         app    = lambda { |env| [200, {}, ['success!']] }
         mapper.get '/weblog', :to => app
@@ -241,7 +241,7 @@ module ActionDispatch
 
       def test_recognize_with_unbound_regexp
         add_routes @router, [
-          Router::Strexp.new("/foo", { }, ['/', '.', '?'], false)
+          Router::Strexp.build("/foo", { }, ['/', '.', '?'], false)
         ]
 
         env = rails_env 'PATH_INFO' => '/foo/bar'
@@ -254,7 +254,7 @@ module ActionDispatch
 
       def test_bound_regexp_keeps_path_info
         add_routes @router, [
-          Router::Strexp.new("/foo", { }, ['/', '.', '?'], true)
+          Router::Strexp.build("/foo", { }, ['/', '.', '?'], true)
         ]
 
         env = rails_env 'PATH_INFO' => '/foo'
@@ -321,7 +321,7 @@ module ActionDispatch
       def test_generate_slash
         params = [ [:controller, "tasks"],
                    [:action, "show"] ]
-        str = Router::Strexp.new("/", Hash[params], ['/', '.', '?'], true)
+        str = Router::Strexp.build("/", Hash[params], ['/', '.', '?'], true)
         path  = Path::Pattern.new str
 
         @router.routes.add_route @app, path, {}, {}, {}
@@ -463,7 +463,7 @@ module ActionDispatch
       end
 
       def test_namespaced_controller
-        strexp = Router::Strexp.new(
+        strexp = Router::Strexp.build(
           "/:controller(/:action(/:id))",
           { :controller => /.+?/ },
           ["/", ".", "?"]
diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb
index 25e0321d31..2103cbb9b8 100644
--- a/actionpack/test/journey/routes_test.rb
+++ b/actionpack/test/journey/routes_test.rb
@@ -5,7 +5,7 @@ module ActionDispatch
     class TestRoutes < ActiveSupport::TestCase
       def test_clear
         routes = Routes.new
-        exp    = Router::Strexp.new '/foo(/:id)', {}, ['/.?']
+        exp    = Router::Strexp.build '/foo(/:id)', {}, ['/.?']
         path   = Path::Pattern.new exp
         requirements = { :hello => /world/ }
 
-- 
cgit v1.2.3


From 333a4d09ab8b024b3d23618e9d668bef2bab1355 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:27:46 -0700
Subject: pass the parsed path from mapper to the Strexp

---
 actionpack/lib/action_dispatch/routing/mapper.rb    | 5 +++--
 actionpack/lib/action_dispatch/routing/route_set.rb | 9 ++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 38d6d6539a..879e8daa33 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -80,7 +80,7 @@ module ActionDispatch
           path_params = path_params ast
           @options = normalize_options!(options, path_params, ast)
           normalize_requirements!(path_params)
-          normalize_conditions!(path_params, path)
+          normalize_conditions!(path_params, path, ast)
           normalize_defaults!
         end
 
@@ -192,8 +192,9 @@ module ActionDispatch
             end
           end
 
-          def normalize_conditions!(path_params, path)
+          def normalize_conditions!(path_params, path, ast)
             @conditions[:path_info] = path
+            @conditions[:parsed_path_info] = ast
 
             constraints.each do |key, condition|
               unless path_params.include?(key) || key == :controller
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 66d1805702..bdda802195 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -418,7 +418,9 @@ module ActionDispatch
             "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created"
         end
 
-        path = build_path(conditions.delete(:path_info), requirements, SEPARATORS, anchor)
+        path = conditions.delete :path_info
+        ast  = conditions.delete :parsed_path_info
+        path = build_path(path, ast, requirements, SEPARATORS, anchor)
         conditions = build_conditions(conditions, path.names.map { |x| x.to_sym })
 
         route = @set.add_route(app, path, conditions, defaults, name)
@@ -426,8 +428,9 @@ module ActionDispatch
         route
       end
 
-      def build_path(path, requirements, separators, anchor)
-        strexp = Journey::Router::Strexp.build(
+      def build_path(path, ast, requirements, separators, anchor)
+        strexp = Journey::Router::Strexp.new(
+            ast,
             path,
             requirements,
             SEPARATORS,
-- 
cgit v1.2.3


From 5682596db740d53e1deaf9244fcc004ccffe1fb4 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:31:45 -0700
Subject: Strexp#names is only used in a test, so rm

---
 .../lib/action_dispatch/journey/router/strexp.rb   |  4 ---
 actionpack/test/journey/router/strexp_test.rb      | 32 ----------------------
 2 files changed, 36 deletions(-)
 delete mode 100644 actionpack/test/journey/router/strexp_test.rb

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb
index 29196d20bb..4b7738f335 100644
--- a/actionpack/lib/action_dispatch/journey/router/strexp.rb
+++ b/actionpack/lib/action_dispatch/journey/router/strexp.rb
@@ -21,10 +21,6 @@ module ActionDispatch
           @separators   = separators
           @anchor       = anchor
         end
-
-        def names
-          @path.scan(/:\w+/).map { |s| s.tr(':', '') }
-        end
       end
     end
   end
diff --git a/actionpack/test/journey/router/strexp_test.rb b/actionpack/test/journey/router/strexp_test.rb
deleted file mode 100644
index 9cb333757c..0000000000
--- a/actionpack/test/journey/router/strexp_test.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-require 'abstract_unit'
-
-module ActionDispatch
-  module Journey
-    class Router
-      class TestStrexp < ActiveSupport::TestCase
-        def test_many_names
-          exp = Strexp.build(
-            "/:controller(/:action(/:id(.:format)))",
-            {:controller=>/.+?/},
-            ["/", ".", "?"],
-            true)
-
-          assert_equal ["controller", "action", "id", "format"], exp.names
-        end
-
-        def test_names
-          {
-            "/bar(.:format)"    => %w{ format },
-            ":format"           => %w{ format },
-            ":format-"          => %w{ format },
-            ":format0"          => %w{ format0 },
-            ":format1,:format2" => %w{ format1 format2 },
-          }.each do |string, expected|
-            exp = Strexp.build(string, {}, ["/", ".", "?"])
-            assert_equal expected, exp.names
-          end
-        end
-      end
-    end
-  end
-end
-- 
cgit v1.2.3


From bb207ea7b6df6101860d5de6e47ad0e0f1ea2cdf Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:44:54 -0700
Subject: Path::Pattern is instantiated internally, so make the contructor
 require a strexp object

---
 .../lib/action_dispatch/journey/path/pattern.rb    | 13 ++++-----
 actionpack/test/journey/path/pattern_test.rb       | 18 ++++++------
 actionpack/test/journey/route_test.rb              | 22 +++++++-------
 actionpack/test/journey/router_test.rb             | 34 ++++++++++++----------
 actionpack/test/journey/routes_test.rb             |  8 ++---
 5 files changed, 49 insertions(+), 46 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb
index da9d54cf6a..c39f8e850a 100644
--- a/actionpack/lib/action_dispatch/journey/path/pattern.rb
+++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb
@@ -1,18 +1,17 @@
+require 'action_dispatch/journey/router/strexp'
+
 module ActionDispatch
   module Journey # :nodoc:
     module Path # :nodoc:
       class Pattern # :nodoc:
         attr_reader :spec, :requirements, :anchored
 
-        def initialize(strexp)
-          @anchored = true
+        def self.from_string string
+          new Journey::Router::Strexp.build(string, {}, ["/.?"], true)
+        end
 
+        def initialize(strexp)
           case strexp
-          when String
-            parser        = Journey::Parser.new
-            @spec         = parser.parse(strexp)
-            @requirements = {}
-            @separators   = "/.?"
           when Router::Strexp
             @spec         = strexp.ast
             @requirements = strexp.requirements
diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb
index dfb20d2977..cdb3cdad7b 100644
--- a/actionpack/test/journey/path/pattern_test.rb
+++ b/actionpack/test/journey/path/pattern_test.rb
@@ -101,7 +101,7 @@ module ActionDispatch
             ['/:foo(/:bar)', %w{ bar }],
             ['/:foo(/:bar)/:lol(/:baz)', %w{ bar baz }],
           ].each do |pattern, list|
-            path = Pattern.new pattern
+            path = Pattern.from_string pattern
             assert_equal list.sort, path.optional_names.sort
           end
         end
@@ -205,20 +205,20 @@ module ActionDispatch
         end
 
         def test_to_regexp_defaults
-          path = Pattern.new '/:controller(/:action(/:id))'
+          path = Pattern.from_string '/:controller(/:action(/:id))'
           expected = %r{\A/([^/.?]+)(?:/([^/.?]+)(?:/([^/.?]+))?)?\Z}
           assert_equal expected, path.to_regexp
         end
 
         def test_failed_match
-          path = Pattern.new '/:controller(/:action(/:id(.:format)))'
+          path = Pattern.from_string '/:controller(/:action(/:id(.:format)))'
           uri = 'content'
 
           assert_not path =~ uri
         end
 
         def test_match_controller
-          path = Pattern.new '/:controller(/:action(/:id(.:format)))'
+          path = Pattern.from_string '/:controller(/:action(/:id(.:format)))'
           uri = '/content'
 
           match = path =~ uri
@@ -230,7 +230,7 @@ module ActionDispatch
         end
 
         def test_match_controller_action
-          path = Pattern.new '/:controller(/:action(/:id(.:format)))'
+          path = Pattern.from_string '/:controller(/:action(/:id(.:format)))'
           uri = '/content/list'
 
           match = path =~ uri
@@ -242,7 +242,7 @@ module ActionDispatch
         end
 
         def test_match_controller_action_id
-          path = Pattern.new '/:controller(/:action(/:id(.:format)))'
+          path = Pattern.from_string '/:controller(/:action(/:id(.:format)))'
           uri = '/content/list/10'
 
           match = path =~ uri
@@ -254,7 +254,7 @@ module ActionDispatch
         end
 
         def test_match_literal
-          path = Path::Pattern.new "/books(/:action(.:format))"
+          path = Path::Pattern.from_string "/books(/:action(.:format))"
 
           uri = '/books'
           match = path =~ uri
@@ -264,7 +264,7 @@ module ActionDispatch
         end
 
         def test_match_literal_with_action
-          path = Path::Pattern.new "/books(/:action(.:format))"
+          path = Path::Pattern.from_string "/books(/:action(.:format))"
 
           uri = '/books/list'
           match = path =~ uri
@@ -274,7 +274,7 @@ module ActionDispatch
         end
 
         def test_match_literal_with_action_and_format
-          path = Path::Pattern.new "/books(/:action(.:format))"
+          path = Path::Pattern.from_string "/books(/:action(.:format))"
 
           uri = '/books/list.rss'
           match = path =~ uri
diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb
index cbe6284714..21d867aca0 100644
--- a/actionpack/test/journey/route_test.rb
+++ b/actionpack/test/journey/route_test.rb
@@ -5,7 +5,7 @@ module ActionDispatch
     class TestRoute < ActiveSupport::TestCase
       def test_initialize
         app      = Object.new
-        path     = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
+        path     = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))'
         defaults = {}
         route    = Route.new("name", app, path, {}, defaults)
 
@@ -16,7 +16,7 @@ module ActionDispatch
 
       def test_route_adds_itself_as_memo
         app      = Object.new
-        path     = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
+        path     = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))'
         defaults = {}
         route    = Route.new("name", app, path, {}, defaults)
 
@@ -26,21 +26,21 @@ module ActionDispatch
       end
 
       def test_ip_address
-        path  = Path::Pattern.new '/messages/:id(.:format)'
+        path  = Path::Pattern.from_string '/messages/:id(.:format)'
         route = Route.new("name", nil, path, {:ip => '192.168.1.1'},
                           { :controller => 'foo', :action => 'bar' })
         assert_equal '192.168.1.1', route.ip
       end
 
       def test_default_ip
-        path  = Path::Pattern.new '/messages/:id(.:format)'
+        path  = Path::Pattern.from_string '/messages/:id(.:format)'
         route = Route.new("name", nil, path, {},
                           { :controller => 'foo', :action => 'bar' })
         assert_equal(//, route.ip)
       end
 
       def test_format_with_star
-        path  = Path::Pattern.new '/:controller/*extra'
+        path  = Path::Pattern.from_string '/:controller/*extra'
         route = Route.new("name", nil, path, {},
                           { :controller => 'foo', :action => 'bar' })
         assert_equal '/foo/himom', route.format({
@@ -50,7 +50,7 @@ module ActionDispatch
       end
 
       def test_connects_all_match
-        path  = Path::Pattern.new '/:controller(/:action(/:id(.:format)))'
+        path  = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))'
         route = Route.new("name", nil, path, {:action => 'bar'}, { :controller => 'foo' })
 
         assert_equal '/foo/bar/10', route.format({
@@ -61,21 +61,21 @@ module ActionDispatch
       end
 
       def test_extras_are_not_included_if_optional
-        path  = Path::Pattern.new '/page/:id(/:action)'
+        path  = Path::Pattern.from_string '/page/:id(/:action)'
         route = Route.new("name", nil, path, { }, { :action => 'show' })
 
         assert_equal '/page/10', route.format({ :id => 10 })
       end
 
       def test_extras_are_not_included_if_optional_with_parameter
-        path  = Path::Pattern.new '(/sections/:section)/pages/:id'
+        path  = Path::Pattern.from_string '(/sections/:section)/pages/:id'
         route = Route.new("name", nil, path, { }, { :action => 'show' })
 
         assert_equal '/pages/10', route.format({:id => 10})
       end
 
       def test_extras_are_not_included_if_optional_parameter_is_nil
-        path  = Path::Pattern.new '(/sections/:section)/pages/:id'
+        path  = Path::Pattern.from_string '(/sections/:section)/pages/:id'
         route = Route.new("name", nil, path, { }, { :action => 'show' })
 
         assert_equal '/pages/10', route.format({:id => 10, :section => nil})
@@ -85,10 +85,10 @@ module ActionDispatch
         constraints = {:required_defaults => [:controller, :action]}
         defaults = {:controller=>"pages", :action=>"show"}
 
-        path = Path::Pattern.new "/page/:id(/:action)(.:format)"
+        path = Path::Pattern.from_string "/page/:id(/:action)(.:format)"
         specific = Route.new "name", nil, path, constraints, defaults
 
-        path = Path::Pattern.new "/:controller(/:action(/:id))(.:format)"
+        path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)"
         generic = Route.new "name", nil, path, constraints
 
         knowledge = {:id=>20, :controller=>"pages", :action=>"show"}
diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb
index 48d1e78b96..e092432b01 100644
--- a/actionpack/test/journey/router_test.rb
+++ b/actionpack/test/journey/router_test.rb
@@ -225,7 +225,7 @@ module ActionDispatch
       end
 
       def test_defaults_merge_correctly
-        path  = Path::Pattern.new '/foo(/:id)'
+        path  = Path::Pattern.from_string '/foo(/:id)'
         @router.routes.add_route nil, path, {}, {:id => nil}, {}
 
         env = rails_env 'PATH_INFO' => '/foo/10'
@@ -308,7 +308,7 @@ module ActionDispatch
       end
 
       def test_nil_path_parts_are_ignored
-        path  = Path::Pattern.new "/:controller(/:action(.:format))"
+        path  = Path::Pattern.from_string "/:controller(/:action(.:format))"
         @router.routes.add_route @app, path, {}, {}, {}
 
         params = { :controller => "tasks", :format => nil }
@@ -331,7 +331,7 @@ module ActionDispatch
       end
 
       def test_generate_calls_param_proc
-        path  = Path::Pattern.new '/:controller(/:action)'
+        path  = Path::Pattern.from_string '/:controller(/:action)'
         @router.routes.add_route @app, path, {}, {}, {}
 
         parameterized = []
@@ -348,7 +348,7 @@ module ActionDispatch
       end
 
       def test_generate_id
-        path  = Path::Pattern.new '/:controller(/:action)'
+        path  = Path::Pattern.from_string '/:controller(/:action)'
         @router.routes.add_route @app, path, {}, {}, {}
 
         path, params = @formatter.generate(
@@ -358,7 +358,7 @@ module ActionDispatch
       end
 
       def test_generate_escapes
-        path  = Path::Pattern.new '/:controller(/:action)'
+        path  = Path::Pattern.from_string '/:controller(/:action)'
         @router.routes.add_route @app, path, {}, {}, {}
 
         path, _ = @formatter.generate(nil,
@@ -369,7 +369,7 @@ module ActionDispatch
       end
 
       def test_generate_escapes_with_namespaced_controller
-        path  = Path::Pattern.new '/:controller(/:action)'
+        path  = Path::Pattern.from_string '/:controller(/:action)'
         @router.routes.add_route @app, path, {}, {}, {}
 
         path, _ = @formatter.generate(
@@ -380,7 +380,7 @@ module ActionDispatch
       end
 
       def test_generate_extra_params
-        path  = Path::Pattern.new '/:controller(/:action)'
+        path  = Path::Pattern.from_string '/:controller(/:action)'
         @router.routes.add_route @app, path, {}, {}, {}
 
         path, params = @formatter.generate(
@@ -394,7 +394,7 @@ module ActionDispatch
       end
 
       def test_generate_uses_recall_if_needed
-        path  = Path::Pattern.new '/:controller(/:action(/:id))'
+        path  = Path::Pattern.from_string '/:controller(/:action(/:id))'
         @router.routes.add_route @app, path, {}, {}, {}
 
         path, params = @formatter.generate(
@@ -406,7 +406,7 @@ module ActionDispatch
       end
 
       def test_generate_with_name
-        path  = Path::Pattern.new '/:controller(/:action)'
+        path  = Path::Pattern.from_string '/:controller(/:action)'
         @router.routes.add_route @app, path, {}, {}, {}
 
         path, params = @formatter.generate(
@@ -423,7 +423,7 @@ module ActionDispatch
         '/content/show/10'  => { :controller => 'content', :action => 'show', :id => "10" },
       }.each do |request_path, expected|
         define_method("test_recognize_#{expected.keys.map(&:to_s).join('_')}") do
-          path  = Path::Pattern.new "/:controller(/:action(/:id))"
+          path  = Path::Pattern.from_string "/:controller(/:action(/:id))"
           app   = Object.new
           route = @router.routes.add_route(app, path, {}, {}, {})
 
@@ -445,7 +445,7 @@ module ActionDispatch
         :splat   => ['/segment/a/b%20c+d', { :segment => 'segment', :splat => 'a/b c+d' }]
       }.each do |name, (request_path, expected)|
         define_method("test_recognize_#{name}") do
-          path  = Path::Pattern.new '/:segment/*splat'
+          path  = Path::Pattern.from_string '/:segment/*splat'
           app   = Object.new
           route = @router.routes.add_route(app, path, {}, {}, {})
 
@@ -489,7 +489,7 @@ module ActionDispatch
       end
 
       def test_recognize_literal
-        path   = Path::Pattern.new "/books(/:action(.:format))"
+        path   = Path::Pattern.from_string "/books(/:action(.:format))"
         app    = Object.new
         route  = @router.routes.add_route(app, path, {}, {:controller => 'books'})
 
@@ -506,7 +506,7 @@ module ActionDispatch
       end
 
       def test_recognize_head_request_as_get_route
-        path   = Path::Pattern.new "/books(/:action(.:format))"
+        path   = Path::Pattern.from_string "/books(/:action(.:format))"
         app    = Object.new
         conditions = {
           :request_method => 'GET'
@@ -525,7 +525,7 @@ module ActionDispatch
       end
 
       def test_recognize_cares_about_verbs
-        path   = Path::Pattern.new "/books(/:action(.:format))"
+        path   = Path::Pattern.from_string "/books(/:action(.:format))"
         app    = Object.new
         conditions = {
           :request_method => 'GET'
@@ -553,7 +553,11 @@ module ActionDispatch
 
       def add_routes router, paths
         paths.each do |path|
-          path  = Path::Pattern.new path
+          if String === path
+            path  = Path::Pattern.from_string path
+          else
+            path  = Path::Pattern.new path
+          end
           router.routes.add_route @app, path, {}, {}, {}
         end
       end
diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb
index 2103cbb9b8..a4efc82b8c 100644
--- a/actionpack/test/journey/routes_test.rb
+++ b/actionpack/test/journey/routes_test.rb
@@ -18,7 +18,7 @@ module ActionDispatch
 
       def test_ast
         routes = Routes.new
-        path   = Path::Pattern.new '/hello'
+        path   = Path::Pattern.from_string '/hello'
 
         routes.add_route nil, path, {}, {}, {}
         ast = routes.ast
@@ -28,7 +28,7 @@ module ActionDispatch
 
       def test_simulator_changes
         routes = Routes.new
-        path   = Path::Pattern.new '/hello'
+        path   = Path::Pattern.from_string '/hello'
 
         routes.add_route nil, path, {}, {}, {}
         sim = routes.simulator
@@ -40,8 +40,8 @@ module ActionDispatch
         #def add_route app, path, conditions, defaults, name = nil
         routes = Routes.new
 
-        one   = Path::Pattern.new '/hello'
-        two   = Path::Pattern.new '/aaron'
+        one   = Path::Pattern.from_string '/hello'
+        two   = Path::Pattern.from_string '/aaron'
 
         routes.add_route nil, one, {}, {}, 'aaron'
         routes.add_route nil, two, {}, {}, 'aaron'
-- 
cgit v1.2.3


From da2cf937aa68c2470c0d4a73c29fe3555b2667c0 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Thu, 29 May 2014 15:45:42 -0700
Subject: no more is_a checks on instantiation

---
 actionpack/lib/action_dispatch/journey/path/pattern.rb | 13 ++++---------
 actionpack/test/journey/path/pattern_test.rb           |  4 ----
 2 files changed, 4 insertions(+), 13 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb
index c39f8e850a..3af940a02f 100644
--- a/actionpack/lib/action_dispatch/journey/path/pattern.rb
+++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb
@@ -11,15 +11,10 @@ module ActionDispatch
         end
 
         def initialize(strexp)
-          case strexp
-          when Router::Strexp
-            @spec         = strexp.ast
-            @requirements = strexp.requirements
-            @separators   = strexp.separators.join
-            @anchored     = strexp.anchor
-          else
-            raise ArgumentError, "Bad expression: #{strexp}"
-          end
+          @spec         = strexp.ast
+          @requirements = strexp.requirements
+          @separators   = strexp.separators.join
+          @anchored     = strexp.anchor
 
           @names          = nil
           @optional_names = nil
diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb
index cdb3cdad7b..9dfdfc23ed 100644
--- a/actionpack/test/journey/path/pattern_test.rb
+++ b/actionpack/test/journey/path/pattern_test.rb
@@ -75,10 +75,6 @@ module ActionDispatch
           end
         end
 
-        def test_to_raise_exception_with_bad_expression
-          assert_raise(ArgumentError, "Bad expression: []") { Pattern.new [] }
-        end
-
         def test_to_regexp_with_extended_group
           strexp = Router::Strexp.build(
             '/page/:name',
-- 
cgit v1.2.3