Skip to content
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

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

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

simongoricar
Copy link
Contributor

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.

Made the fields match the existing example in `utoipa_swagger_ui::SwaggerUi` (field `path`).

Fixes juhaku#805
@simongoricar
Copy link
Contributor Author

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.

Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@juhaku
Copy link
Owner

juhaku commented Feb 18, 2024

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

@juhaku juhaku merged commit 5c6b0e2 into juhaku:master Feb 18, 2024
5 checks passed
@simongoricar
Copy link
Contributor Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to dynamically set spec_url in RapiDoc?
2 participants