Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

Commit

Permalink
refactor: rewrite the whole parser (AGAIN) (#23)
Browse files Browse the repository at this point in the history
* feat: add new @scalar/resolver package

* chore: clean up

* test: improve test:prepare error message

* refactor: remove resolver package, integrate into the openapi parser (wip)

* fix: references with whitespace

* fix: invalid reference error message

* test: use new invalid reference error message

* test: disable file system tests

* chore: clean up

* chore: try to add some more tests

* chore: add diff testing

* chore: add failing test case

* chore: make the example even simpler

* chore: add some comments

* feat: recursively resolve pointer reference before pushing to object

* fix: pointers with white space can’t be resolved

* chore: disable old test

* fix: can’t access $ref of undefined

* fix: references inside of arrays can’t be found

* chore: disable old test

* chore: test more real-world examples

* fix: original properties are overwritten with the referenced content

* chore: diff testing for more files

* fix: existing properties are overwritten with referenced content

* fix: referenced content loses reference to original object

* fix: throws exception when the pointer does not exist

* fix: $ref is not a string

* chore: add failing max call stack example file (wip)

* refactor: migrate checkReferences method

* chore: rewrite resolve function to use less memory wip

* refactor: rename basic test file

* chore: add another test

* refactor: write resolve function from scratch (wip)

* refactor: write resolve function from scratch (wip)

* refactor: rename findReference to resolveUri

* fix: detect circular references and throw error

* chore: resolve circular refs wip

* fix: resolve circular references

* chore: docstrings and cleanup

* chore: update diff test invalid files list

* chore: improve circular reference test

* chore: rewrite the resolve function from scratch (again)

* refactor: rewrite the resolve method *again* (wip)

* refactor: minor bug fixes

* test: output errors (if they appear)

* chore: clean up

* docs(changeset): refactor: rewrote the whole parser (again)

* docs(changeset): fix: circular references can not be resolved

* docs(changeset): fix: references inside references are not resolved

* docs: update README

* docs: add mentions to the README

* chore: clean up

* fix: ts errors

---------

Co-authored-by: tmastrom <thomas.mastromonaco@gmail.com>
  • Loading branch information
hanspagel and tmastrom authored Mar 7, 2024
1 parent 5fa736f commit c28d766
Show file tree
Hide file tree
Showing 136 changed files with 1,029 additions and 6,789 deletions.
5 changes: 5 additions & 0 deletions .changeset/heavy-tools-peel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@scalar/openapi-parser": minor
---

fix: circular references can not be resolved
5 changes: 5 additions & 0 deletions .changeset/plenty-steaks-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@scalar/openapi-parser": minor
---

fix: references inside references are not resolved
5 changes: 5 additions & 0 deletions .changeset/violet-guests-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@scalar/openapi-parser": minor
---

refactor: rewrote the whole parser (again)
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Modern OpenAPI parser written in TypeScript with support for OpenAPI 3.1, OpenAP
- [x] Written in TypeScript
- [x] Runs in Node.js and in the browser (without any polyfills or configuration)
- [x] Tested with hundreds of real world examples
- [x] Amazing error output
- [ ] Amazing error output
- [ ] Support for OpenAPI 4.0 👀

## Limitations
Expand Down Expand Up @@ -98,6 +98,15 @@ const specification = openapi()

We are API nerds. You too? Let’s chat on Discord: <https://discord.gg/8HeZcRGPFS>

## Thank you!

Thanks a ton for all the help and inspiration:

- [@philsturgeon](https://github.com/philsturgeon) to make sure we build something we won’t hate.
- We took a lot of inspiration from [@seriousme]https://github.com/seriousme and his package [openapi-schema-validator](https://github.com/seriousme/openapi-schema-validator) early-on.
- You could consider this package the modern successor of [@apidevtools/swagger-parser](https://github.com/APIDevTools/swagger-parser), we even test against it to make sure we’re getting the same results (where intended).
- We stole a lot of example specification from [@mermade](https://github.com/mermade) to test against.

## Contributors

<!-- readme: collaborators,contributors -start -->
Expand Down
10 changes: 5 additions & 5 deletions packages/openapi-parser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,21 @@
"test": "vitest"
},
"devDependencies": {
"@apidevtools/swagger-parser": "^10.1.0",
"@modyfi/vite-plugin-yaml": "^1.1.0",
"@types/js-yaml": "^4.0.9",
"glob": "^10.3.10",
"just-diff": "^6.0.2",
"vite": "^5.1.4",
"vite-plugin-dts": "^3.7.3"
},
"dependencies": {
"@humanwhocodes/momoa": "^3.0.1",
"@types/node": "^20.11.24",
"@hyperjump/browser": "^1.1.3",
"@hyperjump/json-schema": "^1.7.2",
"@types/node": "^20.11.20",
"ajv": "^8.12.0",
"ajv-draft-04": "^1.0.0",
"ajv-formats": "^2.1.1",
"glob": "^10.3.10",
"js-yaml": "^4.1.0",
"json-to-ast": "^2.1.0",
"jsonpointer": "^5.0.1",
"leven": "^4.0.0",
"openapi-types": "^12.1.3",
Expand Down
4 changes: 3 additions & 1 deletion packages/openapi-parser/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ export const ERRORS = {
// URI_MUST_BE_STRING: 'uri parameter or $id attribute must be a string',
OPENAPI_VERSION_NOT_SUPPORTED:
'Cannot find supported Swagger/OpenAPI version in specification, version must be a string.',
INVALID_REFERENCE: 'Can’t resolve $ref: %s',
INVALID_REFERENCE: 'Can’t resolve URI: %s',
EXTERNAL_REFERENCE_NOT_SUPPORTED:
'External references are not supported yet: %s',
} as const

export type VALIDATOR_ERROR = keyof typeof ERRORS
Expand Down
6 changes: 4 additions & 2 deletions packages/openapi-parser/src/lib/Validator/Validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ export class Validator {
public specification: AnyObject

resolveReferences(filesystem?: Filesystem) {
return resolveReferences(filesystem, true)
return resolveReferences(
filesystem.find((file) => file.isEntrypoint === true).specification,
)
}

/**
Expand Down Expand Up @@ -89,7 +91,7 @@ export class Validator {

// Check if the references are valid
if (schemaResult) {
return checkReferences(filesystem)
return checkReferences(entrypoint.specification)
}

// Error handling
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { path } from '../../polyfills'
import type { Filesystem, FilesystemEntry } from '../../types'
// @ts-nocheck
import { path } from '../../../polyfills'
import type { Filesystem, FilesystemEntry } from '../../../types'
import { resolveReferences } from './resolveReferences'

export function resolveFromFilesystem(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
// @ts-nocheck
import { describe, expect, it } from 'vitest'

import { getListOfReferences, loadFiles, resolve, validate } from '../../../src'
import { relativePath } from '../../../tests/utils'
import { getListOfReferences, loadFiles, resolve, validate } from '../../..'
import { relativePath } from '../../../../tests/utils'

const EXAMPLE_FILE = relativePath(
import.meta.url,
'../../tests/resolveUri/invalid/openapi.yaml',
'../../../tests/resolveUri/invalid/openapi.yaml',
)

describe('resolveUri', async () => {
describe.skip('resolveUri', async () => {
it('resolves internal references', async () => {
const specification = {
openapi: '3.0.3',
Expand Down Expand Up @@ -48,6 +49,7 @@ describe('resolveUri', async () => {

const result = await resolve(specification)

expect(result.errors).toBe(undefined)
expect(result.valid).toBe(true)
expect(
result.schema?.paths?.['/upload']?.post?.responses?.[401]?.content[
Expand Down Expand Up @@ -78,7 +80,7 @@ describe('resolveUri', async () => {
expect(result.valid).toBe(false)
expect(result.errors?.length).toEqual(1)
expect(result.errors?.[0].error).toEqual(
'Can’t resolve $ref: #/foo/bar/does_not_exist',
'Can’t resolve URI: #/foo/bar/does_not_exist',
)
})

Expand All @@ -90,11 +92,11 @@ describe('resolveUri', async () => {
expect(result.valid).toBe(false)
expect(result.errors.length).toBe(1)
expect(result.errors[0].error).toBe(
`Can’t resolve $ref: schemas/does-not-exist.yaml`,
`Can’t resolve URI: schemas/does-not-exist.yaml`,
)
})

it('resolves file references', async () => {
it.todo('resolves file references', async () => {
const baseFile = {
openapi: '3.0.3',
info: {
Expand Down Expand Up @@ -145,6 +147,7 @@ describe('resolveUri', async () => {
},
])

expect(result.errors).toBe(undefined)
expect(result.valid).toBe(true)
expect(
result.schema?.paths?.['/upload']?.post?.responses?.[401]?.content[
Expand All @@ -153,7 +156,7 @@ describe('resolveUri', async () => {
).toEqual('string')
})

it('resolves file references with pointers', async () => {
it.todo('resolves file references with pointers', async () => {
const baseFile = {
openapi: '3.0.3',
info: {
Expand Down Expand Up @@ -209,6 +212,7 @@ describe('resolveUri', async () => {
},
])

expect(result.errors).toBe(undefined)
expect(result.valid).toBe(true)
expect(
result.schema?.paths?.['/upload']?.post?.responses?.[401]?.content[
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// @ts-nocheck
import { unescapeJsonPointer } from 'ajv/dist/compile/util'

import { ERRORS } from '../../configuration'
import type { AnyObject, Filesystem, FilesystemEntry } from '../../types'
import { ERRORS } from '../../../configuration'
import type { AnyObject, Filesystem, FilesystemEntry } from '../../../types'
import { resolveFromFilesystem } from './resolveFromFilesystem'

/**
Expand Down
106 changes: 65 additions & 41 deletions packages/openapi-parser/src/lib/Validator/checkReferences.test.ts
Original file line number Diff line number Diff line change
@@ -1,57 +1,81 @@
import { describe, expect, it } from 'vitest'

import { makeFilesystem } from '../../utils/makeFilesystem'
import { checkReferences } from './checkReferences'

describe('checkReferences', () => {
it('returns true for a simple internal reference', () => {
const specification = `openapi: 3.1.0
info:
title: Hello World
version: 2.0.0
paths:
const specification = {
openapi: '3.1.0',
info: {
title: 'Hello World',
version: '2.0.0',
},
paths: {
'/foobar': {
post: {
description: 'Example',
requestBody: {
content: {
'application/json': {
schema: {
$ref: '#/components/schemas/Foobar',
},
},
},
},
},
},
},
components: {
schemas: {
Foobar: {
type: 'string',
example: 'Hello World!',
},
},
},
}

'/foobar':
post:
description: 'Example'
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Foobar'
components:
schemas:
Foobar:
type: string
example: 'Hello World!'`

const result = checkReferences(makeFilesystem(specification))
const result = checkReferences(specification)
expect(result.errors).toBe(undefined)
expect(result.valid).toBe(true)
expect(result.errors).toBeUndefined()
})

it('returns false for a broken internal reference', () => {
const specification = `openapi: 3.1.0
info:
title: Hello World
version: 2.0.0
paths:
'/foobar':
post:
description: 'Example'
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Barfoo'
components:
schemas:
Foobar:
type: string
example: 'Hello World!'`
const specification = {
openapi: '3.1.0',
info: {
title: 'Hello World',
version: '2.0.0',
},
paths: {
'/foobar': {
post: {
description: 'Example',
requestBody: {
content: {
'application/json': {
schema: {
$ref: '#/components/schemas/Barfoo',
},
},
},
},
},
},
},
components: {
schemas: {
Foobar: {
type: 'string',
example: 'Hello World!',
},
},
},
}

const result = checkReferences(makeFilesystem(specification))
const result = checkReferences(specification)

expect(result.valid).toBe(false)
expect(result.errors).not.toBeUndefined()
Expand Down
13 changes: 6 additions & 7 deletions packages/openapi-parser/src/lib/Validator/checkReferences.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import type { Filesystem } from '../../types'
import { AnyObject } from '../../types'
import { resolveReferences } from './resolveReferences'
import { transformErrors } from './transformErrors'

export function checkReferences(filesystem: Filesystem) {
const entrypoint = filesystem.find((file) => file.isEntrypoint === true)

// TODO: Adapat for the new function
export function checkReferences(specification: AnyObject) {
try {
resolveReferences(filesystem, false)
resolveReferences(specification)

return {
valid: true,
}
} catch (err) {
} catch (error) {
return {
valid: false,
errors: transformErrors(entrypoint, err.message),
errors: transformErrors(specification, error.message),
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,14 @@ import { describe, expect, it } from 'vitest'

import { escapeJsonPointer } from './escapeJsonPointer'

describe.todo('escapeJsonPointer', async () => {})
describe('escapeJsonPointer', async () => {
it('should escape a slash', () => {
expect(escapeJsonPointer('application/json')).toBe('application~1json')
})

it('should escape multiple slashes', () => {
expect(escapeJsonPointer('/api/users/{id}/reports')).toBe(
'~1api~1users~1{id}~1reports',
)
})
})
1 change: 1 addition & 0 deletions packages/openapi-parser/src/lib/Validator/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './Validator'
export * from './resolveReferences'

This file was deleted.

Loading

0 comments on commit c28d766

Please sign in to comment.