From a02a84ee0812e63c1a83cf223da3a30b1100e477 Mon Sep 17 00:00:00 2001 From: David Perez Date: Mon, 29 Sep 2025 14:45:56 -0500 Subject: [PATCH] PM-25642: Force sync or clear last sync time on sync notification (#5958) --- .../data/platform/manager/PushManager.kt | 4 +-- .../data/platform/manager/PushManagerImpl.kt | 9 ++++--- .../manager/model/NotificationPayload.kt | 8 ++++++ .../vault/manager/VaultSyncManagerImpl.kt | 8 +++++- .../importlogins/ImportLoginsViewModel.kt | 2 +- .../data/platform/manager/PushManagerTest.kt | 12 ++++----- .../vault/manager/VaultSyncManagerTest.kt | 22 ++++++++++++---- .../importlogins/ImportLoginsViewModelTest.kt | 26 ++++++++++++------- 8 files changed, 63 insertions(+), 28 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt index d050fd6545..d6f07394b1 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManager.kt @@ -15,9 +15,9 @@ import kotlinx.coroutines.flow.Flow */ interface PushManager { /** - * Flow that represents requests intended for full syncs. + * Flow that represents requests intended for full syncs for the user ID provided. */ - val fullSyncFlow: Flow + val fullSyncFlow: Flow /** * Flow that represents requests intended to log a user out. diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt index 177edd8d3a..1f00b22e9a 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerImpl.kt @@ -55,7 +55,7 @@ class PushManagerImpl @Inject constructor( private val ioScope = CoroutineScope(dispatcherManager.io) private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) - private val mutableFullSyncSharedFlow = bufferedMutableSharedFlow() + private val mutableFullSyncSharedFlow = bufferedMutableSharedFlow() private val mutableLogoutSharedFlow = bufferedMutableSharedFlow() private val mutablePasswordlessRequestSharedFlow = bufferedMutableSharedFlow() @@ -73,7 +73,7 @@ class PushManagerImpl @Inject constructor( private val mutableSyncSendUpsertSharedFlow = bufferedMutableSharedFlow() - override val fullSyncFlow: SharedFlow + override val fullSyncFlow: SharedFlow get() = mutableFullSyncSharedFlow.asSharedFlow() override val logoutFlow: SharedFlow @@ -204,7 +204,10 @@ class PushManagerImpl @Inject constructor( NotificationType.SYNC_SETTINGS, NotificationType.SYNC_VAULT, -> { - mutableFullSyncSharedFlow.tryEmit(Unit) + json + .decodeFromString(notification.payload) + .userId + ?.let { mutableFullSyncSharedFlow.tryEmit(it) } } NotificationType.SYNC_FOLDER_CREATE, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt index 5ca4855340..111197aa1d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/model/NotificationPayload.kt @@ -83,4 +83,12 @@ sealed class NotificationPayload { @JsonNames("UserId", "userId") override val userId: String?, @JsonNames("Id", "id") val loginRequestId: String?, ) : NotificationPayload() + + /** + * A notification payload for syncing a users vault. + */ + @Serializable + data class SyncNotification( + @JsonNames("UserId", "userId") override val userId: String?, + ) : NotificationPayload() } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerImpl.kt index aeab542120..061a34c286 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerImpl.kt @@ -209,7 +209,13 @@ class VaultSyncManagerImpl( pushManager .fullSyncFlow - .onEach { sync(forced = false) } + .onEach { userId -> + if (userId == activeUserId) { + sync(forced = false) + } else { + settingsDiskSource.storeLastSyncTime(userId = userId, lastSyncTime = null) + } + } .launchIn(unconfinedScope) databaseSchemeManager diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt index 09b83ad53c..2cc20eab6f 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt @@ -205,7 +205,7 @@ class ImportLoginsViewModel @Inject constructor( private fun syncVault() { viewModelScope.launch { - val result = vaultRepository.syncForResult() + val result = vaultRepository.syncForResult(forced = true) sendAction(ImportLoginsAction.Internal.VaultSyncResultReceived(result)) } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt index 94a7ef70a6..0d1f7a86f2 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/PushManagerTest.kt @@ -161,7 +161,7 @@ class PushManagerTest { pushManager.fullSyncFlow.test { pushManager.onMessageReceived(SYNC_CIPHERS_NOTIFICATION_MAP) assertEquals( - Unit, + "078966a2-93c2-4618-ae2a-0a2394c88d37", awaitItem(), ) } @@ -180,7 +180,7 @@ class PushManagerTest { pushManager.fullSyncFlow.test { pushManager.onMessageReceived(SYNC_SETTINGS_NOTIFICATION_MAP) assertEquals( - Unit, + "078966a2-93c2-4618-ae2a-0a2394c88d37", awaitItem(), ) } @@ -191,7 +191,7 @@ class PushManagerTest { pushManager.fullSyncFlow.test { pushManager.onMessageReceived(SYNC_VAULT_NOTIFICATION_MAP) assertEquals( - Unit, + "078966a2-93c2-4618-ae2a-0a2394c88d37", awaitItem(), ) } @@ -580,7 +580,7 @@ class PushManagerTest { pushManager.fullSyncFlow.test { pushManager.onMessageReceived(SYNC_CIPHERS_NOTIFICATION_MAP) assertEquals( - Unit, + "078966a2-93c2-4618-ae2a-0a2394c88d37", awaitItem(), ) } @@ -599,7 +599,7 @@ class PushManagerTest { pushManager.fullSyncFlow.test { pushManager.onMessageReceived(SYNC_SETTINGS_NOTIFICATION_MAP) assertEquals( - Unit, + "078966a2-93c2-4618-ae2a-0a2394c88d37", awaitItem(), ) } @@ -610,7 +610,7 @@ class PushManagerTest { pushManager.fullSyncFlow.test { pushManager.onMessageReceived(SYNC_VAULT_NOTIFICATION_MAP) assertEquals( - Unit, + "078966a2-93c2-4618-ae2a-0a2394c88d37", awaitItem(), ) } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt index aaef195b99..dd80bc978b 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultSyncManagerTest.kt @@ -126,7 +126,7 @@ class VaultSyncManagerTest { private val userLogoutManager: UserLogoutManager = mockk { every { softLogout(any(), any()) } just runs } - private val mutableFullSyncFlow = bufferedMutableSharedFlow() + private val mutableFullSyncFlow = bufferedMutableSharedFlow() private val pushManager: PushManager = mockk { every { fullSyncFlow } returns mutableFullSyncFlow } @@ -1105,15 +1105,27 @@ class VaultSyncManagerTest { } @Test - fun `fullSyncFlow emission should trigger unforced sync`() { + fun `fullSyncFlow emission with active user ID should trigger an unforced sync`() { val userId = "mockId-1" fakeAuthDiskSource.userState = MOCK_USER_STATE - every { settingsDiskSource.getLastSyncTime(userId = userId) } returns null coEvery { syncService.sync() } just awaits - mutableFullSyncFlow.tryEmit(Unit) + mutableFullSyncFlow.tryEmit(userId) - coVerify { syncService.sync() } + coVerify(exactly = 1) { syncService.sync() } + } + + @Suppress("MaxLineLength") + @Test + fun `fullSyncFlow emission with non-active user ID should clear last sync time for that user`() { + val userId = "mockId-2" + fakeAuthDiskSource.userState = MOCK_USER_STATE + + mutableFullSyncFlow.tryEmit(userId) + + coVerify(exactly = 1) { + settingsDiskSource.storeLastSyncTime(userId = userId, lastSyncTime = null) + } } @Suppress("MaxLineLength") diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt index df3a4a8c76..f17ee217ce 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModelTest.kt @@ -33,7 +33,9 @@ import org.junit.jupiter.api.Test class ImportLoginsViewModelTest : BaseViewModelTest() { private val vaultRepository: VaultRepository = mockk { - coEvery { syncForResult() } returns SyncVaultDataResult.Success(itemsAvailable = true) + coEvery { + syncForResult(forced = true) + } returns SyncVaultDataResult.Success(itemsAvailable = true) } private val firstTimeActionManager: FirstTimeActionManager = mockk { @@ -312,21 +314,23 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ) cancelAndIgnoreRemainingEvents() } - coVerify { vaultRepository.syncForResult() } + coVerify(exactly = 1) { vaultRepository.syncForResult(forced = true) } } @Suppress("MaxLineLength") @Test fun `RetryVaultSync sets isVaultSyncing to true and clears dialog state and calls syncForResult`() = runTest { - coEvery { vaultRepository.syncForResult() } returns SyncVaultDataResult.Error(Exception()) + coEvery { + vaultRepository.syncForResult(forced = true) + } returns SyncVaultDataResult.Error(Exception()) val viewModel = createViewModel() viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) viewModel.stateFlow.test { assertNotNull(awaitItem().dialogState) - coEvery { vaultRepository.syncForResult() } returns SyncVaultDataResult.Success( - itemsAvailable = true, - ) + coEvery { + vaultRepository.syncForResult(forced = true) + } returns SyncVaultDataResult.Success(itemsAvailable = true) viewModel.trySendAction(ImportLoginsAction.RetryVaultSync) assertEquals( ImportLoginsState( @@ -339,7 +343,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { ) cancelAndIgnoreRemainingEvents() } - coVerify { vaultRepository.syncForResult() } + coVerify(exactly = 2) { vaultRepository.syncForResult(forced = true) } } @Suppress("MaxLineLength") @@ -367,7 +371,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { fun `MoveToSyncInProgress should set no items imported error dialog state when sync succeeds but no items are available`() = runTest { coEvery { - vaultRepository.syncForResult() + vaultRepository.syncForResult(forced = true) } returns SyncVaultDataResult.Success(itemsAvailable = false) val viewModel = createViewModel() viewModel.stateFlow.test { @@ -402,7 +406,9 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { @Test fun `SyncVaultDataResult Error should remove loading state and show error dialog`() = runTest { - coEvery { vaultRepository.syncForResult() } returns SyncVaultDataResult.Error(Exception()) + coEvery { + vaultRepository.syncForResult(forced = true) + } returns SyncVaultDataResult.Error(Exception()) val viewModel = createViewModel() viewModel.trySendAction(ImportLoginsAction.MoveToSyncInProgress) assertEquals( @@ -420,7 +426,7 @@ class ImportLoginsViewModelTest : BaseViewModelTest() { fun `FailSyncAcknowledged should remove dialog state and send NavigateBack event`() = runTest { coEvery { - vaultRepository.syncForResult() + vaultRepository.syncForResult(forced = true) } returns SyncVaultDataResult.Error(Exception()) val viewModel = createViewModel() viewModel.eventFlow.test {