diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 3f7cf0950d..b618b9c400 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -664,7 +664,6 @@ module ActionDispatch def define_generate_prefix(app, name) _route = @set.named_routes.get name _routes = @set - _url_helpers = @set.url_helpers script_namer = ->(options) do prefix_options = options.slice(*_route.segment_keys) @@ -676,7 +675,7 @@ module ActionDispatch # We must actually delete prefix segment keys to avoid passing them to next url_for. _route.segment_keys.each { |k| options.delete(k) } - _url_helpers.send("#{name}_path", prefix_options) + @set.url_helpers.send("#{name}_path", prefix_options) end app.routes.define_mounted_helper(name, script_namer) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index acce8a7ef3..8f21722a6a 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -378,6 +378,7 @@ module ActionDispatch @disable_clear_and_finalize = false @finalized = false @env_key = "ROUTES_#{object_id}_SCRIPT_NAME".freeze + @url_helpers = nil @set = Journey::Routes.new @router = Journey::Router.new @set @@ -437,6 +438,7 @@ module ActionDispatch return if @finalized @append.each { |blk| eval_block(blk) } @finalized = true + @url_helpers = build_url_helper_module true end def clear! @@ -465,11 +467,10 @@ module ActionDispatch return if MountedHelpers.method_defined?(name) routes = self - helpers = routes.url_helpers MountedHelpers.class_eval do define_method "_#{name}" do - RoutesProxy.new(routes, _routes_context, helpers, script_namer) + RoutesProxy.new(routes, _routes_context, routes.url_helpers, script_namer) end end @@ -480,7 +481,20 @@ module ActionDispatch RUBY end + class UnfinalizedRouteSet < StandardError + end + def url_helpers(supports_path = true) + raise UnfinalizedRouteSet, "routes have not been finalized. Please call `finalize!` or use `draw(&block)`" unless @finalized + + if supports_path + @url_helpers + else + build_url_helper_module false + end + end + + def build_url_helper_module(supports_path) routes = self Module.new do diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index af41521c5c..0e8712f8d9 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -138,6 +138,20 @@ module ActionDispatch assert_generates(path.is_a?(Hash) ? path[:path] : path, generate_options, defaults, extras, message) end + # Provides a hook on `finalize!` so we can mutate a controller after the + # route set has been drawn. + class WithRouting < ActionDispatch::Routing::RouteSet # :nodoc: + def initialize(&block) + super() + @block = block + end + + def finalize! + super + @block.call self + end + end + # A helper to make it easier to test different route configurations. # This method temporarily replaces @routes with a new RouteSet instance. # @@ -152,16 +166,19 @@ module ActionDispatch # end # def with_routing - old_routes, @routes = @routes, ActionDispatch::Routing::RouteSet.new - if defined?(@controller) && @controller - old_controller, @controller = @controller, @controller.clone - _routes = @routes + old_routes = @routes + old_controller = nil + @routes = WithRouting.new do |_routes| + if defined?(@controller) && @controller + old_controller, @controller = @controller, @controller.clone + _routes = @routes - @controller.singleton_class.include(_routes.url_helpers) + @controller.singleton_class.include(_routes.url_helpers) - if @controller.respond_to? :view_context_class - @controller.view_context_class = Class.new(@controller.view_context_class) do - include _routes.url_helpers + if @controller.respond_to? :view_context_class + @controller.view_context_class = Class.new(@controller.view_context_class) do + include _routes.url_helpers + end end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 65dd28b3d7..7c9f15108d 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -100,7 +100,10 @@ end class ActionDispatch::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) - RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| + routes ||= ActionDispatch::Routing::RouteSet.new.tap { |rs| + rs.draw { } + } + RoutedRackApp.new(routes) do |middleware| middleware.use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public") middleware.use ActionDispatch::DebugExceptions middleware.use ActionDispatch::Callbacks diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 39ede1442a..b078e8ad9f 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -542,9 +542,6 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest def with_test_route_set with_routing do |set| controller = ::IntegrationProcessTest::IntegrationController.clone - controller.class_eval do - include set.url_helpers - end set.draw do get "moved" => redirect("/method") @@ -555,6 +552,10 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + controller.class_eval do + include set.url_helpers + end + singleton_class.include(set.url_helpers) yield diff --git a/actionpack/test/dispatch/routing/ipv6_redirect_test.rb b/actionpack/test/dispatch/routing/ipv6_redirect_test.rb index 31559bffc7..fb423d2951 100644 --- a/actionpack/test/dispatch/routing/ipv6_redirect_test.rb +++ b/actionpack/test/dispatch/routing/ipv6_redirect_test.rb @@ -4,6 +4,11 @@ require "abstract_unit" class IPv6IntegrationTest < ActionDispatch::IntegrationTest Routes = ActionDispatch::Routing::RouteSet.new + Routes.draw do + get "/", to: "bad_route_request#index", as: :index + get "/foo", to: "bad_route_request#foo", as: :foo + end + include Routes.url_helpers class ::BadRouteRequestController < ActionController::Base @@ -17,11 +22,6 @@ class IPv6IntegrationTest < ActionDispatch::IntegrationTest end end - Routes.draw do - get "/", to: "bad_route_request#index", as: :index - get "/foo", to: "bad_route_request#foo", as: :foo - end - def _routes Routes end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index aef9351de1..b0096d26be 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -4,16 +4,13 @@ require "abstract_unit" module TestUrlGeneration class WithMountPoint < ActionDispatch::IntegrationTest - Routes = ActionDispatch::Routing::RouteSet.new - include Routes.url_helpers - class ::MyRouteGeneratingController < ActionController::Base - include Routes.url_helpers def index render plain: foo_path end end + Routes = ActionDispatch::Routing::RouteSet.new Routes.draw do get "/foo", to: "my_route_generating#index", as: :foo @@ -22,6 +19,10 @@ module TestUrlGeneration mount MyRouteGeneratingController.action(:index), at: "/bar" end + class ::MyRouteGeneratingController + include Routes.url_helpers + end + APP = build_app Routes def _routes