Finistere / antidote

Dependency injection for Python
MIT License
90 stars 9 forks source link

Difference in Dependencies for register and inject #16

Closed keelerm84 closed 4 years ago

keelerm84 commented 4 years ago

Considering the following two bits of code.

I want to use Service directly, so I add the inject annotation and provide the only dependency required for the __init__ method.

from antidote import LazyConstantsMeta, inject, register

class Conf(metaclass=LazyConstantsMeta):
    DOMAIN = 'domain'
    PORT = 'port'

    def __init__(self):
        self._data = {"domain":'doman value', "port":"port value" }

    def get(self, key):
        return self._data[key]

@inject(dependencies=(Conf.DOMAIN,))
class Service:
    def __init__(self, configuration):
        self.configuration = configuration

    def call_me(self):
        print("Service is being called")
        print(self.configuration)

Service().call_me()

In this version of the code, I want the Service class to be registered as a dependency which I can inject into another class. As far as I understand, I should use the @register annotation and provide the dependencies there.

from antidote import LazyConstantsMeta, inject, register

class Conf(metaclass=LazyConstantsMeta):
    DOMAIN = 'domain'
    PORT = 'port'

    def __init__(self):
        self._data = {"domain":'doman value', "port":"port value" }

    def get(self, key):
        return self._data[key]

@register(dependencies=(None, Conf.DOMAIN,))
class Service:
    def __init__(self, configuration):
        self.configuration = configuration

    def call_me(self):
        print("Service is being called")
        print(self.configuration)

@inject(dependencies=(Service,))
class Testing:
    def __init__(self, service: Service):
        self.service = service

    def test(self):
        self.service.call_me()

Testing().test()

In order to get this to work, I have to provide None as the first dependency to the Service class, which I didn't have to do with the @injection annotation. Is this mismatch expected, or am I somehow misunderstanding how to appropriately use the library?

If I don't provide none, I get the error TypeError: __init__() missing 1 required positional argument: 'configuration'

Finistere commented 4 years ago

@register does not work in the same way as you might think:

@register(dependencies=(None, Conf.DOMAIN,))
class Service:
    def __init__(self, configuration):
        self.configuration = configuration

is actually equivalent to:

class Service:
    @inject(dependencies=(None, Conf.DOMAIN,))
    def __init__(self, configuration):
        self.configuration = configuration

Using @inject directly on the class, should not be supported because Service won't be a class anymore but a instance of the function wrapper:

@inject(dependencies=(Conf.DOMAIN,))
class Service:
    def __init__(self, configuration):
        self.configuration = configuration

    def call_me(self):
        print("Service is being called")
        print(self.configuration)

import inspect
inspect.isclass(Service) # False

I will add an error to make this explicit. If you ever need to only inject dependencies in a class, consider using @wire (which is what @register uses underneath).

Regarding the positional arguments in @register, it was the expected behavior. But seeing your example, I realize how counterintuitive it is. So I intend to change it to make your initial guess @register(dependencies=(Conf.DOMAIN,)) work. I will double check in depth later today.

However using @inject directly will always need None to be specified as it can't know that it is a method.

In the meantime, consider using @register(dependencies=dict(configuration=Conf.DOMAIN)).

Thanks for raising the issue. :)

keelerm84 commented 4 years ago

Okay, so to make sure I fully understand before closing this issue . . .

My usage of @register in the second code sample I provied is correct but instead of @inject on Testing I should instead use @wire(dependencies(Service,), methods={"__init__"]) on the Testing class?

Also as a side note, I've really enjoyed this library so far. Thank you for contributing this as open source!

Finistere commented 4 years ago

My usage of @register in the second code sample I provied is correct

yes

instead of @inject on Testing I should instead use @wire(dependencies(Service,), methods={"__init__"]) on the Testing class

Yes, if you only want to inject services, you either use @wire on the class or @inject on __init__() directly.

The usage of @inject on a class directly is incorrect, it is only meant to be used on functions. So in your example the class will be treated as a function. While this works from a practical standpoint, it changes the nature of Testing which is a bad thing. So this is a bug.

As you have only 1 method to inject, you could in this case use either:

@wire(methods=['__init__'])
class Testing:
    def __init__(self, service: Service):
        self.service = service

    def test(self):
        self.service.call_me()

or

class Testing:
    @inject
    def __init__(self, service: Service):
        self.service = service

    def test(self):
        self.service.call_me()

Note that you don't even need to specify the dependencies explicitly as they can be inferred from the type annotation.

@wire is meant as a helper when you want to inject multiple methods of a class or wire a herited method (see wire_super argument).

As @wire is aware that it injects method, I intend to change how positional dependencies are working for it. It would not take into account the self argument so that @wire(dependencies=(Conf.DOMAIN,)) would work. This should be released during the week. :)

Glad you like it ! If you find any other issue or missing feature I'm definitely interested to know. :)

keelerm84 commented 4 years ago

Thank you for the thorough explanation!

Finistere commented 4 years ago

Just FYI, actually found a proper way to do:

class Service:
    @inject(dependencies=(Conf.DOMAIN,))
    def __init__(self, configuration):
        self.configuration = configuration

    def call_me(self):
        print("Service is being called")
        print(self.configuration)

and

@register(dependencies=(Conf.DOMAIN,))
class Service:
    def __init__(self, configuration):
        self.configuration = configuration

    def call_me(self):
        print("Service is being called")
        print(self.configuration)

Will publish it with the 0.7 this week with your patch.