Prevent duplicate `v1/configuration` GET requests

[scannillo]

Summary

See Victoria's original PR https://github.com/braintree/braintree_ios/pull/1308 for full description and background.

Short summary: our SDK would trigger multiple of the same outbound API request to GET v1/configuration if a merchant instantiated their BTAPIClient and also called tokenize() all upon button click. This is a common pattern for merchants, since nowhere do our docs recommend otherwise.

Goal: Prevent duplicate network requests to GET v1/configuration if one is already pending over the network.

Changes

• Keep track of pending requests via an array of completionHandler references in ConfigurationLoader

Manual Testing

• I created this git patch (trigger_duplicate_config_calls.patch) which you can download and apply to both the main branch and then the branch of this PR config-loader-dedupe to compare behavior.
  • Apply a patch like: git apply /path/to/trigger_duplicate_config_calls.patch
  • This patch includes a log statement for the path of each HTTP request executed, as well as a Demo app update to more easily replicate the duplicate v1/configuration calls in quick sequence.

Checklist

• [X] Added a changelog entry

Authors

@scannillo

[KunJeongPark]

Great job Sammy!

[KunJeongPark]

One last thing for future improvement is to make array handling thread safe by using locks or DispatchQueues. Also managing cache read and writes for possible race conditions. That was original reason for me using dispatch queue with barrier.

I've also thought about adding syncing in ConfigurationCache functions as well since it is possible to have multiple instances of ConfigurationLoader and BTAPIClient. I have no idea if this is so unlikely as to be negligible.

[scannillo]

One last thing for future improvement is to make array handling thread safe by using locks or DispatchQueues.

Yeh, this is another good call. I'm thinking of using an Actor like we did here for our analytics events. This will be easier if everything is in async-await syntax. I might make that change in another PR, and Draft this one for now.

[scannillo]

Ok, I trialed several ways of making the array of completions thread-safe.

1st I tried actors, similar to BTAnalyticsEventsStorage. The problem with this is needing to call all actor methods with await. This means all callsites must either be in a Task block, or in an async function. Wrapping all callsites in Task blocks borks our test suite since there is no way for the test to know when a Task block completed. There are ways around this, but it got messy and it didn't feel worth it.

Converting our ConfigurationLoader.fetchConfig method to only use async-await style syntax was another thing I tried, but we can't store references to the async Task to call at a later point, like we can with completions. There might be a way to do this I'm not aware of.

Lastly, I ended up going with the tried and true DispatchQueue for ensuring all array operations are serial.