fix connection issue

[ianlin-bbpos]

Summary

fix connection issues with beta.20, improve the method of fetching and setting token.

Motivation

To fix the issues reported by users that connecting device would timeout on the first running after app installed.

Testing

• [x] I tested this manually
• [ ] I added automated tests

Documentation

Select one:

• [ ] I have added relevant documentation for my changes.
• [x] This PR does not result in any developer-facing changes.

[ianlin-bbpos]

Could you explain the problem a bit more? I think I'm missing a bit of context here. Are we currently overwriting the ConnectionTokenCallback instance when receiving more than one request? Is the ConnectionTokenCallback implemented in native code (I couldn't find anything in the repo) or is it a react native component?

yes, user reported that the connecting the device on the first running of the app after installation would timeout, and connect successful after reopen the app. and we have some findings here https://docs.google.com/document/d/1PMfnDrshVFXo3H2WkPITNvGwMwkme0irJ482heOeq4w now we are looking for a solution to resolve it. In this PR, we tried to use a HashMap to record the callback via a uuid as the keys on each intent to fetch token. then we can identify the callback for each request, to avoid that callback would be covered by later request. ConnectionTokenCallback is a Terminal native component, it's impletemented in native sdk part

[ebarrenechea-stripe]

I understand the issue better now, thanks for the doc. If this code is being called from multiple threads it might be better to use a HashTable instead of a HashMap as the former is thread safe. I wonder if we could re-use the first token instead of fetching a new one by synchronizing the requests. In any case, this is good enough for now.

Changes look good to me but I'll leave the final approval with @bric-stripe to double check the iOS side.

[ianlin-bbpos]

I understand the issue better now, thanks for the doc. If this code is being called from multiple threads it might be better to use a HashTable instead of a HashMap as the former is thread safe. I wonder if we could re-use the first token instead of fetching a new one by synchronizing the requests. In any case, this is good enough for now.

Changes look good to me but I'll leave the final approval with @bric-stripe to double check the iOS side.

Thanks, let me change to use a HashTable.

[tim-lin-bbpos]

I understand the issue better now, thanks for the doc. If this code is being called from multiple threads it might be better to use a HashTable instead of a HashMap as the former is thread safe. I wonder if we could re-use the first token instead of fetching a new one by synchronizing the requests. In any case, this is good enough for now.

Changes look good to me but I'll leave the final approval with @bric-stripe to double check the iOS side.

Q: This might be off-topic and thread safe is definitely a good thing if we can't control the upstream, but I still like to clarify on the usage of HashTable. If the only purpose is thread safe, are we gonna consider to use ConcurrentHashMap instead? According to the HashTable javadoc, it should be a better option as far as I know. p.s. definitely we can go with HashTable for now as it will only called a few times here (and should only one in the future?).

Ref: https://docs.oracle.com/javase/8/docs/api/java/util/Hashtable.html

As of the Java 2 platform v1.2, this class was retrofitted to implement the Map interface, making it a member of the Java Collections Framework. Unlike the new collection implementations, Hashtable is synchronized. If a thread-safe implementation is not needed, it is recommended to use HashMap in place of Hashtable. If a thread-safe highly-concurrent implementation is desired, then it is recommended to use ConcurrentHashMap in place of Hashtable.

[ebarrenechea-stripe]

Yeah, ConcurrentHashMap will work as well. We just need a data structure that can handle concurrency without issues.

[maestrodrew]

@nazli-stripe When can we expect the next release (v21) that will contain this fix?