utoipa-axum: silently overwrites schemas on name collisions

[sergiimk]

I'm integrating utoipa-axum into a project where individual handlers are separated into modules with uniform naming of DTO types, e.g.:

mod handler_a {
    #[utoipa::path(
        get,
        path = "/a",
        responses((status = OK, body = ResponseBody)),
    )]
    async fn handler_a() { ... }

    #[derive(utoipa::ToSchema)]
    struct ResponseBody { ... }
}

mod handler_b {
    #[utoipa::path(
        get,
        path = "/b",
        responses((status = OK, body = ResponseBody)),
    )]
    async fn handler_b() { ... }

    #[derive(utoipa::ToSchema)]
    struct ResponseBody { ... }
}

fn router() -> OpenApiRouter {
    OpenApiRouter::new()
        .routes(routes!(handler_a::handler_a))
        .routes(routes!(handler_a::handler_b))
}

When used like this, the resulting openapi.json contains only one ResponseBody schema - last handler wins during the merge.

I can of course use this to rename the schemas:

#[derive(utoipa::ToSchema)]
#[schema(as = ResponseBodyA)]
struct ResponseBody { ... }

Having to rename is reasonable, as they end up in a shared namespace, but the key problem is that utoipa doesn't return errors on name collisions, thus making such mistakes very likely in a large project.

[juhaku]

Yeah while this would be a beneficial to check whether type already exist before adding it the the schemas. It comes with performance cost as it would be then necessary before adding schema to loop over all schemas and check whether same name already exists.

Nonetheless only thing that could be done is either panic!() if same schema already exists, or println!() to warn users of schema with name already exists.

[sergiimk]

Axum panics on conflicting routes, so panicking on schema conflicts is what I would expect too.

loop over all schemas

I'm not familiar with internals of utoipa-axum yet, but if a builder can be modified to use HashMap to make lookups constant time - a small loss of performance at initialization time is something I would gladly accept over the broken API spec that goes unnoticed. In my example I have collisions in top-level schemas, but when you have a lot of nested objects the chance of collisions increases very significantly.

[juhaku]

Right, this needs some experimenting to see what can be done to fix this. This is not just only a utoipa-axum problem, but happens more broadly in utoipa.

[juhaku]

After giving this more thought. I don't think that panic is actually a good idea. You can have type A and type B that both depend on a type C. And with panic when type A is added, all will work just fine but when B is added it would then panic and not let it to be added because C will panic. In short, panic would not let people have a type which depends on the same type.

What we could perhaps do is, not to add already added types. But that is effectively same as adding it. If we added a println statement when ever we find already added type people would find it more annoying than helpful because the information like the case above is redundant and cannot be controlled by the user unless again user changed the C to something else in one of the types.

[sergiimk]

Can the builder that merges schemas into #/components/schemas keep track of the TypeId of the objects that schemas are extracted from? I think this would provide the most reliable way to deduplicate schemas and at the same time notice name collisions and panic.

[juhaku]

Perhaps the TypeId could be used there, but it does not still remove the point I made earlier:

You can have type A and type B that both depend on a type C. And with panic when type A is added, all will work just fine but when B is added it would then panic and not let it to be added because C will panic. In short, panic would not let people have a type which depends on the same type.

If it was to panic on existing type ids, the behavior would not allow using types with the same references as some other type might have as a request_body or as response body.

Personally I don't think that it is very good to limit users to allow only new types as schema references disallowing completely reuse of types implementing ToSchema.