-
Notifications
You must be signed in to change notification settings - Fork 0
feat(CrossAppLogin): Forward compatibility, and don't wait for all apps #607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
771aad6 to
c8bbf7a
Compare
CrossAppLogin/Back/src/main/kotlin/com/infomaniak/core/crossapplogin/back/CrossAppLoginImpl.kt
Outdated
Show resolved
Hide resolved
| // Lazy start to not send an empty list before the first available responds. | ||
| val alreadyConnectedEmailsUpdatingJob = launch(start = CoroutineStart.LAZY) { | ||
| alreadyConnectedEmailsFlow.collect { alreadyConnectedEmails -> | ||
| send(accountsFlow.updateAndGet { it.withAccounts(emptyList(), alreadyConnectedEmails) }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the usage of the flow here, instead of just using a mutableList ?
Is it because of concurrent write ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, it's to use the atomic reference capability of MutableStateFlow.
A MutableList would definitely lead to random ConcurrentModificationException being thrown.
kotlin's AtomicReference supports the equivalent updateAndFetch since 2.2.20 (we could also have made it on top of compareAndSet, it's fairly trivial), so after we update our fork of Realm, we could use it instead if we want.
Not sure it provides any performance or readability improvements, but I can do the replacement later on if you want.
Don't wait for all apps anymore. Will provide a smoother experience when one app is significantly slower to start/respond than other ones, and will mitigate any communication issues when there are multiple apps with connected accounts.
180cfe1 to
166f635
Compare
|



I tested thoroughly a bunch of cases with all our 3 compatible apps together.
Here they are:
Worked perfectly for all of these.