Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

different calls to provide() in a singleton scope do not return a singleton #43

Open
jfaleiro opened this issue Nov 20, 2019 · 9 comments

Comments

@jfaleiro
Copy link

This test should not fail, i.e., a and b.a should be the same instance in a singleton scope:

   def test_graph_creation_with_binding_to_instance(self):
        class A:
            def do(self):
                return 1

        class B:                
            def __init__(self, a):
                self.a = a

        class Binding(BindingSpec):
            def configure(self, bind):
                bind('a', to_instance=A())
             

        graph = new_object_graph(classes=[A, B], binding_specs=[Binding()])
        a = graph.provide(A)
        self.assertEqual(1, a.do())

        b = graph.provide(B)
        self.assertEqual(1, b.a.do())        

        self.assertIs(a, b.a)

The last assertion fails:

AssertionError: <test_injection.Test.test_graph_creation_with_binding.<locals>.A object at 0x10422f310> is not <test_injection.Test.test_graph_creation_with_binding.<locals>.A object at 0x1043fa2d0>
@MrGreenTea
Copy link

MrGreenTea commented Apr 23, 2020

You're instantiating a new A() instance in Binding.configure.
Of course it's going to be a different instance than from graph.provide(A).

>>> graph.provide(A) is graph.provide(A)
False

But

>>> graph.provide(B) is graph.provide(B)
False
>>> graph.provide(B).a is graph.provide(B).a
True

@loweryjk
Copy link

loweryjk commented Nov 17, 2020

As @MrGreenTea said, this is not a bug. However, I'm seeing the same behavior when I: bind('foo', to_class=Foo, in_scope=pinject.SINGLETON)

@loweryjk
Copy link

loweryjk commented Nov 17, 2020

This test fails.

def test():
    class TestDependency:
        pass

    class TestSpec(pinject.BindingSpec):
        def configure(self, bind):
            bind('test_dependency', to_class=TestDependency, in_scope=pinject.SINGLETON)

    object_graph = pinject.new_object_graph(modules=None, binding_specs=[TestSpec()])
    request_one = object_graph.provide(TestDependency)
    request_two = object_graph.provide(TestDependency)
    assert request_two is request_one

@loweryjk
Copy link

After further review of the documentation, I think it's working as designed. However, I think the design should change. It is reasonable to expect that if a class is injectable with scope singleton and you ask object_graph to provide an instance of that class multiple times, then you'd always receive the same instance.

@MrGreenTea
Copy link

After further review of the documentation, I think it's working as designed. However, I think the design should change. It is reasonable to expect that if a class is injectable with scope singleton and you ask object_graph to provide an instance of that class multiple times, then you'd always receive the same instance.

You're not binding TestDependency as singleton but the name 'test_dependency'.
There is no explicit binding for TestDependency in your spec so of course separate instances will be provided.

@loweryjk
Copy link

The provide method expects a type not a string.

def test():
    class TestDependency:
        pass

    class TestSpec(pinject.BindingSpec):
        def configure(self, bind):
            bind('test_dependency', to_class=TestDependency, in_scope=pinject.SINGLETON)

    object_graph = pinject.new_object_graph(modules=None, binding_specs=[TestSpec()])
    request_one = object_graph.provide('test_dependency')
    request_two = object_graph.provide('test_dependency')
    assert request_two is request_one

The test above yields:

E           pinject.errors.WrongArgTypeError: wrong type for arg cls: expected class but got str

@crclz
Copy link

crclz commented Nov 12, 2021

use this trick: @loweryjk

def test_1():
    class TestDependency:
        pass

    class TestDependencyLocator:
        def __init__(self, test_dependency) -> None:
            self.item = test_dependency

    class TestSpec(pinject.BindingSpec):
        def configure(self, bind):
            G.a += 1
            bind("test_dependency", to_class=TestDependency)

    object_graph = pinject.new_object_graph(modules=None, binding_specs=[TestSpec()])
    request_one = object_graph.provide(TestDependencyLocator)
    request_two = object_graph.provide(TestDependencyLocator)
    assert request_two.item is request_one.item

@crclz
Copy link

crclz commented Nov 12, 2021

@loweryjk
or use this resolve_by_name magic:

def resolve_by_name(graph, name: str):
    class PowerfulLocator:
        def __init__(self, a) -> None:
            self.item = a

    ctx = {"r": []}

    code = """
def fn0(self, {name}):
    self.item = {name}
r.append(fn0)
    """

    code = code.replace("{name}", name)
    exec(code, {}, ctx)

    old_ctor = PowerfulLocator.__init__

    PowerfulLocator.__init__ = ctx["r"][0]

    # to make pinject happy
    PowerfulLocator.__init__.__module__ = old_ctor.__module__

    locator = graph.provide(PowerfulLocator)

    return locator.item


def test_2():
    class TestDependency:
        pass

    class TestSpec(pinject.BindingSpec):
        def configure(self, bind):
            bind("test_dependency", to_class=TestDependency)

    object_graph = pinject.new_object_graph(modules=None, binding_specs=[TestSpec()])

    # tests
    o1 = resolve_by_name(object_graph, "test_dependency")
    o2 = resolve_by_name(object_graph, "test_dependency")

    assert o1 is o2
    assert isinstance(o1, TestDependency)

@netanel-haber
Copy link

netanel-haber commented Nov 2, 2022

Instead of using exec, we implemented a solution that receives all of the class bindings, and creates a wrapper class for each one. For every wrapper class, we augmented the __signature__ of the init method so as to receive the snakecased name of the bound class (as pinject expects). Inside the init implementation, save the received class intstance to a named
underlying_dependency.
This is the init function, roughly:

def __init__(self, **kwargs):
   self.underlying_dependency = kwargs[<dep_name>]

# The signature that pinject sees:
def __init__(self,  <dep_name>):

Create a dict that maps each bound class to its corresponding wrapper class. Then create a custom provide function that receives a depended class, gets the respective wrapper class from the dict, calls provide on it, then .underlying_dependency on the result.

This way, you create wrapper classes once, upfront. The lib provide still creates a new instance of the passed wrapper class every time its called, but it is very lightweight, with just the necessary code to get pinject to provide the injected dependecy.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants