Repository: reagento/dishka (python)

Issue:
bug: Provider doesn't override factory with lower scope
```python
from dishka import Provider, make_container, Scope, provide


class RealProvider(Provider):
    @provide(scope=Scope.REQUEST)
    def get_int(self) -> int:
        return 1


class FakeProvider(Provider):
    @provide(scope=Scope.APP)
    def get_int(self) -> int:
        return 42


class Interactor:
    def __init__(self, dep: int) -> None:
        self.dep = dep

    def __call__(self) -> int:
        return self.dep


class ServiceProvider(Provider):
    get_interactor = provide(Interactor, scope=Scope.REQUEST)


container = make_container(RealProvider(), ServiceProvider(), FakeProvider())
with container() as request_container:
    service = request_container.get(Interactor)
    assert service() == 42  # AssertionError 1 != 42
```

Expected behavior: `FakeProvider` has a bigger prior than `RealProvider`, so `int` dependency should be `42`

Btw, the following code works fine

```python
class RealProvider(Provider):
    @provide(scope=Scope.APP)
    def get_int(self) -> int:
        return 1

class FakeProvider(Provider):
    @provide(scope=Scope.REQUEST)
    def get_int(self) -> int:
        return 42
```

Pull request:
Defer scopes processing to handle overriding properly
/closes #435 

Patch:
diff --git a/src/dishka/registry_builder.py b/src/dishka/registry_builder.py
index 9a7f2ec..e0fe0f1 100644
--- a/src/dishka/registry_builder.py
+++ b/src/dishka/registry_builder.py
@@ -12,7 +12,7 @@ from .dependency_source import (
 from .entities.component import DEFAULT_COMPONENT, Component
 from .entities.factory_type import FactoryType
 from .entities.key import DependencyKey
-from .entities.scope import BaseScope, InvalidScopes, Scope
+from .entities.scope import BaseScope, InvalidScopes
 from .entities.validation_settigs import ValidationSettings
 from .exceptions import (
     AliasedFactoryNotFoundError,
@@ -127,7 +127,6 @@ class RegistryBuilder:
     ) -> None:
         self.scopes = scopes
         self.providers = providers
-        self.registries: dict[BaseScope, Registry] = {}
         self.dependency_scopes: dict[DependencyKey, BaseScope] = {}
         self.components: set[Component] = {DEFAULT_COMPONENT}
         self.alias_sources: dict[DependencyKey, Any] = {}
@@ -167,7 +166,8 @@ class RegistryBuilder:
                 self.alias_sources[provides] = alias_source
                 self.aliases[provides] = alias
 
-    def _init_registries(self) -> None:
+    def _make_registries(self) -> tuple[Registry, ...]:
+        registries: dict[BaseScope, Registry] = {}
         has_fallback = True
         for scope in self.scopes:
             registry = Registry(scope, has_fallback=has_fallback)
@@ -178,8 +178,12 @@ class RegistryBuilder:
             )
             for component in self.components:
                 registry.add_factory(context_var.as_factory(component))
-            self.registries[scope] = registry
+            registries[scope] = registry
             has_fallback = False
+        for key, factory in self.processed_factories.items():
+            scope = cast(BaseScope, factory.scope)
+            registries[scope].add_factory(factory, key)
+        return tuple(registries.values())
 
     def _process_factory(
             self, provider: BaseProvider, factory: Factory,
@@ -206,8 +210,6 @@ class RegistryBuilder:
             )
 
         self.processed_factories[provides] = factory
-        registry = self.registries[cast(Scope, factory.scope)]
-        registry.add_factory(factory)
 
     def _process_alias(
             self, provider: BaseProvider, alias: Alias,
@@ -241,7 +243,6 @@ class RegistryBuilder:
                 ])
 
         scope = self.dependency_scopes[alias_source]
-        registry = self.registries[scope]
 
         factory = alias.as_factory(scope, component)
         if (
@@ -265,26 +266,23 @@ class RegistryBuilder:
 
         self.dependency_scopes[factory.provides] = scope
         self.processed_factories[factory.provides] = factory
-        registry.add_factory(factory)
 
     def _process_generic_decorator(
             self, provider: BaseProvider, decorator: Decorator,
     ) -> None:
         found = []
         provides = decorator.provides.with_component(provider.component)
-        for registry in self.registries.values():
-            for factory in registry.factories.values():
+        for factory in self.processed_factories.values():
                 if factory.provides.component != provides.component:
                     continue
                 if factory.type is FactoryType.CONTEXT:
                     continue
                 if decorator.match_type(factory.provides.type_hint):
-                    found.append((registry, factory))
+                    found.append(factory)
         if found:
-            for registry, factory in found:
+            for factory in found:
                 self._decorate_factory(
                     decorator=decorator,
-                    registry=registry,
                     old_factory=factory,
                 )
         else:
@@ -320,25 +318,19 @@ class RegistryBuilder:
                     component=provider.component,
                 )],
             )
