Merge pull request #363 from LeFnord/master

some more parameter corrections

Author: peter scholz

.rubocop_todo.yml

@@ -13,7 +13,7 @@ Metrics/AbcSize:
 # Offense count: 1
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 230
+  Max: 233
 
 # Offense count: 6
 Metrics/CyclomaticComplexity:

lib/grape-swagger.rb

@@ -3,7 +3,6 @@
 require 'grape-swagger/endpoint'
 require 'grape-swagger/errors'
 
-# TODO: loading should be simplified by using a better/more clever dependincy structure
 require 'grape-swagger/doc_methods/produces_consumes'
 require 'grape-swagger/doc_methods/data_type'
 require 'grape-swagger/doc_methods/extensions'
@@ -12,6 +11,7 @@
 require 'grape-swagger/doc_methods/path_string'
 require 'grape-swagger/doc_methods/tag_name_description'
 require 'grape-swagger/doc_methods/parse_params'
+# require 'grape-swagger/doc_methods/move_params'
 
 require 'grape-swagger/doc_methods'
 

lib/grape-swagger/doc_methods/data_type.rb

@@ -4,6 +4,7 @@ class DataType
       class << self
         def call(value)
           raw_data_type = value[:type] if value.is_a?(Hash)
+          raw_data_type = value unless value.is_a?(Hash)
           raw_data_type ||= 'string'
           case raw_data_type.to_s
           when 'Boolean', 'Date', 'Integer', 'String', 'Float', 'JSON', 'Array'

lib/grape-swagger/endpoint.rb

@@ -89,8 +89,6 @@ def path_item(routes, options)
         @item, path = GrapeSwagger::DocMethods::PathString.build(route.route_path, options)
         @entity = route.route_entity || route.route_success
 
-        # ... replacing version params through submitted version
-
         method = route.route_method.downcase.to_sym
         request_params = method_object(route, options, path)
 
@@ -124,14 +122,6 @@ def description_object(route, markdown)
       description
     end
 
-    def consumes_object(route, format)
-      method = route.route_method.downcase.to_sym
-      format = route.route_settings[:description][:consumes] if route.route_settings[:description] && route.route_settings[:description][:consumes]
-      mime_types = GrapeSwagger::DocMethods::ProducesConsumes.call(format) if [:post, :put].include?(method)
-
-      mime_types
-    end
-
     def produces_object(route, format)
       mime_types = GrapeSwagger::DocMethods::ProducesConsumes.call(format)
 
@@ -143,13 +133,27 @@ def produces_object(route, format)
       route_mime_types.present? ? route_mime_types : mime_types
     end
 
+    def consumes_object(route, format)
+      method = route.route_method.downcase.to_sym
+      format = route.route_settings[:description][:consumes] if route.route_settings[:description] && route.route_settings[:description][:consumes]
+      mime_types = GrapeSwagger::DocMethods::ProducesConsumes.call(format) if [:post, :put].include?(method)
+
+      mime_types
+    end
+
+    def params_object(route)
+      partition_params(route).map do |param, value|
+        value = { required: false }.merge(value) if value.is_a?(Hash)
+        GrapeSwagger::DocMethods::ParseParams.call(param, value, route)
+      end
+    end
+
     def response_object(route)
-      default_code = default_staus_codes[route.route_method.downcase.to_sym]
+      default_code = default_status_codes[route.route_method.downcase.to_sym]
       default_code[:model] = @entity if @entity
       default_code[:message] = route.route_description || default_code[:message].sub('{item}', @item)
 
       codes = [default_code] + (route.route_http_codes || route.route_failure || [])
-
       codes.map! { |x| x.is_a?(Array) ? { code: x[0], message: x[1], model: x[2] } : x }
 
       codes.each_with_object({}) do |value, memo|
@@ -158,6 +162,8 @@ def response_object(route)
         response_model = @item
         response_model = expose_params_from_model(value[:model]) if value[:model]
 
+        memo[204] = memo.delete(200) if memo.key?(200) && route.route_method == 'DELETE' && value[:model].nil?
+
         next unless !response_model.start_with?('Swagger_doc') &&
                     ((@definitions[response_model] && value[:code].to_s.start_with?('2')) || value[:model])
 
@@ -170,40 +176,32 @@ def response_object(route)
       end
     end
 
