IServiceProvider resolve differences

[PleasantD]

I noticed a discrepancy between using the built-in IServiceProvider implementation for ASP.NET Core (RC2) and the autofac implementation using builder.Populate(services).

Say I have a collection of service descriptions which contain multiple entries for IStartupFilter (which by default you will if you use IIS integration). The order of these is in the IServiceCollection is: StartupA, StartupB.

Using the default IServiceProvider implementation, these are resolved in the same order.

Using builder.Populate(services) registers all of the services in the collection into the BuilderContainer in the same order. This is important so that later registrations hide earlier ones when resolving a single service. However when resolving all services it inverts the list. Resolving IEnumerable<IStartupFilter> now returns StartupB, StartupA.

Since the order of services matters for IStartupFilter, the middleware registered by these filters are running out of order. I can foresee this causing problems since StartupA is AutoRequestServicesStartupFilter which registers the middleware that initializes the per-request scope.

[tillig]

We saw something similar with #695. I'm not sure if there's anything we can do about it, but it's worth a second look.

[PleasantD]

A simple solution would be to have the IServiceProvider implementation reverse any resolution of IEnumerable. This at least makes autofac behave as expected by ASP.NET Core. However, this could still cause issues if registrations are made on the ContainerBuilder before calling builder.Populate(services).

I think a better pattern would be to create an extension method for IServiceCollection that returns a ContainerBuilder. Then the usage pattern shifts to:

public IServiceProvider ConfigureServices(IServiceCollection services)
{
    // Add services to IServiceCollection
    services.AddMvc();

    // Create autofac builder
    var builder = services.CreateAutofacBuilder();

    // Register against autofac
    builder.Register<MyService>().AsSelf();

    // could be replaced by something like --> return builder.BuildServiceProvider();
    var container = containerBuilder.Build();
    return container.Resolve<IServiceProvider>();
}

[tillig]

Before we make a decision, let me ask the aspnet team if they have a prescribed/required order for enumerables. I feel like either way, someone's going to feel like there's a breaking difference - existing Autofac users porting to aspnet core will wonder why things are backwards from what they're used to; aspnet core users will wonder why the default service provider has different behavior than the Autofac backing implementation. If there's a specific aspnet team recommendation that would influence the solution.

[tillig]

Looks like there's already an issue to get some feedback on matters like this from the team. I chimed in and referenced this issue; you can follow along over there if you like. Once we get input (or some indication that they abstain from providing input) we can decide how best to proceed.

[davidfowl]

We're looking into the things we currently rely on order for. Will get back to you as soon as possible with some data and recommendations.

[alexmg]

Thanks @davidfowl

[davidfowl]

We're going to require container vendors to order services using registration order. We'll be updating the spec tests to make sure that requirement is clearer.

[tillig]

Thanks, @davidfowl - that helps. We'll see what we can do to update the Autofac implementation accordingly.

[alexmg]

@davidfowl I’m really not sure if we will be able to guarantee the order of resolved services due to the way that registrations assigned to the same service type are handled. When a second registration for the same service type is made, the order that implementations are kept in depends on whether or not the user wants that registration to become the default when a single instance is resolved (it gets moved to the front of the implementation list when overriding an existing registration). We also can’t control the order of registrations dynamically created by users through registration sources. This is less common but something that will impact order nonetheless.

I have personally never relied on the container providing services in a particular order as it just seems like a weird assumption to make. If I want to ensure ordering I make it an explicit part of the service interface, implement the IComparable interface, or using Meta<T> metadata to supply the ordering information. In any case the consuming service ensures ordering before using the services.

I worry more requests like this will add fuel to the fire for those arguing against the “conforming container” interface you are forcing container vendors to adhere to. The leaky nature of the abstraction has never sat particularly well with me, and despite the fact that we’ve been lucky up to this point, I suspected some aspect of the specification we didn’t conform to would crop up eventually. As you demand more conformance from the containers, the number of containers able to comply will keep shrinking.

If we do manage to work around this issue we are going to have to make some significant changes and break the API in order to conform. I'm willing to try as we want to come along on the journey, but this sort of thing does make me question the approach.

[tillig]

