From ad0eb3e2e2f4e557688b8ceac0a237768dafd760 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 14 May 2026 19:36:52 +0200 Subject: [PATCH 1/2] [DRAFT] Generalize middleware options to per-class Data value objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Demonstration / discussion PR. Right now every middleware that wants typed accessors over its options has to hand-write the same boilerplate: DEFAULT_OPTIONS = { foo: nil, bar: nil, ... }.freeze attr_reader :foo, :bar, ... def initialize(app, **options) super @foo = @options[:foo] @bar = @options[:bar] ... end Since `@options` was already frozen by design (Middleware::Base#initialize post-PR #2696), the natural next step is to replace the Hash with a per-subclass `Options = Data.define(...)` and let `Forwardable` cover the accessor wiring. Mechanism added in this draft: - `Grape::Middleware::OptionsCompat` — a small mixin Options classes include to keep the legacy `options[:key]` idiom working (notably for `Middleware::Base#content_types` and `#content_type`). Unknown keys return `nil` to match Hash semantics. - `Middleware::Base#initialize` detects `self.class::Options` and routes kwargs through `Options.new(**options)`. Subclasses that still rely on `DEFAULT_OPTIONS` Hash + deep_merge keep working unchanged. Demonstrated on `Middleware::Formatter`: - Replaces 5-line DEFAULT_OPTIONS Hash + 4-line `attr_reader` list + 6-line initialize body with: Options = Data.define(:content_types, :default_format, :format, :formatters, :parsers) do include Grape::Middleware::OptionsCompat def initialize(content_types: nil, default_format: :txt, format: nil, formatters: nil, parsers: nil) super end end def_delegators :options, :default_format, :format, :formatters, :parsers - Defaults move from the freeze'd Hash to `#initialize` signature. - Immutability is implicit (Data instances). Behaviour change: passing an unknown kwarg to a middleware whose `Options` class doesn't declare it now raises `ArgumentError` instead of being silently swallowed by `**options`. One formatter spec was passing `rescue_options:` (dead weight; Formatter doesn't read it) — dropped. If this direction is acceptable, follow-ups would convert `Middleware::Error`, `Versioner::Base`, etc., each shedding the same boilerplate. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/grape/middleware/base.rb | 14 ++++++++++++-- lib/grape/middleware/formatter.rb | 25 +++++++++---------------- lib/grape/middleware/options_compat.rb | 20 ++++++++++++++++++++ spec/grape/middleware/formatter_spec.rb | 1 - 4 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 lib/grape/middleware/options_compat.rb diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index 05316a563..3da8c9569 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -8,10 +8,14 @@ class Base 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. + # @param [Hash] options Options forwarded to the subclass. When the + # subclass declares an `Options` Data class (recommended; see + # `Grape::Middleware::OptionsCompat`), the kwargs are routed through + # it. Otherwise they 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 + @options = build_options(options) @app_response = nil end @@ -78,6 +82,12 @@ def merge_headers(response) end end + def build_options(options) + return self.class::Options.new(**options) if self.class.const_defined?(:Options, false) + + merge_default_options(options).freeze + end + def merge_default_options(options) if respond_to?(:default_options) default_options.deep_merge(options) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 66dcdf9a0..bd99f1f59 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -3,27 +3,20 @@ 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::OptionsCompat - 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 + ALL_MEDIA_TYPES = '*/*' - def initialize(app, **options) - super - @default_format = @options[:default_format] - @format = @options[:format] - @formatters = @options[:formatters] - @parsers = @options[:parsers] - end + def_delegators :options, :default_format, :format, :formatters, :parsers def before negotiate_content_type diff --git a/lib/grape/middleware/options_compat.rb b/lib/grape/middleware/options_compat.rb new file mode 100644 index 000000000..e722eb1ab --- /dev/null +++ b/lib/grape/middleware/options_compat.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Grape + module Middleware + # Mixin for per-middleware `Options` Data classes. + # + # Provides a Hash-like `[](key)` reader so the legacy `options[:key]` + # idiom in `Grape::Middleware::Base` (notably `#content_types` and + # `#content_type`) keeps working when a subclass swaps its Hash + # `DEFAULT_OPTIONS` for a `Data.define(...)`-backed `Options` class. + # Unknown keys return +nil+ to match the old behaviour. + module OptionsCompat + def [](key) + return nil unless respond_to?(key) + + public_send(key) + end + end + end +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}') From 13add91d583294384743b267e8435d43c1df844a Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 14 May 2026 20:47:57 +0200 Subject: [PATCH 2/2] Convert Error, Versioner::Base to Options Data; drop OptionsCompat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building on the merged refactor/version-guard-clause (VersionOptions Data) and feature/error-formatter-kwargs-2527 (RescueOptions Data), convert the remaining two middlewares that include `PrecomputedContentTypes` to per-class `Options = Data.define(...)`: - Middleware::Error: 14-field Options replacing the 14-line DEFAULT_OPTIONS Hash + the 12-line attr_reader / ivar-set initialize body. `rescue_options:` defaults to `Grape::DSL::RescueOptions.new`; initialize coerces an explicit nil (from `Endpoint#error_middleware_options` when no `rescue_from` was called) to the default. `Forwardable.def_delegators :options, ...` covers every accessor; the existing `def_delegator :rescue_options, :backtrace, :include_backtrace` (and `:original_exception`) carry through unchanged. - Middleware::Versioner::Base: 7-field Options (adds `content_types:` / `format:` so the mixin's accessor reads land cleanly). `version_options:` defaults to `Grape::DSL::VersionOptions.new`. The four `def_delegators :version_options, :cascade, :parameter, :strict, :vendor` stay; `mount_path` / `pattern` / `prefix` / `version_options` are now delegated via `def_delegators :options, ...`. With every PrecomputedContentTypes consumer now using an Options Data class, switch the mixin to accessor reads: options.content_types # was options[:content_types] options.format # was options[:format] …and delete `Grape::Middleware::OptionsCompat` entirely. Two supporting tweaks: - `Middleware::Base#build_options` switches `const_defined?(:Options, false)` → `const_defined?(:Options)` so Versioner subclasses (`Path`, `Header`, `Param`, `AcceptVersionHeader`) inherit `Versioner::Base::Options` without redeclaring it. - `Middleware::Formatter#read_rack_input` switches `options[:parsers]` to the existing `parsers` delegator. `Filter` and `Auth::Base` / `Auth::*` remain on the legacy `DEFAULT_OPTIONS` Hash path — `Base#build_options` keeps the fallback, so they continue to work unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/grape/middleware/base.rb | 9 +-- lib/grape/middleware/error.rb | 66 ++++++++----------- lib/grape/middleware/formatter.rb | 4 +- lib/grape/middleware/options_compat.rb | 20 ------ .../middleware/precomputed_content_types.rb | 11 ++-- lib/grape/middleware/versioner/base.rb | 26 ++++---- 6 files changed, 54 insertions(+), 82 deletions(-) delete mode 100644 lib/grape/middleware/options_compat.rb diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index 3da8c9569..56f6ebbef 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -9,9 +9,8 @@ class Base # @param [Rack Application] app The standard argument for a Rack middleware. # @param [Hash] options Options forwarded to the subclass. When the - # subclass declares an `Options` Data class (recommended; see - # `Grape::Middleware::OptionsCompat`), the kwargs are routed through - # it. Otherwise they are deep-merged with the subclass's + # subclass declares an `Options` Data class, the kwargs are routed + # through it. Otherwise they are deep-merged with the subclass's # `DEFAULT_OPTIONS` Hash (legacy path) and frozen. def initialize(app, **options) @app = app @@ -83,7 +82,9 @@ def merge_headers(response) end def build_options(options) - return self.class::Options.new(**options) if self.class.const_defined?(:Options, false) + # Search ancestors so subclasses (e.g. Versioner::Path → Versioner::Base) + # inherit their parent's Options Data class without redeclaring it. + return self.class::Options.new(**options) if self.class.const_defined?(:Options) merge_default_options(options).freeze end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 89b050e29..8f4338f5b 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -6,26 +6,35 @@ 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 + 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 + + def_delegators :options, + :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 +42,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 bd99f1f59..1a505b938 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -7,8 +7,6 @@ class Formatter < Base include PrecomputedContentTypes Options = Data.define(:content_types, :default_format, :format, :formatters, :parsers) do - include Grape::Middleware::OptionsCompat - def initialize(content_types: nil, default_format: :txt, format: nil, formatters: nil, parsers: nil) super end @@ -94,7 +92,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/options_compat.rb b/lib/grape/middleware/options_compat.rb deleted file mode 100644 index e722eb1ab..000000000 --- a/lib/grape/middleware/options_compat.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -module Grape - module Middleware - # Mixin for per-middleware `Options` Data classes. - # - # Provides a Hash-like `[](key)` reader so the legacy `options[:key]` - # idiom in `Grape::Middleware::Base` (notably `#content_types` and - # `#content_type`) keeps working when a subclass swaps its Hash - # `DEFAULT_OPTIONS` for a `Data.define(...)`-backed `Options` class. - # Unknown keys return +nil+ to match the old behaviour. - module OptionsCompat - def [](key) - return nil unless respond_to?(key) - - public_send(key) - end - end - end -end diff --git a/lib/grape/middleware/precomputed_content_types.rb b/lib/grape/middleware/precomputed_content_types.rb index ca6b1f7c4..f22d2e17c 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 +options.content_types+ and + # +options.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(options.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] || options.format) || 'text/html' end private diff --git a/lib/grape/middleware/versioner/base.rb b/lib/grape/middleware/versioner/base.rb index 8b807e681..59ccd16a3 100644 --- a/lib/grape/middleware/versioner/base.rb +++ b/lib/grape/middleware/versioner/base.rb @@ -7,12 +7,16 @@ 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 + 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 CASCADE_PASS_HEADER = { 'X-Cascade' => 'pass' }.freeze @@ -21,18 +25,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 :options, :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 = self.options.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