Skip to content
This repository has been archived by the owner on Nov 2, 2018. It is now read-only.

Contextual information when defining creation of dependencies #498

Closed
MatthewLymer opened this issue Mar 6, 2017 · 9 comments
Closed

Contextual information when defining creation of dependencies #498

MatthewLymer opened this issue Mar 6, 2017 · 9 comments
Milestone

Comments

@MatthewLymer
Copy link

MatthewLymer commented Mar 6, 2017

This is a feature request, so please forgive me if this is the wrong format / wrong spot.

I ran into an issue when using aspnet/Logging in that I was trying to use the non-generic interface ILogger.

The problem is that when creating an instance of ILogger the logger requires type information to determine the message-scope of the logger.

An example of the current behavior is:

public class UsersController : Controller
{
    private readonly ILogger _logger;

    public UsersController(ILogger<UsersController> logger)
    {
        _logger = logger;
    }
}

Where my desired constructor would look like:

public UsersController(ILogger logger)
{
    _logger = logger;
}

The problem here is that the generic definition is redundant, as the actual interface does nothing with T at all. I get suggestions from ReSharper telling me to use the non-generic interface.

Using the non-generic interface fails, however, as the it's not possible to tell aspnet/DependencyInjection how to create an instance of Logger.

The feature I am requesting is to have contextual information of the type the dependency is going to be injected into. This would be helpful not only for my particular use case, but for other use cases like when using decorators.

I syntax I am proposing would be:

services.AddTransient<ILogger>(
    (s, ctx) => s.GetService<ILoggerFactory>().CreateLogger(ctx.DestinationType));

Where ctx.DestinationType would be the type that the given instance is being injected into.

@khellang
Copy link
Contributor

khellang commented Mar 7, 2017

I've asked for a similar, but slightly different feature before, to support decoration of open generic types; get access to the requested service type in the factory delegate.

Example:

services.AddScoped(typeof(IService<>), typeof(Service<>));

// the call below is basically a way to do decoration with MS.Ext.DI...
services.AddScoped(typeof(IService<>), (provider, context) =>
{
    var typeArgument = context.RequestedType.GetGenericArguments().First();

    var decoratorType = typeof(DecoratedService<>).MakeGenericType(typeArgument);

    var decoratedInstance = /* get the decorated instance from the service provider */

    return ActivatorUtilities.CreateInstance(provider, decoratorType, decoratedInstance);
});

Where context.RequestedType is the actual service type passed to the service provider, i.e. provider.GetService<IService<string>>() -> typeof(IService<string>).

I think the main issue here, is that this would bring in more "requirements" for MS.Ext.DI adapters. Suddenly, they all have to be able to provide this information.

@seesharper
Copy link
Member

Maybe this could be used as a ground for further discussion.

LightInject provides the RegisterConstructorDependency/RegisterPropertyDependency methods that provides "context" related to the dependency being injected.

The following example shows the example with creating a logger that is tied to the type that requests the logger
http://www.lightinject.net/webapirequestlogging/

@carusology
Copy link

carusology commented Apr 5, 2017

Huh. I stumbled here due to this little conversation, which stemmed from a StackOverflow question I posted some time ago with the same general concern here. I'd like to know where @khellang asked for this feature before. Is it another issue?

In case it's helpful, my question on StackOverflow is clear about what I'm asking for, but not why this functionality would be helpful. The rationale becomes clear when you use composition to break down behaviors into wrappers. Since specificity is the soul of narrative, let me give an example of how that can be used:

Let's say I have an IRepository<TModel> concept, with two classes that I can store in it, FreeSpirit and Curmudgeon. I'll define those here:

public interface IRepository<TModel> where TModel: Model {
    public TModel GetById(Guid id);
    public TModel Create(TModel toCreate);
}

public class FreeSpirit : Model {
    public Guid Id { get; }
    public String Name { get; }

    public FreeSpirit(Guid id, String name) {
        this.Id = id;
        this.Name = name;
    }
}

public class Curmudgeon : Model {
    public Guid Id { get; }
    public DateTime EventTimestamp { get; }

    public Curmudgeon(Guid id, DateTime eventTimestamp) {
        this.Id = id;
        this.EventTimestamp = eventTimestamp;
    }
}

Assuming a base persistence implementation of StandardRepository<TModel>, we can register this code with ASP.NET's Dependency Injection tooling like so:

services.AddScoped(typeof(IRepository<>), typeof(StandardRepository<>));

Now, let's say that every time someone updates a Curmudgeon we want to know about it, recording that delta to some audit log. So we create an [Audited] attribute, apply it to Curmudgeon, and create an AuditingRepositoryWrapper<TModel> implementation, like so:

[Audited]
public class Curmudgeon : Model {
    public Guid Id { get; }
    public DateTime EventTimestamp { get; }

    public Curmudgeon(Guid id, DateTime eventTimestamp) {
        this.Id = id;
        this.EventTimestamp = eventTimestamp;
    }
}

public class AuditingRepositoryWrapper<TModel> : IRepository<TModel> {

    private IRepository<TModel> inner { get; }
    private IAuditingService auditService { get; }

