diff --git a/CHANGELOG.md b/CHANGELOG.md index 719265202..d11d198f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ * [#2733](https://github.com/ruby-grape/grape/pull/2733): Drop the dead `active_support/core_ext/hash/reverse_merge` require; call `ActiveSupport::HashWithIndifferentAccess.new(...)` directly at call sites - [@ericproulx](https://github.com/ericproulx). * [#2734](https://github.com/ruby-grape/grape/pull/2734): Extract `options_route_enabled` from the Endpoint options Hash into a dedicated `attr_accessor` - [@ericproulx](https://github.com/ericproulx). * [#2736](https://github.com/ruby-grape/grape/pull/2736): Collapse `Endpoint#run_validators` rescue branches via `ValidationErrors` flatten; `ValidationErrors#initialize` keyword renamed `errors:` → `exceptions:` - [@ericproulx](https://github.com/ericproulx). +* [#2718](https://github.com/ruby-grape/grape/pull/2718): Generalize middleware options to per-class `Options` `Data` value objects (`Middleware::Error`, `::Formatter`, `::Versioner::Base`); expose them via a new `config` reader, keep `options` Hash for back-compat, deprecate `Options#[]` Hash-style access - [@ericproulx](https://github.com/ericproulx). * [#2746](https://github.com/ruby-grape/grape/pull/2746): Hoist `using:` / `except:` from `**opts` to explicit kwargs on `DSL::Parameters#requires` and `#optional` - [@ericproulx](https://github.com/ericproulx). * [#2742](https://github.com/ruby-grape/grape/pull/2742): Prune unused requires in `lib/grape.rb`; narrow `active_support/inflector` to `core_ext/string/inflections` - [@ericproulx](https://github.com/ericproulx). * [#2741](https://github.com/ruby-grape/grape/pull/2741): Readability pass: guard clauses and small extractions across `lib/` - [@ericproulx](https://github.com/ericproulx). diff --git a/UPGRADING.md b/UPGRADING.md index db9538930..86e9ce3c7 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -55,6 +55,45 @@ http_basic(realm: 'API') auth :http_digest, realm: 'API', opaque: 'secret' ``` +#### Middleware options now route through per-class `Options` `Data` value objects + +`Grape::Middleware::Error`, `Grape::Middleware::Formatter`, and `Grape::Middleware::Versioner::Base` each declare an `Options` `Data.define` and route their `**options` kwargs through it on `initialize`. This means **unknown kwargs now raise `ArgumentError`** instead of being silently swallowed: + +```ruby +# previously: silently swallowed (Formatter doesn't actually read :rescue_options) +Grape::Middleware::Formatter.new(app, rescue_options: { backtrace: true }) + +# now: ArgumentError (unknown keyword: :rescue_options) +``` + +Each `Options` class accepts exactly the kwargs the middleware actually reads. The supported sets: + +- `Middleware::Error::Options`: `all_rescue_handler`, `base_only_rescue_handlers`, `content_types`, `default_error_formatter`, `default_message`, `default_status`, `error_formatters`, `format`, `grape_exceptions_rescue_handler`, `internal_grape_exceptions_rescue_handler`, `rescue_all`, `rescue_grape_exceptions`, `rescue_handlers`, `rescue_options`. +- `Middleware::Formatter::Options`: `content_types`, `default_format`, `format`, `formatters`, `parsers`. +- `Middleware::Versioner::Base::Options`: `content_types`, `format`, `mount_path`, `pattern`, `prefix`, `version_options`, `versions`. + +The `Hash`-based `options` reader on `Grape::Middleware::Base` continues to return a frozen Hash representation of the Data (`config.to_h.freeze`) for back-compat with subclasses that read `options[:key]`. A new `config` reader exposes the typed Data instance — prefer the named accessors going forward: + +```ruby +# back-compat (still works) +options[:format] + +# preferred +config.format +# or, on converted middlewares, just `format` (provided via def_delegators) +``` + +`Options#[]` is defined as a Hash-style shim with a deprecation warning so legacy `data[:key]` callers get a migration nudge: + +```ruby +# emits Grape.deprecator warning +Grape::Middleware::Error::Options.new[:format] +``` + +#### `DEFAULT_OPTIONS` constants on converted middlewares are deprecated + +`Grape::Middleware::Error::DEFAULT_OPTIONS`, `Grape::Middleware::Formatter::DEFAULT_OPTIONS`, and `Grape::Middleware::Versioner::Base::DEFAULT_OPTIONS` still exist as a frozen `Hash` representation of the `Options` defaults (`Options.new.to_h.freeze`), for back-compat with any code that referenced these constants directly. They will be removed in a future release; introspect the `Options` `Data` class itself instead. + #### `Grape::Middleware::Globals` removed `Grape::Middleware::Globals` and the three env constants it set (`Grape::Env::GRAPE_REQUEST`, `Grape::Env::GRAPE_REQUEST_HEADERS`, `Grape::Env::GRAPE_REQUEST_PARAMS`) have been deleted. The middleware was introduced in 2013 (commit `9987090b`) but never mounted by Grape's own stack — the `Grape::Request` it built is now constructed directly inside `Grape::Endpoint`. Nothing in `lib/` read those env keys. diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index 7446c4523..f47ec752e 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -5,13 +5,25 @@ module Middleware class Base include Grape::DSL::Headers - attr_reader :app, :env, :options + attr_reader :app, :env, :options, :config # @param [Rack Application] app The standard argument for a Rack middleware. - # @param [Hash] options A hash of options, simply stored for use by subclasses. + # @param [Hash] options Options forwarded to the subclass. When the + # subclass declares an `Options` Data class, the kwargs are routed + # through it and exposed via {#config}; {#options} keeps returning a + # frozen Hash representation for back-compat with subclasses that read + # `options[:key]`. Otherwise the kwargs are deep-merged with the + # subclass's `DEFAULT_OPTIONS` Hash (legacy path) and frozen. def initialize(app, **options) @app = app - @options = merge_default_options(options).freeze + if self.class.const_defined?(:Options) + # Search ancestors so subclasses (e.g. Versioner::Path → Versioner::Base) + # inherit their parent's Options Data class without redeclaring it. + @config = self.class::Options.new(**options) + @options = @config.to_h.freeze + else + @options = merge_default_options(options).freeze + end @app_response = nil end diff --git a/lib/grape/middleware/deprecated_options_hash_access.rb b/lib/grape/middleware/deprecated_options_hash_access.rb new file mode 100644 index 000000000..413b8faab --- /dev/null +++ b/lib/grape/middleware/deprecated_options_hash_access.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Grape + module Middleware + # Mixin for per-middleware +Options+ +Data+ classes that need to keep + # accepting legacy +data[:key]+ Hash-style access while nudging callers + # toward the named accessor. Emits a +Grape.deprecator+ warning then + # forwards to +public_send(key)+. + module DeprecatedOptionsHashAccess + def [](key) + Grape.deprecator.warn( + "`#{self.class.name}#[]` is deprecated. " \ + "Use the named accessor `#{key}` instead." + ) + public_send(key) if members.include?(key) + end + end + end +end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 0c8279c62..928d689ae 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -6,26 +6,41 @@ class Error < Base extend Forwardable include PrecomputedContentTypes - DEFAULT_OPTIONS = { - all_rescue_handler: nil, - base_only_rescue_handlers: nil, - default_error_formatter: nil, - default_message: '', - default_status: 500, - error_formatters: nil, - format: :txt, - grape_exceptions_rescue_handler: nil, - internal_grape_exceptions_rescue_handler: nil, - rescue_all: false, - rescue_grape_exceptions: false, - rescue_handlers: nil, - rescue_options: Grape::DSL::RescueOptions.new - }.freeze - - attr_reader :all_rescue_handler, :base_only_rescue_handlers, :default_error_formatter, - :default_message, :default_status, :error_formatters, :format, - :grape_exceptions_rescue_handler, :internal_grape_exceptions_rescue_handler, - :rescue_all, :rescue_grape_exceptions, :rescue_handlers, :rescue_options + Options = Data.define( + :all_rescue_handler, :base_only_rescue_handlers, :content_types, + :default_error_formatter, :default_message, :default_status, + :error_formatters, :format, + :grape_exceptions_rescue_handler, :internal_grape_exceptions_rescue_handler, + :rescue_all, :rescue_grape_exceptions, :rescue_handlers, :rescue_options + ) do + include Grape::Middleware::DeprecatedOptionsHashAccess + + def initialize( + all_rescue_handler: nil, base_only_rescue_handlers: nil, content_types: nil, + default_error_formatter: nil, default_message: '', default_status: 500, + error_formatters: nil, format: :txt, + grape_exceptions_rescue_handler: nil, internal_grape_exceptions_rescue_handler: nil, + rescue_all: false, rescue_grape_exceptions: false, rescue_handlers: nil, + rescue_options: nil + ) + # `rescue_options:` arrives nil from `Endpoint#error_middleware_options` + # when no `rescue_from` has been called — fall back to the documented + # defaults rather than letting nil propagate to `def_delegator + # :rescue_options, :backtrace`. + rescue_options ||= Grape::DSL::RescueOptions.new + super + end + end + + # @deprecated Kept as a frozen Hash representation of the {Options} + # defaults for back-compat. Will be removed in a future release. + DEFAULT_OPTIONS = Options.new.to_h.freeze + + def_delegators :config, + :all_rescue_handler, :base_only_rescue_handlers, :default_error_formatter, + :default_message, :default_status, :error_formatters, :format, + :grape_exceptions_rescue_handler, :internal_grape_exceptions_rescue_handler, + :rescue_all, :rescue_grape_exceptions, :rescue_handlers, :rescue_options # +:backtrace+ / +:original_exception+ on the rescue options become # +#include_backtrace+ / +#include_original_exception+ on the middleware, @@ -33,23 +48,6 @@ class Error < Base def_delegator :rescue_options, :backtrace, :include_backtrace def_delegator :rescue_options, :original_exception, :include_original_exception - def initialize(app, **options) - super - @all_rescue_handler = @options[:all_rescue_handler] - @base_only_rescue_handlers = @options[:base_only_rescue_handlers] - @default_error_formatter = @options[:default_error_formatter] - @default_message = @options[:default_message] - @default_status = @options[:default_status] - @error_formatters = @options[:error_formatters] - @format = @options[:format] - @grape_exceptions_rescue_handler = @options[:grape_exceptions_rescue_handler] - @internal_grape_exceptions_rescue_handler = @options[:internal_grape_exceptions_rescue_handler] - @rescue_all = @options[:rescue_all] - @rescue_grape_exceptions = @options[:rescue_grape_exceptions] - @rescue_handlers = @options[:rescue_handlers] - @rescue_options = @options[:rescue_options] || Grape::DSL::RescueOptions.new - end - def call!(env) @env = env error_response(catch(:error) { return @app.call(@env) }) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 51c660bff..2bbeea2c6 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -3,27 +3,24 @@ module Grape module Middleware class Formatter < Base + extend Forwardable include PrecomputedContentTypes - DEFAULT_OPTIONS = { - content_types: nil, - default_format: :txt, - format: nil, - formatters: nil, - parsers: nil - }.freeze + Options = Data.define(:content_types, :default_format, :format, :formatters, :parsers) do + include Grape::Middleware::DeprecatedOptionsHashAccess - ALL_MEDIA_TYPES = '*/*' + def initialize(content_types: nil, default_format: :txt, format: nil, formatters: nil, parsers: nil) + super + end + end - attr_reader :default_format, :format, :formatters, :parsers + # @deprecated Kept as a frozen Hash representation of the {Options} + # defaults for back-compat. Will be removed in a future release. + DEFAULT_OPTIONS = Options.new.to_h.freeze - def initialize(app, **options) - super - @default_format = @options[:default_format] - @format = @options[:format] - @formatters = @options[:formatters] - @parsers = @options[:parsers] - end + ALL_MEDIA_TYPES = '*/*' + + def_delegators :config, :default_format, :format, :formatters, :parsers def before negotiate_content_type @@ -99,7 +96,7 @@ def read_rack_input(body) fmt = media_type ? mime_types[media_type] : default_format throw :error, Grape::Exceptions::ErrorResponse.new(status: 415, message: "The provided content-type '#{media_type}' is not supported.") unless content_type_for(fmt) - parser = Grape::Parser.parser_for fmt, options[:parsers] + parser = Grape::Parser.parser_for fmt, parsers return env[Grape::Env::API_REQUEST_BODY] = body unless parser begin diff --git a/lib/grape/middleware/precomputed_content_types.rb b/lib/grape/middleware/precomputed_content_types.rb index 3677c8310..6c69e27d0 100644 --- a/lib/grape/middleware/precomputed_content_types.rb +++ b/lib/grape/middleware/precomputed_content_types.rb @@ -4,9 +4,10 @@ module Grape module Middleware # Include in a middleware subclass that needs content-type negotiation. # Provides +content_types+ / +mime_types+ / +content_type_for+ / - # +content_type+ resolved from +options[:content_types]+ and - # +options[:format]+, and warms those caches on the parent instance at - # initialization so per-request +dup+s inherit them (avoiding + # +content_type+ resolved from +config.content_types+ and + # +config.format+ — so the consuming middleware's +Options+ Data class + # must declare both fields. Warms those caches on the parent instance + # at initialization so per-request +dup+s inherit them (avoiding # ~1 µs/request of +with_indifferent_access+ recomputation). # # Opt-in: plain +Grape::Middleware::Base+ subclasses that don't need @@ -20,7 +21,7 @@ def initialize(app, **options) end def content_types - @content_types ||= Grape::ContentTypes.content_types_for(options[:content_types]) + @content_types ||= Grape::ContentTypes.content_types_for(config.content_types) end def mime_types @@ -32,7 +33,7 @@ def content_type_for(format) end def content_type - content_type_for(env[Grape::Env::API_FORMAT] || options[:format]) || 'text/html' + content_type_for(env[Grape::Env::API_FORMAT] || config.format) || 'text/html' end private diff --git a/lib/grape/middleware/versioner/base.rb b/lib/grape/middleware/versioner/base.rb index 8b807e681..24eb321f5 100644 --- a/lib/grape/middleware/versioner/base.rb +++ b/lib/grape/middleware/versioner/base.rb @@ -7,12 +7,22 @@ class Base < Grape::Middleware::Base extend Forwardable include Grape::Middleware::PrecomputedContentTypes - DEFAULT_OPTIONS = { - mount_path: nil, - pattern: /.*/i, - prefix: nil, - version_options: Grape::DSL::VersionOptions.new - }.freeze + Options = Data.define( + :content_types, :format, :mount_path, :pattern, :prefix, :version_options, :versions + ) do + include Grape::Middleware::DeprecatedOptionsHashAccess + + def initialize( + content_types: nil, format: nil, mount_path: nil, pattern: /.*/i, prefix: nil, + version_options: Grape::DSL::VersionOptions.new, versions: nil + ) + super + end + end + + # @deprecated Kept as a frozen Hash representation of the {Options} + # defaults for back-compat. Will be removed in a future release. + DEFAULT_OPTIONS = Options.new.to_h.freeze CASCADE_PASS_HEADER = { 'X-Cascade' => 'pass' }.freeze @@ -21,18 +31,14 @@ def self.inherited(klass) Versioner.register(klass) end - attr_reader :available_media_types, :error_headers, :mount_path, :pattern, - :prefix, :version_options, :versions + attr_reader :available_media_types, :error_headers, :versions + def_delegators :config, :mount_path, :pattern, :prefix, :version_options def_delegators :version_options, :cascade, :parameter, :strict, :vendor def initialize(app, **options) super - @version_options = @options[:version_options] - @mount_path = @options[:mount_path] - @pattern = @options[:pattern] - @prefix = @options[:prefix] - @versions = @options[:versions]&.map(&:to_s) # making sure versions are strings to ease potential match + @versions = config.versions&.map(&:to_s) # making sure versions are strings to ease potential match @error_headers = cascade ? CASCADE_PASS_HEADER : {} @available_media_types = build_available_media_types end diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 9cf382a50..1e8df01fd 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -467,7 +467,6 @@ def self.call(_, _) it 'adds the backtrace and original_exception to the error output' do subject = described_class.new( app, - rescue_options: { backtrace: true, original_exception: true }, parsers: { json: ->(_object, _env) { raise StandardError, 'fail' } } ) io = StringIO.new('{invalid}')