utoipa icon indicating copy to clipboard operation
utoipa copied to clipboard

Missing actix::web::scope support

Open bhoudebert opened this issue 2 years ago • 3 comments

There is many way to declare route with actix::web but for one of them is using scope (which is a must have imho, even better with macro route usage) https://docs.rs/actix-web/3.3.3/actix_web/struct.Scope.html

/// main.rs
// app definition
  ...
  .service(web::scope("/health").service(ping_pong))

/// health.rs
#[utoipa::path(
    tag = "Health",
    responses(
        (status = 200
         , description = "It should respond with \"pong\""
         , body = String),
    ),
)]
#[get("/ping")]
pub async fn ping_pong() -> impl Responder {
  HttpResponse::Ok().body("pong")
}

What will happen here is that if we do not explicitly declared the path (with full value) it will just be generate on openapi with /ping instead of /health/ping.

Example here is simple but in an application with a lot of subscope: version, resources, action, sub resources it becomes more and more tricky to be sure to keep everything aligned (people will obviously drop declaring partial scope in favor or routes hidden in some other files),

Would it be possible to support scope?

bhoudebert avatar Apr 28 '22 20:04 bhoudebert

This is supported. But needs manual intervention since unfortunately there is no way to access to information given to web::scope(...) function.

  1. Yo could declare context_path within utoipa::path macro: See example here: https://github.com/juhaku/utoipa/blob/master/examples/actix-web-multiple-api-docs-with-scopes/src/main.rs
...
  .service(web::scope("/health").service(ping_pong))
/// ....
#[utoipa::path(
    context_path = "/health",
    tag = "Health",
    responses(
        (status = 200
         , description = "It should respond with \"pong\""
         , body = String),
    ),
)]
  1. You could use Server type found within OpenAPI https://docs.rs/utoipa/1.0.1/utoipa/openapi/server/struct.Server.html
impl Modify for CustomServer {
  fn modify(&self, openapi: &mut OpenApi) {
    openapi.servers.as_mut().map(|servers| {
      servers.push(Server::new("/api/v1"))
    })
  }
}

Server can also be defined in PathItem level, but accessing it might be cumber some so far since I haven't yet added ability to add it via utoipa::path or #[openapi(...)] attributes.

Context path is exactly to counter this scoped paths issue and it will concatenated to the path. And servers can be used to change the actual path or even server where the api request would go but will generage the same paths without the scope.

juhaku avatar Apr 28 '22 22:04 juhaku

Yes I got that helper but was hoping to get something magic. To me, this far from safe as it would mean the scope is duplicated between the actix macro and the app definition and there is no guarantee you wrote something correct.

NB: The only solution I had in mind, did not used it yet, was to use several global variables but it is far from a gold solution.

If we imagine writing something like this (totally random code/entity, it does not make sense):

App::new()
    .service(ping_pong)
    .service(
      web::scope("/v1")
        .service(web::scope("/resource_one").service(resource_one_r::get_all))
        .service(web::scope("/games").service(games_r::get_all))
        .service(
          web::scope("/auth")
            .service(auth_r::login)
            .service(auth_r::register)
            .service(users_r::get_me)
            .service(auth_r::verify),
        )
        .service(
          web::scope("/users")
            .service(
              web::scope("/me/requests")
                .service(current_crate::routes::user_requests_r::add_request)
                .service(current_crate::routes::user_requests_r::cancel_my_request)
                .service(current_crate::routes::user_requests_r::get_closed_request)
                .service(
                  current_crate::routes::user_requests_r::get_pending_request_from_user,
                ),
            )
            .service(
              web::scope("/me/sidekicks")
                .service(current_crate::routes::user_sidekick_r::get_all)
                .service(
                  web::scope("/sidekick_requests")
                    .service(
                      current_crate::routes::user_sidekick_r::validate_request,
                    )
                    .service(current_crate::routes::user_sidekick_r::refuse_request),
                )
                .service(web::scope("/auto_requests").service(
                  current_crate::routes::user_requests_r::auto_user_request,
                )),

In that peculiar case would you agree that even using context_path is cumbersome and prone to errors? I understand that it might not be possible as if I compare with other crates that does this, they create an actix::web around all service definition which I did find a little bit too intrusive.

bhoudebert avatar Apr 29 '22 07:04 bhoudebert

Yes I agree its is far from perfect and is prone to errors and unnecessary duplication. Currently context_path is not able to receive a variable reference and only expects literal string. Yeah I originally thought about this and didn't want to implement with the context_path but I found no alternative ways to do it. So so far it is best we got.

Also I want to keep the the utoipa library seprated as much as possible from the internals and management of other frameworks as possible. It could be possible if we wrap the application or the scopes with utoipa specific macro function that would do the context_path gathering from scopes something like this:

App::new()
    .service(ping_pong)
    .service(
       utoipa::scoped!(
           web::scope("/v1")
               .service(resource_one_r::get_all)
       ) 

But then not sure how to attach the scope to the get_all endpoint. And with this approach people need to mix and match the application with some random external functions.

juhaku avatar Apr 29 '22 08:04 juhaku