Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* [#978](https://github.com/ruby-grape/grape-swagger/pull/978): Fix Grape 3.2+ compatibility: desc kwargs, custom types, multi-type param recovery; bump Grape to `>= 2.1, < 5.0`. See [UPGRADING](UPGRADING.md) - [@numbata](https://github.com/numbata).
* [#982](https://github.com/ruby-grape/grape-swagger/pull/982): Fix test suite compatibility with Grape 4.0 (grape=HEAD CI) - [@numbata](https://github.com/numbata).
* [#981](https://github.com/ruby-grape/grape-swagger/pull/981): Use `endpoint.endpoints` instead of `endpoint.options[:app]` - [@ericproulx](https://github.com/ericproulx).
* [#985](https://github.com/ruby-grape/grape-swagger/pull/985): Fix the test suite against unreleased Grape 4.0: give collection custom types a `parse`, and stop building `Grape::Util::StackableValues` directly now that it is a read-only view. See [UPGRADING](UPGRADING.md) - [@ericproulx](https://github.com/ericproulx).
* [#983](https://github.com/ruby-grape/grape-swagger/pull/983): Read route metadata via reader methods instead of `route.options[...]` - [@ericproulx](https://github.com/ericproulx).
* [#984](https://github.com/ruby-grape/grape-swagger/pull/984): Drop the undocumented `formats:` / `content_types:` aliases for `produces:` - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.
Expand Down
2 changes: 2 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
end
```

On **Grape 4.0** the same requirement extends to collections: `type: Array[MyClass]` and `type: Set[MyClass]` now raise the same `ArgumentError` while the `params` block is evaluated, i.e. as the API class loads. Grape used to build the element coercer lazily, on the first request that supplied the parameter, so a documentation-only collection type was never coerced and never raised — until it was, as a confusing per-request `400 "is invalid"`. See [ruby-grape/grape#2817](https://github.com/ruby-grape/grape/pull/2817) and Grape's UPGRADING. The fix is the same `parse` — note that `parse(val) = val` performs no validation and is only appropriate when the type is documentation-only and the route never actually coerces client input; otherwise implement real coercion or supply `coerce_with:`, or drop `type:` and declare the type under `documentation:` if it only describes the parameter.

- **Multi-type params (`type: [A, B]`) on Grape 3.2+**: swagger output now reflects the first declared type (e.g. `type: [Integer, Float]` produces `"integer"`). Previously, Grape 3.2+ serialized the `VariantCollectionCoercer` wrapper via `#to_s`, leaking `"#<Grape::Validations::Types::VariantCollectionCoercer:0x...>"` into the documentation. No action required, but if you were programmatically post-processing that string, the fix will change the output.
- **`desc(..., formats:)` and `desc(..., content_types:)` no longer set the swagger `produces` field.** These were undocumented aliases for `produces:`. Use `produces:` instead (e.g. `desc 'x', produces: ['application/xml']`), which is unchanged.

Expand Down
180 changes: 112 additions & 68 deletions spec/lib/request_param_parsers/route_spec.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,42 @@
# frozen_string_literal: true

describe GrapeSwagger::RequestParamParsers::Route do
# Grape::Util::StackableValues's constructor signature is a Grape-internal
# detail that differs across supported Grape versions (and grape=HEAD), so
# rather than calling `.new` directly, these specs harvest a real instance
# through the public API and stub its readers for the scenario under test.
# `instance_double` won't pass `route.rb`'s `is_a?(Grape::Util::StackableValues)`
# check, hence harvesting a real instance instead of building a plain double.
# `#[]` reads Grape's own ivars, not the stubbed readers below, so it is
# stubbed too -- otherwise it would silently return the harvest probe's real
# (empty) namespace instead of the fixture if a caller ever read through `[]`.
def stackable_values_double(new_values, inherited_values = {})
harvested = Class.new(Grape::API) do
namespace(':harvest') { get('/probe') { 'probe' } }
end.routes.first.app.inheritable_setting.namespace_stackable

allow(harvested).to receive_messages(new_values: new_values, inherited_values: inherited_values)
allow(harvested).to receive(:[]) { |key| new_values[key] || inherited_values[key] }
harvested
end

let(:route) { instance_double('route', app: nil) }
let(:parser) { described_class.new(route, nil, nil, nil) }

describe '#parse' do
subject(:parse_request_params) { described_class.parse(route, nil, nil, nil) }

context 'when inherited namespace stackable values contain path params across levels' do
let(:root_stackable) { Grape::Util::StackableValues.new }
let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) }
let(:root_stackable) do
stackable_values_double(namespace: [instance_double('namespace', space: ':account_id', options: { required: true, type: 'Integer' })])
end
let(:nested_stackable) do
stackable_values_double({ namespace: [instance_double('namespace', space: ':id', options: { required: true, type: 'String' })] }, root_stackable)
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) }
let(:app) { instance_double('app', inheritable_setting: inheritable_setting) }

before do
root_stackable[:namespace] = instance_double('namespace', space: ':account_id', options: { required: true, type: 'Integer' })
nested_stackable[:namespace] = instance_double('namespace', space: ':id', options: { required: true, type: 'String' })

allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(
'account_id' => {},
Expand Down Expand Up @@ -50,12 +70,13 @@
end

context 'when route.params contains only symbol-keyed params' do
let(:stackable) { Grape::Util::StackableValues.new }
let(:stackable) do
stackable_values_double(namespace: [instance_double('namespace', space: ':id', options: { required: true, type: 'Integer' })])
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
stackable[:namespace] = instance_double('namespace', space: ':id', options: { required: true, type: 'Integer' })
allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(id: {})
end
Expand All @@ -68,12 +89,13 @@
end

context 'when namespace space is a colon-prefixed string for a symbol-keyed param' do
let(:stackable) { Grape::Util::StackableValues.new }
let(:stackable) do
stackable_values_double(namespace: [instance_double('namespace', space: ':account_id', options: { required: true, type: 'Integer' })])
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
stackable[:namespace] = instance_double('namespace', space: ':account_id', options: { required: true, type: 'Integer' })
allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(account_id: {})
end
Expand All @@ -86,12 +108,13 @@
end

context 'when namespace space has more than one leading colon' do
let(:stackable) { Grape::Util::StackableValues.new }
let(:stackable) do
stackable_values_double(namespace: [instance_double('namespace', space: '::id', options: { required: true, type: 'Integer' })])
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
stackable[:namespace] = instance_double('namespace', space: '::id', options: { required: true, type: 'Integer' })
allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(':id' => {})
end
Expand All @@ -104,15 +127,16 @@
end

context 'when inherited namespace stackable values redefine the same path param' do
let(:root_stackable) { Grape::Util::StackableValues.new }
let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) }
let(:root_stackable) do
stackable_values_double(namespace: [instance_double('namespace', space: ':id', options: { required: true, type: 'Integer' })])
end
let(:nested_stackable) do
stackable_values_double({ namespace: [instance_double('namespace', space: ':id', options: { required: true, type: 'String' })] }, root_stackable)
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
root_stackable[:namespace] = instance_double('namespace', space: ':id', options: { required: true, type: 'Integer' })
nested_stackable[:namespace] = instance_double('namespace', space: ':id', options: { required: true, type: 'String' })

allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(
'id' => {}
Expand All @@ -127,23 +151,28 @@
end

context 'when inherited namespace stackable values partially override the same path param' do
let(:root_stackable) { Grape::Util::StackableValues.new }
let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) }
let(:root_stackable) do
stackable_values_double(
namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { type: 'integer', format: 'int64' } }
)]
)
end
let(:nested_stackable) do
stackable_values_double(
{ namespace: [instance_double(
'namespace',
space: ':id',
options: { desc: 'inner description' }
)] }, root_stackable
)
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
root_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { type: 'integer', format: 'int64' } }
)
nested_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { desc: 'inner description' }
)

allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(
'id' => {}
Expand All @@ -161,23 +190,28 @@
end

context 'when inherited namespace stackable values partially override nested documentation' do
let(:root_stackable) { Grape::Util::StackableValues.new }
let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) }
let(:root_stackable) do
stackable_values_double(
namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { type: 'integer', format: 'int64' } }
)]
)
end
let(:nested_stackable) do
stackable_values_double(
{ namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { desc: 'inner description' } }
)] }, root_stackable
)
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
root_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { type: 'integer', format: 'int64' } }
)
nested_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { desc: 'inner description' } }
)

allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(
'id' => {}
Expand All @@ -194,23 +228,28 @@
end

context 'when inherited namespace stackable values partially override deeply nested hashes' do
let(:root_stackable) { Grape::Util::StackableValues.new }
let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) }
let(:root_stackable) do
stackable_values_double(
namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { schema: { type: 'integer', format: 'int64' } } }
)]
)
end
let(:nested_stackable) do
stackable_values_double(
{ namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { schema: { desc: 'inner description' } } }
)] }, root_stackable
)
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
root_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { schema: { type: 'integer', format: 'int64' } } }
)
nested_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { schema: { desc: 'inner description' } } }
)

allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(
'id' => {}
Expand All @@ -227,23 +266,28 @@
end

context 'when inherited namespace stackable values override array options' do
let(:root_stackable) { Grape::Util::StackableValues.new }
let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) }
let(:root_stackable) do
stackable_values_double(
namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { values: %w[outer-a outer-b] } }
)]
)
end
let(:nested_stackable) do
stackable_values_double(
{ namespace: [instance_double(
'namespace',
space: ':id',
options: { documentation: { values: %w[inner-a inner-b] } }
)] }, root_stackable
)
end
let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) }
let(:app) { instance_double('app', inheritable_setting:) }

before do
root_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { values: %w[outer-a outer-b] } }
)
nested_stackable[:namespace] = instance_double(
'namespace',
space: ':id',
options: { documentation: { values: %w[inner-a inner-b] } }
)

allow(route).to receive(:app).and_return(app)
allow(route).to receive(:params).and_return(
'id' => {}
Expand Down
32 changes: 23 additions & 9 deletions spec/lib/swagger_routing_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
# frozen_string_literal: true

describe GrapeSwagger::SwaggerRouting do
# Grape::Util::StackableValues's constructor and writers are Grape-internal
# details that differ across supported Grape versions (and grape=HEAD), where
# the class is a read-only view with no `.new(0 args)` and no `[]=`.
# `combine_namespaces` only ever reads keys off it, so these specs stub that
# contract — `[](key) => Array` — rather than building a real instance.
def namespace_stackable_double(values)
instance_double('namespace_stackable').tap do |stackable|
allow(stackable).to receive(:[]) { |key| values[key] }
end
end

let(:routing) do
Class.new do
include GrapeSwagger::SwaggerRouting
Expand Down Expand Up @@ -43,9 +54,10 @@

context 'when joined mount paths contain multiple double-slash runs' do
it 'normalizes all doubled slashes in the namespace key' do
namespace_stackable = Grape::Util::StackableValues.new
namespace_stackable[:namespace] = namespace
namespace_stackable[:mount_path] = ['//foo/', '/bar']
namespace_stackable = namespace_stackable_double(
namespace: [namespace],
mount_path: ['//foo/', '/bar']
)
endpoint = instance_double(
'endpoint',
endpoints: nil,
Expand All @@ -63,14 +75,16 @@

context 'when an endpoint exposes nested endpoints' do
it 'processes sub-endpoints returned by the endpoint accessor' do
parent_namespace_stackable = Grape::Util::StackableValues.new
parent_namespace_stackable[:namespace] = namespace
parent_namespace_stackable[:mount_path] = ['/api']
parent_namespace_stackable = namespace_stackable_double(
namespace: [namespace],
mount_path: ['/api']
)

child_namespace = instance_double('child_namespace', options: {})
child_namespace_stackable = Grape::Util::StackableValues.new
child_namespace_stackable[:namespace] = child_namespace
child_namespace_stackable[:mount_path] = ['/api', '/v1']
child_namespace_stackable = namespace_stackable_double(
namespace: [child_namespace],
mount_path: ['/api', '/v1']
)

child_endpoint = instance_double(
'child_endpoint',
Expand Down
9 changes: 7 additions & 2 deletions spec/support/model_parsers/mock_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ class QueryInputElement < OpenStruct; end

class QueryInput < OpenStruct; end

class ApiError < OpenStruct; end
class ApiError < OpenStruct
# Custom types need a one-argument .parse for Grape to coerce them:
# `type: X` since Grape 3.2, `type: Array[X]` since Grape 4.0 (grape#2817).
def self.parse(val) = val
end

class SecondApiError < OpenStruct; end

Expand All @@ -73,7 +77,8 @@ class DocumentedHashAndArrayModel < OpenStruct; end

module NestedModule
class ApiResponse < OpenStruct
# Grape 3.2+ requires unknown types to implement .parse (arity 1)
# Custom types need a one-argument .parse for Grape to coerce them:
# `type: X` since Grape 3.2, `type: Array[X]` since Grape 4.0 (grape#2817).
def self.parse(val) = val
end
end
Expand Down
Loading
Loading