From c21823347281088db273937be27c2dee9a05f636 Mon Sep 17 00:00:00 2001
From: Woody Peterson <woody.peterson@gmail.com>
Date: Wed, 28 Aug 2019 16:57:55 -0700
Subject: [PATCH] Fix route from "(:a)(:b)" when given only :a or :b

Previously, when generating a root path from a route having nested
optional scopes, and when providing an option, would result in an
invalid path leading with "//". For example, the route
"(:locale)(:region)", given `locale: 'es'`, would output "//es".

Commit 7670d60977a introduced this bug while fixing the edge case of
empty strings returned when no options are passed to such routes.

This commit fixes both cases.
---
 .../lib/action_dispatch/routing/mapper.rb       | 17 +++++++++++++----
 actionpack/test/dispatch/routing_test.rb        | 16 +++++++++++++++-
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index da95e14cbb..8cdf7cd283 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -398,12 +398,21 @@ module ActionDispatch
           end
       end
 
-      # Invokes Journey::Router::Utils.normalize_path and ensure that
-      # (:locale) becomes (/:locale) instead of /(:locale). Except
-      # for root cases, where the latter is the correct one.
+      # Invokes Journey::Router::Utils.normalize_path, then ensures that
+      # /(:locale) becomes (/:locale). Except for root cases, where the
+      # former is the correct one.
       def self.normalize_path(path)
         path = Journey::Router::Utils.normalize_path(path)
-        path.gsub!(%r{/(\(+)/?}, '\1/') unless %r{^/(\(+[^)]+\)){1,}$}.match?(path)
+
+        # the path for a root URL at this point can be something like
+        # "/(/:locale)(/:platform)/(:browser)", and we would want
+        # "/(:locale)(/:platform)(/:browser)"
+
+        # reverse "/(", "/((" etc to "(/", "((/" etc
+        path.gsub!(%r{/(\(+)/?}, '\1/')
+        # if a path is all optional segments, change the leading "(/" back to
+        # "/(" so it evaluates to "/" when interpreted with no options.
+        path.sub!(%r{^(\(+)/}, '/\1') if %r{^(\(+[^)]+\)){1,}$}.match?(path)
         path
       end
 
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index d4a667a13a..9ca35837a0 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -1382,7 +1382,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal "projects#index", @response.body
   end
 
-  def test_optionally_scoped_root_unscoped_access
+  def test_optional_scoped_root_hierarchy
     draw do
       scope "(:locale)" do
         scope "(:platform)" do
@@ -1394,8 +1394,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
 
     assert_equal "/", root_path
+    assert_equal "/en", root_path(locale: "en")
+    assert_equal "/en/osx", root_path(locale: "en", platform: "osx")
+    assert_equal "/en/osx/chrome",
+      root_path(locale: "en", platform: "osx", browser: "chrome")
+
     get "/"
     assert_equal "projects#index", @response.body
+
+    get "/en"
+    assert_equal "projects#index", @response.body
+
+    get "/en/osx"
+    assert_equal "projects#index", @response.body
+
+    get "/en/osx/chrome"
+    assert_equal "projects#index", @response.body
   end
 
   def test_scope_with_format_option