-    def default_staus_codes
-      {
-        get: { code: 200, message: 'get {item}(s)' },
-        post: { code: 201, message: 'created {item}' },
-        put: { code: 200, message: 'updated {item}' },
-        patch: { code: 200, message: 'patched {item}' },
-        delete: { code: 200, message: 'deleted {item}' }
-      }
+    def tag_object(route, version)
+      Array(route.route_path.split('{')[0].split('/').reject(&:empty?).delete_if { |i| ((i == route.route_prefix.to_s) || (i == version)) }.first)
     end
 
-    def params_object(route)
-      partition_params(route).map do |param, value|
-        value = { required: false }.merge(value) if value.is_a?(Hash)
-        GrapeSwagger::DocMethods::ParseParams.call(param, value, route)
-      end
-    end
+    private
 
     def partition_params(route)
       declared_params = route.route_settings[:declared_params] if route.route_settings[:declared_params].present?
       required, exposed = route.route_params.partition { |x| x.first.is_a? String }
 
       unless declared_params.nil?
-        required_params = parse_request_params(required)
+        request_params = parse_request_params(required)
       end
 
-      if !exposed.empty? && !@entity
+      if !exposed.empty?
         exposed_params = exposed.each_with_object({}) { |x, memo| memo[x.first] = x.last }
         properties = parse_response_params(exposed_params)
-
-        @definitions[@item] = { properties: properties }
+      else
+        properties = parse_response_params(required)
       end
 
+      key = model_name(@entity || @item)
+      @definitions[key] = { type: 'object', properties: properties } unless properties.empty? || @definitions.key?(key) || (route.route_method == 'DELETE' && !@entity)
+
       return route.route_params if route.route_params && !route.route_settings[:declared_params].present?
-      required_params || {}
+      request_params || {}
     end
 
     def parse_request_params(required)
@@ -237,11 +235,12 @@ def parse_response_params(params)
                             { '$ref' => "#/definitions/#{name}" }
                           end
         else
-
-          data_type = GrapeSwagger::DocMethods::DataType.call(x.last[:documentation] || x.last)
+          documented_type = x.last[:type]
+          documented_type ||= x.last[:documentation][:type] if x.last[:documentation]
+          data_type = GrapeSwagger::DocMethods::DataType.call(documented_type)
 
           if GrapeSwagger::DocMethods::DataType.primitive?(data_type)
-            data = GrapeSwagger::DocMethods::DataType::PRIMITIVE_MAPPINGS[data_type]
+            data = GrapeSwagger::DocMethods::DataType.mapping(data_type)
             memo[x.first] = { type: data.first, format: data.last }
           else
             memo[x.first] = { type: data_type }
@@ -253,9 +252,8 @@ def parse_response_params(params)
     end
 
     def expose_params_from_model(model)
-      model_name = model.respond_to?(:name) ? model.name.demodulize.camelize : model.split('::').last
+      model_name = model_name(model)
 
-      # DONE: has to be adept, to be ready for grape-entity >0.5.0
       # TODO: this should only be a temporary hack ;)
       if GrapeEntity::VERSION =~ /0\.4\.\d/
         parameters = model.exposures ? model.exposures : model.documentation
@@ -271,6 +269,20 @@ def expose_params_from_model(model)
       model_name
     end
 
