juhaku / utoipa

Simple, Fast, Code first and Compile time generated OpenAPI documentation for Rust
Apache License 2.0
2.01k stars 160 forks source link

Refactor `RapiDoc` to take `Cow<'static, str>` instead of borrowed `str` #867

Closed simongoricar closed 4 months ago

simongoricar commented 4 months ago

This PR fixes #805

Problem

As far as I can tell, it was previously impossible to dynamically construct URLs or custom HTML in certain cases. This was due to RapiDoc's path, spec_url and html fields being &'_ str.

Not bad in principle of course, but a problem arose when trying to use this in combination with actix_web (possibly other integrations, but I haven't tried). actix_web's HttpServer::new requires that you pass a factory closure with the following conditions: Fn() -> I + Send + Clone + 'static, and inside of that closure we need to initialize RapiDoc. As such, the issue is that we can't dynamically construct a 'static string in these conditions (excluding quite terrible practices).

For example, this would not compile beforehand (in this case due to temporary value dropped while borrowed):

use actix_web::{App, HttpServer};
use utoipa_rapidoc::RapiDoc;

fn main() {
    let base_url = "/some/path";
    let spec_url = format!("{}/api-docs/openapi.json", base_url);

    HttpServer::new(move || App::new().service(RapiDoc::new(spec_url.clone().as_str())));
    // ...
}

Solution

Instead of having RapiDoc's methods take &'_ str, I modified them to take Into<Cow<'static, str>> and modified the path, spec_url and html fields to be Cow<'static, str>. This is also how this is handled in the adjacent utoipa_swagger_ui crate.

This example with a dynamic URL now compiles:

use actix_web::{App, HttpServer};
use utoipa_rapidoc::RapiDoc;

fn main() {
    let base_url = "/some/path";
    let spec_url = format!("{}/api-docs/openapi.json", base_url);

    HttpServer::new(move || App::new().service(RapiDoc::new(spec_url.clone())));
    // ...
}

Breaking changes

As far as the methods go, I don't think this has any breaking changes due to the Into<Cow<'static, str>> generic. However, I think the removal of the three lifetime parameters on RapiDoc can break some code.

simongoricar commented 4 months ago

Now that I think about it, I think it's possible to use Cow<'a, str>'s instead of 'statics. This would eliminate breaking changes as the three lifetimes would still be present on RapiDoc (one for each field, as before) and we could still pass in owned strings.

Let me know if you'd prefer manual lifetimes instead of 'statics. It might make sense to refactor other UI crates to the same idea if we go that route, though.

juhaku commented 4 months ago

I think this is just fine with the 'static unless there is someone needing the manual lifetimes. Manual lifetimes might sometimes be quite annoying.

simongoricar commented 4 months ago

Nice update, probably something that should be done for the Redoc if not already there. 👍 But that is another thing for another time. Thanks 🙂

Sure, why not. I'll submit another PR :)