All user input syncs should be forced (#4370)

This commit is contained in:
David Perez 2024-11-22 13:24:38 -06:00 committed by GitHub
parent a1881ce4d9
commit 366c86da41
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 107 additions and 93 deletions

View file

@ -109,7 +109,7 @@ interface VaultRepository : CipherManager, VaultLockManager {
* Unlike [syncIfNecessary], this will always perform the requested sync and should only be * Unlike [syncIfNecessary], this will always perform the requested sync and should only be
* utilized in cases where the user specifically requested the action. * utilized in cases where the user specifically requested the action.
*/ */
fun sync() fun sync(forced: Boolean = false)
/** /**
* Checks if conditions have been met to perform a sync request and, if so, syncs the vault * Checks if conditions have been met to perform a sync request and, if so, syncs the vault

View file

@ -345,7 +345,7 @@ class VaultRepositoryImpl(
} }
} }
override fun sync() { override fun sync(forced: Boolean) {
val userId = activeUserId ?: return val userId = activeUserId ?: return
if (!syncJob.isCompleted) return if (!syncJob.isCompleted) return
mutableCiphersStateFlow.updateToPendingOrLoading() mutableCiphersStateFlow.updateToPendingOrLoading()
@ -353,7 +353,7 @@ class VaultRepositoryImpl(
mutableFoldersStateFlow.updateToPendingOrLoading() mutableFoldersStateFlow.updateToPendingOrLoading()
mutableCollectionsStateFlow.updateToPendingOrLoading() mutableCollectionsStateFlow.updateToPendingOrLoading()
mutableSendDataStateFlow.updateToPendingOrLoading() mutableSendDataStateFlow.updateToPendingOrLoading()
syncJob = ioScope.launch { syncInternal(userId) } syncJob = ioScope.launch { syncInternal(userId = userId, forced = forced) }
} }
@Suppress("MagicNumber") @Suppress("MagicNumber")
@ -374,10 +374,9 @@ class VaultRepositoryImpl(
} }
override suspend fun syncForResult(): SyncVaultDataResult { override suspend fun syncForResult(): SyncVaultDataResult {
val userId = activeUserId val userId = activeUserId ?: return SyncVaultDataResult.Error(throwable = null)
?: return SyncVaultDataResult.Error(throwable = null)
syncJob = ioScope syncJob = ioScope
.async { syncInternal(userId) } .async { syncInternal(userId = userId, forced = false) }
.also { .also {
return try { return try {
it.await() it.await()
@ -1339,17 +1338,20 @@ class VaultRepositoryImpl(
//endregion Push Notification helpers //endregion Push Notification helpers
@Suppress("LongMethod") @Suppress("LongMethod")
private suspend fun syncInternal(userId: String): SyncVaultDataResult { private suspend fun syncInternal(
userId: String,
forced: Boolean,
): SyncVaultDataResult {
if (!forced) {
// Skip this check if we are forcing the request.
val lastSyncInstant = settingsDiskSource val lastSyncInstant = settingsDiskSource
.getLastSyncTime(userId = userId) .getLastSyncTime(userId = userId)
?.toEpochMilli() ?.toEpochMilli()
?: 0 ?: 0
val lastDatabaseSchemeChangeInstant = databaseSchemeManager val lastDatabaseSchemeChangeInstant = databaseSchemeManager
.lastDatabaseSchemeChangeInstant .lastDatabaseSchemeChangeInstant
?.toEpochMilli() ?.toEpochMilli()
?: 0 ?: 0
syncService syncService
.getAccountRevisionDateMillis() .getAccountRevisionDateMillis()
.fold( .fold(
@ -1359,7 +1361,7 @@ class VaultRepositoryImpl(
) { ) {
// We can skip the actual sync call if there is no new data or database // We can skip the actual sync call if there is no new data or database
// scheme changes since the last sync. // scheme changes since the last sync.
vaultDiskSource.resyncVaultData(userId) vaultDiskSource.resyncVaultData(userId = userId)
settingsDiskSource.storeLastSyncTime( settingsDiskSource.storeLastSyncTime(
userId = userId, userId = userId,
lastSyncTime = clock.instant(), lastSyncTime = clock.instant(),
@ -1367,23 +1369,22 @@ class VaultRepositoryImpl(
val itemsAvailable = vaultDiskSource val itemsAvailable = vaultDiskSource
.getCiphers(userId) .getCiphers(userId)
.firstOrNull() .firstOrNull()
?.isNotEmpty() ?.isNotEmpty() == true
?: false
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable) return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
} }
}, },
onFailure = { onFailure = {
updateVaultStateFlowsToError(it) updateVaultStateFlowsToError(throwable = it)
return SyncVaultDataResult.Error(it) return SyncVaultDataResult.Error(throwable = it)
}, },
) )
}
syncService return syncService
.sync() .sync()
.fold( .fold(
onSuccess = { syncResponse -> onSuccess = { syncResponse ->
val localSecurityStamp = val localSecurityStamp = authDiskSource.userState?.activeAccount?.profile?.stamp
authDiskSource.userState?.activeAccount?.profile?.stamp
val serverSecurityStamp = syncResponse.profile.securityStamp val serverSecurityStamp = syncResponse.profile.securityStamp
// Log the user out if the stamps do not match // Log the user out if the stamps do not match
@ -1395,9 +1396,7 @@ class VaultRepositoryImpl(
} }
// Update user information with additional information from sync response // Update user information with additional information from sync response
authDiskSource.userState = authDiskSource authDiskSource.userState = authDiskSource.userState?.toUpdatedUserStateJson(
.userState
?.toUpdatedUserStateJson(
syncResponse = syncResponse, syncResponse = syncResponse,
) )
@ -1414,12 +1413,12 @@ class VaultRepositoryImpl(
lastSyncTime = clock.instant(), lastSyncTime = clock.instant(),
) )
vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse) vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse)
val itemsAvailable = syncResponse.ciphers?.isNotEmpty() ?: false val itemsAvailable = syncResponse.ciphers?.isNotEmpty() == true
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable) SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
}, },
onFailure = { throwable -> onFailure = { throwable ->
updateVaultStateFlowsToError(throwable) updateVaultStateFlowsToError(throwable = throwable)
return SyncVaultDataResult.Error(throwable) SyncVaultDataResult.Error(throwable = throwable)
}, },
) )
} }

View file

@ -99,7 +99,7 @@ class OtherViewModel @Inject constructor(
mutableStateFlow.update { mutableStateFlow.update {
it.copy(dialogState = OtherState.DialogState.Loading(R.string.syncing.asText())) it.copy(dialogState = OtherState.DialogState.Loading(R.string.syncing.asText()))
} }
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
private fun handleInternalAction(action: OtherAction.Internal) { private fun handleInternalAction(action: OtherAction.Internal) {

View file

@ -255,7 +255,7 @@ class SendViewModel @Inject constructor(
private fun handleRefreshClick() { private fun handleRefreshClick() {
// No need to update the view state, the vault repo will emit a new state during this time. // No need to update the view state, the vault repo will emit a new state during this time.
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
private fun handleSearchClick() { private fun handleSearchClick() {
@ -266,7 +266,7 @@ class SendViewModel @Inject constructor(
mutableStateFlow.update { mutableStateFlow.update {
it.copy(dialogState = SendState.DialogState.Loading(R.string.syncing.asText())) it.copy(dialogState = SendState.DialogState.Loading(R.string.syncing.asText()))
} }
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
private fun handleCopyClick(action: SendAction.CopyClick) { private fun handleCopyClick(action: SendAction.CopyClick) {
@ -321,7 +321,7 @@ class SendViewModel @Inject constructor(
mutableStateFlow.update { it.copy(isRefreshing = true) } mutableStateFlow.update { it.copy(isRefreshing = true) }
// The Pull-To-Refresh composable is already in the refreshing state. // The Pull-To-Refresh composable is already in the refreshing state.
// We will reset that state when sendDataStateFlow emits later on. // We will reset that state when sendDataStateFlow emits later on.
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
} }

View file

@ -255,7 +255,7 @@ class VaultItemViewModel @Inject constructor(
private fun handleRefreshClick() { private fun handleRefreshClick() {
// No need to update the view state, the vault repo will emit a new state during this time // No need to update the view state, the vault repo will emit a new state during this time
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleCopyCustomHiddenFieldClick( private fun handleCopyCustomHiddenFieldClick(

View file

@ -300,14 +300,14 @@ class VaultItemListingViewModel @Inject constructor(
} }
private fun handleRefreshClick() { private fun handleRefreshClick() {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleRefreshPull() { private fun handleRefreshPull() {
mutableStateFlow.update { it.copy(isRefreshing = true) } mutableStateFlow.update { it.copy(isRefreshing = true) }
// The Pull-To-Refresh composable is already in the refreshing state. // The Pull-To-Refresh composable is already in the refreshing state.
// We will reset that state when sendDataStateFlow emits later on. // We will reset that state when sendDataStateFlow emits later on.
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleConfirmOverwriteExistingPasskeyClick( private fun handleConfirmOverwriteExistingPasskeyClick(
@ -877,7 +877,7 @@ class VaultItemListingViewModel @Inject constructor(
), ),
) )
} }
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleSearchIconClick() { private fun handleSearchIconClick() {

View file

@ -299,7 +299,7 @@ class VaultViewModel @Inject constructor(
mutableStateFlow.update { mutableStateFlow.update {
it.copy(dialog = VaultState.DialogState.Syncing) it.copy(dialog = VaultState.DialogState.Syncing)
} }
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleLockClick() { private fun handleLockClick() {
@ -346,7 +346,7 @@ class VaultViewModel @Inject constructor(
} }
private fun handleTryAgainClick() { private fun handleTryAgainClick() {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleDialogDismiss() { private fun handleDialogDismiss() {
@ -359,7 +359,7 @@ class VaultViewModel @Inject constructor(
mutableStateFlow.update { it.copy(isRefreshing = true) } mutableStateFlow.update { it.copy(isRefreshing = true) }
// The Pull-To-Refresh composable is already in the refreshing state. // The Pull-To-Refresh composable is already in the refreshing state.
// We will reset that state when sendDataStateFlow emits later on. // We will reset that state when sendDataStateFlow emits later on.
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleOverflowOptionClick(action: VaultAction.OverflowOptionClick) { private fun handleOverflowOptionClick(action: VaultAction.OverflowOptionClick) {

View file

@ -120,14 +120,14 @@ class VerificationCodeViewModel @Inject constructor(
} }
private fun handleRefreshClick() { private fun handleRefreshClick() {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleRefreshPull() { private fun handleRefreshPull() {
mutableStateFlow.update { it.copy(isRefreshing = true) } mutableStateFlow.update { it.copy(isRefreshing = true) }
// The Pull-To-Refresh composable is already in the refreshing state. // The Pull-To-Refresh composable is already in the refreshing state.
// We will reset that state when sendDataStateFlow emits later on. // We will reset that state when sendDataStateFlow emits later on.
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleSearchIconClick() { private fun handleSearchIconClick() {
@ -144,7 +144,7 @@ class VerificationCodeViewModel @Inject constructor(
), ),
) )
} }
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
private fun handleInternalAction(action: VerificationCodeAction.Internal) { private fun handleInternalAction(action: VerificationCodeAction.Internal) {

View file

@ -816,6 +816,21 @@ class VaultRepositoryTest {
coVerify(exactly = 0) { syncService.sync() } coVerify(exactly = 0) { syncService.sync() }
} }
@Test
fun `sync with forced should skip checks and call the syncService sync`() {
fakeAuthDiskSource.userState = MOCK_USER_STATE
coEvery { syncService.sync() } returns Throwable("failure").asFailure()
vaultRepository.sync(forced = true)
coVerify(exactly = 0) {
syncService.getAccountRevisionDateMillis()
}
coVerify(exactly = 1) {
syncService.sync()
}
}
@Suppress("MaxLineLength") @Suppress("MaxLineLength")
@Test @Test
fun `sync with syncService Success should unlock the vault for orgs if necessary and update AuthDiskSource and VaultDiskSource`() = fun `sync with syncService Success should unlock the vault for orgs if necessary and update AuthDiskSource and VaultDiskSource`() =

View file

@ -100,7 +100,6 @@ class SearchViewModelTest : BaseViewModelTest() {
private val vaultRepository: VaultRepository = mockk { private val vaultRepository: VaultRepository = mockk {
every { vaultFilterType } returns VaultFilterType.AllVaults every { vaultFilterType } returns VaultFilterType.AllVaults
every { vaultDataStateFlow } returns mutableVaultDataStateFlow every { vaultDataStateFlow } returns mutableVaultDataStateFlow
every { sync() } just runs
} }
private val mutableUserStateFlow = MutableStateFlow<UserState?>(DEFAULT_USER_STATE) private val mutableUserStateFlow = MutableStateFlow<UserState?>(DEFAULT_USER_STATE)
private val authRepository: AuthRepository = mockk { private val authRepository: AuthRepository = mockk {

View file

@ -149,7 +149,7 @@ class OtherViewModelTest : BaseViewModelTest() {
@Test @Test
fun `on SyncNowButtonClick should sync repo`() = runTest { fun `on SyncNowButtonClick should sync repo`() = runTest {
every { vaultRepository.sync() } just runs every { vaultRepository.sync(forced = true) } just runs
val viewModel = createViewModel() val viewModel = createViewModel()
viewModel.stateFlow.test { viewModel.stateFlow.test {
assertEquals(DEFAULT_STATE, awaitItem()) assertEquals(DEFAULT_STATE, awaitItem())
@ -163,7 +163,7 @@ class OtherViewModelTest : BaseViewModelTest() {
awaitItem(), awaitItem(),
) )
} }
verify { vaultRepository.sync() } verify { vaultRepository.sync(forced = true) }
} }
@Test @Test

View file

@ -105,12 +105,12 @@ class SendViewModelTest : BaseViewModelTest() {
@Test @Test
fun `RefreshClick should call sync`() { fun `RefreshClick should call sync`() {
val viewModel = createViewModel() val viewModel = createViewModel()
every { vaultRepo.sync() } just runs every { vaultRepo.sync(forced = true) } just runs
viewModel.trySendAction(SendAction.RefreshClick) viewModel.trySendAction(SendAction.RefreshClick)
verify { verify {
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
} }
@ -223,7 +223,7 @@ class SendViewModelTest : BaseViewModelTest() {
@Test @Test
fun `SyncClick should call sync`() { fun `SyncClick should call sync`() {
val viewModel = createViewModel() val viewModel = createViewModel()
every { vaultRepo.sync() } just runs every { vaultRepo.sync(forced = true) } just runs
viewModel.trySendAction(SendAction.SyncClick) viewModel.trySendAction(SendAction.SyncClick)
@ -234,7 +234,7 @@ class SendViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value, viewModel.stateFlow.value,
) )
verify { verify {
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
} }
@ -419,13 +419,13 @@ class SendViewModelTest : BaseViewModelTest() {
@Test @Test
fun `RefreshPull should call vault repository sync`() { fun `RefreshPull should call vault repository sync`() {
every { vaultRepo.sync() } just runs every { vaultRepo.sync(forced = true) } just runs
val viewModel = createViewModel() val viewModel = createViewModel()
viewModel.trySendAction(SendAction.RefreshPull) viewModel.trySendAction(SendAction.RefreshPull)
verify(exactly = 1) { verify(exactly = 1) {
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
} }

View file

@ -950,13 +950,13 @@ class VaultItemViewModelTest : BaseViewModelTest() {
@Test @Test
fun `on RefreshClick should sync`() = runTest { fun `on RefreshClick should sync`() = runTest {
every { vaultRepo.sync() } just runs every { vaultRepo.sync(forced = true) } just runs
val viewModel = createViewModel(state = DEFAULT_STATE) val viewModel = createViewModel(state = DEFAULT_STATE)
viewModel.trySendAction(VaultItemAction.Common.RefreshClick) viewModel.trySendAction(VaultItemAction.Common.RefreshClick)
verify(exactly = 1) { verify(exactly = 1) {
vaultRepo.sync() vaultRepo.sync(forced = true)
} }
} }

View file

@ -135,7 +135,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
every { vaultFilterType } returns VaultFilterType.AllVaults every { vaultFilterType } returns VaultFilterType.AllVaults
every { vaultDataStateFlow } returns mutableVaultDataStateFlow every { vaultDataStateFlow } returns mutableVaultDataStateFlow
every { lockVault(any()) } just runs every { lockVault(any()) } just runs
every { sync() } just runs every { sync(forced = true) } just runs
coEvery { coEvery {
getDecryptedFido2CredentialAutofillViews(any()) getDecryptedFido2CredentialAutofillViews(any())
} returns DecryptFido2CredentialAutofillViewResult.Error } returns DecryptFido2CredentialAutofillViewResult.Error
@ -351,7 +351,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value, viewModel.stateFlow.value,
) )
verify(exactly = 1) { verify(exactly = 1) {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
} }
@ -1029,7 +1029,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
fun `RefreshClick should sync`() = runTest { fun `RefreshClick should sync`() = runTest {
val viewModel = createVaultItemListingViewModel() val viewModel = createVaultItemListingViewModel()
viewModel.trySendAction(VaultItemListingsAction.RefreshClick) viewModel.trySendAction(VaultItemListingsAction.RefreshClick)
verify { vaultRepository.sync() } verify { vaultRepository.sync(forced = true) }
} }
@Test @Test
@ -2127,7 +2127,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(VaultItemListingsAction.RefreshPull) viewModel.trySendAction(VaultItemListingsAction.RefreshPull)
verify(exactly = 1) { verify(exactly = 1) {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
} }

View file

@ -120,7 +120,7 @@ class VaultViewModelTest : BaseViewModelTest() {
mockk { mockk {
every { vaultFilterType = any() } just runs every { vaultFilterType = any() } just runs
every { vaultDataStateFlow } returns mutableVaultDataStateFlow every { vaultDataStateFlow } returns mutableVaultDataStateFlow
every { sync() } just runs every { sync(forced = any()) } just runs
every { syncIfNecessary() } just runs every { syncIfNecessary() } just runs
every { lockVaultForCurrentUser() } just runs every { lockVaultForCurrentUser() } just runs
every { lockVault(any()) } just runs every { lockVault(any()) } just runs
@ -478,7 +478,7 @@ class VaultViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value, viewModel.stateFlow.value,
) )
verify { verify {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
} }
@ -1323,7 +1323,7 @@ class VaultViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(VaultAction.TryAgainClick) viewModel.trySendAction(VaultAction.TryAgainClick)
verify { vaultRepository.sync() } verify { vaultRepository.sync(forced = true) }
} }
@Test @Test
@ -1365,7 +1365,7 @@ class VaultViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(VaultAction.RefreshPull) viewModel.trySendAction(VaultAction.RefreshPull)
verify(exactly = 1) { verify(exactly = 1) {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
} }
@ -1830,6 +1830,7 @@ class VaultViewModelTest : BaseViewModelTest() {
snackbarRelayManager.clearRelayBuffer(SnackbarRelay.MY_VAULT_RELAY) snackbarRelayManager.clearRelayBuffer(SnackbarRelay.MY_VAULT_RELAY)
} }
} }
private fun createViewModel(): VaultViewModel = private fun createViewModel(): VaultViewModel =
VaultViewModel( VaultViewModel(
authRepository = authRepository, authRepository = authRepository,

View file

@ -47,7 +47,7 @@ class VerificationCodeViewModelTest : BaseViewModelTest() {
private val vaultRepository: VaultRepository = mockk { private val vaultRepository: VaultRepository = mockk {
every { vaultFilterType } returns VaultFilterType.AllVaults every { vaultFilterType } returns VaultFilterType.AllVaults
every { getAuthCodesFlow() } returns mutableAuthCodeFlow.asStateFlow() every { getAuthCodesFlow() } returns mutableAuthCodeFlow.asStateFlow()
every { sync() } just runs every { sync(forced = true) } just runs
} }
private val environmentRepository: EnvironmentRepository = mockk { private val environmentRepository: EnvironmentRepository = mockk {
@ -140,7 +140,7 @@ class VerificationCodeViewModelTest : BaseViewModelTest() {
fun `RefreshClick should sync`() = runTest { fun `RefreshClick should sync`() = runTest {
val viewModel = createViewModel() val viewModel = createViewModel()
viewModel.trySendAction(VerificationCodeAction.RefreshClick) viewModel.trySendAction(VerificationCodeAction.RefreshClick)
verify { vaultRepository.sync() } verify { vaultRepository.sync(forced = true) }
} }
@Test @Test
@ -167,7 +167,7 @@ class VerificationCodeViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value, viewModel.stateFlow.value,
) )
verify(exactly = 1) { verify(exactly = 1) {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
} }
@ -456,7 +456,7 @@ class VerificationCodeViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(VerificationCodeAction.RefreshPull) viewModel.trySendAction(VerificationCodeAction.RefreshPull)
verify(exactly = 1) { verify(exactly = 1) {
vaultRepository.sync() vaultRepository.sync(forced = true)
} }
} }