Skip to content

Allow passing of Array with brackets (eg. Array[String], Array[Integer]) #455

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 10 commits into from
Jun 16, 2016
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Metrics/AbcSize:
# Offense count: 3
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 206
Max: 209

# Offense count: 10
Metrics/CyclomaticComplexity:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#### Fixes

* [#455](https://github.com/ruby-grape/grape-swagger/pull/455): Setting `type:` option as `Array[Class]` creates `array` type in JSON - [@tyspring](https://github.com/tyspring).
* [#450](https://github.com/ruby-grape/grape-swagger/pull/438): Do not add :description to definitions if :description is missing on path - [@texpert](https://github.com/texpert).
* [#447](https://github.com/ruby-grape/grape-swagger/pull/447): Version part of the url is now ignored when generating tags for endpoint - [@anakinj](https://github.com/anakinj).
* [#444](https://github.com/ruby-grape/grape-swagger//pull/444): Default value provided in the documentation hash, override the grape default [@scauglog](https://github.com/scauglog).
Expand Down
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,30 @@ end
}
```

#### Array type

Array types are also supported.

```ruby
params do
requires :action_ids, type: Array[Integer]
end
post :act do
...
end
```

```json
{
"in": "formData",
"name": "action_ids",
"type": "array",
"items": {
"type": "integer"
},
"required": true
}
```

#### Multi types

Expand Down
3 changes: 2 additions & 1 deletion lib/grape-swagger/doc_methods/parse_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ def document_array_param(value_type)
if value_type[:is_array]
if value_type[:documentation].present?
param_type = value_type[:documentation][:param_type]
type = GrapeSwagger::DocMethods::DataType.mapping(value_type[:documentation][:type])
doc_type = value_type[:documentation][:type]
type = GrapeSwagger::DocMethods::DataType.mapping(doc_type) if doc_type
end
array_items = { 'type' => type || value_type[:data_type] }

Expand Down
23 changes: 14 additions & 9 deletions lib/grape-swagger/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,19 +222,24 @@ def default_type(params)
end

def parse_request_params(required)
array_key = nil
required.each_with_object({}) do |param, memo|
@array_key = param.first.to_s.gsub('[', '[][') if param.last[:type] == 'Array'
possible_key = param.first.to_s.gsub('[', '[][')
if @array_key && possible_key.start_with?(@array_key)
key = possible_key
param.last[:is_array] = true
else
key = param.first
end
memo[key] = param.last unless (param.last[:type] == 'Hash' || param.last[:type] == 'Array') && !param.last.key?(:documentation)
array_key = param.first.to_s if param_type_is_array?(param.last[:type])

param.last[:is_array] = true if array_key && param.first.start_with?(array_key)
memo[param.first] = param.last unless (param.last[:type] == 'Hash' || param.last[:type] == 'Array') && !param.last.key?(:documentation)
end
end

def param_type_is_array?(param_type)
return false unless param_type
return true if param_type == 'Array'
param_types = param_type.match(/\[(.*)\]$/)
return false unless param_types
param_types = param_types[0].split(',') if param_types
param_types.size == 1
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're parsing this twice, perpetuating the mess we have here and adding to it :) Refactor param_type_is_array into parse_param_type or something like that and return the array key. Nuke the whole possible_key = param.first.to_s.gsub('[', '[][') as well as part of that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dblock … the whole ('[', '[][') junk could be removed by avoiding the introducing of it here endpoint.rb#L224

I had to make a decision, should the name of the parameter indicating if it is a Hash, Array or not, this solution following the rails naming convention for form data

questions is, is it really needed … think removing it, could made many things easier

Copy link
Contributor Author

@TySpring TySpring Jun 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so @LeFnord are you suggesting that the output for nested Arrays in formData would look like address[line1] as opposed to address[][line1] ?

Also @dblock are you saying you want me to remove the method param_type_is_array? altogether, and move the logic into parse_param_type? This seems contradictory to your comment here: #455 (comment).

@LeFnord @dblock Is your general goal just to "clean up the parsing code" without changing logic? Or is it actually to change the output of this parsing logic to remove the added [] for array types?


def expose_params_from_model(model)
model_name = model_name(model)

Expand Down
12 changes: 12 additions & 0 deletions spec/lib/data_type_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,16 @@

it { expect(subject).to eql 'string' }
end

describe '[String]' do
let(:value) { { type: '[String]' } }

it { expect(subject).to eq('string') }
end

describe '[Integer]' do
let(:value) { { type: '[Integer]' } }

it { expect(subject).to eq('integer') }
end
end
13 changes: 13 additions & 0 deletions spec/lib/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,17 @@

describe Grape::Endpoint do
subject { described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get) }

describe '#param_type_is_array?' do
it 'returns true if the value passed represents an array' do
expect(subject.send(:param_type_is_array?, 'Array')).to be_truthy
expect(subject.send(:param_type_is_array?, '[String]')).to be_truthy
expect(subject.send(:param_type_is_array?, 'Array[Integer]')).to be_truthy
end

it 'returns false if the value passed does not represent an array' do
expect(subject.send(:param_type_is_array?, 'String')).to be_falsey
expect(subject.send(:param_type_is_array?, '[String, Integer]')).to be_falsey
end
end
end
2 changes: 1 addition & 1 deletion spec/swagger_v2/api_swagger_v2_detail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def app
specify do
expect(subject['paths']['/use_gfm_rc_detail']['get']).to include('description')
expect(subject['paths']['/use_gfm_rc_detail']['get']['description']).to eql(
"<h1>This returns something</h1>\n\n<p># Burgers in Heaven</p>\n\n<blockquote>\n<p>A burger doesn&#39;t come for free</p>\n</blockquote>\n\n<p>If you want to reserve a burger in heaven, you have to do\nsome crazy stuff on earth.</p>\n<pre class=\"highlight plaintext\"><code>def do_good\nputs 'help people'\nend\n</code></pre>\n<ul>\n<li><em>Will go to Heaven:</em> Probably</li>\n<li><em>Will go to Hell:</em> Probably not</li>\n</ul>"
"<h1>This returns something</h1>\n\n<p># Burgers in Heaven</p>\n\n<blockquote>\n<p>A burger doesn&#39;t come for free</p>\n</blockquote>\n\n<p>If you want to reserve a burger in heaven, you have to do\nsome crazy stuff on earth.</p>\n<pre class=\"highlight plaintext\"><code>def do_good\nputs 'help people'\nend\n</code></pre>\n\n<ul>\n<li><em>Will go to Heaven:</em> Probably</li>\n<li><em>Will go to Hell:</em> Probably not</li>\n</ul>"
)
end
end
Expand Down
64 changes: 58 additions & 6 deletions spec/swagger_v2/params_array_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@ def app
{ 'declared_params' => declared(params) }
end

params do
requires :array_of_string, type: Array[String], documentation: { param_type: 'body', desc: 'nested array of strings' }
requires :array_of_integer, type: Array[Integer], documentation: { param_type: 'body', desc: 'nested array of integers' }
end

post '/array_of_type' do
{ 'declared_params' => declared(params) }
end

params do
requires :array_of_string, type: Array[String]
requires :array_of_integer, type: Array[Integer]
end

post '/array_of_type_in_form' do
{ 'declared_params' => declared(params) }
end

add_swagger_documentation
end
end
Expand All @@ -40,8 +58,8 @@ def app
specify do
expect(subject['paths']['/groups']['post']['parameters']).to eql(
[
{ 'in' => 'formData', 'name' => 'required_group[][required_param_1]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } },
{ 'in' => 'formData', 'name' => 'required_group[][required_param_2]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } }
{ 'in' => 'formData', 'name' => 'required_group[required_param_1]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } },
{ 'in' => 'formData', 'name' => 'required_group[required_param_2]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } }
]
)
end
Expand All @@ -56,10 +74,44 @@ def app
specify do
expect(subject['paths']['/type_given']['post']['parameters']).to eql(
[
{ 'in' => 'formData', 'name' => 'typed_group[][id]', 'description' => 'integer given', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'integer' } },
{ 'in' => 'formData', 'name' => 'typed_group[][name]', 'description' => 'string given', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } },
{ 'in' => 'formData', 'name' => 'typed_group[][email]', 'description' => 'email given', 'required' => false, 'type' => 'array', 'items' => { 'type' => 'string' } },
{ 'in' => 'formData', 'name' => 'typed_group[][others]', 'required' => false, 'type' => 'array', 'items' => { 'type' => 'integer' }, 'enum' => [1, 2, 3] }
{ 'in' => 'formData', 'name' => 'typed_group[id]', 'description' => 'integer given', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'integer' } },
{ 'in' => 'formData', 'name' => 'typed_group[name]', 'description' => 'string given', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } },
{ 'in' => 'formData', 'name' => 'typed_group[email]', 'description' => 'email given', 'required' => false, 'type' => 'array', 'items' => { 'type' => 'string' } },
{ 'in' => 'formData', 'name' => 'typed_group[others]', 'required' => false, 'type' => 'array', 'items' => { 'type' => 'integer' }, 'enum' => [1, 2, 3] }
]
)
end
end

describe 'retrieves the documentation for parameters that are arrays of primitive types' do
subject do
get '/swagger_doc/array_of_type'
JSON.parse(last_response.body)
end

specify do
expect(subject['definitions']['postArrayOfType']['properties']).to eql(
'array_of_string' => {
'type' => 'array', 'items' => { 'type' => 'string' }, 'description' => 'nested array of strings'
},
'array_of_integer' => {
'type' => 'array', 'items' => { 'type' => 'integer' }, 'description' => 'nested array of integers'
}
)
end
end

describe 'retrieves the documentation for typed group parameters' do
subject do
get '/swagger_doc/array_of_type_in_form'
JSON.parse(last_response.body)
end

specify do
expect(subject['paths']['/array_of_type_in_form']['post']['parameters']).to eql(
[
{ 'in' => 'formData', 'name' => 'array_of_string', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true },
{ 'in' => 'formData', 'name' => 'array_of_integer', 'type' => 'array', 'items' => { 'type' => 'integer' }, 'required' => true }
]
)
end
Expand Down
4 changes: 2 additions & 2 deletions spec/swagger_v2/params_nested_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ def app
specify do
expect(subject['paths']['/nested_array']['post']['parameters']).to eql(
[
{ 'in' => 'formData', 'name' => 'a_array[][param_1]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'integer' } },
{ 'in' => 'formData', 'name' => 'a_array[][b_array][][param_2]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } }
{ 'in' => 'formData', 'name' => 'a_array[param_1]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'integer' } },
{ 'in' => 'formData', 'name' => 'a_array[b_array][param_2]', 'required' => true, 'type' => 'array', 'items' => { 'type' => 'string' } }
]
)
end
Expand Down