search

Byron / google-apis-rs

A binding and CLI generator for all Google APIs
http://byron.github.io/google-apis-rs
Other
983 stars 132 forks source link

Derive utoipa::ToSchema by default for generating openapi specs by other libraries #511

Closed that-ambuj closed 1 month ago

that-ambuj commented 1 month ago

I needed this feature because our consuming API needs to generate docs for the frontend client but could not because of Rust's orphan rule.

This is PR is incomplete, I think it would be better to put this feature in a feature flag because everyone would not need it.

Please validate that this feature is a good idea.

that-ambuj commented 1 month ago

For context, we are using this library and to locally derive utoipa::ToSchema on external types is very hacky and we have to maintain a copy of the external types(which are too large for this crate) and since the versions don't mean anything here, it is very hard to keep them in sync. The hack in question is mentioned here https://github.com/juhaku/utoipa/issues/507

that-ambuj commented 1 month ago

Hey @Byron, could you tell me where do these dependencies come from in the below snippet?

https://github.com/Byron/google-apis-rs/blob/7b70b66c634ff7c798cda36fc68f5b99ae5a703a/src/generator/templates/Cargo.toml.mako#L41-L59

Byron commented 1 month ago

They are probably from here:

https://github.com/Byron/google-apis-rs/blob/7b70b66c634ff7c798cda36fc68f5b99ae5a703a/etc/api/type-api.yaml#L28-L33

that-ambuj commented 1 month ago

@Byron Do you think it would be a good idea to enable utoipa/url in the default feature set for utoipa?

that-ambuj commented 1 month ago

Would this be a good place to add documentation about this feature? or should it be mentioned separately as a Crate Feature? https://github.com/Byron/google-apis-rs/blob/7b70b66c634ff7c798cda36fc68f5b99ae5a703a/src/generator/templates/api/lib/lib.mako#L69-L70

Byron commented 1 month ago

@Byron Do you think it would be a good idea to enable utoipa/url in the default feature set for utoipa?

I don't know - if in doubt don't enable it, as users of utoipa can always enable it in their own tree, along with any other features they might need.

Would this be a good place to add documentation about this feature? or should it be mentioned separately as a Crate Feature?

https://github.com/Byron/google-apis-rs/blob/7b70b66c634ff7c798cda36fc68f5b99ae5a703a/src/generator/templates/api/lib/lib.mako#L69-L70

I'd probably put it at the very end of the library documentation, and cal it Cargo Features.

Besides that, the PR seems to be in a mergeable state, thanks a lot!

PS: It would be nice to see how these docs actually look like, in case you can post a screenshot for instance.

that-ambuj commented 1 month ago

We only need to enable the features that are used by this library crate. I can make utoipa/url a separate feature but since we are using url crate in the library, I would argue that it should enabled in the default utoipa feature set.

I'm really not sure where and what the url crate is used for. I'll just test it in my project if the crate works without that feature for utoipa.

that-ambuj commented 1 month ago

Just confirmed, it works without the url feature. I should remove it from the utoipa feature set.

that-ambuj commented 1 month ago

image

@Byron This is what the docs look like with cargo doc.

Byron commented 1 month ago

Thanks!

The PR looks like it can be merged now, thanks a lot for working on this.

To wrap it up, would you have a screenshot of the utoipa output? I have never seen it, and basically don't know what this is doing.

that-ambuj commented 1 month ago

image Definition of Policy struct from androidmanagement1

This is a screenshot from the swagger ui. It is generated from a big openapi.json file(with help of utoipa). The openapi.json can be used to generate client SDK using a tool like OpenAPI generator. You can also see a sample of this at https://petstore.swagger.io .

Byron commented 1 month ago

Thanks you!

So it does come full circle! There of course will be a good reason for doing that and not using the API definition that was used as source in the first place, but maybe it's subtly incompatible.

Maybe what would have helped to do the same is to keep a copy of the API definition JSON in the crate itself or make it very easy to download?

that-ambuj commented 1 month ago

From my use case standpoint, using the swagger json instead of this feature would achieve the same result. utoipa generated the specs from structs and does not keep a copy of openapi.json. Even if it did, we would have to manually edit the file and put the required parts into the json file, which is very prone to human error and unreliable.

With this feature we can automatically generate the specs for only the types we need.

Google uses the JSON Schema for it's APIs, which I found out today. We could use codegen for that standard instead but this crate works more reliably than the codegen tools I've used, so I'm still very sceptical of them.

P.S: You could check out https://github.com/Marwes/schemafy/ to see if it does anything better than your implementation. And I doubt if it's any better. And again, thanks for your good work and saving me and my teammates a lot of time.

that-ambuj commented 1 month ago

Just checked, there are no open source or actively maintained tools to generate code from Google's Discovery format. The current ones for JSON Schema only generate models, not API call functions.

Byron commented 1 month ago

Thanks for elaborating, that's all very interesting to me :)!

For a moment there I thought that Google doesn't provide bindings to their APIs in Rust because it's trivial to generate them using Swagger and Co., but from your experience that doesn't seem to be the case at all. Maybe one day… .