From d6ca73306c375539bcdcd832142cdf3357d03919 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 16 Feb 2021 10:10:25 -0800 Subject: [PATCH 1/2] Force _routes to be redefined on re-inclusion This fixes an regression where _routes were set incorrectly when the inheritance chain went from one route namespace (isolated engine or main app) to another and then back to the original. Because the url_helpers module was being cached and was being re-included this was not setting _routes. This commit solves the issue by detecting that case and redefining _routes in that case. We could have always performed the redefinition, but it's a pretty uncommon case, so we might as well only do it when necessary. --- .../railties/routes_helpers.rb | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/abstract_controller/railties/routes_helpers.rb b/actionpack/lib/abstract_controller/railties/routes_helpers.rb index fbd93705edee3..b728c7144a339 100644 --- a/actionpack/lib/abstract_controller/railties/routes_helpers.rb +++ b/actionpack/lib/abstract_controller/railties/routes_helpers.rb @@ -7,11 +7,27 @@ def self.with(routes, include_path_helpers = true) Module.new do define_method(:inherited) do |klass| super(klass) - if namespace = klass.module_parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) } + + 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 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 From 715d6bc4d0ce953a4c53ac51d084db587be8ecc1 Mon Sep 17 00:00:00 2001 From: Josh Goodall Date: Mon, 21 Sep 2020 16:34:37 +1000 Subject: [PATCH 2/2] test case to illustrate isolated engine inheritance issue --- railties/test/railties/engine_test.rb | 29 ++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index b6548660318fd..f397070e24d24 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -644,7 +644,7 @@ class Engine < ::Rails::Engine assert_equal Rails.application.routes, env["action_dispatch.routes"] end - test "isolated engine should include only its own routes and helpers" do + test "isolated engine routes and helpers are isolated to that engine" do @plugin.write "lib/bukkits.rb", <<-RUBY module Bukkits class Engine < ::Rails::Engine @@ -729,6 +729,30 @@ def polymorphic_path_without_namespace end RUBY + @plugin.write "app/controllers/bukkits/session_controller.rb", <<-RUBY + module Bukkits + class SessionController < ApplicationController + def index + render plain: default_path + end + + private + def default_path + foo_path + end + end + end + RUBY + + controller "bar", <<-RUBY + class BarController < Bukkits::SessionController + private + def default_path + bar_path + end + end + RUBY + @plugin.write "app/mailers/bukkits/my_mailer.rb", <<-RUBY module Bukkits class MyMailer < ActionMailer::Base @@ -746,6 +770,9 @@ class MyMailer < ActionMailer::Base assert ::Bukkits::MyMailer.method_defined?(:foo_url) assert_not ::Bukkits::MyMailer.method_defined?(:bar_url) + get("/bar") + assert_equal "/bar", last_response.body + get("/bukkits/from_app") assert_equal "false", last_response.body