Skip to content

Commit

Permalink
Force request body to be an schema object (#922)
Browse files Browse the repository at this point in the history
* Force request body to be an schema object

* Require "OpenStruct" explicitly in specs

* Update changelog and upgrading documentation
  • Loading branch information
numbata authored Apr 22, 2024
1 parent 262cdb9 commit e2b50b3
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 34 deletions.
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

0 comments on commit e2b50b3

Please sign in to comment.