-
Notifications
You must be signed in to change notification settings - Fork 25
[BUG] Tags are duplicated when using cbv #192
Description
Describe the bug
When using a cbv with an APIRouter that defines tags, those tags are duplicated.
To Reproduce
from fastapi import APIRouter
from fastapi_restful.cbv import cbv
testrouter = APIRouter(prefix='/testing', tags=['test'])
@cbv(testrouter)
class TestService:
@testrouter.get("/test")
def test_stuff(self) -> list[dict]:
return [{}]Expected behavior
Each route to have a single 'test' tag. This fixes a bug where ReDoc-style documentation lists two entries per endpoint when using cbv. See: fastapi/fastapi#6165
Additional context
The problem seems to be that, during the processing in the _register_endpoints function in
FastApi-RESTful/fastapi_restful/cbv.py
Lines 112 to 117 in 1201235
| for route in cbv_routes: | |
| router.routes.remove(route) | |
| route.path = route.path[prefix_length:] | |
| _update_cbv_route_endpoint_signature(cls, route) | |
| cbv_router.routes.append(route) | |
| router.include_router(cbv_router) |
Here the route is removed from the router that is passed in and then added to a new router. When a route is added to a router, the router's tags are assigned to the route's
tags attribute. So when the route is removed from the previous router and added to cbv_router, and then cbv_router is included back into the original router, include_router will go through and add router's tags to each route, but that was already done once, so it ends up with two identical tags.
I don't actually understand this code very well. Why can't the routes be modified inline? I feel like this was maybe more necessary when using an InferringRouter? I tested it out just preliminarily and it seems fine, but the test suite does not pass so clearly there's more to it than this.
For now I'll submit a PR that simply removes the tags from the route that are added by that router. It works well enough and passes tests. :)