-        scope = self.dependency_scopes[provides]
-        registry = self.registries[scope]
-        # factory is expected to be as we already processed
-        # it according to dependency_scopes
-        old_factory = cast(Factory, registry.get_factory(provides))
+        old_factory = self.processed_factories[provides]
         self._decorate_factory(
             decorator=decorator,
-            registry=registry,
             old_factory=old_factory,
         )
 
     def _is_alias_decorated(
         self,
         decorator: Decorator,
-        registry: Registry,
         alias: Factory,
     ) -> bool:
         dependency = alias.dependencies[0]
-        factory = registry.get_factory(dependency)
+        factory = self.processed_factories.get(dependency)
         if factory is None:
             raise AliasedFactoryNotFoundError(dependency, alias)
         return factory.source is decorator.factory.source
@@ -346,7 +338,6 @@ class RegistryBuilder:
     def _decorate_factory(
         self,
         decorator: Decorator,
-        registry: Registry,
         old_factory: Factory,
     ) -> None:
         provides = old_factory.provides
@@ -356,7 +347,7 @@ class RegistryBuilder:
             )
         if (
             old_factory.type is FactoryType.ALIAS
-                and self._is_alias_decorated(decorator, registry, old_factory)
+                and self._is_alias_decorated(decorator, old_factory)
         ):
             return
         depth = self.decorator_depth[provides]
@@ -376,14 +367,14 @@ class RegistryBuilder:
             old_factory.provides.type_hint, decorated_component,
         )
         new_factory = decorator.as_factory(
-            scope=registry.scope,
+            scope=cast(BaseScope, old_factory.scope),  # all scopes validated
             new_dependency=old_factory.provides,
             cache=old_factory.cache,
             component=provides.component,
         )
         new_factory.provides = provides
-        registry.add_factory(old_factory)
-        registry.add_factory(new_factory)
+        self.processed_factories[old_factory.provides] = old_factory
+        self.processed_factories[new_factory.provides] = new_factory
 
     def _process_context_var(
             self, provider: BaseProvider, context_var: ContextVariable,
@@ -396,7 +387,6 @@ class RegistryBuilder:
                     "Define it explicitly in Provider or from_context"
                 ),
             )
-        registry = self.registries[context_var.scope]
         for component in self.components:
             factory = context_var.as_factory(component)
             if (
@@ -417,14 +407,18 @@ class RegistryBuilder:
                     factory,
                     self.processed_factories[factory.provides],
                 )
-            self.processed_factories[context_var.provides] = factory
-            registry.add_factory(factory)
+            self.processed_factories[factory.provides] = factory
 
     def build(self) -> tuple[Registry, ...]:
         self._collect_components()
         self._collect_provided_scopes()
         self._collect_aliases()
-        self._init_registries()
+
+        for provider in self.providers:
+            for factory in provider.factories:
+                self.dependency_scopes[
+                    factory.provides.with_component(provider.component)
+                ] = cast(BaseScope, factory.scope)
 
         for provider in self.providers:
             for factory in provider.factories:
@@ -439,19 +433,19 @@ class RegistryBuilder:
                 else:
                     self._process_normal_decorator(provider, decorator)
         self._post_process_generic_factories()
-        registries = list(self.registries.values())
+
+        registries = self._make_registries()
         if not self.skip_validation:
             GraphValidator(registries).validate()
-        return tuple(registries)
+        return registries
 
     def _post_process_generic_factories(self) -> None:
         found = [
-            (registry, factory)
-            for registry in self.registries.values()
-            for factory in registry.factories.values()
+            factory
+            for factory in self.processed_factories.values()
             if is_generic(factory.provides.type_hint)
         ]
-        for registry, factory in found:
+        for factory in found:
             origin = get_origin(factory.provides.type_hint)
             origin_key = DependencyKey(origin, factory.provides.component)
-            registry.add_factory(factory, origin_key)
+            self.processed_factories[origin_key] = factory


Test patch:
diff --git a/tests/unit/container/override/test_alias.py b/tests/unit/container/override/test_alias.py
index a1deddc..aa9f01a 100644
--- a/tests/unit/container/override/test_alias.py
+++ b/tests/unit/container/override/test_alias.py
@@ -86,3 +86,55 @@ def test_skip_cant_override() -> None:
         TestProvider(),
         validation_settings=ValidationSettings(nothing_overridden=False),
     )
