From 2740943634fe151fb3fb87e2af7881a93f5dd6b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 29 May 2010 22:29:14 +0200 Subject: Remove the laziness from the middleware stack. --- actionpack/lib/action_dispatch/middleware/stack.rb | 72 +++++----------------- actionpack/test/dispatch/middleware_stack_test.rb | 19 +----- railties/lib/rails/application.rb | 28 ++++++++- railties/lib/rails/application/bootstrap.rb | 2 +- railties/lib/rails/application/configuration.rb | 28 +-------- railties/lib/rails/tasks/middleware.rake | 2 +- railties/test/application/middleware_test.rb | 2 +- .../generators/integration_test_generator_test.rb | 2 +- .../generators/performance_test_generator_test.rb | 2 +- 9 files changed, 51 insertions(+), 106 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 5c5362ce4a..0e5ab507df 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -3,49 +3,15 @@ require "active_support/inflector/methods" module ActionDispatch class MiddlewareStack < Array class Middleware - def self.new(klass, *args, &block) - if klass.is_a?(self) - klass - else - super - end - end - attr_reader :args, :block def initialize(klass, *args, &block) - @klass = klass - - options = args.extract_options! - if options.has_key?(:if) - @conditional = options.delete(:if) - else - @conditional = true - end - args << options unless options.empty? - - @args = args - @block = block + @klass, @args, @block = klass, args, block end def klass - if @klass.respond_to?(:new) - @klass - elsif @klass.respond_to?(:call) - @klass.call - else - ActiveSupport::Inflector.constantize(@klass.to_s) - end - end - - def active? - return false unless klass - - if @conditional.respond_to?(:call) - @conditional.call - else - @conditional - end + return @klass if @klass.respond_to?(:new) + @klass = ActiveSupport::Inflector.constantize(@klass.to_s) end def ==(middleware) @@ -58,7 +24,7 @@ module ActionDispatch if lazy_compare?(@klass) && lazy_compare?(middleware) normalize(@klass) == normalize(middleware) else - klass.name == middleware.to_s + klass.name == normalize(middleware.to_s) end end end @@ -68,25 +34,18 @@ module ActionDispatch end def build(app) - if block - klass.new(app, *build_args, &block) - else - klass.new(app, *build_args) - end + klass.new(app, *args, &block) end - private - def lazy_compare?(object) - object.is_a?(String) || object.is_a?(Symbol) - end + private - def normalize(object) - object.to_s.strip.sub(/^::/, '') - end + def lazy_compare?(object) + object.is_a?(String) || object.is_a?(Symbol) + end - def build_args - Array(args).map { |arg| arg.respond_to?(:call) ? arg.call : arg } - end + def normalize(object) + object.to_s.strip.sub(/^::/, '') + end end def initialize(*args, &block) @@ -119,15 +78,14 @@ module ActionDispatch end def active - find_all { |middleware| middleware.active? } + ActiveSupport::Deprecation.warn "All middlewares in the chaing are active since the laziness " << + "was removed from the middleware stack", caller end def build(app = nil, &blk) app ||= blk - raise "MiddlewareStack#build requires an app" unless app - - active.reverse.inject(app) { |a, e| e.build(a) } + reverse.inject(app) { |a, e| e.build(a) } end end end diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index 7cf6365af3..170c5b8565 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -66,29 +66,14 @@ class MiddlewareStackTest < ActiveSupport::TestCase assert_equal BazMiddleware, @stack[0].klass end - test "active returns all only enabled middleware" do - assert_no_difference "@stack.active.size" do - assert_difference "@stack.size" do - @stack.use BazMiddleware, :if => lambda { false } - end - end - end - test "lazy evaluates middleware class" do assert_difference "@stack.size" do - @stack.use lambda { BazMiddleware } + @stack.use "MiddlewareStackTest::BazMiddleware" end assert_equal BazMiddleware, @stack.last.klass end - test "lazy evaluates middleware arguments" do - assert_difference "@stack.size" do - @stack.use BazMiddleware, lambda { :foo } - end - assert_equal [:foo], @stack.last.send(:build_args) - end - - test "lazy compares so unloaded constants can be loaded" do + test "lazy compares so unloaded constants are not loaded" do @stack.use "UnknownMiddleware" @stack.use :"MiddlewareStackTest::BazMiddleware" assert @stack.include?("::MiddlewareStackTest::BazMiddleware") diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index c588a41443..7416e94eeb 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -124,7 +124,10 @@ module Rails end def app - @app ||= config.middleware.build(routes) + @app ||= begin + config.middleware = config.middleware.merge_into(default_middleware_stack) + config.middleware.build(routes) + end end def call(env) @@ -148,6 +151,29 @@ module Rails protected + def default_middleware_stack + ActionDispatch::MiddlewareStack.new.tap do |middleware| + middleware.use ::ActionDispatch::Static, paths.public.to_a.first if config.serve_static_assets + middleware.use ::Rack::Lock if !config.allow_concurrency + middleware.use ::Rack::Runtime + middleware.use ::Rails::Rack::Logger + middleware.use ::ActionDispatch::ShowExceptions, config.consider_all_requests_local if config.action_dispatch.show_exceptions + middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies + middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header + middleware.use ::ActionDispatch::Callbacks, !config.cache_classes + middleware.use ::ActionDispatch::Cookies + + if config.session_store + middleware.use config.session_store, config.session_options + middleware.use ::ActionDispatch::Flash + end + + middleware.use ::ActionDispatch::ParamsParser + middleware.use ::Rack::MethodOverride + middleware.use ::ActionDispatch::Head + end + end + def initialize_tasks require "rails/tasks" task :environment do diff --git a/railties/lib/rails/application/bootstrap.rb b/railties/lib/rails/application/bootstrap.rb index e62eed8a87..0a435f0f36 100644 --- a/railties/lib/rails/application/bootstrap.rb +++ b/railties/lib/rails/application/bootstrap.rb @@ -47,7 +47,7 @@ module Rails silence_warnings { Object.const_set "RAILS_CACHE", ActiveSupport::Cache.lookup_store(config.cache_store) } if RAILS_CACHE.respond_to?(:middleware) - config.middleware.insert_after(:"Rack::Lock", RAILS_CACHE.middleware) + config.middleware.insert_before("Rack::Runtime", RAILS_CACHE.middleware) end end end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index f3a326f36d..25e54e9dce 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -9,7 +9,7 @@ module Rails attr_accessor :allow_concurrency, :cache_classes, :cache_store, :encoding, :consider_all_requests_local, :dependency_loading, - :filter_parameters, :log_level, :logger, + :filter_parameters, :log_level, :logger, :middleware, :plugins, :preload_frameworks, :reload_engines, :reload_plugins, :secret_token, :serve_static_assets, :session_options, :time_zone, :whiny_nils @@ -25,6 +25,7 @@ module Rails @session_store = :cookie_store @session_options = {} @time_zone = "UTC" + @middleware = app_middleware end def encoding=(value) @@ -41,10 +42,6 @@ module Rails end end - def middleware - @middleware ||= app_middleware.merge_into(default_middleware_stack) - end - def paths @paths ||= begin paths = super @@ -134,27 +131,6 @@ module Rails @session_options = args.shift || {} end end - - protected - - def default_middleware_stack - ActionDispatch::MiddlewareStack.new.tap do |middleware| - middleware.use('::ActionDispatch::Static', lambda { paths.public.to_a.first }, :if => lambda { serve_static_assets }) - middleware.use('::Rack::Lock', :if => lambda { !allow_concurrency }) - middleware.use('::Rack::Runtime') - middleware.use('::Rails::Rack::Logger') - middleware.use('::ActionDispatch::ShowExceptions', lambda { consider_all_requests_local }, :if => lambda { action_dispatch.show_exceptions }) - middleware.use('::ActionDispatch::RemoteIp', lambda { action_dispatch.ip_spoofing_check }, lambda { action_dispatch.trusted_proxies }) - middleware.use('::Rack::Sendfile', lambda { action_dispatch.x_sendfile_header }) - middleware.use('::ActionDispatch::Callbacks', lambda { !cache_classes }) - middleware.use('::ActionDispatch::Cookies') - middleware.use(lambda { session_store }, lambda { session_options }) - middleware.use('::ActionDispatch::Flash', :if => lambda { session_store }) - middleware.use('::ActionDispatch::ParamsParser') - middleware.use('::Rack::MethodOverride') - middleware.use('::ActionDispatch::Head') - end - end end end end diff --git a/railties/lib/rails/tasks/middleware.rake b/railties/lib/rails/tasks/middleware.rake index 251da67c96..e670989345 100644 --- a/railties/lib/rails/tasks/middleware.rake +++ b/railties/lib/rails/tasks/middleware.rake @@ -1,6 +1,6 @@ desc 'Prints out your Rack middleware stack' task :middleware => :environment do - Rails.configuration.middleware.active.each do |middleware| + Rails.configuration.middleware.each do |middleware| puts "use #{middleware.inspect}" end puts "run #{Rails::Application.instance.class.name}.routes" diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 78e7c39660..bab17d8af5 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -164,7 +164,7 @@ module ApplicationTests end def middleware - AppTemplate::Application.middleware.active.map(&:klass).map(&:name) + AppTemplate::Application.middleware.map(&:klass).map(&:name) end def remote_ip(env = {}) diff --git a/railties/test/generators/integration_test_generator_test.rb b/railties/test/generators/integration_test_generator_test.rb index cf282a0911..d05ed76d24 100644 --- a/railties/test/generators/integration_test_generator_test.rb +++ b/railties/test/generators/integration_test_generator_test.rb @@ -7,6 +7,6 @@ class IntegrationTestGeneratorTest < Rails::Generators::TestCase def test_integration_test_skeleton_is_created run_generator - assert_file "test/integration/integration_test.rb", /class IntegrationTest < ActionController::IntegrationTest/ + assert_file "test/integration/integration_test.rb", /class IntegrationTest < ActionDispatch::IntegrationTest/ end end diff --git a/railties/test/generators/performance_test_generator_test.rb b/railties/test/generators/performance_test_generator_test.rb index 8fda83b36f..37f9857193 100644 --- a/railties/test/generators/performance_test_generator_test.rb +++ b/railties/test/generators/performance_test_generator_test.rb @@ -7,6 +7,6 @@ class PerformanceTestGeneratorTest < Rails::Generators::TestCase def test_performance_test_skeleton_is_created run_generator - assert_file "test/performance/performance_test.rb", /class PerformanceTest < ActionController::PerformanceTest/ + assert_file "test/performance/performance_test.rb", /class PerformanceTest < ActionDispatch::PerformanceTest/ end end -- cgit v1.2.3