meadsteve / lagom

πŸ“¦ Autowiring dependency injection container for python 3
https://lagom-di.readthedocs.io/en/latest/
MIT License
283 stars 15 forks source link

how to apply Decorator design pattern with lagom? #165

Open Eldar1205 opened 3 years ago

Eldar1205 commented 3 years ago

Hi, How to apply the Decorator design pattern with lagom, on single instance registrations and/or collection registrations?

Example how I used to do it with a DI container from C# .Net called SimpleInjector (if same syntax were to be in lagom):

class HttpClient(ABC):
    async def send(self, request: Request) -> Response:
        pass

class ActualHttpClient(HttpClient):
    async def send(self, request: Request) -> Response:
        # Implement actual request send, e.g. with aiohttp

class LoggingHttpClientDecorator(HttpClient):
    def __init__(self, decoratee: HttpClient):
        self.__decoratee = decoratee
    async def send(self, request: Request) -> Response:
        print("Sending request")
        response = await self.__decoratee.send(request);
        print("Received response")
        return response;

container = Container()
container[HttpClient] = Singleton(ActualHttpClient)
container.RegisterSingletonDecorator[HttpClient, LoggingHttpClientDecorator](); # mimics the SimpleInjector syntax

With this code, resolving HttpClient would return LoggingHttpClientDecorator decorating ActualHttpClient. SimpleInjector also supports collection injections, e.g. injecting something like Iterable[HttpClient], and apply a decorator to all of the HttpClient instances registered to the collection.

meadsteve commented 3 years ago

@Eldar1205 that's a good question. At the moment lagom doesn't have any pattern for "apply decorator X to all things of type Y" or anything similar.

For your specific example I've generally favoured being explicit so I would have something like:

container = Container()
container[HttpClient] = Singleton(lambda c: LoggingHttpClientDecorator(c[ActualHttpClient]))

but I'll keep this issue open as I think having a way of doing this might be useful.

Eldar1205 commented 3 years ago

Thank you! With SimpleInjector the decorator could other injectables in the constructor so library support for this feature has advantages over users doing it themselves with lambda. I hope to find the time to contribute it myself, are there tips/expectations of me for that?

On Mon, Aug 30, 2021, 14:15 Steve B @.***> wrote:

@Eldar1205 https://github.com/Eldar1205 that's a good question. At the moment lagom doesn't have any pattern for "apply decorator X to all things of type Y" or anything similar.

For your specific example I've generally favoured being explicit so I would have something like:

container = Container()container[HttpClient] = Singleton(lambda c: LoggingHttpClientDecorator(c[ActualHttpClient]))

but I'll keep this issue open as I think having a way of doing this might be useful.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/meadsteve/lagom/issues/165#issuecomment-908257278, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIFRN35BJF67UVX6C5FP6DT7NR6LANCNFSM5CVTH2FA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

meadsteve commented 3 years ago

Hmmmm. Not sure I have any useful tips/expectations other than I'm trying to keep the test coverage high for the project (though I'm being pragmatic about this).

I'm not sure about implementation but what would you think about an interface like this:

# this step would tell the container that any HttpClient should have the following decorator applied and how to apply it.
container.register_decorator(HttpClient, lambda client: LoggingHttpClientDecorator(client))
# I was thinking about a lambda for the second argument so extra config can be supplied
container.register_decorator(HttpClient, lambda client: LoggingHttpClientDecorator(client, level=INFO))

# this could then be a decorated singleton
container[HttpClient] = Singleton(ActualHttpClient)
meadsteve commented 3 years ago

oh and there's a section of the docs about contributing to the project: https://lagom-di.readthedocs.io/en/latest/development_of_lagom/ though to be honest it could do with some extra :laughing:

Eldar1205 commented 3 years ago

