-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generic type parameters are always inlined #1182
Comments
That is expected behavior indeed. There are some places where the |
@juhaku looks like it's what I was looking for! It's weird that it inlines |
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
In my case,
Therefore: it is inlined. My understanding is that since it's in 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 Because if that is possible, then it would make sense to create a |
So I have a (really funny IMHO) workaround based on the fact that 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": {}
} |
Pretty nifty indeed.
Yes, I think it should have access to it but it is only in form of In any case this change to make references instead of inlining needs to be behind a feature flag or a config option, maybe. 🤔 |
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 In general the references are created when |
It's worth nothing that it works with
@juhaku can I make I could hide this additional trait behind a feature flag. |
Using the trick above, the impl of 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 #[derive(ToSchema)]
pub struct MyResponse(ResponsePayload<PrimaryData>); |
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 |
I have added the 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 + #[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. |
You can use the
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.
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 |
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. |
#[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. |
Yeah, the inline is only considered when |
@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 So I am a bit lost now... |
I mean with 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 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()
}
}
The reason why it goes to If you use the #[derive(OpenApi)]
#[openapi(components(schemas(ResponsePayload<String>)))]
struct Api; You will get following output. Note the <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 |
@juhaku where is that done? |
For component it is done in utoipa/utoipa-gen/src/openapi.rs Lines 151 to 163 in c45215c
For utoipa/utoipa-gen/src/path/media_type.rs Lines 287 to 293 in c45215c
|
But maybe for schema references it can just be inline enforced? 🤔 |
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 But with the original code,
You were right: this solution does not work for native types (ex: IMHO the behavior should be as follow:
But I have no idea how to tackle this since:
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. |
Yup, |
Ah, true for the 1. 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. |
@juhaku can we agree on this:
Then we'll see what needs to be changed. |
@JMLX42 Yeah, seems quite right to me. |
Ok cool 😎 Now how comes this patch works when it's in the |
I believe the else branch will only get executed when |
Ok so:
So I guess a first step would be to change the |
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 {...} |
Example
Expected definition
Actual definition
@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 onT
.The text was updated successfully, but these errors were encountered: