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

Add a way to replace services in an IServiceCollection #155

Closed
halter73 opened this issue Dec 16, 2014 · 12 comments
Closed

Add a way to replace services in an IServiceCollection #155

halter73 opened this issue Dec 16, 2014 · 12 comments

Comments

@halter73
Copy link
Member

Currently, it isn't possible to replace or remove IServiceDescriptors in an IServiceCollection. The best you can do is to copy into a new collection only the descriptors you want to keep unmodified. Then you can add new descriptors to replace any descriptors from the original collection that you didn't copy over. This is a pain.

Previously, I thought we could work around this issue by depending on IoC containers to only inject the last registered service. Unfortunately, Ninject will fail to inject a single service into a constructor if it is registered multiple times. This work-around also doesn't address how to remove/replace services that are resolved using IEnumerable<TService>.

The TryAdd extension method allows you to only add services if they are not yet defined in the IServiceCollection, but offers no help if you want to replace services already in the collection.

@ilmax
Copy link

ilmax commented Jan 16, 2015

Is this needed to replace framework registered services with custom implementation?

@halter73
Copy link
Member Author

It is needed to do it easily in a way that will work with all IoC containers (such as Ninject).

As I mention in the issue, you can always create a new IServiceCollection that copies everything but what you want to replace.

@ilmax
Copy link

ilmax commented Jan 16, 2015

@halter73 what you need replace service for?

@halter73
Copy link
Member Author

There are many framework services that have default implementations that need to be replaced in some scenarios.

For example, you might want to replace SignalR's default in-memory IMessageBus with a Redis based message bus.

@ilmax
Copy link

ilmax commented Jan 16, 2015

It appears to me that this way of replacing services is not well discoverable, I would like change something on a configuration object, not replace a servicedescriptor in a collection of service descriptors.

Since almost every repo here uses the pattern services.AddXXX() to register default services, this could accept a configuration object where I can replace default services. I think this is a simpler and more discoverable approach.

@pranavkm
Copy link
Contributor

pranavkm commented Feb 8, 2015

I should've read this thread before jumping the gun. Based on @davidfowl 's suggestion I added a Replace extension method to IServiceCollection as part of #172. Mvc uses this method to override a service registered by default - https://github.com/aspnet/Mvc/pull/1908/files#diff-3873192961235e19b67b85342bec5373R80. Since this isn't a user provided service, we can't rely on the configuration object to perform the switch.

That said, the semantics of how Replace is currently implemented is iffy when multiple services of a particular service type are registered and it needs to be replaced. Should Replace

  1. replace the first \ last service descriptor and leave the rest unchanged
  2. replace one and remove all the others
  3. throw if more than one service of the specified service type is found. If we choose the option, is there a ReplaceAll that behaves like option 2?

@yishaigalatzer
Copy link
Contributor

  1. replace the first \ last service descriptor and leave the rest unchanged
  2. replace one and remove all the others
  3. throw if more than one service of the specified service type is found. If we choose the option, is > there a ReplaceAll that behaves like option 2?

3 works for me, as it works correctly in most cases, and throws when something unexpected happens. But I see how 2 is simpler and also works in more cases. 1 is just weird.

When it comes to configuration

It appears to me that this way of replacing services is not well discoverable, I would like change something on a configuration object, not replace a servicedescriptor in a collection of service descriptors.

Since almost every repo here uses the pattern services.AddXXX() to register default services, this could accept a configuration object where I can replace default services. I think this is a simpler and more discoverable approach.

At least in MVC we are strongly encouraging users not to replace services (that's a last resort large hammer approach), and we attempt provide all the required extensibility through MvcOptions, so I'm not a fun of making service replacement through a configuration object like MvcServices a first class support.

@ilmax
Copy link

ilmax commented Feb 8, 2015

@yishaigalatzer I hope this Replace will throw if called after the container is fully configured (e.g after serving the first instance or after manually locking the configuration) otherwise I think it will be a source of pain.

so I'm not a fun of making service replacement through a configuration object like MvcServices a first class support.

I wasn't referring to service replacement, but instead to service registration. All user replaceable services can be put on a configuration object used by MVC for services registration.

@davidfowl
Copy link
Member

Replace is on the service collection not the the service provider

@yishaigalatzer
Copy link
Contributor

I wasn't referring to service replacement, but instead to service registration. All user replaceable services can be put on a configuration object used by MVC for services registration.

  1. What @davidfowl said. Replace works on the service collection not on the service provider (or the DI container). Think of the ServiceCollection as the configuration object.
  2. There are no user replaceable services, that's the point i'm trying to make. They are all not intended to be replaced, at least not without taking a huge responsibility to understand what and how they interact with other services. Unlike MVC 5 and Web API 2, this is not the way to customize your app.
  3. If you have scenarios in mind where you think you need to replace services, please let us know (possibly start a discussion in www.github.com/aspnet/mvc, so we can either show how it should be done with the new framework, or we can make that scenario more user friendly from replacement).
  4. If we do end up with a set of services that are intended for user replacement, this could be a reasonable idea, but like I said we would rather build something that does it for you rather than the approach suggested.

@ilmax
Copy link

ilmax commented Feb 9, 2015

@davidfowl @yishaigalatzer Thanks, this all make sense now.

@Eilon
Copy link
Member

Eilon commented Jul 27, 2015

There are Replace and Remove methods that can be used for this.

@Eilon Eilon closed this as completed Jul 27, 2015
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