From 597930b486b702546c29477eef73318e0759dc07 Mon Sep 17 00:00:00 2001 From: LeFnord Date: Sun, 10 Jul 2016 02:49:02 +0200 Subject: [PATCH] Fixes required property for request definitions - adds changelog entry --- CHANGELOG.md | 2 + lib/grape-swagger/doc_methods/move_params.rb | 35 +++++++++++++---- spec/lib/move_params_spec.rb | 4 +- ..._swagger_v2_param_type_body_nested_spec.rb | 6 ++- spec/swagger_v2/params_array_spec.rb | 39 +++++++++++++++---- 5 files changed, 67 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 601d8938..1ec3cc96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#470](https://github.com/ruby-grape/grape-swagger/pull/470): Document request definitions inline - [@LeFnord](https://github.com/LeFnord). * [#448](https://github.com/ruby-grape/grape-swagger/pull/448): Header parameters are now prepended to the parameter list - [@anakinj](https://github.com/anakinj). * [#444](https://github.com/ruby-grape/grape-swagger/pull/444): With multi types parameter the first type is use as the documentation type - [@scauglog](https://github.com/scauglog). * [#463](https://github.com/ruby-grape/grape-swagger/pull/463): Added 'hidden' option for parameter to be exclude from generated documentation - [@anakinj](https://github.com/anakinj). @@ -9,6 +10,7 @@ #### Fixes +* [#472](https://github.com/ruby-grape/grape-swagger/pull/472): Fixes required property for request definitions - [@LeFnord](https://github.com/LeFnord). * [#467](https://github.com/ruby-grape/grape-swagger/pull/467): Refactors moving of body params - [@LeFnord](https://github.com/LeFnord). * [#464](https://github.com/ruby-grape/grape-swagger/pull/464): Fixes array params, sets correct type and format for items - [@LeFnord](https://github.com/LeFnord). * [#461](https://github.com/ruby-grape/grape-swagger/pull/461): Fixes issue by adding extensions to definitions. It appeared, if for the given status code, no definition could be found - [@LeFnord](https://github.com/LeFnord). diff --git a/lib/grape-swagger/doc_methods/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index 7a2e1261..88906b16 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -18,6 +18,8 @@ def to_definition(params, route, definitions) params end + private + def parent_definition_of_params(params, route) definition_name = GrapeSwagger::DocMethods::OperationId.manipulate(parse_model(route.path)) referenced_definition = build_definition(definition_name, params, route.request_method.downcase) @@ -56,16 +58,14 @@ def build_nested_properties(params, properties = {}) def recursive_call(properties, property, nested_params) if should_expose_as_array?(nested_params) - properties[property] = { type: 'array', items: { type: 'object', properties: {}, required: [] } } + properties[property] = array_type move_params_to_new(properties[property][:items], nested_params) else - properties[property] = { type: 'object', properties: {}, required: [] } + properties[property] = object_type move_params_to_new(properties[property], nested_params) end end - private - def movable_params(params) to_delete = params.each_with_object([]) { |x, memo| memo << x if deletable?(x) } delete_from(params, to_delete) @@ -78,9 +78,20 @@ def delete_from(params, to_delete) end def add_properties_to_definition(definition, properties, required) - definition[:properties].merge!(properties) - definition[:required] = required - definition.delete(:required) if definition[:required].blank? + if definition.key?(:items) + definition[:items][:properties].merge!(properties) + add_to_required(definition[:items], required) + else + definition[:properties].merge!(properties) + add_to_required(definition, required) + end + end + + def add_to_required(definition, value) + return if value.blank? + + definition[:required] ||= [] + definition[:required].push(*value) end def build_properties(params) @@ -129,11 +140,19 @@ def build_body_parameter(reference, name) def build_definition(name, params, verb = nil) name = "#{verb}#{name}" if verb - @definitions[name] = { type: should_exposed_as(params), properties: {}, required: [] } + @definitions[name] = should_expose_as_array?(params) ? array_type : object_type name end + def array_type + { type: 'array', items: { type: 'object', properties: {} } } + end + + def object_type + { type: 'object', properties: {} } + end + def prepare_nested_types(params) params.each do |param| next unless param[:items] diff --git a/spec/lib/move_params_spec.rb b/spec/lib/move_params_spec.rb index 561d578c..76becf97 100644 --- a/spec/lib/move_params_spec.rb +++ b/spec/lib/move_params_spec.rb @@ -186,7 +186,7 @@ specify do definition = definitions.to_a.first expect(definition.first).to eql 'postFoo' - expect(definition.last).to eql(type: 'object', properties: {}, required: []) + expect(definition.last).to eql(type: 'object', properties: {}) end end @@ -198,7 +198,7 @@ specify do definition = definitions.to_a.first expect(definition.first).to eql 'FooBar' - expect(definition.last).to eql(type: 'object', properties: {}, required: []) + expect(definition.last).to eql(type: 'object', properties: {}) end end end diff --git a/spec/swagger_v2/api_swagger_v2_param_type_body_nested_spec.rb b/spec/swagger_v2/api_swagger_v2_param_type_body_nested_spec.rb index 5eed9266..f988a7d8 100644 --- a/spec/swagger_v2/api_swagger_v2_param_type_body_nested_spec.rb +++ b/spec/swagger_v2/api_swagger_v2_param_type_body_nested_spec.rb @@ -139,7 +139,8 @@ def app 'required' => %w(street postcode city) } } - } + }, + 'required' => %w(name) } }, 'description' => 'post in body with nested parameters' @@ -228,7 +229,8 @@ def app 'country' => { 'type' => 'string', 'description' => 'country' } } } - } + }, + 'required' => %w(name) } }, 'description' => 'put in body with multiple nested parameters' diff --git a/spec/swagger_v2/params_array_spec.rb b/spec/swagger_v2/params_array_spec.rb index 4c1baa2b..460bfefb 100644 --- a/spec/swagger_v2/params_array_spec.rb +++ b/spec/swagger_v2/params_array_spec.rb @@ -40,11 +40,11 @@ def app end params do - requires :array_of_string, type: Array[String], documentation: { param_type: 'body', desc: 'nested array of strings' } - requires :array_of_integer, type: Integer, documentation: { param_type: 'body', desc: 'nested array of integers' } + requires :array_of_string, type: Array[String], documentation: { param_type: 'body', desc: 'array of strings' } + requires :integer_value, type: Integer, documentation: { param_type: 'body', desc: 'integer value' } end - post '/object_of_array_and_type' do + post '/object_and_array' do { 'declared_params' => declared(params) } end @@ -103,12 +103,37 @@ def app specify do expect(subject['definitions']['postArrayOfType']['type']).to eql 'array' - expect(subject['definitions']['postArrayOfType']['properties']).to eql( + expect(subject['definitions']['postArrayOfType']['items']).to eql( + 'type' => 'object', + 'properties' => { + 'array_of_string' => { + 'type' => 'string', 'description' => 'nested array of strings' + }, + 'array_of_integer' => { + 'type' => 'integer', 'format' => 'int32', 'description' => 'nested array of integers' + } + }, + 'required' => %w(array_of_string array_of_integer) + ) + end + end + + describe 'documentation for simple and array parameters' do + subject do + get '/swagger_doc/object_and_array' + JSON.parse(last_response.body) + end + + specify do + expect(subject['definitions']['postObjectAndArray']['type']).to eql 'object' + expect(subject['definitions']['postObjectAndArray']['properties']).to eql( 'array_of_string' => { - 'type' => 'string', 'description' => 'nested array of strings' + 'type' => 'array', 'items' => { + 'type' => 'string', 'description' => 'array of strings' + } }, - 'array_of_integer' => { - 'type' => 'integer', 'format' => 'int32', 'description' => 'nested array of integers' + 'integer_value' => { + 'type' => 'integer', 'format' => 'int32', 'description' => 'integer value' } ) end