feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07#1006
Conversation
| }); | ||
|
|
||
| it('should handle schemas that use draft-04', async () => { | ||
| const content = `openapi: "3.0.0" |
There was a problem hiding this comment.
I specifically wanted to test an openapi v3.0.0 schema because that's what is not working in my IDE.
| service.registerCustomSchemaProvider(() => { | ||
| return new Promise<string | string[]>((resolve) => { | ||
| resolve('http://fakeschema.faketld'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
I came up with this to enforce the retrieval of the schema. Not sure if this is the best, feel free to suggest better ways.
| @@ -0,0 +1,1662 @@ | |||
| { | |||
There was a problem hiding this comment.
Maybe having the entire openapi v3.0.0 spec is a bit too much. Let me know if you want me to just use a simple one that uses draft-04.
I used this because it's the use case I wanted to cover (and I think supporting openapi v3.0.0 is very important as there are tools that are unable to upgrade to 3.1).
…s a string instead of parsed json
|
Bump 🥇 |
|
Is there anything I can do to help you all review this one? |
msivasubramaniaan
left a comment
There was a problem hiding this comment.
Please fix lint issues
|
The issue can't be resolved because the bot lint github-advanced-security What can be done to resolve that? It looks like the code that ignored that rule before did not go through the same "external" linting. |
|
I removed support for draft-06 just to avoid the import. I guess if anyone needs support for that will need to deal with the linting issue. |
|
Thanks for the review! |
|
It looks like it is broken again in 1.17.0. |
|
This PR was reverted because some users complained about the validation failing when the This can be verified using the following command: Since this seems to be a source of confusion to users, I guess that a solution could be reached by either not validating the schemas or providing a better error message to convey the above. Reference on the schema identifier actually being right in this PR:
Issues that cause the PR to be reverted: |
This is the correct behaviour. Can you help me understand the decision to revert this PR please? |
|
You know, newer modern tooling which does validation using JSON Schema, often will throw an error in such a situation, and not try to download any schems based on the As such, I'd argue that by reverting this, you're doing a diservice to users, allowing them to believe that their schema is valid and will work in compliant tooling. |
|
The canonical metaschema URI is actually |
From what I understood, it was reverted from some issues raised in the repo. The revert happened here: And it was attempted here: If you follow through both PRs, these are some of the raised issues:
Just for clarification, I didn't take part in the revert, just explaining what I saw from the outside. I think this PR should be reapplied but I don't call the shots on how the repo is managed. @msivasubramaniaan Can you help us here? From what I understand, this is just lack of knowledge (which is understandable) on how the actual schema ids and references work and that there's a difference between where they can be downloaded and how they are referenced.
That's right, I fixed my previous comment. |
This helps to support the schema for openapi 3.0.0, among others, that use an older draft of JSON schema that has some type differences that make it incompatible with JSON schema draft 07. See redhat-developer#1006 , I think the root cause for the issues that PR caused is that we were trying to download and cache the metaschema from the "URL" instead of using the copy that's bundled with `ajv`. Fixes redhat-developer#780, Fixes redhat-developer#752 (and many, many duplicates we'll have to find and clean up) Signed-off-by: David Thompson <davthomp@redhat.com>
This helps to support the schema for openapi 3.0.0, among others, that use an older draft of JSON schema that has some type differences that make it incompatible with JSON schema draft 07. See redhat-developer#1006 , I think the root cause for the issues that PR caused is that we were trying to download and cache the metaschema from the "URL" instead of using the copy that's bundled with `ajv`. Fixes redhat-developer#780, Fixes redhat-developer#752 (and many, many duplicates we'll have to find and clean up) Signed-off-by: David Thompson <davthomp@redhat.com>
This is not working and I need to touch grass.
How am I testing this? In L27, replace b0 for t0.
- &fg-dim-1 !alpha [*fg-base, tt]
etc. for other patterns e.g., short hex colors.
Some relevant docs:
https://json-schema.org/learn/getting-started-step-by-step
https://github.com/redhat-developer/vscode-yaml?tab=readme-ov-file#associating-a-schema-in-the-yaml-file
https://json-schema.org/understanding-json-schema/reference/combining
https://json-schema.org/understanding-json-schema/reference/array
https://json-schema.org/understanding-json-schema/structuring#defs
https://json-schema.org/understanding-json-schema/reference/string
https://json-schema.org/understanding-json-schema/reference/numeric#integer
Perhaps the "#defs" part isn't working? Try to inline it and/or refactor
it out to another file.
Might also be that I should be using oneOf, rather than anyOf. But this
shouldn't be the breaking part.
My current research re: the runner/executor of this schema validation:
https://github.com/Microsoft/vscode/blob/main/extensions/json-language-features/server/README.md#participate
https://github.com/microsoft/vscode-json-languageservice/blob/main/CHANGELOG.md?plain=1#L30
- how can I configure which version of the schema I'm targeting tho?
https://github.com/redhat-developer/vscode-yaml/releases
https://github.com/redhat-developer/vscode-yaml/releases/tag/1.16.0
- how do I target these drafts? how can I know which one's in use?
redhat-developer/yaml-language-server#1006
redhat-developer/yaml-language-server#780
https://github.com/redhat-developer/yaml-language-server?tab=readme-ov-file#language-server-settings
https://github.com/redhat-developer/yaml-language-server?tab=readme-ov-file#clients
https://github.com/redhat-developer/vscode-yaml?tab=readme-ov-file#associating-schemas
- yup, vscode-yaml extension only ships support for JSON Schema Draft 7
- what does that mean? which JSON Schema features are/not supported?
- how much work would it take to merge the LSP support of different
drafts into the extension?
I could also try converting it back to .jsonc or even .json to see if
that provides any clarity to the whole issue.
It's unresolvable bc its an issue in redhat yaml lang server redhat-developer/yaml-language-server#1006
This helps to support the schema for openapi 3.0.0, among others, that use an older draft of JSON schema that has some type differences that make it incompatible with JSON schema draft 07. See redhat-developer#1006 , I think the root cause for the issues that PR caused is that we were trying to download and cache the metaschema from the "URL" instead of using the copy that's bundled with `ajv`. Fixes redhat-developer#780, Fixes redhat-developer#752 (and many, many duplicates we'll have to find and clean up) Signed-off-by: David Thompson <davthomp@redhat.com>
This helps to support the schema for openapi 3.0.0, among others, that use an older draft of JSON schema that has some type differences that make it incompatible with JSON schema draft 07. See redhat-developer#1006 , I think the root cause for the issues that PR caused is that we were trying to download and cache the metaschema from the "URL" instead of using the copy that's bundled with `ajv`. Fixes redhat-developer#780, Fixes redhat-developer#752 (and many, many duplicates we'll have to find and clean up) Signed-off-by: David Thompson <davthomp@redhat.com>
This helps to support the schema for openapi 3.0.0, among others, that use an older draft of JSON schema that has some type differences that make it incompatible with JSON schema draft 07. See redhat-developer#1006 , I think the root cause for the issues that PR caused is that we were trying to download and cache the metaschema from the "URL" instead of using the copy that's bundled with `ajv`. Fixes redhat-developer#780, Fixes redhat-developer#752 (and many, many duplicates we'll have to find and clean up) Signed-off-by: David Thompson <davthomp@redhat.com>
This helps to support the schema for openapi 3.0.0, among others, that use an older draft of JSON schema that has some type differences that make it incompatible with JSON schema draft 07. See #1006 , I think the root cause for the issues that PR caused is that we were trying to download and cache the metaschema from the "URL" instead of using the copy that's bundled with `ajv`. Fixes #780, Fixes #752 (and many, many duplicates we'll have to find and clean up) Signed-off-by: David Thompson <davthomp@redhat.com>
|
Hi, this PR seemed to support for draft 04, 2019 and 2020 but was eventually revert due to issue with draft 04 (if I understand correctly). If there were no particular issues with 2019 and 2020, cant we add them back ? If it's just about adding back validators for 2019 and 2020, I dont mind creating the PR myself, but I d like to know first what was the main issue with them (if any). |
|
I wanted to focus on getting draft-04 working, because that's what a lot of openapi documents were using (we had many bugs filed specifically about openapi documents). The process for supporting 2019 and 2020 should be similar to what I did for draft-04; I think that ajv has validators for them. The implementation used in the older PR that got reverted was causing problems. If you're interested in implementing the support, I can review it. |
|
ajv doesn't fully support draft-2019 or draft-2020-12 because they don't support $dynamic* keywords. Using ajv to validate OpenAPI definitions is very brittle because OpenAPI schemas are a superset and subset of draft-04. Not all draft-04 features are supported and some keywords have slight behavioral differences. ajv-validator/ajv#1573 there's another implementation of all JSON Schema drafts, including OpenAPI versions at www.github.com/hyperjump-io/json-schema |
* checked const value for the boolean type
* test case added
* added positive test case as well
* Fix corner case for 'flow sequence forbidden' quickfix
- Do not include any trailing spaces in the error range
- This is a change in behaviour; it seems this behaviour was
intentional based on the test cases,
but I don't agree with it, since the error range should only cover
the erroneous code.
- Fix a bug where `]` or `}` are not included in the error range if they're the last character in the document
Fixes redhat-developer#1060
Signed-off-by: David Thompson <davthomp@redhat.com>
* added dockercompose and github actions in setting handler
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* Fix YAML JSON Schema files starting with %YAML 1.x
Fix a bug where YAML files containing a JSON Schema that start with
%YAML 1.x or a comment that contains a number would fail to to be parsed.
This is caused by the fact that the JSON Schema parser would contain the
number, e.g. 1.2 as the value of `unresolvedJsonSchema.schema` and so
the test for `=== undefined` would fail. This would cause the error from
the JSON parser to be returned instead of trying to parse the file as a
YAML file.
To work around this, also check if `unresolvedJsonSchema.schema` is a
number. It if is, it clearly was not a JSON Schema and so we can safely
try to parse the file as a YAML file.
Fixes: redhat-developer#922
Signed-off-by: David Lechner <dlechner@baylibre.com>
* show the matching value on top
* fix eslint issue
* removed enumMarkdown entries
* Fix bool enums (redhat-developer#1080)
* add (failing) test case for boolean enum and const values
failing with:
```
[
{
code: 1,
data: {
schemaUri: [ 'file:///default_schema_id.yaml' ],
values: [ true, false ]
},
message: 'Value is not accepted. Valid values: true, false.',
range: {
end: { character: 15, line: 0 },
start: { character: 11, line: 0 }
},
severity: 1,
source: 'yaml-schema: file:///default_schema_id.yaml'
}
]
```
* fix the issue for `boolean` values in `enum`
this is the same fix for `enum` that has been applied a while back for `const` values here: redhat-developer@e6165e4
fixes issue: redhat-developer#1078
---------
Co-authored-by: xxx <xxx>
* Support draft-04 schemas (redhat-developer#1065)
This helps to support the schema for openapi 3.0.0,
among others, that use an older draft of JSON schema that has some type
differences that make it incompatible with JSON schema draft 07.
See redhat-developer#1006 , I think the root cause for the issues that PR caused
is that we were trying to download and cache the metaschema from
the "URL" instead of using the copy that's bundled with `ajv`.
Fixes redhat-developer#780, Fixes redhat-developer#752
(and many, many duplicates we'll have to find and clean up)
Signed-off-by: David Thompson <davthomp@redhat.com>
* Fix recursion bug in ast-conversion (redhat-developer#1076)
* Ensure recursion works as expected
* fix test safety for when parse fails
* use a cache to keep track of resolved aliases
* Added l10n bundle for various language and translate the local strings (redhat-developer#1086)
* added l10n bundle for various language and translate the local strings
* upstream node
* upstream node
* update to node 18
* Changed NodeJS.Timeout as per node 18
* upstream to node 20
* updated coversall script
* fixed test case
* override json service translation
* fixed test cases
* fix/improve enum value descriptions for merged enum lists (redhat-developer#1085)
This is a follow-up submission for redhat-developer#1028, which removes duplicate enum values in merged enum lists.
This fix/improvement ensures that the first **non-empty** enum description is used for the corresponding enum value.
Before this fix: if the first non-unique enum value was not providing a description but the second one was, it was not
picked up and the enum value description stayed empty.
Co-authored-by: xxx <xxx>
Co-authored-by: Muthurajan Sivasubramanian <93245779+msivasubramaniaan@users.noreply.github.com>
* added test cases for bundle and configuration through testhelper class
* Fix array of const completion
* Fix ignoreScalar in value completion
* apply patch
* Updated changelog for the release 1.19.0 (redhat-developer#1094)
* Updated changelog for the release 1.19.0
* updated chhangelog.md
* addressed review comments
* @vscode/l10n is a runtime dependency (redhat-developer#1096)
* check NODE_AUTH_TOKEN is available or not (redhat-developer#1098)
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* Fix auth on npm publish (redhat-developer#1099)
* check NODE_AUTH_TOKEN is available or not
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* set NODE_AUTH_TOKEN
---------
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* Fix auth on npm publish (redhat-developer#1100)
* check NODE_AUTH_TOKEN is available or not
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* set NODE_AUTH_TOKEN
* set npmAuthToken $NODE_AUTH_TOKEN
* set npmAuthToken $NODE_AUTH_TOKEN
---------
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* Update CI.yaml (redhat-developer#1101)
just set token to 123 and check what error occur
* Revert "Update CI.yaml (redhat-developer#1101)" (redhat-developer#1102)
This reverts commit 8c4c22f.
* Fix auth on npm publish (redhat-developer#1103)
* check NODE_AUTH_TOKEN is available or not
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* set NODE_AUTH_TOKEN
* set npmAuthToken $NODE_AUTH_TOKEN
* set npmAuthToken $NODE_AUTH_TOKEN
* all the required parameters are set
---------
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
* Fix the fallback path to the l10n folder (redhat-developer#1104)
If the language server client doesn't specify the location of the l10n
folder, then the language server falls back to a path relative to the
complied file.
This path was wrong; `server.js` is in `./out/server/src/server.js`,
so the relative path of the `l10n` folder is `../../../l10n` instead of
`../l10n`.
Signed-off-by: David Thompson <davthomp@redhat.com>
* Fix the fallback path to the l10n folder (redhat-developer#1104) (redhat-developer#1105)
If the language server client doesn't specify the location of the l10n
folder, then the language server falls back to a path relative to the
complied file.
This path was wrong; `server.js` is in `./out/server/src/server.js`,
so the relative path of the `l10n` folder is `../../../l10n` instead of
`../l10n`.
Signed-off-by: David Thompson <davthomp@redhat.com>
Co-authored-by: David Thompson <davidethompson@me.com>
* Fix NPM broken (redhat-developer#1095)
* set always-auth true
* added registry url and release ci on main push for testing
* removed main push
* renamed to NPM_TOKEN
* used YARN_NPM_AUTH_TOKEN
* used npm_token
* used YARN_NPM_AUTH_TOKEN
* added registry-url
* tried NODE_AUTH_TOKEN
* created .npmrc file
* removed .npmrc
* check NODE_AUTH_TOKEN available or not
* moved the check code on top
* checked NPM_TOKEN
* tried with NODE_AUTH_TOKEN
* Try with NPM instead of YARN
* switch to npm
* removed install command
* fix build issue
* fix build issue
* done pkg fix
* Completely moved to npm from yarn
* review comments addressed
* Bump serialize-javascript and mocha
Bumps [serialize-javascript](https://github.com/yahoo/serialize-javascript) to 6.0.2 and updates ancestor dependency [mocha](https://github.com/mochajs/mocha). These dependencies need to be updated together.
Updates `serialize-javascript` from 6.0.0 to 6.0.2
- [Release notes](https://github.com/yahoo/serialize-javascript/releases)
- [Commits](yahoo/serialize-javascript@v6.0.0...v6.0.2)
Updates `mocha` from 9.2.2 to 11.7.1
- [Release notes](https://github.com/mochajs/mocha/releases)
- [Changelog](https://github.com/mochajs/mocha/blob/main/CHANGELOG.md)
- [Commits](mochajs/mocha@v9.2.2...v11.7.1)
---
updated-dependencies:
- dependency-name: serialize-javascript
dependency-version: 6.0.2
dependency-type: indirect
- dependency-name: mocha
dependency-version: 11.7.1
dependency-type: direct:development
...
Signed-off-by: dependabot[bot] <support@github.com>
* Remove coveralls dependency (redhat-developer#1107)
From my understanding, I think we just need the GitHub Action in order
for things to work.
Signed-off-by: David Thompson <davthomp@redhat.com>
* set .npmrc (redhat-developer#1109)
* add encoding dependency
* upstream @vsode/l10n dependency
* updated changelog.md (redhat-developer#1111)
* Fixed broken link to JSON schema website
* Fix release GitHub Action
- setup-node@v5 creates a .npmrc if you provide a registry,
so there should be no need to do that separately
- after setup-node@v5 is run, `npm publish` expects the token to be
accessible from the environment variable `NODE_AUTH_TOKEN`
- also fix coveralls, because that's getting annoying
(root cause is that it tried to publish the results for each platform
instead of just once)
See https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#publish-to-npmjs-and-gpr-with-npm
Signed-off-by: David Thompson <davthomp@redhat.com>
* Fix prerelease publishing
The options that were being used in `npm publish` to modify the version
to include the git hash at the end are intended to be used
with the `npm version` command beforehand instead.
I think this got messed up somewhere along the way while rewriting the
GitHub Action.
Fixes redhat-developer#1128
Signed-off-by: David Thompson <davthomp@redhat.com>
* Upversion to 1.19.1
We burned the 1.19.0 release by publishing a prerelease version labelled
1.19.0. In order to release, we'll need to use a different version
number.
Signed-off-by: David Thompson <davthomp@redhat.com>
* Upversion to 1.20.0
* remove \n in README (redhat-developer#1068)
fix typo
Signed-off-by: roc <roc@imroc.cc>
Co-authored-by: David Lechner <david@lechnology.com>
* Wrapping code action title with string (redhat-developer#1117)
* Wrapping code action title with string
* adding textedit value field wrapped as string
* README.md: Mention kate as a Client
The `kate` editor has `yaml-language-server` as its default for the YAML format built-in (no plugins or manual configuration needed).
* Support `yaml-textmate` and `yaml-tmlanguage` languages
* redhat-developer/vscode-yaml#1093
* fix: cast cleanupInterval to any before clearing interval
* fix: remove max-warnings option from lint scripts for better flexibility
* Remove @microsoft/eslint-formatter-sarif from devDependencies in package.json
---------
Signed-off-by: David Thompson <davthomp@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: roc <roc@imroc.cc>
Co-authored-by: msivasubramaniaan <msivasub@redhat.com>
Co-authored-by: David Thompson <davthomp@redhat.com>
Co-authored-by: David Lechner <dlechner@baylibre.com>
Co-authored-by: Kosta <2526664+Kosta-Github@users.noreply.github.com>
Co-authored-by: David Thompson <davidethompson@me.com>
Co-authored-by: pjsk-stripe <55514208+pjsk-stripe@users.noreply.github.com>
Co-authored-by: Muthurajan Sivasubramanian <93245779+msivasubramaniaan@users.noreply.github.com>
Co-authored-by: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com>
Co-authored-by: Daniel M. Capella <polyzen@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: debbie <148627186+webdevred@users.noreply.github.com>
Co-authored-by: roc <roc@imroc.cc>
Co-authored-by: David Lechner <david@lechnology.com>
Co-authored-by: Arunvenmany <arun.kumar.v.n@ibm.com>
Co-authored-by: Niels Thykier <niels@thykier.net>
Co-authored-by: RedCMD <33529441+RedCMD@users.noreply.github.com>
What does this PR do?
Adds support for draft-04, draft-2019-09 and draft-2020-12 json schemas. I first started adding support for draft-04 but some of the changes actually make the code validate the schemas more strictly so I felt the need to add the rest of the drafts. I am not entirely sure of the implications of adding the other drafts which are more complex so I would need some help there testing.
The use of avj-draft-04 is documented here: https://ajv.js.org/json-schema.html#draft-04 so it should not be deprecated or out of support.
According to https://ajv.js.org/api.html#ajv-validateschema-schema-object-boolean using that method should be preferred but I am not sure of the consequences of it.
Citing to avoid having to click there:
What issues does this PR fix or reference?
Is it tested? How?
I added a unit test but I would like to test it in my IDE. I have no idea how to do that so help is more than welcome.