Fix regression from multiple mountpoint support

4 files changed, 65 insertions, 1 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index eaa0e12b67..28809d7e67 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -658,6 +658,11 @@ module ActionDispatch
             script_namer = ->(options) do
               prefix_options = options.slice(*_route.segment_keys)
               prefix_options[:relative_url_root] = "".freeze
+
+              if options[:_recall]
+                prefix_options.reverse_merge!(options[:_recall].slice(*_route.segment_keys))
+              end
+
               # We must actually delete prefix segment keys to avoid passing them to next url_for.
               _route.segment_keys.each { |k| options.delete(k) }
               _routes.url_helpers.send("#{name}_path", prefix_options)
diff --git a/actionpack/lib/action_dispatch/routing/routes_proxy.rb b/actionpack/lib/action_dispatch/routing/routes_proxy.rb
index 7a6c2e95d8..af0de7366b 100644
--- a/actionpack/lib/action_dispatch/routing/routes_proxy.rb
+++ b/actionpack/lib/action_dispatch/routing/routes_proxy.rb
@@ -31,7 +31,14 @@ module ActionDispatch
             def #{method}(*args)
               options = args.extract_options!
               options = url_options.merge((options || {}).symbolize_keys)
-              options.reverse_merge!(script_name: @script_namer.call(options)) if @script_namer
+
+              if @script_namer
+                options[:script_name] = merge_script_names(
+                  options[:script_name],
+                  @script_namer.call(options)
+                )
+              end
+
               args << options
               @helpers.#{method}(*args)
             end
@@ -41,6 +48,20 @@ module ActionDispatch
           super
         end
       end
+
+      # Keeps the part of the script name provided by the global
+      # context via ENV["SCRIPT_NAME"], which `mount` doesn't know
+      # about since it depends on the specific request, but use our
+      # script name resolver for the mount point dependent part.
+      def merge_script_names(previous_script_name, new_script_name)
+        return new_script_name unless previous_script_name
+
+        resolved_parts = new_script_name.count("/")
+        previous_parts = previous_script_name.count("/")
+        context_parts = previous_parts - resolved_parts + 1
+
+        (previous_script_name.split("/").slice(0, context_parts).join("/")) + new_script_name
+      end
     end
   end
 end
diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb
index 0379394f31..6f762d2d3f 100644
--- a/railties/test/railties/engine_test.rb
+++ b/railties/test/railties/engine_test.rb
@@ -1427,6 +1427,35 @@ YAML
       assert_equal "/vegetables/1/bukkits/posts", last_response.body
     end
 
+    test "route helpers resolve script name correctly when called with different script name from current one" do
+      @plugin.write "app/controllers/posts_controller.rb", <<-RUBY
+        class PostsController < ActionController::Base
+          def index
+            render plain: fruit_bukkits.posts_path(fruit_id: 2)
+          end
+        end
+      RUBY
+
+      app_file "config/routes.rb", <<-RUBY
+        Rails.application.routes.draw do
+          resources :fruits do
+            mount Bukkits::Engine => "/bukkits"
+          end
+        end
+      RUBY
+
+      @plugin.write "config/routes.rb", <<-RUBY
+        Bukkits::Engine.routes.draw do
+          resources :posts, only: :index
+        end
+      RUBY
+
+      boot_rails
+
+      get("/fruits/1/bukkits/posts")
+      assert_equal "/fruits/2/bukkits/posts", last_response.body
+    end
+
   private
     def app
       Rails.application
diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb
index 6639e55382..6eb2c5acc6 100644
--- a/railties/test/railties/mounted_engine_test.rb
+++ b/railties/test/railties/mounted_engine_test.rb
@@ -111,6 +111,7 @@ module ApplicationTests
       @plugin.write "config/routes.rb", <<-RUBY
         Blog::Engine.routes.draw do
           resources :posts
+          get '/different_context', to: 'posts#different_context'
           get '/generate_application_route', to: 'posts#generate_application_route'
           get '/application_route_in_view', to: 'posts#application_route_in_view'
           get '/engine_polymorphic_path', to: 'posts#engine_polymorphic_path'
@@ -125,6 +126,10 @@ module ApplicationTests
               render plain: blog.post_path(1)
             end
 
+            def different_context
+              render plain: blog.post_path(1, user: "ada")
+            end
+
             def generate_application_route
               path = main_app.url_for(controller: "/main",
                                  action: "index",
@@ -196,6 +201,10 @@ module ApplicationTests
       get "/john/blog/posts"
       assert_equal "/john/blog/posts/1", last_response.body
 
+      # test generating engine route from engine with a different context
+      get "/john/blog/different_context"
+      assert_equal "/ada/blog/posts/1", last_response.body
+
       # test generating engine's route from engine with default_url_options
       get "/john/blog/posts", {}, "SCRIPT_NAME" => "/foo"
       assert_equal "/foo/john/blog/posts/1", last_response.body