+
+
+@pytest.mark.parametrize(
+    ("scope1", "scope2"),
+    [
+        (Scope.APP, Scope.REQUEST),
+        (Scope.REQUEST, Scope.APP),
+        (Scope.APP, Scope.APP),
+    ],
+)
+def test_override_alias_to_provide_different_scope(
+        scope1: Scope, scope2: Scope,
+) -> None:
+    p1 = Provider(scope1)
+    p1.provide(lambda: True, provides=bool)
+    p1.alias(bool, provides=int)
+
+    @p1.decorate
+    def decorate_int(a: int) -> int:
+        return -a
+
+    p2 = Provider(scope2)
+    p2.provide(lambda: 2, provides=int)
+    container = make_container(p1, p2)
+    with container() as request_container:
+        assert request_container.get(int) == 2
+
+
+@pytest.mark.parametrize(
+    ("scope1", "scope2"),
+    [
+        (Scope.APP, Scope.REQUEST),
+        (Scope.REQUEST, Scope.APP),
+        (Scope.APP, Scope.APP),
+    ],
+)
+def test_override_source_to_different_scope(
+        scope1: Scope, scope2: Scope,
+) -> None:
+    p1 = Provider(scope1)
+    p1.provide(lambda: False, provides=bool)
+    p1.alias(bool, provides=int)
+
+    @p1.decorate
+    def decorate_int(a: int) -> int:
+        return -a
+
+    p2 = Provider(scope2)
+    p2.provide(lambda: True, provides=bool)
+    container = make_container(p1, p2)
+    with container() as request_container:
+        assert request_container.get(int) == -1
diff --git a/tests/unit/container/override/test_context_var.py b/tests/unit/container/override/test_context_var.py
index 122880b..3f7c546 100644
--- a/tests/unit/container/override/test_context_var.py
+++ b/tests/unit/container/override/test_context_var.py
@@ -77,3 +77,47 @@ def test_skip_cant_override() -> None:
         TestProvider(),
         validation_settings=ValidationSettings(nothing_overridden=False),
     )
+
+
+@pytest.mark.parametrize(
+    ("scope1", "scope2", "app_context", "req_context"),
+    [
+        (Scope.APP, Scope.REQUEST, {}, {int: 2}),
+        (Scope.REQUEST, Scope.APP, {int: 2}, {}),
+        (Scope.APP, Scope.APP, {}, {int: 2}),
+    ],
+)
+def test_override_to_context(
+        scope1: Scope, scope2: Scope, app_context: dict, req_context: dict,
+) -> None:
+    p1 = Provider(scope1)
+    p1.provide(lambda: 1, provides=int)
+
+    @p1.decorate
+    def decorate_int(a: int) -> int:
+        return -a
+
+    p2 = Provider(scope2)
+    p2.from_context(provides=int)
+    container = make_container(p1, p2, context=app_context)
+    with container(context=req_context) as request_container:
+        assert request_container.get(int) == 2
+
+
+@pytest.mark.parametrize(
+    ("scope1", "scope2"),
+    [
+        (Scope.APP, Scope.REQUEST),
+        (Scope.REQUEST, Scope.APP),
+        (Scope.APP, Scope.APP),
+    ],
+)
+def test_override_context_to_provide(scope1: Scope, scope2: Scope) -> None:
+    p1 = Provider(scope1)
+    p1.from_context(provides=int)
+
+    p2 = Provider(scope2)
+    p2.provide(lambda: 2, provides=int)
+    container = make_container(p1, p2)
+    with container() as request_container:
+        assert request_container.get(int) == 2
diff --git a/tests/unit/container/override/test_provide.py b/tests/unit/container/override/test_provide.py
index 156f6a2..c3e8842 100644
--- a/tests/unit/container/override/test_provide.py
+++ b/tests/unit/container/override/test_provide.py
@@ -77,3 +77,26 @@ def test_skip_cant_override() -> None:
         TestProvider(),
         validation_settings=ValidationSettings(nothing_overridden=False),
     )
+
+
+@pytest.mark.parametrize(
+    ("scope1", "scope2"),
+    [
+        (Scope.APP, Scope.REQUEST),
+        (Scope.REQUEST, Scope.APP),
+        (Scope.APP, Scope.APP),
+    ],
+)
+def test_override_different_scope(scope1: Scope, scope2: Scope) -> None:
+    p1 = Provider(scope1)
+    p1.provide(lambda: 1, provides=int)
+
+    @p1.decorate
+    def decorate_int(a: int) -> int:
+        return -a
+
+    p2 = Provider(scope2)
+    p2.provide(lambda: 2, provides=int)
+    container = make_container(p1, p2)
+    with container() as request_container:
+        assert request_container.get(int) == 2
diff --git a/tests/unit/container/test_context_vars.py b/tests/unit/container/test_context_vars.py
index d9603c6..f2002ee 100644
--- a/tests/unit/container/test_context_vars.py
+++ b/tests/unit/container/test_context_vars.py
@@ -104,3 +104,8 @@ def test_decorate():
 
     with pytest.raises(InvalidGraphError):
         make_container(MyProvider(), context={int: 1})
+
+    p2 = Provider(Scope.APP)
+    p2.provide(lambda: 2, provides=int)
+    with pytest.raises(InvalidGraphError):
+        make_container(MyProvider(), p2)