That's all true. Not to pile on, but looking closer at what it will take... it will be pretty painful. Probably not impossible but definitely not a five-minute job. (We take PRs! 😉 )

[fubar-coder]

All those Options stuff depend on the initialization order. I attached an example:
TestAutoFacOrder.zip

[davidfowl]

It's not just options, various other services depend on order (like IStartupFilter).

I worry more requests like this will add fuel to the fire for those arguing against the “conforming container” interface you are forcing container vendors to adhere to. The leaky nature of the abstraction has never sat particularly well with me, and despite the fact that we’ve been lucky up to this point, I suspected some aspect of the specification we didn’t conform to would crop up eventually. As you demand more conformance from the containers, the number of containers able to comply will keep shrinking.

I totally understand this and we debated it for a few days and came to this conclusion. We're also beefing up our spec to make sure that we can know if containers can support the ASP.NET contract or not. We haven't been good enough at documenting all of the things we rely on though the spec tests gets pretty close.

If we do manage to work around this issue we are going to have to make some significant changes and break the API in order to conform. I'm willing to try as we want to come along on the journey, but this sort of thing does make me question the approach.

Can you guys explain why it's a breaking change? I don't understand that one. Are you saying that there's no way to have IEnumerable<T> be ordered with respect to registration order? That's basically the feature (right now it's in reverse for autofac).

We also discussed a few other options:

• GetService<IOrderedEnumerable<T>> - Not great since other containers don't work like this and this only helps know what to order at consumption time, not registration time
• Add something on ServiceDescriptor that describes the order IEnumerable<T> services is a more first class way so container authors don't have guess

[Ph47]

I believe that the list of entities can be considered orderly if the entity has a property that allows you to make ordering. The order of registration of service is an ordering property of a service in the services collection. "Enumerable" is not "Orderable" and hides valuable property. I seen right decisions in either using an ordered collection or specifying the property clearly. Using only Enumerable for ordering can be an expensive architectural mistake.

[Ph47]

My workaround while issue discussed in flow listed below:

#region Autofac integration
   ContainerBuilder servicesUpdateBuilder = new ContainerBuilder();
   servicesUpdateBuilder.Populate(services.Reverse());
   servicesUpdateBuilder.Update(Container);
   return Container.Resolve<IServiceProvider>();
#endregion

https://msdn.microsoft.com/en-us/library/bb358497(v=vs.100).aspx

[tillig]

I don't recommend reversing the service collection as a workaround.

If you reverse the services on Populate that will fix the IEnumerable problem but you will get the wrong default registered service if you resolve an individual item - it's basically registering all the services in Autofac in the reverse order, so the last one registered will be the first one in the collection - the one that should have been overridden by the last one in.

That's why this is such a tricky problem, especially for Autofac. The order we return the IEnumerable is not necessarily the order in which it was registered, but registration order is very important for service defaults.

[Ph47]

I understand that. DI in my App divided into two flow: dotnet and my own. My own DI use Autofac, dotnet uses internal. Inversion (and merge) occur at an end of this two separate process in ConfigureServices in last lines. I understand whole problem as i posted before and awaits Autofac update. In my current situation and in my current app I can afford this crutch for several weeks with reason to not fallback to "facatory" DI. My tests show no problem after this move and i still subscribed to this issue. It is better what sit and wait without work because update to dotnet rc2 breaks my app at this issue.

[tillig]

That's fair. I didn't know if you were aware; and I definitely didn't want someone encountering the same issue to "skim" the comments here and think that was something they could do without side effect.

[khellang]

What happened to PreserveExistingDefaults? I don't remember the details, but I seem to remember we added this to the Nancy bootstrapper for multi-registrations when someone had a problem like this. Will that only guarantee that you still resolve the first registration when you resolve a single service?

[tillig]

PreserveExistingDefaults still exists.

Basically the way it works in Autofac is:

• Create a list of registrations.
• Every time a registration is added, add it to the list. The last item on the list is the active default registration.
• If the registration is added as PreserveExistingDefaults add it to the top of the list instead of the bottom of the list.

Standard disclaimers apply - I'm going from memory rather than diving into the code; there's more to it than that; registration sources play in differently than standard component registrations; etc.

[halter73]

As @davidfowl mentioned:

We're going to require container vendors to order services using registration order. We'll be updating the spec tests to make sure that requirement is clearer.

This change has been merged to the DI spec tests for 1.0.0. There are currently no plans to update ASP.NET not to rely on order. aspnet/DependencyInjection#416

[halter73]

@tillig So if I'm understanding correctly, the primary issue with the ordering requirement is how that interacts with our other requirement that the last service in the IServiceCollection replaces previous services when resolved individually. Basically this would require significant changes to Autofac (possibly requiring a new way of registering services specifically for ASP.NET) to have the last service resolved in an IEnumerable also be the service that is resolved when requested individually.

If we remove the LastServiceReplacesPreviousServices specification test, and promise to never resolve a service with multiple entries in the IServiceCollection as a single service (we would only resolve these services using IEnumerable), would that make it more reasonable to meet our ordering requirements?

One thing to consider if we do this is how customers could replace existing services and add to IEnumerable services defined in the IServiceCollection by registering them directly with the ContainerBuilder. Hopefully this will remain pretty straightforward.

[tillig]

Last service in currently wins. We pass all the current tests in the spec collection (DependencyInjectionSpecificationTests). The challenge we have is that we don't internally maintain the strict order that things were registered. From an external perspective:

• We keep the list of all services registered.
• If you register multiple services of the same type, all things considered equal, last in wins.
• Internally the list of services registered may or may not reflect the order registered so it's not just a matter of reversing a collection order or whatever.
• Resolving an IEnumerable<T> of a given service type T will not necessarily yield the list of registrations in registration order.

The breaking/challenging bit will be that last point - trying to return the IEnumerable<T>in registration order.

The challenge is slightly compounded by Autofac's ability to support the concept of a "registration source" - which is, basically, a class that can lazily produce registrations, like a dynamic service registration factory. When the container is asked, "Do you have a registration for ISomeInterface?" there may not actually be a specific service registration for that service type - instead, a "registration source" may say, "Yup, I got this - I can create an ISomeInterface." That's how we handle support for IEnumerable<T>, Lazy<T>, and other things you don't explicitly register.

Anyway, that's the challenge we've got to figure out - if there's a (new) requirement to return IEnumerable<T> in registration order, then...

• How do we change Autofac to maintain the current "default" registration (for handling that LastServiceReplacesPreviousServices use case) whilst supporting concepts like PreserverExistingDefaults and "registration sources" and still return a list in registration order?
• How does this change interact with the other Autofac integration libraries which may or may not be relying on the current ordering mechanism? (That is, assuming, for better or worse, a certain ordering like we use now.)
• When we look at handling nested lifetime scopes, how does such a change affect that? (Right now, nested lifetime scopes take the parent lifetime scope's list of services/sources and and aggregate them into a single registration source passed from parent to child. Handling of registration sources suddenly becomes important in ordering.)
• Once we start trying to fix the top-level problem mentioned (just dealing with the service registration order, PreserveExistingDefaults, and registration sources), is there anything hidden below that we haven't anticipated?

Not five minutes' worth of work, to be sure. Not technically insurmountable, but definitely not something we were considering diving into near-term.

[alexmg]

Posted a comment on the DI repo about the issues we are having conforming to the ordering requirement.

aspnet/DependencyInjection#416 (comment)

[alexmg]

The ordering behaviour is described in these unit tests:

https://github.com/autofac/Autofac/blob/develop/test/Autofac.Test/Features/Collections/CollectionOrderingTests.cs

From @halter73 in #768 (comment):

Given the choice, I think it would be nice if services registered via the Populate method are put at the front of this list if Populate is called before other explicit registrations and at the back of the list if called afterwards.

I understand this doesn't work for automatic registrations though. In that case, I would put the services registered via Populate at the front of this list. In the case of IConfigureOptions, framework code adds some initialization logic for TOptions, so if automatically registered IConfigureOptions services want TOptions to be initialized, it should be put later in the list in order to run later.

Services for explicit registrations will always be returned in added order. Registrations from registration sources will appear at the end of the list.

https://github.com/autofac/Autofac/blob/develop/test/Autofac.Extensions.DependencyInjection.Test/AutofacRegistrationTests.cs#L146