diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index cf144c94fea12..3fe505ca8d437 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,12 @@ +* Defer route drawing to the first request, or when url_helpers are called + + Executes the first routes reload in middleware, or when a route set's + url_helpers receives a route call / asked if it responds to a route. + Previously, this was executed unconditionally on boot, which can + slow down boot time unnecessarily for larger apps with lots of routes. + + *Gannon McGibbon* + * Add options to bin/rails app:update. `bin/rails app:update` now supports the same generic options that generators do: diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index bf20f1f29bfaa..db2cad9196471 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -9,6 +9,7 @@ require "active_support/encrypted_configuration" require "active_support/hash_with_indifferent_access" require "active_support/configuration_file" +require "active_support/parameter_filter" require "rails/engine" require "rails/autoloaders" diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 738e41f94274f..7a79fed064216 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -62,6 +62,8 @@ def build_stack middleware.use ::ActionDispatch::ActionableExceptions end + middleware.use ::Rails::Rack::LoadRoutes, app.routes_reloader unless app.config.eager_load + if config.reloading_enabled? middleware.use ::ActionDispatch::Reloader, app.reloader end diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 61246b7f0e91a..f144b45009855 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -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 @@ -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 diff --git a/railties/lib/rails/application/routes_reloader.rb b/railties/lib/rails/application/routes_reloader.rb index 569766dc5f64d..012499b0e884e 100644 --- a/railties/lib/rails/application/routes_reloader.rb +++ b/railties/lib/rails/application/routes_reloader.rb @@ -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 @@ -17,9 +17,11 @@ def initialize @route_sets = [] @external_routes = [] @eager_load = false + @loaded = false end def reload! + @loaded = true clear! load_paths finalize! @@ -28,6 +30,13 @@ def reload! revert end + def execute_unless_loaded + unless @loaded + execute + true + end + end + private def updater @updater ||= begin diff --git a/railties/lib/rails/commands/routes/routes_command.rb b/railties/lib/rails/commands/routes/routes_command.rb index c979a9014b8a1..e9a60101a2ed2 100644 --- a/railties/lib/rails/commands/routes/routes_command.rb +++ b/railties/lib/rails/commands/routes/routes_command.rb @@ -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) diff --git a/railties/lib/rails/commands/unused_routes/unused_routes_command.rb b/railties/lib/rails/commands/unused_routes/unused_routes_command.rb index 9fd4fbff14890..61179aa36c847 100644 --- a/railties/lib/rails/commands/unused_routes/unused_routes_command.rb +++ b/railties/lib/rails/commands/unused_routes/unused_routes_command.rb @@ -41,6 +41,7 @@ def action_missing? def perform(*) boot_application! + Rails.application.routes_reloader.execute_unless_loaded require "action_dispatch/routing/inspector" say(inspector.format(formatter, routes_filter)) diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 652b1c97565b7..a3ebdda19e8ae 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -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 @@ -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 diff --git a/railties/lib/rails/engine/route_set.rb b/railties/lib/rails/engine/route_set.rb new file mode 100644 index 0000000000000..7de3427871bae --- /dev/null +++ b/railties/lib/rails/engine/route_set.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +# :markup: markdown + +require "action_dispatch/routing/route_set" + +module Rails + class Engine + class RouteSet < ActionDispatch::Routing::RouteSet # :nodoc: + def initialize(config = DEFAULT_CONFIG) + super + named_routes.url_helpers_module.prepend(method_missing_module) + named_routes.path_helpers_module.prepend(method_missing_module) + end + + private + def method_missing_module + @method_missing_module ||= Module.new do + private + def method_missing(method_name, *args, &block) + application = Rails.application + 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 + + def respond_to_missing?(method_name, *args) + application = Rails.application + if application && application.initialized? && application.routes_reloader.execute_unless_loaded + ActiveSupport.run_load_hooks(:after_routes_loaded, application) + respond_to?(method_name, *args) + else + super(method_name, *args) + end + end + end + end + end + end +end diff --git a/railties/lib/rails/rack.rb b/railties/lib/rails/rack.rb index 579fb25cc4bd4..07b9006913192 100644 --- a/railties/lib/rails/rack.rb +++ b/railties/lib/rails/rack.rb @@ -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 diff --git a/railties/lib/rails/rack/load_routes.rb b/railties/lib/rails/rack/load_routes.rb new file mode 100644 index 0000000000000..d932aa02b7252 --- /dev/null +++ b/railties/lib/rails/rack/load_routes.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Rails + module Rack + class LoadRoutes + def initialize(app, routes_reloader) + @app = app + @called = false + @routes_reloader = routes_reloader + end + + def call(env) + @called ||= begin + @routes_reloader.execute_unless_loaded + true + end + @app.call(env) + end + end + end +end diff --git a/railties/test/application/initializers/frameworks_test.rb b/railties/test/application/initializers/frameworks_test.rb index 28f9d9b19031b..872352a0376b0 100644 --- a/railties/test/application/initializers/frameworks_test.rb +++ b/railties/test/application/initializers/frameworks_test.rb @@ -67,8 +67,8 @@ def notify RUBY app("development") - assert Foo.method_defined?(:foo_url) - assert Foo.method_defined?(:main_app) + assert Foo.new.respond_to?(:foo_url) + assert Foo.new.respond_to?(:main_app) end test "allows to not load all helpers for controllers" do diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index 360b550698cb6..e655e3380cc9b 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -310,6 +310,7 @@ class User Rails.configuration.after_routes_loaded do $counter *= 3 end + Rails.application.reload_routes! RUBY app_file "app/models/user.rb", <<-MODEL @@ -373,6 +374,7 @@ class User Rails.configuration.after_routes_loaded do $counter *= 3 end + Rails.application.reload_routes! RUBY boot_app "development" diff --git a/railties/test/application/rack/load_routes_test.rb b/railties/test/application/rack/load_routes_test.rb new file mode 100644 index 0000000000000..52690e7dc1c4b --- /dev/null +++ b/railties/test/application/rack/load_routes_test.rb @@ -0,0 +1,38 @@ +# 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 "/test" + + assert_equal(true, Rails.application.routes_reloader.loaded) + end + + test "loads routes only once" do + assert_called(Rails.application.routes_reloader, :execute_unless_loaded, 1) do + 5.times { get "/test" } + end + end + end + end +end diff --git a/railties/test/application/route_set_test.rb b/railties/test/application/route_set_test.rb new file mode 100644 index 0000000000000..531150aa3d181 --- /dev/null +++ b/railties/test/application/route_set_test.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" + +module Rails + class Engine + class RouteSetTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + setup :build_app + + teardown :teardown_app + + test "app lazily loads routes when invoking url helpers" do + require "#{app_path}/config/environment" + + assert_not_operator(:root_path, :in?, app_url_helpers.methods) + assert_equal("/", app_url_helpers.root_path) + end + + test "engine lazily loads routes when invoking url helpers" do + require "#{app_path}/config/environment" + + assert_not_operator(:root_path, :in?, engine_url_helpers.methods) + assert_equal("/plugin/", engine_url_helpers.root_path) + end + + test "app lazily loads routes when checking respond_to?" do + require "#{app_path}/config/environment" + + assert_not_operator(:root_path, :in?, app_url_helpers.methods) + assert_operator(app_url_helpers, :respond_to?, :root_path) + end + + test "engine lazily loads routes when checking respond_to?" do + require "#{app_path}/config/environment" + + assert_not_operator(:root_path, :in?, engine_url_helpers.methods) + assert_operator(engine_url_helpers, :respond_to?, :root_path) + end + + test "app lazily loads routes when making a request" do + require "#{app_path}/config/environment" + + @app = Rails.application + + assert_not_operator(:root_path, :in?, app_url_helpers.methods) + response = get("/") + assert_equal(200, response.first) + end + + test "engine lazily loads routes when making a request" do + require "#{app_path}/config/environment" + + @app = Rails.application + + assert_not_operator(:root_path, :in?, engine_url_helpers.methods) + response = get("/plugin/") + assert_equal(200, response.first) + end + + private + def build_app + super + + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + root to: proc { [200, {}, []] } + + mount Plugin::Engine, at: "/plugin" + end + RUBY + + build_engine + end + + def build_engine + engine "plugin" do |plugin| + plugin.write "lib/plugin.rb", <<~RUBY + module Plugin + class Engine < ::Rails::Engine + end + end + RUBY + plugin.write "config/routes.rb", <<~RUBY + Plugin::Engine.routes.draw do + root to: proc { [200, {}, []] } + end + RUBY + end + end + + def app_url_helpers + Rails.application.routes.url_helpers + end + + def engine_url_helpers + Plugin::Engine.routes.url_helpers + end + end + end +end diff --git a/railties/test/application/url_generation_test.rb b/railties/test/application/url_generation_test.rb index dc737089f6370..35e995e37a147 100644 --- a/railties/test/application/url_generation_test.rb +++ b/railties/test/application/url_generation_test.rb @@ -21,6 +21,7 @@ class MyApp < Rails::Application config.eager_load = false config.hosts << proc { true } config.secret_key_base = "b3c631c314c0bbca50c1b2843150fe33" + config.middleware.delete Rails::Rack::LoadRoutes end Rails.application.initialize! diff --git a/railties/test/commands/middleware_test.rb b/railties/test/commands/middleware_test.rb index 70b6f875826ea..b0373610ec821 100644 --- a/railties/test/commands/middleware_test.rb +++ b/railties/test/commands/middleware_test.rb @@ -39,6 +39,7 @@ def app "Rails::Rack::Logger", "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", + "Rails::Rack::LoadRoutes", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "ActiveRecord::Migration::CheckPending", @@ -76,6 +77,7 @@ def app "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", "ActionDispatch::ActionableExceptions", + "Rails::Rack::LoadRoutes", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "ActiveRecord::Migration::CheckPending", @@ -108,6 +110,7 @@ def app "Rails::Rack::Logger", "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", + "Rails::Rack::LoadRoutes", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "Rack::Head", diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index c5a24d4ac2c17..970c7d0b95a05 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -264,6 +264,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! diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 43b8bd3535224..26d01400e3ae8 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -950,8 +950,8 @@ class MyMailer < ActionMailer::Base assert_equal "bukkits_", Bukkits.table_name_prefix assert_equal "bukkits", Bukkits::Engine.engine_name assert_equal Bukkits.railtie_namespace, Bukkits::Engine - assert ::Bukkits::MyMailer.method_defined?(:foo_url) - assert_not ::Bukkits::MyMailer.method_defined?(:bar_url) + assert ::Bukkits::MyMailer.new.respond_to?(:foo_url) + assert_not ::Bukkits::MyMailer.new.respond_to?(:bar_url) get("/bar") assert_equal "/bar", last_response.body