mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Eagerly build the routing helper module after routes are committed
This commit eagerly builds the route helper module after the routes have been drawn and finalized. This allows us to cache the helper module but not have to worry about people accessing the module while route definition is "in-flight", and automatically deals with cache invalidation as the module is regenerated anytime someone redraws the routes. The restriction this commit introduces is that the url helper module can only be accessed *after* the routes are done being drawn. Refs #24554 and #32892
This commit is contained in:
parent
d34bd0d2d5
commit
8dc784292b
7 changed files with 60 additions and 25 deletions
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue