Bug: using grouped dependencies with duplicate arguments causes overrides in unexpected ways

[rseeley]

Description

Using multiple dependencies that have the same function arguments, like in litestar-fullstack's provide_created_filter and provide_updated_filter filter dependencies, and passing in one of those arguments as query params causes overrides in seemingly unexpected ways.

For example, if you have the following dependencies:

from dataclasses import dataclass

@dataclass
class FilterA:
    field: str | None
    value: str | None

@dataclass
class FilterB:
    field: str | None
    value: str | None

def provide_filter_a_filter(
    field: str | None = Parameter(query='fieldA', default='a', required=False),
    value: str | None = Parameter(query='valueA', default=None, required=False),
) -> FilterA:
    return FilterA(field=field, value=value)

def provide_filter_b_filter(
    field: str | None = Parameter(query='fieldB', default='b', required=False),
    value: str | None = Parameter(query='valueB', default=None, required=False),
) -> FilterB:
    return FilterB(field=field, value=value)

def provide_filter_dependencies(filter_a: FilterA, filter_b: FilterB) -> list[FilterA | FilterB]:
    return [filter_a, filter_b]

FilterTypes = FilterA | FilterB

and use them in a route handler like

@get('')
async def r(filters: list[FilterA | FilterB] = Dependency(skip_validation=True)) -> list[dict]:
    return [asdict(f) for f in filters]

then pass in the query parameters {'valueA': 'a', 'valueB': 'b'}, the filters will be set to FilterA(field='b', value='b') and FilterB(field='b', value='b'). The values here could change, as noted in the MCVE test code. The field and value values in the filters can be mix and matched, but always duplicated in some way between them (so field=a field=a, value=b, value=b, field=b field=b value=a value=a, etc.).

Using the query argument in Parameter in dependency function arguments doesn't seem to help, though it seems like maybe it should, as this requires you to pass in the query param via the query value used, so it should make them "unique". This would allow you to keep the function arguments duplicated across dependencies (which seems to be behavior that makes sense, since the dependency functions shouldn't have to care/know what other function arguments are set to).

Default values when query params aren't passed in also have this behavior (as you can see in the tests). For example, in the test_duplicated_dependency_arguments__only_passing_values__default_field_values test, field isn't passed in as a query param, and both field values have a default value of 'a' and 'b', respectively. When the test is run, you'll see that both field values will be set to the same value ('a' or 'b', but never separate, as defined in their function). If you run the tests multiple times, you'll see that the order isn't deterministic. In some instances, for example, the field value will be 'a' for both, and others it'll be 'b'.

I haven't tested this, but I assume this only happens when you're using dependencies in the way that's being done here, where like filters (e.g. BeforeAfter) are grouped in a list. I'm not sure if a bug ticket is correct here, or if it should be an enhancement request for Litestar to warn you when you don't have unique names for dependency arguments (maybe only when grouped in a list, if that's possible and actually the root cause here).

URL to code causing the issue

https://github.com/rseeley/litestar-mcves/blob/mcve/filter-names/tests/test_filters.py

MCVE

No response

Steps to reproduce

1. Install dependencies (including the `test` dev dep)
2. Run `pytest` to see which tests fail

Screenshots

No response

Logs

No response

Litestar Version

2.2.1

Platform

• 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.

[provinzkraut]

Hey @rseeley, sorry it took so long for you to get a reply here!

While there is a bug here, it's actually not the one reported; The reported behaviour is expected and simply a result, and as of this time unfortunately also a constraint, of Litestar's dependency injection implementation.

To understand what's happening here, it's important to know that the injection of parameters into dependency callables / handler functions is done via the same mechanisms as the regular dependency injection. It can basically be seen as declaring an implicit dependency, with the shortcut of not needing an extra callable for that. But it does go into the same dependency graph like all other dependencies, and since in Litestar's DI, dependencies are injected by name, this means that names have to be unique within their graph (every handler gets its own graph).

Each of these graphs has a "KwargsModel" associated with it, which is a msgspec.Struct, representing all the dependencies that need to be provided, and is responsible for the parsing and validation steps. If a dependency is requested more than once, it will still only have one attribute on this model, and once its value is resolved, that value will be reused.

Now in this case, what's happening is that an competing dependencies are declared:

• field: str = Parameter(query='fieldA'): A dependency pulling a value from the query parameter fieldA and providing it under the name field
• field: str = Parameter(query='fieldB'): A dependency pulling a value from the query parameter fieldB and providing it under the name field

So now when Litestar resolves this and gets to either one of them, their value will be extracted and stored as the value for the dependency field. When it gets to the second one, it sees that the value for the field dependency already has been resolved, so it won't be resolved again. And even if it would, that would just overwrite the previous value.

The actual bug here is that no exception is raised at startup. Litestar should not allow this case because it's ambiguous.

[rseeley]

Thanks for the thorough explanation, @provinzkraut ! An exception being thrown in this scenario would definitely be helpful to avoid confusion. I'd be happy to test that out when it comes through.