diff --git a/CHANGELOG.md b/CHANGELOG.md index cbee83225..b5b2b7996 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/UPGRADING.md b/UPGRADING.md index 8c1744959..3538d029c 100644 --- a/UPGRADING.md +++ b/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). diff --git a/lib/grape/middleware/auth/base.rb b/lib/grape/middleware/auth/base.rb index d7703cd78..46b230ec1 100644 --- a/lib/grape/middleware/auth/base.rb +++ b/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 diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index d7a50d598..1c46f28cd 100644 --- a/lib/grape/middleware/base.rb +++ b/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 diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 56a6d5443..77b66d6d2 100644 --- a/lib/grape/middleware/error.rb +++ b/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) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 359792a40..1c6907a1b 100644 --- a/lib/grape/middleware/formatter.rb +++ b/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 diff --git a/lib/grape/middleware/helpers.rb b/lib/grape/middleware/helpers.rb deleted file mode 100644 index 013a9587d..000000000 --- a/lib/grape/middleware/helpers.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -module Grape - module Middleware - # Common methods for all types of Grape middleware - module Helpers - def context - env[Grape::Env::API_ENDPOINT] - end - end - end -end diff --git a/lib/grape/middleware/versioner/base.rb b/lib/grape/middleware/versioner/base.rb index 4cd18a8c0..b12026906 100644 --- a/lib/grape/middleware/versioner/base.rb +++ b/lib/grape/middleware/versioner/base.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 diff --git a/spec/grape/middleware/base_spec.rb b/spec/grape/middleware/base_spec.rb index 7bfedcfe6..81d488ba1 100644 --- a/spec/grape/middleware/base_spec.rb +++ b/spec/grape/middleware/base_spec.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 diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index cc8a735d4..88eb88725 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_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 diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index ccd48b7b7..6d7712ea9 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_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 diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 807edaad4..cb353aa4e 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_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 diff --git a/spec/grape/middleware/versioner/accept_version_header_spec.rb b/spec/grape/middleware/versioner/accept_version_header_spec.rb index 7693cb92d..cf5fab10a 100644 --- a/spec/grape/middleware/versioner/accept_version_header_spec.rb +++ b/spec/grape/middleware/versioner/accept_version_header_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] } } diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index 230133cba..d673e08b9 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/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] } } diff --git a/spec/grape/middleware/versioner/param_spec.rb b/spec/grape/middleware/versioner/param_spec.rb index 4e6bb4aa7..00099dfc9 100644 --- a/spec/grape/middleware/versioner/param_spec.rb +++ b/spec/grape/middleware/versioner/param_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) { {} } diff --git a/spec/grape/middleware/versioner/path_spec.rb b/spec/grape/middleware/versioner/path_spec.rb index 4593eca03..79aff5376 100644 --- a/spec/grape/middleware/versioner/path_spec.rb +++ b/spec/grape/middleware/versioner/path_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) { {} } diff --git a/spec/integration/grape_entity/entity_spec.rb b/spec/integration/grape_entity/entity_spec.rb index eaea82382..cdac039a6 100644 --- a/spec/integration/grape_entity/entity_spec.rb +++ b/spec/integration/grape_entity/entity_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