+    def model_name(name)
+      name.respond_to?(:name) ? name.name.demodulize.camelize : name.split('::').last
+    end
+
+    def default_status_codes
+      {
+        get: { code: 200, message: 'get {item}(s)' },
+        post: { code: 201, message: 'created {item}' },
+        put: { code: 200, message: 'updated {item}' },
+        patch: { code: 200, message: 'patched {item}' },
+        delete: { code: 200, message: 'deleted {item}' }
+      }
+    end
+
     def could_it_be_a_model?(value)
       (
         value[:type].to_s.include?('Entity') || value[:type].to_s.include?('Entities')
@@ -289,9 +301,5 @@ def hidden?(route)
 
       false
     end
-
-    def tag_object(route, version)
-      Array(route.route_path.split('{')[0].split('/').reject(&:empty?).delete_if { |i| ((i == route.route_prefix.to_s) || (i == version)) }.first)
-    end
   end
 end

spec/support/api_swagger_v2_result.rb

@@ -151,14 +151,14 @@ class ApiError < Grape::Entity
             "delete"=>{
               "produces"=>["application/json"],
               "parameters"=>[{"in"=>"path", "name"=>"id", "description"=>nil, "required"=>true, "type"=>"integer", "format"=>"int32"}],
-              "responses"=>{"200"=>{"description"=>"dummy route."}, "401"=>{"description"=>"Unauthorized"}},
+              "responses"=>{"204"=>{"description"=>"dummy route."}, "401"=>{"description"=>"Unauthorized"}},
               "tags"=>["dummy"],
               "operationId"=>"deleteDummyId"
         }}},
         "definitions"=>{
-          "QueryInputElement"=>{"type"=>"object", "properties"=>{"key"=>{"type"=>"string"}, "value"=>{"type"=>"string"}}},
           "QueryInput"=>{"type"=>"object", "properties"=>{"elements"=>{"type"=>"array", "items"=>{"$ref"=>"#/definitions/QueryInputElement"}}}},
-          "Thing"=>{"properties"=>{"id"=>{"type"=>"integer", "format"=>"int32"}, "text"=>{"type"=>"string"}, "links"=>{"type"=>"link"}, "others"=>{"type"=>"text"}}},
+          "QueryInputElement"=>{"type"=>"object", "properties"=>{"key"=>{"type"=>"string"}, "value"=>{"type"=>"string"}}},
+          "Thing"=>{"type"=>"object", "properties"=>{"id"=>{"type"=>"integer", "format"=>"int32"}, "text"=>{"type"=>"string"}, "links"=>{"type"=>"link"}, "others"=>{"type"=>"text"}}},
           "ApiError"=>{"type"=>"object", "properties"=>{"code"=>{"type"=>"integer", "format"=>"int32"}, "message"=>{"type"=>"string"}}},
           "Something"=>{"type"=>"object", "properties"=>{"id"=>{"type"=>"integer", "format"=>"int32"}, "text"=>{"type"=>"string"}, "links"=>{"type"=>"link"}, "others"=>{"type"=>"text"}}}
     }}

spec/swagger_v2/api_swagger_v2_param_type_spec.rb

@@ -50,7 +50,7 @@ class ParamTypeApi < Grape::API
           optional :in_header, type: String, documentation: { in: 'header' }
         end
 
-        get '/defined_param_type' do
+        get '/defined_in' do
           { "declared_params" => declared(params) }
         end
 
@@ -62,19 +62,18 @@ class ParamTypeApi < Grape::API
           optional :in_header, type: String, documentation: { in: 'header' }
         end
 
-        get '/defined_param_type/:in_path' do
+        get '/defined_in/:in_path' do
           { "declared_params" => declared(params) }
         end
 
-        desc 'full set of request param types using `:in`',
-          success: TheApi::Entities::UseResponse
+        desc 'full set of request param types using `:in`'
         params do
           optional :in_path, type: Integer
           optional :in_query, type: String, documentation: { in: 'query' }
           optional :in_header, type: String, documentation: { in: 'header' }
         end
 
-        delete '/defined_param_type/:in_path' do
+        delete '/defined_in/:in_path' do
           { "declared_params" => declared(params) }
         end
 
@@ -109,6 +108,25 @@ def app
     TheApi::ParamTypeApi
   end
 
+  describe 'foo' do
+    subject do
+      get '/swagger_doc'
+      JSON.parse(last_response.body)
+    end
+
+    specify do
+      expect(subject['paths']['/defined_param_type/{in_path}']['delete']['responses']).to eql({
+        "200"=>{"description"=>"full set of request param types", "schema"=>{"$ref"=>"#/definitions/UseResponse"}}
+      })
+    end
+
+    specify do
+      expect(subject['paths']['/defined_in/{in_path}']['delete']['responses']).to eql({
+        "204"=>{"description"=>"full set of request param types using `:in`"}
+      })
+    end
+  end
+
   describe 'defined param types' do
     subject do
       get '/swagger_doc/defined_param_type'
@@ -139,7 +157,37 @@ def app
     end
   end
 
