From 5f8e48cbd297aca4add4b48efa2136ba6ac851b1 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 14 Dec 2009 17:54:41 -0600 Subject: [PATCH] Move route reloading into railties --- .../action_controller/dispatch/dispatcher.rb | 5 - .../lib/action_dispatch/routing/route_set.rb | 109 ++++-------------- .../testing/assertions/routing.rb | 2 - actionpack/test/controller/dispatcher_test.rb | 13 --- railties/lib/rails/application.rb | 49 +++++++- railties/lib/rails/console_app.rb | 2 +- railties/lib/rails/plugin.rb | 4 +- 7 files changed, 69 insertions(+), 115 deletions(-) diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb index e04da42637..cf02757cf6 100644 --- a/actionpack/lib/action_controller/dispatch/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatch/dispatcher.rb @@ -13,11 +13,6 @@ module ActionController # Run prepare callbacks before every request in development mode self.prepare_each_request = true - # Development mode callbacks - ActionDispatch::Callbacks.before_dispatch do |app| - ActionController::Routing::Routes.reload - end - ActionDispatch::Callbacks.after_dispatch do # Cleanup the application before processing the current request. ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 6f35e9b4e3..bf2443c1be 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -203,23 +203,18 @@ module ActionDispatch end end - attr_accessor :routes, :named_routes, :configuration_files, :controller_paths + attr_accessor :routes, :named_routes + attr_accessor :disable_clear_and_finalize def initialize - self.configuration_files = [] - self.controller_paths = [] - self.routes = [] self.named_routes = NamedRouteCollection.new - @clear_before_draw = true - @finalize_set_on_draw = true - - clear! + @disable_clear_and_finalize = false end def draw(&block) - clear! if @clear_before_draw + clear! unless @disable_clear_and_finalize mapper = Mapper.new(self) if block.arity == 1 @@ -228,16 +223,20 @@ module ActionDispatch mapper.instance_exec(&block) end - if @finalize_set_on_draw - @set.add_route(NotFound) - install_helpers - @set.freeze - end + finalize! unless @disable_clear_and_finalize nil end + def finalize! + @set.add_route(NotFound) + install_helpers + @set.freeze + end + def clear! + # Clear the controller cache so we may discover new ones + @controller_constraints = nil routes.clear named_routes.clear @set = ::Rack::Mount::RouteSet.new(:parameters_key => PARAMETERS_KEY) @@ -252,75 +251,6 @@ module ActionDispatch routes.empty? end - def add_configuration_file(path) - self.configuration_files << path - end - - # Deprecated accessor - def configuration_file=(path) - add_configuration_file(path) - end - - # Deprecated accessor - def configuration_file - configuration_files - end - - def load! - # Clear the controller cache so we may discover new ones - @controller_constraints = nil - - load_routes! - end - - # reload! will always force a reload whereas load checks the timestamp first - alias reload! load! - - def reload - if configuration_files.any? && @routes_last_modified - if routes_changed_at == @routes_last_modified - return # routes didn't change, don't reload - else - @routes_last_modified = routes_changed_at - end - end - - load! - end - - def load_routes! - if configuration_files.any? - @finalize_set_on_draw = false - configuration_files.each_with_index do |config, index| - @finalize_set_on_draw = true if index == (configuration_files.length - 1) - load(config) - @clear_before_draw = false if index == 0 - end - @clear_before_draw = true - @finalize_set_on_draw = true - - @routes_last_modified = routes_changed_at - else - draw do |map| - map.connect ":controller/:action/:id" - end - end - end - - def routes_changed_at - routes_changed_at = nil - - configuration_files.each do |config| - config_changed_at = File.stat(config).mtime - - if routes_changed_at.nil? || config_changed_at > routes_changed_at - routes_changed_at = config_changed_at - end - end - - routes_changed_at - end - CONTROLLER_REGEXP = /[_a-zA-Z0-9]+/ def controller_constraints @@ -340,11 +270,14 @@ module ActionDispatch namespaces << controller_name.split('/')[0...-1].join('/') end - # Find namespaces in controllers/ directory - controller_paths.each do |load_path| - load_path = File.expand_path(load_path) - Dir["#{load_path}/**/*_controller.rb"].collect do |path| - namespaces << File.dirname(path).sub(/#{load_path}\/?/, '') + # TODO: Move this into Railties + if defined?(Rails.application) + # Find namespaces in controllers/ directory + Rails.application.configuration.controller_paths.each do |load_path| + load_path = File.expand_path(load_path) + Dir["#{load_path}/**/*_controller.rb"].collect do |path| + namespaces << File.dirname(path).sub(/#{load_path}\/?/, '') + end end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 794fb888b7..fc477afb17 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -46,7 +46,6 @@ module ActionDispatch request_method = nil end - ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? request = recognized_request_for(path, request_method) expected_options = expected_options.clone @@ -80,7 +79,6 @@ module ActionDispatch def assert_generates(expected_path, options, defaults={}, extras = {}, message=nil) expected_path = "/#{expected_path}" unless expected_path[0] == ?/ # Load routes.rb if it hasn't been loaded. - ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? generated_path, extra_keys = ActionController::Routing::Routes.generate_extras(options, defaults) found_extras = options.reject {|k, v| ! extra_keys.include? k} diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index 622d67287d..64f1ad7610 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -15,7 +15,6 @@ class DispatcherTest < Test::Unit::TestCase ActionDispatch::Callbacks.reset_callbacks(:call) ActionController::Routing::Routes.stubs(:call).returns([200, {}, 'response']) - ActionController::Routing::Routes.stubs(:reload) Dispatcher.stubs(:require_dependency) end @@ -28,18 +27,6 @@ class DispatcherTest < Test::Unit::TestCase dispatch(false) end - def test_reloads_routes_before_dispatch_if_in_loading_mode - ActionController::Routing::Routes.expects(:reload).once - dispatch(false) - end - - def test_leaves_dependencies_after_dispatch_if_not_in_loading_mode - ActionController::Routing::Routes.expects(:reload).never - ActiveSupport::Dependencies.expects(:clear).never - - dispatch - end - def test_prepare_callbacks a = b = c = nil ActionDispatch::Callbacks.to_prepare { |*args| a = b = c = 1 } diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 110311558c..047d252625 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -42,8 +42,13 @@ module Rails end end + attr_reader :route_configuration_files + def initialize Rails.application ||= self + + @route_configuration_files = [] + run_initializers(self) end @@ -65,6 +70,32 @@ module Rails ActionController::Routing::Routes end + def routes_changed_at + routes_changed_at = nil + + route_configuration_files.each do |config| + config_changed_at = File.stat(config).mtime + + if routes_changed_at.nil? || config_changed_at > routes_changed_at + routes_changed_at = config_changed_at + end + end + + routes_changed_at + end + + def reload_routes! + routes.disable_clear_and_finalize = true + + routes.clear! + route_configuration_files.each { |config| load(config) } + routes.finalize! + + nil + ensure + routes.disable_clear_and_finalize = false + end + def initializers initializers = super plugins.each { |p| initializers += p.initializers } @@ -359,6 +390,18 @@ module Rails next unless configuration.frameworks.include?(:action_controller) require 'rails/dispatcher' unless defined?(::Dispatcher) Dispatcher.define_dispatcher_callbacks(configuration.cache_classes) + + unless configuration.cache_classes + # Setup dev mode route reloading + routes_last_modified = routes_changed_at + reload_routes = lambda do + unless routes_changed_at == routes_last_modified + routes_last_modified = routes_changed_at + reload_routes! + end + end + ActionDispatch::Callbacks.before_dispatch { |callbacks| reload_routes } + end end # Routing must be initialized after plugins to allow the former to extend the routes @@ -368,10 +411,8 @@ module Rails # loading module used to lazily load controllers (Configuration#controller_paths). initializer :initialize_routing do next unless configuration.frameworks.include?(:action_controller) - - ActionController::Routing::Routes.controller_paths += configuration.controller_paths - ActionController::Routing::Routes.add_configuration_file(configuration.routes_configuration_file) - ActionController::Routing::Routes.reload! + route_configuration_files << configuration.routes_configuration_file + reload_routes! end # # # Observers are loaded after plugins in case Observers or observed models are modified by plugins. diff --git a/railties/lib/rails/console_app.rb b/railties/lib/rails/console_app.rb index 1ad62e5058..2c4a7a51e8 100644 --- a/railties/lib/rails/console_app.rb +++ b/railties/lib/rails/console_app.rb @@ -27,6 +27,6 @@ end def reload! puts "Reloading..." ActionDispatch::Callbacks.new(lambda {}, true) - ActionController::Routing::Routes.reload + Rails.application.reload_routes! true end diff --git a/railties/lib/rails/plugin.rb b/railties/lib/rails/plugin.rb index 86bf032641..8025135a64 100644 --- a/railties/lib/rails/plugin.rb +++ b/railties/lib/rails/plugin.rb @@ -55,8 +55,8 @@ module Rails initializer :add_routing_file, :after => :initialize_routing do |app| routing_file = "#{path}/config/routes.rb" if File.exist?(routing_file) - app.routes.add_configuration_file(routing_file) - app.routes.reload! + app.route_configuration_files << routing_file + app.reload_routes! end end end