Skip to content

Commit cdb2835

Browse files
committed
Defer route drawing to the first request, or when url_helpers called
Executes the first routes reload when the route set is first used to serve a request, generate a url, or otherwise. Previously, this was executed unconditionally on boot, which can slow down boot time unnecessarily for larger apps with lots of routes.
1 parent e3ec553 commit cdb2835

File tree

15 files changed

+412
-52
lines changed

15 files changed

+412
-52
lines changed

actionpack/lib/action_dispatch/routing/route_set.rb

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ def define_url_helper(mod, name, helper, url_strategy)
351351
PATH = ->(options) { ActionDispatch::Http::URL.path_for(options) }
352352
UNKNOWN = ->(options) { ActionDispatch::Http::URL.url_for(options) }
353353

354-
attr_accessor :formatter, :set, :named_routes, :default_scope, :router
354+
attr_accessor :formatter, :set, :named_routes, :router
355355
attr_accessor :disable_clear_and_finalize, :resources_path_names
356356
attr_accessor :default_url_options, :draw_paths
357357
attr_reader :env_key, :polymorphic_mappings
@@ -363,7 +363,7 @@ def self.default_resources_path_names
363363
end
364364

365365
def self.new_with_config(config)
366-
route_set_config = DEFAULT_CONFIG
366+
route_set_config = DEFAULT_CONFIG.dup
367367

368368
# engines apparently don't have this set
369369
if config.respond_to? :relative_url_root
@@ -374,14 +374,18 @@ def self.new_with_config(config)
374374
route_set_config.api_only = config.api_only
375375
end
376376

377+
if config.respond_to? :default_scope
378+
route_set_config.default_scope = config.default_scope
379+
end
380+
377381
new route_set_config
378382
end
379383

380-
Config = Struct.new :relative_url_root, :api_only
384+
Config = Struct.new :relative_url_root, :api_only, :default_scope
381385

382-
DEFAULT_CONFIG = Config.new(nil, false)
386+
DEFAULT_CONFIG = Config.new(nil, false, nil)
383387

384-
def initialize(config = DEFAULT_CONFIG)
388+
def initialize(config = DEFAULT_CONFIG.dup)
385389
self.named_routes = NamedRouteCollection.new
386390
self.resources_path_names = self.class.default_resources_path_names
387391
self.default_url_options = {}
@@ -416,6 +420,14 @@ def api_only?
416420
@config.api_only
417421
end
418422

423+
def default_scope
424+
@config.default_scope
425+
end
426+
427+
def default_scope=(new_default_scope)
428+
@config.default_scope = new_default_scope
429+
end
430+
419431
def request_class
420432
ActionDispatch::Request
421433
end

actionpack/test/abstract_unit.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ def env
3434
@_env ||= ActiveSupport::StringInquirer.new(ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "test")
3535
end
3636

37+
def application; end
38+
3739
def root; end
3840
end
3941
end

actionpack/test/dispatch/routing_assertions_test.rb

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
require "rails/engine"
55
require "controller/fake_controllers"
66

7-
class SecureArticlesController < ArticlesController; end
7+
class SecureArticlesController < ArticlesController
8+
def index
9+
render(inline: "")
10+
end
11+
end
812
class BlockArticlesController < ArticlesController; end
913
class QueryArticlesController < ArticlesController; end
1014

@@ -276,6 +280,20 @@ class RoutingAssertionsControllerTest < ActionController::TestCase
276280

277281
class WithRoutingTest < ActionController::TestCase
278282
include RoutingAssertionsSharedTests::WithRoutingSharedTests
283+
284+
test "with_routing routes are reachable" do
285+
@controller = SecureArticlesController.new
286+
287+
with_routing do |routes|
288+
routes.draw do
289+
get :new_route, to: "secure_articles#index"
290+
end
291+
292+
get :index
293+
294+
assert_predicate(response, :ok?)
295+
end
296+
end
279297
end
280298
end
281299

@@ -295,6 +313,18 @@ class RoutingAssertionsIntegrationTest < ActionDispatch::IntegrationTest
295313

296314
class WithRoutingTest < ActionDispatch::IntegrationTest
297315
include RoutingAssertionsSharedTests::WithRoutingSharedTests
316+
317+
test "with_routing routes are reachable" do
318+
with_routing do |routes|
319+
routes.draw do
320+
get :new_route, to: "secure_articles#index"
321+
end
322+
323+
get "/new_route"
324+
325+
assert_predicate(response, :ok?)
326+
end
327+
end
298328
end
299329