Thanks for your attention and collaboration with this issue :) I think your suggestion for decorator with lambda would requires users of this feature to do more work in the following case: Imagine the decorator has another dependency other than the decorated class, e.g. Logger, then as a user I would need to write the lambda to include that other dependency which should also be resolved from the container somehow. Now imagine I wrote that down, and my decorator was changed to have an additional dependency in the constructor - now I need to fix my lambda registration to match that as well.

An API that supports both container.register_decorator(HttpClient, LoggingHttpClientDecorator) or container.register_decorator(HttpClient, Singleton(LoggingHttpClientDecorator)) would resolve both this issues if the container would inject into LoggingHttpClientDecorator the decorated HttpClient and any other dependencies it has just with any normal registered class. Note that users should be able to apply multiple decorator registrations such that the other matter to determine the decorator chain, e.g. LoggingHttpClientDecorator decorator which is then decorated itself with CachingHttpClientDecorator

meadsteve commented 3 years ago

hmmm. I really like your suggestion. It's quite easy to read. My only concern about it is that it could end up making it easy to create some construction infinite loops depending on how the decoration is implemented.

Maybe it's worth trying out. If it doesn't work then something like the following is also an option:

container.register_decorator(HttpClient, lambda decorated, c: LoggingHttpClientDecorator(decorated, logger=c[Logger]))

having the container passed as an argument to the lambda is a pattern I've used in other parts of the library so it shouldn't be surprising.

thiromi commented 1 year ago

Hum, I believe Python has already the decorator built-in, as opposed to C#, so I I'm not sure how useful is to apply the decorator pattern in Python πŸ€”

from functools import wraps
from logging import Logger

def logit(logger:str | None = None):
    def logging_decorator(func):
        @wraps(func)
        def wrapped_function(*args, **kwargs):
            logger = logging.getLogger(name)
            logger.info("Sending request")
            try:
                return func(*args, **kwargs)
            finally:
                logger.info("Received response")
        return wrapped_function
    return logging_decorator

class HttpClient(ABC):
    async def send(self, request: Request) -> Response:
        pass

class ActualHttpClient(HttpClient):
    @logit(logger=f"{__name__}.ActualHttpClient")
    async def send(self, request: Request) -> Response:
        # Implement actual request send, e.g. with aiohttp

container = Container()
container[HttpClient] = Singleton(ActualHttpClient)

And I'd agree with @meadsteve regarding calling it explicitly instead

container = Container()
container[HttpClient] = Singleton(lambda c: LoggingHttpClientDecorator(c[ActualHttpClient]))
Eldar1205 commented 1 year ago

The Python decorator applies to functions, nit interfaces, so how would I use it to decorate an interface so the lagom container resolves the decorator implementation wrapping another class I want?

Re. registering a lambda, how can I use to inject additional dependencies into my decorator?

On Mon, Sep 18, 2023, 12:40 Thiago Hiromi @.***> wrote:

Hum, I believe Python has already the decorator built-in https://peps.python.org/pep-0318/, as opposed to C#, so I I'm not sure how useful is to apply the decorator pattern in Python πŸ€”

from functools import wrapsfrom logging import Logger def logit(logger:str | None = None): def logging_decorator(func): @wraps(func) def wrapped_function(*args, *kwargs): logger = logging.getLogger(name) logger.info("Sending request") try: return func(args, **kwargs) finally: logger.info("Received response") return wrapped_function return logging_decorator class HttpClient(ABC): async def send(self, request: Request) -> Response: pass class ActualHttpClient(HttpClient): @logit(logger=f"{name}.ActualHttpClient") async def send(self, request: Request) -> Response:

Implement actual request send, e.g. with aiohttp

container = Container()container[HttpClient] = Singleton(ActualHttpClient)

And I'd agree with @meadsteve https://github.com/meadsteve regarding calling it explicitly instead

container = Container()container[HttpClient] = Singleton(lambda c: LoggingHttpClientDecorator(c[ActualHttpClient]))

