kurtbuilds / oasgen

Generates OpenAPI 3.0 spec based on Rust code. Works with axum, actix-web, or any/no framework.
https://crates.io/crates/oasgen
112 stars 8 forks source link

Tuple path parameters #24

Closed BertrandD closed 2 months ago

BertrandD commented 2 months ago

Hi,

Using axum, I have a query like that:

pub async fn get_stuff(
    Path((a, b)): Path<(TypeA, TypeB)>,
) -> Result<Json<Stuff>> {

}

the point here is to have a tuple in the Path

when starting the server, I get this error:

Call parameters() or body_schema() for tuples, not schema()
stack backtrace:
   0: rust_begin_unwind
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/panicking.rs:72:14
   2: oasgen_core::schema::tuple::<impl oasgen_core::schema::OaSchema for (A1,A2)>::schema
             at /home/bertrand/.cargo/registry/src/index.crates.io-6f17d22bba15001f/oasgen-core-0.21.1/src/schema/tuple.rs:24:9
   3: oasgen_core::schema::OaSchema::schema_ref
             at /home/bertrand/.cargo/registry/src/index.crates.io-6f17d22bba15001f/oasgen-core-0.21.1/src/schema.rs:34:27
   4: oasgen_core::schema::axum::<impl oasgen_core::schema::OaSchema for axum::extract::path::Path<T>>::parameters
             at /home/bertrand/.cargo/registry/src/index.crates.io-6f17d22bba15001f/oasgen-core-0.21.1/src/schema/axum.rs:79:45

I digged a bit in the source code and expanded the #[oasgen] macro in my code and found that the issue come from there

    fn parameters() -> Vec<ReferenceOr<oa::Parameter>> {
        let p = oa::Parameter::path("path", T::schema_ref());
        vec![ReferenceOr::Item(p)]
    }

where T is a tuple... And as the error says, we are not supposed to call schema() on a tuple... Are tuple supported anyhow ?

Thank you for your help !

BertrandD commented 2 months ago

here is a test to reproduce the error:

use axum::extract::{Json, Path};
use oasgen::{oasgen, OaSchema, Server};
use serde::Deserialize;

/// Send a code to a mobile number
#[derive(Deserialize, OaSchema)]
pub struct TaskFilter {
    pub completed: bool,
    pub assigned_to: i32,
}

#[oasgen]
async fn get_task(Path(_id): Path<u64>) -> Json<()> {
    Json(())
}

#[oasgen]
async fn get_stuff(Path((_id, _tu)): Path<(u64, u64)>) -> Json<()> {
    Json(())
}

fn main() {
    use pretty_assertions::assert_eq;
    let server = Server::axum()
        .get("/tasks/:id/", get_task)
        .get("/stuff/:id/:tu", get_stuff);

    let spec = serde_yaml::to_string(&server.openapi).unwrap();
    let other = include_str!("03-path.yaml");
    assert_eq!(spec.trim(), other);
    let router = axum::Router::new().merge(server.freeze().into_router());
    router.into_make_service();
}
kurtbuilds commented 2 months ago

Thanks for catching this.

I went through some refactors earlier this year that broke this. The fix is to remove the implementation for (A1, A2): OaSchema so that the blanket Path doesn't include tuples, and then individually re-implement Path<(A1, A2)>. A tuple reduces code dup.

Technically this is a breaking change, but because this was broken, I don't think it was plausibly being used. I cut as core=0.21.2 so the fix gets automatically pulled in for most people. Pulling that in should just be a cargo update.

BertrandD commented 2 months ago

Hi ! Thank you for the quick fix ! I can get only the 0.21.1, and it fails compile with error " no impl_parameters in the root", but it seems you already fixed it. Did you publish 0.21.2 ?

kurtbuilds commented 2 months ago

If you update oasgen itself (not core) as well, that should be fixed.

I'm realizing though that this approach might not work because it's causing a downstream probably. Might have another revision out later today.

kurtbuilds commented 2 months ago

I don't think fully fixing this was compatible possible without making breaking changes.

I split OaSchema into OaSchema & OaParameter, which I've been meaning to do for a while.

The rule of thumb for implementation is, implement OaSchema for "data types" and OaParameter for what I'll call "extractor types" - types that tell axum/actix/etc where to pull data out from the request or place it in the response.

Released the new version as 0.22. It has breaking changes for more advanced library usage, but the basics should be totally unchanged.

Let me know if you encounter any issues.