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

)

.gitignore

@@ -5,6 +5,9 @@ doc
 # bundler
 .bundle
 
+#IDEA temp files
+.idea
+
 # jeweler generated
 pkg
 

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)

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
 

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 = {})

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

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,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)

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

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 }

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
 

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' },

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

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