yuval9313/FastApi-RESTful

[BUG] Tags are duplicated when using cbv

Opened this issue · 1 comments

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

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. :)

Really the entire _register_endpoints is a bit of a mess. router.routes is iterated no less than three times, and these lines seem really contradictory:

if not isinstance(route, APIRoute):

if isinstance(route, (Route, WebSocketRoute)) and route.endpoint in functions_set

Should they be Route or APIRoute? Why is WebsocketRoute valid in the second check, but APIWebsocketRoute isn't in the first? I only ask because mypy is mad since WebsocketRoute has no tags attribute, and I wasn't sure if a WebsocketRoute would be valid here since the first check ensures that it cannot be.