-
Notifications
You must be signed in to change notification settings - Fork 63
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
ReadOnly / WriteOnly clarification toward required field #101
Comments
Different applications will do different things with the readOnly/writeOnly metadata.
This is not in the spec, but they are conventions that good tools use. A huge part of my brain worries when I hear statements like that, because that means inconsistent experiences across different tools, but only for the rare few people making their OAS available in multiple documentation tools, or using multiple linters, generally thats not going to be an issue, and the majority of tooling is moving towards these sensible standards because everyone expects those tools to wokr that way and request it when missing. |
The issue is that you have a lot of case where the OAS spec is involved by several layers , and several tooling especially , Linter , Mock server , Proxy validator (including API gateway , or just tooling like prism) , as well as code generator with validation enabled . Having a consistent , pre determined behaviour and standardized is usefull here I understand that readOnly is meta data for JsonSchema (to me it should even be dropped from it) , but as it is touching the payload validation in OAS context having it clarified would be good thing , long time ago it was even part from OAS |
Yes I'm very aware the same OpenAPI could be put through a linter, mock server, proxy, validater, etc, I've worked on all these tools for years. I'm saying that JSON Schema makes the choice of considering this metadata to be used by the application in ways it sees fit for the same reasons that OpenAPI also does: it's impossible to know what its being used for so trying to clamp down on exactly how this should be used when the same spec is used by so many different tools is difficult and probably counter productive. You don't want the spec venturing off into guidance of how linters should handle this or how validators should handle this or how SDKs should handle this. What I'm saying is, I do not think this is a problem, and I do not think anything here needs solving. I think it's ok to leave this as metadata, and tooling should be aware of the context in an appropriate way. I've noticed you talking in Vaccum issues about this and I have noticed Vacuum handles required wrong with readOnly/writeOnly, that's an issue for Vacuum and I'll talk to David about it, but I don't see that as a reason to try and fix this in the spec because I don't think it can be fixed in the spec. |
@LasneF you can combine Combining This is all rather involved to include in the spec, but might make for a great page on the Learn site. |
@OAI/tsc review request: I don't think there's anything to do for the spec here, as there are too many possibilities that might "look" wrong but actually have value. This is not entirely unrelated to the question of how strict to make the schemas that came up in OAI/OpenAPI-Specification#3837 (comment) Unless we want to change something in the spec we should close this (and maybe open an issue on the Learn site, as it would be worthwhile to have a more in-depth discussion of this complex topic). |
thanks for the tips OAI/OpenAPI-Specification#2110 , is clear as well as the "old" specification in 3.0 was clear " If the property is marked as readOnly being true and is in the required list, the required will take effect on the response only." with the switch to JSON schema this rules becomes less clear, but the spirit is there. as it is clarified, i am closing this ticket ... i can open a ticket on my favorite linters 😅 |
@LasneF this guidance was removed intentionally. It is NOT the spirit in which the keyword should be used in 3.1, and a failure to enforce |
@LasneF I'm going to re-open this and transfer it to the Learn site because you assuming that you can apply the 3.0 to the 3.1 text when we intentionally removed that text in 3.1 shows that we need to do something about it. |
It would be interesting to have a statement on the precedence of readOnly rules vs required rules
given the specification here
small.json
the object contains Id , name , password , both are required ... in differents scenario
id is set to ReadOnly
password as WriteOnly
use case is legitimate, for instance you want to have a single model for an Object (here Pet) , and the id beeing mandatory in read , but not in write (ie POST)
should this schema be valid or not ?
it so it would be good to add a line when we set OAS is leveraging JSON schema for validation .
discussing with JSON schema team the readOnly / writeOnly flag are just meta data and are not taken into account in the validation, it is up to the upper layer to take those points in consideration
to me we have 2 ways of handling this ,
as this it is not the Json schema policy to defines this behaviour , would be good to have a precision in the OAS Spec
notice than looking on linter , most of them consider the spec as invalid ... my guess because leveraging raw json schema library without pre processing the schema with the context of usage
The text was updated successfully, but these errors were encountered: