feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07 #1006

Davidonium · 2024-12-26T18:06:26Z

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:

Validates schema. This method should be used to validate schemas rather than validate due to the inconsistency of uri format in JSON Schema standard.

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.

test/yamlLanguageService.test.ts

test/yamlSchemaService.test.ts

Davidonium · 2024-12-26T18:09:33Z

test/yamlSchemaService.test.ts

@@ -67,6 +67,50 @@ describe('YAML Schema Service', () => {
      expect(schema.schema.type).eqls('array');
    });

+    it('should handle schemas that use draft-04', async () => {
+      const content = `openapi: "3.0.0"


I specifically wanted to test an openapi v3.0.0 schema because that's what is not working in my IDE.

Davidonium · 2024-12-26T18:10:11Z

test/yamlSchemaService.test.ts

+      service.registerCustomSchemaProvider(() => {
+        return new Promise<string | string[]>((resolve) => {
+          resolve('http://fakeschema.faketld');
+        });
+      });


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.

Davidonium · 2024-12-26T18:12:35Z

test/fixtures/sample-openapiv3.0.0-schema.json

@@ -0,0 +1,1662 @@
+{


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

HampusHauffman · 2025-01-13T22:37:51Z

Bump 🥇

Davidonium · 2025-01-18T22:18:31Z

Is there anything I can do to help you all review this one?

src/languageservice/services/yamlSchemaService.ts

coveralls · 2025-01-21T13:22:22Z

coverage: 84.24% (-0.08%) from 84.321%
when pulling adb309f on Davidonium:add-support-for-draft-04-schemas
into 8a38d41 on redhat-developer:main.

msivasubramaniaan

Please fix lint issues

Davidonium · 2025-01-22T06:35:07Z

The issue can't be resolved because the bot lint github-advanced-security
is ignoring the rule ignore just above. Running yarn lint-ci is fine locally but that linter is misconfigured.
I tried using es imports and addin json resolution but the umd build then fails because it's not supported.

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.

Davidonium · 2025-01-22T22:58:08Z

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.

msivasubramaniaan

LGTM

Davidonium · 2025-02-06T13:11:58Z

Thanks for the review!

lantw44 · 2025-03-10T00:35:13Z

It looks like it is broken again in 1.17.0.

Davidonium · 2025-03-12T20:26:04Z

This PR was reverted because some users complained about the validation failing when the $schema field starts with https.
Validation failing is techincally correct, as https://json-schema.org/draft-07/schema is a correct URL but not a correct schema identifier. The correct schema identifier for draft-07 is http://json-schema.org/draft-07/schema#.

This can be verified using the following command:

$ curl -fsSL https://json-schema.org/draft-07/schema | jq -r '.["$id"]'
http://json-schema.org/draft-07/schema#

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:

Relequestual · 2025-03-14T14:12:55Z

This PR was reverted because some users complained about the validation failing when the $schema field starts with https. Validation failing is techincally correct, as https://json-schema.org/draft-07/schema is a correct URL but not a correct schema identifier. The correct schema identifier for draft-07 is http://json-schema.org/draft-07/schema.

This is the correct behaviour. Can you help me understand the decision to revert this PR please?

Relequestual · 2025-03-14T14:48:09Z

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 $schema value. Unless they are the meta-schemas, they usually have to be pre-loaded.

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.

karenetheridge · 2025-03-14T18:12:00Z

The canonical metaschema URI is actually http://json-schema.org/draft-07/schema# (with the #).

Davidonium · 2025-03-15T15:00:47Z

This is the correct behaviour. Can you help me understand the decision to revert this PR please?

From what I understood, it was reverted from some issues raised in the repo.

The revert happened here:

revert-ajv-draft04 as it not support HTTPS #1026

And it was attempted here:

reverted aja-draft04 npm as it not works on validating the schema on HTTPS #1025

If you follow through both PRs, these are some of the raised issues:

After upgrade to version 1.16.0 validation stop working vscode-yaml#1100
Request textDocument/hover failed with message: no schema with key or ref "https://json-schema.org/draft-07/schema" #1023
[BUG] Schema using https://json-schema.org/draft-07/schema# with v1.16.0 vscode-yaml#1096

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.

The canonical metaschema URI is actually http://json-schema.org/draft-07/schema# (with the #).

That's right, I fixed my previous comment.

feat: add support for draft-04 json schemas

5704546

Davidonium requested a review from msivasubramaniaan as a code owner December 26, 2024 18:06

Davidonium commented Dec 26, 2024

View reviewed changes

test/yamlLanguageService.test.ts Outdated Show resolved Hide resolved

Davidonium commented Dec 26, 2024

View reviewed changes

test/yamlSchemaService.test.ts Outdated Show resolved Hide resolved

Davidonium commented Dec 26, 2024

View reviewed changes

Davidonium mentioned this pull request Dec 26, 2024

OpenAPI 3.1 Schema causes errors in OpenAPI 3.0.x files redhat-developer/vscode-yaml#532

Open

feat: add support for the rest of the drafts in ajv

90c46d2

Davidonium changed the title ~~feat: add support for draft-04 json schemas while supporting draft-07~~ feat: add support for draft-04 (06, 2019 and 2020 included) json schemas while supporting draft-07 Dec 27, 2024

fix: use readFile instead of require to load fixture that is needed a…

cecc234

…s a string instead of parsed json

Davidonium mentioned this pull request Dec 27, 2024

YAML Language Server checks against JSON Schema Draft 7 instead of the defined one #780

Open

4 tasks

github-advanced-security bot found potential problems Jan 21, 2025

View reviewed changes

src/languageservice/services/yamlSchemaService.ts Fixed Show fixed Hide fixed

Merge branch 'main' into add-support-for-draft-04-schemas

e41cbc5

msivasubramaniaan requested changes Jan 21, 2025

View reviewed changes

Davidonium added 2 commits January 22, 2025 07:08

resolveJsonModule to true to be able to require ajv json schema

8a39698

revert the use of resolve json because the umd build does not support it

3ec09e4

remove support for draft-06 to avoid linting problems

4aa761e

Davidonium changed the title ~~feat: add support for draft-04 (06, 2019 and 2020 included) json schemas while supporting draft-07~~ feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07 Jan 22, 2025

Davidonium requested a review from msivasubramaniaan January 24, 2025 23:43

Merge branch 'main' into add-support-for-draft-04-schemas

adb309f

msivasubramaniaan approved these changes Feb 6, 2025

View reviewed changes

msivasubramaniaan merged commit 8ebaf45 into redhat-developer:main Feb 6, 2025
4 checks passed

Relequestual mentioned this pull request Mar 14, 2025

Enable/Disable yaml validation for specific yaml schema redhat-developer/vscode-yaml#649

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07 #1006

feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07 #1006

Davidonium commented Dec 26, 2024 •

edited

Loading

Davidonium Dec 26, 2024

Davidonium Dec 26, 2024

Davidonium Dec 26, 2024

HampusHauffman commented Jan 13, 2025

Davidonium commented Jan 18, 2025

coveralls commented Jan 21, 2025 •

edited

Loading

msivasubramaniaan left a comment

Davidonium commented Jan 22, 2025 •

edited

Loading

Davidonium commented Jan 22, 2025

msivasubramaniaan left a comment

Davidonium commented Feb 6, 2025

lantw44 commented Mar 10, 2025

Davidonium commented Mar 12, 2025 •

edited

Loading

Relequestual commented Mar 14, 2025

Relequestual commented Mar 14, 2025 •

edited

Loading

karenetheridge commented Mar 14, 2025

Davidonium commented Mar 15, 2025 •

edited

Loading

feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07 #1006

feat: add support for draft-04 (2019 and 2020 included) json schemas while supporting draft-07 #1006

Conversation

Davidonium commented Dec 26, 2024 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Is it tested? How?

Davidonium Dec 26, 2024

Choose a reason for hiding this comment

Davidonium Dec 26, 2024

Choose a reason for hiding this comment

Davidonium Dec 26, 2024

Choose a reason for hiding this comment

HampusHauffman commented Jan 13, 2025

Davidonium commented Jan 18, 2025

coveralls commented Jan 21, 2025 • edited Loading

msivasubramaniaan left a comment

Choose a reason for hiding this comment

Davidonium commented Jan 22, 2025 • edited Loading

Davidonium commented Jan 22, 2025

msivasubramaniaan left a comment

Choose a reason for hiding this comment

Davidonium commented Feb 6, 2025

lantw44 commented Mar 10, 2025

Davidonium commented Mar 12, 2025 • edited Loading

Relequestual commented Mar 14, 2025

Relequestual commented Mar 14, 2025 • edited Loading

karenetheridge commented Mar 14, 2025

Davidonium commented Mar 15, 2025 • edited Loading

Davidonium commented Dec 26, 2024 •

edited

Loading

coveralls commented Jan 21, 2025 •

edited

Loading

Davidonium commented Jan 22, 2025 •

edited

Loading

Davidonium commented Mar 12, 2025 •

edited

Loading

Relequestual commented Mar 14, 2025 •

edited

Loading

Davidonium commented Mar 15, 2025 •

edited

Loading