    public AuditingRepositoryWrapper(IRepository<TModel> inner, IAuditingService auditService) {
        this.inner = inner;
        this.auditService = auditService;
    }

    public TModel GetById(Guid id) {
        return this.inner.GetById(id);
    }

    public TModel Create(TModel toCreate) {
        var result = this.inner.Create(toCreate);

        this.auditService.AuditChanges(toCreate, result);

        return result;
    }

}

Now registration is tricky. I need to wrap my StandardRepository<TModel> implementations with an AuditingRepositoryWrapper<TModel> if and only if there is an [Audited] attribute. Playing off @khellang's example of a context variable, I could do the following:

Generic, desired result:

services.AddScoped(typeof(StandardRepository<>));
services.AddScoped(typeof(IAuditingService), typeof(AuditingService));

services.AddScoped(typeof(IService<>), (provider, context) => {
    var typeArgument = context.RequestedType.GetGenericArguments().First();

    var isAudited =
        typeArgument
            .GetTypeInfo()
            .CustomAttributes
            .Any(attr => attr.AttributeType == typeof(AuditedAttribute));

    // Would need to solve the lack of closing type for the repository variable at this point.
    // (I think the best way to solve this is with a factory at this point that takes in the 'context'.)
    // See: https://github.com/invio/Invio.Extensions.DependencyInjection

    IRepository<?> repository = provider.GetRequiredService(typeof(StandardRepository<?>), typeArgument);
    if (isAudited) {
        repository = new AuditingRepositoryWrapper<?>(
            provider.GetRequiredService(repository, typeof(IAuditingService));
        );
    }

    return repository;
});

My team gets around this today by doing something that is similar to the following:

services.AddScoped(typeof(StandardRepository<>));
services.AddScoped(typeof(IAuditingService), typeof(AuditingService));

services.AddRepository<FreeSpirit>();
services.AddRepository<Curmudgeon>();
[ ... ]

public static IServiceCollection AddRepository<TModel>(this IServiceCollection services) {
    var isAudited =
        typeof(TModel)
            .GetTypeInfo()
            .CustomAttributes
            .Any(attr => attr.AttributeType == typeof(AuditedAttribute));

    if (isAudited) {
        services.AddScoped(
            typeof(IRepository<TModel>),
            (provider) => {
                new AuditingRepositoryWrapper<TModel>(
                    provider.GetRequiredService<IAuditingService>(),
                    provider.GetRequiredService<StandardRepository<TModel>()
                );
            }
        );
    } else {
        services.AddScoped(typeof(IRepository<TModel>), typeof(StandardRepository<TModel>));
    }

    return services;
});

The key to this workaround is not using open generics. They are closed when they are registered as services in the services collection. This only works when you know how you want to register the service on boot. If you want to register the service differently based upon some kind of user-level configuration, it's possible, but it gets more complicated.

I would love to know what Microsoft's stance on this feature is. If they never want to support it, that's fine, I'll just use other DI frameworks. I've banged my head against it already with my own library.

@khellang
Copy link
Contributor

khellang commented Apr 5, 2017

It' been mentioned in #474 as well. I can't remember if it was an issue, in chat or in person, but it's definitely been discussed.

@carusology
Copy link

Thanks, @khellang. For what it's worth, StructureMap solves the problems with Microsoft's DI around composition. We've internally decided to go that way the next time we have to invest into our DI logic.

@seesharper
Copy link
Member

@khellang Seem to remember you played around with decorator support at one time?

Anyways, thought I just wanted to show you how this could be solved in LightInject

    using System;
    using System.Diagnostics;
    using System.Reflection;
    using LightInject;

    class Program
    {
        static void Main(string[] args)
        {            
            var container = new ServiceContainer();
            container.Register(typeof(IService<>), typeof(Service<>));
            container.Decorate(typeof(IService<>), typeof(ServiceDecorator<>),
                sr => sr.ImplementingType.GetGenericArguments()[0].GetTypeInfo().IsDefined(typeof(AuditedAttribute)));

            var fooService = container.GetInstance<IService<Foo>>();
            Debug.Assert(fooService is Service<Foo>);
            var barService = container.GetInstance<IService<Bar>>();
            Debug.Assert(barService is ServiceDecorator<Bar>);
        }
    }

    public interface IService<T>
    {
    }

    public class Service<T> : IService<T>
    {
        
    }

    public class ServiceDecorator<T> : IService<T>
    {
        public ServiceDecorator(IService<T> service)
        {
        }
    }


    public class AuditedAttribute : Attribute
    {
    }


    public class Foo
    {
    }

    [Audited]
    public class Bar
    {
    }

@khellang
Copy link
Contributor

khellang commented Apr 6, 2017

@seesharper Yes, it's actually published as a NuGet package and the source is here: https://github.com/khellang/Scrutor

The only issue left is to be able to decorate open generics. The problem is that, unlike in LightInject, the MS.Ext.DI factory doesn't give you access to ImplementingType, so there's no way of know the actual generic arguments being asked for... 😢

@carusology
Copy link

@khellang found it #450

@muratg muratg added this to the Discussions milestone Apr 14, 2017
@Eilon
Copy link
Member

Eilon commented Jun 9, 2017

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.

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

6 participants