Middleware registration order is inverted during resolve

[tillig]

Describe the Bug

If you look at the OWIN self hosting example you can see the expected output to respect the registration order of the middleware. However, when you execute the example, the middleware is executed in the opposite order:

Inside the 'Invoke' method of the 'SecondMiddleware' middleware.
Inside the 'Invoke' method of the 'FirstMiddleware' middleware.
Inside the 'OnActionExecutingAsync' method of the custom action filter.
Inside the 'Get' method of the 'TestController' controller.
Inside the 'OnActionExecutedAsync' method of the custom action filter.
StatusCode: 200, ReasonPhrase: 'OK', Version: 1.1, Content: System.Net.Http.StreamContent, Headers:
{
  Date: Wed, 24 Nov 2021 16:21:09 GMT
  Server: Microsoft-HTTPAPI/2.0
  Content-Length: 15
  Content-Type: application/json; charset=utf-8
}
"Hello, world!"

(Issue based on this StackOverflow question.)

Steps to Reproduce

1. Clone the Examples repo.
2. cd src/WebApiExample.OwinSelfHost
3. dotnet run

Expected Behavior

As documented, the middleware should execute in the order registered.

Dependency Versions

• Autofac: 6.0.0
• Autofac.Owin: 6.0.0
• Autofac.WebApi2: 6.0.0
• Autofac.WebApi2.Owin: 6.0.0

Additional Info

The logic for resolving the set of middleware items looks like it's been stable for six years. However, I recall we had to change some internals in Autofac to handle resolving IEnumerable<T> in registration order and it involved doing things like this which reverse the order in which registrations are returned when enumerating them manually.

I'm guessing that due to that reversal in core Autofac, we also need to reverse the order in which the manual enumeration of registered middleware is handled. We probably should also add a unit test to make sure we don't regress this.

[alistairjevans]

The Implementations property returns the set of implementations of a service in the same order as v4; the reversal you link to in core Autofac was actually just a re-implementation of a reversal which existed in v4 of Autofac.

Looking in to the actual implementation of how the set of middleware is determined indicates that the library depends on the order of the Registrations property, not Implementations:

internal static IEnumerable<Type> GenerateAllAutofacMiddleware(IComponentContext container)
{
  return container.ComponentRegistry.Registrations.SelectMany(r => r.Services)
                  .OfType<TypedService>()
                  .Where(s => IsMiddlewareButNotAutofac(s.ServiceType))
                  .Select(service => typeof(AutofacMiddleware<>).MakeGenericType(service.ServiceType))
                  .ToArray();
}

It doesn't even attempt to use our IEnumerable support. Prior to v5, the Registrations property was a List<T> of component registrations, hence a dependable order. In v5, the backing type for that set was changed to a ConcurrentBag to reduce locking, but the order of that set is then basically "undefined". Even stating "Reverse the order of your registrations" isn't perfect advice, because they could come out in any order.

Other code across the Autofac extensions expressly avoids depending on this order, preferring instead the predictable and documented order of components registered for a given service, when resolving via IEnumerable<T>.

The simplest fix here may be to apply an OrderBy line that retrieves the ServiceRegistration details, and through that the RegistrationOrder value to sort by. That does give a predictable order at least.

internal static IEnumerable<Type> GenerateAllAutofacMiddleware(IComponentContext container)
{
    return container.ComponentRegistry.Registrations.SelectMany(r => r.Services)
        .OfType<TypedService>()
        .Where(s => IsMiddlewareButNotAutofac(s.ServiceType))
        .OrderBy(s => container.ComponentRegistry.TryGetServiceRegistration(s, out var reg) ? reg.GetRegistrationOrder() : 0)
        .Select(service => typeof(AutofacMiddleware<>).MakeGenericType(service.ServiceType))
        .ToArray();
}

It's not a great fix, and if this was a more 'current' integration library, I'd look to do a cleaner fix by switching to registering middleware differently under a single Service or something. But for Owin this may be sufficient.

[tillig]

I think the OrderBy line is probably a "good enough" fix. I'm not super interested in spending a lot of time here given it appears no one even noticed this for almost two years (the length of time it's been since we updated Autofac internals to support .NET core ordering, around v5).

[tillig]

Forgot about this one. Sigh. I'll add a PR for this after #30 goes through.

[alistairjevans]

I'll put this in, just approved #30, so will merge that and apply this fix.