Skip to content

Conversation

@yasuzuki
Copy link

@yasuzuki yasuzuki commented Jun 30, 2022

Current Behaviour

When generating a schema with a response with an empty array, the item property is marked as nullable.

responses:
  '200':
    content:
      application/json:
        schema:
          type: array
          items:
            type: object
            properties:
              id:
                type: integer
              ...
            nullable: true

Desired Behaviour

Should generate schema of typed as an array with the unknown property rather than nullable for the array itself.
Unknown property is expressed as properties: {} based on OAS 3.0.

reference: https://swagger.io/docs/specification/data-models/data-types/#array

When combined with other RSpec examples has a response with an array filled with any items, then generated schema should be merged correctly.

Thank you for this beautiful gem!

yasuzuki and others added 2 commits June 30, 2022 15:22
Co-authored-by: Masanori Kando <m.kando@creativesurvey.com>
@yasuzuki yasuzuki marked this pull request as ready for review July 1, 2022 08:22
Comment on lines +45 to +48
it 'can return an empty array' do
get '/tables', params: { filter: { "name" => "Never Match" } }, headers: { authorization: 'k0kubun' }
expect(response.status).to eq(200)
end
Copy link
Owner

@exoego exoego Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this test case passes on the current master branch without the changes on this PR.
I am afraid that the changes are not covered or probably I do not get the intention of this test.

Copy link
Owner

@exoego exoego Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be clearer if the behavior change is covered in the fixture files (JSON/YAML).

          items:
            type: object
            properties:
              id:
                type: integer
              ...
            nullable: true

===>


          items:
            type: object
            properties:
              id:
                type: integer
              ...

@exoego
Copy link
Owner

exoego commented Aug 24, 2022

@yasuzuki Thanks for proposing this change.
I've tweaked tests in #70 and merged.
Closing as resolved.

@exoego exoego closed this Aug 24, 2022
@yasuzuki yasuzuki deleted the acceptable-for-empty-array branch August 25, 2022 00:07
@yasuzuki
Copy link
Author

@exoego Thank you for taking over our work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants