Skip to content
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

Support for generics? #108

Closed
oliver-impero opened this issue Apr 26, 2022 · 14 comments · Fixed by #109, #124 or #144
Closed

Support for generics? #108

oliver-impero opened this issue Apr 26, 2022 · 14 comments · Fixed by #109, #124 or #144

Comments

@oliver-impero
Copy link

I have a struct like this:

#[derive(Debug, Serialize, Component)]
pub struct Foo<D, R> {
    pub data: D,
    pub resources: R,
}

And I want to use it like this with a rocket request handler:

#[utoipa::path(
    get,
    context_path = "/api/v1",
    responses(
        (status = 200, description = "Foo found", body = Foo<Bar, BarResources>),
        (status = 404, description = "Foo was not found")
    ),
    params(
        ("id" = Uuid, description = "Foo id"),
    )
)]
#[get("/foo/<id>")]

I'm getting multiple errors:

  • Uuid is not a Component. I'm forced to write String instead. I wish I could write Uuid because it makes the doc clearer.
  • rustc complains about the <, it expects a comma. So I guess the proc-macro can't handle generics yet, is this planned? :)

I tried this instead:

pub type GetFooBody = Foo<Bar, BarResources>;

and then

(status = 200, description = "Foo found", body = GetFooBody),

then it compiles but I get an error in the browser when browsing the swagger doc:

Resolver error at paths./api/v1/foo/{id}.get.responses.200.content.application/json.schema.properties.data.$ref
Could not resolve reference: Could not resolve pointer: /components/schemas/D does not exist in document
Resolver error at paths./api/v1/foo/{id}.get.responses.200.content.application/json.schema.properties.resources.$ref
Could not resolve reference: Could not resolve pointer: /components/schemas/R does not exist in document

This refers to the type params D and R. Even though both Bar and BarResources are Component, the proc-macro doesn't treat them as expected. And in the swagger UI, it shows both fields' type as string :(

{
  "data": "string",
  "resources": "string"
}

Is it possible / planned to add support for generics? :)
So that I don't have to write a second set of types just for the doc, but can use the actual types that I'm using in the request handlers also for the doc.

@juhaku
Copy link
Owner

juhaku commented Apr 26, 2022

Hi,

This seems to touch the same topic already discussed here #101. But in short currently there is a limited support for generics. Since this is library is compile time and more over macro compile time thus it needs to know the types when macros are being executed and this makes it bit of a challenge.

But as what comes for the Uuid support this can be done quite easily and can be added in further releases. But for documentation:

params(
  ("id" = Uuid, description = "Foo id"),
)

This is merely for documentation purposes and does not really matter whether this is Uuid or String. As if this was Uuid it would anyways render as String in the doc because there is no Uuid type in OpenAPI specification. I would just add additional information to description to actually describe the field better description = "Foo uuid". And after next release you can even define example for paramete as well. E.g. example = json!(Uuid::new_v4()) assuming Uuid can be deserialized with serde.

params(
    ("id" = String, description = "Foo id"),
)

If you have a struct like one below you can use value_type to override the actual type representation to in the generated doc.

#[derive(Deserialize, Serialize, Component)]
struct Foo {
   #[component(value_type = String)]
   id: Uuid,
}

And currently body = Foo<Bar, BarResources> and similarly request_body = does not support generic argumenst and expects only concrete types wich can be archieved with Rust's new type idiom or with type alias. But using them does not remove the fact that generic field arguments does not get resolved.

@juhaku juhaku linked a pull request Apr 26, 2022 that will close this issue
@juhaku
Copy link
Owner

juhaku commented Apr 26, 2022

For what comes to UUID coming with next release the Uuid type will be represented in api doc as String with format uuid when uuid feature is enabled.

The Uuid can directly be defined for parameter like ("id" = Uuid, ...) and for structs struct Value { id: Uuid }

@juhaku
Copy link
Owner

juhaku commented Apr 26, 2022

#[derive(Debug, Serialize, Component)]
pub struct Foo<D, R> {
    pub data: D,
    pub resources: R,
}

And I want to use it like this with a rocket request handler:

#[utoipa::path(
    get,
    context_path = "/api/v1",
    responses(
        (status = 200, description = "Foo found", body = Foo<Bar, BarResources>),
        (status = 404, description = "Foo was not found")
    ),
    params(
        ("id" = Uuid, description = "Foo id"),
    )
)]
#[get("/foo/<id>")]

In a nutshell the the biggest obstacle with the generics is that in code above I don't know how to map Foo<Bar, BarResources> to the Foo<D, R>in compile time without a centralized state. And how to make sure that D is mapped to the Bar and R is mapped to the BarResources.

Then the next question is if this was somehow possible. How would Foo reflect to the OpenAPI doc? Because Foo cannot be present within api doc more than once. For example if one would like to have types like Foo<Bar, BarResources> and Foo<BarFoo, BarFooResources> then how to distinct these types within api doc since both are Foo that is a clear conflict. And then if Foo is only used with Bar and BarResources then why to add them as a generics in a first place?

The current generic support works pretty well with types with lifetimes or traits. In general if there is a way to get around these obstacles the support for these kind of generics could be added for sure. :) But so far I haven't figured out a solution yet.

It's also typical to some cases actually have different types for data transfers like dto pattern in java (data transfer object) where there is another set of types for communication layer which is typically in a shared codebase between api:s or clients and another set of types for internals. But yes some cases bit of a overhead I agree.

@bhoudebert
Copy link

bhoudebert commented Apr 28, 2022

@juhaku to me, like we discussed a bit, to be openapi compliant (even for everyone safety) noone should use directly Foo<Bar, BarResources> but instead create an alias FooBar therefore (not saying it would be easy).

Best world to me, crate could/should support type aliases over generics and not generics directly like mention by issuer as it will never be possible to be compliant (just talking about end use of struct type).

@juhaku
Copy link
Owner

juhaku commented Apr 28, 2022

@juhaku to me, like we discussed a bit, to be openapi compliant (even for everyone safety) noone should use directly Foo<Bar, BarResources> but instead create an alias FooBar therefore (not saying it would be easy).

Best world to me, crate could/should support type aliases over generics and not generics directly like mention by issuer as it will never be possible to be compliant (just talking about end use of struct type).

Yes, the direct generic types cannot be presented in OpenAPI spec as components which itself rules out the use of direct generics as is unless there is a way to get around it. But as so far the type aliases cannot have their own Component definition since its merely alias to the original type. So it leads to situation where the option is to use a type e.g. struct or enum.

Thinking this further. I need to test it out whether I could "use" the type aliases as part of the source to the component with generic types. Perhaps there will be then another trait to use something like SuperComponent etc.

@bhoudebert
Copy link

bhoudebert commented Apr 28, 2022

For the time being to avoid complete manual declaration, rework to struct/enum, I was wondering if it would be possible to achieve something manual+automatic for these types in an impl Modify?

I had in mind keeping everything as it is, alias on api declaration but later on doing something like this:

// this does not compile obviously as it comes from my mind!
impl Modify for ExtraSchemas {
   fn modify(&self, openapi: &mut openapi::OpenApi) {
       openapi.components.as_mut().map(|lst| {
         // I do not know if such feature already exists
         l.schemas.insert("FooBar", Components::From(Foo<Bar, BarResources>))
      }
  }
}

Just throwing an idea like this as it is something I did for some very simple type alias over String, Uuid etc. To me it could be an acceptable alternative until/if we found a more suitable approach.

Drawback is that currently components( will still need the full generic entry otherwise it will not compile and missing to declare aliases would not be detected at compile time, but I might be okay with that but this is only a quick idea/opinion without too much thinking.

Is this idea completely stupid or not? What do you think @juhaku?

NB: Anyway just a positive note, thanks for your work!

@juhaku
Copy link
Owner

juhaku commented Apr 28, 2022

I think this has some point. For simple entities something like this is probably feasible. But for entities like structs with fields the issue that arises is that how to get access to the fields of the Foo<Bar, BarResources> entity.

  openapi.components.as_mut().map(|lst| {
    // I do not know if such feature already exists
    l.schemas.insert("FooBar", Components::From(Foo<Bar, BarResources>))
 }

Most likely there is a need for derive type of macro. As it is easiest way to get access to the TokenStream of the object. And also is highly likely that it needs manual+automatic implementation of some sort to accomplish this in either case.

Like calling the figuragive function Components::From(...) even if it was a function macro because Rust does not have reflection I only have access to information what has been passed to the function as arguments. Also searching through the source code for right instance could be possible but would definitely add unwanted complexitity to the implementation.

NB: Anyway just a positive note, thanks for your work!

😄 yeah Anytime.. Good that this has been found useful. :)

@oliver-impero
Copy link
Author

@juhaku Hm, do you know how the other OpenAPI crate (e.g. okapi, opg) support generic types?

To monomorphize my generic type for each case where it's used, I'm trying to use a macro to avoid duplication:

macro_rules! foo {
    ($typ: ident, $d: ty, $r: ty) => {
        #[derive(Debug, Serialize, Component)]
        pub struct $typ {
            pub data: $d,
            pub resources: $r,
        }
    };
}

foo!(GetFooBody, Foo, FooResources);

But I'm getting the error:

error: unexpected type, expected Type::Path
   |
16 |         #[derive(Debug, Serialize, Component)]
   |                                    ^^^^^^^^^
...
25 | foo!(GetFooBody, Foo, FooResources);

Any idea why?

@juhaku
Copy link
Owner

juhaku commented May 3, 2022

@juhaku Hm, do you know how the other OpenAPI crate (e.g. okapi, opg) support generic types?

Actually no, I have no idea about internals of other frameworks and how things are implemented there.

But I'm getting the error:

error: unexpected type, expected Type::Path
|
16 | #[derive(Debug, Serialize, Component)]
| ^^^^^^^^^
...
25 | foo!(GetFooBody, Foo, FooResources);

Any idea why?

Actually yes, seemingly the foo! macro rules above forms little bit different TokenStream than directly decorating the Rust types with #[derive(Component)]. But more interestingly there are tests in the utoipa which uses macro rules to generate components which does not error out like this. However I make change to the code which allows this syntax to use macro rules to create components.

@juhaku juhaku linked a pull request May 3, 2022 that will close this issue
@juhaku
Copy link
Owner

juhaku commented May 3, 2022

As what I checked about opg. It indeed has some sort of support for generics, but it also does not come without drawbacks. It leverages the idea that when you call describe_api! macro it will use the generics used in describe_api! as arguments to the type. But this way you can only define your generic type only once with the types you want to use, since the generic type name will always be the name of the component as well and OpenAPI cannot have multiple types for one name. Unless it supports aliasing somehow.

Better approach would be Rust type aliases with some macros most likely, if I can leverage them. Still need to investigate possibilities. Or if direct generics is wanted behaviour, then the types need to be directly inlined and not added under "components" in OpenAPI schema.

@juhaku
Copy link
Owner

juhaku commented May 6, 2022

@bhoudebert @oliver-impero I have now played a little with the syntax and now I have rought idea how the generics could roll out. See the example:

struct A {
    a: String,
}
struct B {
    b: i64,
}
struct CC {
    a: String,
}
struct DD {
    b: i64,
}

#[derive(Component)]
#[aliases(FooAB = Foo<A, B>, FooCD = Foo<CC, DD>)]
struct Foo<T, R> {
    field_1: T,
    field_2: R,
}

#[derive(OpenApi)]
#[openapi(
    components(FooAB, FooCD)
)]
struct ApiDoc;

Note that the Foo itself is not being registered as component within components(...). This is still quite some work but I am positive that it is plausable in this way. The aliases would basically create type FooAB = Foo<A, B> under the hood which can be referred later in the application code as well.

@juhaku juhaku moved this to In Progress in utoipa kanban May 6, 2022
@bhoudebert
Copy link

@juhaku oh nice!, I think this would be very nice indeed, afaik it would solve a lot of my current issues!

@juhaku
Copy link
Owner

juhaku commented May 24, 2022

@bhoudebert Now there is a support for this in master. Feel free to try it out. It will be released along with few other things in short future. :)

@juhaku
Copy link
Owner

juhaku commented Jun 10, 2022

This has now been released, so closing this issue for now.

@juhaku juhaku closed this as completed Jun 10, 2022
Repository owner moved this from In Progress to Done in utoipa kanban Jun 10, 2022
@juhaku juhaku moved this from Done to Released in utoipa kanban Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Released
3 participants