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

Utoipa users poll: How do you use responses and how would you like them to work? Answer would help. #1068

Open
juhaku opened this issue Sep 27, 2024 · 6 comments
Labels
question Further information is requested

Comments

@juhaku
Copy link
Owner

juhaku commented Sep 27, 2024

Responses poll

Currently while I am working on refactoring multiple aspects of the utoipa lib for the coming release 5.0.0 this is good place to introduce some improvements to current workflow within boundaries set by its users. So here comes the question.

How do you currently use the responses with utoipa lib?

  1. Only tuples
  2. Tuples with derived ToResponse
  3. Derived IntoResponses
  4. Any other way, please describe.

Second question is how would you like the responses work, or is there any aspect you would like to change in order to them to be more user friendly?

@juhaku juhaku added the question Further information is requested label Sep 27, 2024
@juhaku juhaku pinned this issue Sep 27, 2024
@juhaku juhaku changed the title Utoipa users poll: How do you use the responses and how would you like them to work? Answer would help. Utoipa users poll: How do you use responses and how would you like them to work? Answer would help. Sep 27, 2024
@michaelvlach
Copy link

Hi, first of all huge thank you for this crate!

To answer your questions. I use dedicated structs and deriving from ToSchema: https://github.com/agnesoft/agdb/blob/main/agdb_api/rust/src/api_types.rs

I find it quite nice and I would not change it. There are two aspects I think would be nice if improved but I am not sure if possible:

  1. I miss validation of the stuff in the derived against actual code (route signature / types). Obviously things like status codes cannot be validated but I hope types could be. Often I missed the change in return type in the derive annotation and nothing complained. Luckily I have comprehensive test suite so I caught it but a compile error would be nice if there was a mismatch. Probably not possible though.

  2. More comprehensive documentation on the derive macro. I use a lots of stuff from utoipa and sometimes it was difficult to find out how to achieve what I wanted. E.g. https://github.com/agnesoft/agdb/blob/main/agdb_server/src/routes/db.rs#L82 Since I generate apis for different languages and they require things like tags, operation_id etc. That is a common struggle, no such thing as enough docs. :-)

@juhaku
Copy link
Owner Author

juhaku commented Oct 1, 2024

Great, thanks for a reply.

  1. There is improvement for this actually coming in the 5.0.0 release of utoipa. However if you completely forget the add the response(...) declaration then the improvement will not help there yet since the return types are not inferred. But could be with certain limitations. In the 5.0.0 release the request_body and response body schemas will be correctly spanned thus it will give compile error if an item does not exists and will also get renamed if you rename the type as well.
  2. True, more docs, that is forever target of improvement. 🙂

@irevoire
Copy link

irevoire commented Oct 3, 2024

Hey, not really an answer on how I uses it, but I would really like to be able to share responses between routes.

For example, in Meilisearch, most routes will return something we call a SummarizedTaskView. And from what I understand, currently I have to write a json!({ ... }).
I would really like to be able to reference it without rewriting the example entirely.

Going from:

/// Get a task
///
/// Get a [task](https://www.meilisearch.com/docs/learn/async/asynchronous_operations)
#[utoipa::path(
    post,
    path = "/{taskUid}",
    tag = "Tasks",
    params(("taskUid", format = UInt32, example = 0, description = "The task identifier", nullable = false)),
    responses(
        (status = 200, description = "Task successfully enqueued", body = SummarizedTaskView, content_type = "application/json", example = json!(
            {
              "uid": 1,
              "indexUid": "movies",
              "status": "succeeded",
              "type": "documentAdditionOrUpdate",
              "canceledBy": null,
              "details": {
                "receivedDocuments": 79000,
                "indexedDocuments": 79000
              },
              "error": null,
              "duration": "PT1S",
              "enqueuedAt": "2021-01-01T09:39:00.000000Z",
              "startedAt": "2021-01-01T09:39:01.000000Z",
              "finishedAt": "2021-01-01T09:39:02.000000Z"
            }
        ))
    )
)]
async fn get_task(
...

To:

/// Get a task
///
/// Get a [task](https://www.meilisearch.com/docs/learn/async/asynchronous_operations)
#[utoipa::path(
    post,
    path = "/{taskUid}",
    tag = "Tasks",
    params(("taskUid", format = UInt32, example = 0, description = "The task identifier", nullable = false)),
    responses(
        (status = 200, description = "Task successfully enqueued", body = SummarizedTaskView, content_type = "application/json", example = task_succeeded())
    )
)]
async fn get_task(
...

I’m using the latest beta version, let me know if I missed something.

@juhaku
Copy link
Owner Author

juhaku commented Oct 3, 2024

Hey, not really an answer on how I uses it, but I would really like to be able to share responses between routes.

Responses can be shared if used with ToResponse. Types that implement ToResponse can be added to the responses section of OpenApi. and then they can be references with response = MyResponse.

struct MyResponse; 

impl<'s> ToResponse<'s> for MyResponse {
  fn response() -> (&'s str, Response) -> {
    // add all the examples here programmatically
    ("MyResponse", Response::builder().build())
  }
}

#[utoipa::path(
    post,
    path = "/{taskUid}",
    tag = "Tasks",
    params(("taskUid", format = UInt32, example = 0, description = "The task identifier", nullable = false)),
    responses(
        (status = 200, response = inline(MyResponse))
    )
)]
async fn get_task(} {}

You could also derive the ToResponse but I am most likely going to change the ToResponse implementation. And I am not sure about the current behavior of the derive. I think it lacking something but I am not sure what, and perhaps I am going to remove that completely if I find some better way to do shareable responses.

example and examples attributes could be allowed to use method references though nonetheless but hasn't been done so far.

@irevoire
Copy link

irevoire commented Oct 9, 2024

Humm, yes, I tried to use it, but it didn’t work, and I don’t really have the time to understand why, It's probably an issue on my side.

But in terms of API, I find it strange that the ResponseBuilder doesn’t contain the HTTP status code and the schema 🤔
But it’s pretty nice to be able to define multiple examples for one type.
Defining and using the examples through a structure is also pretty good, IMO

Ideally, in our case, almost every route can have three different errors that share the same error code: If you forget to specify an API key, we don’t understand the format of your API key, or your API key doesn’t give you the right to access this route.
Iirc the first two returns an HTTP 400 while the last one return a 401 or something.
Then, they all share the same schema.
It would be really nice to be able to define all that on a single struct and then use it easily in most of my routes:

#[utoipa::path(
    post,
    path = "/{taskUid}",
    tag = "Tasks",
    params(("taskUid", format = UInt32, example = 0, description = "The task identifier", nullable = false)),
    responses(
        inline(ApiKeyError), // here I can define multiple examples with multiple HTTP status code
        inline(SummarizedTaskView), // here I only have one example
        (status = BAD_REQUEST, schema = ResponseError, example = json!(...)),
    )
)]
async fn get_task(} {}

I hope that can help you in your decisions!

@juhaku
Copy link
Owner Author

juhaku commented Nov 14, 2024

@irevoire Thanks for the response. Yeah that is worth thinking about. The current ResponseBuilder and IntoResponses schema thing is a bit convoluted and is really in a need of reassessment of how it actually should behave.

ResponseBuilder accpets content and then you need to use the ContentBuilder to provide the schema. However it does not contain the status code and that is in ResponsesBuilder. This is all because the implementation mimics the OpenAPI specification on the types and its definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants