aspnet / DependencyInjection

[Archived] Contains common DI abstractions that ASP.NET Core and Entity Framework Core use. Project moved to https://github.com/aspnet/Extensions
Apache License 2.0
877 stars 320 forks source link

Replace services independent from their lifetime #618

Closed peterwurzinger closed 6 years ago

peterwurzinger commented 6 years ago

Hi,

I was wondering if it possible to replace a service implementation independently from its service lifetime..? For now, afaik, one is only able to call the IServiceCollection.Replace(...) - extension providing the lifetime of the service to be replaced.

The point is, that I only would like to specify the lifetime of a service once and only once. Imagine the following:

public IServiceCollection UseExtraCostlyDependencies(this IServiceCollection services) {
    services.AddSingleton(...);
    services.AddSingleton(...);
    services.AddTransient(...);
    services.AddScoped(...);
}

Looking at other extension methods to use fake implementations for those costly dependencies, I'm forced to replace them dependent on their lifetime:

public IServiceCollection UseFakesForExtraCostlyDependencies(this IServiceCollection services) {
    services.Replace(ServiceDescriptor.Singleton(...));
    services.Replace(ServiceDescriptor.Singleton(...));
    services.Replace(ServiceDescriptor.Transient(...));
    services.Replace(ServiceDescriptor.Scoped(...));
}

Think of them as being serveral extensions methods, I'm just too lazy to write them all down - but I guess you get the point.

Long story short: The higher the count of real/fake-implementation - correlations, the higher the chance to mismatch a service lifetime when replacing them with fakes. This would potentially lead to inconsistet application behavior in different stages.

Is there maybe already a solution to my problem? Do I maybe apply the pattern wrong?

kind regards, Peter

khellang commented 6 years ago

It should be easy enough to write an extension method that pulls an existing ServiceDescriptor and replaces it with a new one, keeping the ServiceType and Lifetime. Let me see if I can find one...

khellang commented 6 years ago

I knew I had something lying around in one of my test projects;

public static class ServiceCollectionExtensions
{
    public static IServiceCollection Replace<TService, TImplementation>(this IServiceCollection services)
        where TImplementation : TService
    {
        return services.Replace<TService>(typeof(TImplementation));
    }

    public static IServiceCollection Replace<TService>(this IServiceCollection services, Type implementationType)
    {
        return services.Replace(typeof(TService), implementationType);
    }

    public static IServiceCollection Replace(this IServiceCollection services, Type serviceType, Type implementationType)
    {
        if (services == null)
        {
            throw new ArgumentNullException(nameof(services));
        }

        if (serviceType == null)
        {
            throw new ArgumentNullException(nameof(serviceType));
        }

        if (implementationType == null)
        {
            throw new ArgumentNullException(nameof(implementationType));
        }

        if (!services.TryGetDescriptors(serviceType, out var descriptors))
        {
            throw new ArgumentException($"No services found for {serviceType.FullName}.", nameof(serviceType));
        }

        foreach (var descriptor in descriptors)
        {
            var index = services.IndexOf(descriptor);

            services.Insert(index, descriptor.WithImplementationType(implementationType));

            services.Remove(descriptor);
        }

        return services;
    }

    private static bool TryGetDescriptors(this IServiceCollection services, Type serviceType, out ICollection<ServiceDescriptor> descriptors)
    {
        return (descriptors = services.Where(service => service.ServiceType == serviceType).ToArray()).Any();
    }

    private static ServiceDescriptor WithImplementationType(this ServiceDescriptor descriptor, Type implementationType)
    {
        return new ServiceDescriptor(descriptor.ServiceType, implementationType, descriptor.Lifetime);
    }
}

You use it like this:

var services = new ServiceCollection();

services.AddSingleton<ISingleton, Singleton>();
services.AddScoped<IScoped, Scoped>();

services.Replace<ISingleton, FakeSingleton>();
services.Replace<IScoped, FakeScoped>();

var provider = services.BuildServiceProvider();

var singleton = provider.GetRequiredService<ISingleton>(); // FakeSingleton
var scoped = provider.GetRequiredService<IScoped>(); // FakeScoped

I wonder if these extensions is something MS would be willing to accept as a contribution? They're pretty handy for the exact scenario you outlined above; replacing services with fakes or mocks when unit testing.

peterwurzinger commented 6 years ago

That's absolutely what I was thinking of, I didn't know one is able to pull the Descriptor-Objects out of the IServiceCollection. Thanks! I would absolutely support integrating this extension, wondering if it's gonna happen.

