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

Derived OpenAPI Docs in 5.x+ generates incorrect method call for swagger ui when function name is the same across two crates #1190

Open
fenryka opened this issue Nov 5, 2024 · 6 comments

Comments

@fenryka
Copy link

fenryka commented Nov 5, 2024

Note, this worked fine in 4.x but is now broken in 5.x+

Essentially, if you have two functions with the same name in different crates annotated with different paths, for example:

#[derive(OpenApi)] #[openapi( paths( v3::get_projects, v2::get_projects, ) )]

Then in the generated swagger docs page the invoked URL path for the V3 method will be wrong. What happens will be that the method will invoke the V2 path instead of V3. V2 will work as expected.

Both the generated Curl and RequestUrl values are also wrong (see screenshot)

Screenshot 2024-11-05 at 19 08 56

Where despite being in the V3 section, the value are wrong (and in this case the example has only returned two elements in the list as for our simple test case, v2 returns two things and v3 should return 3).

Link to the testcase is here

Update:

When comparing the generated openapi.json between 4.x and 5.x they are identical (beyond the version number)

openapi-4.2.json
openapi-5.2.json

Would this indicate an issue with swagger?

@JMLX42
Copy link
Contributor

JMLX42 commented Nov 7, 2024

IIRC the doc says the function names have to be unique because they are used as the operationId. And the macro cannot possibly know about other crates.

We might be able to prefix the operationId with the module name though. That would make collisions less likely.

Still the best solution is to have unique function names.

@juhaku
Copy link
Owner

juhaku commented Nov 7, 2024

Yup, Probably Swagger UI has worked before but since then and at least those latest versions are using operationId to determine which endpoint to call. Since the beginning of utoipa the operationId has always been set from the handler function name. This is actually duplicate for this #1170 and explained more specifically here #1170 (comment).

@fenryka
Copy link
Author

fenryka commented Nov 7, 2024

I agree it cannot know about other crates, but adding the module name into the operationid as a prefix would greatly reduce collisions, we can't be the only people who version APIs this way

@juhaku
Copy link
Owner

juhaku commented Nov 7, 2024

While prepending the operationId with the module path when paths are defined with #[openapi(paths(...))] or #[openapi(nest(...))] using full path to the handler including the module, is possible but when it is defined without the full path to the handler then things get rough. At least I am not aware of a way to do that. What's more is it would be good to have consistent behavior across the methods the path can be registered to an OpenApi.

@srkaj
Copy link

srkaj commented Dec 19, 2024

I think the issue should be solved by having the OpenApiRouter.nest(..) method should be able to modify the operationId of all the endpoints in the nested router. Maybe also add a tag to all the endpoints inside.

Or just to have a possibility to do that on the nested router, maybe something like this:

OpenApiRouter::with_openapi(OpenApi::builder().info(myinfo).build())
    .nest("/one", one::router().with_operation_id_prefix("one_").with_added_tag("one"))

I admit that having to set operationId manually or choose unique method names is only a rather minor inconvenience in (probably) most projects, but it does make some cases of code reuse harder than they should be.

@Cheban1996
Copy link

I think the issue should be solved by having the OpenApiRouter.nest(..) method should be able to modify the operationId of all the endpoints in the nested router. Maybe also add a tag to all the endpoints inside.

Or just to have a possibility to do that on the nested router, maybe something like this:

OpenApiRouter::with_openapi(OpenApi::builder().info(myinfo).build())
    .nest("/one", one::router().with_operation_id_prefix("one_").with_added_tag("one"))

I admit that having to set operationId manually or choose unique method names is only a rather minor inconvenience in (probably) most projects, but it does make some cases of code reuse harder than they should be.

juhaku Will you implement this .with_added_tag("one"), it was really cool!

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

No branches or pull requests

5 participants