-
Notifications
You must be signed in to change notification settings - Fork 835
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
Update JSON Schema to be v1.0 compliant #1600
Conversation
amazing work dude |
Thank you for the awesome work. It seems that we could wire up a CI job to test the schema file against the fixtures you've provided using, say, ajv? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great
According to this PR #1127, i've added a schema to validate requests that update a resource or relationship. |
Actually, the JSON schema for v1.0 uses the draft-07 version. |
Once this pr is merged (and the #1603 also), we can update the reference in https://github.com/SchemaStore/schemastore (see #1011 ). |
I read this comment (#975 (comment)) and I realized that by modifying the tree structure (and the schema file name), I introduced some backward breaks. |
Could someone tell me what is missing for this PR to be merged ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as #1603 (review) on 1.1; typo s/forbiden/forbidden
Oups. Shame on me. |
Woah! Wish I'd found this a couple days ago - Took a while to realize the schema linked on the jsonapi.org page is not valid (blocker was relationships vs relationshipsFromRequest) This really needs to be on the website! Thank you! |
@derekwsgray Agreed, this is a high priority! @VGirol Thanks so much for your hard work on this. I'll finish reviewing this very soon. |
There are some places that don't work in OpenAPI 3.1 (thought I thought all of json schema should have worked.) |
did you check the v1.1 complaint schema PR #1603 ? would love to have some review |
Nope, didn't realize it existed! |
should we consider this and the v 1.1 json-schema updates for v1.1 final release? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this awesome work!
I left a few minor comments on the test cases. Please have a look.
I haven't reviewed the JSON Schema itself (yet). I'm not an expert on JSON Schema. The extensive test cases helps me a lot to verify correctness of the JSON Schema. Thanks a lot for putting that much time into them!
Do you mind resetting all changes, which are not related to the JSON Schema? Especially the changes to _format/1.1/index.md
, examples/index.md
, implementations/index.md
, and recommendations/index.md
?
Could we have /schema
as a HTTP redirect to _schemas/1.0/schema.json
? Or could we maybe copy it over when building the website? I worry that users may rely on that URL to fetch latest version of the schema. Would be great to have it as an alias pointing to the schema of the latest stable version.
Would be great if you could add documentation how to run the tests for the JSON Schema. CONTRIBUTING.md
seems to be a good fit for it.
Additionally we should run the tests with the schema in GitHub actions. Do you mind setting CI up?
@dgeb I think we should try to land this first and than build the schema for v1.1 on top of it. What do you think?
...mas/1.0/tests/request/resource/create/invalid/relationship_with_bad_resource_identifier.json
Outdated
Show resolved
Hide resolved
_schemas/1.0/tests/response/invalid/relationships/to_many_linkage_twice.json
Outdated
Show resolved
Hide resolved
_schemas/1.0/tests/response/invalid/resource/relationship_named_id.json
Outdated
Show resolved
Hide resolved
8e90d52
to
abd872c
Compare
These changes are present on the PR for version 1.1 (#1603). I don't know why they appear in this PR. I will remove them quickly.
It's a good idea, but I'm not sure I can make it happen. I don't know how the website is built. I'll take a look soon...
Well, i've used an hand-made validator coded in PHP.
I've never set up CI. I'm going to try... |
ajv is a great tool for validating json-schema, it can also validate a schema against the meta-schema, as would be the case here as well. I've set up CI for something like this for proprietary/in house projects before, if you'd like the help. |
Add meta member to all test files with informations about errors present in the document.
…hemas based on the results of the script.
…ne using files from previous versions
This reverts commit 8ce88f1.
Well, i've rebased the branch to the main branch and forced-pushed the result (but i'm not sure it was a good idea...). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you most probably put the node_modules in gitignore?
@@ -1,5 +1,5 @@ | |||
{ | |||
"$schema": "http://json-schema.org/draft-07/schema#", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regarding the schema version we will be using in v1.0, we need to get input from community experts. may be using the latest schema version with v1.0 specs is OK somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any harm in using the latest version, but it's up to you. Let's see what the community experts think about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By now i would say, just use the latest version
The node_modules is already put in gitignore. |
Ok, sorry, i saw what you meant : I removed the node_modules from PR. |
@dgeb Dan it would be great if you could have a look when you have some free time! |
Hello folks, any updates in this area ? |
Might this PR be the reason why the old link to the schema is dead? The link above is referenced in FAQ: https://jsonapi.org/faq/#is-there-a-json-schema-describing-json-api |
Yes. This PR made https://jsonapi.org/schema unavailable, not providing a (reachable) replacement. |
I allowed myself to continue the work started by Relequestual in the pull request #1555.
I've updated the JSON schema for version 1.0 and added many sample files to test it. In these sample files there are both valids and invalids documents.
I've also added a JSON schema which allows to validate requests for creating new resources (still for the 1.0 version).
I hope this will help.
if that's okay with you, i could do the same for version 1.1.
closes #1127
closes #1554
closes #1522
closes #867