From ed2f86a28615e00d7861c180fd79aa431e73ff8c Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Tue, 28 Jul 2026 00:26:49 +0200 Subject: [PATCH] Document the definition-time rejection of uncoercible collection types #2817 made ArrayCoercer build its element coercer eagerly in the constructor rather than memoizing it on first use, so that coercers -- shared across requests -- create no state at request time. A side effect went unrecorded: an Array/Set whose element type Grape cannot coerce is now rejected while the params block is evaluated, so an application that boots today can fail to boot on 4.0 with ArgumentError: type Foo should support coercion via `[]` Previously the element coercer was built on the first request that supplied the parameter, so a declaration never exercised by a request -- a documentation-only type, which is what this breaks in practice -- never raised at all; one that was exercised got the same ArgumentError swallowed by the coercion validator into a generic 400 "is invalid". The tightening is right: the non-collection form (type: Foo) has always raised at definition time, and the collection form's leniency was an accident of the lazy build rather than a supported behavior. But it is a load-time break, so it needs an UPGRADING entry, which this adds -- with the three fixes that actually work (a one-argument parse, coerce_with, or dropping type: in favour of documentation:), and a warning that defining self.[] only silences the load-time error while leaving the parameter failing per request. Also pin the behavior: nothing asserted it anywhere, so a future refactor could revert it silently. The three "raises" examples fail against the pre-#2817 lazy build and pass against the eager one. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 3 +- UPGRADING.md | 57 ++++++++++++++++++++++++++++ spec/grape/validations/types_spec.rb | 42 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44c056462..5b74e5cfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ * [#2831](https://github.com/ruby-grape/grape/pull/2831): Document and test open (beginless/endless) `Range`s as a `values:`/`except_values:` alternative to a dedicated numericality validator - [@dblock](https://github.com/dblock). * [#2830](https://github.com/ruby-grape/grape/pull/2830): Remove `Grape::Util::InheritableValues`, resolving inheritable settings by walking `Grape::Util::InheritableSetting#parent` instead of layering a second chain of stores, and keep namespace settings in a plain Hash - [@ericproulx](https://github.com/ericproulx). * [#2833](https://github.com/ruby-grape/grape/pull/2833): Fix `Grape::API::Instance.to_s` returning an empty string instead of the class name when the instance has no base (regression from #2581) - [@ericproulx](https://github.com/ericproulx). +* [#2837](https://github.com/ruby-grape/grape/pull/2837): Document the definition-time rejection of an `Array`/`Set` of an uncoercible element type introduced by #2817, and pin it with specs - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes @@ -47,7 +48,7 @@ * [#2816](https://github.com/ruby-grape/grape/pull/2816): Guard `length`, `same_as` and `oneof` validators against non-hash params (500 → 400) - [@ericproulx](https://github.com/ericproulx). * [#2814](https://github.com/ruby-grape/grape/pull/2814): Fix `type: [A, B]` combined with `values:`/`except_values:` raising `IncompatibleOptionValues` at definition time - [@ericproulx](https://github.com/ericproulx). * [#2815](https://github.com/ruby-grape/grape/pull/2815): Fix line-anchored regexes: `type: JSON` payloads containing a blank line were silently coerced to `nil` - [@ericproulx](https://github.com/ericproulx). -* [#2817](https://github.com/ruby-grape/grape/pull/2817): Remove request-time mutable state from Array and custom-type coercers - [@ericproulx](https://github.com/ericproulx). +* [#2817](https://github.com/ruby-grape/grape/pull/2817): Remove request-time mutable state from Array and custom-type coercers, which also makes an `Array`/`Set` of an uncoercible element type raise when the API is defined instead of on the first request that supplies it (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2819](https://github.com/ruby-grape/grape/pull/2819): Freeze coercers at construction, synchronize `Grape::Util::Cache` lookups, and assign `Route#regexp_capture_index` eagerly - [@ericproulx](https://github.com/ericproulx). * [#2824](https://github.com/ruby-grape/grape/pull/2824): Fix cascaded routes (`X-Cascade: pass`) leaking `route_info` and path captures into the next matched route's `route` and `params` - [@ericproulx](https://github.com/ericproulx). * [#2825](https://github.com/ruby-grape/grape/pull/2825): Stop `present` entity autodetection from picking up a top-level `::Entity` constant - [@ericproulx](https://github.com/ericproulx). diff --git a/UPGRADING.md b/UPGRADING.md index dacb7e55b..ab4838229 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,6 +3,63 @@ Upgrading Grape ### Upgrading to >= 4.0.0 +#### `Array`/`Set` of an unsupported type is rejected when the API is defined + +Declaring a collection whose element type Grape cannot coerce — `type: Array[Foo]` or `type: Set[Foo]` where `Foo` is neither a primitive, a structure, nor a valid custom type — now raises as soon as the `params` block is evaluated, i.e. while the API class is being loaded: + +``` +ArgumentError: type Foo should support coercion via `[]` +``` + +This is a **load-time** failure. An application that boots today can fail to boot on 4.0 without any request being made. + +A valid custom type is one that implements a class-level `parse` taking exactly one argument (`Grape::Validations::Types.custom?`). A class that implements neither that nor `[]` was never coercible, so such a declaration was always a misconfiguration — but it used to surface much later, and much less clearly. + +**What changed.** Nothing about which types are supported; only *when* the element coercer is built. [#2817](https://github.com/ruby-grape/grape/pull/2817) made `ArrayCoercer` build it eagerly in the constructor rather than memoizing it on first use, because coercers are shared across requests and must not create state at request time. Building it eagerly means the unsupported element type is discovered while the route is being defined. + +Previously the coercer was built on the first request that actually supplied the parameter. An API that declared the parameter but was never sent one — a documentation-only declaration, for instance — never built it and never raised. When a request did supply it, the same `ArgumentError` was raised inside the coercer and swallowed by the coercion validator into a generic `400`: + +```json +{ "error": "foo is invalid" } +``` + +so the misconfiguration presented as a puzzling per-request validation failure rather than as a broken declaration. + +Note that the non-collection form has always raised at definition time: + +| declaration | before | 4.0 | +| --- | --- | --- | +| `type: Foo` | `ArgumentError` when defined | unchanged | +| `type: Array[Foo]` | accepted; `400 "is invalid"` per request | `ArgumentError` when defined | + +The collection form's leniency was an accident of the lazy build, not a supported behavior. The two are now consistent. + +**Fixing a declaration that now raises.** Give the type a one-argument `parse`, which is what makes it a custom type: + +```ruby +class Foo + def self.parse(value) + new(value) + end +end + +params { requires :foos, type: Array[Foo] } +``` + +Or coerce the collection yourself, in which case Grape does not build an element coercer at all: + +```ruby +params { requires :foos, type: Array[Foo], coerce_with: ->(value) { Array(value).map { |v| Foo.new(v) } } } +``` + +Or, if the class is only there to describe the parameter and never to coerce it — the common case behind this break — drop `type:` and document it instead: + +```ruby +params { requires :foos, documentation: { type: Foo } } +``` + +Defining `self.[]` on the class silences the load-time error, because that is the escape hatch for `dry-types` objects, but it does **not** make the type coercible: such a parameter still fails with `400 "is invalid"` on every request. Prefer `parse` or `coerce_with`. + #### The `cascade` getter returns the configured value Calling `cascade` with no argument used to report whether cascading had been *configured at all* (`cascade false` still read back as `true`, contradicting the actual runtime behavior, which was correctly disabled). It now returns the configured value itself — `true`/`false` as set, or `true` when never set. Code that used the getter to detect "was `cascade` called" rather than "does this API cascade" must track that separately. diff --git a/spec/grape/validations/types_spec.rb b/spec/grape/validations/types_spec.rb index 8fd26d475..24e55ab95 100644 --- a/spec/grape/validations/types_spec.rb +++ b/spec/grape/validations/types_spec.rb @@ -98,6 +98,48 @@ def self.parse; end expect(a_coercer.object_id).to eq(b_coercer.object_id) end + # The element coercer is built eagerly (see ArrayCoercer), so a collection + # of a type Grape cannot coerce is rejected here -- while the params block + # is evaluated -- rather than on the first request that supplies the + # parameter, where it used to surface as a generic 400. See UPGRADING. + describe 'a collection of an unsupported element type' do + before { stub_const('UncoercibleType', Class.new) } + + it 'raises for an Array of it' do + expect { described_class.build_coercer(Array[UncoercibleType]) } + .to raise_error(ArgumentError, 'type UncoercibleType should support coercion via `[]`') + end + + it 'raises for a Set of it' do + expect { described_class.build_coercer(Set[UncoercibleType]) } + .to raise_error(ArgumentError, 'type UncoercibleType should support coercion via `[]`') + end + + it 'raises while the params block is evaluated' do + expect do + Class.new(Grape::API) do + params { requires :foos, type: Array[UncoercibleType] } + get('/foos') { 'never reached' } + end + end.to raise_error(ArgumentError, 'type UncoercibleType should support coercion via `[]`') + end + + # A one-argument `parse` makes it a custom type, which ::custom? accepts + # and which routes to CustomTypeCollectionCoercer instead of dry-types. + it 'is accepted once the element type implements a one-argument parse' do + stub_const('CoercibleType', Class.new { def self.parse(value) = value }) + + expect { described_class.build_coercer(Array[CoercibleType]) }.not_to raise_error + end + + # coerce_with takes over the whole collection, so no element coercer is + # built and the element type is never looked up. + it 'is accepted when coerce_with supplies the coercion' do + expect { described_class.build_coercer(Array[UncoercibleType], method: ->(v) { Array(v) }) } + .not_to raise_error + end + end + # Coercer instances are shared across requests, so they must be frozen at # construction — a request-time lazy ivar write would be a data race and # now raises FrozenError instead.