300330
class WithRoutingSettingsTest < ActionDispatch::IntegrationTest

railties/CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
* Defer route drawing to the first request, or when url_helpers are called
2+
3+
Executes the first routes reload in middleware, or when a route set's
4+
url_helpers receives a route call / asked if it responds to a route.
5+
Previously, this was executed unconditionally on boot, which can
6+
slow down boot time unnecessarily for larger apps with lots of routes.
7+
8+
Environments like production that have `config.eager_load = true` will
9+
continue to eagerly load routes on boot.
10+
11+
*Gannon McGibbon*
12+
113
* Generate form helpers to use `textarea*` methods instead of `text_area*` methods
214

315
*Sean Doyle*

railties/lib/rails/application.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
require "active_support/encrypted_configuration"
1010
require "active_support/hash_with_indifferent_access"
1111
require "active_support/configuration_file"
12+
require "active_support/parameter_filter"
1213
require "rails/engine"
1314
require "rails/autoloaders"
1415

@@ -153,6 +154,10 @@ def reload_routes!
153154
routes_reloader.reload!
154155
end
155156

157+
def reload_routes_unless_loaded # :nodoc:
158+
initialized? && routes_reloader.execute_unless_loaded
159+
end
160+
156161
# Returns a key generator (ActiveSupport::CachingKeyGenerator) for a
157162
# specified +secret_key_base+. The return value is memoized, so additional
158163
# calls with the same +secret_key_base+ will return the same key generator

railties/lib/rails/application/finisher.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ def self.complete(_state)
160160
initializer :set_routes_reloader_hook do |app|
161161
reloader = routes_reloader
162162
reloader.eager_load = app.config.eager_load
163-
reloader.execute
164163
reloaders << reloader
165164

166165
app.reloader.to_run do
@@ -178,7 +177,7 @@ def self.complete(_state)
178177
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
179178
end
180179

181-
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
180+
reloader.execute_unless_loaded if !app.routes.is_a?(Engine::LazyRouteSet) || app.config.eager_load
182181
end
183182

184183
# Set clearing dependencies after the finisher hook to ensure paths

railties/lib/rails/application/routes_reloader.rb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class Application
77
class RoutesReloader
88
include ActiveSupport::Callbacks
99

10-
attr_reader :route_sets, :paths, :external_routes
10+
attr_reader :route_sets, :paths, :external_routes, :loaded
1111
attr_accessor :eager_load
1212
attr_writer :run_after_load_paths # :nodoc:
1313
delegate :execute_if_updated, :execute, :updated?, to: :updater
@@ -17,9 +17,11 @@ def initialize
1717
@route_sets = []
1818
@external_routes = []
1919
@eager_load = false
20+
@loaded = false
2021
end
2122

2223
def reload!
24+
@loaded = true
2325
clear!
2426
load_paths
2527
finalize!
@@ -28,6 +30,14 @@ def reload!
2830
revert
2931
end
3032

33+
def execute_unless_loaded
34+
unless @loaded
35+
execute
36+
ActiveSupport.run_load_hooks(:after_routes_loaded, Rails.application)
37+
true
38+
end
39+
end
40+
3141
private
3242
def updater
3343
@updater ||= begin

railties/lib/rails/engine.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ module Rails
349349
# config.railties_order = [Blog::Engine, :main_app, :all]
350350
class Engine < Railtie
351351
autoload :Configuration, "rails/engine/configuration"
352+
autoload :LazyRouteSet, "rails/engine/lazy_route_set"
352353

353354
class << self
354355
attr_accessor :called_from, :isolated
@@ -385,7 +386,8 @@ def endpoint(endpoint = nil)
385386
def isolate_namespace(mod)
386387
engine_name(generate_railtie_name(mod.name))
387388

388-
routes.default_scope = { module: ActiveSupport::Inflector.underscore(mod.name) }
389+
config.default_scope = { module: ActiveSupport::Inflector.underscore(mod.name) }
390+
389391
self.isolated = true
390392

391393
unless mod.respond_to?(:railtie_namespace)
@@ -543,7 +545,7 @@ def env_config
543545
# Defines the routes for this engine. If a block is given to
544546
# routes, it is appended to the engine.
545547
def routes(&block)
546-
@routes ||= ActionDispatch::Routing::RouteSet.new_with_config(config)
548+
@routes ||= config.route_set_class.new_with_config(config)
547549
@routes.append(&block) if block_given?
548550
@routes
549551
end
@@ -588,6 +590,10 @@ def load_seed
588590
config.eager_load_paths.freeze
589591
end
590592

