tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.23k stars 387 forks source link

Best practice for multiple `IFlurlClientFactory` implementations? #766

Closed rcdailey closed 1 year ago

rcdailey commented 1 year ago

FWIW, I did try to ask this question on SO first but I got downvoted and voted to close. I guess not all questions related to Flurl are a good fit for that platform.


There are two distinct backend APIs that my application talks to. Furthermore, each service uses different styles of JSON and has different client settings. It's obvious to me that I need to implement two different IFlurlClientFactory classes.

However, when used in conjunction with Autofac (or any DI container, really), what is the best practice for differentiating between the two of them in my service classes? I can think of a couple of solutions. The one I hate the least right now is creating an interface per factory that derives from IFlurlClientFactory and then I'd implement that more specific interface instead. This would allow me to inject each factory differently via interface.

But I don't know if this is the "best" way. I'm hoping there's a more idiomatic solution.

EDIT:

Here's an example of one of my factory classes:

public sealed class AppriseClientFactory : IFlurlClientFactory
{
    private readonly PerBaseUrlFlurlClientFactory _factory;

    public AppriseClientFactory()
    {
        _factory = new PerBaseUrlFlurlClientFactory();
    }

    public IFlurlClient Get(Url url)
    {
        var client = _factory.Get(url);
        client.Settings = GetClientSettings();
        return client;
    }

    private static ClientFlurlHttpSettings GetClientSettings()
    {
        return new ClientFlurlHttpSettings
        {
            JsonSerializer = new DefaultJsonSerializer(new JsonSerializerOptions
            {
                DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
                PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
                PropertyNameCaseInsensitive = false
            })
        };
    }

    public void Dispose()
    {
        _factory.Dispose();
    }
}

And I have another utility class that helps build a IFlurlRequest I can use for more specific API calls:

public class ServiceRequestBuilder : IServiceRequestBuilder
{
    private readonly IFlurlClientFactory _clientFactory;

    public ServiceRequestBuilder(IFlurlClientFactory clientFactory)
    {
        _clientFactory = clientFactory;
    }

    public IFlurlRequest Request(IServiceConfiguration config, params object[] path)
    {
        var client = _clientFactory.Get(config.BaseUrl);
        return client.Request(new[] {"api", "v3"}.Concat(path).ToArray())
            .WithHeader("X-Api-Key", config.ApiKey);
    }
}

It's possible I might be going about this wrong, so I wanted to share some example code to see if I'm asking the right questions.

tmenier commented 1 year ago

It looks like you just want to configure settings specific to 1 client, and for that I think a custom factory is overkill. I would just configure the client directly, probably from inside your service's ctor:

public MyService(IFlurlClient client) {
    _client = client;
   _client.Settings.JsonSerializer = new DefaultJsonSerializer(...);
}
rcdailey commented 1 year ago

I saw your response yesterday. I really appreciate the tip. I haven't had a chance to try it out yet. If it doesn't work out for whatever reason I'll let you know.

I will say this does leave me wondering about what a "valid" use case for implementing the factory would be. I assumed it would be interesting if I needed to customize the client and that client needs to be constructed in more than one place.

rcdailey commented 1 year ago

@tmenier In my previous example, I demonstrated a class implementing IServiceRequestBuilder. I actually have about a dozen API service classes that all talk to the same backend. In effect, they all utilize the same Flurl settings. I believe early on this is why I went with the factory approach.

Are you suggesting that I instead move the custom factory implementation details into my IServiceRequestBuilder subclass?

I think DI adds a layer of confusion here. Because I need to ensure it's a PerBaseUrlFlurlClientFactory, I believe. Although after reviewing the docs again, I'm not sure using this factory over the default will result in a change of behavior for me.

I think it would be helpful to have some design guidelines regarding factories and DI. It's unclear to me when a custom IFlurlClientFactory has value, versus some custom factory-style class (as is the case with my ServiceRequestBuilder class).

tmenier commented 1 year ago

Just a heads up that factories are going away in 4.0. Named clients might be your best bet here. Have a look at #770.