Skip to content

Force request body to be an schema object #922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 22, 2024
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#### Fixes

* Your contribution here.

* [#922](https://github.com/ruby-grape/grape-swagger/pull/922): Force request body to be an schema object - [@numbata](https://github.com/numbata)

### 2.0.2 (Februar 2, 2024)

Expand Down
9 changes: 9 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
## Upgrading Grape-swagger

### Upgrading to >= x.y.z

- Grape-swagger now documents array parameters within an object schema in Swagger. This aligns with grape's JSON structure requirements and ensures the documentation is correct.
- Previously, arrays were documented as standalone arrays, which could be incorrect based on grape's expectations.
- Check your API documentation and update your code or tests that use the old array format.

Attention: This update may require you to make changes to ensure your API integrations continue to work correctly.
For detailed reasons behind this update, refer to GitHub issue #666.

### Upgrading to >= 1.5.0

- The names generated for body parameter definitions and their references has changed. It'll now include the HTTP action as well as any path parameters.
Expand Down
17 changes: 3 additions & 14 deletions lib/grape-swagger/doc_methods/move_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,18 @@ def to_definition(path, params, route, definitions)

params_to_move = movable_params(params)

return (params + correct_array_param(params_to_move)) if should_correct_array?(params_to_move)

params << parent_definition_of_params(params_to_move, path, route)

params
end

private

def should_correct_array?(param)
param.length == 1 && param.first[:in] == 'body' && param.first[:type] == 'array'
end

def correct_array_param(param)
param.first[:schema] = { type: param.first.delete(:type), items: param.first.delete(:items) }

param
end

def parent_definition_of_params(params, path, route)
definition_name = OperationId.build(route, path)
build_definition(definition_name, params)
# NOTE: Parent definition is always object
@definitions[definition_name] = object_type
definition = @definitions[definition_name]

move_params_to_new(definition, params)

definition[:description] = route.description if route.try(:description)
Expand All @@ -53,6 +41,7 @@ def move_params_to_new(definition, params)
params, nested_params = params.partition { |x| !x[:name].to_s.include?('[') }
params.each do |param|
property = param[:name]

param_properties, param_required = build_properties([param])
add_properties_to_definition(definition, param_properties, param_required)
related_nested_params, nested_params = nested_params.partition { |x| x[:name].start_with?("#{property}[") }
Expand Down
2 changes: 1 addition & 1 deletion lib/grape-swagger/doc_methods/parse_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def document_add_extensions(settings)

def document_array_param(value_type, definitions)
if value_type[:documentation].present?
param_type = value_type[:documentation][:param_type]
param_type = value_type[:documentation][:param_type] || value_type[:documentation][:in]
doc_type = value_type[:documentation][:type]
type = DataType.mapping(doc_type) if doc_type && !DataType.request_primitive?(doc_type)
collection_format = value_type[:documentation][:collectionFormat]
Expand Down
26 changes: 21 additions & 5 deletions spec/issues/539_array_post_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,35 @@ class ArrayOfElements < Grape::Entity
let(:definitions) { subject['definitions'] }

specify do
expect(parameters).to eql(
expect(parameters).to match(
[
{
'in' => 'body', 'name' => 'elements', 'required' => true, 'schema' => {
'type' => 'array', 'items' => { '$ref' => '#/definitions/Element' }
}
'name' => 'postIssue539',
'required' => true,
'in' => 'body',
'schema' => { '$ref' => '#/definitions/postIssue539' }
}
]
)
end

specify do
expect(definitions).to eql(
expect(definitions).to include(
'postIssue539' => {
'description' => 'create account',
'type' => 'object',
'properties' => {
'elements' => {
'type' => 'array', 'items' => { '$ref' => '#/definitions/Element' }
}
},
'required' => ['elements']
}
)
end

specify do
expect(definitions).to include(
'Element' => {
'type' => 'object',
'properties' => {
Expand Down
21 changes: 18 additions & 3 deletions spec/issues/542_array_of_type_in_post_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,37 @@
end

let(:parameters) { subject['paths']['/issue_542']['post']['parameters'] }
let(:definitions) { subject['definitions'] }

specify do
expect(parameters).to eql(
[
{
'in' => 'body',
'name' => 'logs',
'name' => 'postIssue542',
'required' => true,
'schema' => {
'$ref' => '#/definitions/postIssue542'
}
}
]
)
end

specify do
expect(definitions).to include(
'postIssue542' => {
'type' => 'object',
'properties' => {
'logs' => {
'type' => 'array',
'items' => {
'type' => 'string'
}
}
}
]
},
'required' => ['logs']
}
)
end
end
38 changes: 30 additions & 8 deletions spec/issues/553_align_array_put_post_params_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,38 @@
describe 'in body' do
describe 'post request' do
let(:params) { subject['paths']['/in_body']['post']['parameters'] }
let(:definitions) { subject['definitions'] }

specify do
expect(params).to eql(
[
{
'in' => 'body',
'name' => 'guid',
'name' => 'postInBody',
'required' => true,
'schema' => {
'schema' => { '$ref' => '#/definitions/postInBody' }
}
]
)
expect(definitions).to include(
'postInBody' => {
'description' => 'create foo',
'type' => 'object',
'properties' => {
'guid' => {
'type' => 'array',
'items' => { 'type' => 'string' }
}
}
]
},
'required' => ['guid']
}
)
end
end

describe 'put request' do
let(:params) { subject['paths']['/in_body/{id}']['put']['parameters'] }
let(:definitions) { subject['definitions'] }

specify do
expect(params).to eql(
Expand All @@ -128,14 +140,24 @@
},
{
'in' => 'body',
'name' => 'guid',
'name' => 'putInBodyId',
'required' => true,
'schema' => {
'schema' => { '$ref' => '#/definitions/putInBodyId' }
}
]
)
expect(definitions).to include(
'putInBodyId' => {
'description' => 'put specific foo',
'type' => 'object',
'properties' => {
'guid' => {
'type' => 'array',
'items' => { 'type' => 'string' }
}
}
]
},
'required' => ['guid']
}
)
end
end
Expand Down
47 changes: 47 additions & 0 deletions spec/issues/666_array_of_entities_in_request_body_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# frozen_string_literal: true

require 'spec_helper'

describe 'definition names' do
before :all do
module TestDefinition
module Entity
class Account < Grape::Entity
expose :cma, documentation: { type: Integer, desc: 'Company number', param_type: 'body' }
expose :name, documentation: { type: String, desc: 'Company Name' }
expose :environment, documentation: { type: String, desc: 'Test Environment' }
expose :sites, documentation: { type: Integer, desc: 'Amount of sites' }
expose :username, documentation: { type: String, desc: 'Username for Dashboard' }
expose :password, documentation: { type: String, desc: 'Password for Dashboard' }
end

class Accounts < Grape::Entity
expose :accounts, documentation: { type: Entity::Account, is_array: true, param_type: 'body', required: true }
end
end
end
end

let(:app) do
Class.new(Grape::API) do
namespace :issue_666 do
desc 'createTestAccount',
params: TestDefinition::Entity::Accounts.documentation
post 'create' do
present params
end
end

add_swagger_documentation format: :json
end
end

subject do
get '/swagger_doc'
JSON.parse(last_response.body)
end

specify do
expect(subject['definitions']['postIssue666Create']['type']).to eq('object')
end
end
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

MODEL_PARSER = ENV.key?('MODEL_PARSER') ? ENV['MODEL_PARSER'].to_s.downcase.sub('grape-swagger-', '') : 'mock'

require 'ostruct'
require 'grape'
require 'grape-swagger'

Expand Down
3 changes: 1 addition & 2 deletions spec/swagger_v2/params_array_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@
end

specify do
expect(subject['definitions']['postArrayOfType']['type']).to eql 'array'
expect(subject['definitions']['postArrayOfType']['items']).to eql(
expect(subject['definitions']['postArrayOfType']).to eql(
'type' => 'object',
'properties' => {
'array_of_string' => {
Expand Down