From f3ad55afa72943cb93df5d049a3f85f2b4ce05ec Mon Sep 17 00:00:00 2001 From: Ivan Semenov Date: Wed, 19 Oct 2022 20:43:17 +0200 Subject: [PATCH] refactor[collab]: split token flow and stateflow GitOrigin-RevId: ed257b3bfc36c80ebb2ea52777a4c799c76964c8 --- .../collaboration/auth/AccountManager.kt | 10 +++- .../collaboration/auth/AccountManagerBase.kt | 47 +++++++++---------- .../AccountUrlAuthenticationFailuresHolder.kt | 2 +- .../ui/LazyLoadingAccountsDetailsProvider.kt | 2 +- ...ountUrlAuthenticationFailuresHolderTest.kt | 2 +- ...positoryAndAccountSelectorViewModelBase.kt | 17 ++++--- .../GithubAuthenticationManager.kt | 2 +- .../GHRepositoryConnectionManager.kt | 2 +- .../GHCloneDialogExtensionComponentBase.kt | 2 +- .../api/GitLabProjectConnectionManager.kt | 2 +- ...positoryAndAccountSelectorViewModelTest.kt | 6 +-- 11 files changed, 48 insertions(+), 46 deletions(-) diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManager.kt b/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManager.kt index 84c57352669a..7b5d8c00a872 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManager.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManager.kt @@ -1,6 +1,7 @@ // Copyright 2000-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.collaboration.auth +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow @@ -42,7 +43,12 @@ interface AccountManager { /** * Flow of account credentials - * Will be closed when the account is removed */ - fun getCredentialsFlow(account: A, withCurrent: Boolean = true): Flow + fun getCredentialsFlow(account: A): Flow + + /** + * Flow of account credentials with the latest state + * Credentials are acquired and updated under [scope] + */ + suspend fun getCredentialsState(scope: CoroutineScope, account: A): StateFlow } \ No newline at end of file diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManagerBase.kt b/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManagerBase.kt index 46410e376e93..e4c388d8b190 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManagerBase.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountManagerBase.kt @@ -4,10 +4,13 @@ package com.intellij.collaboration.auth import com.intellij.collaboration.async.disposingScope import com.intellij.openapi.Disposable import com.intellij.openapi.diagnostic.Logger -import kotlinx.coroutines.* +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.flow.* import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext /** * Base class for account management application service @@ -115,34 +118,26 @@ abstract class AccountManagerBase( override suspend fun findCredentials(account: A): Cred? = persistentCredentials.retrieveCredentials(account) - override fun getCredentialsFlow(account: A, withCurrent: Boolean): Flow = channelFlow { - mutex.withLock { - // subscribe to map updates first - launch(start = CoroutineStart.UNDISPATCHED) { - accountsEventsFlow.collect { - when (it) { - is Event.AccountsAddedOrUpdated -> { - send(it.map[account]) - } - is Event.AccountsRemoved -> { - if (account in it.accounts) { - cancel() - } - } + override suspend fun getCredentialsState(scope: CoroutineScope, account: A): StateFlow = + withContext(scope.coroutineContext + Dispatchers.Default) { + mutex.withLock { + getCredentialsFlow(account).stateIn(scope, SharingStarted.Eagerly, findCredentials(account)) + } + } + + override fun getCredentialsFlow(account: A): Flow = + accountsEventsFlow.transform { + when (it) { + is Event.AccountsAddedOrUpdated -> { + emit(it.map[account]) + } + is Event.AccountsRemoved -> { + if (account in it.accounts) { + emit(null) } } } - if (withCurrent) { - try { - send(persistentCredentials.retrieveCredentials(account)) - } - catch (e: Exception) { - logger.warn("Failed to retrieve credentials", e) - send(null) - } - } - } - }.flowOn(Dispatchers.Default) + }.flowOn(Dispatchers.Default) override fun dispose() = Unit diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolder.kt b/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolder.kt index 0c26be975f74..2cc82acfc3d3 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolder.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolder.kt @@ -17,7 +17,7 @@ class AccountUrlAuthenticationFailuresHolder( fun markFailed(account: A, url: String) { storeMap.computeIfAbsent(account) { cs.launch { - accountManager().getCredentialsFlow(account, false).first() + accountManager().getCredentialsFlow(account).first() storeMap.remove(account) } ContainerUtil.newConcurrentSet() diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/auth/ui/LazyLoadingAccountsDetailsProvider.kt b/platform/collaboration-tools/src/com/intellij/collaboration/auth/ui/LazyLoadingAccountsDetailsProvider.kt index 5bce4ff5d5b8..4bdc0e81d2e0 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/auth/ui/LazyLoadingAccountsDetailsProvider.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/auth/ui/LazyLoadingAccountsDetailsProvider.kt @@ -134,7 +134,7 @@ fun LazyLoadingAccountsDetailsProvider.cancelOnRemoval(scope coroutineScope { for (account in it) { launch { - accountManager.getCredentialsFlow(account, false).collect { + accountManager.getCredentialsFlow(account).collect { clearDetails(account) } } diff --git a/platform/collaboration-tools/test/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolderTest.kt b/platform/collaboration-tools/test/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolderTest.kt index 4ab61012ba2b..9ab027b8893e 100644 --- a/platform/collaboration-tools/test/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolderTest.kt +++ b/platform/collaboration-tools/test/com/intellij/collaboration/auth/AccountUrlAuthenticationFailuresHolderTest.kt @@ -17,7 +17,7 @@ class AccountUrlAuthenticationFailuresHolderTest { fun `test not failed on token change`() = runTest(UnconfinedTestDispatcher()) { val credsFlow = MutableSharedFlow() val accountManager = mock> { - on(it.getCredentialsFlow(any(), any())).thenReturn(credsFlow) + on(it.getCredentialsFlow(any())).thenReturn(credsFlow) } val holder = AccountUrlAuthenticationFailuresHolder(this) { accountManager diff --git a/plugins/git4idea/src/git4idea/remote/hosting/ui/RepositoryAndAccountSelectorViewModelBase.kt b/plugins/git4idea/src/git4idea/remote/hosting/ui/RepositoryAndAccountSelectorViewModelBase.kt index db4a2d2082cf..d06131e9b9ea 100644 --- a/plugins/git4idea/src/git4idea/remote/hosting/ui/RepositoryAndAccountSelectorViewModelBase.kt +++ b/plugins/git4idea/src/git4idea/remote/hosting/ui/RepositoryAndAccountSelectorViewModelBase.kt @@ -8,7 +8,9 @@ import com.intellij.collaboration.util.URIUtil import git4idea.remote.hosting.HostedGitRepositoriesManager import git4idea.remote.hosting.HostedGitRepositoryMapping import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.* abstract class RepositoryAndAccountSelectorViewModelBase( @@ -32,14 +34,15 @@ abstract class RepositoryAndAccountSelectorViewModelBase(null) + @OptIn(ExperimentalCoroutinesApi::class) final override val missingCredentialsState: StateFlow = - channelFlow { - accountSelectionState.collectLatest { - if(it == null) { - send(false) - } else { - accountManager.getCredentialsFlow(it, true).collect { creds -> - send(creds == null) + accountSelectionState.transformLatest { + if(it == null) { + emit(false) + } else { + coroutineScope { + accountManager.getCredentialsState(this, it).collect { creds -> + emit(creds == null) } } } diff --git a/plugins/github/src/org/jetbrains/plugins/github/authentication/GithubAuthenticationManager.kt b/plugins/github/src/org/jetbrains/plugins/github/authentication/GithubAuthenticationManager.kt index 26f7b8be80a9..d46a7814327f 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/authentication/GithubAuthenticationManager.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/authentication/GithubAuthenticationManager.kt @@ -47,7 +47,7 @@ class GithubAuthenticationManager internal constructor() { listener.onAccountListChanged(prev, current) current.forEach { acc -> async { - accountManager.getCredentialsFlow(acc, false).collectLatest { + accountManager.getCredentialsFlow(acc).collectLatest { listener.onAccountCredentialsChanged(acc) } } diff --git a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/toolwindow/GHRepositoryConnectionManager.kt b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/toolwindow/GHRepositoryConnectionManager.kt index 0a0761a01538..6fc9a62fe269 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/toolwindow/GHRepositoryConnectionManager.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/toolwindow/GHRepositoryConnectionManager.kt @@ -40,7 +40,7 @@ internal class GHRepositoryConnectionManager(scope: CoroutineScope, } }.collectLatest { coroutineScope { - accountManager.getCredentialsFlow(account).collectLatest { token -> + accountManager.getCredentialsState(this, account).collectLatest { token -> if (token == null) { throw CancellationException() } diff --git a/plugins/github/src/org/jetbrains/plugins/github/ui/cloneDialog/GHCloneDialogExtensionComponentBase.kt b/plugins/github/src/org/jetbrains/plugins/github/ui/cloneDialog/GHCloneDialogExtensionComponentBase.kt index a5a49e858e90..b629e8898086 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/ui/cloneDialog/GHCloneDialogExtensionComponentBase.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/ui/cloneDialog/GHCloneDialogExtensionComponentBase.kt @@ -375,7 +375,7 @@ internal abstract class GHCloneDialogExtensionComponentBase( if (!currentAccounts.contains(it)) { model.add(it) async { - accountManager.getCredentialsFlow(it, false).collect { _ -> + accountManager.getCredentialsFlow(it).collect { _ -> model.contentsChanged(it) } } diff --git a/plugins/gitlab/src/org/jetbrains/plugins/gitlab/api/GitLabProjectConnectionManager.kt b/plugins/gitlab/src/org/jetbrains/plugins/gitlab/api/GitLabProjectConnectionManager.kt index 529ff1239156..612cf079553a 100644 --- a/plugins/gitlab/src/org/jetbrains/plugins/gitlab/api/GitLabProjectConnectionManager.kt +++ b/plugins/gitlab/src/org/jetbrains/plugins/gitlab/api/GitLabProjectConnectionManager.kt @@ -41,7 +41,7 @@ internal class GitLabProjectConnectionManagerImpl(scope: CoroutineScope, } }.collectLatest { coroutineScope { - accountManager.getCredentialsFlow(account).collectLatest { token -> + accountManager.getCredentialsState(this, account).collectLatest { token -> if (token == null) { throw CancellationException() } diff --git a/plugins/gitlab/test/org/jetbrains/plugins/gitlab/mergerequest/ui/GitLabRepositoryAndAccountSelectorViewModelTest.kt b/plugins/gitlab/test/org/jetbrains/plugins/gitlab/mergerequest/ui/GitLabRepositoryAndAccountSelectorViewModelTest.kt index 3db849abda60..4bc277cba161 100644 --- a/plugins/gitlab/test/org/jetbrains/plugins/gitlab/mergerequest/ui/GitLabRepositoryAndAccountSelectorViewModelTest.kt +++ b/plugins/gitlab/test/org/jetbrains/plugins/gitlab/mergerequest/ui/GitLabRepositoryAndAccountSelectorViewModelTest.kt @@ -22,10 +22,7 @@ import org.junit.Test import org.mockito.Mock import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoRule -import org.mockito.kotlin.doAnswer -import org.mockito.kotlin.doReturn -import org.mockito.kotlin.mock -import org.mockito.kotlin.whenever +import org.mockito.kotlin.* @OptIn(ExperimentalCoroutinesApi::class) internal class GitLabRepositoryAndAccountSelectorViewModelTest { @@ -62,6 +59,7 @@ internal class GitLabRepositoryAndAccountSelectorViewModelTest { val account = GitLabAccount(name = "test", server = GitLabServerPath.DEFAULT_SERVER) whenever(accountManager.accountsState) doReturn MutableStateFlow(setOf(account)) + whenever(accountManager.getCredentialsState(any(), any())) doReturn MutableStateFlow("") val scope = childScope(Dispatchers.Main) val vm = GitLabRepositoryAndAccountSelectorViewModel(scope, connectionManager, projectManager, accountManager)