firebase / firebase-admin-dotnet

Firebase Admin .NET SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
370 stars 131 forks source link

Replace HttpClientFactory as interface #287

Open gsino opened 3 years ago

gsino commented 3 years ago

Hello.

In the GoogleApi repository, an implementation of Google.Apis.Http.IHttpClientFactory using Microsoft.Extensions.Http.IHttpMessageHandlerFactory is presented.

https://github.com/googleapis/google-api-dotnet-client/issues/1756

https://github.com/googleapis/google-api-dotnet-client/blob/bfc9090dc2fae5b06a654328c4da7bc6b05b66f1/Src/Support/IntegrationTests/HttpClientFromMessageHandlerFactoryTests.cs#L38

To apply this method to Firebase as well, change the type of the AppOptions.HttpClientFactory property from Google.Apis.Http.HttpClientFactory to Google.Apis.Http.IHttpClientFactory?

google-oss-bot commented 3 years ago

I found a few problems with this issue:

hiranya911 commented 3 years ago

@gsino can you add some example use cases or code samples to this issue to elaborate how this can be used with Firebase? Wouldn't you be able to achieve the same result with something like https://github.com/firebase/firebase-admin-dotnet/blob/master/FirebaseAdmin/FirebaseAdmin.Tests/MockHttpClientFactory.cs?

gsino commented 3 years ago

Wouldn't you be able to achieve the same result with something like https://github.com/firebase/firebase-admin-dotnet/blob/master/FirebaseAdmin/FirebaseAdmin.Tests/MockHttpClientFactory.cs?

Half Yes. In fact, I can get any HttpMessageHandler by injecting System.Net.Http.IHttpMessageHandlerFactory.

public class MyHttpClientFactory : Google.Apis.Http.HttpClientFactory
{
    private readonly System.Net.Http.IHttpMessageHandlerFactory _httpMessageHandlerFactory;

    public MyHttpClientFactory (System.Net.Http.IHttpMessageHandlerFactory httpMessageHandlerFactory)
    {
        _httpMessageHandlerFactory = httpMessageHandlerFactory;
    }

    protected override HttpMessageHandler CreateHandler(Google.Apis.Http.CreateHttpClientArgs args)
    {
        return _httpMessageHandlerFactory.CreateHandler("MyHttpClientFactory");
    }
}

However, compared to Google.Apis.Http.HttpClientFactory, it lacks the following two features:

  1. Support for CreateHttpClientArgs
  2. Support for StreamInterceptionHandler

For Google.Apis.Http.HttpClientFromMessageHandlerFactory, both features are supported.

If the type of the AppOptions.HttpClientFactory property is Google.Apis.Http.IHttpClientFactory, you can set Google.Apis.HttpHttpClientFromMessageHandlerFactory in addition to Google.Apis.Http.HttpClientFactory.

public sealed class AppOptions
{
    public IHttpClientFactory HttpClientFactory { get; set; }
}
var defaultHttpOption = new AppOptions
{
   HttpClientFactory = new HttpClientFactory(),
}
var customHttpOption1 = new AppOptions
{
   HttpClientFactory = new HttpHttpClientFromMessageHandlerFactory(...),
}
//or use DI Container
var customHttpOption2 = new AppOptions
{
   HttpClientFactory = serviceProvider.GetService<IHttpClientFactory>(),
}
hiranya911 commented 3 years ago

Fair enough. We can look into making this change in a future release (shouldn't be a breaking change as far as I can tell).

gsino commented 3 years ago

Thank you! :)

mikequ-taggysoft commented 2 years ago

@hiranya911 Any updates on this? We'd really like to use .NET's IHttpClientFactory to manage the underlying http connections for efficiency. Since this is already supported in other Google APIs, and the pattern is now the universal best-practice for all modern .NET applications, it would be great for the Firebase SDK to support it as well. Thanks!

tiaan-lg commented 1 year ago

Are there any updated on this. or any other methods of optimizing the HTTP client usage on FireBase Messaging?