β€” Reply to this email directly, view it on GitHub https://github.com/meadsteve/lagom/issues/165#issuecomment-1723068796, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIFRN3RO4DFZVYT27F5WY3X3AJJXANCNFSM5CVTH2FA . You are receiving this because you were mentioned.Message ID: @.***>

thiromi commented 1 year ago

how would I use it to decorate an interface so the lagom container resolves the decorator implementation wrapping another class I want?

That's a good question. If the decorator has dependencies to be resolved by Lagom, it'd be a bit tricky to apply, indeed.

Re. registering a lambda, how can I use to inject additional dependencies into my decorator?

I'd say DI containers should hide the object creation logic from their clients, not from themselves 😬 it's okay to manually add an additional dependency to the lambda, since you're doing it once.

also implementing this behavior would add unnecessary complexity to the library, for example:

What to do if the decorator does not depend on the decoratee?

class LoggingHttpClientDecorator(HttpClient):
    def __init__(self):
        pass

container = Container()
container.register_decorator(HttpClient, LoggingHttpClientDecorator)  # ???

what about a decorator who abused the DI by requesting two instances from the same type?

class LoggingHttpClientDecorator(HttpClient):
    def __init__(self, client1: HttpClient, client2: HttpClient):
        pass

container = Container()
container.register_decorator(HttpClient, LoggingHttpClientDecorator)

would it be possible to stack up decorator calls?

container.register_decorator(HttpClient, LoggingHttpClientDecorator)
container.register_decorator(HttpClient, RedisCachedHttpClientDecorator)

If so, would it be possible to overwrite them for testing?

container.unregister_decorator(HttpClient, RedisCachedHttpClientDecorator)   # I don't want redis caching results in my tests!
container.unregister_decorator(HttpClient, lambda: RedisCachedHttpClientDecorator)  # how to handle this?

using lambda with predefined arguments, obliging the use of decoratee and container would deal with some of the questions above, but it seems it defeats the purpose of auto-wiring the classes only with their types. It'd be almost the same as registering a type with a lambda, only with an injected decoratee.

But I'm not the one that decides this, though πŸ˜›

meadsteve commented 1 year ago

I think my hesitation here is that I don't really see much advantage of

container = Container()
container[HttpClient] = Singleton(ActualHttpClient)
container.RegisterSingletonDecorator[HttpClient, LoggingHttpClientDecorator](); 

over

container = Container()
container[HttpClient] = Singleton(lambda: LoggingHttpClientDecorator(ActualHttpClient()))

I'm not strongly against it but it introduces a few concepts to lagom that don't already exist. The biggest of which is ordering of application of these registered decorators. Currently ordering isn't a concept in lagom.

I can see how this might be useful in c# where you had lots of classes implementing a common interface and you want them all decorated in the same way. But this isn't really something I've seen used much in python.

Eldar1205 commented 1 year ago

With the described lambda we don't have 2 things:

  1. Ability to resolve additional decorator/decoratee dependencies using the DI container
  2. When decorator/decorated have new dependencies we need to modify the lambda since constructors are invoked directly, instead of the DI container auto-handling the injection of dependencies based on constructor signatures

On Wed, Sep 20, 2023, 22:21 Steve B @.***> wrote:

I think my hesitation here is that I don't really see much advantage of

container = Container()container[HttpClient] = Singleton(ActualHttpClient)container.RegisterSingletonDecorator[HttpClient, LoggingHttpClientDecorator]();

over

container = Container()container[HttpClient] = Singleton(lambda: LoggingHttpClientDecorator(ActualHttpClient()))

I',m not strongly against it but it introduces a few concepts to lagom that don't already exist. The biggest of which is ordering of application of these registered decorators. Currently ordering isn't a concept in lagom.

β€” Reply to this email directly, view it on GitHub https://github.com/meadsteve/lagom/issues/165#issuecomment-1728302282, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIFRNY4QSSJMVBXMLL2IADX3M65BANCNFSM5CVTH2FA . You are receiving this because you were mentioned.Message ID: @.***>