Revisit Grape's middlewares default options (#2561)

* Grape::Middleware::Base's options is now double splatted
`default_options` have been transformed has const
Grape::Middleware::Helpers has been removed

* Fix entity_spec.rb

* Add CHANGELOG.md + Upgrading notes

* Do not change caller options

---------

Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]>

Author: ericproulx

CHANGELOG.md

@@ -3,16 +3,17 @@
 #### Features
 
 * [#2532](https://github.com/ruby-grape/grape/pull/2532): Update RuboCop 1.71.2 - [@ericproulx](https://github.com/ericproulx).
-* [#2535](https://github.com/ruby-grape/grape/pull/2535): Delegates calls to inner objects - [@ericproulx](https://github.com/ericproulx).
+* [#2535](https://github.com/ruby-grape/grape/pull/2535): Delegate calls to inner objects - [@ericproulx](https://github.com/ericproulx).
 * [#2537](https://github.com/ruby-grape/grape/pull/2537): Use activesupport `try` pattern - [@ericproulx](https://github.com/ericproulx).
 * [#2536](https://github.com/ruby-grape/grape/pull/2536): Update normalize_path like Rails - [@ericproulx](https://github.com/ericproulx).
-* [#2540](https://github.com/ruby-grape/grape/pull/2540): Introduce Params builder with symbolized short name - [@ericproulx](https://github.com/ericproulx).
+* [#2540](https://github.com/ruby-grape/grape/pull/2540): Introduce params builder with symbolized short name - [@ericproulx](https://github.com/ericproulx).
 * [#2550](https://github.com/ruby-grape/grape/pull/2550): Drop ActiveSupport 6.0 - [@ericproulx](https://github.com/ericproulx).
 * [#2549](https://github.com/ruby-grape/grape/pull/2549): Delegate cookies management to `Grape::Request` - [@ericproulx](https://github.com/ericproulx).
 * [#2554](https://github.com/ruby-grape/grape/pull/2554): Remove `Grape::Http::Headers` and `Grape::Util::Lazy::Object` - [@ericproulx](https://github.com/ericproulx).
 * [#2556](https://github.com/ruby-grape/grape/pull/2556): Remove unused `Grape::Request::DEFAULT_PARAMS_BUILDER` constant - [@eriklovmo](https://github.com/eriklovmo).
 * [#2558](https://github.com/ruby-grape/grape/pull/2558): Add Ruby's option `enable_frozen_string_literal` in CI - [@ericproulx](https://github.com/ericproulx).
-* [#2557](https://github.com/ruby-grape/grape/pull/2557): Add lint! - [@ericproulx](https://github.com/ericproulx).
+* [#2557](https://github.com/ruby-grape/grape/pull/2557): Add `lint!` - [@ericproulx](https://github.com/ericproulx).
+* [#2561](https://github.com/ruby-grape/grape/pull/2561): Optimize hash alloc for middleware's default options - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

UPGRADING.md

@@ -3,6 +3,11 @@ Upgrading Grape
 
 ### Upgrading to >= 2.4.0
 
+#### Grape::Middleware::Base
+
+- Second argument `options` is now a double splat (**) instead of single splat (*). If you're redefining `initialize` in your middleware and/or calling `super` in it, you might have to adapt the signature and the `super` call. Also, you might have to remove `{}` if you're pass `options` as a literal `Hash` or add `**` if you're using a variable.
+- `Grape::Middleware::Helpers` has been removed. The equivalent method `context` is now part of `Grape::Middleware::Base`.
+
 #### Grape::Http::Headers, Grape::Util::Lazy::Object
 
 Both have been removed. See [2554](https://github.com/ruby-grape/grape/pull/2554).

lib/grape/middleware/auth/base.rb

@@ -4,13 +4,11 @@ module Grape
   module Middleware
     module Auth
       class Base
-        include Helpers
-
         attr_accessor :options, :app, :env
 
-        def initialize(app, *options)
+        def initialize(app, **options)
           @app = app
-          @options = options.shift
+          @options = options
         end
 
         def call(env)
@@ -19,24 +17,14 @@ def call(env)
 
         def _call(env)
           self.env = env
+          return app.call(env) unless options.key?(:type)
 
-          if options.key?(:type)
-            auth_proc = options[:proc]
-            auth_proc_context = context
-
-            strategy_info = Grape::Middleware::Auth::Strategies[options[:type]]
-
-            throw(:error, status: 401, message: 'API Authorization Failed.') if strategy_info.blank?
-
-            strategy = strategy_info.create(@app, options) do |*args|
-              auth_proc_context.instance_exec(*args, &auth_proc)
-            end
-
-            strategy.call(env)
+          strategy_info = Grape::Middleware::Auth::Strategies[options[:type]]
+          throw :error, status: 401, message: 'API Authorization Failed.' if strategy_info.blank?
 
-          else
-            app.call(env)
-          end
+          strategy_info.create(@app, options) do |*args|
+            env[Grape::Env::API_ENDPOINT].instance_exec(*args, &options[:proc])
+          end.call(env)
         end
       end
     end

lib/grape/middleware/base.rb

@@ -3,23 +3,18 @@
 module Grape
   module Middleware
     class Base
-      include Helpers
       include Grape::DSL::Headers
 
       attr_reader :app, :env, :options
 
       # @param [Rack Application] app The standard argument for a Rack middleware.
       # @param [Hash] options A hash of options, simply stored for use by subclasses.
-      def initialize(app, *options)
+      def initialize(app, **options)
         @app = app
-        @options = options.any? ? default_options.deep_merge(options.shift) : default_options
+        @options = merge_default_options(options)
         @app_response = nil
       end
 
-      def default_options
-        {}
-      end
-
       def call(env)
         dup.call!(env).to_a
       end
@@ -56,10 +51,14 @@ def rack_request
         @rack_request ||= Rack::Request.new(env)
       end
 
+      def context
+        env[Grape::Env::API_ENDPOINT]
+      end
+
       def response
         return @app_response if @app_response.is_a?(Rack::Response)
 
-        @app_response = Rack::Response.new(@app_response[2], @app_response[0], @app_response[1])
+        @app_response = Rack::Response[*@app_response]
       end
 
       def content_types
@@ -100,6 +99,16 @@ def merge_headers(response)
       def content_types_indifferent_access
         @content_types_indifferent_access ||= content_types.with_indifferent_access
       end
+
+      def merge_default_options(options)
+        if respond_to?(:default_options)
+          default_options.deep_merge(options)
+        elsif self.class.const_defined?(:DEFAULT_OPTIONS)
+          self.class::DEFAULT_OPTIONS.deep_merge(options)
+        else
+          options
+        end
+      end
     end
   end
 end

lib/grape/middleware/error.rb

@@ -3,30 +3,22 @@
 module Grape
   module Middleware
     class Error < Base
-      def default_options
-        {
-          default_status: 500, # default status returned on error
-          default_message: '',
-          format: :txt,
-          helpers: nil,
-          formatters: {},
-          error_formatters: {},
-          rescue_all: false, # true to rescue all exceptions
-          rescue_grape_exceptions: false,
-          rescue_subclasses: true, # rescue subclasses of exceptions listed
-          rescue_options: {
-            backtrace: false, # true to display backtrace, true to let Grape handle Grape::Exceptions
-            original_exception: false # true to display exception
-          },
-          rescue_handlers: {}, # rescue handler blocks
-          base_only_rescue_handlers: {}, # rescue handler blocks rescuing only the base class
-          all_rescue_handler: nil # rescue handler block to rescue from all exceptions
-        }
-      end
-
-      def initialize(app, *options)
+      DEFAULT_OPTIONS = {
+        default_status: 500,
+        default_message: '',
+        format: :txt,
+        rescue_all: false,
+        rescue_grape_exceptions: false,
+        rescue_subclasses: true,
+        rescue_options: {
+          backtrace: false,
+          original_exception: false
+        }.freeze
+      }.freeze
+
+      def initialize(app, **options)
         super
-        self.class.include(@options[:helpers]) if @options[:helpers]
+        self.class.include(options[:helpers]) if options[:helpers]
       end
 
       def call!(env)

lib/grape/middleware/formatter.rb

@@ -3,13 +3,9 @@
 module Grape
   module Middleware
     class Formatter < Base
-      def default_options
-        {
-          default_format: :txt,
-          formatters: {},
-          parsers: {}
-        }
-      end
+      DEFAULT_OPTIONS = {
+        default_format: :txt
+      }.freeze
 
       def before
         negotiate_content_type

lib/grape/middleware/helpers.rb

@@ -4,28 +4,20 @@ module Grape
   module Middleware
     module Versioner
       class Base < Grape::Middleware::Base
-        DEFAULT_PATTERN = /.*/i.freeze
-        DEFAULT_PARAMETER = 'apiver'
+        DEFAULT_OPTIONS = {
+          pattern: /.*/i.freeze,
+          version_options: {
+            strict: false,
+            cascade: true,
+            parameter: 'apiver'
+          }.freeze
+        }.freeze
 
         def self.inherited(klass)
           super
           Versioner.register(klass)
         end
 
-        def default_options
-          {
-            versions: nil,
-            prefix: nil,
-            mount_path: nil,
-            pattern: DEFAULT_PATTERN,
-            version_options: {
-              strict: false,
-              cascade: true,
-              parameter: DEFAULT_PARAMETER
-            }
-          }
-        end
-
         def versions
           options[:versions]
         end

lib/grape/middleware/versioner/base.rb

@@ -141,9 +141,7 @@
     context 'defaults' do
       let(:example_ware) do
         Class.new(Grape::Middleware::Base) do
-          def default_options
-            { monkey: true }
-          end
+          const_set(:DEFAULT_OPTIONS, { monkey: true }.freeze)
         end
       end
 

spec/grape/middleware/base_spec.rb

@@ -29,7 +29,7 @@ def call(_env)
     context = self
     Rack::Builder.app do
       use Spec::Support::EndpointFaker
-      use Grape::Middleware::Error, opts # rubocop:disable RSpec/DescribedClass
+      use Grape::Middleware::Error, **opts # rubocop:disable RSpec/DescribedClass
       run context.err_app
     end
   end

spec/grape/middleware/error_spec.rb

@@ -66,7 +66,7 @@ def call(_env)
       use Rack::Lint
       use Spec::Support::EndpointFaker
       if opts.any?
-        use Grape::Middleware::Error, opts
+        use Grape::Middleware::Error, **opts
       else
         use Grape::Middleware::Error
       end

spec/grape/middleware/exception_spec.rb

@@ -236,7 +236,7 @@ def to_xml
     end
 
     it 'uses custom json formatter' do
-      subject.options[:formatters][:json] = ->(_obj, _env) { 'CUSTOM JSON FORMAT' }
+      subject.options[:formatters] = { json: ->(_obj, _env) { 'CUSTOM JSON FORMAT' } }
       r = Rack::MockResponse[*subject.call(Rack::PATH_INFO => '/info.json')]
       expect(r.body).to eq('CUSTOM JSON FORMAT')
     end

spec/grape/middleware/formatter_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe Grape::Middleware::Versioner::AcceptVersionHeader do
-  subject { described_class.new(app, @options) }
+  subject { described_class.new(app, **@options) }
 
   let(:app) { ->(env) { [200, env, env] } }
 

spec/grape/middleware/versioner/accept_version_header_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe Grape::Middleware::Versioner::Header do
-  subject { described_class.new(app, @options) }
+  subject { described_class.new(app, **@options) }
 
   let(:app) { ->(env) { [200, env, env] } }
 

spec/grape/middleware/versioner/header_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe Grape::Middleware::Versioner::Param do
-  subject { described_class.new(app, options) }
+  subject { described_class.new(app, **options) }
 
   let(:app) { ->(env) { [200, env, env[Grape::Env::API_VERSION]] } }
   let(:options) { {} }

spec/grape/middleware/versioner/param_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe Grape::Middleware::Versioner::Path do
-  subject { described_class.new(app, options) }
+  subject { described_class.new(app, **options) }
 
   let(:app) { ->(env) { [200, env, env[Grape::Env::API_VERSION]] } }
   let(:options) { {} }

spec/grape/middleware/versioner/path_spec.rb

@@ -348,7 +348,7 @@ def call(_env)
       opts = options
       Rack::Builder.app do
         use Spec::Support::EndpointFaker
-        use Grape::Middleware::Error, opts
+        use Grape::Middleware::Error, **opts
         run ErrApp
       end
     end