mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
40fc1651ad
While trying to fix #16433, we made the middleware deletions always happen at the end. While this works for the case of deleting the Rack::Runtime middleware, it makes operations like the following misbehave. ```ruby gem "bundler", "< 1.16" begin require "bundler/inline" rescue LoadError => e $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler" raise e end gemfile(true) do source "https://rubygems.org" git_source(:github) { |repo| "https://github.com/#{repo}.git" } gem "rails", github: "rails/rails" end require "action_controller/railtie" class TestApp < Rails::Application config.root = __dir__ secrets.secret_key_base = "secret_key_base" config.logger = Logger.new($stdout) Rails.logger = config.logger middleware.insert_after ActionDispatch::Session::CookieStore, ::Rails::Rack::Logger, config.log_tags middleware.delete ::Rails::Rack::Logger end require "minitest/autorun" require "rack/test" class BugTest < Minitest::Test include Rack::Test::Methods def test_returns_success get "/" assert last_response.ok? end private def app Rails.application end end ``` In the case ☝️ the ::Rails::Rack::Logger would be deleted instead of moved, because the order of middleware stack building execution will be: ```ruby [:insert, ActionDispatch::Session::CookieStore, [::Rails::Rack::Logger]] [:delete, ::Rails::Rack::Logger, [config.log_tags]] ``` This is pretty surprising and hard to reason about behaviour, unless you go spelunking into the Rails configuration code. I have a few solutions in mind and all of them have their drawbacks. 1. Introduce a `Rails::Configuration::MiddlewareStackProxy#delete!` that delays the deleted operations. This will make `#delete` to be executed in order. The drawback here is backwards incompatible behavior and a new public method. 2. Just revert to the old operations. This won't allow people to delete the `Rack::Runtime` middleware. 3. Legitimize the middleware moving with the new `#move_after` and `#move_before` methods. This does not breaks any backwards compatibility, but includes 2 new methods to the middleware stack. I have implemented `3.` in this pull request. Happy holidays! 🎄
170 lines
4.1 KiB
Ruby
170 lines
4.1 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
require "active_support/inflector/methods"
|
|
require "active_support/dependencies"
|
|
|
|
module ActionDispatch
|
|
class MiddlewareStack
|
|
class Middleware
|
|
attr_reader :args, :block, :klass
|
|
|
|
def initialize(klass, args, block)
|
|
@klass = klass
|
|
@args = args
|
|
@block = block
|
|
end
|
|
|
|
def name; klass.name; end
|
|
|
|
def ==(middleware)
|
|
case middleware
|
|
when Middleware
|
|
klass == middleware.klass
|
|
when Class
|
|
klass == middleware
|
|
end
|
|
end
|
|
|
|
def inspect
|
|
if klass.is_a?(Class)
|
|
klass.to_s
|
|
else
|
|
klass.class.to_s
|
|
end
|
|
end
|
|
|
|
def build(app)
|
|
klass.new(app, *args, &block)
|
|
end
|
|
|
|
def build_instrumented(app)
|
|
InstrumentationProxy.new(build(app), inspect)
|
|
end
|
|
end
|
|
|
|
# This class is used to instrument the execution of a single middleware.
|
|
# It proxies the `call` method transparently and instruments the method
|
|
# call.
|
|
class InstrumentationProxy
|
|
EVENT_NAME = "process_middleware.action_dispatch"
|
|
|
|
def initialize(middleware, class_name)
|
|
@middleware = middleware
|
|
|
|
@payload = {
|
|
middleware: class_name,
|
|
}
|
|
end
|
|
|
|
def call(env)
|
|
ActiveSupport::Notifications.instrument(EVENT_NAME, @payload) do
|
|
@middleware.call(env)
|
|
end
|
|
end
|
|
end
|
|
|
|
include Enumerable
|
|
|
|
attr_accessor :middlewares
|
|
|
|
def initialize(*args)
|
|
@middlewares = []
|
|
yield(self) if block_given?
|
|
end
|
|
|
|
def each
|
|
@middlewares.each { |x| yield x }
|
|
end
|
|
|
|
def size
|
|
middlewares.size
|
|
end
|
|
|
|
def last
|
|
middlewares.last
|
|
end
|
|
|
|
def [](i)
|
|
middlewares[i]
|
|
end
|
|
|
|
def unshift(klass, *args, &block)
|
|
middlewares.unshift(build_middleware(klass, args, block))
|
|
end
|
|
ruby2_keywords(:unshift) if respond_to?(:ruby2_keywords, true)
|
|
|
|
def initialize_copy(other)
|
|
self.middlewares = other.middlewares.dup
|
|
end
|
|
|
|
def insert(index, klass, *args, &block)
|
|
index = assert_index(index, :before)
|
|
middlewares.insert(index, build_middleware(klass, args, block))
|
|
end
|
|
ruby2_keywords(:insert) if respond_to?(:ruby2_keywords, true)
|
|
|
|
alias_method :insert_before, :insert
|
|
|
|
def insert_after(index, *args, &block)
|
|
index = assert_index(index, :after)
|
|
insert(index + 1, *args, &block)
|
|
end
|
|
ruby2_keywords(:insert_after) if respond_to?(:ruby2_keywords, true)
|
|
|
|
def swap(target, *args, &block)
|
|
index = assert_index(target, :before)
|
|
insert(index, *args, &block)
|
|
middlewares.delete_at(index + 1)
|
|
end
|
|
ruby2_keywords(:swap) if respond_to?(:ruby2_keywords, true)
|
|
|
|
def delete(target)
|
|
middlewares.delete_if { |m| m.klass == target }
|
|
end
|
|
|
|
def move(target, source)
|
|
source_index = assert_index(source, :before)
|
|
source_middleware = middlewares.delete_at(source_index)
|
|
|
|
target_index = assert_index(target, :before)
|
|
middlewares.insert(target_index, source_middleware)
|
|
end
|
|
|
|
alias_method :move_before, :move
|
|
|
|
def move_after(target, source)
|
|
source_index = assert_index(source, :after)
|
|
source_middleware = middlewares.delete_at(source_index)
|
|
|
|
target_index = assert_index(target, :after)
|
|
middlewares.insert(target_index + 1, source_middleware)
|
|
end
|
|
|
|
def use(klass, *args, &block)
|
|
middlewares.push(build_middleware(klass, args, block))
|
|
end
|
|
ruby2_keywords(:use) if respond_to?(:ruby2_keywords, true)
|
|
|
|
def build(app = nil, &block)
|
|
instrumenting = ActiveSupport::Notifications.notifier.listening?(InstrumentationProxy::EVENT_NAME)
|
|
middlewares.freeze.reverse.inject(app || block) do |a, e|
|
|
if instrumenting
|
|
e.build_instrumented(a)
|
|
else
|
|
e.build(a)
|
|
end
|
|
end
|
|
end
|
|
|
|
private
|
|
def assert_index(index, where)
|
|
i = index.is_a?(Integer) ? index : middlewares.index { |m| m.klass == index }
|
|
raise "No such middleware to insert #{where}: #{index.inspect}" unless i
|
|
i
|
|
end
|
|
|
|
def build_middleware(klass, args, block)
|
|
Middleware.new(klass, args, block)
|
|
end
|
|
end
|
|
end
|