-
Notifications
You must be signed in to change notification settings - Fork 865
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
Creating new resources and validating with JSON Schema #867
Comments
See #851, which is another reason not to use the published schema to validate incoming requests. Basically, that schema is only for validating output/response documents, and we're already thinking about whether to create a separate one for incoming requests (or just to keep a single one, but make it more lenient). |
Citing the official documentation
|
@dgeb @tkellen @steveklabnik Do you guys have an opinion here on whether it's worth it to maintain multiple, stricter schemas or just go with one looser one? I was leaning toward the looser schema approach and started mocking it up, as you can see from the links. But, on reflection, ditching the |
If I may offer my 2 cents (as someone who has used JSON-API in a production environment and had the chance to look at it quite thoroughly). One of the most nice aspects about JSON-API is that once a request goes through a JSON-Schema, you can trust it's okay because it's so strict, and you know that certain data is at your disposal. |
@ethanresnick : same like @mvdstam |
@ethanresnick I see no harm in maintaining multiple schemas. In fact, I think it's a pretty great idea (as long as their separate usages are clearly defined). |
Here is an updated schema to allow for creating new resources. I'd like to get some feedback before creating a pull request. In this version, it allows for a new |
Thanks @eneuhauser! I may be misunderstanding something, but I think that schema will accept a single If that's right, your version is still very interesting because it's stricter than the version I proposed, so maybe it's a good compromise. But, if we do decide that we want to make all the schemas as strict as possible, I don't think we can get around actually providing distinct files for the different cases. |
@ethanresnick, you are correct, a PATCH without an ID would also yield a false positive. As an aside, the spec specifically says this is invalid, but in my implementation it's moot since the URI has the ID. The request schema and response schema solution would still have this problem. I'm using RAML to define my API and extend the JSON API schema with my own schema to explicitly enumerate the attributes. With my proposed change, you could reference the |
Right. So I was imagining that one schema would be for the "create resource document" case and another would be for "all other request/response documents". Then, for each of these two cases, we'd need two files, one with with That's why I was a bit hesitant about that approach. At the same time, the changes are small and pretty mechanical, and having these schemas helps implementors, so maybe it's the way to go. If @tkellen's still onboard given the above, then I am too.
Can you explain to me how this works? Is it something that a user can do only if they're using RAML, or does a generic JSON Schema parser allow the user to call out some definition in the schema and check just against that? |
Have you read the article I posted above? |
@sebilasse I just did, and here's what I got from it:
|
@ethanresnick, below is an example RAML. At the top, you can see the two custom schemas that extend the JSON API Schema. You don't have to use RAML, you could have the custom schemas without defining it in a RAML file. In my custom schemas, I overwrote the attributes to enumerate the specific properties for my implementation. In this case, I specify that it only allows a title attribute. This schema allows for any @sebilasse, I hadn't heard of hyper-schema before. It looks really interesting and I would like to play with that more. In your specific implementation, you could use hyper-schema that references definitions in http://jsonapi.org/schema. I believe implementations should be using a custom schema that extends the base JSON API Schema with more detailed rules. If you don't extend the base JSON Schema as demonstrated below, it opens the API up to more issues than just omitting the ID. Potentially, a client could be PATCHing a resource with a misspelled attribute. By explicitly declaring the attributes, you could avoid those issues. Take a look how the below RAML uses the JSON API Schema as a base and extends it with the specific rules. #%RAML 0.8
---
title: JSON API Examples
version: v1.0
baseUri: http://jsonapi.org
mediaType: application/vnd.api+json
schemas:
- BlogPost: |
{
"$schema": "http://json-schema.org/draft-04/schema#",
"title": "JSON API Existing Blog Post Example Schema",
"allOf": [
{
"$ref": "http://jsonapi.org/schema#/definitions/success"
},
{
"properties": {
"data": {
"allOf": [
{
"$ref": "http://jsonapi.org/schema#/definitions/resource"
},
{
"properties": {
"attributes": {
"type": "object",
"properties": {
"title": { "type": "string" }
},
"required": ["title"],
"additionalProperties": false
}
}
}
]
}
}
}
]
}
- NewBlogPost: |
{
"$schema": "http://json-schema.org/draft-04/schema#",
"title": "JSON API New Blog Post Example Schema",
"allOf": [
{
"$ref": "http://jsonapi.org/schema#/definitions/post"
},
{
"properties": {
"data": {
"allOf": [
{
"$ref": "http://jsonapi.org/schema#/definitions/newResource"
},
{
"properties": {
"attributes": {
"type": "object",
"properties": {
"title": { "type": "string" }
},
"required": ["title"],
"additionalProperties": false
}
}
}
]
}
}
}
]
}
/posts:
get:
responses:
200:
body:
schema: BlogPost
post:
body:
schema: NewBlogPost
/{postId}:
get:
responses:
200:
body:
schema: BlogPost
patch:
body:
schema: BlogPost
delete: |
off course, common tooling supports it. sorry, just wanted to underline the needs for different schemas for a consumer here ... |
please see / should fix json-api#867 json-api#851 partially fixes json-api#406 ---> see my following answer in json-api#867 regarding versioning
@tkellen @ethanresnick @eneuhauser @mvdstam @bf4 We have three DRY schemas for
regarding versioning This means that the schema would implement
https://github.com/redaktor/json-api/tree/gh-pages/schemas/1_1 🔍 |
@sebilasse Thanks for putting these together! I'll take a look at them later tonight. On a quick glance, though, they look very nice 👍 One thing, though: whether That is, imagine that a server generates a response document. If the server wants to check that it generated it correctly, it needs to test with Now, if it's too much work to make both the As far as versioning goes... json api's never remove, only add versioning strategy is atypical, and it means that having a separate schema for each version might not make sense. But, regardless, let's punt on that question for now since 1.1 isn't even out yet :) |
ok - I see - I did the request/response naming because when we think of using JSON hyperschema, there is a "requester" and a "responder" :
generated means 'response' in JSON hyperschema
yes, but also if the client 'request' something at the server, the spec. says
and
well, you have only only one "public" schema [ @ethanresnick : I did that after your comment ... ] --> |
And @tkellen @ethanresnick regarding the original schema one question:
|
My point is that it's not a question of naming. Please see issue #851 and let me know if what I'm saying makes more sense then.
Yes, but remember that the |
@sebilasse yes, i think it can be an anyOf. Also, success can require a |
@ethanresnick - important when you check the schemas: In JSON schema I have read #851
isn't ... and I would have commented there what @bf4 had commented there. |
this might get a bit philosophical ... currently it is not add-only because if the ;) going to bed now - late here... |
Ok. But the problem is the few places where you have
No :) Let me give an example. Imagine the client requests
Here, the server has made a mistake: it's put the attributes directly on the resource object rather than under the
So, on the client, the schema that's validating the server's response MUST have However, if the server author had written some unit tests to make sure that their server was behaving properly, those unit tests would want to check the server's response with That's what I mean when I say that the same document—which could be a request document or a response document—needs to have Now, what I was saying here is that the test case situation is much less common then the other situation, so if we were only going to set Here's also a more concrete example of why
But the schema isn't normative :) The schema is just meant as a helpful tool for developers. Only the text on the specification page is normative. And, as you pointed out earlier, the 1.0 text already says that id is optional on creation. So we're not actually making any changes to that between 1.0 and 1.1. |
@ethanresnick ... apart from that you wrote:
But how can the server make this mistake at all ? spec.:
schema ...response/#/definitions/data:
= the server is not able to put the properties 'directly' under 'data' ok - about
Please let me ping @geraintluff - he has specified this v5 proposal which might be an alternative:
this is exactly the reason why I think, versioning is fine. step by step
Another benefit is now that the server can 'reason' the version and due to the "top schema version switch" the server knows that somebody wants to communicate in version 1.0. A clever server might now respond with a 1.0 response (same logic as in RPC) – however –
So think of |
@ethanresnick Please let me also answer this in detail (just fyi) :
This is not entirely true. JSON Schema was invented by Kris Zyp and he is so genius ;)
can be any template and it can also be an empty template, see 5.1.1. URI Templating just in case anybody wants to communicate in realtime about this - |
I agree with this in theory, but I think the picture changes somewhat when you consider that many server implementations will be built on top of generic libraries (like JSONAPI::Resources or endpoints). Those generic libraries will likely run a validation step where it would be convenient to check (against a schema) that the request document is valid JSON API at all. But, for validating the particular fields, it may be more convenient for those libraries to use whatever internal model class the user has defined, rather than forcing the user to provide an extended schema (or generating an extended schema from those models). So I don’t think we can just assume that subschemas will be created (even though that would be nice, because it would give the users a way to pull out just the relevant definitions without us needing separate files).
You’re right that the spec prohibits the server from responding that way. That’s why I called that response a mistake. It’s a bug in the server. But the whole point of unit tests (where you’d have
Ok, I think I understand what you’re saying now. You’re saying that every incoming piece of data counts as a request (and is checked against the request schema), and every outgoing piece of data counts as a response (and is checked against the response schema). There are two problems here: First, there is a naming issue. (I apologize for saying the issue wasn’t naming earlier; I misunderstood.) In HTTP, “request” and “response” to mean different things than how you’re using them. Request means “something sent from the client to the server” and response is “something sent from the server to the client”. So, by that meaning, it wouldn’t make sense for the client to client check the incoming data from the server against something called a “request” schema. But, more importantly, there’s a conceptual problem: the incoming data to the client is in a different format than the incoming data to the server—so they can’t both be validated by the same schema, whether that schema’s called “request” or anything else. To give an example: It’s completely valid for a client to receive only a No matter how you slice it, there are four basic cases:
As I’ve said two or three times already, though, maybe the latter two cases can be ignored, if we think that using schemas to handle them (e.g. in unit tests) is uncommon enough that we don’t need to make distinct, maximally-strict schemas for these case. But, if we were to make such a schema, it would need
I understand. But it’s not going to change now post 1.0, and implementations have to account for it being optional. This conversation seems to be going in circles, so I’m going to bow out now, but I’d be more than happy to review PRs from either @eneuhauser or @sebilasse that take this thread into account. |
Right - and OK, now I've also got your point !!! (so it is not a circle anymore ;) Let me make it even more clear : however before I can do a PR - what would you like ? For case 1 and 2 we should simply list the differences between those two here - so that I can change the schemas... |
@ethanresnick btw - currently reading a nice article beginning with
This leads me to think about the "$schema" keyword. In the case of our JSON Schema it validates against the "meta-schema" (the spec. for JSON Schema) And when you have annoying people saying "I want and pps. - our v5 proposal for multilanguage doc. |
I understand, and I appreciate you wanting to help out! Apologies if I was a bit snappy; yesterday was a long day.
When we take create into account, I count 6 being needed; there are the four I mentioned above, except that cases 2 and 3 each get split into two, so we have:
Now, as for whether to do all 6 or just do 3: let's start with 3. Then, once those are done, we can see if we can add the other 3 in a DRY way. For now, let's avoid geraint's proposal because it isn't standardized yet. If it makes it into JSON Schema draft 5, we can reconsider.
I don't remember them all by heart, so you might have to dig through the specification to get an exhaustive list. But here are the big ones:
I think if we start with that, we'll be good, and we can improve it over time.
Let's talk about that in a separate issue. I'm definitely up for considering it (i.e. whether to recommend |
I have created two new files for creates and updates to have their own schema. In doing so, I had a question about the spec: in a client request, is it valid to include meta and jsonapi properties? I assume top-level I ran into another problem in separating the schemas: should the schema be self-contained or should the create and update schema reference the primary schema? Referencing the primary schema would be DRY, but many tools do not respect the external schemas very well. I'll defer to the group whether it's better to be self-contained or DRY. I had considered moving the schema under a @ethanresnick and @sebilasse, please checkout my branches and let me know what you think. I wanted a ruling on which style before submitting a pull request. I have not updated the FAQ page yet to point to the new schema. Once we decide on a direction, I will include the FAQ update in my pull request. |
Thanks @eneuhauser! I'll try to take a look at them over the weekend. For now, please make sure they account for the latest couple bugfixes. And, if these schemas are meant to be used for validating incoming data (i.e. a client validating a server's response, or server validating a client's request), make sure they've got
I think assuming that
Right, that was my concern as well. Maybe we can host separate DRY schemas and then use some build tool that will create self-contained ones? Or even just link users to some tool online that does that, so they can make them for themselves? |
Thanks @ethanresnick for the feedback.
The files I submitted addressed:
Sorry to drum up this subject, but I feel that if there were only three, having the less restrictive schemas aren't as good.
All that said, I'd be all for creating the first three schemas so long as the last three schemas exist. Having a build tool would ease that process. It just so happens I already have a build tool that could work. It was designed to piece together RAML documents using Gulp and generate an HTML file. In it, there are tasks that piece together schema files with shared I will work on a new pull request that creates all 6 documents using my build tool. I'll be sure to include to two bugfixes you referenced. |
The problem with that logic, I think, is that clients and servers won't generally be maintained by the same organization. If organization X deploys a 1.0 server, they may well forget about it at some point (or at least not have the manpower to update it to the latest spec version). So that 1.0 server really, really should be set up to ignore but not reject additional properties, or it won't work with any 1.1+ clients. Independent evolvability of client/server is essential for REST and a good protocol in general.
I get that, so I understand why they seem odd. But, in the protocol design world, this apparent "contradiction" is actually a logical prerequisite for extensibility. Please take a look at this great article if you have a second. Basically, it proves that you need to make the set of legally-producable messages smaller than the set of legally consumable messages (which is what
Awesome! It'd be nice to incorporate this build tool into the
👍 💯 |
@eneuhauser Any update on this? |
So is there a schema we can use to validate creating resources? I am wanting to use it for unit testing purposes. (Sorry if I missed it in the long chain above.) |
This bit me today. Would be great to have schemas for both request and response. |
Perhaps instead of
|
@eneuhauser Sorry for missing your last reply.
I don't think so - and the DRY branch is fine as a start. There is always one or two leading tools in each language and personally I prefer to split the dereferencing and validation into two seperate tools and sometimes cache the dereferenced schema and so on. Just please see the above comments by @aaronshaf and @bf4 – I am open to work on this. |
Would be great to use the schema to validate the examples :) |
Hi, On the basis that this issue doesn't seem to be progressing, could I suggest that the FAQ be updated with an interim message about the schema? For now, it could be explicit that the schema should be applied only to responses, not requests, which would help newcomers to avoid this pitfall until the replacements are ready. Many thanks! |
Hi folks- I'm popping in from the JSON [Hyper-]Schema project where we're about to release a new set of drafts. This issue was just brought to my attention- I think a lot of things we've been doing in the last two drafts would make things easier for you:
I'll comment more when I have a chance to dig through all of this, but I wanted to say "hi" and offer my assistance. I expect to formally publish the draft-07 documents by the end of the month, but if anyone wants a preview we have the work-in-progress documents posted. |
The official JSON API Schema (as described at http://jsonapi.org/schema) explicitly expects the ID field to be sent by clients when creating new resources. This conflicts with the following example (pulled from the official documentation):
When I validate the above to the official JSON Schema document, validation fails. Perhaps a seperate JSON Schema document can be provided solely for creating new resources, where the ID field is optional?
The text was updated successfully, but these errors were encountered: