litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.66k stars 384 forks source link

Bug: Same handler is allowed to be registered twice #912

Closed Alc-Alc closed 1 year ago

Alc-Alc commented 1 year ago

Describe the bug Adding a previously attached route using app.register should not be allowed. starlite.exceptions.http_exceptions.ImproperlyConfiguredException should be raised.

To Reproduce

from starlite import Starlite, get, Router

@get('/{name:str}')
def get_name(name: str) -> str:
    return name

router = Router('/v1', route_handlers=[get_name])
app = Starlite([router])
app.register(get_name)  # allowed

Additional context Both the routes /v1/foo and /foo are accessible.

Alc-Alc commented 1 year ago

Looking at https://github.com/starlite-api/starlite/blob/main/starlite/router.py#L287, the isinstance check seems to only check for Router and not for handlers.http.get?

if isinstance(value, Router):
    if value.owner:
        raise ImproperlyConfiguredException(f"Router with path {value.path} has already been registered")
Goldziher commented 1 year ago

Ok, so looking at this- its actually not a bug: https://starlite-api.github.io/starlite/usage/1-routing/4-registering-components-multiple-times/#dynamic-route-registration, closing this.

tsiresymila1 commented 1 year ago

Hello,

image

Can anyone tell me white this(starlite instance route register) was called twice, It creates a bug with Router because of

if isinstance(value, Router):
    if value.owner:
        raise ImproperlyConfiguredException(f"Router with path {value.path} has already been registered")