mirror of https://github.com/rails/rails
Revert "Merge pull request #51614 from gmcgibbon/defer_route_drawing"
This reverts commite97db3b395
, reversing changes made toa27a1751cf
. This is breaking application routes when running without eager load enabled.
This commit is contained in:
parent
272aa3bd15
commit
d189cbcb56
|
@ -46,9 +46,7 @@ module ActionDispatch
|
|||
def create_routes
|
||||
app = self.app
|
||||
routes = ActionDispatch::Routing::RouteSet.new
|
||||
middleware = app.config.middleware.dup
|
||||
middleware.delete(Rails::Rack::LoadRoutes) if defined?(Rails::Rack::LoadRoutes)
|
||||
rack_app = middleware.build(routes)
|
||||
rack_app = app.config.middleware.build(routes)
|
||||
https = integration_session.https?
|
||||
host = integration_session.host
|
||||
|
||||
|
|
|
@ -4,11 +4,7 @@ require "abstract_unit"
|
|||
require "rails/engine"
|
||||
require "controller/fake_controllers"
|
||||
|
||||
class SecureArticlesController < ArticlesController
|
||||
def index
|
||||
render(inline: "")
|
||||
end
|
||||
end
|
||||
class SecureArticlesController < ArticlesController; end
|
||||
class BlockArticlesController < ArticlesController; end
|
||||
class QueryArticlesController < ArticlesController; end
|
||||
|
||||
|
@ -280,20 +276,6 @@ class RoutingAssertionsControllerTest < ActionController::TestCase
|
|||
|
||||
class WithRoutingTest < ActionController::TestCase
|
||||
include RoutingAssertionsSharedTests::WithRoutingSharedTests
|
||||
|
||||
test "with_routing routes are reachable" do
|
||||
@controller = SecureArticlesController.new
|
||||
|
||||
with_routing do |routes|
|
||||
routes.draw do
|
||||
get :new_route, to: "secure_articles#index"
|
||||
end
|
||||
|
||||
get :index
|
||||
|
||||
assert_predicate(response, :ok?)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -313,18 +295,6 @@ class RoutingAssertionsIntegrationTest < ActionDispatch::IntegrationTest
|
|||
|
||||
class WithRoutingTest < ActionDispatch::IntegrationTest
|
||||
include RoutingAssertionsSharedTests::WithRoutingSharedTests
|
||||
|
||||
test "with_routing routes are reachable" do
|
||||
with_routing do |routes|
|
||||
routes.draw do
|
||||
get :new_route, to: "secure_articles#index"
|
||||
end
|
||||
|
||||
get "/new_route"
|
||||
|
||||
assert_predicate(response, :ok?)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
class WithRoutingSettingsTest < ActionDispatch::IntegrationTest
|
||||
|
|
|
@ -9,7 +9,6 @@ require "active_support/deprecation"
|
|||
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"
|
||||
|
||||
|
|
|
@ -62,8 +62,6 @@ module Rails
|
|||
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
|
||||
|
|
|
@ -159,6 +159,7 @@ module Rails
|
|||
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
|
||||
|
@ -176,12 +177,7 @@ module Rails
|
|||
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
|
||||
end
|
||||
|
||||
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
|
||||
ActiveSupport.run_load_hooks(:after_routes_loaded, self)
|
||||
end
|
||||
|
||||
# Set clearing dependencies after the finisher hook to ensure paths
|
||||
|
|
|
@ -7,7 +7,7 @@ module Rails
|
|||
class RoutesReloader
|
||||
include ActiveSupport::Callbacks
|
||||
|
||||
attr_reader :route_sets, :paths, :external_routes, :loaded
|
||||
attr_reader :route_sets, :paths, :external_routes
|
||||
attr_accessor :eager_load
|
||||
attr_writer :run_after_load_paths # :nodoc:
|
||||
delegate :execute_if_updated, :execute, :updated?, to: :updater
|
||||
|
@ -17,11 +17,9 @@ module Rails
|
|||
@route_sets = []
|
||||
@external_routes = []
|
||||
@eager_load = false
|
||||
@loaded = false
|
||||
end
|
||||
|
||||
def reload!
|
||||
@loaded = true
|
||||
clear!
|
||||
load_paths
|
||||
finalize!
|
||||
|
@ -30,13 +28,6 @@ module Rails
|
|||
revert
|
||||
end
|
||||
|
||||
def execute_unless_loaded
|
||||
unless @loaded
|
||||
execute
|
||||
true
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
def updater
|
||||
@updater ||= begin
|
||||
|
|
|
@ -23,7 +23,6 @@ module Rails
|
|||
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)
|
||||
|
|
|
@ -41,7 +41,6 @@ module Rails
|
|||
|
||||
def perform(*)
|
||||
boot_application!
|
||||
Rails.application.routes_reloader.execute_unless_loaded
|
||||
require "action_dispatch/routing/inspector"
|
||||
|
||||
say(inspector.format(formatter, routes_filter))
|
||||
|
|
|
@ -349,7 +349,6 @@ 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 +543,7 @@ module Rails
|
|||
# Defines the routes for this engine. If a block is given to
|
||||
# routes, it is appended to the engine.
|
||||
def routes(&block)
|
||||
@routes ||= RouteSet.new_with_config(config)
|
||||
@routes ||= ActionDispatch::Routing::RouteSet.new_with_config(config)
|
||||
@routes.append(&block) if block_given?
|
||||
@routes
|
||||
end
|
||||
|
|
|
@ -1,43 +0,0 @@
|
|||
# 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
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
module Rails
|
||||
module Rack
|
||||
autoload :Logger, "rails/rack/logger"
|
||||
autoload :LoadRoutes, "rails/rack/load_routes"
|
||||
autoload :Logger, "rails/rack/logger"
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,21 +0,0 @@
|
|||
# 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
|
|
@ -67,8 +67,8 @@ module ApplicationTests
|
|||
RUBY
|
||||
|
||||
app("development")
|
||||
assert Foo.new.respond_to?(:foo_url)
|
||||
assert Foo.new.respond_to?(:main_app)
|
||||
assert Foo.method_defined?(:foo_url)
|
||||
assert Foo.method_defined?(:main_app)
|
||||
end
|
||||
|
||||
test "allows to not load all helpers for controllers" do
|
||||
|
|
|
@ -310,7 +310,6 @@ class LoadingTest < ActiveSupport::TestCase
|
|||
Rails.configuration.after_routes_loaded do
|
||||
$counter *= 3
|
||||
end
|
||||
Rails.application.reload_routes!
|
||||
RUBY
|
||||
|
||||
app_file "app/models/user.rb", <<-MODEL
|
||||
|
@ -374,7 +373,6 @@ class LoadingTest < ActiveSupport::TestCase
|
|||
Rails.configuration.after_routes_loaded do
|
||||
$counter *= 3
|
||||
end
|
||||
Rails.application.reload_routes!
|
||||
RUBY
|
||||
|
||||
boot_app "development"
|
||||
|
|
|
@ -1,38 +0,0 @@
|
|||
# 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
|
|
@ -1,102 +0,0 @@
|
|||
# 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
|
|
@ -21,7 +21,6 @@ module ApplicationTests
|
|||
config.eager_load = false
|
||||
config.hosts << proc { true }
|
||||
config.secret_key_base = "b3c631c314c0bbca50c1b2843150fe33"
|
||||
config.middleware.delete Rails::Rack::LoadRoutes
|
||||
end
|
||||
|
||||
Rails.application.initialize!
|
||||
|
|
|
@ -39,7 +39,6 @@ class Rails::Command::MiddlewareTest < ActiveSupport::TestCase
|
|||
"Rails::Rack::Logger",
|
||||
"ActionDispatch::ShowExceptions",
|
||||
"ActionDispatch::DebugExceptions",
|
||||
"Rails::Rack::LoadRoutes",
|
||||
"ActionDispatch::Reloader",
|
||||
"ActionDispatch::Callbacks",
|
||||
"ActiveRecord::Migration::CheckPending",
|
||||
|
@ -77,7 +76,6 @@ class Rails::Command::MiddlewareTest < ActiveSupport::TestCase
|
|||
"ActionDispatch::ShowExceptions",
|
||||
"ActionDispatch::DebugExceptions",
|
||||
"ActionDispatch::ActionableExceptions",
|
||||
"Rails::Rack::LoadRoutes",
|
||||
"ActionDispatch::Reloader",
|
||||
"ActionDispatch::Callbacks",
|
||||
"ActiveRecord::Migration::CheckPending",
|
||||
|
@ -110,7 +108,6 @@ class Rails::Command::MiddlewareTest < ActiveSupport::TestCase
|
|||
"Rails::Rack::Logger",
|
||||
"ActionDispatch::ShowExceptions",
|
||||
"ActionDispatch::DebugExceptions",
|
||||
"Rails::Rack::LoadRoutes",
|
||||
"ActionDispatch::Reloader",
|
||||
"ActionDispatch::Callbacks",
|
||||
"Rack::Head",
|
||||
|
|
|
@ -264,7 +264,6 @@ module TestHelpers
|
|||
@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!
|
||||
|
|
|
@ -950,8 +950,8 @@ en:
|
|||
assert_equal "bukkits_", Bukkits.table_name_prefix
|
||||
assert_equal "bukkits", Bukkits::Engine.engine_name
|
||||
assert_equal Bukkits.railtie_namespace, Bukkits::Engine
|
||||
assert ::Bukkits::MyMailer.new.respond_to?(:foo_url)
|
||||
assert_not ::Bukkits::MyMailer.new.respond_to?(:bar_url)
|
||||
assert ::Bukkits::MyMailer.method_defined?(:foo_url)
|
||||
assert_not ::Bukkits::MyMailer.method_defined?(:bar_url)
|
||||
|
||||
get("/bar")
|
||||
assert_equal "/bar", last_response.body
|
||||
|
|
Loading…
Reference in New Issue