Skip to content
This repository has been archived by the owner on Nov 10, 2024. It is now read-only.

[BUG] Tags are duplicated when using cbv #192

Open
jgentil opened this issue Jun 20, 2023 · 1 comment · May be fixed by #193
Open

[BUG] Tags are duplicated when using cbv #192

jgentil opened this issue Jun 20, 2023 · 1 comment · May be fixed by #193
Labels
bug Something isn't working

Comments

@jgentil
Copy link

jgentil commented Jun 20, 2023

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

@jgentil jgentil added the bug Something isn't working label Jun 20, 2023
jgentil added a commit to jgentil/FastApi-RESTful that referenced this issue Jun 20, 2023
@jgentil
Copy link
Author

jgentil commented Jun 20, 2023

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.

@jgentil jgentil linked a pull request Jun 20, 2023 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant