Skip to content

Fix incorrect data type linking for request params of entity types #511

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 1 commit into from
Oct 13, 2016
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ doc
# bundler
.bundle

#IDEA temp files
.idea

# jeweler generated
pkg

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

* [#515](https://github.com/ruby-grape/grape-swagger/pull/515): Removes limit on model names - [@LeFnord](https://github.com/LeFnord).
* [#511](https://github.com/ruby-grape/grape-swagger/pull/511): Fix incorrect data type linking for request params of entity types - [@serggl](https://github.com/serggl).
* Your contribution here.

### 0.24.0 (September 23, 2016)
Expand Down
11 changes: 6 additions & 5 deletions lib/grape-swagger/doc_methods/data_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ def parse_multi_type(raw_data_type)
end

def parse_entity_name(model)
if model.respond_to?(:entity_name)
if model.methods(false).include?(:entity_name)
model.entity_name
elsif model.to_s.end_with?('::Entity', '::Entities')
model.to_s.split('::')[-2]
elsif model.respond_to?(:name)
model.name.demodulize.camelize
else
name = model.to_s
entity_parts = name.split('::')
entity_parts.reject! { |p| p == 'Entity' || p == 'Entities' }
entity_parts.join('::')
model.to_s.split('::').last
end
end

Expand Down
10 changes: 9 additions & 1 deletion lib/grape-swagger/doc_methods/move_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,15 @@ def document_as_array(param)
end

def document_as_property(param)
property_keys.each_with_object({}) { |x, memo| memo[x] = param[x] if param[x].present? }
property_keys.each_with_object({}) do |x, memo|
value = param[x]
next if value.blank?
if x == :type && @definitions[value].present?
memo['$ref'] = "#/definitions/#{value}"
else
memo[x] = value
end
end
end

def build_nested_properties(params, properties = {})
Expand Down
17 changes: 10 additions & 7 deletions lib/grape-swagger/doc_methods/parse_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module GrapeSwagger
module DocMethods
class ParseParams
class << self
def call(param, settings, route)
def call(param, settings, route, definitions)
path = route.path
method = route.request_method

Expand All @@ -21,7 +21,7 @@ def call(param, settings, route)
# optional properties
document_description(settings)
document_type_and_format(data_type)
document_array_param(value_type)
document_array_param(value_type, definitions)
document_default_value(settings)
document_range_values(settings)
document_required(settings)
Expand Down Expand Up @@ -60,7 +60,7 @@ def document_type_and_format(data_type)
end
end

def document_array_param(value_type)
def document_array_param(value_type, definitions)
if value_type[:is_array]
if value_type[:documentation].present?
param_type = value_type[:documentation][:param_type]
Expand All @@ -69,10 +69,13 @@ def document_array_param(value_type)
collection_format = value_type[:documentation][:collectionFormat]
end

array_items = {
type: type || @parsed_param[:type],
format: @parsed_param.delete(:format)
}.delete_if { |_, value| value.blank? }
array_items = {}
if definitions[value_type[:data_type]]
array_items['$ref'] = "#/definitions/#{@parsed_param[:type]}"
else
array_items[:type] = type || @parsed_param[:type]
end
array_items[:format] = @parsed_param.delete(:format) if @parsed_param[:format]

@parsed_param[:in] = param_type || 'formData'
@parsed_param[:items] = array_items
Expand Down
43 changes: 23 additions & 20 deletions lib/grape-swagger/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,12 @@ def params_object(route)
parameters = partition_params(route).map do |param, value|
value = { required: false }.merge(value) if value.is_a?(Hash)
_, value = default_type([[param, value]]).first if value == ''
GrapeSwagger::DocMethods::ParseParams.call(param, value, route)
if value[:type]
expose_params(value[:type])
elsif value[:documentation]
expose_params(value[:documentation][:type])
end
GrapeSwagger::DocMethods::ParseParams.call(param, value, route, @definitions)
end

if GrapeSwagger::DocMethods::MoveParams.can_be_moved?(parameters, route.request_method)
Expand Down Expand Up @@ -262,39 +267,37 @@ def param_type_is_array?(param_type)
param_types.size == 1
end

def expose_params(value)
if value.is_a?(Class) && GrapeSwagger.model_parsers.find(value)
expose_params_from_model(value)
elsif value.is_a?(String)
begin
expose_params(Object.const_get(value.gsub(/\[|\]/, ''))) # try to load class from its name
rescue NameError
nil
end
end
end

def expose_params_from_model(model)
model_name = model_name(model)

return model_name if @definitions.key?(model_name)
@definitions[model_name] = nil

properties = nil
parser = nil

GrapeSwagger.model_parsers.each do |klass, ancestor|
next unless model.ancestors.map(&:to_s).include?(ancestor)
parser = klass.new(model, self)
break
end

properties = parser.call unless parser.nil?

parser = GrapeSwagger.model_parsers.find(model)
raise GrapeSwagger::Errors::UnregisteredParser, "No parser registered for #{model_name}." unless parser

properties = parser.new(model, self).call
raise GrapeSwagger::Errors::SwaggerSpec, "Empty model #{model_name}, swagger 2.0 doesn't support empty definitions." unless properties && properties.any?

@definitions[model_name] = { type: 'object', properties: properties }

model_name
end

def model_name(entity)
if entity.methods(false).include?(:entity_name)
entity.entity_name
elsif entity.to_s.end_with?('::Entity', '::Entities')
entity.to_s.split('::')[-2]
else
entity.name.demodulize.camelize
end
def model_name(name)
GrapeSwagger::DocMethods::DataType.parse_entity_name(name)
end

def hidden?(route, options)
Expand Down
7 changes: 7 additions & 0 deletions lib/grape-swagger/model_parsers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,12 @@ def each
yield klass, ancestor
end
end

def find(model)
GrapeSwagger.model_parsers.each do |klass, ancestor|
return klass if model.ancestors.map(&:to_s).include?(ancestor)
end
nil
end
end
end
7 changes: 7 additions & 0 deletions spec/support/model_parsers/entity_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ class ApiError < Grape::Entity
expose :message, documentation: { type: String, desc: 'error message' }
end

module NestedModule
class ApiResponse < Grape::Entity
expose :status, documentation: { type: String }
expose :error, documentation: { type: ::Entities::ApiError }
end
end

class SecondApiError < Grape::Entity
expose :code, documentation: { type: Integer }
expose :severity, documentation: { type: String }
Expand Down
3 changes: 3 additions & 0 deletions spec/support/model_parsers/mock_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class ApiError < OpenStruct; end
class SecondApiError < OpenStruct; end
class RecursiveModel < OpenStruct; end
class DocumentedHashAndArrayModel < OpenStruct; end
module NestedModule
class ApiResponse < OpenStruct; end
end
end
end

Expand Down
11 changes: 10 additions & 1 deletion spec/support/model_parsers/representable_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ class ApiError < Representable::Decorator
property :message, documentation: { type: String, desc: 'error message' }
end

module NestedModule
class ApiResponse < Representable::Decorator
include Representable::JSON

property :status, documentation: { type: String }
property :error, documentation: { type: ::Entities::ApiError }
end
end

class SecondApiError < Representable::Decorator
include Representable::JSON

Expand Down Expand Up @@ -236,7 +245,7 @@ class DocumentedHashAndArrayModel < Representable::Decorator
'prop_file' => { 'description' => 'prop_file description', 'type' => 'file' },
'prop_float' => { 'description' => 'prop_float description', 'type' => 'number', 'format' => 'float' },
'prop_integer' => { 'description' => 'prop_integer description', 'type' => 'integer', 'format' => 'int32' },
'prop_json' => { 'description' => 'prop_json description', 'type' => 'Representable::JSON' },
'prop_json' => { 'description' => 'prop_json description', 'type' => 'JSON' },
'prop_long' => { 'description' => 'prop_long description', 'type' => 'integer', 'format' => 'int64' },
'prop_password' => { 'description' => 'prop_password description', 'type' => 'string', 'format' => 'password' },
'prop_string' => { 'description' => 'prop_string description', 'type' => 'string' },
Expand Down
56 changes: 56 additions & 0 deletions spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ class BodyParamTypeApi < Grape::API
end
end

namespace :with_entity_param do
desc 'put in body with entity parameter'
params do
optional :data, type: ::Entities::NestedModule::ApiResponse, documentation: { desc: 'request data' }
end

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

add_swagger_documentation
end
end
Expand Down Expand Up @@ -156,4 +167,49 @@ def app
)
end
end

describe 'complex entity given' do
let(:request_parameters_definition) do
[
{
'name' => 'WithEntityParam',
'in' => 'body',
'required' => true,
'schema' => {
'$ref' => '#/definitions/postWithEntityParam'
}
}
]
end

let(:request_body_parameters_definition) do
{
'type' => 'object',
'properties' => {
'data' => {
'$ref' => '#/definitions/ApiResponse',
'description' => 'request data'
}
},
'description' => 'put in body with entity parameter'
}
end

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

specify do
expect(subject['paths']['/with_entity_param']['post']['parameters']).to eql(request_parameters_definition)
end

specify do
expect(subject['definitions']['ApiResponse']).not_to be_nil
end

specify do
expect(subject['definitions']['postWithEntityParam']).to eql(request_body_parameters_definition)
end
end
end
36 changes: 36 additions & 0 deletions spec/swagger_v2/params_array_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'spec_helper'

describe 'Group Params as Array' do
include_context "#{MODEL_PARSER} swagger example"

def app
Class.new(Grape::API) do
format :json
Expand Down Expand Up @@ -57,6 +59,14 @@ def app
{ 'declared_params' => declared(params) }
end

params do
requires :array_of_entities, type: Array[Entities::ApiError]
end

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

add_swagger_documentation
end
end
Expand Down Expand Up @@ -156,4 +166,30 @@ def app
)
end
end

describe 'documentation for entity array parameters' do
let(:parameters) do
[
{
'in' => 'formData',
'name' => 'array_of_entities',
'type' => 'array',
'items' => {
'$ref' => '#/definitions/ApiError'
},
'required' => true
}
]
end

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

specify do
expect(subject['definitions']['ApiError']).not_to be_blank
expect(subject['paths']['/array_of_entities']['post']['parameters']).to eql(parameters)
end
end
end