diff --git a/.gitignore b/.gitignore index c5370f6a..80edba87 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,9 @@ doc # bundler .bundle +#IDEA temp files +.idea + # jeweler generated pkg diff --git a/CHANGELOG.md b/CHANGELOG.md index 37da3029..ca4649c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/grape-swagger/doc_methods/data_type.rb b/lib/grape-swagger/doc_methods/data_type.rb index 5b83a55d..5341600b 100644 --- a/lib/grape-swagger/doc_methods/data_type.rb +++ b/lib/grape-swagger/doc_methods/data_type.rb @@ -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 diff --git a/lib/grape-swagger/doc_methods/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index ecff9145..9c7828ac 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -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 = {}) diff --git a/lib/grape-swagger/doc_methods/parse_params.rb b/lib/grape-swagger/doc_methods/parse_params.rb index 53555c0d..e34edb41 100644 --- a/lib/grape-swagger/doc_methods/parse_params.rb +++ b/lib/grape-swagger/doc_methods/parse_params.rb @@ -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 @@ -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) @@ -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] @@ -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 diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index b1b86068..5b91e1f1 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -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) @@ -262,24 +267,28 @@ 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 } @@ -287,14 +296,8 @@ def expose_params_from_model(model) 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) diff --git a/lib/grape-swagger/model_parsers.rb b/lib/grape-swagger/model_parsers.rb index 6f01d52a..711443ae 100644 --- a/lib/grape-swagger/model_parsers.rb +++ b/lib/grape-swagger/model_parsers.rb @@ -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 diff --git a/spec/support/model_parsers/entity_parser.rb b/spec/support/model_parsers/entity_parser.rb index c6f32209..d69f7a04 100644 --- a/spec/support/model_parsers/entity_parser.rb +++ b/spec/support/model_parsers/entity_parser.rb @@ -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 } diff --git a/spec/support/model_parsers/mock_parser.rb b/spec/support/model_parsers/mock_parser.rb index 91be4c99..fe9708ef 100644 --- a/spec/support/model_parsers/mock_parser.rb +++ b/spec/support/model_parsers/mock_parser.rb @@ -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 diff --git a/spec/support/model_parsers/representable_parser.rb b/spec/support/model_parsers/representable_parser.rb index 8093b3d5..d84816c3 100644 --- a/spec/support/model_parsers/representable_parser.rb +++ b/spec/support/model_parsers/representable_parser.rb @@ -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 @@ -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' }, diff --git a/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb b/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb index 84aaf9c7..4197364c 100644 --- a/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb +++ b/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb @@ -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 @@ -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 diff --git a/spec/swagger_v2/params_array_spec.rb b/spec/swagger_v2/params_array_spec.rb index 4f1f7fb2..b8a1b884 100644 --- a/spec/swagger_v2/params_array_spec.rb +++ b/spec/swagger_v2/params_array_spec.rb @@ -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 @@ -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 @@ -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