OpenAPI schema generation silently allows missing schemas

[jayvdb]

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.

[juhaku]

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.

[djrenren]

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]

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.

[juhaku]

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.

[djrenren]

yeah basically right here:

utoipa/utoipa-gen/src/openapi.rs

Line 442 in 2986e5a

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.

[juhaku]

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.

[Kinrany]

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

[juhaku]

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.

[Kinrany]

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.

[jayvdb]

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

[qrilka]

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

[jfrader]

Is there anyway to make this complain now ? Its very hard to trust code reviews would catch this before its consumed by another project like in my case. I even tried to use a different library just for validation on the outputted JSON schema but failed. Maybe someone has a better idea?

[juhaku]

Related #1025

[juhaku]

Is there anyway to make this complain now ? Its very hard to trust code reviews would catch this before its consumed by another project like in my case. I even tried to use a different library just for validation on the outputted JSON schema but failed. Maybe someone has a better idea?

@jfrader In master there is version that actually enforces the types to exist before they can be added to the OpenApi or to #[utoipa::path(...)] macro. This in full probably will land eventually in utoipa 5.0.0 release once it is out.

[jayvdb]

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

There are PRs on that repo which do this, and they are available at https://github.com/ProbablyClem/utoipauto which is published at https://crates.io/crates/utoipauto

[juhaku]

I could consider this as done, as all mandatory things for this is implemented excluding support for ToReponse and IntoReponses. Those can be followed in a separate issue and I link the issue here as well.