593+
initializer :make_routes_lazy, before: :set_routes_reloader_hook do |app|
594+
config.route_set_class = LazyRouteSet if Rails.env.local?
595+
end
596+
591597
initializer :add_routing_paths do |app|
592598
routing_paths = paths["config/routes.rb"].existent
593599
external_paths = self.paths["config/routes"].paths

railties/lib/rails/engine/configuration.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module Rails
66
class Engine
77
class Configuration < ::Rails::Railtie::Configuration
88
attr_reader :root
9-
attr_accessor :middleware, :javascript_path
9+
attr_accessor :middleware, :javascript_path, :route_set_class, :default_scope
1010
attr_writer :eager_load_paths, :autoload_once_paths, :autoload_paths
1111

1212
# An array of custom autoload paths to be added to the ones defined
@@ -44,6 +44,8 @@ def initialize(root = nil)
4444
@generators = app_generators.dup
4545
@middleware = Rails::Configuration::MiddlewareStackProxy.new
4646
@javascript_path = "javascript"
47+
@route_set_class = ActionDispatch::Routing::RouteSet
48+
@default_scope = nil
4749

4850
@autoload_paths = []
4951
@autoload_once_paths = []
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# frozen_string_literal: true
2+
3+
# :markup: markdown
4+
5+
require "action_dispatch/routing/route_set"
6+
7+
module Rails
8+
class Engine
9+
class LazyRouteSet < ActionDispatch::Routing::RouteSet # :nodoc:
10+
class NamedRouteCollection < ActionDispatch::Routing::RouteSet::NamedRouteCollection
11+
def route_defined?(name)
12+
Rails.application&.reload_routes_unless_loaded
13+
super
14+
end
15+
end
16+
17+
module ProxyUrlHelpers
18+
def url_for(options)
19+
Rails.application&.reload_routes_unless_loaded
20+
super
21+
end
22+
23+
def full_url_for(options)
24+
Rails.application&.reload_routes_unless_loaded
25+
super
26+
end
27+
28+
def route_for(name, *args)
29+
Rails.application&.reload_routes_unless_loaded
30+
super
31+
end
32+
33+
def optimize_routes_generation?
34+
Rails.application&.reload_routes_unless_loaded
35+
super
36+
end
37+
38+
def polymorphic_url(record_or_hash_or_array, options = {})
39+
Rails.application&.reload_routes_unless_loaded
40+
super
41+
end
42+
43+
def polymorphic_path(record_or_hash_or_array, options = {})
44+
Rails.application&.reload_routes_unless_loaded
45+
super
46+
end
47+
end
48+
49+
def initialize(config = DEFAULT_CONFIG)
50+
super
51+
self.named_routes = NamedRouteCollection.new
52+
named_routes.url_helpers_module.prepend(method_missing_module)
53+
named_routes.path_helpers_module.prepend(method_missing_module)
54+
end
55+
56+
def generate_extras(options, recall = {})
57+
Rails.application&.reload_routes_unless_loaded
58+
59+
super(options, recall)
60+
end
61+
62+
def generate_url_helpers(supports_path)
63+
super.tap { |mod| mod.singleton_class.prepend(ProxyUrlHelpers) }
64+
end
65+
66+
def call(req)
67+
Rails.application&.reload_routes_unless_loaded
68+
super
69+
end
70+
71+
def draw(&block)
72+
Rails.application&.reload_routes_unless_loaded
73+
super
74+
end
75+
76+
def routes
77+
Rails.application&.reload_routes_unless_loaded
78+
super
79+
end
80+
81+
private
82+
def method_missing_module
83+
@method_missing_module ||= Module.new do
84+
private
85+
def method_missing(method_name, *args, &block)
86+
if Rails.application&.reload_routes_unless_loaded
87+
public_send(method_name, *args, &block)
88+
else
89+
super(method_name, *args, &block)
90+
end
91+
end
92+
93+
def respond_to_missing?(method_name, *args)
94+
if Rails.application&.reload_routes_unless_loaded
95+
respond_to?(method_name, *args)
96+
else
97+
super(method_name, *args)
98+
end
99+
end
100+
end
101+
end
102+
end
103+
end
104+
end

0 commit comments

Comments
 (0)