Flurl icon indicating copy to clipboard operation
Flurl copied to clipboard

IFlurlClientFactory to use string instead of Url key

Open Pyrobolser opened this issue 3 years ago • 4 comments

Hello,

I have been inheriting from FlurlClientFactoryBase for a while now, I have multiple clients returned by the factory depending on multiple BaseUrl, each client uses its own user/password and way of authenticating.

But, I am now facing a new challenge. I need to use two different couple user/password for the same client, everything else is going to be the same. So I was just thinking of creating two different clients, one configured for each couple user/password APIClientUser1 and APIClientUser2. But since they share the same BaseUrl I can't really base my caching strategy on the BaseUrl anymore. I wanted to use a simple string as a key or even a combo BaseUrl/Username but it looks like the factory only wants the get method like this Get(Url url)

Do you think that would make sense as a feature to be able to just put whatever you want as a key? Not necessarily based on the BaseUrl for your request? I would rather ask even if I realize I might need to create my own factory right now.

Thank you!

Pyrobolser avatar Mar 10 '21 20:03 Pyrobolser

I might consider something like this for a future major version, but for now I don't think it can be done without breaking some contracts. For example, I'm not sure how the static FlurlHttp.ConfigureClient method would work if we can't count on a lookup by URL returning a single client. I'll add this to the backlog for 4.0 consideration though.

In the mean time, keep in mind the only time IFlurlClientFactory is used is when using the url.DoSomethingAsync() pattern where a client needs to be looked up implicitly. You could certainly switch to a pattern where you're using FlurlClient explicitly, then keep them in a static ConcurrentDictionary keyed by username, or use an IoC container to manage these instances if it supports a feature like this.

tmenier avatar Apr 28 '21 15:04 tmenier

Thank you for your answer.

The alternative solution you're talking about is more or less what I ended up doing and it worked for my use case!

Pyrobolser avatar Apr 28 '21 15:04 Pyrobolser

I was working with FlurlClientFactory as well, but what it confuses me is the final usage of the URL on the Create(Url url) method. It gives the idea that you're receiving the base URL for constructing the FlurlClient, but it ends up being the CacheKey, which of course comes as a Url object, but it's not even a valid URL string. What is the intention of that?

CesarD avatar May 19 '21 17:05 CesarD

@CesarD I'm not certain I'm following the question, but it might help to look at the differences between DefaultFlurlClientFactory and PerBaseUrlFlurlClientFactory. In both cases, the most important thing is the GetCacheKey implementation. The Url passed to Create doesn't necessarily have to be used. In the case PerBaseUrlFlurlClientFactory, it's used to set the BaseUrl property of FlurlClient, as a convenience feature; in the case of DefaultFlurlClientFactory it's ignored. Why? Because with PerBaseUrlFlurlClientFactory it's implied that every call using a given client will start with that base URL because it is the cache key. In the default implementation, the cache key is based on host, scheme, and port, so calls using the same client do not necessarily have the same base URL, so Flurl doesn't assume one for the client.

tmenier avatar May 19 '21 21:05 tmenier

@tmenier It would be nice to use a URL + some kind of custom key. I my case I have the same url and endpoints, but different headers and certificate files. So I created static ConcurrentDictionary<string, IFlurlClient> Clients to manage flurl clients explicitly:

foreach (var (headerKey, certificateFilePath) in CertificateFiles)
{
    var certificate = new X509Certificate2(certificateFilePath, certificatePassword);
    var flurlClient = new FlurlClient(Foo.Url)
        .Configure(settings => settings.HttpClientFactory = new ApiHttpClientFactory(certificate));
    
    Clients.TryAdd(headerKey, flurlClient);
}  

umbarov avatar Nov 15 '22 10:11 umbarov

At long last I'm dusting off this issue and actually attempting to solve it. 😄

The idea to just use any arbitrary string as a key doesn't really address how Flurl then decides what key to use to look up a client. The current interface allows you to customize how this question is answered:

"Given this URL, what client should I use?"

What if we change the question to:

"Given this HTTP request, what client should I use?"

The request contains the URL, but also other information such as headers that might be useful. My plan for 4.0 is to change the current GetCacheKey implementation to take an IFlurlRequest instead of a Url. I'm hoping that will satisfy use cases like those brought up here, but I'm interested to hear other opinions.

tmenier avatar Sep 05 '23 20:09 tmenier

I did some experimenting per the above ideas and didn't like where things were going. It occurred to me that something along the lines of named clients is really all that's being asked for here (correct me if that's wrong). I'll consider something like this - I'd like to move Flurl closer to a model like IHttpClientFactory anyway since most people are familiar with it.

tmenier avatar Sep 15 '23 19:09 tmenier

Hi,

Thanks for working on this issue, I think you're onto something.

I feel like the example you shared is a good way to solve this, I am already using named options in other places of the applications so this would indeed be a familiar solution.

Pyrobolser avatar Sep 15 '23 20:09 Pyrobolser

Closing in favor of #770. Flurl is getting a BIG overhaul in this area and I encourage you weigh in there with any thoughts you may have.

tmenier avatar Oct 18 '23 18:10 tmenier