Make RequestServicesContainerMiddleware the first middleware in the pipeline

[weidazhao]

When we add a middleware through IStartupFilter, it will be added before RequestServicesContainerMiddleware in the pipeline (see the impl at https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Hosting/Internal/WebHost.cs#L163) Therefore, the middleware won't be able to get any scoped service instance from HttpContext.RequestServices since RequestServices is not constructed by RequestServicesContainerMiddleware yet at the moment.

This could be fixed by ensuring RequestServicesContainerMiddleware always to be the first middleware in the pipeline. A possible fix is to move https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Hosting/WebHostBuilder.cs#L202 to https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Hosting/WebHostBuilder.cs#L218.

For more context and details, check out weidazhao/Hosting#8.

[weidazhao]

Took a further look at the code when I was trying to make a pull request.

I think I understand the reason that RequestServicesContainerMiddleware is currently added after the middlewares registered by IStartupFilter so that those 3rd party middlewares can plug in their own implementations of IServiceProvidersFeature. However, IServiceProvidersFeature is now defined under Microsoft.AspNetCore.Http.Features.Internal. Is it really expected for 3rd party to provide custom implementation of IServiceProvidersFeature?

[davidfowl]

I think I understand the reason that RequestServicesContainerMiddleware is currently added after the middlewares registered by IStartupFilter so that those 3rd party middlewares.

I think it's reasonable to pick an order that allows hosts that plug into the system the ability to run after the built in middleware. The main reason we add ours to the collection first is because we want configure services to see all of the registered services so it can add/remove or replace them. If we add them after the callback then it's impossible to observe/change that specific hosting service.

However, IServiceProvidersFeature is now defined under Microsoft.AspNetCore.Http.Features.Internal. Is it really expected for 3rd party to provide custom implementation of IServiceProvidersFeature?

@dougbu is fixing this very soon. It's going to be moved to Microsoft.AspNetCore.Http.Features. /cc @Tratcher

[weidazhao]

Thanks. Fair enough.

The special place in my scenario is that I need the built-in feature IServiceProvidersFeature to be present in my middleware that is pluged in via IStartupFilter, which is then invoked before RequestServicesContainerMiddleware, So I would have to provide an instance of IServiceProvidersFeature by my own in the middleware. Since I don't have any special requirement on the implementation of IServiceProvidersFeature, the default RequestServicesContainerFeature would meet my needs.

Would it make sense to also move RequestServicesContainerFeature out of internal or I should copy/paste the code? @davidfowl

[davidfowl]

@weidazhao We're running startup filters in the wrong order. We actually want it to behave more like middleware where the ones registered first, get run first.

[weidazhao]

lol, that makes more sense. I like 'first in first run', which is consistent with how middlewares run, as you said. Thanks.

[weidazhao]

For completeness, after the discussion with @davidfowl , now I think the right fix will be adding Reverse() to https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Hosting/Internal/WebHost.cs#L165

foreach (var filter in startupFilters.Reverse())