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

example-value-or-externalValue should only target OAS3 #883

Closed
nulltoken opened this issue Jan 2, 2020 · 5 comments · Fixed by #1059
Closed

example-value-or-externalValue should only target OAS3 #883

nulltoken opened this issue Jan 2, 2020 · 5 comments · Fixed by #1059
Labels
t/bug Something isn't working

Comments

@nulltoken
Copy link
Contributor

nulltoken commented Jan 2, 2020

#882 puts under the light a minor issue: value or externalValue only make sense in the context of OAS3, but the rule example-value-or-externalValue apply to both OAS2 and OAS3.

#882 could be merged without incurring any breaking change. However, maybe should we rename the rule to examples-value-or-externalValue (and update the doco accordingly) to make it clear what it actually targets.

@m-mohr
Copy link
Contributor

m-mohr commented Jan 7, 2020

Does #882 also fix that the rule example-value-or-externalValue does get applied to example(s) properties in actual examples, too? I have an example that contains the properties example(s) and spectral complains about it not having a value or externalValue.
It also seems to get applied to schemas that contain a property example(s), which is not an example itself but defined a field example(s) sent in the response.

Edit:
The JSON path for the examples is $..examples.
After checking the OpenAPI 3 spec, I think the following paths should capture all examples:

  • $..content.*.examples
  • $.components.examples
  • $..parameters.*.examples

Not sure how to combine them, I am new to JSONPath. Thought it would be $..[?(@.content.*),?(@.components),?(@.parameters.*)].examples, but seems to be wrong?!

@nulltoken
Copy link
Contributor Author

It also seems to get applied to schemas that contain a property example(s), which is not an example itself but defined a field example(s) sent in the response.

@m-mohr Good catch! This path seems indeed too broad.

Not sure how to combine them, I am new to JSONPath.

I wonder if they should be combined. That might hinder the maintainability. #799 looks like bringing a capability that should make that easier.

@m-mohr
Copy link
Contributor

m-mohr commented Jan 9, 2020

Oh great, didn't know about #799. I started a PR in #899.

P0lip added a commit that referenced this issue Jan 14, 2020
)

* fix: $..examples is too broad (#883)

* Added examples for headers.

* Fix existing tests

* Added new tests

* A little more fine-grained JSON paths

Co-authored-by: Jakub Rożek <jakub@rozek.tech>
Co-authored-by: Vincenzo Chianese <vincenz.chianese@icloud.com>
@tillig
Copy link
Contributor

tillig commented Feb 13, 2020

There's a bit of a windy path here and I'm curious if the issue I'm seeing is related or something new.

I have an OAS3 doc like this, where the example is on the object in the schema rather than on the request/response. The example-value-or-externalValue in Spectral 5.0.0 catches the example there and complains there's no value... but I think what I have is valid. If I try to put a value property under example then the oas-valid-schema-example rule fails.

I'm thinking the rule should only catch examples (plural) not example (singular).

openapi: 3.0.0
info:
  title: Authentication
  version: 0.0.1
  description: Authentication and credential functionality.
  contact:
    name: My Name
    url: https://my-company.com
servers:
  - url: https://localhost
    description: Local development host
tags:
  - name: users
    description: Management for user accounts and related data.
paths:
  /authentication/v1/users/{id}/password:
    put:
      operationId: put-password-by-id
      summary: Sets the specified user's password.
      description: This operation sets the password for the specified user. The user's current password may be required depending on the user's authorization rights. The new password will be validated against password complexity and history requirements.
      parameters:
        - name: id
          description: The ID of the user for whom the password should be set.
          in: path
          required: true
          schema:
            type: string
          example: "758ce33c-5d7a-4300-bcb9-f26a19a22791"
      requestBody:
        description: An object containing the user's current and desired new passwords.
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/passwordChange'
      responses:
        '204':
          description: The password was successfully changed.
      tags:
        - users
components:
  schemas:
    passwordChange:
      type: object
      description: Information required to change a user's password.
      properties:
        current:
          description: The user's current password.
          type: string
        new:
          description: The user's desired new password.
          type: string
      required:
        - new
      example:
        current: "my-current-password"
        new: "my-new-password"

@m-mohr
Copy link
Contributor

m-mohr commented Feb 13, 2020

This should be fixed with the PRs mentioned above. We just need a new release...

P0lip added a commit that referenced this issue Apr 6, 2020
…1059)

* chore(doc): Slightly enhance xor function description

* chore(doc): Make the xor function example is little less abstract

This example has been taken from the initial `example-value-or-externalValue`
definition in the ruleset. The rule has since been updated but this doco example
hasn't.

Let's make the doco example a litlle less dependent on the concrete rule implementation.
It's sole purpose is to provide a sample usage of the `xor`function.

* fix(ruleset): Make `example-value-or-externalValue` only target oas3

Fix #883

Co-authored-by: Jakub Rożek <jakub@stoplight.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants