Fix middleware with keywords (#2546)

ericproulx · web-flow · commit 97f2413b49e7 · 2025-03-09T08:02:50.000-04:00
* Refactor Stack and Middleware closer to ActionDispatch::MiddlewareStack
Flag hash arg with `ruby2_keywords_hash`
Add spec

* Add CHANGELOG.md

* Change 1 liner
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 
 * [#2538](https://github.com/ruby-grape/grape/pull/2538): Fix validating nested json array params - [@mohammednasser-32](https://github.com/mohammednasser-32).
 * [#2543](https://github.com/ruby-grape/grape/pull/2543): Fix array allocation on mount - [@ericproulx](https://github.com/ericproulx).
+* [#2546](https://github.com/ruby-grape/grape/pull/2546): Fix middleware with keywords - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 2.3.0 (2025-02-08)
diff --git a/lib/grape/middleware/stack.rb b/lib/grape/middleware/stack.rb
@@ -7,18 +7,18 @@ module Middleware
     class Stack
       extend Forwardable
       class Middleware
-        extend Forwardable
-
         attr_reader :args, :block, :klass
 
-        def_delegators :klass, :name
-
-        def initialize(klass, *args, &block)
+        def initialize(klass, args, block)
           @klass = klass
           @args = args
           @block = block
         end
 
+        def name
+          klass.name
+        end
+
         def ==(other)
           case other
           when Middleware
@@ -32,7 +32,11 @@ def inspect
           klass.to_s
         end
 
-        def use_in(builder)
+        def build(builder)
+          # we need to force the ruby2_keywords_hash for middlewares that initialize contains keywords
+          # like ActionDispatch::RequestId since middleware arguments are serialized
+          # https://rubyapi.org/3.4/o/hash#method-c-ruby2_keywords_hash
+          args[-1] = Hash.ruby2_keywords_hash(args[-1]) if args.last.is_a?(Hash) && Hash.respond_to?(:ruby2_keywords_hash)
           builder.use(klass, *args, &block)
         end
       end
@@ -48,51 +52,50 @@ def initialize
         @others = []
       end
 
-      def insert(index, *args, &block)
+      def insert(index, klass, *args, &block)
         index = assert_index(index, :before)
-        middleware = self.class::Middleware.new(*args, &block)
-        middlewares.insert(index, middleware)
+        middlewares.insert(index, self.class::Middleware.new(klass, args, block))
       end
-      ruby2_keywords :insert if respond_to?(:ruby2_keywords, true)
 
       alias 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 use(...)
-        middleware = self.class::Middleware.new(...)
+      def use(klass, *args, &block)
+        middleware = self.class::Middleware.new(klass, args, block)
         middlewares.push(middleware)
       end
 
       def merge_with(middleware_specs)
-        middleware_specs.each do |operation, *args|
+        middleware_specs.each do |operation, klass, *args|
           if args.last.is_a?(Proc)
             last_proc = args.pop
-            public_send(operation, *args, &last_proc)
+            public_send(operation, klass, *args, &last_proc)
           else
-            public_send(operation, *args)
+            public_send(operation, klass, *args)
           end
         end
       end
 
       # @return [Rack::Builder] the builder object with our middlewares applied
-      def build(builder = Rack::Builder.new)
-        others.shift(others.size).each { |m| merge_with(m) }
-        middlewares.each do |m|
-          m.use_in(builder)
+      def build
+        Rack::Builder.new.tap do |builder|
+          others.shift(others.size).each { |m| merge_with(m) }
+          middlewares.each do |m|
+            m.build(builder)
+          end
         end
-        builder
       end
 
       # @description Add middlewares with :use operation to the stack. Store others with :insert_* operation for later
       # @param [Array] other_specs An array of middleware specifications (e.g. [[:use, klass], [:insert_before, *args]])
       def concat(other_specs)
-        @others << Array(other_specs).reject { |o| o.first == :use }
-        merge_with(Array(other_specs).select { |o| o.first == :use })
+        use, not_use = other_specs.partition { |o| o.first == :use }
+        others << not_use
+        merge_with(use)
       end
 
       protected
diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
@@ -1525,6 +1525,34 @@ def before
         expect(last_response).to be_bad_request
         expect(last_response.body).to eq('Caught in the Net')
       end
+
+      context 'when middleware initialize as keywords' do
+        let(:middleware_with_keywords) do
+          Class.new do
+            def initialize(app, keyword:)
+              @app = app
+              @keyword = keyword
+            end
+
+            def call(env)
+              env['middleware_with_keywords'] = @keyword
+              @app.call(env)
+            end
+          end
+        end
+
+        before do
+          subject.use middleware_with_keywords, keyword: 'hello'
+          subject.get '/' do
+            env['middleware_with_keywords']
+          end
+          get '/'
+        end
+
+        it 'returns the middleware value' do
+          expect(last_response.body).to eq('hello')
+        end
+      end
     end
 
     describe '.insert_before' do
