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

[BUG] Router's prefix is added twice when using CBV

Open WouldYouKindly opened this issue 3 years ago • 6 comments

Describe the bug

When I use CBV with a router that has a prefix, the prefix is included twice.

from fastapi import APIRouter
from fastapi_utils.cbv import cbv


router = APIRouter(prefix='/api/v1')


@cbv(router)
class C:
    @router.get('')
    def f(self):
        ...


assert router.routes[-1].path == '/api/v1/api/v1'
>>> print(fastapi_utils.__version__)
0.2.1
>>> print(fastapi.__version__)
0.63.0

This is because the path already has a prefix before CBV removes and re-adds it to a router:

  1. @router.get calls router.add_api_route which adds a prefix to the path.
  2. cbv calls router.include_router(cbv_router) which again calls router.add_api_route which adds a prefix to the path.

One solution would be to not remove and re-add routes here, and only change the signature, so instead of

    for route in cbv_routes:
        router.routes.remove(route)
        _update_cbv_route_endpoint_signature(cls, route)
        cbv_router.routes.append(route)
    router.include_router(cbv_router)

do

    for route in cbv_routes:
        _update_cbv_route_endpoint_signature(cls, route)

WouldYouKindly avatar Jan 05 '21 19:01 WouldYouKindly

@dmontagu I'm willing to submit a PR with tests if it's okay with you

WouldYouKindly avatar Jan 05 '21 19:01 WouldYouKindly

Describe the bug

When I use CBV with a router that has a prefix, the prefix is included twice.

from fastapi import APIRouter
from fastapi_utils.cbv import cbv


router = APIRouter(prefix='/api/v1')


@cbv(router)
class C:
    @router.get('')
    def f(self):
        ...


assert router.routes[-1].path == '/api/v1/api/v1'
>>> print(fastapi_utils.__version__)
0.2.1
>>> print(fastapi.__version__)
0.63.0

This is because the path already has a prefix before CBV removes and re-adds it to a router:

1. `@router.get` calls `router.add_api_route` which adds a prefix to the path.

2. `cbv` calls `router.include_router(cbv_router)` which again calls `router.add_api_route` which adds a prefix to the path.

One solution would be to not remove and re-add routes here, and only change the signature, so instead of

    for route in cbv_routes:
        router.routes.remove(route)
        _update_cbv_route_endpoint_signature(cls, route)
        cbv_router.routes.append(route)
    router.include_router(cbv_router)

do

    for route in cbv_routes:
        _update_cbv_route_endpoint_signature(cls, route)

I think this also fixes #85

negadive avatar Jan 12 '21 02:01 negadive

Perhaps you could make a PR to fastapi_restful, a fork of this project, and is possibly more actively maintained: https://github.com/yuval9313/fastapi-restful/?

I did confirm that this issue also exists in fastapi-restful.

falkben avatar Jan 28 '21 16:01 falkben

It took some time but as @falkben said I had this PR merged in my fork feel free to check it out :)

yuval9313 avatar May 05 '21 20:05 yuval9313

Any update on this? Thanks

abdulmateen59 avatar May 06 '22 07:05 abdulmateen59

Any update on this? Thanks

Consider using the forked and maintained version of this lib: https://github.com/yuval9313/FastApi-RESTful. They've already fixed this issue.

nitaibezerra avatar Jun 29 '22 12:06 nitaibezerra