From 392bbd79b8b1fea1804e4643ee9c1ed13b0432bb Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Fri, 1 May 2026 14:14:26 +0200 Subject: [PATCH] Reduce per-request allocations on the request hot path A bundle of small, behavior-preserving cleanups that drop allocations or chain walks from every request. Stable values (filter buckets, versioner options, etc.) are computed once at compile/init and read via `attr_reader`, instead of being re-resolved through nested hash lookups on every request. * `Endpoint#run`: precompute filter buckets (befores, before_validations, after_validations, afters, finallies) and `:build_params_with` once in `compile!`; replace the dynamic `define_method` accessor block with `attr_reader`; remove a dead `header 'Allow', header['Allow']` self-assignment. * `Versioner::Base`: replace the two `define_method`-per-key blocks (top-level and `version_options` keys) with `attr_reader` plus ivars precomputed in `initialize`. Per-request `pattern`/`prefix`/`vendor`/ `strict`/etc. lookups now hit attr loads. `DEFAULT_OPTIONS`, `attr_reader` list, and ivar assignments alphabetized; redundant `version_options` reader dropped. * `Middleware::Error` / `Middleware::Formatter`: same `options[...]` -> `attr_reader` migration. All option keys live in `DEFAULT_OPTIONS` (alphabetized), assigned to ivars in `initialize`, read via attr at request time. Drop the dead `rescue_subclasses` middleware key (the per-handler `:rescue_options` hash carries it, not the middleware options). * `Middleware::Base`: freeze `@options` after `merge_default_options` -- immutable once initialized. One spec adjusted to construct a fresh middleware instead of mutating `subject.options[...]`. * `ErrorFormatter::Base.call`: third argument is now the `rescue_options` hash directly (not the wrapping middleware options). Top-level `:backtrace` / `:original_exception` reads instead of `dig(:rescue_options, ...)`. * `Request#make_params`: skip the `except(:version, :route_info)` hash allocation in the common case where routing args contain only the known keys. * `Validators::Base#validate!`: lazy-allocate `array_errors`. Saves one Array allocation per validator per request when validation succeeds. * `InheritableValues#[]`: drop the per-call merge with `Hash#fetch` plus a fallback block. Lookup is 1.55x faster microbenched, allocates zero. Mostly helps boot/compile time (where ~30+ reads per endpoint hit this). * `Grape::API::Instance`: rename `ROOT_PREFIX_VERSIONING_KEY` to `ROOT_PREFIX_VERSIONING_KEYS` (it holds 3 symbols), pair via `zip` instead of `each_with_index` + manual indexing. * `Validations::Types::CustomTypeCoercer#infer_coercion_method`: guard-clause refactor of the nested if/else. End-to-end, 3-run averaged on Ruby 4.0.3, arm64-darwin25, against `/api/v1/hello` returning a small JSON object: no-yjit yjit master HEAD: 52,088 i/s 101,563 i/s master + this PR: 57,433 i/s 107,982 i/s +10.3% +6.3% Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + UPGRADING.md | 26 +++ benchmark/version_throughput/README.md | 48 +++++ benchmark/version_throughput/app.rb | 16 ++ benchmark/version_throughput/bench.rb | 27 +++ benchmark/version_throughput/run.rb | 183 ++++++++++++++++++ lib/grape/api/instance.rb | 10 +- lib/grape/endpoint.rb | 16 +- lib/grape/error_formatter/base.rb | 7 +- lib/grape/middleware/base.rb | 2 +- lib/grape/middleware/error.rb | 57 ++++-- lib/grape/middleware/formatter.rb | 28 ++- lib/grape/middleware/versioner/base.rb | 31 ++- lib/grape/request.rb | 6 +- lib/grape/util/inheritable_values.rb | 2 +- .../validations/types/custom_type_coercer.rb | 15 +- lib/grape/validations/validators/base.rb | 6 +- spec/grape/middleware/formatter_spec.rb | 10 +- 18 files changed, 410 insertions(+), 81 deletions(-) create mode 100644 benchmark/version_throughput/README.md create mode 100644 benchmark/version_throughput/app.rb create mode 100644 benchmark/version_throughput/bench.rb create mode 100644 benchmark/version_throughput/run.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c6952da08..9c3ef4bd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * [#2692](https://github.com/ruby-grape/grape/pull/2692): Replace per-request `Proc` allocation in `Router#transaction` with a `halt?` helper - [@ericproulx](https://github.com/ericproulx). * [#2694](https://github.com/ruby-grape/grape/pull/2694): Split `Versioner::Base#available_media_types` into an `attr_reader` plus `build_available_media_types` - [@ericproulx](https://github.com/ericproulx). * [#2695](https://github.com/ruby-grape/grape/pull/2695): Lift trailing `if/else` into guard clauses - [@ericproulx](https://github.com/ericproulx). +* [#2696](https://github.com/ruby-grape/grape/pull/2696): Reduce per-request allocations on the request hot path; migrate middleware options to `attr_reader` and freeze `@options` post-init - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/UPGRADING.md b/UPGRADING.md index c078a175b..dea74a8a5 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,6 +3,32 @@ Upgrading Grape ### Upgrading to >= 3.3 +#### `Grape::Middleware::Base#options` is now frozen + +`@options` is frozen at the end of `Grape::Middleware::Base#initialize` (after `merge_default_options`). The hash is initialized once and treated as immutable for the lifetime of the middleware. Custom middleware that mutates `options[...]` at runtime will now raise `FrozenError`. + +If your custom middleware was patching its own options on the fly: + +```ruby +# Before +class MyMiddleware < Grape::Middleware::Base + def before + options[:flag] = compute_flag + # ... + end +end + +# After — store mutable runtime state on a dedicated ivar +class MyMiddleware < Grape::Middleware::Base + def before + @flag = compute_flag + # ... + end +end +``` + +Reading `options[...]` is unchanged. + #### `Grape::Request#grape_routing_args` has been removed `grape_routing_args` was previously public to support third-party `params_builder` extensions, which have since been removed. With no remaining callers, the method has been removed. If you were calling it externally, read `env[Grape::Env::GRAPE_ROUTING_ARGS]` directly. diff --git a/benchmark/version_throughput/README.md b/benchmark/version_throughput/README.md new file mode 100644 index 000000000..4a461568b --- /dev/null +++ b/benchmark/version_throughput/README.md @@ -0,0 +1,48 @@ +# version_throughput + +Cross-version throughput benchmark for Grape. Measures `BenchAPI.call(env)` requests-per-second for a tiny JSON endpoint, against the same `app.rb` definition exec'd under several Grape releases plus `master`. Used to track regressions and validate refactor wins. + +## Files + +| File | Role | +|---|---| +| `app.rb` | The API under test. Kept deliberately small and version-agnostic — only DSL surface stable across Grape 3.x, so the same file can run against `3.0.0 … master` without edits. | +| `bench.rb` | Single-version benchmark. Loads `app.rb`, sanity-checks the response, runs `Benchmark.ips` (2s warmup + 5s measure), prints one `RESULT,,<μs>,,` line for the orchestrator. | +| `run.rb` | Orchestrator. For each version: writes a `Gemfile`, runs `bundle install` under `tmp/bench-versions//`, exec's `bench.rb` once without YJIT and once with `--yjit` (if available), parses results, writes `RESULTS.md`. | +| `RESULTS.md` | Generated report — overwritten on every run. | + +## Usage + +```sh +# default: 3.0.0, 3.0.1, 3.1.0, 3.1.1, 3.2.0, 3.2.1, master +ruby benchmark/version_throughput/run.rb + +# subset +GRAPE_VERSIONS="3.2.1,master" ruby benchmark/version_throughput/run.rb + +# different Ruby (e.g. one built with YJIT) +RBENV_VERSION=4.0.3 ruby benchmark/version_throughput/run.rb +``` + +`master` is benched against the working tree (`gemspec path: `), so unstaged changes are picked up. All other versions resolve to released gems on rubygems.org. + +## Output + +Each version produces a row in `RESULTS.md`: + +| Version | No-YJIT (i/s) | μs/req | YJIT (i/s) | μs/req | YJIT speedup | +|---|---:|---:|---:|---:|---:| +| … | … | … | … | … | … | + +YJIT columns are only emitted if the running Ruby was built with YJIT support (`run.rb` probes via `ruby --yjit -e 'exit(defined?(RubyVM::YJIT) ? 0 : 1)'`). + +## Interpreting results + +- **Noise floor is ~5-8%.** A single 5s window on macOS can easily move a few percent under thermal throttling or background load. Rerun before drawing conclusions on small deltas. +- **Run on a quiet machine.** Close other apps, plug in the laptop, don't touch the keyboard during the run. Each version takes ~14s of wall-clock measurement plus bundle install on first use. +- **`master` vs released gems is not apples-to-apples for code paths that changed.** If a refactor moved code between files, both numbers still measure the same `app.rb` request — that's the point — but interpret deltas as "end-to-end request cost" rather than per-method. +- **YJIT speedup is `(yjit_ips - no_yjit_ips) / no_yjit_ips`.** Both passes share the same Ruby binary; only the `--yjit` flag differs. + +## Adding a version + +Edit `DEFAULT_VERSIONS` in `run.rb`. The orchestrator handles `bundle install` and gemfile generation; nothing else needs to change as long as the new version exposes the DSL `app.rb` uses (`prefix`, `format`, `version 'v1', using: :path`, `get`). diff --git a/benchmark/version_throughput/app.rb b/benchmark/version_throughput/app.rb new file mode 100644 index 000000000..8e5a14105 --- /dev/null +++ b/benchmark/version_throughput/app.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Version-agnostic Grape API used by version_throughput benchmark. +# Kept tiny and using only DSL surface that's been stable across Grape 3.x — +# so the same script can be exec'd against 3.0.0 ... master without changes. +require 'grape' + +class BenchAPI < Grape::API + prefix :api + format :json + version 'v1', using: :path + + get '/hello' do + { hello: 'world' } + end +end diff --git a/benchmark/version_throughput/bench.rb b/benchmark/version_throughput/bench.rb new file mode 100644 index 000000000..fd0d1aae2 --- /dev/null +++ b/benchmark/version_throughput/bench.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# Single-version throughput bench. Invoked once per Grape version by +# `run.rb`. Loads `app.rb` (which is what gets compared across versions), +# warms up, then prints a single `RESULT,,,` +# line the orchestrator parses. +$LOAD_PATH.unshift(File.expand_path('.', __dir__)) +require 'benchmark/ips' +require 'app' + +env_template = Rack::MockRequest.env_for('/api/v1/hello', method: Rack::GET).freeze + +# Sanity check: 200 OK, body contains expected payload +status, _, body = BenchAPI.call(env_template.dup) +abort("sanity check failed: status=#{status}") unless status == 200 +collected = +'' +body.each { |c| collected << c } +abort("sanity check failed: body=#{collected.inspect}") unless collected.include?('world') + +report = Benchmark.ips do |ips| + ips.config(time: 5, warmup: 2, quiet: true) + ips.report('throughput') { BenchAPI.call(env_template.dup) } +end + +entry = report.entries.first +yjit = (defined?(RubyVM::YJIT) && RubyVM::YJIT.enabled?) ? 'on' : 'off' +puts format('RESULT,%.2f,%.4f,%.2f,%s', entry.ips, 1_000_000.0 / entry.ips, entry.error_percentage, yjit) diff --git a/benchmark/version_throughput/run.rb b/benchmark/version_throughput/run.rb new file mode 100644 index 000000000..f6b5b9b77 --- /dev/null +++ b/benchmark/version_throughput/run.rb @@ -0,0 +1,183 @@ +# frozen_string_literal: true + +# Orchestrator: runs benchmark/version_throughput/bench.rb against each +# Grape version listed below in a clean subprocess (one bundle per version +# under tmp/), parses the RESULT line, and writes a Markdown table to +# benchmark/version_throughput/RESULTS.md. +# +# Each version is benched twice: once without YJIT, once with `--yjit` +# (skipped if the running Ruby wasn't built with YJIT). Results show both +# columns plus the YJIT speedup. +# +# Usage: +# ruby benchmark/version_throughput/run.rb +# +# To bench against a specific subset: +# GRAPE_VERSIONS="3.0.0,3.2.1,master" ruby benchmark/version_throughput/run.rb +# +# To run a YJIT-enabled Ruby that isn't the project default: +# RBENV_VERSION=4.0.3 ruby benchmark/version_throughput/run.rb + +require 'fileutils' +require 'open3' +require 'rbconfig' + +ROOT = File.expand_path('../..', __dir__) +HERE = __dir__ +TMP = File.join(ROOT, 'tmp', 'bench-versions') + +DEFAULT_VERSIONS = %w[3.0.0 3.0.1 3.1.0 3.1.1 3.2.0 3.2.1 master].freeze +versions = (ENV['GRAPE_VERSIONS']&.split(',')&.map(&:strip) || DEFAULT_VERSIONS).freeze + +def gemfile_for(version) + if version == 'master' + <<~G + source 'https://rubygems.org' + gemspec path: '#{ROOT}' + gem 'benchmark-ips' + G + else + <<~G + source 'https://rubygems.org' + gem 'grape', '#{version}' + gem 'benchmark-ips' + gem 'rack' + G + end +end + +def prepare(version) + dir = File.join(TMP, version) + FileUtils.mkdir_p(dir) + File.write(File.join(dir, 'Gemfile'), gemfile_for(version)) + dir +end + +def run_bundle_install(dir) + Open3.capture2e({ 'BUNDLE_GEMFILE' => File.join(dir, 'Gemfile') }, 'bundle', 'install', '--quiet', chdir: dir) +end + +def run_bench(dir, yjit:) + args = ['bundle', 'exec', 'ruby'] + args << '--yjit' if yjit + args << File.join(HERE, 'bench.rb') + Open3.capture2e({ 'BUNDLE_GEMFILE' => File.join(dir, 'Gemfile') }, *args) +end + +def parse_result(stdout) + line = stdout.lines.reverse.find { |l| l.start_with?('RESULT,') } + return nil unless line + + _, ips, us, stddev, yjit = line.strip.split(',') + { ips: ips.to_f, us: us.to_f, stddev: stddev.to_f, yjit: yjit } +end + +def yjit_available? + out, status = Open3.capture2e('ruby', '--yjit', '-e', 'exit(defined?(RubyVM::YJIT) ? 0 : 1)') + status.success? && !out.include?('without YJIT support') +end + +with_yjit = yjit_available? +puts "YJIT available in current Ruby: #{with_yjit}" + +results = {} +versions.each do |version| + print "[#{version}] preparing... " + dir = prepare(version) + install_out, install_status = run_bundle_install(dir) + unless install_status.success? + puts "FAILED (bundle install)\n#{install_out}" + results[version] = { error: 'bundle install failed' } + next + end + + results[version] = {} + + # Pass 1: no YJIT + print 'no-yjit... ' + bench_out, bench_status = run_bench(dir, yjit: false) + if bench_status.success? && (parsed = parse_result(bench_out)) + results[version][:no_yjit] = parsed + printf('%.0f i/s', parsed[:ips]) + else + print 'FAILED' + results[version][:no_yjit] = { error: bench_status.success? ? 'no RESULT line' : 'bench failed', stdout: bench_out } + end + + # Pass 2: --yjit (skip if not available) + if with_yjit + print ' yjit... ' + bench_out, bench_status = run_bench(dir, yjit: true) + if bench_status.success? && (parsed = parse_result(bench_out)) + results[version][:yjit] = parsed + printf('%.0f i/s', parsed[:ips]) + else + print 'FAILED' + results[version][:yjit] = { error: bench_status.success? ? 'no RESULT line' : 'bench failed', stdout: bench_out } + end + end + puts +end + +# Write Markdown report +ruby_desc = `ruby -e 'puts RUBY_DESCRIPTION'`.strip +host_desc = `uname -mrs 2>/dev/null`.strip +report_path = File.join(HERE, 'RESULTS.md') + +format_ips = ->(n) { n.round.to_s.reverse.scan(/\d{1,3}/).join(',').reverse } + +File.open(report_path, 'w') do |f| + f.puts "# Grape throughput by version\n\n" + f.puts "Generated: #{Time.now.strftime('%Y-%m-%d %H:%M:%S %Z')} " + f.puts "Ruby: #{ruby_desc} " + f.puts "Host: #{host_desc} " + f.puts "YJIT available: #{with_yjit}\n\n" + f.puts 'Single-threaded `Benchmark.ips`, 2s warmup + 5s measure, ' \ + '`BenchAPI.call(env)` against `/api/v1/hello` returning a small JSON object. ' \ + "Reproduce with `ruby benchmark/version_throughput/run.rb`.\n\n" + + if with_yjit + f.puts '| Version | No-YJIT (i/s) | μs/req | YJIT (i/s) | μs/req | YJIT speedup |' + f.puts '|---|---:|---:|---:|---:|---:|' + else + f.puts '| Version | Throughput (i/s) | μs/req | ± stddev |' + f.puts '|---|---:|---:|---:|' + end + + versions.each do |version| + r = results[version] + if r.is_a?(Hash) && r[:error] + cols = with_yjit ? 5 : 3 + f.puts "| #{version} | error: #{r[:error]} #{'|' * cols}" + next + end + + no_yjit = r[:no_yjit] + yjit = r[:yjit] + + if with_yjit + no_yjit_cell = no_yjit&.dig(:ips) ? format_ips.call(no_yjit[:ips]) : 'err' + no_yjit_us = no_yjit&.dig(:us) ? format('%.2f', no_yjit[:us]) : '' + yjit_cell = yjit&.dig(:ips) ? format_ips.call(yjit[:ips]) : 'err' + yjit_us = yjit&.dig(:us) ? format('%.2f', yjit[:us]) : '' + speedup = (no_yjit&.dig(:ips) && yjit&.dig(:ips)) ? + format('%+.1f%%', (yjit[:ips] - no_yjit[:ips]) / no_yjit[:ips] * 100.0) : '—' + f.puts "| #{version} | #{no_yjit_cell} | #{no_yjit_us} | #{yjit_cell} | #{yjit_us} | #{speedup} |" + else + cell = no_yjit&.dig(:ips) ? format_ips.call(no_yjit[:ips]) : 'err' + us = no_yjit&.dig(:us) ? format('%.2f', no_yjit[:us]) : '' + stddev = no_yjit&.dig(:stddev) ? format('±%.2f%%', no_yjit[:stddev]) : '' + f.puts "| #{version} | #{cell} | #{us} | #{stddev} |" + end + end + + f.puts "\n## Notes" + f.puts '- All versions exercised through the same `BenchAPI` definition (kept stable in `app.rb`).' + f.puts '- Results are noisy at this scale (±5-8%); rerun if a number looks off.' + if with_yjit + f.puts '- `YJIT speedup` is `(yjit_ips - no_yjit_ips) / no_yjit_ips`.' + f.puts '- YJIT pass uses `ruby --yjit`; both passes share the same Ruby binary.' + end +end + +puts "\nWritten: #{report_path}" diff --git a/lib/grape/api/instance.rb b/lib/grape/api/instance.rb index 40eca7378..c471b8766 100644 --- a/lib/grape/api/instance.rb +++ b/lib/grape/api/instance.rb @@ -175,18 +175,18 @@ def collect_route_config_per_pattern(all_routes) end end - ROOT_PREFIX_VERSIONING_KEY = %i[version version_options root_prefix].freeze - private_constant :ROOT_PREFIX_VERSIONING_KEY + ROOT_PREFIX_VERSIONING_KEYS = %i[version version_options root_prefix].freeze + private_constant :ROOT_PREFIX_VERSIONING_KEYS # Allows definition of endpoints that ignore the versioning configuration # used by the rest of your API. def without_root_prefix_and_versioning inheritable_setting = self.class.inheritable_setting - deleted_values = inheritable_setting.namespace_inheritable.delete(*ROOT_PREFIX_VERSIONING_KEY) + deleted_values = inheritable_setting.namespace_inheritable.delete(*ROOT_PREFIX_VERSIONING_KEYS) yield ensure - ROOT_PREFIX_VERSIONING_KEY.each_with_index do |key, index| - inheritable_setting.namespace_inheritable[key] = deleted_values[index] + ROOT_PREFIX_VERSIONING_KEYS.zip(deleted_values) do |key, value| + inheritable_setting.namespace_inheritable[key] = value end end end diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 75a6c8877..659c9c27d 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -141,7 +141,7 @@ def inspect def run ActiveSupport::Notifications.instrument('endpoint_run.grape', endpoint: self, env:) do - @request = Grape::Request.new(env, build_params_with: inheritable_setting.namespace_inheritable[:build_params_with]) + @request = Grape::Request.new(env, build_params_with: @build_params_with) begin run_filters befores, :before @before_filter_passed = true @@ -150,7 +150,6 @@ def run header['Allow'] = env[Grape::Env::GRAPE_ALLOWED_METHODS].join(', ') raise Grape::Exceptions::MethodNotAllowed.new(header) unless options? - header 'Allow', header['Allow'] response_object = '' status 204 else @@ -215,11 +214,7 @@ def run_filters(filters, type = :other) end end - %i[befores before_validations after_validations afters finallies].each do |method| - define_method method do - inheritable_setting.namespace_stackable[method] - end - end + attr_reader :befores, :before_validations, :after_validations, :afters, :finallies def options? options[:options_route_enabled] && @@ -233,6 +228,13 @@ def options? def compile! @app = options[:app] || build_stack @helpers = build_helpers + stackable = inheritable_setting.namespace_stackable + @befores = stackable[:befores] + @before_validations = stackable[:before_validations] + @after_validations = stackable[:after_validations] + @afters = stackable[:afters] + @finallies = stackable[:finallies] + @build_params_with = inheritable_setting.namespace_inheritable[:build_params_with] end def to_routes diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb index 1bbb48ff0..050ea0603 100644 --- a/lib/grape/error_formatter/base.rb +++ b/lib/grape/error_formatter/base.rb @@ -5,13 +5,10 @@ module ErrorFormatter class Base class << self def call(message, backtrace, options = {}, env = nil, original_exception = nil) - merge_backtrace = backtrace.present? && options.dig(:rescue_options, :backtrace) - merge_original_exception = original_exception && options.dig(:rescue_options, :original_exception) - wrapped_message = wrap_message(present(message, env)) if wrapped_message.is_a?(Hash) - wrapped_message[:backtrace] = backtrace if merge_backtrace - wrapped_message[:original_exception] = original_exception.inspect if merge_original_exception + wrapped_message[:backtrace] = backtrace if backtrace.present? && options.dig(:rescue_options, :backtrace) + wrapped_message[:original_exception] = original_exception.inspect if original_exception && options.dig(:rescue_options, :original_exception) end format_structured_message(wrapped_message) diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index 6dba0187f..a0c8ee9a9 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -11,7 +11,7 @@ class Base # @param [Hash] options A hash of options, simply stored for use by subclasses. def initialize(app, **options) @app = app - @options = merge_default_options(options) + @options = merge_default_options(options).freeze @app_response = nil end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 709166eba..58b73993b 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -6,18 +6,43 @@ class Error < Base include PrecomputedContentTypes DEFAULT_OPTIONS = { - default_status: 500, + 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, rescue_all: false, rescue_grape_exceptions: false, - rescue_subclasses: true, + rescue_handlers: nil, rescue_options: { backtrace: false, original_exception: false }.freeze }.freeze + attr_reader :all_rescue_handler, :base_only_rescue_handlers, :default_error_formatter, + :default_message, :default_status, :error_formatters, :format, + :grape_exceptions_rescue_handler, :rescue_all, :rescue_grape_exceptions, + :rescue_handlers + + 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] + @rescue_all = @options[:rescue_all] + @rescue_grape_exceptions = @options[:rescue_grape_exceptions] + @rescue_handlers = @options[:rescue_handlers] + end + def call!(env) @env = env error_response(catch(:error) { return @app.call(@env) }) @@ -33,13 +58,13 @@ def rack_response(status, headers, message) end def format_message(message, backtrace, original_exception = nil) - format = env[Grape::Env::API_FORMAT] || options[:format] - formatter = Grape::ErrorFormatter.formatter_for(format, options[:error_formatters], options[:default_error_formatter]) + current_format = env[Grape::Env::API_FORMAT] || format + formatter = Grape::ErrorFormatter.formatter_for(current_format, error_formatters, default_error_formatter) return formatter.call(message, backtrace, options, env, original_exception) if formatter throw :error, status: 406, - message: "The requested format '#{format}' is not supported.", + message: "The requested format '#{current_format}' is not supported.", backtrace:, original_exception: end @@ -52,9 +77,9 @@ def find_handler(klass) end def error_response(error = {}) - status = error[:status] || options[:default_status] + status = error[:status] || default_status env[Grape::Env::API_ENDPOINT].status(status) # error! may not have been called - message = error[:message] || options[:default_message] + message = error[:message] || default_message headers = { Rack::CONTENT_TYPE => content_type }.tap do |h| h.merge!(error[:headers]) if error[:headers].is_a?(Hash) end @@ -68,12 +93,12 @@ def default_rescue_handler(exception) end def registered_rescue_handler(klass) - rescue_handler_from(:base_only_rescue_handlers) { |err| klass == err } || - rescue_handler_from(:rescue_handlers) { |err| klass <= err } + rescue_handler_from(base_only_rescue_handlers) { |err| klass == err } || + rescue_handler_from(rescue_handlers) { |err| klass <= err } end - def rescue_handler_from(key) - error, handler = options[key]&.find { |err, _handler| yield(err) } + def rescue_handler_from(handlers) + error, handler = handlers&.find { |err, _handler| yield(err) } return unless error @@ -83,16 +108,16 @@ def rescue_handler_from(key) def rescue_handler_for_grape_exception(klass) return unless klass <= Grape::Exceptions::Base return method(:error_response) if klass == Grape::Exceptions::InvalidVersionHeader - return unless options[:rescue_grape_exceptions] || !options[:rescue_all] + return unless rescue_grape_exceptions || !rescue_all - options[:grape_exceptions_rescue_handler] || method(:error_response) + grape_exceptions_rescue_handler || method(:error_response) end def rescue_handler_for_any_class(klass) return unless klass <= StandardError - return unless options[:rescue_all] || options[:rescue_grape_exceptions] + return unless rescue_all || rescue_grape_exceptions - options[:all_rescue_handler] || method(:default_rescue_handler) + all_rescue_handler || method(:default_rescue_handler) end def run_rescue_handler(handler, error, endpoint) @@ -110,7 +135,7 @@ def run_rescue_handler(handler, error, endpoint) end end - def error!(message, status = options[:default_status], headers = {}, backtrace = [], original_exception = nil) + def error!(message, status = default_status, headers = {}, backtrace = [], original_exception = nil) env[Grape::Env::API_ENDPOINT].status(status) # not error! inside route rack_response( status, headers.reverse_merge(Rack::CONTENT_TYPE => content_type), diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index d3e47518d..d51d73ffc 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -6,11 +6,25 @@ class Formatter < Base include PrecomputedContentTypes DEFAULT_OPTIONS = { - default_format: :txt + content_types: nil, + default_format: :txt, + format: nil, + formatters: nil, + parsers: nil }.freeze ALL_MEDIA_TYPES = '*/*' + attr_reader :default_format, :format, :formatters, :parsers + + def initialize(app, **options) + super + @default_format = @options[:default_format] + @format = @options[:format] + @formatters = @options[:formatters] + @parsers = @options[:parsers] + end + def before negotiate_content_type read_body_input @@ -39,7 +53,7 @@ def build_formatted_response(status, headers, bodies) end else # Allow content-type to be explicitly overwritten - formatter = fetch_formatter(headers, options) + formatter = fetch_formatter(headers) bodymap = ActiveSupport::Notifications.instrument('format_response.grape', formatter:, env:) do bodies.map { |body| formatter.call(body, env) } end @@ -49,9 +63,9 @@ def build_formatted_response(status, headers, bodies) throw :error, status: 500, message: e.message, backtrace: e.backtrace, original_exception: e end - def fetch_formatter(headers, options) + def fetch_formatter(headers) api_format = env.fetch(Grape::Env::API_FORMAT) { mime_types[headers[Rack::CONTENT_TYPE]] } - Grape::Formatter.formatter_for(api_format, options[:formatters]) + Grape::Formatter.formatter_for(api_format, formatters) end # Set the content type header for the API format if it is not already present. @@ -84,10 +98,10 @@ def read_rack_input(body) return if body.empty? media_type = rack_request.media_type - fmt = media_type ? mime_types[media_type] : options[:default_format] + fmt = media_type ? mime_types[media_type] : default_format throw :error, 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 @@ -121,7 +135,7 @@ def read_body_input? end def negotiate_content_type - fmt = format_from_extension || query_params['format'] || options[:format] || format_from_header || options[:default_format] + fmt = format_from_extension || query_params['format'] || format || format_from_header || default_format if content_type_for(fmt) env[Grape::Env::API_FORMAT] = fmt.to_sym else diff --git a/lib/grape/middleware/versioner/base.rb b/lib/grape/middleware/versioner/base.rb index bfd2a4e50..b2b4151ce 100644 --- a/lib/grape/middleware/versioner/base.rb +++ b/lib/grape/middleware/versioner/base.rb @@ -7,42 +7,39 @@ class Base < Grape::Middleware::Base include Grape::Middleware::PrecomputedContentTypes DEFAULT_OPTIONS = { + mount_path: nil, pattern: /.*/i, prefix: nil, - mount_path: nil, version_options: { - strict: false, cascade: true, parameter: 'apiver', + strict: false, vendor: nil }.freeze }.freeze CASCADE_PASS_HEADER = { 'X-Cascade' => 'pass' }.freeze - DEFAULT_OPTIONS.each_key do |key| - define_method key do - options[key] - end - end - - DEFAULT_OPTIONS[:version_options].each_key do |key| - define_method key do - options[:version_options][key] - end - end - def self.inherited(klass) super Versioner.register(klass) end - attr_reader :error_headers, :versions, :available_media_types + attr_reader :available_media_types, :cascade, :error_headers, :mount_path, :parameter, + :pattern, :prefix, :strict, :vendor, :versions def initialize(app, **options) super - @error_headers = cascade ? CASCADE_PASS_HEADER : {} - @versions = options[:versions]&.map(&:to_s) # making sure versions are strings to ease potential match + version_options = @options[:version_options] + @cascade = version_options[:cascade] + @mount_path = @options[:mount_path] + @parameter = version_options[:parameter] + @pattern = @options[:pattern] + @prefix = @options[:prefix] + @strict = version_options[:strict] + @vendor = version_options[:vendor] + @versions = @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 diff --git a/lib/grape/request.rb b/lib/grape/request.rb index bdd155cf5..f972c011a 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -160,8 +160,10 @@ def cookies def make_params params = @params_builder.call(rack_params) - routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS]&.except(:version, :route_info)&.presence - routing_args ? params.deep_merge!(routing_args) : params + routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS] + return params unless routing_args&.any? { |k, _| k != :version && k != :route_info } + + params.deep_merge!(routing_args.except(:version, :route_info)) rescue *Grape::RACK_ERRORS raise Grape::Exceptions::RequestError end diff --git a/lib/grape/util/inheritable_values.rb b/lib/grape/util/inheritable_values.rb index 48cebb23a..3c539752e 100644 --- a/lib/grape/util/inheritable_values.rb +++ b/lib/grape/util/inheritable_values.rb @@ -4,7 +4,7 @@ module Grape module Util class InheritableValues < BaseInheritable def [](name) - values[name] + @new_values.fetch(name) { @inherited_values[name] } end def []=(name, value) diff --git a/lib/grape/validations/types/custom_type_coercer.rb b/lib/grape/validations/types/custom_type_coercer.rb index b0a1e54be..1f72a016f 100644 --- a/lib/grape/validations/types/custom_type_coercer.rb +++ b/lib/grape/validations/types/custom_type_coercer.rb @@ -73,17 +73,10 @@ def coerced?(val) # @param method see #new # @return [#call] coercion method def infer_coercion_method(type, method) - if method - if method.respond_to? :parse - method.method :parse - else - method - end - else - # Try to use parse() declared on the target type. - # This may raise an exception, but we are out of ideas anyway. - type.method :parse - end + return type.method(:parse) unless method + return method unless method.respond_to?(:parse) + + method.method(:parse) end # Determine how the type validity of a coerced diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 33c8e322c..3cb947f5c 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -88,7 +88,7 @@ def validate!(params) attributes = SingleAttributeIterator.new(@attrs, @scope, params) # we collect errors inside array because # there may be more than one error per field - array_errors = [] + array_errors = nil attributes.each do |val, attr_name, empty_val| next if !@scope.required? && empty_val @@ -96,10 +96,10 @@ def validate!(params) validate_param!(attr_name, val) if @required || (hash_like?(val) && val.key?(attr_name)) rescue Grape::Exceptions::Validation => e - array_errors << e + (array_errors ||= []) << e end - raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors.any? + raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors end protected diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index dda260449..b0ece4a91 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -20,15 +20,13 @@ end context 'default format' do + subject { described_class.new(app, default_format: :json) } + let(:body) { ['foo'] } let(:env) do { Rack::PATH_INFO => '/somewhere', 'HTTP_ACCEPT' => '*/*' } end - before do - subject.options[:default_format] = :json - end - it 'returns JSON' do body.instance_eval do def to_json(*_args) @@ -240,8 +238,8 @@ def to_xml end it 'uses custom json formatter' do - subject.options[:formatters] = { json: ->(_obj, _env) { 'CUSTOM JSON FORMAT' } } - r = Rack::MockResponse[*subject.call(Rack::PATH_INFO => '/info.json')] + s = described_class.new(app, formatters: { json: ->(_obj, _env) { 'CUSTOM JSON FORMAT' } }) + r = Rack::MockResponse[*s.call(Rack::PATH_INFO => '/info.json')] expect(r.body).to eq('CUSTOM JSON FORMAT') end end