• G
    Delayed middleware delete does not allow move operations · 40fc1651
    Genadi Samokovarov 提交于
    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! 🎄
    40fc1651
configuring.md 92.7 KB