openapiv3
openapiv3 copied to clipboard
Support for OpenAPI v3.1
Resolves #50 .
I'm leaving this as a draft for now so people can take a look at the approach and ideally try using this branch to see if it meets their needs. I'll be doing the same for a couple of weeks as I'm not yet convinced that this is the right way of adding 3.1 support.
Also note that lots of the lines changed in this PR were due to moving the 3.0 support to a separate sub-module. It would probably make sense to review the commits separately.
I was skeptical of this approach, but seeing it in action, I'm warming to it. I haven't been through it in fine details, but I had a couple of high level points and questions.
-
I think a desirable outcome would be if this were compatible with past versions by default. I could see this as excessively limiting, but it would be nice if there were a way to introduce this compatibly into version 1.x of this crate, get some use, and then make an incompatible change with 2.0. For example, in
lib.rs
do apub use v3_0::*
and rename the multi-version enum to something likeOpenAPIVersion
orOpenAPIGeneric
(neither of those names is great). Alternatively, I could imagine just telling people to s/openapiv3::/openapiv3::v3_0::/g and that would suffice... -
What would you think of a "upgrade" function that would convert 3.0.x to 3.1? It seems pretty straightforward (https://www.openapis.org/blog/2021/02/16/migrating-from-openapi-3-0-to-3-1-0). For consumers of OpenAPI documents, putting into a canonical form seems very helpful.
-
One of the key benefits of OpenAPI v3.1 is convergence with JSON Schema. I really like that you're using
schemars::schema::Schema
here! I'm not sure, however, about flattening the untagged enum. I also think while we don't need to addextensions
which are part ofSchemaObject
we will need to addexample
anddiscriminator
(https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#fixed-fields-20). -
With regard to this review, I appreciate the wrapping of comments, but perhaps we could pull that out to minimize the diffs?
-
There's a lot of code/comment in comment between v3_0 and v3_1. It might be worth considering how we share that code where it's literally identical e.g. via
include!
(sinceuse self::*
could replaceuse crate::v3_X::*
).
In summary: this is great stuff; let's move it forward.
Also: you might want to run this through the repo I've used for testing: https://github.com/ahl/openapiv3-test
@JamesHinshelwood - Are you still available to carry this to victory? If not, I'd be happy to clone your branch and make the necessary changes.
@rrichardson I think at this point it would be reasonable to build on this work if you have cycles. Please see my comments above if you're going to take a swing https://github.com/glademiller/openapiv3/pull/55#issuecomment-1073045225
@ahl - I will work on this today.
One question regarding Security Schemes: In The SecuritySchemes Spec they casually mention
This object MAY be extended with Specification Extensions.
How do you propose we deal with this? (I think it's true for 3.0 as well)
This is of special interest to me, because my company uses openAPI to generate Protobuf, among other things, so we take advantage of the fact that we're not specifically tied to the limited set of Auth mechanisms that are "supported" by OpenAPI
One more thing: Regarding upgrades, it seems the most idiomatic approach would be just to impl TryFrom<v3_0::Foo> for v3_1::Foo
(From
, I'm not sure if they're perfectly compatible) for every type, so that anyone upgrading could just
let my_old_schema: v3_0::OpenAPI = serde_json::frrom_str(&x).unwrap();
let my_new_schema: v3_1::OpenAPI = my_old_schema.try_into().unwrap();
let bytes = serde_json::to_vec(my_new_schema).unwrap();
I guess the question is: Where should we put such an impl? Part of me says that v3_1 and v3_0 shouldn't know about each other. The other part says that it should be acceptable if newer versions know about older versions.
Thoughts?
I'm ambivalent about including conversion logic in this library (which in my mind is just serde (de)serialization implementations!), but if you do:
The other part says that it should be acceptable if newer versions know about older versions.
^ I agree with this :)
How do you propose we deal with this?
I think we need to add extensions to each of the SecurityScheme
variants; alternatives?
Agreed on the conversion approach.
One more wrinkle for 3.1: the schemars
crate seems to be only sporadically maintained. I think it's fine to prototype against it, but before we publish a version that depends upon it, I think we should get clarity on its future... or fork the repo if it's a dead end.
One more wrinkle for 3.1: the schemars crate seems to be only sporadically maintained.
It seems to get a new version every couple of months, it was updated as recently as 6 days ago. The PRs and Issues, while numerous, do seem to make some progress.
It does seem like it could use a couple more reviewers/committers, but it seems actively maintained.
I guess we'll see in a bit how it performs in the context of OpenAPI.
Ok. I't not quite ready, but it is close. I need to do something about the From<> for Schema.
https://github.com/glademiller/openapiv3/pull/58
Sorry I dropped this, thanks for picking it up @rrichardson - Your PR looks good to me FWIW. Since this PR is a subset of yours, I will close it.