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

Parameter with required and example fails valid-example rule #223

Closed
1 task done
cbautista1002 opened this issue May 22, 2019 · 12 comments · Fixed by #472
Closed
1 task done

Parameter with required and example fails valid-example rule #223

cbautista1002 opened this issue May 22, 2019 · 12 comments · Fixed by #472
Assignees
Labels
enhancement New feature or request t/bug Something isn't working

Comments

@cbautista1002
Copy link

I'm submitting a...

  • bug report

What is the current behavior?

If you have a parameter that has required: true and an example, the valid-example rule fails. See "Other information" for details.

What is the expected behavior?

The valid-example rule executes successfully for a valid parameter example as it understands that required is not a list for parameters but a boolean.

What is the motivation / use case for changing the behavior?

The valid-example rule is very helpful and it should work for parameters

Please tell us about your environment:

$ spectral --version
@stoplight/spectral/3.0.0 linux-x64 node-v12.2.0

Other information

Given this schema

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Swagger Petstore
  description: Swagger Petstore
  license:
    name: MIT
  contact:
    name: Test
servers:
  - url: http://petstore.swagger.io/v1
paths:
  /pets/{petId}:
    get:
      summary: Info for a specific pet
      operationId: showPetById
      description: Get a pet
      tags:
        - pets
      parameters:
        - name: petId
          in: path
          required: true
          description: The id of the pet to retrieve
          schema:
            type: string
          example: "1234567890"
      responses:
        '200':
          description: Expected response to a valid request

Running spectral lint myschema.yml results in:

Adding OpenAPI 3.x functions
OpenAPI 3.x detected
Encountered error when running rule 'valid-example' on node at path '$,paths,/pets/{petId},get,parameters,0':
Error: schema is invalid: data/required should be array
No errors or warnings found!

As an aside, an "error" was found but yet spectral says "No errors or warnings found!" and the return code is 0. Why the discrepancy?

Thanks a lot. Let me know if I can help!

@philsturgeon
Copy link
Contributor

philsturgeon commented May 22, 2019 via email

@cbautista1002
Copy link
Author

Hi @philsturgeon, thanks for responding, but I don't see what's wrong with my example. Do you mind pointing out what's wrong?

I tried another example (changed string to integer)

      parameters:
        - in: path
          name: petId
          required: true
          description: The id of the pet to retrieve
          schema:
            type: integer
          example: 3

and got the same response from spectral.

I also just tried this as a query parameter with an invalid example (and no required) and spectral did not catch it:

      parameters:
        - in: query
          name: petId
          # required: true
          description: The id of the pet to retrieve
          schema:
            type: integer
          example: "3"
Adding OpenAPI 3.x functions
OpenAPI 3.x detected
No errors or warnings found!

@philsturgeon
Copy link
Contributor

philsturgeon commented May 23, 2019

Ah dangit I messed up. Ok, two things, your example of example is completely fine. Sorry for saying otherwise.

The first problem is that there are three different types of example in OpenAPI:

  1. examples next to schema but they're responses only so we can ignore them here
  2. example next to schema like you have here, this is an "OpenAPI Example"
  3. example inside schema which is a "JSON Schema Example"

My pointer is greedy: "$..[?(@.example)]", this will snatch up OpenAPI examples and JSON Schema examples. I think I will need to make valid-openapi-example and valid-schema-example so support 2 and 3, which... sigh

For now you can just hit tab and punt that example inside schema to avoid the problem.

$ spectral lint test.yaml
Adding OpenAPI 3.x functions
OpenAPI 3.x detected
No errors or warnings found!

@cbautista1002
Copy link
Author

The first problem is that there are three different types of example in OpenAPI:

Ah, good to know. I didn't realize the specifics behind these example placements.

For now you can just hit tab and punt that example inside schema to avoid the problem.

Awesome, that works for me!

Thanks @philsturgeon

@chris-miaskowski
Copy link
Contributor

Hi @cbautista1002, @philsturgeon.

Great conversation so far and thanks for reporting! We've made some progress on this but aren't quite done yet - bear with us!

Just wanted to add my two cents here.

The OAS documentation says

The example SHOULD match the specified schema and encoding properties if present.

Note it says "SHOULD" not "MUST". This means that the final solution we'll present should imo return a warning (vs error) if there is a mismatch.

@cbautista1002 FYI you can still use 'required' if you put the example inside the schema. The following example works:

parameters:
        - name: petId
          in: path
          required: true
          description: The id of the pet to retrieve
          schema:
            type: string
            example: "1234567890"

@philsturgeon
Copy link
Contributor

philsturgeon commented Jun 13, 2019

@chris-miaskowski but we aren't building a validator, we are building a linter. We do not need to make rules which are following the spec exactly, in fact many of our opinionated linter rules are not mandated by the spec. That's kinda the whole point of this thing.

For example, we ask people to add descriptions to parameters, which is not required by OpenAPI. It's required by the default OAS3 ruleset we've bundled because it's one of our opinions that people should do that, but people can use a different ruleset based on different opinions that still may or may not be spec-based.

So with that in mind, do you still think this should be downgraded from error to warning? I don't have strong opinions on which is which, I just don't want to make decisions based on some misplaced belief that we are only allowed to have opinions that match the letter of OAS.

@chris-miaskowski
Copy link
Contributor

chris-miaskowski commented Jun 13, 2019

@philsturgeon good point! Not opinionated on that either. My gut feeling is that following standards will pay off. The less disconnects there are in the world the better the world is :)

But yeah we'll see!

PS. And we will one day be able to say (warning: "Silicon Valley" reference) "We are making the world a better place with consistent API specifications"! ;)

@cbautista1002
Copy link
Author

Since we use the example fields for our documentation its important that they're accurate for us. I would prefer that this validation results in an error instead of a warning, if possible.

Or perhaps this can be configurable through some kind of strict flag or something similar?

@philsturgeon
Copy link
Contributor

@chris-miaskowski we 100% follow standards and we do not deviate from them, we will never tell somebody they have to do something which is not allowed by the specification.

Imagine if ESLint or TSLint only ever gave you feedback about syntax errors and never let anyone define anything else. Want to define where a open bracket should go? Meh, not in the spec. 🤷‍♂️

That would make the tool fairly useless. 😅

@cbautista1002 yeah that's what I'm thinking. The way examples are used, if they are flat out lies then that should be considered a big deal. I'd be happy to keep them as errors.

@chris-miaskowski
Copy link
Contributor

Sounds good! Let's keep errors :)

@brianmrock brianmrock added this to the Quacker Sprint milestone Jul 6, 2019
@brianmrock
Copy link

@philsturgeon I have this tagged for next sprint, but if its not needed fro v4.0 release, lets leave it on the backlog.

@philsturgeon
Copy link
Contributor

philsturgeon commented Jul 7, 2019 via email

@marbemac marbemac removed this from the Quacker Sprint milestone Jul 28, 2019
@chris-miaskowski chris-miaskowski added this to the September '19 milestone Aug 13, 2019
@chris-miaskowski chris-miaskowski added t/bug Something isn't working enhancement New feature or request labels Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants