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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move off @apidevtools/openapi-schema #259

Merged
merged 18 commits into from
May 28, 2024

Conversation

darrenyong
Copy link

@darrenyong darrenyong commented May 10, 2024

馃殽 Resolves RM-9343

馃О Changes

The schemas we were validating against hasn't been updated in ages and there's a bug with a specific version where it's validating against descriptions instead of description. This makes it so that adding a valid description is considered invalid.

This is being changed to use our fork of @apidevtools/openapi-schema.

馃К QA & Testing

Provide as much information as you can on how to test what you've done.

@erunion
Copy link
Member

erunion commented May 10, 2024

Not that I want us to have to fork and maintain more open source repos but it might be cleaner for us to fork @apidevtools/openapi-schema to keep it up to date? I'm a bit hesitant to wholesale pull these schemas over here without an easy way to update them, which @apidevtools/openapi-schema has support for with some npm run-script tooling.

Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

cool if you just want to do this tho, let me know and i can set up a fork or if you want me to merge + publish this

@darrenyong
Copy link
Author

opting to fork the repo. closing this out!

@darrenyong darrenyong closed this May 10, 2024
@darrenyong darrenyong reopened this May 13, 2024
@darrenyong darrenyong requested a review from erunion May 13, 2024 17:24
@darrenyong
Copy link
Author

darrenyong commented May 13, 2024

this spec (link to OAS) is failing off this specific jsonschema validation block. everything seems valid though?

package.json Show resolved Hide resolved
@darrenyong
Copy link
Author

@darrenyong
Copy link
Author

tagged you guys @Dashron and @julshotal since Jon and Kanad are OOO for the week!

"@apidevtools/swagger-methods": "^3.0.2",
"@jsdevtools/ono": "^7.1.3",
"@readme/better-ajv-errors": "^1.6.0",
"@readme/json-schema-ref-parser": "^1.2.0",
"@readme/openapi-schemas": "^3.0.1",
Copy link
Author

Choose a reason for hiding this comment

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

if the newest version of @readme/openapi-schemas is 3.1.0, does the dependency version also need to be updated?

Copy link
Member

Choose a reason for hiding this comment

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

not sure i follow

@darrenyong
Copy link
Author

darrenyong commented May 28, 2024

@erunion, some of these tests take a while to run since there are some pretty large OAS files so they're timing out. is this something we need to change via --testTimeout?

edit: one of the tests takes intentionally long (~8.5s) so I put the test timeout to 9s. tested it and none of the tests timeout anymore. let me know if that's the best way to do it!

package.json Outdated Show resolved Hide resolved
@erunion erunion merged commit 9094528 into main May 28, 2024
5 checks passed
@erunion erunion deleted the darren/update-oas3.1-validator branch May 28, 2024 18:24
darrenyong added a commit to readmeio/rdme that referenced this pull request May 28, 2024
| [![PR App][icn]][pr] | 馃殽 Resolves RM-9343 |
| :------------------: | :------------------: |

## 馃О Changes
This bumps `@readme/openapi-parser` to include these changes:
- readmeio/openapi-parser#259
- readmeio/openapi-parser#264

These fix an issue where the existence of the `description` property
under `server.variables` would fail validation.


## 馃拋 Customer Impact
Customers should be able to upload valid OAS 3.1 files again without
error'ing.

## 馃К QA & Testing
The following OAS file should not hit any validation errors:

<details>
<summary>OAS 3.1 file</summary>

```json
{
  "openapi": "3.1.0",
  "info": {
    "version": "1.0",
    "title": "Invalid API"
  },
  "servers": [
    {
      "url": "https://{subdomain}.io",
      "variables": {
        "subdomain": {
          "default": "petstore",
          "description": "Subdomain description"
        }

      }
    }
  ],
  "paths": {
    "/anything": {
      "get": {
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/User-Information"
                  }
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "User-Information": {
        "type": "object",
        "properties": {
          "first": {
            "type": "boolean"
          },
          "last": {
            "type": "boolean"
          }
        }
      }
    }
  }
}
```
</details>


- [Broken on next][next].
- [Working in this PR][pr]!

[next]: https://next.readme.ninja
[pr]: https://readme-pr-PR_NUMBER.readme.ninja
[ui]: https://readme-pr-PR_NUMBER.readme.ninja/ui
[icn]:
https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg

<!-- Uncomment and unescape this if you don't want a PR app! -->
<!-- \[skip preview\] -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants