Merge pull request rails#54705 from Edouard-chin/ec-with-routing

Don't rebuild the middleware stack when using `with_routing`

Author: byroot

actionpack/lib/action_dispatch/testing/assertions/routing.rb

@@ -5,6 +5,7 @@
 require "uri"
 require "active_support/core_ext/hash/indifferent_access"
 require "active_support/core_ext/string/access"
+require "active_support/core_ext/module/redefine_method"
 require "action_controller/metal/exceptions"
 
 module ActionDispatch
@@ -20,38 +21,43 @@ module WithIntegrationRouting # :nodoc:
         module ClassMethods
           def with_routing(&block)
             old_routes = nil
+            old_routes_call_method = nil
             old_integration_session = nil
 
             setup do
               old_routes = app.routes
+              old_routes_call_method = old_routes.method(:call)
               old_integration_session = integration_session
               create_routes(&block)
             end
 
             teardown do
-              reset_routes(old_routes, old_integration_session)
+              reset_routes(old_routes, old_routes_call_method, old_integration_session)
             end
           end
         end
 
         def with_routing(&block)
           old_routes = app.routes
+          old_routes_call_method = old_routes.method(:call)
           old_integration_session = integration_session
           create_routes(&block)
         ensure
-          reset_routes(old_routes, old_integration_session)
+          reset_routes(old_routes, old_routes_call_method, old_integration_session)
         end
 
         private
           def create_routes
             app = self.app
             routes = ActionDispatch::Routing::RouteSet.new
-            rack_app = app.config.middleware.build(routes)
+
+            @original_routes ||= app.routes
+            @original_routes.singleton_class.redefine_method(:call, &routes.method(:call))
+
             https = integration_session.https?
             host = integration_session.host
 
             app.instance_variable_set(:@routes, routes)
-            app.instance_variable_set(:@app, rack_app)
             @integration_session = Class.new(ActionDispatch::Integration::Session) do
               include app.routes.url_helpers
               include app.routes.mounted_helpers
@@ -63,11 +69,9 @@ def create_routes
             yield routes
           end
 
-          def reset_routes(old_routes, old_integration_session)
-            old_rack_app = app.config.middleware.build(old_routes)
-
+          def reset_routes(old_routes, old_routes_call_method, old_integration_session)
             app.instance_variable_set(:@routes, old_routes)
-            app.instance_variable_set(:@app, old_rack_app)
+            @original_routes.singleton_class.redefine_method(:call, &old_routes_call_method)
             @integration_session = old_integration_session
             @routes = old_routes
           end

actionpack/test/dispatch/routing_assertions_test.rb

@@ -351,3 +351,54 @@ class WithRoutingSettingsTest < ActionDispatch::IntegrationTest
     end
   end
 end
+
+class WithRoutingResetTest < ActionDispatch::IntegrationTest
+  test "with_routing doesn't rebuild the middleware stack" do
+    middleware = Class.new do
+      def initialize(app)
+        @app = app
+        @config = {}
+
+        yield(@config)
+      end
+
+      def call(env)
+        [200, { Rack::CONTENT_TYPE => "text/plain; charset=UTF-8" }, [@config[:foo]]]
+      end
+    end
+
+    middleware_config = nil
+
+    @app = self.class.build_app do |middleware_stack|
+      middleware_stack.use middleware do |config|
+        middleware_config = config
+      end
+    end
+    @app.routes.draw do
+      get("/purchase", to: "store#purchase")
+    end
+
+    middleware_config[:foo] = "bar"
+
+    # Ensure the middleware is properly configured before calling `with_routing`
+    get "/purchase"
+    assert_response(:ok)
+    assert_equal("bar", response.body)
+
+    with_routing do |route_set|
+      route_set.draw do
+        get "/purchase", to: "store#purchase"
+      end
+
+      # Ensure the middleware is properly configured inside `with_routing`
+      get "/purchase"
+      assert_response(:ok)
+      assert_equal("bar", response.body)
+    end
+
+    # Ensure the middleware is properly configured after `with_routing`
+    get "/purchase"
+    assert_response(:ok)
+    assert_equal("bar", response.body)
+  end
+end