Merge pull request rails#46530 from jonathanhefner/url_helpers-use-dup-for-reinclusion

Use memoized dup of `url_helpers` for reinclusion

Author: rafaelfranca

actionpack/lib/abstract_controller/railties/routes_helpers.rb

@@ -10,26 +10,11 @@ def self.with(routes, include_path_helpers = true)
           define_method(:inherited) do |klass|
             super(klass)
 
-            namespace = klass.module_parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) }
-            actual_routes = namespace ? namespace.railtie_routes_url_helpers._routes : routes
-
-            if namespace
+            if namespace = klass.module_parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) }
               klass.include(namespace.railtie_routes_url_helpers(include_path_helpers))
             else
               klass.include(routes.url_helpers(include_path_helpers))
             end
-
-            # In the case that we have ex.
-            #   class A::Foo < ApplicationController
-            #   class Bar < A::Foo
-            # We will need to redefine _routes because it will not be correct
-            # via inheritance.
-            unless klass._routes.equal?(actual_routes)
-              klass.redefine_singleton_method(:_routes) { actual_routes }
-              klass.include(Module.new do
-                define_method(:_routes) { @_routes || actual_routes }
-              end)
-            end
           end
         end
       end

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -574,6 +574,20 @@ def url_options; {}; end
           end
 
           private :_generate_paths_by_default
+
+          # If the module is included more than once (for example, in a subclass
+          # of an ancestor that includes the module), ensure that the `_routes`
+          # singleton and instance methods return the desired route set by
+          # including a new copy of the module (recursively if necessary). Note
+          # that this method is called for each inclusion, whereas the above
+          # `included` block is run only for the initial inclusion of each copy.
+          def self.included(base)
+            super
+            if !base._routes.equal?(@_proxy._routes)
+              @dup_for_reinclude ||= self.dup
+              base.include @dup_for_reinclude
+            end
+          end
         end
       end
 

actionpack/test/controller/route_helpers_test.rb

@@ -3,19 +3,6 @@
 require "abstract_unit"
 
 class RouteHelperIntegrationTest < ActionDispatch::IntegrationTest
-  def self.routes
-    @routes ||= ActionDispatch::Routing::RouteSet.new
-  end
-
-  class FakeACBase < ActionController::Base
-    # Normally done by app initialization to ActionController::Base
-    app = RouteHelperIntegrationTest
-    extend ::AbstractController::Railties::RoutesHelpers.with(app.routes)
-  end
-
-  class ApplicationController < FakeACBase
-  end
-
   class FooController < ApplicationController
   end
 
@@ -24,18 +11,16 @@ class FooController < ApplicationController
   # duplicate these modules and make method cache invalidation expensive.
   # https://github.com/rails/rails/pull/37927
   test "only includes one module with route helpers" do
-    app = self.class
-
-    url_helpers_module = app.routes.named_routes.url_helpers_module
-    path_helpers_module = app.routes.named_routes.path_helpers_module
+    url_helpers_module = SharedTestRoutes.named_routes.url_helpers_module
+    path_helpers_module = SharedTestRoutes.named_routes.path_helpers_module
 
     assert_operator FooController, :<, url_helpers_module
     assert_operator ApplicationController, :<, url_helpers_module
-    assert_not_operator FakeACBase, :<, url_helpers_module
+    assert_not_operator ActionController::Base, :<, url_helpers_module
 
     assert_operator FooController, :<, path_helpers_module
     assert_operator ApplicationController, :<, path_helpers_module
-    assert_not_operator FakeACBase, :<, path_helpers_module
+    assert_not_operator ActionController::Base, :<, path_helpers_module
 
     included_modules = FooController.ancestors.grep_v(Class)
     included_modules -= [url_helpers_module, path_helpers_module]