simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 155 forks source link

How to use decorator for shared interface? #994

Closed GeraldLx closed 5 months ago

GeraldLx commented 5 months ago

I really love to separate logic in specific classes by using decorators, but I am struggling on using them when I resolve specific types.

I have an external library asking for IJob instances. (Quartz.NET)

For the sake of simplicity lets assume something like this:

public interface IJob
{
  public void Execute();
}

I have a sample Job like this:

public class MyJob : IJob
{
   public void Execute()
  {
    Console.WriteLine("MyJob");
  }
}

For integration purposes with Quartz.NET all I have to do is to write a job factory, which in my case is just using container.GetInstance

As I want to have multiple jobs, I want to have logging, exception handling and scope creation in one place. So I write decorators for this.

Essentially I write this:

    using var container = new Container();
    container.Register<MyJob>();
    container.RegisterDecorator<IJob, JobDecorator>();

    IJob job = container.GetInstance<MyJob>();
    job.Execute();

This code runs, but ignores the decorator. This is quite logic, as I tell Simple Injector to decorate all requests to IJob, but later I do request MyJob. So the decorator is not applied.

Is there any way to tell Simple Injector to use the decorator in that scenario? At the moment I "new" the decorators manually, but that feels quite ugly.

dotnetjunkie commented 5 months ago

Perhaps this StackOverflow answer helps? What the answer does is that it creates SimpleInjector.InstanceProducer instances like this:

var myJobProducer =
    Lifestyle.Transient.CreateProducer(typeof(IJob), typeof(MyJob), container);

While service instances for created InstanceProducer instances can't be resolved by calling container.GetInstance(), you can get the instance by calling myJobProducer.GetInstance() in this case. Resolves on an InstanceProducer go through the complete Simple Injector pipeline (because InstanceProducer instances are used under the covers by container.GetInstance() as well). As the service type of myJobProducer is IJob any decorator on IJob is applied as well.

The StackOverflow answer shows how those InstanceProducer instances are captured in a Dictionary, such that you can map the implementation back to the InstanceProducer allowing you to create the object graph for that particular MyJob.

I hope this helps.

GeraldLx commented 5 months ago

Thank you very much, seems to work as expected.

With your hint I made these extensions:

public static class ContainerExtensions
{
  private static readonly ConcurrentDictionary<Type, InstanceProducer> InstanceProducers = new();

  public static IJob GetJobInstance<TJob>(this Container container)
    where TJob : IJob
  {
    return CreateJobInstance(container, typeof(TJob));
  }

  public static IJob GetJobInstance(this Container container, Type jobType)
  {
    if (!jobType.IsAssignableTo(typeof(IJob)))
    {
      throw new ArgumentException("Type must implement IJob.", nameof(jobType));
    }

    return CreateJobInstance(container, jobType);
  }

  private static IJob CreateJobInstance(Container container, Type jobType)
  {
    var producer = InstanceProducers.GetOrAdd(
      jobType,
      Lifestyle.Transient.CreateProducer(typeof(IJob), jobType, container));

    var instance = producer.GetInstance();

    return (IJob)instance;
  }
}
dotnetjunkie commented 5 months ago

Sorry, the code you have will cause problems. InstanceProducer instances are not meant to be created on the fly and over and over again. They act as a mini container for one registration. Just as it's bad to create an infinite amount of container instances, it's bad to create an infinite amount of InstanceProducer instances. It can cause memory and performance problems.

So instead, I urge you to cache the InstanceProducer instances that you create, similar to what I demonstrated in the previously referenced StackOverflow answer.

GeraldLx commented 5 months ago

Thanks for your additional valuable content.

I updated my code to cache the producers.