firebase-android-sdk icon indicating copy to clipboard operation
firebase-android-sdk copied to clipboard

Functions: Allow setting OkHttpClient instance

Open svenjacobs opened this issue 10 months ago • 8 comments

What feature would you like to see?

I would like to be able to set or change the OkHttpClient instance of FirebaseFunctions. Currently this is not possible because the private field is initialized in the constructor.

This would enable using a shared and preconfigured OkHttpClient instance which could lead to improved I/O performance.

How would you use it?

As of now I only want to configure a custom User-Agent string so that we can distinguish between different clients that execute callable functions in the backend.

svenjacobs avatar Oct 12 '23 14:10 svenjacobs

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

google-oss-bot avatar Oct 12 '23 14:10 google-oss-bot

@svenjacobs Thanks for filing the feature request and putting together a PR. I brought this up for discussion internally and will comment on this issue whenever there's an update.

In the meantime, something that crossed my mind is that we might want to port the same feature to other platforms (iOS, Web, C++, Unity and Flutter), and in that case we might prefer to provide functions like setHeader("User-Agent", value) and/or setHeaders(hashMapOf("User-Agent", value)). Would this satisfy your use case?

Also happy to discuss other advantages of setting an OkHtttpClient instance. (I'm guessing the ability of adding interceptors would be one)

thatfiredev avatar Oct 24 '23 14:10 thatfiredev

Hello @thatfiredev, being able to set headers would satisfy my use case. However setting the OkHttpClient has other advantages, like interceptors that you mentioned, and probably other benefits that I currently can't imagine 🙂

svenjacobs avatar Oct 24 '23 14:10 svenjacobs

@thatfiredev As an alternative to #5430 I could provide a PR that extends HttpsCallOptions with the possibility to set headers. What do you think?

svenjacobs avatar Oct 25 '23 11:10 svenjacobs

@svenjacobs That's exactly what I had in mind. Note that these types of changes need to be approved by an internal API Review Council and the process might take 2 or more weeks. I'm currently in the process of documenting the proposed changes for their review and will let you know once I have heard back from them.

I'd rather wait for their feedback before putting up another PR. :)

thatfiredev avatar Oct 25 '23 11:10 thatfiredev

@thatfiredev Hey Rosário, are there any news regarding this feature and the related pull request?

svenjacobs avatar Dec 08 '23 08:12 svenjacobs

@svenjacobs Sorry, I had to prioritize some other work before the end of the year and didn't get a chance to submit this for API review. 😓

I finally did it today and we should hear back in a few weeks, I'll update this thread by then.

I proposed adding a new addHeaders function to HttpsCallableReference so that we can use it like this:

    // Function payload
    val data = hashMapOf(
        "text" to text,
        "push" to true,
    )

  // Additional headers
  val headers = hashMapOf(
    "User-Agent": "Android 14"
  )

  Firebase.functions.getHttpsCallable("functionName")
    .addHeaders(headers) <-- new method to pass custom headers
    .call(data)

thatfiredev avatar Jan 10 '24 10:01 thatfiredev

Thanks @thatfiredev for your feedback. Your proposal looks good, however being able to set the whole OkHttpClient instance is a bit different request. For instance it would allow to reuse an instance which could lead to improved I/O performance.

svenjacobs avatar Jan 10 '24 11:01 svenjacobs