Only use shallow nested scope when depth is > 1

By tracking the depth of resource nesting we can push the need for nested
shallow scoping to only those routes that are nested more than one deep.
This allows us to keep the fix for #12498 and fix the regression in #14224.

Fixes #14224.

2 files changed, 83 insertions, 20 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 5da6582b15..314b966bb0 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -1323,8 +1323,10 @@ module ActionDispatch
           end
 
           with_scope_level(:member) do
-            scope(parent_resource.member_scope) do
-              yield
+            if shallow?
+              shallow_scope(parent_resource.member_scope) { yield }
+            else
+              scope(parent_resource.member_scope) { yield }
             end
           end
         end
@@ -1347,16 +1349,8 @@ module ActionDispatch
           end
 
           with_scope_level(:nested) do
-            if shallow?
-              with_exclusive_scope do
-                if @scope[:shallow_path].blank?
-                  scope(parent_resource.nested_scope, nested_options) { yield }
-                else
-                  scope(@scope[:shallow_path], :as => @scope[:shallow_prefix]) do
-                    scope(parent_resource.nested_scope, nested_options) { yield }
-                  end
-                end
-              end
+            if shallow? && nesting_depth > 1
+              shallow_scope(parent_resource.nested_scope, nested_options) { yield }
             else
               scope(parent_resource.nested_scope, nested_options) { yield }
             end
@@ -1567,11 +1561,13 @@ module ActionDispatch
 
           def resource_scope(kind, resource) #:nodoc:
             old_resource, @scope[:scope_level_resource] = @scope[:scope_level_resource], resource
+            @nesting.push(resource)
 
             with_scope_level(kind) do
               scope(parent_resource.resource_scope) { yield }
             end
           ensure
+            @nesting.pop
             @scope[:scope_level_resource] = old_resource
           end
 
@@ -1584,6 +1580,10 @@ module ActionDispatch
             options
           end
 
+          def nesting_depth #:nodoc:
+            @nesting.size
+          end
+
           def param_constraint? #:nodoc:
             @scope[:constraints] && @scope[:constraints][parent_resource.param].is_a?(Regexp)
           end
@@ -1596,18 +1596,20 @@ module ActionDispatch
             flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s)
           end
 
-          def shallow_scoping? #:nodoc:
-            shallow? && @scope[:scope_level] == :member
+          def shallow_scope(path, options = {}) #:nodoc:
+            old_name_prefix, old_path = @scope[:as], @scope[:path]
+            @scope[:as], @scope[:path] = @scope[:shallow_prefix], @scope[:shallow_path]
+
+            scope(path, options) { yield }
+          ensure
+            @scope[:as], @scope[:path] = old_name_prefix, old_path
           end
 
           def path_for_action(action, path) #:nodoc:
-            prefix = shallow_scoping? ?
-              "#{@scope[:shallow_path]}/#{parent_resource.shallow_scope}" : @scope[:path]
-
             if canonical_action?(action, path.blank?)
-              prefix.to_s
+              @scope[:path].to_s
             else
-              "#{prefix}/#{action_path(action, path)}"
+              "#{@scope[:path]}/#{action_path(action, path)}"
             end
           end
 
@@ -1645,7 +1647,7 @@ module ActionDispatch
             when :new
               [prefix, :new, name_prefix, member_name]
             when :member
-              [prefix, shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name]
+              [prefix, name_prefix, member_name]
             when :root
               [name_prefix, collection_name, prefix]
             else
@@ -1786,6 +1788,7 @@ module ActionDispatch
         @set = set
         @scope = { :path_names => @set.resources_path_names }
         @concerns = {}
+        @nesting = []
       end
 
       include Base
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 1fa2cc6cf2..8dad3bcbb3 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3033,6 +3033,66 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal '/admin/posts/1/comments', admin_post_comments_path('1')
   end
 
+  def test_shallow_path_and_prefix_are_not_added_to_non_shallow_routes
+    draw do
+      scope shallow_path: 'projects', shallow_prefix: 'project' do
+        resources :projects do
+          resources :files, controller: 'project_files', shallow: true
+        end
+      end
+    end
+
+    get '/projects'
+    assert_equal 'projects#index', @response.body
+    assert_equal '/projects', projects_path
+
+    get '/projects/new'
+    assert_equal 'projects#new', @response.body
+    assert_equal '/projects/new', new_project_path
+
+    post '/projects'
+    assert_equal 'projects#create', @response.body
+
+    get '/projects/1'
+    assert_equal 'projects#show', @response.body
+    assert_equal '/projects/1', project_path('1')
+
+    get '/projects/1/edit'
+    assert_equal 'projects#edit', @response.body
+    assert_equal '/projects/1/edit', edit_project_path('1')
+
+    patch '/projects/1'
+    assert_equal 'projects#update', @response.body
+
+    delete '/projects/1'
+    assert_equal 'projects#destroy', @response.body
+
+    get '/projects/1/files'
+    assert_equal 'project_files#index', @response.body
+    assert_equal '/projects/1/files', project_files_path('1')
+
+    get '/projects/1/files/new'
+    assert_equal 'project_files#new', @response.body
+    assert_equal '/projects/1/files/new', new_project_file_path('1')
+
+    post '/projects/1/files'
+    assert_equal 'project_files#create', @response.body
+
+    get '/projects/files/2'
+    assert_equal 'project_files#show', @response.body
+    assert_equal '/projects/files/2', project_file_path('2')
+
+    get '/projects/files/2/edit'
+    assert_equal 'project_files#edit', @response.body
+    assert_equal '/projects/files/2/edit', edit_project_file_path('2')
+
+    patch '/projects/files/2'
+    assert_equal 'project_files#update', @response.body
+
+    delete '/projects/files/2'
+    assert_equal 'project_files#destroy', @response.body
+  end
+
 private
 
   def draw(&block)