utoipa icon indicating copy to clipboard operation
utoipa copied to clipboard

OpenAPI schema generation silently allows missing schemas

Open jayvdb opened this issue 1 year ago • 12 comments

I am using the OpenAPI schema generation, and the generated OAS doc for the following includes schema SchemaA correctly, however it refers to SchemaB which isn't present in the generated OAS, making it invalid. There should be some failure to indicate that SchemaA cant be included in the OAS without SchemaB.

#[derive(OpenApi)]
#[openapi(
    security(
        ("api_key" = [])
    ),

    paths(
        foo::bar,
    ),
    components(
        schemas(
            SchemaA,
        ),
    )
)]
pub struct ApiDoc;

Adding SchemaB in the above solves the problem.

This also occurred in the 2.x release, so it isnt a regression.

jayvdb avatar Jan 27 '23 11:01 jayvdb

This is something that indeed needs some thought. That is, there is currently no way to know what schemas have been added to the OpenAPI and what are not. I am planning to investigate possibility to automatically add these to the schema but there are no guarantees whether it is possible in any feasible way.

Difficulty here comes from that OpenApi derive macro does not know and it has no access to the internals of the types so it does not keep track on what has been added and whether some schema depends on another schema.

Only thing I can think of is if ToSchema types are stored in one central location and then the check could be done within ToSchema derive macro instead by checking whether schema was added there or not. Then the macro can emit error for that specific field itself.

Yet if is possible have schemas automatically added to the OpenApi then there would be no need for this kind of check.

But if possible it would be good to have such an safety mechanism to check whether schema is correct or not.

juhaku avatar Jan 27 '23 12:01 juhaku

I think what you'd have to do is thread a registry of some sort, through the path_item invocations (or have a separate function that you generate like: schemas(reg: &mut Registry). The registry is essentially just a HashMap<std::any::TypeId, Schema>. Then when you build the #[openapi] you'll need to invoke the schemas fn for all the paths involved. You'd also need a similar schemas function for things that implement IntoParams and IntoResponses. Those functions would be invoked by the schemas function for the path.

djrenren avatar Feb 02 '23 21:02 djrenren

I guess this could be less awful by just having a trait HasSchemas and impl it for everything that would ref or contain a schema.

djrenren avatar Feb 02 '23 21:02 djrenren

Yeah, I was considering something like a centralized store for ToSchema and other derive macro results as well what then could added to the added to the OpenApi.

I guess this could be less awful by just having a trait HasSchemas and impl it for everything that would ref or contain a schema.

@djrenren Could you elaborate a bit on this. If this trait would be implemented in compile time same time with ToSchema when it should be called? I guess somewhere before the Modify trait is being called.

juhaku avatar Mar 14 '23 00:03 juhaku

yeah basically right here:

https://github.com/juhaku/utoipa/blob/2986e5aa285c3a562f7c6c5d39c39a4e0984f8c8/utoipa-gen/src/openapi.rs#L442

I'm not sure what would happen if there were conflicts between manually described components and auto-discovered ones. probably would just want to error out at instantiation.

djrenren avatar Mar 14 '23 01:03 djrenren

Yeah, here it is a runtime panic and error cannot be targeted to specific span. Unless the spans are collected as well. Though the question remains that how to deal with manually defined components.

juhaku avatar Mar 14 '23 07:03 juhaku

The problem would also be solved by collecting all the types that need to be referenced in components automatically. Is that possible?

Kinrany avatar May 08 '23 17:05 Kinrany

The problem would also be solved by collecting all the types that need to be referenced in components automatically. Is that possible?

Yes that is possible, but there is a but. That only will work to the extend of single crate. I have been planning to do something like this for other purposes and it could be leveraged here as well but I haven't got a hold of it yet. While I haven't tested it with cross crate references I don't belive it will work because they are seprate processes does not know anything about each others existense. In such cases there still is a need to manually point a schema location what to register as a schema. This kind of situation is very common when API is bigger than a simple demo API.

But if cross crate references are possible, that would make it a lot simplier. Perhaps I need to test it out sometime.

juhaku avatar May 08 '23 19:05 juhaku

I don't think the API should use any kind of global registration mechanism, to be clear. It should be enough to use the recursive nature of types with derived traits. If we could somehow collect types from the endpoint handler functions (which we already list out for schema declarations), that should allow for many schemas to exist in parallel.

Kinrany avatar May 08 '23 21:05 Kinrany

Note https://github.com/rxdiscovery/utoipa_auto_discovery by @rxdiscovery is planning to solve this, but hasnt done so yet.

jayvdb avatar Jun 12 '23 02:06 jayvdb

I found a similar problem with requestBody in path: type was renamed but utoipa doesn't complain about that in any way.

qrilka avatar Apr 03 '24 12:04 qrilka