Force request body to be an schema object (#922)

* Force request body to be an schema object

* Require "OpenStruct" explicitly in specs

* Update changelog and upgrading documentation

Author: numbata

CHANGELOG.md

@@ -7,7 +7,7 @@
 #### Fixes
 
 * Your contribution here.
-
+* [#922](https://github.com/ruby-grape/grape-swagger/pull/922): Force request body to be an schema object - [@numbata](https://github.com/numbata)
 
 ### 2.0.2 (Februar 2, 2024)
 

UPGRADING.md

@@ -1,5 +1,14 @@
 ## Upgrading Grape-swagger
 
+### Upgrading to >= x.y.z
+
+- Grape-swagger now documents array parameters within an object schema in Swagger. This aligns with grape's JSON structure requirements and ensures the documentation is correct.
+  - Previously, arrays were documented as standalone arrays, which could be incorrect based on grape's expectations.
+  - Check your API documentation and update your code or tests that use the old array format.
+
+  Attention: This update may require you to make changes to ensure your API integrations continue to work correctly.
+  For detailed reasons behind this update, refer to GitHub issue #666.
+
 ### Upgrading to >= 1.5.0
 
 - The names generated for body parameter definitions and their references has changed. It'll now include the HTTP action as well as any path parameters.

lib/grape-swagger/doc_methods/move_params.rb

@@ -18,30 +18,18 @@ def to_definition(path, params, route, definitions)
 
           params_to_move = movable_params(params)
 
-          return (params + correct_array_param(params_to_move)) if should_correct_array?(params_to_move)
-
           params << parent_definition_of_params(params_to_move, path, route)
 
           params
         end
 
         private
 
-        def should_correct_array?(param)
-          param.length == 1 && param.first[:in] == 'body' && param.first[:type] == 'array'
-        end
-
-        def correct_array_param(param)
-          param.first[:schema] = { type: param.first.delete(:type), items: param.first.delete(:items) }
-
-          param
-        end
-
         def parent_definition_of_params(params, path, route)
           definition_name = OperationId.build(route, path)
-          build_definition(definition_name, params)
+          # NOTE: Parent definition is always object
+          @definitions[definition_name] = object_type
           definition = @definitions[definition_name]
-
           move_params_to_new(definition, params)
 
           definition[:description] = route.description if route.try(:description)
@@ -53,6 +41,7 @@ def move_params_to_new(definition, params)
           params, nested_params = params.partition { |x| !x[:name].to_s.include?('[') }
           params.each do |param|
             property = param[:name]
+
             param_properties, param_required = build_properties([param])
             add_properties_to_definition(definition, param_properties, param_required)
             related_nested_params, nested_params = nested_params.partition { |x| x[:name].start_with?("#{property}[") }

lib/grape-swagger/doc_methods/parse_params.rb

@@ -70,7 +70,7 @@ def document_add_extensions(settings)
 
         def document_array_param(value_type, definitions)
           if value_type[:documentation].present?
-            param_type = value_type[:documentation][:param_type]
+            param_type = value_type[:documentation][:param_type] || value_type[:documentation][:in]
             doc_type = value_type[:documentation][:type]
             type = DataType.mapping(doc_type) if doc_type && !DataType.request_primitive?(doc_type)
             collection_format = value_type[:documentation][:collectionFormat]

spec/issues/539_array_post_body_spec.rb

@@ -39,19 +39,35 @@ class ArrayOfElements < Grape::Entity
   let(:definitions) { subject['definitions'] }
 
   specify do
-    expect(parameters).to eql(
+    expect(parameters).to match(
       [
         {
-          'in' => 'body', 'name' => 'elements', 'required' => true, 'schema' => {
-            'type' => 'array', 'items' => { '$ref' => '#/definitions/Element' }
-          }
+          'name' => 'postIssue539',
+          'required' => true,
+          'in' => 'body',
+          'schema' => { '$ref' => '#/definitions/postIssue539' }
         }
       ]
     )
   end
 
   specify do
-    expect(definitions).to eql(
+    expect(definitions).to include(
+      'postIssue539' => {
+        'description' => 'create account',
+        'type' => 'object',
+        'properties' => {
+          'elements' => {
+            'type' => 'array', 'items' => { '$ref' => '#/definitions/Element' }
+          }
+        },
+        'required' => ['elements']
+      }
+    )
+  end
+
+  specify do
+    expect(definitions).to include(
       'Element' => {
         'type' => 'object',
         'properties' => {

spec/issues/542_array_of_type_in_post_body_spec.rb

@@ -25,22 +25,37 @@
   end
 
   let(:parameters) { subject['paths']['/issue_542']['post']['parameters'] }
+  let(:definitions) { subject['definitions'] }
 
   specify do
     expect(parameters).to eql(
       [
         {
           'in' => 'body',
-          'name' => 'logs',
+          'name' => 'postIssue542',
           'required' => true,
           'schema' => {
+            '$ref' => '#/definitions/postIssue542'
+          }
+        }
+      ]
+    )
+  end
+
+  specify do
+    expect(definitions).to include(
+      'postIssue542' => {
+        'type' => 'object',
+        'properties' => {
+          'logs' => {
             'type' => 'array',
             'items' => {
               'type' => 'string'
             }
           }
-        }
-      ]
+        },
+        'required' => ['logs']
+      }
     )
   end
 end

spec/issues/553_align_array_put_post_params_spec.rb

@@ -96,26 +96,38 @@
     describe 'in body' do
       describe 'post request' do
         let(:params) { subject['paths']['/in_body']['post']['parameters'] }
+        let(:definitions) { subject['definitions'] }
 
         specify do
           expect(params).to eql(
             [
               {
                 'in' => 'body',
-                'name' => 'guid',
+                'name' => 'postInBody',
                 'required' => true,
-                'schema' => {
+                'schema' => { '$ref' => '#/definitions/postInBody' }
+              }
+            ]
+          )
+          expect(definitions).to include(
+            'postInBody' => {
+              'description' => 'create foo',
+              'type' => 'object',
+              'properties' => {
+                'guid' => {
                   'type' => 'array',
                   'items' => { 'type' => 'string' }
                 }
-              }
-            ]
+              },
+              'required' => ['guid']
+            }
           )
         end
       end
 
       describe 'put request' do
         let(:params) { subject['paths']['/in_body/{id}']['put']['parameters'] }
+        let(:definitions) { subject['definitions'] }
 
         specify do
           expect(params).to eql(
@@ -128,14 +140,24 @@
               },
               {
                 'in' => 'body',
-                'name' => 'guid',
+                'name' => 'putInBodyId',
                 'required' => true,
-                'schema' => {
+                'schema' => { '$ref' => '#/definitions/putInBodyId' }
+              }
+            ]
+          )
+          expect(definitions).to include(
+            'putInBodyId' => {
+              'description' => 'put specific foo',
+              'type' => 'object',
+              'properties' => {
+                'guid' => {
                   'type' => 'array',
                   'items' => { 'type' => 'string' }
                 }
-              }
-            ]
+              },
+              'required' => ['guid']
+            }
           )
         end
       end

spec/issues/666_array_of_entities_in_request_body_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'definition names' do
+  before :all do
+    module TestDefinition
+      module Entity
+        class Account < Grape::Entity
+          expose :cma, documentation: { type: Integer, desc: 'Company number', param_type: 'body' }
+          expose :name, documentation: { type: String, desc: 'Company Name' }
+          expose :environment, documentation: { type: String, desc: 'Test Environment' }
+          expose :sites, documentation: { type: Integer, desc: 'Amount of sites' }
+          expose :username, documentation: { type: String, desc: 'Username for Dashboard' }
+          expose :password, documentation: { type: String, desc: 'Password for Dashboard' }
+        end
+
+        class Accounts < Grape::Entity
+          expose :accounts, documentation: { type: Entity::Account, is_array: true, param_type: 'body', required: true }
+        end
+      end
+    end
+  end
+
+  let(:app) do
+    Class.new(Grape::API) do
+      namespace :issue_666 do
+        desc 'createTestAccount',
+             params: TestDefinition::Entity::Accounts.documentation
+        post 'create' do
+          present params
+        end
+      end
+
+      add_swagger_documentation format: :json
+    end
+  end
+
+  subject do
+    get '/swagger_doc'
+    JSON.parse(last_response.body)
+  end
+
+  specify do
+    expect(subject['definitions']['postIssue666Create']['type']).to eq('object')
+  end
+end

spec/spec_helper.rb

@@ -4,6 +4,7 @@
 
 MODEL_PARSER = ENV.key?('MODEL_PARSER') ? ENV['MODEL_PARSER'].to_s.downcase.sub('grape-swagger-', '') : 'mock'
 
+require 'ostruct'
 require 'grape'
 require 'grape-swagger'
 

spec/swagger_v2/params_array_spec.rb

@@ -118,8 +118,7 @@
         end
 
         specify do
-          expect(subject['definitions']['postArrayOfType']['type']).to eql 'array'
-          expect(subject['definitions']['postArrayOfType']['items']).to eql(
+          expect(subject['definitions']['postArrayOfType']).to eql(
             'type' => 'object',
             'properties' => {
               'array_of_string' => {