search

GREsau / okapi

OpenAPI (AKA Swagger) document generation for Rust projects
MIT License
631 stars 112 forks source link

Cannot use the openapi annotation with any of the rocket typed stream response types #68

Closed ardouglas closed 2 years ago

ardouglas commented 2 years ago

Hi,

Thank you so much for this library! It really is quite impressive what you have implemented thus far!

This issue is alluded to in https://github.com/GREsau/okapi/issues/52 but I have observed that this seems to be a larger issue impacting all of the typed streams in rocket 0.5.0-rc.1. I hit the same issue mentioned in https://github.com/GREsau/okapi/issues/52 but I saw that OpenApiResponderInner had already been implemented for TextStream<S>, ReaderStream<S>, and ByteStream<S so I decided to see if TextStream would work for my use case. The result is the same compiler error encountered with EventStream!:

error[E0562]: `impl Trait` not allowed outside of function and inherent method return types

The EventStream! and other typed streams return types ultimately expand to a type like the following and they trip up the openapi macro because it can't just reuse the type as defined.

impl rocket::futures::stream::Stream<Item = rocket::response::stream::Event>

I pulled your latest code and dug around a bit and made an attempt at a fix but I didn't see a clear path forward. In the case of TextStream and ByteStream, I could see where the openapi macro could rewrite the type to one of the string or byte representations that already have an implementation of OpenApiResponderInner. For EventStream, I was thinking along the same lines and tried rewriting the type as the rocket Event type; however, this type doesn't implement rocket::response::Responder so that looked like a dead end.

One alternative I was thinking might be possible would be to add an optional field to the openapi attribute that allows the developer to specify a type to substitute in place of the type that the typed stream macros expand to. I obviously don't have the same insight into the inner workings so this may be a terrible idea!

If you have some guidance on how you would like to see this functionality implemented I would be glad to take on this effort!

Thanks!

lrodriguez14 commented 2 years ago

Hi @ralpha,

Unfortunately all the stream methods are not supported yet, any idea what could be missing forTextStream<S>, ReaderStream<S>, andByteStream<S>?

ralpha commented 2 years ago

Hi @ralpha,

Unfortunately all the stream methods are not supported yet, any idea what could be missing forTextStream<S>, ReaderStream<S>, andByteStream<S>?

These types should work:

With regards to the EventStream! and EventStream Note that one is a macro, so this makes things more complicated. That is why I posted: https://github.com/GREsau/okapi/issues/52#issuecomment-881091896

The EventStream (struct) should be doable. But the Responder implementation is a bit special because of the generics So this should be added here somewhere. https://github.com/GREsau/okapi/blob/master/rocket-okapi/src/response/responder_impls.rs#L302

I'll have a quick look if I can add that, but don't have that much time if it happens to be a bigger problem.

ralpha commented 2 years ago

There, still took me still too long, but here is a fix. It is a somewhat dirty fix, but it works. It had to so with EventStream<impl Stream<Item = Event>> (what the macro also creates) The impl TYPE section is only allowed in return statements. And we are using the type like this: https://github.com/GREsau/okapi/blob/e686b442d6d7bb30913edf1bae900d14ea754cb1/rocket-okapi-codegen/src/openapi_attr/mod.rs#L250

So that resulted in a compile error. It now looks for the identifier (like EventStream) and if there is an impl statement and uses predefined types in that section. This does not change the resulting docs. But because the filter is not incredible, the type has to be imported in the current module (use rocket::response::stream::EventStream;) in order to work.

We can always make this nicer in the future, but this at least gives you a fix. Not going to create/ask for a new release on Crates.io. But you can use the master branch with the patch for the time being.

Created an example to test this and add some docs: https://github.com/GREsau/okapi/blob/master/examples/streams/src/main.rs

I also documented a way you can add undocumented endpoints in case it does not fit your needs or still have some errors because of special return types. (you can still report to error so we can fix it) https://github.com/GREsau/okapi/blob/e686b442d6d7bb30913edf1bae900d14ea754cb1/examples/streams/src/main.rs#L62-L66 https://github.com/GREsau/okapi/blob/e686b442d6d7bb30913edf1bae900d14ea754cb1/examples/streams/src/main.rs#L75-L76

Let me know if this works for you.

ralpha commented 2 years ago

Please also note that RapiDoc and Swagger do not work well with infinite streams. So watch out when using the Try now buttons.