From f659871c6619b42456b0fd5119131ca2ffe9d66a Mon Sep 17 00:00:00 2001 From: Jon-Pierre Gentil Date: Tue, 20 Jun 2023 12:53:40 -0500 Subject: [PATCH 1/3] Fixed an issue where duplicate tags are created during the route rewrite phase inside cbv. References issue #192 --- fastapi_restful/cbv.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fastapi_restful/cbv.py b/fastapi_restful/cbv.py index 8508425c..0ed0ea8c 100644 --- a/fastapi_restful/cbv.py +++ b/fastapi_restful/cbv.py @@ -111,6 +111,12 @@ def _register_endpoints(router: APIRouter, cls: Type[Any], *urls: str) -> None: prefix_length = len(router.prefix) # Until 'black' would fix an issue which causes PEP8: E203 for route in cbv_routes: router.routes.remove(route) + # remove any tags that were previously assigned by router to route + for tag in router.tags: + try: + route.tags.remove(tag) + except ValueError: + pass route.path = route.path[prefix_length:] _update_cbv_route_endpoint_signature(cls, route) cbv_router.routes.append(route) From 77d6ff05bfcbdc8188223b36ae318357f244578a Mon Sep 17 00:00:00 2001 From: Jon-Pierre Gentil Date: Tue, 20 Jun 2023 13:18:43 -0500 Subject: [PATCH 2/3] Added type checking to ensure we only remove tags from APIRoute instances. --- fastapi_restful/cbv.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/fastapi_restful/cbv.py b/fastapi_restful/cbv.py index 0ed0ea8c..d53bce40 100644 --- a/fastapi_restful/cbv.py +++ b/fastapi_restful/cbv.py @@ -112,11 +112,12 @@ def _register_endpoints(router: APIRouter, cls: Type[Any], *urls: str) -> None: for route in cbv_routes: router.routes.remove(route) # remove any tags that were previously assigned by router to route - for tag in router.tags: - try: - route.tags.remove(tag) - except ValueError: - pass + if isinstance(route, APIRoute): + for tag in router.tags: + try: + route.tags.remove(tag) + except ValueError: + pass route.path = route.path[prefix_length:] _update_cbv_route_endpoint_signature(cls, route) cbv_router.routes.append(route) From c8b7ec2197ae1814a1be6607873d13596240638f Mon Sep 17 00:00:00 2001 From: Jon-Pierre Gentil Date: Mon, 7 Aug 2023 11:13:15 -0500 Subject: [PATCH 3/3] Moved tag cleanup to separate function; implemented faster removal logic; added test case --- fastapi_restful/cbv.py | 17 ++++++++++------- tests/test_cbv.py | 11 +++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/fastapi_restful/cbv.py b/fastapi_restful/cbv.py index d53bce40..386826bf 100644 --- a/fastapi_restful/cbv.py +++ b/fastapi_restful/cbv.py @@ -85,6 +85,15 @@ def new_init(self: Any, *args: Any, **kwargs: Any) -> None: setattr(cls, CBV_CLASS_KEY, True) +def _remove_router_tags(route: Route, router: APIRouter) -> None: + """ + Removes tags previously assigned to the route by the router, as they will + be added again later when re-added to the router. + """ + if hasattr(route, 'tags'): + route.tags = list(set(route.tags) - set(router.tags)) + + def _register_endpoints(router: APIRouter, cls: Type[Any], *urls: str) -> None: cbv_router = APIRouter() function_members = inspect.getmembers(cls, inspect.isfunction) @@ -111,13 +120,7 @@ def _register_endpoints(router: APIRouter, cls: Type[Any], *urls: str) -> None: prefix_length = len(router.prefix) # Until 'black' would fix an issue which causes PEP8: E203 for route in cbv_routes: router.routes.remove(route) - # remove any tags that were previously assigned by router to route - if isinstance(route, APIRoute): - for tag in router.tags: - try: - route.tags.remove(tag) - except ValueError: - pass + _remove_router_tags(route, router) route.path = route.path[prefix_length:] _update_cbv_route_endpoint_signature(cls, route) cbv_router.routes.append(route) diff --git a/tests/test_cbv.py b/tests/test_cbv.py index a6ba1af1..aba14091 100644 --- a/tests/test_cbv.py +++ b/tests/test_cbv.py @@ -130,3 +130,14 @@ def root(self) -> str: response = client.get("/api/item") assert response.status_code == 200 assert response.json() == "hello" + + def test_duplicate_tag_removal(self) -> None: + router = APIRouter(prefix="/api", tags=["test"]) + + @cbv(router) + class CBV: + @router.get("/item") + def root(self) -> str: + return "hello" + + assert router.routes[0].tags == ["test"]