Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting required flag provides me some errors when should not #513

Closed
MathRobin opened this issue Jan 13, 2022 · 5 comments
Closed

Linting required flag provides me some errors when should not #513

MathRobin opened this issue Jan 13, 2022 · 5 comments

Comments

@MathRobin
Copy link

I tried to lint an openapi spec file and got some strange errors. Maybe I'm wrong but when i check into samples, i think i'm not.

Steps to reproduce the behavior:

  1. This OpenAPI file extract
...
/log:
    post:
      summary: Add a log to applicative logs
      description: Add a log to applicative logs
      operationId: setLog
      responses:
        '204':
          description: No content
        '401':
          description: You're not authorized. Please provide your token
      requestBody:
        description: ''
        content:
          application/json:
            schema:
              type: object
              properties:
                companyId:
                  required: true
                  type: number
...
  1. Run this command with these arguments...
    npx openapi lint openapi.yml
  2. See error
[1] openapi.yml:166:29 at #/paths/~1log/post/requestBody/content/application~1json/schema/properties/companyId/required

Expected type `array` but got `boolean`.

164 | properties:
165 |   companyId:
166 |     required: true
167 |     type: number
168 |   userId:

Error was generated by the spec rule.

openapi spec: 3.0.0
openapi-cli: 1.0.0-beta.69
node: 14.18.1

Am i wrong ?
Many thanks for your work, it's great to have a tool like this one !

@adamaltman
Copy link
Member

adamaltman commented Jan 14, 2022

This is incorrect:

              type: object
              properties:
                companyId:
                  required: true
                  type: number

Do this instead:

              type: object
              required:
                 - companyId
              properties:
                companyId:
                  type: number

Let's leave this issue open because the problem message is incorrect (or unhelpful).

A message like:

  • required incorrectly defined
  • required needs to be a sibling to properties

I'm not coming up with a good message name off-the-top.

Update: I understand why it says it is an array and not boolean, but it is still defined at the wrong level and that seems like a more important problem.

@MathRobin
Copy link
Author

MathRobin commented Jan 14, 2022

oh ok, thanks for have solved my problem !

The surprising next problem for me is that why it's different between how to define for query parameters and for the body ? Is it normal looking to spec ?

Again, many thanks, that solved my problem !

I wasn't able to find an example like your working solution. Each time i found swagger-like (sorry i know it's a competitor) with required into property, not as a sibling.

Even Insomnia didn't understand required as an array, sibling of properties. But it understands when it's a member. Strange no?

@adamaltman
Copy link
Member

Strange? I'll leave that to the community to decide. Different between parameters and schemas? Yes!

You got it exactly:

parameters:
  filter:
     required: true
     # ...

# schemas set required in a "required" array
schemas:
  adam:
    type: object
    required:
      - filter
    properties:
       filter:
         type: string

@MathRobin
Copy link
Author

I don't know for schemas (I don't use them, I auto generate my openapi file, it was more easy to don't use them). But it's different between query string parameters and body. For the first every param needs a nested required boolean when for the last you need a required array listing fields, as a sibling to list of parameters. Why not use the same way. I'm sure that there a lot of smart people who have work on openapi spec. If they have chosen this way, it's for good reasons

@lornajane
Copy link
Collaborator

Thanks for this, both of you! I don't see that there's actions to take on Redocly CLI so I'm closing the issue.

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

No branches or pull requests

3 participants