Skip to content

Commit

Permalink
Defer route drawing to the first request, or when url_helpers called.
Browse files Browse the repository at this point in the history
Executes the first routes reload in middleware, or when the route set
url_helpers is called. Previously, this was executed unconditionally on
boot, which can slow down boot time unnecessarily for larger apps with
lots of routes.
  • Loading branch information
gmcgibbon committed May 3, 2024
1 parent 3d418c0 commit f292d80
Show file tree
Hide file tree
Showing 15 changed files with 146 additions and 8 deletions.
1 change: 1 addition & 0 deletions actionpack/lib/action_dispatch.rb
Expand Up @@ -71,6 +71,7 @@ class MissingController < NameError
autoload :DebugView
autoload :ExceptionWrapper
autoload :Executor
autoload :Once
autoload :Flash
autoload :PublicExceptions
autoload :Reloader
Expand Down
19 changes: 19 additions & 0 deletions actionpack/lib/action_dispatch/middleware/once.rb
@@ -0,0 +1,19 @@
# frozen_string_literal: true

# :markup: markdown

module ActionDispatch
class Once # :nodoc:
def initialize(app, block)
@app, @block, @called = app, block, false
end

def call(env)
@called ||= begin
@block.call
true
end
@app.call(env)
end
end
end
4 changes: 1 addition & 3 deletions actionpack/lib/action_dispatch/routing/route_set.rb
Expand Up @@ -345,9 +345,7 @@ def self.default_resources_path_names
{ new: "new", edit: "edit" }
end

def self.new_with_config(config)
route_set_config = DEFAULT_CONFIG

def self.new_with_config(config, route_set_config = DEFAULT_CONFIG)
# engines apparently don't have this set
if config.respond_to? :relative_url_root
route_set_config.relative_url_root = config.relative_url_root
Expand Down
26 changes: 26 additions & 0 deletions actionpack/test/dispatch/once_test.rb
@@ -0,0 +1,26 @@
# frozen_string_literal: true

require "abstract_unit"

module ActionDispatch
class OncesTest < ActionDispatch::IntegrationTest
setup do
@call_count = 0
@call_increment = proc { @call_count += 1 }
build_app
end

def test_runs_block_only_once
5.times { get "/test" }

assert_equal(@call_count, 1)
end

private
def build_app
@app = self.class.build_app do |middleware|
middleware.use Once, @call_increment
end
end
end
end
1 change: 1 addition & 0 deletions actionview/lib/action_view/rendering.rb
Expand Up @@ -63,6 +63,7 @@ def build_view_context_class(klass, supports_path, routes, helpers)

Class.new(klass) do
if routes
routes.application.routes_reloader.execute_unless_loaded if routes.respond_to?(:application)
include routes.url_helpers(supports_path)
include routes.mounted_helpers
end
Expand Down
1 change: 1 addition & 0 deletions railties/lib/rails/application/default_middleware_stack.rb
Expand Up @@ -63,6 +63,7 @@ def build_stack
end

if config.reloading_enabled?
middleware.use ::Rails::Rack::LoadRoutes, app.routes_reloader unless app.config.eager_load
middleware.use ::ActionDispatch::Reloader, app.reloader
end

Expand Down
8 changes: 6 additions & 2 deletions railties/lib/rails/application/finisher.rb
Expand Up @@ -159,7 +159,6 @@ def self.complete(_state)
initializer :set_routes_reloader_hook do |app|
reloader = routes_reloader
reloader.eager_load = app.config.eager_load
reloader.execute
reloaders << reloader

app.reloader.to_run do
Expand All @@ -177,7 +176,12 @@ def self.complete(_state)
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
end

ActiveSupport.run_load_hooks(:after_routes_loaded, self)
if reloader.eager_load
reloader.execute
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
elsif reloader.loaded
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
end
end

# Set clearing dependencies after the finisher hook to ensure paths
Expand Down
11 changes: 10 additions & 1 deletion railties/lib/rails/application/routes_reloader.rb
Expand Up @@ -7,7 +7,7 @@ class Application
class RoutesReloader
include ActiveSupport::Callbacks

attr_reader :route_sets, :paths, :external_routes
attr_reader :route_sets, :paths, :external_routes, :loaded
attr_accessor :eager_load
attr_writer :run_after_load_paths # :nodoc:
delegate :execute_if_updated, :execute, :updated?, to: :updater
Expand All @@ -17,9 +17,11 @@ def initialize
@route_sets = []
@external_routes = []
@eager_load = false
@loaded = false
end

def reload!
@loaded = true
clear!
load_paths
finalize!
Expand All @@ -28,6 +30,13 @@ def reload!
revert
end

def execute_unless_loaded
unless @loaded
execute
true
end
end

private
def updater
@updater ||= begin
Expand Down
1 change: 1 addition & 0 deletions railties/lib/rails/commands/routes/routes_command.rb
Expand Up @@ -23,6 +23,7 @@ def invoke_command(*)
desc "routes", "List all the defined routes"
def perform(*)
boot_application!
Rails.application.routes_reloader.execute_unless_loaded
require "action_dispatch/routing/inspector"

say inspector.format(formatter, routes_filter)
Expand Down
4 changes: 3 additions & 1 deletion railties/lib/rails/engine.rb
Expand Up @@ -349,6 +349,7 @@ module Rails
# config.railties_order = [Blog::Engine, :main_app, :all]
class Engine < Railtie
autoload :Configuration, "rails/engine/configuration"
autoload :RouteSet, "rails/engine/route_set"

class << self
attr_accessor :called_from, :isolated
Expand Down Expand Up @@ -544,7 +545,7 @@ def env_config
# Defines the routes for this engine. If a block is given to
# routes, it is appended to the engine.
def routes(&block)
@routes ||= ActionDispatch::Routing::RouteSet.new_with_config(config)
@routes ||= RouteSet.new_with_config(config)
@routes.append(&block) if block_given?
@routes
end
Expand Down Expand Up @@ -599,6 +600,7 @@ def load_seed
app.routes_reloader.paths.unshift(*routing_paths)
app.routes_reloader.route_sets << routes
app.routes_reloader.external_routes.unshift(*external_paths)
routes.application = app
end
end

Expand Down
31 changes: 31 additions & 0 deletions railties/lib/rails/engine/route_set.rb
@@ -0,0 +1,31 @@
# frozen_string_literal: true

# :markup: markdown

require "action_dispatch/routing/route_set"

module Rails
class Engine
class RouteSet < ActionDispatch::Routing::RouteSet # :nodoc:
attr_accessor :application

def generate_url_helpers(supports_path)
application = self.application

method_missing = Module.new do
define_method(:method_missing) do |method_name, *args, &block|
if application && application.initialized? && application.routes_reloader.execute_unless_loaded
ActiveSupport.run_load_hooks(:after_routes_loaded, application)
public_send(method_name, *args, &block)
else
super(method_name, *args, &block)
end
end
end
super.tap do |mod|
mod.singleton_class.prepend(method_missing)
end
end
end
end
end
3 changes: 2 additions & 1 deletion railties/lib/rails/rack.rb
Expand Up @@ -2,6 +2,7 @@

module Rails
module Rack
autoload :Logger, "rails/rack/logger"
autoload :Logger, "rails/rack/logger"
autoload :LoadRoutes, "rails/rack/load_routes"
end
end
11 changes: 11 additions & 0 deletions railties/lib/rails/rack/load_routes.rb
@@ -0,0 +1,11 @@
# frozen_string_literal: true

module Rails
module Rack
class LoadRoutes < ActionDispatch::Once # :nodoc:
def initialize(app, routes_reloader)
super(app, proc { routes_reloader.execute_unless_loaded })
end
end
end
end
32 changes: 32 additions & 0 deletions railties/test/application/rack/load_routes_test.rb
@@ -0,0 +1,32 @@
# frozen_string_literal: true

require "isolation/abstract_unit"
require "active_support/log_subscriber/test_helper"
require "rack/test"

module ApplicationTests
module RackTests
class LoggerTest < ActiveSupport::TestCase
include ActiveSupport::Testing::Isolation
include ActiveSupport::LogSubscriber::TestHelper
include Rack::Test::Methods

setup do
build_app
require "#{app_path}/config/environment"
end

teardown do
teardown_app
end

test "loads routes on request" do
assert_equal(false, Rails.application.routes_reloader.loaded)

get "/blah"

assert_equal(true, Rails.application.routes_reloader.loaded)
end
end
end
end
1 change: 1 addition & 0 deletions railties/test/isolation/abstract_unit.rb
Expand Up @@ -265,6 +265,7 @@ def self.name; "RailtiesTestApp"; end
@app.config.active_support.deprecation = :log
@app.config.log_level = :info
@app.config.secret_key_base = "b3c631c314c0bbca50c1b2843150fe33"
@app.config.middleware.delete Rails::Rack::LoadRoutes

yield @app if block_given?
@app.initialize!
Expand Down

0 comments on commit f292d80

Please sign in to comment.