-  describe 'file up-, download' do
+  describe 'defined param types with `:in`' do
+    subject do
+      get '/swagger_doc/defined_in'
+      JSON.parse(last_response.body)
+    end
+
+    specify do
+      expect(subject['paths']['/defined_in']['get']['parameters']).to eql([
+        {"in"=>"query", "name"=>"in_query", "description"=>nil, "required"=>false, "type"=>"string"},
+        {"in"=>"header", "name"=>"in_header", "description"=>nil, "required"=>false, "type"=>"string"},
+      ])
+    end
+
+    specify do
+      expect(subject['paths']['/defined_in/{in_path}']['get']['parameters']).to eql([
+        {"in"=>"path", "name"=>"in_path", "description"=>nil, "required"=>true, "type"=>"integer", "format"=>"int32"},
+        {"in"=>"query", "name"=>"in_query", "description"=>nil, "required"=>false, "type"=>"string"},
+        {"in"=>"header", "name"=>"in_header", "description"=>nil, "required"=>false, "type"=>"string"},
+      ])
+    end
+
+    specify do
+      expect(subject['paths']['/defined_in/{in_path}']['delete']['parameters']).to eql([
+        {"in"=>"path", "name"=>"in_path", "description"=>nil, "required"=>true, "type"=>"integer", "format"=>"int32"},
+        {"in"=>"query", "name"=>"in_query", "description"=>nil, "required"=>false, "type"=>"string"},
+        {"in"=>"header", "name"=>"in_header", "description"=>nil, "required"=>false, "type"=>"string"},
+      ])
+    end
+  end
+
+  describe 'file' do
     describe 'upload' do
       subject do
         get '/swagger_doc/upload'

spec/swagger_v2/api_swagger_v2_response_spec.rb

@@ -149,7 +149,7 @@ def app
                 "400"=>{"description"=>"NotFound", "schema"=>{"$ref"=>"#/definitions/ApiError"}}}
         }}},
         "definitions"=>{
-          "ParamsResponse"=>{"properties"=>{"description"=>{"type"=>"string"}, "$responses"=>{"type"=>"string"}}},
+          "ParamsResponse"=>{"type"=>"object", "properties"=>{"description"=>{"type"=>"string"}, "$responses"=>{"type"=>"string"}}},
           "ApiError"=>{"type"=>"object", "properties"=>{"code"=>{"type"=>"integer", "format"=>"int32"}, "message"=>{"type"=>"string"}}}
       }})
     end

spec/swagger_v2/simple_mounted_api_spec.rb

@@ -113,16 +113,17 @@ def app
               "parameters"=>[{"in"=>"formData", "name"=>"items[]", "description"=>"array of items", "required"=>false, "type"=>"array", "items"=>{"type"=>"string"}}],
               "tags"=>["items"],
               "operationId"=>"postItems",
-              "responses"=>{"201"=>{"description"=>"this takes an array of parameters"}}
+              "responses"=>{"201"=>{"description"=>"this takes an array of parameters", "schema"=>{"$ref"=>"#/definitions/Item"}}}
           }},
           "/custom"=>{
             "get"=>{
               "produces"=>["application/json"],
               "parameters"=>[{"in"=>"formData", "name"=>"custom", "description"=>"array of items", "required"=>false, "type"=>"array", "items"=>{"type"=>"CustomType"}}],
               "tags"=>["custom"],
               "operationId"=>"getCustom",
-              "responses"=>{"200"=>{"description"=>"this uses a custom parameter"}}}
-      }}})
+              "responses"=>{"200"=>{"description"=>"this uses a custom parameter", "schema"=>{"$ref"=>"#/definitions/Custom"}}}}
+      }},
+      "definitions"=>{"Item"=>{"type"=>"object", "properties"=>{"items[]"=>{"type"=>"string"}}}, "Custom"=>{"type"=>"object", "properties"=>{"custom"=>{"type"=>"CustomType"}}}}})
     end
   end
 
@@ -216,7 +217,7 @@ def app
               "parameters"=>[{"in"=>"formData", "name"=>"items[]", "description"=>"array of items", "required"=>false, "type"=>"array", "items"=>{"type"=>"string"}}],
               "tags"=>["items"],
               "operationId"=>"postItems",
-              "responses"=>{"201"=>{"description"=>"this takes an array of parameters"}}}
+              "responses"=>{"201"=>{"description"=>"this takes an array of parameters", "schema"=>{"$ref"=>"#/definitions/Item"}}}}
           }})
       end
     end
@@ -235,7 +236,7 @@ def app
               "parameters"=>[{"in"=>"formData", "name"=>"custom", "description"=>"array of items", "required"=>false, "type"=>"array", "items"=>{"type"=>"CustomType"}}],
               "tags"=>["custom"],
               "operationId"=>"getCustom",
-              "responses"=>{"200"=>{"description"=>"this uses a custom parameter"}}}
+              "responses"=>{"200"=>{"description"=>"this uses a custom parameter", "schema"=>{"$ref"=>"#/definitions/Custom"}}}}
           }})
       end
     end