Lancetnik / FastDepends

FastDepends - FastAPI Dependency Injection system extracted from FastAPI and cleared of all HTTP logic. Async and sync modes are both supported.
https://lancetnik.github.io/FastDepends/
MIT License
281 stars 10 forks source link

keyword arguments resolved to None instead of default values when using `inject(cast=False)` #115

Closed marcodlk closed 1 month ago

marcodlk commented 1 month ago

Hi, first of all - thanks for creating fastdepends. Big fan!

Recently, ran into some unexpected behavior which does not appear intended. When inject(cast=False) is used, keyword arguments are resolved to None instead of the default value. For example, in the code below, get_hello() returns "hello, None" instead of "hello, world".

from fast_depends import Depends, inject

def get_world(value: str = "world"):
    return value

@inject(cast=False)
def get_hello(world: str = Depends(get_world)):
    return f"hello, {world}"

print(get_hello())  # Output: hello, None

When inject(cast=True) is used, the default values are used as expected.

The source of the issue appears to be the following lines in core/build.py: https://github.com/Lancetnik/FastDepends/blob/eebb26d597172114fb63c666b1bf2d6f193371ce/fast_depends/core/build.py#L174-L177

In this case, param.kind is param.POSITIONAL_OR_KEYWORD, which is not param.KEYWORD_ONLY, so the param_name gets added to the positional_args list instead of keyword_args.

Is this intended behavior? It was unexpected to me and feels like an inconsistency.

Lancetnik commented 1 month ago

@marcodlk thank you for the Issue! I think, this is the bug and we should fix it. Unfortunatelly, I am too busy with FastStream now, but planning to make FastDepends sprint next month

marcodlk commented 1 month ago

Sounds good. Not too much rush - thanks to your flexible design, I can just use a wrap_model function to make necessary patch in the meantime :)

marcodlk commented 1 month ago

Ok it will be a bit trickier to patch than I thought. I believe the following line will need to be changed...

https://github.com/Lancetnik/FastDepends/blob/eebb26d597172114fb63c666b1bf2d6f193371ce/fast_depends/core/model.py#L296

...to something like this:

kwargs_ = {arg: solved_kw[arg] for arg in keyword_args if arg in solved_kw}

In the current logic, the solved_kw does not contain the value, so the params in keyword_args get value set to None and the None is injected as the keyword arg in the function call, overwriting the default.