Skip to content

Commit 6dc7188

Browse files
committed
Fix non-deterministic collection order for module-scanned injectables
_find_objects_in_module collected discovered classes into a set, so Sequence[T]/Mapping[T] ordering was hash-order (id()-derived) rather than the documented registration order whenever injectables came from a scanned module instead of an explicit list. Directory walks also relied on Path.iterdir(), which isn't guaranteed stable. Switch discovery to an insertion-ordered dict-as-set and sort the directory walk, so scanned modules produce deterministic results. Fixes #144
1 parent 7497a17 commit 6dc7188

4 files changed

Lines changed: 62 additions & 11 deletions

File tree

test/unit/services/collection_scan/__init__.py

Whitespace-only changes.
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
from typing import Protocol
2+
3+
from wireup import injectable
4+
5+
6+
class Greeter(Protocol):
7+
def hi(self) -> str: ...
8+
9+
10+
@injectable(as_type=Greeter, qualifier="delta")
11+
class Delta:
12+
def hi(self) -> str:
13+
return "delta"
14+
15+
16+
@injectable(as_type=Greeter, qualifier="beta")
17+
class Beta:
18+
def hi(self) -> str:
19+
return "beta"
20+
21+
22+
@injectable(as_type=Greeter)
23+
class Alpha:
24+
def hi(self) -> str:
25+
return "alpha"
26+
27+
28+
@injectable(as_type=Greeter, qualifier="gamma")
29+
class Gamma:
30+
def hi(self) -> str:
31+
return "gamma"

test/unit/test_discovery_order.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import unittest
2+
from collections.abc import Sequence
3+
4+
import wireup
5+
6+
from test.unit.services.collection_scan import greeters
7+
8+
9+
class DiscoveryOrderTest(unittest.TestCase):
10+
def test_module_scan_collection_injection_has_deterministic_order(self):
11+
container = wireup.create_sync_container(injectables=[greeters])
12+
result = [g.hi() for g in container.get(Sequence[greeters.Greeter])]
13+
14+
self.assertEqual(["alpha", "beta", "delta", "gamma"], result)
15+
16+
def test_module_scan_discovery_order_is_stable_across_repeated_scans(self):
17+
first = [g.hi() for g in wireup.create_sync_container(injectables=[greeters]).get(Sequence[greeters.Greeter])]
18+
second = [g.hi() for g in wireup.create_sync_container(injectables=[greeters]).get(Sequence[greeters.Greeter])]
19+
20+
self.assertEqual(first, second)

wireup/_discovery.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ def _is_valid_wireup_target(obj: Any) -> bool:
2424
# "from flask import g" would cause a hasattr call to g outside of app context.
2525
return (isinstance(obj, FunctionType) or inspect.isclass(obj)) and hasattr(obj, "__wireup_registration__")
2626

27-
all_targets = {
28-
m for module in injectable_modules for m in _find_objects_in_module(module, predicate=_is_valid_wireup_target)
29-
}
27+
all_targets: dict[type, None] = {}
28+
for module in injectable_modules:
29+
all_targets.update(dict.fromkeys(_find_objects_in_module(module, predicate=_is_valid_wireup_target)))
3030

3131
for cls in all_targets:
3232
reg = getattr(cls, "__wireup_registration__", None)
@@ -39,14 +39,14 @@ def _is_valid_wireup_target(obj: Any) -> bool:
3939
return abstract_registrations, injectable_registrations
4040

4141

42-
def _find_objects_in_module(module: ModuleType, predicate: Callable[[Any], bool]) -> set[type]:
43-
classes: set[type[Any]] = set()
42+
def _find_objects_in_module(module: ModuleType, predicate: Callable[[Any], bool]) -> list[type]:
43+
classes: dict[type[Any], None] = {}
4444

45-
def _module_get_objects(m: ModuleType) -> set[type]:
46-
return {obj for _, obj in inspect.getmembers(m) if predicate(obj)}
45+
def _module_get_objects(m: ModuleType) -> list[type]:
46+
return [obj for _, obj in inspect.getmembers(m) if predicate(obj)]
4747

4848
def _find_in_path(path: Path, parent_module_name: str) -> None:
49-
for file in path.iterdir():
49+
for file in sorted(path.iterdir()):
5050
if file.name == "__pycache__":
5151
continue
5252

@@ -60,12 +60,12 @@ def _find_in_path(path: Path, parent_module_name: str) -> None:
6060
parent_module_name if file.name == "__init__.py" else f"{parent_module_name}.{file.name[:-3]}"
6161
)
6262
sub_module = importlib.import_module(full_module_name)
63-
classes.update(_module_get_objects(sub_module))
63+
classes.update(dict.fromkeys(_module_get_objects(sub_module)))
6464

6565
if f := module.__file__:
6666
if f.endswith("__init__.py"):
6767
_find_in_path(Path(f).parent, module.__name__)
6868
else:
69-
classes.update(_module_get_objects(module))
69+
classes.update(dict.fromkeys(_module_get_objects(module)))
7070

71-
return classes
71+
return list(classes)

0 commit comments

Comments
 (0)