JasperFx / lamar

Fast Inversion of Control Tool and Successor to StructureMap
https://jasperfx.github.io/lamar
MIT License
572 stars 119 forks source link

Lamar's default scanning conventions replace ASP.Net Core 2.1's typed HttpClient registrations #70

Closed CodingGorilla closed 6 years ago

CodingGorilla commented 6 years ago

So in my ConfigureServices I have the following:

services.AddHttpClient<ISubscriptionsServiceClient, SubscriptionsServiceClient>()
        .ConfigureHttpClient((sp, client) =>
                             {
                                 var baseUrl = sp.GetRequiredService<IOptions<AuthServiceOptions>>().Value.SubscriptionServiceUri;
                                 client.BaseAddress = new Uri(baseUrl, UriKind.Absolute);
                             })
        .AddHttpMessageHandler<SystemServiceAuthorizationHandler>();

This is the new HttpClientFactory convention introduced in ASP.NET Core 2.1. Subsequently I have a ConfigureContainer method that looks like:

public void ConfigureContainer(ServiceRegistry services)
{
    services.Scan(scanner =>
                  {
                      scanner.WithDefaultConventions();
                      scanner.TheCallingAssembly();
                      scanner.LookForRegistries();
                  });
}

It appears that the default convention scanning "overwrites" the previous registration from ConfigureServices as the configuration lambda is never fired, and indeed my injected HttpClient does not have the proper configuration. Checking the build plan for the ISubscriptionServiceClient produces the following:

using Microsoft.Extensions.Options;
using System.Threading.Tasks;

namespace Jasper.Generated
{
    // START: LaMotte_Authentication_ApiClients_ISubscriptionsServiceClient_subscriptionsServiceClient
    public class LaMotte_Authentication_ApiClients_ISubscriptionsServiceClient_subscriptionsServiceClient : Lamar.IoC.Resolvers.TransientResolver<LaMotte.Authentication.ApiClients.ISubscriptionsServiceClient>
    {
        private readonly Microsoft.Extensions.Options.IOptions<LaMotte.Authentication.AuthServiceOptions> _options_of_AuthServiceOptions;

        public LaMotte_Authentication_ApiClients_ISubscriptionsServiceClient_subscriptionsServiceClient(Microsoft.Extensions.Options.IOptions<LaMotte.Authentication.AuthServiceOptions> options_of_AuthServiceOptions)
        {
            _options_of_AuthServiceOptions = options_of_AuthServiceOptions;
        }

        public override LaMotte.Authentication.ApiClients.ISubscriptionsServiceClient Build(Lamar.IoC.Scope scope)
        {
            var httpClient = new System.Net.Http.HttpClient();
            scope.Disposables.Add(httpClient);
            return new LaMotte.Authentication.ApiClients.SubscriptionsServiceClient(_options_of_AuthServiceOptions, httpClient);
        }

    }

    // END: LaMotte_Authentication_ApiClients_ISubscriptionsServiceClient_subscriptionsServiceClient

}

Changing the registration in ConfigureServices to use just the concrete type, rather than the interface/concrete pair behaves as I would expect (meaning the configuration from the factory is properly set).

Moving the client registration into the ConfigureContainer method, after the scanning also solves the problem. Is this the expected behavior, should additional registrations not be done using the ConfigureServices method and rather have everything moved into ConfigureContainer?

jeremydmiller commented 6 years ago

You can beat this a couple different ways. You can either set up a rule to ignore those types in the type scanning, or just watch the registration ordering to call:

services.AddHttpClient<ISubscriptionsServiceClient, SubscriptionsServiceClient>()
        .ConfigureHttpClient((sp, client) =>
                             {
                                 var baseUrl = sp.GetRequiredService<IOptions<AuthServiceOptions>>().Value.SubscriptionServiceUri;
                                 client.BaseAddress = new Uri(baseUrl, UriKind.Absolute);
                             })

after the type scanning, and Lamar will honor the registration order. Unlike SM, Lamar follows the ASP.net Core rule of "last registration wins"

CodingGorilla commented 6 years ago

I think my concern is that type scanning always happens after the framework calls ConfigureServices, and based on instruction and examples from Microsoft you would normally add these registrations there in ConfigureServices. So it's a bit counter-intuitive unless you know that you have to move those registrations down into the ConfigureContainer (or write an excluding rule).

Would it not be better to prevent the duplicate registration? I mean if I manually register it, it seems reasonable to me that my manual registration would take precedence over an automatic registration.

jeremydmiller commented 6 years ago

You can do the call to AddHttpClient in the ConfigureContainer() method. Or the ExcludeTypes works no matter what order things are done in.

jeremydmiller commented 6 years ago

@CodingGorilla You'd need a custom convention in this case to understand that what they're doing should override what the type scanning is doing. I don't see this as general enough to build into Lamar itself.