Set default parameter location based on consumes (#927)

* Set default parameter location based on consumes * fix rubocop warnings and update todos. * fix spec * change rubocop fix * correct PR number in changelog --------- Co-authored-by: Eugene Lim <[email protected]>
ruby-grape · May 10, 2024 · e7856de · e7856de
1 parent 953075a
commit e7856de
Show file tree

Hide file tree

Showing 31 changed files with 265 additions and 36 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2023-05-20 18:23:47 UTC using RuboCop version 1.51.0.
+# on 2024-05-07 07:32:56 UTC using RuboCop version 1.63.4.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -13,17 +13,17 @@ Gemspec/RequiredRubyVersion:
   Exclude:
     - 'grape-swagger.gemspec'
 
-# Offense count: 32
+# Offense count: 33
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
-  Max: 56
+  Max: 59
 
-# Offense count: 30
+# Offense count: 32
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
   Max: 28
 
-# Offense count: 8
+# Offense count: 9
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/PerceivedComplexity:
   Max: 16
@@ -35,6 +35,7 @@ Style/ClassVars:
 
 # Offense count: 1
 # This cop supports unsafe autocorrection (--autocorrect-all).
+# Configuration parameters: AllowedReceivers.
 Style/CollectionCompact:
   Exclude:
     - 'lib/grape-swagger/endpoint.rb'

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 #### Features
 
+* [#927](https://github.com/ruby-grape/grape-swagger/pull/927): Set default parameter location based on consumes - [@spaceraccoon](https://github.com/spaceraccoon)
 * Your contribution here.
 
 #### Fixes

diff --git a/lib/grape-swagger/doc_methods/parse_params.rb b/lib/grape-swagger/doc_methods/parse_params.rb
@@ -4,7 +4,7 @@ module GrapeSwagger
   module DocMethods
     class ParseParams
       class << self
-        def call(param, settings, path, route, definitions)
+        def call(param, settings, path, route, definitions, consumes) # rubocop:disable Metrics/ParameterLists
           method = route.request_method
           additional_documentation = settings.fetch(:documentation, {})
           settings.merge!(additional_documentation)
@@ -14,7 +14,7 @@ def call(param, settings, path, route, definitions)
 
           # required properties
           @parsed_param = {
-            in: param_type(value_type),
+            in: param_type(value_type, consumes),
             name: settings[:full_name] || param
           }
 
@@ -148,14 +148,18 @@ def document_example(settings)
           @parsed_param[:example] = example if example
         end
 
-        def param_type(value_type)
+        def param_type(value_type, consumes)
           param_type = value_type[:param_type] || value_type[:in]
           if value_type[:path].include?("{#{value_type[:param_name]}}")
             'path'
           elsif param_type
             param_type
           elsif %w[POST PUT PATCH].include?(value_type[:method])
-            DataType.request_primitive?(value_type[:data_type]) ? 'formData' : 'body'
+            if consumes.include?('application/x-www-form-urlencoded') || consumes.include?('multipart/form-data')
+              'formData'
+            else
+              'body'
+            end
           else
             'query'
           end

diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb
@@ -119,7 +119,7 @@ def method_object(route, options, path)
       method[:description] = description_object(route)
       method[:produces]    = produces_object(route, options[:produces] || options[:format])
       method[:consumes]    = consumes_object(route, options[:consumes] || options[:format])
-      method[:parameters]  = params_object(route, options, path)
+      method[:parameters]  = params_object(route, options, path, method[:consumes])
       method[:security]    = security_object(route)
       method[:responses]   = response_object(route, options)
       method[:tags]        = route.options.fetch(:tags, tag_object(route, path))
@@ -175,7 +175,7 @@ def consumes_object(route, format)
       GrapeSwagger::DocMethods::ProducesConsumes.call(route.settings.dig(:description, :consumes) || format)
     end
 
-    def params_object(route, options, path)
+    def params_object(route, options, path, consumes)
       parameters = build_request_params(route, options).each_with_object([]) do |(param, value), memo|
         next if hidden_parameter?(value)
 
@@ -187,7 +187,7 @@ def params_object(route, options, path)
         elsif value[:type]
           expose_params(value[:type])
         end
-        memo << GrapeSwagger::DocMethods::ParseParams.call(param, value, path, route, @definitions)
+        memo << GrapeSwagger::DocMethods::ParseParams.call(param, value, path, route, @definitions, consumes)
       end
 
       if GrapeSwagger::DocMethods::MoveParams.can_be_moved?(route.request_method, parameters)

diff --git a/spec/issues/532_allow_custom_format_spec.rb b/spec/issues/532_allow_custom_format_spec.rb
@@ -6,6 +6,10 @@
   let(:app) do
     Class.new(Grape::API) do
       namespace :issue_532 do
+        desc 'issue_532' do
+          consumes ['application/x-www-form-urlencoded']
+        end
+
         params do
           requires :logs, type: String, documentation: { format: 'log' }
           optional :phone_number, type: Integer, documentation: { format: 'phone_number' }

diff --git a/spec/issues/553_align_array_put_post_params_spec.rb b/spec/issues/553_align_array_put_post_params_spec.rb
@@ -6,15 +6,19 @@
   let(:app) do
     Class.new(Grape::API) do
       namespace :in_form_data do
-        desc 'create foo'
+        desc 'create foo' do
+          consumes ['application/x-www-form-urlencoded']
+        end
         params do
           requires :guid, type: Array[String]
         end
         post do
           # your code goes here
         end
 
-        desc 'put specific foo'
+        desc 'put specific foo' do
+          consumes ['application/x-www-form-urlencoded']
+        end
         params do
           requires :id
           requires :guid, type: Array[String]
@@ -25,15 +29,19 @@
       end
 
       namespace :in_body do
-        desc 'create foo'
+        desc 'create foo' do
+          consumes ['application/x-www-form-urlencoded']
+        end
         params do
           requires :guid, type: Array[String], documentation: { param_type: 'body' }
         end
         post do
           # your code goes here
         end
 
-        desc 'put specific foo'
+        desc 'put specific foo' do
+          consumes ['application/x-www-form-urlencoded']
+        end
         params do
           requires :id
           requires :guid, type: Array[String], documentation: { param_type: 'body' }

diff --git a/spec/issues/579_align_put_post_parameters_spec.rb b/spec/issues/579_align_put_post_parameters_spec.rb
@@ -21,6 +21,7 @@ class Spec < Grape::Entity
         namespace :implicit do
           namespace :body_parameter do
             desc 'update spec',
+                 consumes: ['application/x-www-form-urlencoded'],
                  success: BodySpec,
                  params: BodySpec.documentation
             put ':guid' do
@@ -30,6 +31,7 @@ class Spec < Grape::Entity
 
           namespace :form_parameter do
             desc 'update spec',
+                 consumes: ['application/x-www-form-urlencoded'],
                  success: Spec,
                  params: Spec.documentation
             put ':guid' do
@@ -41,6 +43,7 @@ class Spec < Grape::Entity
         namespace :explicit do
           namespace :body_parameter do
             desc 'update spec',
+                 consumes: ['application/x-www-form-urlencoded'],
                  success: BodySpec,
                  params: BodySpec.documentation
             params do
@@ -53,6 +56,7 @@ class Spec < Grape::Entity
 
           namespace :form_parameter do
             desc 'update spec',
+                 consumes: ['application/x-www-form-urlencoded'],
                  success: Spec,
                  params: Spec.documentation
             params do
@@ -68,6 +72,7 @@ class Spec < Grape::Entity
           route_param :guid do
             namespace :body_parameter do
               desc 'update spec',
+                   consumes: ['application/x-www-form-urlencoded'],
                    success: BodySpec,
                    params: BodySpec.documentation
               put do
@@ -77,6 +82,7 @@ class Spec < Grape::Entity
 
             namespace :form_parameter do
               desc 'update spec',
+                   consumes: ['application/x-www-form-urlencoded'],
                    success: Spec,
                    params: Spec.documentation
               put do

diff --git a/spec/issues/650_params_array_spec.rb b/spec/issues/650_params_array_spec.rb
@@ -5,6 +5,9 @@
 describe '#605 Group Params as Array' do
   let(:app) do
     Class.new(Grape::API) do
+      desc 'array_of_range' do
+        consumes ['application/x-www-form-urlencoded']
+      end
       params do
         requires :array_of_range_string, type: [String], values: %w[a b c]
         requires :array_of_range_integer, type: [Integer], values: [1, 2, 3]
@@ -13,6 +16,9 @@
         { 'declared_params' => declared(params) }
       end
 
+      desc 'array_with_default' do
+        consumes ['application/x-www-form-urlencoded']
+      end
       params do
         requires :array_with_default_string, type: [String], default: 'abc'
         requires :array_with_default_integer, type: Array[Integer], default: 123

diff --git a/spec/issues/721_set_default_parameter_location_based_on_consumes_spec.rb b/spec/issues/721_set_default_parameter_location_based_on_consumes_spec.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe '#721 set default parameter location based on consumes' do
+  let(:app) do
+    Class.new(Grape::API) do
+      namespace :issue_721 do
+        desc 'create item' do
+          consumes ['application/json']
+        end
+
+        params do
+          requires :logs, type: String
+          optional :phone_number, type: Integer
+        end
+
+        post do
+          present params
+        end
+
+        desc 'modify item' do
+          consumes ['application/x-www-form-urlencoded']
+        end
+
+        params do
+          requires :id, type: Integer
+          requires :logs, type: String
+          optional :phone_number, type: Integer
+        end
+
+        put ':id' do
+          present params
+        end
+      end
+
+      add_swagger_documentation
+    end
+  end
+
+  subject do
+    get '/swagger_doc'
+    JSON.parse(last_response.body)
+  end
+
+  let(:post_parameters) { subject['paths']['/issue_721']['post']['parameters'] }
+  let(:post_schema) { subject['definitions']['postIssue721'] }
+  let(:put_parameters) { subject['paths']['/issue_721/{id}']['put']['parameters'] }
+
+  specify do
+    expect(post_parameters).to eql(
+      [{ 'in' => 'body', 'name' => 'postIssue721', 'required' => true, 'schema' => { '$ref' => '#/definitions/postIssue721' } }]
+    )
+    expect(post_schema).to eql(
+      { 'description' => 'create item', 'properties' => { 'logs' => { 'type' => 'string' }, 'phone_number' => { 'format' => 'int32', 'type' => 'integer' } }, 'required' => ['logs'], 'type' => 'object' }
+    )
+    puts put_parameters
+    expect(put_parameters).to eql(
+      [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }, { 'in' => 'formData', 'name' => 'logs', 'type' => 'string', 'required' => true }, { 'in' => 'formData', 'name' => 'phone_number', 'type' => 'integer', 'format' => 'int32', 'required' => false }]
+    )
+  end
+end
diff --git a/spec/issues/784_extensions_on_params_spec.rb b/spec/issues/784_extensions_on_params_spec.rb
@@ -6,6 +6,10 @@
   let(:app) do
     Class.new(Grape::API) do
       namespace :issue_784 do
+        desc 'issue_784' do
+          consumes ['application/x-www-form-urlencoded']
+        end
+
         params do
           requires :logs, type: String, documentation: { format: 'log', x: { name: 'Log' } }
           optional :phone_number, type: Integer, documentation: { format: 'phone_number', x: { name: 'PhoneNumber' } }

diff --git a/spec/issues/832_array_hash_float_decimal_spec.rb b/spec/issues/832_array_hash_float_decimal_spec.rb
@@ -6,6 +6,9 @@
   let(:app) do
     Class.new(Grape::API) do
       resource :issue_832 do
+        desc 'issue_832' do
+          consumes ['application/x-www-form-urlencoded']
+        end
         params do
           requires :array_param, type: Array do
             requires :float_param, type: Float

diff --git a/spec/support/model_parsers/entity_parser.rb b/spec/support/model_parsers/entity_parser.rb
@@ -234,7 +234,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
           'post' => {
             'description' => 'This creates Thing.',
             'produces' => ['application/json'],
-            'consumes' => ['application/json'],
+            'consumes' => ['application/x-www-form-urlencoded'],
             'parameters' => [
               { 'in' => 'formData', 'name' => 'text', 'description' => 'Content of something.', 'type' => 'string', 'required' => true },
               { 'in' => 'formData', 'name' => 'links', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true }
@@ -256,7 +256,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
           'put' => {
             'description' => 'This updates Thing.',
             'produces' => ['application/json'],
-            'consumes' => ['application/json'],
+            'consumes' => ['application/x-www-form-urlencoded'],
             'parameters' => [
               { 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true },
               { 'in' => 'formData', 'name' => 'text', 'description' => 'Content of something.', 'type' => 'string', 'required' => false },

diff --git a/spec/support/model_parsers/mock_parser.rb b/spec/support/model_parsers/mock_parser.rb
@@ -242,7 +242,7 @@ class ApiResponse < OpenStruct; end
           'post' => {
             'description' => 'This creates Thing.',
             'produces' => ['application/json'],
-            'consumes' => ['application/json'],
+            'consumes' => ['application/x-www-form-urlencoded'],
             'parameters' => [
               { 'in' => 'formData', 'name' => 'text', 'description' => 'Content of something.', 'type' => 'string', 'required' => true },
               { 'in' => 'formData', 'name' => 'links', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true }
@@ -264,7 +264,7 @@ class ApiResponse < OpenStruct; end
           'put' => {
             'description' => 'This updates Thing.',
             'produces' => ['application/json'],
-            'consumes' => ['application/json'],
+            'consumes' => ['application/x-www-form-urlencoded'],
             'parameters' => [
               { 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true },
               { 'in' => 'formData', 'name' => 'text', 'description' => 'Content of something.', 'type' => 'string', 'required' => false },

diff --git a/spec/support/model_parsers/representable_parser.rb b/spec/support/model_parsers/representable_parser.rb
@@ -306,7 +306,7 @@ class DocumentedHashAndArrayModel < Representable::Decorator
           'post' => {
             'description' => 'This creates Thing.',
             'produces' => ['application/json'],
-            'consumes' => ['application/json'],
+            'consumes' => ['application/x-www-form-urlencoded'],
             'parameters' => [
               { 'in' => 'formData', 'name' => 'text', 'description' => 'Content of something.', 'type' => 'string', 'required' => true },
               { 'in' => 'formData', 'name' => 'links', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true }
@@ -328,7 +328,7 @@ class DocumentedHashAndArrayModel < Representable::Decorator
           'put' => {
             'description' => 'This updates Thing.',
             'produces' => ['application/json'],
-            'consumes' => ['application/json'],
+            'consumes' => ['application/x-www-form-urlencoded'],
             'parameters' => [
               { 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true },
               { 'in' => 'formData', 'name' => 'text', 'description' => 'Content of something.', 'type' => 'string', 'required' => false },