litestar-org
Bug: query parameters can also be picked up from constructors of injected classes
Description
class HealthController(Controller):
path = "health"
@get(path="", cache=False, tags=["health"])
async def get_health_status(self, health_service: HealthService) -> Response[HealthStatusDto]:
result = await health_service.get_health_status()
return Response(result, status_code=HTTP_200_OK if result.all_ok else HTTP_400_BAD_REQUEST)
class HealthService:
def __init__(self,
ldap_connection_pool: LdapConnectionPool
):
self._ldap_connection_pool: LdapConnectionPool = ldap_connection_pool
# rest of class
class LdapConnectionPool:
def __init__(self, config_service: ConfigService):
self._config_service = config_service
# rest of class
class ConfigService:
def __init__(self, config: Config | None = None):
if config:
self._internal_config = config
else:
self._internal_config = None
self.read_config()
# rest of class
class Config(BaseModel):
# Allow none for testing
a_config: AConfig | None
b_config: BConfig | None
after this our swagger showed:
I would assume, qParams (and such) would only be picked u in the signature of the method in the controller?
URL to code causing the issue
No response
MCVE
No response
Steps to reproduce
No response
Screenshots
No response
Logs
No response
Litestar Version
2.2.1
Platform
- [X] Linux
- [ ] Mac
- [ ] Windows
- [ ] Other (Please specify in the description above)
[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.Check out all issues funded or available for funding on our Polar.sh Litestar dashboard
- If you would like to see an issue prioritized, make a pledge towards it!
- We receive the pledge once the issue is completed & verified
- This, along with engagement in the community, helps us know which features are a priority to our users.
This is intended behaviour and generally how the dependency injection works, it's not unique to classes. Any callable can request parameters.
If this were not the case, you couldn't really use any parameters in dependencies, as you'd have to explicitly request them in the handler and pass them to a dependency callable, greatly reducing their usefulness.
I'd suggest that you keep your dependency chain / other utility functions separated to avoid cases like these.
:scream: so you can have qParams, hidden away in dependencies? This seems like a dangerous thing. I would expect parameters, qParams and expected bodies to be only definable on the controller. This way, you have a clean picture of what is exposed.
I think it actually makes sense if you think of query parameters as implicitly defined dependencies. You can request dependencies within dependencies without explicitly declaring them, so query parameters shouldn't be an exception to that.
That being said, we have previously talked about getting rid of the implicit requests for anything other than dependencies in favour of a more explicit approach. This might look something like this then:
def some_dependency(another_query_param: Query[str]) -> str:
return another_query_param
@get("/{path_param:str}", dependencies={"some": some_dependency})
async def handler(
path_param: str,
some: str,
data: Body[dict[str, str]],
query_param: Query[int],
header_param: Header[str],
) -> str:
...
where everything that's not directly injected into the handler has to be explicitly declared.
Maybe it's worth bringing this up again @litestar-org/maintainers? If we were to introduce this, we could also make it optional / hide it in the experimental features for now and maybe then make it the default for 3.0.
I like the idea, and it would allow us to get rid of reserved kwargs, e.g., users could name "data" whatever they want (recent relevant discord discussion: https://discord.com/channels/919193495116337154/1164442955357110292).
