Permalink
Browse files

Remove the laziness from the middleware stack.

  • Loading branch information...
1 parent d7f6f2b commit 19d8c8cbe4aae570e3b48080e3182e5634aa7aec @josevalim josevalim committed May 29, 2010
@@ -3,49 +3,15 @@
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 @@ def ==(middleware)
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 @@ def inspect
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 @@ def use(*args, &block)
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
@@ -66,29 +66,14 @@ def setup
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")
@@ -124,7 +124,10 @@ def load_generators
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 @@ def initializers
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
@@ -47,7 +47,7 @@ module Bootstrap
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
@@ -9,7 +9,7 @@ class Configuration < ::Rails::Engine::Configuration
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 @@ def initialize(*)
@session_store = :cookie_store
@session_options = {}
@time_zone = "UTC"
+ @middleware = app_middleware
end
def encoding=(value)
@@ -41,10 +42,6 @@ def encoding=(value)
end
end
- def middleware
- @middleware ||= app_middleware.merge_into(default_middleware_stack)
- end
-
def paths
@paths ||= begin
paths = super
@@ -134,27 +131,6 @@ def session_store(*args)
@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
@@ -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"
@@ -164,7 +164,7 @@ def boot!
end
def middleware
- AppTemplate::Application.middleware.active.map(&:klass).map(&:name)
+ AppTemplate::Application.middleware.map(&:klass).map(&:name)
end
def remote_ip(env = {})
@@ -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
@@ -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

7 comments on commit 19d8c8c

Contributor

anildigital replied May 30, 2010

Any reason why remove laziness from the middleware stack?

Contributor

josevalim replied May 30, 2010

Do you have any case where you need it? Rails doesn't.

Contributor

anildigital replied May 30, 2010

okay cool. just wanted to know.

Contributor

josevalim replied May 30, 2010

:)

Member

josh replied May 30, 2010

thank god this is gone

you could take it one step further and use Rack::Builder instead of MiddlewareStack

Contributor

jeroenvandijk replied May 31, 2010

I'm not sure whether Cucumber should adapt or not (I have to figure that out), but this is the error I get after this commit using cucumber: http://gist.github.com/419678

Contributor

josevalim replied May 31, 2010

This happens because Cucumber is trying to insert something after a middleware that does not exist necessarily. Please check: http://github.com/rails/rails/commit/19d8c8cbe4aae570e3b48080e3182e5634aa7aec#L2R160

Please sign in to comment.