fastapi-crudrouter icon indicating copy to clipboard operation
fastapi-crudrouter copied to clipboard

Add names to routes

Open gaganpreet opened this issue 4 years ago • 4 comments

Very helpful project, thanks!

I came across a small issue: I use this recipe to simplify the operation ids generated in the OpenAPI schema. A caveat of applying this simplification is that route names have to be unique.

Unfortunately, if no explicit route name is specified, the fallback is endpoint.__name__ (endpoint here being the endpoint function), which results from this starlette function. In CRUDRouter's case, everything ends up being the same boring value called route.

This PR uses the route prefix to assign a unique name to every route. Grammatically, it's not always correct. eg: If your SQLAlchemy table is named users, get_one_user is correct, but get_all_user isn't. I'm not sure if there's already a way to handle pluralisation in the project, maybe that warrants a discussion first?

gaganpreet avatar Sep 28 '21 21:09 gaganpreet

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/flortz/fastapi-crudrouter/CNzfmvhSrsUUGtrvaiGt8KHKghQq
✅ Preview: https://fastapi-crudrouter-git-fork-gaganpreet-unique-rou-96d782-flortz.vercel.app

vercel[bot] avatar Sep 28 '21 21:09 vercel[bot]

@gaganpreet @grigorov to further this, do you have any objections to directly setting the operation id?

some sample operation_id / names could be:

GET /users -> get_all_users
POST /users -> create_user
POST /user/item_id -> update_user_by_id
...

awtkns avatar Oct 02 '21 07:10 awtkns

Hi @gaganpreet. I ran the master vs this PR and compared the generated openapi spec. You can see this from the /openapi.json route by default. As you can see below, operationIds are already set Fastapi (or something under it). Your changes alter both the operationId and title in the openapi spec.

Before we merge this PR. I think it would be best if alter the operation_id and title to make them more readble.

Method Path Field Old New
GET /potato operationId route_potato_get get_all_potato_potato_get
GET /potato title Response Route Potato Get Response Get All Potato Potato Get
POST /potato operationId route_potato_post create_one_potato_potato_post
...

Thoughts?

Full diff:

 diff master.json unique-route-names.json
14c14
<         "operationId": "get_all_potato_potato_get",
---
>         "operationId": "route_potato_get",
43c43
<                   "title": "Response Get All Potato Potato Get",
---
>                   "title": "Response Route Potato Get",
69c69
<         "operationId": "create_one_potato_potato_post",
---
>         "operationId": "route_potato_post",
108c108
<         "operationId": "delete_all_potato_potato_delete",
---
>         "operationId": "route_potato_delete",
115c115
<                   "title": "Response Delete All Potato Potato Delete",
---
>                   "title": "Response Route Potato Delete",
133c133
<         "operationId": "get_one_potato_potato__item_id__get",
---
>         "operationId": "route_potato__item_id__get",
176c176
<         "operationId": "update_one_potato_potato__item_id__put",
---
>         "operationId": "route_potato__item_id__put",
229c229
<         "operationId": "delete_one_potato_potato__item_id__delete",
---
>         "operationId": "route_potato__item_id__delete",
275c275
<         "operationId": "get_all_carrot_carrot_get",
---
>         "operationId": "route_carrot_get",
303c303
<                   "title": "Response Get All Carrot Carrot Get",
---
>                   "title": "Response Route Carrot Get",
330c330
<         "operationId": "create_one_carrot_carrot_post",
---
>         "operationId": "route_carrot_post",
370c370
<         "operationId": "delete_all_carrot_carrot_delete",
---
>         "operationId": "route_carrot_delete",
377c377
<                   "title": "Response Delete All Carrot Carrot Delete",
---
>                   "title": "Response Route Carrot Delete",
396c396
<         "operationId": "get_one_carrot_carrot__item_id__get",
---
>         "operationId": "route_carrot__item_id__get",
440c440
<         "operationId": "update_one_carrot_carrot__item_id__put",
---
>         "operationId": "route_carrot__item_id__put",
494c494
<         "operationId": "delete_one_carrot_carrot__item_id__delete",
---
>         "operationId": "route_carrot__item_id__delete",

awtkns avatar Oct 05 '21 06:10 awtkns

Hey Adam, busy schedule got in the way. I just looked into the two points you mentioned.

First of all, title: it's generated in this part of FastAPI. It looks like a generated field and I don't see a way to override that with something more pleasant. :frowning_face:

With regards to operation id, it's going through a similar transformation. We can change it to the same value as name (so operation id will look like get_all_potato, create_one_potato etc.), with the caveat that if someone uses duplicate schema names to create FastAPI CRUD routers, things will break (I don't know how it will break as I haven't tried it yet).

Default FastAPI operation ids aren't usually pretty which is why I use the simplify operation ids transformation (which prompted this PR in the first place).

gaganpreet avatar Oct 13 '21 20:10 gaganpreet