Skip to content

Commit e328cdb

Browse files
authored
fix: give inline providers in Factory kwargs the scope of the providers using them (#734)
* fix: give inline providers in Factory kwargs the scope of the providers using them A provider written inline in a Factory's kwargs was never stamped and always resolved at APP scope. An inline cache=True Factory under a REQUEST parent was silently shared by every request, and inline providers over request-scoped sources failed validate(). At registration, each inline provider without its own scope now takes the shallowest scope among the providers that use it, and is frozen there. Closes #732 * fix: pass the inline scope through a derived provider to its inline source
1 parent 53b159a commit e328cdb

6 files changed

Lines changed: 299 additions & 5 deletions

File tree

‎CONTEXT.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ _Avoid_: computed (implies caching), selector (picks a provider by key in other
5454
A provider with no bound type, resolvable only by passing the provider itself.
5555
_Avoid_: unbound provider, anonymous provider
5656

57+
**Inline provider**:
58+
A provider written as a value in another factory's `kwargs` instead of declared as a group class
59+
attribute. Without its own scope it takes the shallowest scope among the providers that use it.
60+
_Avoid_: nested provider
61+
5762
**Wiring plan**:
5863
The partition of a creator's parameters by how each is satisfied.
5964
_Avoid_: compiled kwargs

‎docs/providers/factories.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ them, for example `bound_type=None` to resolve each by reference. Before modern-
7777

7878
The provider's lifetime. Without `scope=`, the provider takes its group's default scope, or
7979
`Scope.APP` when the group sets none; see
80-
[Group-level default scope](scopes.md#group-level-default-scope).
80+
[Group-level default scope](scopes.md#group-level-default-scope). A provider written inline in
81+
another factory's `kwargs` takes the scope of the providers that use it; see
82+
[Inline providers](scopes.md#inline-providers).
8183
[Scopes](scopes.md) covers the five scopes and the dependency rule.
8284

8385
### bound_type

‎docs/providers/scopes.md‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,14 +220,60 @@ Scope resolution follows a priority order:
220220

221221
1. An explicit `scope=` on the provider always wins.
222222
2. Then the group's `scope=` kwarg, inherited via MRO by subclasses. A subclass may override it with its own `scope=` kwarg, which applies to the providers declared in its own body; inherited providers keep the scope their declaring class gave them.
223-
3. Otherwise `Scope.APP`, the final default.
223+
3. For a provider written inline in another factory's `kwargs`, the scope of the providers that use it; see [Inline providers](#inline-providers).
224+
4. Otherwise `Scope.APP`, the final default.
224225

225226
`Alias` providers do not participate in group-level scope defaults; an alias's scope always derives from its source.
226227

227228
A scope-defaulted provider instance that is shared between two `Group` subclasses with different defaults raises [`GroupScopeConflictError`](../troubleshooting/group-scope-conflict-error.md) at class-creation time. Sharing the same provider instance with the same default scope across multiple groups is allowed.
228229

229230
A group declared without a `scope=` kwarg stamps nothing, so a provider listed only in such a group keeps the `Scope.APP` default and can still be stamped by a later group, but only until it is registered with a container. After that, a group that would *change* its scope raises [`ProviderScopeFrozenError`](../troubleshooting/provider-scope-frozen-error.md), because resolvers compiled before the change already captured the old scope. Declare every group that lists a provider before building the container, or set `scope=` on the provider explicitly.
230231

232+
## Inline providers
233+
234+
A provider written inline in a factory's `kwargs` is not a class attribute, so no group stamps it.
235+
Without its own `scope=`, it takes the scope of the provider it sits in when the container registers
236+
them:
237+
238+
```python
239+
from modern_di import Container, Group, Scope, providers
240+
241+
242+
class DbSession:
243+
pass
244+
245+
246+
class Handler:
247+
def __init__(self, session: DbSession) -> None:
248+
self.session = session
249+
250+
251+
class Handlers(Group, scope=Scope.REQUEST):
252+
handler = providers.Factory(Handler, kwargs={"session": providers.Factory(DbSession, cache=True)})
253+
254+
255+
handlers_container = Container(groups=[Handlers])
256+
with handlers_container.build_child_container(scope=Scope.REQUEST) as first:
257+
first_session = first.resolve(Handler).session
258+
with handlers_container.build_child_container(scope=Scope.REQUEST) as second:
259+
assert second.resolve(Handler).session is not first_session # one DbSession per request
260+
```
261+
262+
When one inline provider object sits in the `kwargs` of several providers, it takes the shallowest of
263+
their scopes, so that each of them can resolve it. A module-level provider for a settings field
264+
shared by an `APP` engine and a `REQUEST` session gets `APP`.
265+
266+
A [derived provider](factories.md#derived-values) has no scope of its own, so the rule passes through
267+
it: in `kwargs={"session": providers.Factory(DbSession, cache=True).derive(fn)}`, the inline
268+
`DbSession` factory takes the scope of the provider that uses the derived one.
269+
270+
An explicit `scope=` on the inline provider wins, as it does everywhere. A provider that is a class
271+
attribute of some group keeps its own scope wherever it is referenced. An inline provider's scope is
272+
frozen when it is first registered. A later `add_providers()` call never changes it, so a new
273+
provider shallower than that scope fails `validate()` with `InvalidScopeDependencyError`, and a group
274+
declared later that would change it raises
275+
[`ProviderScopeFrozenError`](../troubleshooting/provider-scope-frozen-error.md).
276+
231277
## See also
232278

233279
- [Lifecycle](lifecycle.md): finalizers and `close_async()` work per-scope.

‎modern_di/providers/abstract.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ class AbstractProvider(typing.Generic[types.T_co]):
2424
"_declared_as",
2525
"_explicit_scope",
2626
"_group_claim",
27+
"_inline_scope",
2728
"_provider_id",
2829
"_registered",
2930
)
@@ -54,6 +55,7 @@ def __init__(
5455
"""
5556
self._explicit_scope: enum.IntEnum | None = scope if isinstance(scope, enum.IntEnum) else None
5657
self._group_claim: tuple[enum.IntEnum, str] | None = None
58+
self._inline_scope: enum.IntEnum | None = None
5759
self._registered = False
5860
self._bound_type = inferred_bound_type if isinstance(bound_type, types.UnsetType) else bound_type
5961
self._provider_id = next(_provider_id_counter)
@@ -83,7 +85,9 @@ def provider_id(self) -> int:
8385

8486
@property
8587
def scope(self) -> enum.IntEnum:
86-
"""The declared scope: the provider's own ``scope=``, else a Group default, else ``Scope.APP``.
88+
"""The declared scope: own ``scope=``, else a Group default, else an inline scope, else ``Scope.APP``.
89+
90+
An inline provider's scope is the shallowest scope among the providers using it.
8791
8892
For a redirect such as ``Alias`` this is a ``Scope.APP`` placeholder, not the effective scope: an
8993
alias resolves at its source's scope.
@@ -92,8 +96,16 @@ def scope(self) -> enum.IntEnum:
9296
return self._explicit_scope
9397
if self._group_claim is not None:
9498
return self._group_claim[0]
99+
if self._inline_scope is not None:
100+
return self._inline_scope
95101
return Scope.APP
96102

103+
def _stamp_inline_scope(self, scope: enum.IntEnum) -> None:
104+
"""Record the scope an inline provider takes from the providers using it, then freeze it."""
105+
if not self._registered and self._takes_group_scope and self._explicit_scope is None:
106+
self._inline_scope = scope
107+
self._registered = True
108+
97109
def _stamp_group_scope(self, scope: enum.IntEnum, group_name: str) -> None:
98110
"""Record a Group-level default scope; a no-op unless the scope is still an unclaimed default.
99111

‎modern_di/registries/providers_registry.py‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,16 @@
44

55
from modern_di import exceptions, suggester, types
66
from modern_di.dependency_graph import terminal_chain
7-
from modern_di.providers.abstract import AbstractProvider
7+
from modern_di.providers.abstract import AbstractProvider, _Derived
88
from modern_di.providers.alias import Alias
9+
from modern_di.providers.factory import Factory
910
from modern_di.registries.overrides_registry import OverridesRegistry
1011
from modern_di.resolver_compiler import compile_resolver
1112
from modern_di.wiring import WiringPlan
1213

1314

1415
if typing.TYPE_CHECKING:
1516
from modern_di import Container
16-
from modern_di.providers.factory import Factory
1717

1818

1919
_K = typing.TypeVar("_K")
@@ -186,6 +186,7 @@ def _add(
186186
raise exceptions.DuplicateProviderTypeError(
187187
provider_type=provider_type, first_provider=existing, second_provider=provider
188188
)
189+
_stamp_inline_scopes(registered)
189190
self._providers.update(new_providers)
190191
bound = {id(provider) for provider in new_providers.values()}
191192
for provider in registered:
@@ -205,3 +206,47 @@ def _drop_resolvers(self) -> None:
205206
self._resolvers.clear()
206207
self._resolvers_by_type.clear()
207208
self._generation += 1
209+
210+
211+
def _inline_children(provider: AbstractProvider[typing.Any]) -> list[AbstractProvider[typing.Any]]:
212+
"""Return the providers written inline in ``provider``'s kwargs or derive source: none is a class attribute."""
213+
if type(provider) is _Derived:
214+
values: typing.Iterable[typing.Any] = (provider._source,) # noqa: SLF001
215+
elif type(provider) is Factory and provider._kwargs: # noqa: SLF001
216+
values = provider._kwargs.values() # noqa: SLF001
217+
else:
218+
return []
219+
return [value for value in values if isinstance(value, AbstractProvider) and value._declared_as is None] # noqa: SLF001
220+
221+
222+
def _stamp_inline_scopes(registered: tuple[AbstractProvider[typing.Any], ...]) -> None:
223+
"""Give each scope-defaulted inline provider the shallowest scope among the providers using it.
224+
225+
A scope set by an earlier registration is kept.
226+
"""
227+
roots = {id(provider) for provider in registered}
228+
children_of: dict[int, list[AbstractProvider[typing.Any]]] = {}
229+
parents_of: dict[int, list[AbstractProvider[typing.Any]]] = {}
230+
stack = list(registered)
231+
while stack:
232+
parent = stack.pop()
233+
if id(parent) in children_of:
234+
continue
235+
children = [child for child in _inline_children(parent) if id(child) not in roots]
236+
children_of[id(parent)] = children
237+
for child in children:
238+
parents_of.setdefault(id(child), []).append(parent)
239+
stack.append(child)
240+
waiting = {child_id: len(parents) for child_id, parents in parents_of.items()}
241+
passed_down = {id(provider): provider.scope for provider in registered}
242+
ready = list(registered)
243+
while ready:
244+
parent = ready.pop()
245+
for child in children_of[id(parent)]:
246+
waiting[id(child)] -= 1
247+
if waiting[id(child)]:
248+
continue
249+
scope = min(passed_down[id(p)] for p in parents_of[id(child)])
250+
child._stamp_inline_scope(scope) # noqa: SLF001
251+
passed_down[id(child)] = scope if type(child) is _Derived else child.scope
252+
ready.append(child)

‎tests/test_inline_scope.py‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
import dataclasses
2+
3+
import pytest
4+
5+
from modern_di import Container, Group, Scope, providers
6+
from modern_di.exceptions import ProviderScopeFrozenError, ValidationFailedError
7+
8+
9+
class _Session: ...
10+
11+
12+
@dataclasses.dataclass(kw_only=True, slots=True, frozen=True)
13+
class _Request:
14+
user: str
15+
16+
17+
class _Handler:
18+
def __init__(self, session: _Session) -> None:
19+
self.session = session
20+
21+
22+
def _sessions_of_two_requests(container: Container) -> tuple[_Session, _Session]:
23+
with container.build_child_container(scope=Scope.REQUEST) as first:
24+
a = first.resolve(_Handler).session
25+
with container.build_child_container(scope=Scope.REQUEST) as second:
26+
b = second.resolve(_Handler).session
27+
return a, b
28+
29+
30+
def test_inline_cached_factory_takes_group_default_scope() -> None:
31+
class Dependencies(Group, scope=Scope.REQUEST):
32+
handler = providers.Factory(_Handler, kwargs={"session": providers.Factory(_Session, cache=True)})
33+
34+
a, b = _sessions_of_two_requests(Container(groups=[Dependencies]))
35+
36+
assert a is not b
37+
38+
39+
def test_inline_cached_factory_takes_explicit_parent_scope() -> None:
40+
class Dependencies(Group):
41+
handler = providers.Factory(
42+
_Handler, scope=Scope.REQUEST, kwargs={"session": providers.Factory(_Session, cache=True)}
43+
)
44+
45+
a, b = _sessions_of_two_requests(Container(groups=[Dependencies]))
46+
47+
assert a is not b
48+
49+
50+
def test_inline_factory_over_request_source_validates_and_resolves() -> None:
51+
class Dependencies(Group, scope=Scope.REQUEST):
52+
request = providers.ContextProvider(_Request)
53+
user_name = providers.Factory(
54+
str, kwargs={"object": providers.Factory(lambda r: r.user, kwargs={"r": request})}
55+
)
56+
57+
container = Container(groups=[Dependencies])
58+
container.validate()
59+
60+
with container.build_child_container(scope=Scope.REQUEST, context={_Request: _Request(user="alice")}) as child:
61+
assert child.resolve_provider(Dependencies.user_name) == "alice"
62+
63+
64+
def test_inline_context_provider_reads_request_context() -> None:
65+
class Dependencies(Group, scope=Scope.REQUEST):
66+
user = providers.Factory(lambda r: r.user, kwargs={"r": providers.ContextProvider(_Request)})
67+
68+
container = Container(groups=[Dependencies])
69+
70+
with container.build_child_container(scope=Scope.REQUEST, context={_Request: _Request(user="alice")}) as child:
71+
assert child.resolve_provider(Dependencies.user) == "alice"
72+
73+
74+
class _Engine:
75+
def __init__(self, dsn: str) -> None:
76+
self.dsn = dsn
77+
78+
79+
class _DbConnection:
80+
def __init__(self, dsn: str) -> None:
81+
self.dsn = dsn
82+
83+
84+
def test_inline_provider_shared_by_app_and_request_parents_takes_app_scope() -> None:
85+
dsn = providers.Factory(lambda: "postgres://")
86+
87+
class Dependencies(Group):
88+
db_connection = providers.Factory(_DbConnection, scope=Scope.REQUEST, kwargs={"dsn": dsn})
89+
engine = providers.Factory(_Engine, kwargs={"dsn": dsn}, cache=True)
90+
91+
container = Container(groups=[Dependencies])
92+
container.validate()
93+
94+
assert dsn.scope is Scope.APP
95+
with container.build_child_container(scope=Scope.REQUEST) as child:
96+
assert child.resolve(_DbConnection).dsn == child.resolve(_Engine).dsn == "postgres://"
97+
98+
99+
def test_inline_provider_scope_is_fixed_at_first_registration() -> None:
100+
dsn = providers.Factory(lambda: "postgres://")
101+
102+
class Dependencies(Group):
103+
db_connection = providers.Factory(_DbConnection, scope=Scope.REQUEST, kwargs={"dsn": dsn})
104+
105+
container = Container(groups=[Dependencies])
106+
container.add_providers(providers.Factory(_Engine, kwargs={"dsn": dsn}, cache=True))
107+
108+
assert dsn.scope is Scope.REQUEST
109+
with pytest.raises(ValidationFailedError, match="InvalidScopeDependencyError"):
110+
container.validate()
111+
112+
113+
def test_inline_provider_with_explicit_scope_keeps_it() -> None:
114+
class Dependencies(Group, scope=Scope.REQUEST):
115+
handler = providers.Factory(
116+
_Handler, kwargs={"session": providers.Factory(_Session, scope=Scope.APP, cache=True)}
117+
)
118+
119+
a, b = _sessions_of_two_requests(Container(groups=[Dependencies]))
120+
121+
assert a is b
122+
123+
124+
def test_class_attribute_of_another_group_keeps_its_scope() -> None:
125+
class Shared(Group):
126+
session = providers.Factory(_Session, cache=True)
127+
128+
class Dependencies(Group, scope=Scope.REQUEST):
129+
handler = providers.Factory(_Handler, kwargs={"session": Shared.session})
130+
131+
a, b = _sessions_of_two_requests(Container(groups=[Shared, Dependencies]))
132+
133+
assert Shared.session.scope is Scope.APP
134+
assert a is b
135+
136+
137+
class _Outer:
138+
def __init__(self, handler: _Handler) -> None:
139+
self.handler = handler
140+
141+
142+
def test_provider_inline_in_an_inline_provider_takes_the_scope_down_the_chain() -> None:
143+
class Dependencies(Group, scope=Scope.REQUEST):
144+
outer = providers.Factory(
145+
_Outer,
146+
kwargs={
147+
"handler": providers.Factory(_Handler, kwargs={"session": providers.Factory(_Session, cache=True)})
148+
},
149+
)
150+
151+
container = Container(groups=[Dependencies])
152+
with container.build_child_container(scope=Scope.REQUEST) as first:
153+
a = first.resolve(_Outer).handler.session
154+
with container.build_child_container(scope=Scope.REQUEST) as second:
155+
b = second.resolve(_Outer).handler.session
156+
157+
assert a is not b
158+
159+
160+
def test_group_cannot_change_scope_of_registered_inline_provider() -> None:
161+
session = providers.Factory(_Session, cache=True)
162+
163+
class Dependencies(Group, scope=Scope.REQUEST):
164+
handler = providers.Factory(_Handler, kwargs={"session": session})
165+
166+
Container(groups=[Dependencies])
167+
168+
with pytest.raises(ProviderScopeFrozenError, match="Later"):
169+
170+
class Later(Group, scope=Scope.SESSION):
171+
shared_session = session
172+
173+
assert session.scope is Scope.REQUEST
174+
175+
176+
def test_inline_source_of_inline_derived_provider_takes_parent_scope() -> None:
177+
class Dependencies(Group, scope=Scope.REQUEST):
178+
handler = providers.Factory(
179+
_Handler, kwargs={"session": providers.Factory(_Session, cache=True).derive(lambda s: s)}
180+
)
181+
182+
a, b = _sessions_of_two_requests(Container(groups=[Dependencies]))
183+
184+
assert a is not b

0 commit comments

Comments
 (0)