search

juhaku / utoipa

Simple, Fast, Code first and Compile time generated OpenAPI documentation for Rust
Apache License 2.0
2.48k stars 197 forks source link

Generic type parameters are always inlined #1182

Closed JMLX42 closed 1 week ago

JMLX42 commented 2 weeks ago

Example

#[derive(ToSchema)]
pub struct ResponsePayload<T: ToSchema> {
    data: Vec<T>,
}

#[derive(ToSchema)]
pub struct PrimaryData {
    id: String,
    value: u32,
}

Expected definition

{
  "components": {
    "schemas": {
      "PrimaryData": {
        "properties": {
          "id": {
            "type": "string"
          },
          "value": {
            "format": "int32",
            "minimum": 0,
            "type": "integer"
          }
        },
        "required": [
          "id",
          "value"
        ],
        "type": "object"
      },
      "ResponsePayload_PrimaryData": {
        "properties": {
          "data": {
            "items": {
              "$ref": "#/components/schemas/PrimaryData"
            },
            "type": "array"
          }
        },
        "required": [
          "data"
        ],
        "type": "object"
      }
    }
  },
  "info": {
    "description": "",
    "license": {
      "name": ""
    },
    "title": "utoipa-generic-response",
    "version": "0.1.0"
  },
  "openapi": "3.1.0",
  "paths": {}
}

Actual definition

{
  "components": {
    "schemas": {
      "PrimaryData": {
        "properties": {
          "id": {
            "type": "string"
          },
          "value": {
            "format": "int32",
            "minimum": 0,
            "type": "integer"
          }
        },
        "required": [
          "id",
          "value"
        ],
        "type": "object"
      },
      "ResponsePayload_PrimaryData": {
        "properties": {
          "data": {
            "items": {
              "properties": {
                "id": {
                  "type": "string"
                },
                "value": {
                  "format": "int32",
                  "minimum": 0,
                  "type": "integer"
                }
              },
              "required": [
                "id",
                "value"
              ],
              "type": "object"
            },
            "type": "array"
          }
        },
        "required": [
          "data"
        ],
        "type": "object"
      }
    }
  },
  "info": {
    "description": "",
    "license": {
      "name": ""
    },
    "title": "utoipa-generic-response",
    "version": "0.1.0"
  },
  "openapi": "3.1.0",
  "paths": {}
}
25,39c25
<               "properties": {
<                 "id": {
<                   "type": "string"
<                 },
<                 "value": {
<                   "format": "int32",
<                   "minimum": 0,
<                   "type": "integer"
<                 }
<               },
<               "required": [
<                 "id",
<                 "value"
<               ],
<               "type": "object"
---
>               "$ref": "#/components/schemas/PrimaryData"

@juhaku is this expected? Is there a workaround? Where should I start looking if I wanted to be able to fix this or provide an alternative?

What's surprising is that traits associated types (ex: T::SomeType) are not inlined. Yet they very much depend on T.

juhaku commented 2 weeks ago

That is expected behavior indeed. There are some places where the Inline feature is enforced, so you might keep that in mind. But what really breaks the bank here is in the component.rs compose_generics function if I remember right. It recursively calls PartialSchema::schema() for all arguments eventually. And is not able to create any references. For it to be able to create references instead would probably need some changes, or perhaps changes should be done before calling this function so that it will not execute the compose_generics function at all.

JMLX42 commented 2 weeks ago

compose_generics function if I remember right. It recursively calls PartialSchema::schema() for all arguments eventually. And is not able to create any references.

@juhaku looks like it's what I was looking for!

It's weird that it inlines T but not T::SomeAssociatedType. I'll start to understand why that is and see if I can avoid inlining T.

JMLX42 commented 2 weeks ago

The generated code:

    impl<T: ToSchema> utoipa::__dev::ComposeSchema for ResponsePayload<T>
    where
        T: utoipa::ToSchema,
    {
        fn compose(
            mut generics: Vec<utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>>,
        ) -> utoipa::openapi::RefOr<utoipa::openapi::schema::Schema> {
            {
                let mut object = utoipa::openapi::ObjectBuilder::new();
                object = object
                    .property(
                        "data",
                        utoipa::openapi::schema::ArrayBuilder::new()
                            .items({
                                let _ = <T as utoipa::PartialSchema>::schema;
                                if let Some(composed) = generics.get_mut(0usize) {
                                    std::mem::take(composed)
                                } else {
                                    utoipa::openapi::schema::RefBuilder::new()
                                        .ref_location_from_schema_name(
                                            ::alloc::__export::must_use({
                                                let res = ::alloc::fmt::format(
                                                    format_args!("{0}", <T as utoipa::ToSchema>::name()),
                                                );
                                                res
                                            }),
                                        )
                                        .into()
                                }
                            }),
                    )
                    .required("data");
                object
            }
                .into()
        }
    }

IMO it all comes down to the result of generics.get_mut(0usize):

In my case, generics[0] is:

T(
        Object(
            Object {
                schema_type: Type(
                    Object,
                ),
                title: None,
                format: None,
                description: None,
                default: None,
                enum_values: None,
                required: [
                    "id",
                    "value",
                ],
                properties: {
                    "id": T(
                        Object(
                            Object {
                                schema_type: Type(
                                    String,
                                ),
                                title: None,
                                format: None,
                                description: None,
                                default: None,
                                enum_values: None,
                                required: [],
                                properties: {},
                                additional_properties: None,
                                property_names: None,
                                deprecated: None,
                                example: None,
                                examples: [],
                                write_only: None,
                                read_only: None,
                                xml: None,
                                multiple_of: None,
                                maximum: None,
                                minimum: None,
                                exclusive_maximum: None,
                                exclusive_minimum: None,
                                max_length: None,
                                min_length: None,
                                pattern: None,
                                max_properties: None,
                                min_properties: None,
                                extensions: None,
                                content_encoding: "",
                                content_media_type: "",
                            },
                        ),
                    ),
                    "value": T(
                        Object(
                            Object {
                                schema_type: Type(
                                    Integer,
                                ),
                                title: None,
                                format: Some(
                                    KnownFormat(
                                        Int32,
                                    ),
                                ),
                                description: None,
                                default: None,
                                enum_values: None,
                                required: [],
                                properties: {},
                                additional_properties: None,
                                property_names: None,
                                deprecated: None,
                                example: None,
                                examples: [],
                                write_only: None,
                                read_only: None,
                                xml: None,
                                multiple_of: None,
                                maximum: None,
                                minimum: Some(
                                    Float(
                                        0.0,
                                    ),
                                ),
                                exclusive_maximum: None,
                                exclusive_minimum: None,
                                max_length: None,
                                min_length: None,
                                pattern: None,
                                max_properties: None,
                                min_properties: None,
                                extensions: None,
                                content_encoding: "",
                                content_media_type: "",
                            },
                        ),
                    ),
                },
                additional_properties: None,
                property_names: None,
                deprecated: None,
                example: None,
                examples: [],
                write_only: None,
                read_only: None,
                xml: None,
                multiple_of: None,
                maximum: None,
                minimum: None,
                exclusive_maximum: None,
                exclusive_minimum: None,
                max_length: None,
                min_length: None,
                pattern: None,
                max_properties: None,
                min_properties: None,
                extensions: None,
                content_encoding: "",
                content_media_type: "",
            },
        ),
    ),
]

Therefore: it is inlined.

My understanding is that since it's in generics, it cannot be resolved to a "real" type. So a $ref would be a to #/components/schemas/T. Which does not make much sense indeed.

But when I do this:

#[derive(OpenApi)]
#[openapi(components(schemas(ResponsePayload<PrimaryData>, PrimaryData)))]
struct ApiDoc;

I would expect the schema generation code to have access to the PrimaryData type parameter in ResponsePayload<PrimariData>. Is that not possible @juhaku ?

Because if that is possible, then it would make sense to create a $ref to PrimaryData.

JMLX42 commented 2 weeks ago

So I have a (really funny IMHO) workaround based on the fact that T is inlined bu T::SomeAssociatedType is not:

pub trait RefToSelf {
    type Ref: RefToSelf;
}

#[derive(ToSchema)]
pub struct ResponsePayload<T: ToSchema + RefToSelf<Ref = T>> {
    data: Vec<T::Ref>,
}

#[derive(ToSchema)]
pub struct PrimaryData {
    id: String,
    value: u32,
}

impl RefToSelf for PrimaryData {
    type Ref = Self;
}
{
  "components": {
    "schemas": {
      "PrimaryData": {
        "properties": {
          "id": {
            "type": "string"
          },
          "value": {
            "format": "int32",
            "minimum": 0,
            "type": "integer"
          }
        },
        "required": [
          "id",
          "value"
        ],
        "type": "object"
      },
      "ResponsePayload_PrimaryData": {
        "properties": {
          "data": {
            "items": {
              "$ref": "#/components/schemas/PrimaryData"
            },
            "type": "array"
          }
        },
        "required": [
          "data"
        ],
        "type": "object"
      }
    }
  },
  "info": {
    "description": "",
    "license": {
      "name": ""
    },
    "title": "utoipa-generic-response",
    "version": "0.1.0"
  },
  "openapi": "3.1.0",
  "paths": {}
}
juhaku commented 2 weeks ago

Pretty nifty indeed.

I would expect the schema generation code to have access to the PrimaryData type parameter in ResponsePayload<PrimariData>. Is that not possible @juhaku ?

Yes, I think it should have access to it but it is only in form of inline only otherwise since it is able to call the compose for the PrimaryData field. Perhaps if you remove the inline enforcement from the MediaType get_component_schema function call it might make it a reference. But as you stated earlier #/components/schemas/T would not make reference. And it is true, that's why the inline is being enforced there to not to create invalid references.

In any case this change to make references instead of inlining needs to be behind a feature flag or a config option, maybe. :thinking:

juhaku commented 2 weeks ago

It's weird that it inlines T but not T::SomeAssociatedType. I'll start to understand why that is and see if I can avoid inlining T.

That is weird indeed, I haven't tested it with the associated types but that might be because the associated types comes from the type itself and are created when ToSchema derive is executed for the type and not when it is used as a request_body or response body.

In general the references are created when ToSchema derive executes. And the inlining happens when it is being used as request body or response body, etc. This adds to the complexity of ComponentSchema for it is used in both instances to create a schema for TypeTree of rust type definition.

JMLX42 commented 2 weeks ago

Pretty nifty indeed.

It's worth nothing that it works with std's Deref also without a custom trait. But that's not something we want.

In any case this change to make references instead of inlining needs to be behind a feature flag or a config option, maybe. 🤔

@juhaku can I make ToSchema implement RefToSelf? Or directly have the required associated type in ToSchema. This way, we would always use refs (unless #[schema(inline)] is specified of course).

I could hide this additional trait behind a feature flag.

JMLX42 commented 2 weeks ago

Using the trick above, the impl of ComposeSchema for ResponsePayload<T> is completely different, presumably because T::Ref is a "known" type, not a type parameter:

    impl<T: ToSchema + RefToSelf<Target = T>> utoipa::__dev::ComposeSchema
    for ResponsePayload<T>
    where
        T: utoipa::ToSchema,
    {
        fn compose(
            mut generics: Vec<utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>>,
        ) -> utoipa::openapi::RefOr<utoipa::openapi::schema::Schema> {
            {
                let mut object = utoipa::openapi::ObjectBuilder::new();
                object = object
                    .property(
                        "data",
                        utoipa::openapi::schema::ArrayBuilder::new()
                            .items(
                                utoipa::openapi::schema::RefBuilder::new()
                                    .ref_location_from_schema_name(
                                        ::alloc::__export::must_use({
                                            let res = ::alloc::fmt::format(
                                                format_args!("{0}", <T::Target as utoipa::ToSchema>::name()),
                                            );
                                            res
                                        }),
                                    ),
                            ),
                    )
                    .required("data");
                object
            }
                .into()
        }
    }

But they are, at compile time, the same type!

So if only we had a way to resolve T to its actual type... maybe with a special case for unit types such as:

#[derive(ToSchema)]
pub struct MyResponse(ResponsePayload<PrimaryData>);
JMLX42 commented 2 weeks ago

Interestingly, the following change is enough to have the expected output with a ref:

diff --git a/utoipa-gen/src/component.rs b/utoipa-gen/src/component.rs
index 6a0b9b2..3fa5068 100644
--- a/utoipa-gen/src/component.rs
+++ b/utoipa-gen/src/component.rs
@@ -1255,7 +1255,7 @@ impl ComponentSchema {

                         schema.to_tokens(tokens);
                     } else {
-                        let index = container.generics.get_generic_type_param_index(type_tree);
+                        let index: Option<usize> = None;
                         // only set schema references tokens for concrete non generic types
                         if index.is_none() {
                             let reference_tokens = if let Some(children) = &type_tree.children
{
  "components": {
    "schemas": {
      "PrimaryData": {
        "properties": {
          "id": {
            "type": "string"
          },
          "value": {
            "format": "int32",
            "minimum": 0,
            "type": "integer"
          }
        },
        "required": [
          "id",
          "value"
        ],
        "type": "object"
      },
      "ResponsePayload_PrimaryData": {
        "properties": {
          "data": {
            "items": {
              "$ref": "#/components/schemas/PrimaryData"
            },
            "type": "array"
          }
        },
        "required": [
          "data"
        ],
        "type": "object"
      }
    }
  },
  "info": {
    "description": "",
    "license": {
      "name": ""
    },
    "title": "utoipa-generic-response",
    "version": "0.1.0"
  },
  "openapi": "3.1.0",
  "paths": {}
}

Which is both valid and what one would want.

@juhaku why are the generic types inlined then? Just to make sure their actual value doesn't have to be manually added as a schema like I do it?

#[derive(OpenApi)]
#[openapi(components(schemas(PrimaryData, ResponsePayload<PrimaryData>)))]
struct ApiDoc;

Or are there other edge cases I am missing?

Anyway, is it OK to simply set index = None when a specific cargo feature is enabled? It is certainly less hacky than my previous associated type workaround.

JMLX42 commented 2 weeks ago

I have added the force_ref_to_type_parameters Cargo feature:

diff --git a/utoipa-gen/Cargo.toml b/utoipa-gen/Cargo.toml
index d6de157..542ed42 100644
--- a/utoipa-gen/Cargo.toml
+++ b/utoipa-gen/Cargo.toml
@@ -66,6 +66,7 @@ repr = []
 indexmap = []
 rc_schema = []
 config = ["dep:utoipa-config", "dep:once_cell"]
+force_ref_to_type_parameters = []

 # EXPERIEMENTAL! use with cauntion
 auto_into_responses = []
diff --git a/utoipa-gen/src/component.rs b/utoipa-gen/src/component.rs
index 6a0b9b2..a8839f9 100644
--- a/utoipa-gen/src/component.rs
+++ b/utoipa-gen/src/component.rs
@@ -1255,7 +1255,10 @@ impl ComponentSchema {

                         schema.to_tokens(tokens);
                     } else {
+                        #[cfg(not(feature = "force_ref_to_type_parameters"))]
                         let index = container.generics.get_generic_type_param_index(type_tree);
+                        #[cfg(feature = "force_ref_to_type_parameters")]
+                        let index: Option<usize> = None;
                         // only set schema references tokens for concrete non generic types
                         if index.is_none() {
                             let reference_tokens = if let Some(children) = &type_tree.children {

As explained above, it disables forced inlining for generics type parameters.

IMHO what's very cool - and that's what I am trying to achieve - is that it makes it possible to specialize generics to a new schema using the newtype + #[schema(inline)] pattern:

#[derive(ToSchema)]
pub struct MyResponse(#[schema(inline)] ResponsePayload<PrimaryData>);
{
  "components": {
    "schemas": {
      "MyResponse": {
        "properties": {
          "data": {
            "items": {
              "$ref": "#/components/schemas/PrimaryData"
            },
            "type": "array"
          }
        },
        "required": [
          "data"
        ],
        "type": "object"
      },
      "PrimaryData": {
        "properties": {
          "id": {
            "type": "string"
          },
          "value": {
            "format": "int32",
            "minimum": 0,
            "type": "integer"
          }
        },
        "required": [
          "id",
          "value"
        ],
        "type": "object"
      }
    }
  },
  "info": {
    "description": "",
    "license": {
      "name": ""
    },
    "title": "utoipa-generic-response",
    "version": "0.1.0"
  },
  "openapi": "3.1.0",
  "paths": {}
}

@juhaku IMO that's a good solution. Shall I make a PR for this?

Update: in my case it removes a ton of code generated by my macro. And it's Rust idiomatic. Specializing generics with newtype makes a tone of sense.

juhaku commented 2 weeks ago

@juhaku can I make ToSchema implement RefToSelf? Or directly have the required associated type in ToSchema. This way, we would always use refs (unless #[schema(inline)] is specified of course).

You can use the bound attribute to change the bounds of a type. You can find more in the docs https://docs.rs/utoipa/latest/utoipa/derive.ToSchema.html#examples. Almost very bottom there is example on how to use the bound attribute.

@juhaku why are the generic types inlined then? Just to make sure their actual value doesn't have to be manually added as a schema like I do it?

It is inlined in order to resolve correct type in every case. Also what I thought is that when generic types are used there is no need to care what the actual generic arg is but it could be just inlined directly there as in short of that generic type is form of wrapper or a factory that forms concrete types when the generic arguments are resolved.

Anyway, is it OK to simply set index = None when a specific cargo feature is enabled? It is certainly less hacky than my previous associated type workaround.

One thing to make sure here is that what happens if you try to use a primitive type as an argument. I have a hunch that for such cases the inlining e.g. setting index = None is not sufficient. But maybe it works, you an try it out, if you try to use number and string as arguments.

JMLX42 commented 2 weeks ago

Maybe there is a 3rd solution that is even cleaner:

So my original code would become:

#[derive(ToSchema)]
pub struct ResponsePayload<T: ToSchema> {
    #[schema(inline = false)]
    data: Vec<T>,
}

#[derive(ToSchema)]
pub struct PrimaryData {
    id: String,
    value: u32,
}

IMO it's a good solution because:

@juhaku if that makes sense, I'll open a PR.

JMLX42 commented 2 weeks ago

Add support for #[schema(inline[ = bool | path_to_bool_fn])] like it's done for #[schema(ignore)] (cf https://github.com/juhaku/utoipa/pull/1177).

#[schema(inline = false)] is already supported! But the following code still inlines the type parameter:

#[derive(ToSchema)]
pub struct ResponsePayload<T: ToSchema> {
    #[schema(inline = false)]
    data: Vec<T>,
}

#[derive(ToSchema)]
pub struct PrimaryData {
    id: String,
    value: u32,
}

So IMO that's not the expected behavior and quite misleading.

juhaku commented 2 weeks ago

Yeah, the inline is only considered when ToSchema derive macro is executed for the type. But when the type is used with generic arguments there is no access to the #[schema(inline)] attribute of the type's field.

JMLX42 commented 2 weeks ago

Yeah, the inline is only considered when ToSchema derive macro is executed for the type. But when the type is used with generic arguments there is no access to the #[schema(inline)] attribute of the type's field.

@juhaku I'm not sure what you mean.

What I tested is:

#[derive(ToSchema)]
pub struct ResponsePayload<T: ToSchema> {
    #[schema(inline = true)]
    data: Vec<T>,
}

vs

#[derive(ToSchema)]
pub struct ResponsePayload<T: ToSchema> {
    #[schema(inline = false)]
    data: Vec<T>,
}

And it generates exactly the same code. Yet as far as I can tell it enters the if is_inline branch when #[schema(inline = true)] is used.

So I am a bit lost now...

juhaku commented 2 weeks ago

I mean with inline = true it generates schema like this: It directly calls <T as PartialSchema>::schema() inlining T to array.

impl<T: ToSchema> utoipa::__dev::ComposeSchema for ResponsePayload<T>
where
    T: utoipa::ToSchema,
{
    fn compose(
        mut generics: Vec<utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>>,
    ) -> utoipa::openapi::RefOr<utoipa::openapi::schema::Schema> {
        {
            let mut object = utoipa::openapi::ObjectBuilder::new();
            object = object
                .property(
                    "data",
                    utoipa::openapi::schema::ArrayBuilder::new()
                        .items(<T as utoipa::PartialSchema>::schema()), // <--- here
                )
                .required("data");
            object
        }
        .into()
    }
}

And with inline = false it generates this: Here it either creates a reference to the T or a generic argument if generic arg has been defined.

impl<T: ToSchema> utoipa::__dev::ComposeSchema for ResponsePayload<T>
where
    T: utoipa::ToSchema,
{
    fn compose(
        mut generics: Vec<utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>>,
    ) -> utoipa::openapi::RefOr<utoipa::openapi::schema::Schema> {
        {
            let mut object = utoipa::openapi::ObjectBuilder::new();
            object = object
                .property(
                    "data",
                    utoipa::openapi::schema::ArrayBuilder::new().items({
                        let _ = <T as utoipa::PartialSchema>::schema;
                        if let Some(composed) = generics.get_mut(0usize) {
                            std::mem::take(composed)
                        } else {
                            utoipa::openapi::schema::RefBuilder::new()
                                .ref_location_from_schema_name(format!(
                                    "{}",
                                    <T as utoipa::ToSchema>::name()
                                ))
                                .into()
                        }
                    }),
                )
                .required("data");
            object
        }
        .into()
    }
}

And it generates exactly the same code. Yet as far as I can tell it enters the if is_inline branch when #[schema(inline = true)] is used.

The reason why it goes to if inline branch is, in response body and request_body as well as in openapi(components(schemas(...))) the Inline feature has been enforced so that it would generate correct schema for the generic argument inlining it directly with the compose_gnerics function in component.rs

If you use the ResponsePayload<String> for example as an argument to the OpenApi derive like so:

#[derive(OpenApi)]
#[openapi(components(schemas(ResponsePayload<String>)))]
struct Api;

You will get following output. Note the ResponsePayload schema call. In order to compose the ResponsePayload_String type it goes to the inline branch and composes the generic arguments recursively as arguments for the ComposeSchema implementations. Those implementations will then use the logic as can be shown in above ComposeSchema implementation to get the correct type from the generic arguments of the ResponsePayload.

                        <ResponsePayload<String> as utoipa::__dev::ComposeSchema>::compose(
                            [<String as utoipa::PartialSchema>::schema()].to_vec(),
                        ),
impl utoipa::OpenApi for Api {
    fn openapi() -> utoipa::openapi::OpenApi {
        use utoipa::{Path, ToSchema};
        let mut openapi = utoipa::openapi::OpenApiBuilder::new()
            .info(
                // ... omitted 
            )
            .paths({ utoipa::openapi::path::PathsBuilder::new() })
            .components(Some(
                utoipa::openapi::ComponentsBuilder::new()
                    .schemas_from_iter({
                        let mut schemas = Vec::<(
                            String,
                            utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>,
                        )>::new();
                        <ResponsePayload<String> as utoipa::ToSchema>::schemas(&mut schemas);
                        schemas
                    })
                    .schema(
                        std::borrow::Cow::Owned(format!(
                            "{}_{}",
                            <ResponsePayload<String> as utoipa::ToSchema>::name(),
                            std::borrow::Cow::<String>::Owned(
                                [<String as utoipa::ToSchema>::name(),].to_vec().join("_")
                            )
                        )),
                        <ResponsePayload<String> as utoipa::__dev::ComposeSchema>::compose(
                            [<String as utoipa::PartialSchema>::schema()].to_vec(),
                        ),
                    )
                    .build(),
            ))
            .build();
        let components = openapi
            .components
            .get_or_insert(utoipa::openapi::Components::new());
        let mut schemas = Vec::<(
            String,
            utoipa::openapi::RefOr<utoipa::openapi::schema::Schema>,
        )>::new();
        components.schemas.extend(schemas);
        let _mods: [&dyn utoipa::Modify; 0usize] = [];
        _mods
            .iter()
            .for_each(|modifier| modifier.modify(&mut openapi));
        openapi
    }
}

Maybe the Inline enforcement can be set configurable for components(schemas(...)) definition and as well to request_body and responses body definition in #[utoipa::path(...)]

JMLX42 commented 2 weeks ago

the Inline feature has been enforced so that it would generate correct schema for the generic argument inlining it directly with the compose_gnerics function in component.rs

@juhaku where is that done?

juhaku commented 2 weeks ago

For component it is done in openapi.rs https://github.com/juhaku/utoipa/blob/c45215c303ab288ed132a487c4b86b00b9513573/utoipa-gen/src/openapi.rs#L151-L163

For request_body and response body schema references it is done here in media_type.rs https://github.com/juhaku/utoipa/blob/c45215c303ab288ed132a487c4b86b00b9513573/utoipa-gen/src/path/media_type.rs#L287-L293

juhaku commented 2 weeks ago

But maybe for schema references it can just be inline enforced? :thinking:

JMLX42 commented 2 weeks ago

For component it is done in openapi.rs

I tried the following:

impl Schema {
    fn get_component(&self) -> Result<ComponentSchema, Diagnostics> {
        let ty = syn::Type::Path(self.0.clone());
        let type_tree = TypeTree::from_type(&ty)?;
        let generics = type_tree.get_path_generics()?;

        let container = Container {
            generics: &generics,
        };
        let component_schema = ComponentSchema::new(crate::component::ComponentSchemaProps {
            container: &container,
            type_tree: &type_tree,
            features: vec![],
            description: None,
        })?;

        Ok(component_schema)
    }
}

Now #[schema(inline = true)] is ignored entirely on any field.

But with the original code, #[schema(inline)] is supported as expected. Except for the fields types with generics' type parameter (ex: Vec<T>).

But maybe for schema references it can just be inline enforced? 🤔

You were right: this solution does not work for native types (ex: u32): it will create a $ref to #/components/schemas/u32.

IMHO the behavior should be as follow:

But I have no idea how to tackle this since:

  1. features: vec![], in openapi.rs kinda breaks #[schema(inline)] entirely.
  2. I'm not sure how to check for a native type.

I will eventually find for 2. I've seen some code around already in the code base.

But 1. is bugging me... Any help understanding this would be greatly appreciated.

juhaku commented 2 weeks ago

Yup,

  1. is really simple you can use SchemaType to check is it primitive SchemaType { path, is_inline }.is_primitive() Then based on that inline or not But that should also happen within ComponentSchema as well because if it only happens here in openapi.rs it will work only of the immediate child schema but not for inner generic schema. I guess this needs some debugging indeed to see how it behaves.
juhaku commented 2 weeks ago

Ah, true for the 1. vec![] in openapi.rs it cannot be done there. And if the Inline is there it will get to the if inline branch in ComponentSchema anyways.

Only place then where changes can be made with the current behavior is that branch itself. Or there is perhaps need for some architectural changes for better support for different type of generic args.

JMLX42 commented 2 weeks ago

@juhaku can we agree on this:

IMHO the behavior should be as follow:

  • If it's a native type, force inline = true except if #[schema(inline = false)] is specified (because if it is specified, maybe someone does actually want to make their own u32 schema?).
  • If #[schema(inline = ...)] is specified, use it.
  • Otherwise, assume #[schema(inline = true)] (in order to keep the existing behavior).

Then we'll see what needs to be changed.

juhaku commented 2 weeks ago

@JMLX42 Yeah, seems quite right to me.

JMLX42 commented 2 weeks ago

Ok cool 😎

Now how comes this patch works when it's in the else branch?

juhaku commented 2 weeks ago

I believe the else branch will only get executed when ToSchema executes for the type, It will never get to the else branch when type is declared as response body or request body or within #[openapi(components(schemas(...)))].

JMLX42 commented 2 weeks ago

Ok so:

So I guess a first step would be to change the else branch to skip inlining except for native types.

juhaku commented 2 weeks ago

So I guess a first step would be to change the else branch to skip inlining except for native types.

Yup, that should be quite simple. I think you could do something like this in the else block:

let is_primitive = SchemaType { path: Cow::Borrowed(&rewritten_path), nullable: false}.is_primitive();
if index.is_none() && !is_primitive {...}