Add custom headers to HttpsCallableReference

[svenjacobs]

Implements the addHeader functionality as mentioned in this issue.

Unfortunately I was not able to test these changes because deploying Firebase Functions requires a Blaze (pay-as-you-go) plan for a Firebase project. It's not possible to use a free (Spark) plan (and I can't use the production business project for these tests). I kindly ask you to run the tests for me. I prepared a test function headersTest in this PR.

Also, currently we don't support multiple headers with the same name, which is possible by HTTP standards.
If we want to support this, we have to replace the internal collection holding headers by a Multimap (Map<String, Set<String>>).

One more thing:

Currently headers can only be added to an instance of HttpsCallableReference, meaning on a per-function basis. If we want to support global headers and minimize repetition, we could extend HttpsCallOptions to also hold headers, since options can be reused and passed to several functions. Headers would then have the following precedence

Global headers (HttpsCallOptions) < Instance headers (HttpsCallableReference) < Internal headers (Auth token, Instance ID, AppCheck)

meaning any global headers are overwritten by instance headers with same names, lastly internal headers will overwrite any existing headers with the same names.

What do you think?

[thatfiredev]

Oops, I missed the PR description before reviewing the code.

Currently headers can only be added to an instance of HttpsCallableReference, meaning on a per-function basis. If we want to support global headers and minimize repetition, we could extend HttpsCallOptions to also hold headers, since options can be reused and passed to several functions. Headers would then have the following precedence.

That's a really good point! It was actually suggested internally that we provide headers in HttpsCallOptions instead of HttpsCallableReference. I think providing them in both makes sense, as you may have a function that requires different headers. Let me take this back for discussion internally.

[svenjacobs]

That's a really good point! It was actually suggested internally that we provide headers in HttpsCallOptions instead of HttpsCallableReference. I think providing them in both makes sense, as you may have a function that requires different headers. Let me take this back for discussion internally.

@thatfiredev I already went forward and implemented this. Actually it makes a lot more sense this way because the header values are now handled like the timeout option, for example. They are kept in the internal HttpsCallOptions class and can be overwritten by individual functions.

[svenjacobs]

Hello @thatfiredev, any news regarding this PR? 😃

[svenjacobs]

Bump 😉

[thatfiredev]

@svenjacobs Sorry I missed this. Due to the addition of providing headers in HttpsCallableOptions, I have just brought this up internally for discussion. I will ping this thread again once I hear back.

I'm wondering if we should simplify things and only provide HttpsCallableOptions#addHeaders(), thoughts ?

[svenjacobs]

I'm wondering if we should simplify things and only provide HttpsCallableOptions#addHeaders(), thoughts ?

I think either is fine. I dont' have a strong opinion on one or the other. It would be a few less lines of code, but users would have to write addHeaders(mapOf("key" to "value")) if they just want to add a single header. Let's see what the engineers say?

[thatfiredev]

Sorry, I meant add both HttpsCallableOptions#addHeaders() and HttpsCallableOptions#addHeader() (meaning we wouldn't need to add those functions to HttpsCallableReference)

[svenjacobs]

Sorry, I meant add both HttpsCallableOptions#addHeaders() and HttpsCallableOptions#addHeader() (meaning we wouldn't need to add those functions to HttpsCallableReference)

HttpsCallableOptions.Builder already has addHeaders() and addHeader(). Regarding the additional methods on HttpsCallableReference, I just added them there following the same pattern as the timeout value. I mean, we can remove them if users should only be able to set headers via Builder?