Cheers, Peter

PS: I won't change the issue state by myself, so that it can be closed by a potential PR. Feel free to manually close it nevertheless.

davidfowl commented 6 years ago

@khellang Do you want to send a PR for this? I think this is generally useful.

/cc @Eilon @pakrym

khellang commented 6 years ago

Do you want to send a PR for this?

Sure! 😊

pakrym commented 6 years ago

Why wouldn't you care about lifetime of a service, or want it to be explicit?

This would make it so changing lifetime of service inside ASP.NET Core is a possible breaking change to apps that were unintentionally relying on inherited lifetime.

khellang commented 6 years ago

Why wouldn't you care about lifetime of a service, or want it to be explicit?

Cause you've already defined the lifetime when you first registered the service. Why would you want to duplicate this (and accidentally change it, potentially causing a lifetime mismatch) in your test code?

This would make it so changing lifetime of service inside ASP.NET Core is a possible breaking change to apps that were unintentionally relying on inherited lifetime.

Changing the lifetime of an existing service is always a breaking change anyway.

Say you have a singleton service that depends on an ASP.NET Core singleton service... You'd be in trouble if ASP.NET Core suddenly changed that registration to scoped or transient anyway.

I'm not sure why this changes anything?

khellang commented 6 years ago

It's pretty funny how different the worlds of app- and framework developers are 😂

peterwurzinger commented 6 years ago

Why wouldn't you care about lifetime of a service, or want it to be explicit?

Well, imho there are use cases where it could be quite useful to even reverse the definition of service lifetimes. I'm thinking about cases where it is critical to obtain new instances of a service to avoid unwanted side effects:

public class BankingTransaction {
    public BankingTransaction(IDatabaseContext ctx){
        ...
    }
}

In this example I would like to use a new instance of a database connection for every banking transaction instead of reusing a (request-)scoped connection. But I can't ensure that, I need to trust the caller, that an implementation of IDatabaseContext was registered as a transient service. To speak codewise, I cannot do that:

public static IServiceCollection UseCoreBankingServices (this IServiceCollection services) {
    //Add own services...

    //I need one and exactly one instance of IDatabaseContext, which was registered as a transient service
    services.AddTransientRequirement<IDatabaseContext>();

    return services;
}

Hope this isn't too fuzzy so far...

To get to the point of this issue: I know, that the functionality mentioned above will presumably never make it into the Microsoft DI package. But to at least be able to maintain service lifetimes at one point in the code, it would really be a nice feature to replace implementations by fakes/mocks, without having to repeatedly specify their lifetime.

pakrym commented 6 years ago

@khellang but the lifetime isn't a property of service it's property of implementation.

khellang commented 6 years ago

I don't know what to tell you. Maybe we're doing it wrong? :wink:

I've used this code in several projects, and since other people are asking for it, it seems like something that could be generally useful.

Anyway, it's up to you whether you want to accept the PR or not :smile:

davidfowl commented 6 years ago

This is useful , the problem I see with the impl is that it requires the existing registration to be in the service collection and it’s very inefficient since it has to scan.

khellang commented 6 years ago

it requires the existing registration to be in the service collection

How would you replace something that's not there to replace?

it’s very inefficient since it has to scan

Sure, but who cares? You really need this code to be blazing fast? We've been running this code in our unit- and integration tests for over a year and haven't noticed it.

davidfowl commented 6 years ago

How would you replace something that's not there to replace?

You can imagine an operation that basically stores the service descriptor with a null lifetime. When binding happens it would use the lifetime of the previous descriptor in the chain. This would require container changes though so I'm not too big on it.

Sure, but who cares? You really need this code to be blazing fast? We've been running this code in our unit- and integration tests for over a year and haven't noticed it.

It's not about i being "blazingly fast" but we already have TryAdd (that's already inefficient), not with Replace we have more linear scans per service. That could affect startup time in the long run.

khellang commented 6 years ago

That could affect startup time in the long run.

In my eyes that falls into the premature optimization bucket. If it becomes a problem for someone, it can be improved later without API changes.

ajcvickers commented 6 years ago

@davidfowl This is one of the reasons we built EntityFrameworkServicesBuilder and ServiceCollectionMap. It allows EF to define services and scopes which are then optionally replaced by database providers or the application without the the provider/app messing with or guessing at the scope.

aspnet-hello commented 6 years ago

This issue was moved to aspnet/Home#2328