Skip to content

Commit c1be5be

Browse files
M-15177: All user input syncs should be forced (#4369)
1 parent 76b6853 commit c1be5be

File tree

16 files changed

+107
-93
lines changed

16 files changed

+107
-93
lines changed

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ interface VaultRepository : CipherManager, VaultLockManager {
109109
* Unlike [syncIfNecessary], this will always perform the requested sync and should only be
110110
* utilized in cases where the user specifically requested the action.
111111
*/
112-
fun sync()
112+
fun sync(forced: Boolean = false)
113113

114114
/**
115115
* Checks if conditions have been met to perform a sync request and, if so, syncs the vault

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt

Lines changed: 54 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -345,15 +345,15 @@ class VaultRepositoryImpl(
345345
}
346346
}
347347

348-
override fun sync() {
348+
override fun sync(forced: Boolean) {
349349
val userId = activeUserId ?: return
350350
if (!syncJob.isCompleted) return
351351
mutableCiphersStateFlow.updateToPendingOrLoading()
352352
mutableDomainsStateFlow.updateToPendingOrLoading()
353353
mutableFoldersStateFlow.updateToPendingOrLoading()
354354
mutableCollectionsStateFlow.updateToPendingOrLoading()
355355
mutableSendDataStateFlow.updateToPendingOrLoading()
356-
syncJob = ioScope.launch { syncInternal(userId) }
356+
syncJob = ioScope.launch { syncInternal(userId = userId, forced = forced) }
357357
}
358358

359359
@Suppress("MagicNumber")
@@ -374,10 +374,9 @@ class VaultRepositoryImpl(
374374
}
375375

376376
override suspend fun syncForResult(): SyncVaultDataResult {
377-
val userId = activeUserId
378-
?: return SyncVaultDataResult.Error(throwable = null)
377+
val userId = activeUserId ?: return SyncVaultDataResult.Error(throwable = null)
379378
syncJob = ioScope
380-
.async { syncInternal(userId) }
379+
.async { syncInternal(userId = userId, forced = false) }
381380
.also {
382381
return try {
383382
it.await()
@@ -1339,51 +1338,53 @@ class VaultRepositoryImpl(
13391338
//endregion Push Notification helpers
13401339

13411340
@Suppress("LongMethod")
1342-
private suspend fun syncInternal(userId: String): SyncVaultDataResult {
1343-
val lastSyncInstant = settingsDiskSource
1344-
.getLastSyncTime(userId = userId)
1345-
?.toEpochMilli()
1346-
?: 0
1347-
1348-
val lastDatabaseSchemeChangeInstant = databaseSchemeManager
1349-
.lastDatabaseSchemeChangeInstant
1350-
?.toEpochMilli()
1351-
?: 0
1352-
1353-
syncService
1354-
.getAccountRevisionDateMillis()
1355-
.fold(
1356-
onSuccess = { serverRevisionDate ->
1357-
if (serverRevisionDate < lastSyncInstant &&
1358-
lastDatabaseSchemeChangeInstant < lastSyncInstant
1359-
) {
1360-
// We can skip the actual sync call if there is no new data or database
1361-
// scheme changes since the last sync.
1362-
vaultDiskSource.resyncVaultData(userId)
1363-
settingsDiskSource.storeLastSyncTime(
1364-
userId = userId,
1365-
lastSyncTime = clock.instant(),
1366-
)
1367-
val itemsAvailable = vaultDiskSource
1368-
.getCiphers(userId)
1369-
.firstOrNull()
1370-
?.isNotEmpty()
1371-
?: false
1372-
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
1373-
}
1374-
},
1375-
onFailure = {
1376-
updateVaultStateFlowsToError(it)
1377-
return SyncVaultDataResult.Error(it)
1378-
},
1379-
)
1341+
private suspend fun syncInternal(
1342+
userId: String,
1343+
forced: Boolean,
1344+
): SyncVaultDataResult {
1345+
if (!forced) {
1346+
// Skip this check if we are forcing the request.
1347+
val lastSyncInstant = settingsDiskSource
1348+
.getLastSyncTime(userId = userId)
1349+
?.toEpochMilli()
1350+
?: 0
1351+
val lastDatabaseSchemeChangeInstant = databaseSchemeManager
1352+
.lastDatabaseSchemeChangeInstant
1353+
?.toEpochMilli()
1354+
?: 0
1355+
syncService
1356+
.getAccountRevisionDateMillis()
1357+
.fold(
1358+
onSuccess = { serverRevisionDate ->
1359+
if (serverRevisionDate < lastSyncInstant &&
1360+
lastDatabaseSchemeChangeInstant < lastSyncInstant
1361+
) {
1362+
// We can skip the actual sync call if there is no new data or database
1363+
// scheme changes since the last sync.
1364+
vaultDiskSource.resyncVaultData(userId = userId)
1365+
settingsDiskSource.storeLastSyncTime(
1366+
userId = userId,
1367+
lastSyncTime = clock.instant(),
1368+
)
1369+
val itemsAvailable = vaultDiskSource
1370+
.getCiphers(userId)
1371+
.firstOrNull()
1372+
?.isNotEmpty() == true
1373+
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
1374+
}
1375+
},
1376+
onFailure = {
1377+
updateVaultStateFlowsToError(throwable = it)
1378+
return SyncVaultDataResult.Error(throwable = it)
1379+
},
1380+
)
1381+
}
13801382

1381-
syncService
1383+
return syncService
13821384
.sync()
13831385
.fold(
13841386
onSuccess = { syncResponse ->
1385-
val localSecurityStamp =
1386-
authDiskSource.userState?.activeAccount?.profile?.stamp
1387+
val localSecurityStamp = authDiskSource.userState?.activeAccount?.profile?.stamp
13871388
val serverSecurityStamp = syncResponse.profile.securityStamp
13881389

13891390
// Log the user out if the stamps do not match
@@ -1395,11 +1396,9 @@ class VaultRepositoryImpl(
13951396
}
13961397

13971398
// Update user information with additional information from sync response
1398-
authDiskSource.userState = authDiskSource
1399-
.userState
1400-
?.toUpdatedUserStateJson(
1401-
syncResponse = syncResponse,
1402-
)
1399+
authDiskSource.userState = authDiskSource.userState?.toUpdatedUserStateJson(
1400+
syncResponse = syncResponse,
1401+
)
14031402

14041403
unlockVaultForOrganizationsIfNecessary(syncResponse = syncResponse)
14051404
storeProfileData(syncResponse = syncResponse)
@@ -1414,12 +1413,12 @@ class VaultRepositoryImpl(
14141413
lastSyncTime = clock.instant(),
14151414
)
14161415
vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse)
1417-
val itemsAvailable = syncResponse.ciphers?.isNotEmpty() ?: false
1418-
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
1416+
val itemsAvailable = syncResponse.ciphers?.isNotEmpty() == true
1417+
SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
14191418
},
14201419
onFailure = { throwable ->
1421-
updateVaultStateFlowsToError(throwable)
1422-
return SyncVaultDataResult.Error(throwable)
1420+
updateVaultStateFlowsToError(throwable = throwable)
1421+
SyncVaultDataResult.Error(throwable = throwable)
14231422
},
14241423
)
14251424
}

app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModel.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class OtherViewModel @Inject constructor(
9999
mutableStateFlow.update {
100100
it.copy(dialogState = OtherState.DialogState.Loading(R.string.syncing.asText()))
101101
}
102-
vaultRepo.sync()
102+
vaultRepo.sync(forced = true)
103103
}
104104

105105
private fun handleInternalAction(action: OtherAction.Internal) {

app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ class SendViewModel @Inject constructor(
255255

256256
private fun handleRefreshClick() {
257257
// No need to update the view state, the vault repo will emit a new state during this time.
258-
vaultRepo.sync()
258+
vaultRepo.sync(forced = true)
259259
}
260260

261261
private fun handleSearchClick() {
@@ -266,7 +266,7 @@ class SendViewModel @Inject constructor(
266266
mutableStateFlow.update {
267267
it.copy(dialogState = SendState.DialogState.Loading(R.string.syncing.asText()))
268268
}
269-
vaultRepo.sync()
269+
vaultRepo.sync(forced = true)
270270
}
271271

272272
private fun handleCopyClick(action: SendAction.CopyClick) {
@@ -321,7 +321,7 @@ class SendViewModel @Inject constructor(
321321
mutableStateFlow.update { it.copy(isRefreshing = true) }
322322
// The Pull-To-Refresh composable is already in the refreshing state.
323323
// We will reset that state when sendDataStateFlow emits later on.
324-
vaultRepo.sync()
324+
vaultRepo.sync(forced = true)
325325
}
326326
}
327327

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ class VaultItemViewModel @Inject constructor(
245245

246246
private fun handleRefreshClick() {
247247
// No need to update the view state, the vault repo will emit a new state during this time
248-
vaultRepository.sync()
248+
vaultRepository.sync(forced = true)
249249
}
250250

251251
private fun handleCopyCustomHiddenFieldClick(

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +300,14 @@ class VaultItemListingViewModel @Inject constructor(
300300
}
301301

302302
private fun handleRefreshClick() {
303-
vaultRepository.sync()
303+
vaultRepository.sync(forced = true)
304304
}
305305

306306
private fun handleRefreshPull() {
307307
mutableStateFlow.update { it.copy(isRefreshing = true) }
308308
// The Pull-To-Refresh composable is already in the refreshing state.
309309
// We will reset that state when sendDataStateFlow emits later on.
310-
vaultRepository.sync()
310+
vaultRepository.sync(forced = true)
311311
}
312312

313313
private fun handleConfirmOverwriteExistingPasskeyClick(
@@ -877,7 +877,7 @@ class VaultItemListingViewModel @Inject constructor(
877877
),
878878
)
879879
}
880-
vaultRepository.sync()
880+
vaultRepository.sync(forced = true)
881881
}
882882

883883
private fun handleSearchIconClick() {

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ class VaultViewModel @Inject constructor(
299299
mutableStateFlow.update {
300300
it.copy(dialog = VaultState.DialogState.Syncing)
301301
}
302-
vaultRepository.sync()
302+
vaultRepository.sync(forced = true)
303303
}
304304

305305
private fun handleLockClick() {
@@ -346,7 +346,7 @@ class VaultViewModel @Inject constructor(
346346
}
347347

348348
private fun handleTryAgainClick() {
349-
vaultRepository.sync()
349+
vaultRepository.sync(forced = true)
350350
}
351351

352352
private fun handleDialogDismiss() {
@@ -359,7 +359,7 @@ class VaultViewModel @Inject constructor(
359359
mutableStateFlow.update { it.copy(isRefreshing = true) }
360360
// The Pull-To-Refresh composable is already in the refreshing state.
361361
// We will reset that state when sendDataStateFlow emits later on.
362-
vaultRepository.sync()
362+
vaultRepository.sync(forced = true)
363363
}
364364

365365
private fun handleOverflowOptionClick(action: VaultAction.OverflowOptionClick) {

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/verificationcode/VerificationCodeViewModel.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,14 @@ class VerificationCodeViewModel @Inject constructor(
120120
}
121121

122122
private fun handleRefreshClick() {
123-
vaultRepository.sync()
123+
vaultRepository.sync(forced = true)
124124
}
125125

126126
private fun handleRefreshPull() {
127127
mutableStateFlow.update { it.copy(isRefreshing = true) }
128128
// The Pull-To-Refresh composable is already in the refreshing state.
129129
// We will reset that state when sendDataStateFlow emits later on.
130-
vaultRepository.sync()
130+
vaultRepository.sync(forced = true)
131131
}
132132

133133
private fun handleSearchIconClick() {
@@ -144,7 +144,7 @@ class VerificationCodeViewModel @Inject constructor(
144144
),
145145
)
146146
}
147-
vaultRepository.sync()
147+
vaultRepository.sync(forced = true)
148148
}
149149

150150
private fun handleInternalAction(action: VerificationCodeAction.Internal) {

app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,21 @@ class VaultRepositoryTest {
816816
coVerify(exactly = 0) { syncService.sync() }
817817
}
818818

819+
@Test
820+
fun `sync with forced should skip checks and call the syncService sync`() {
821+
fakeAuthDiskSource.userState = MOCK_USER_STATE
822+
coEvery { syncService.sync() } returns Throwable("failure").asFailure()
823+
824+
vaultRepository.sync(forced = true)
825+
826+
coVerify(exactly = 0) {
827+
syncService.getAccountRevisionDateMillis()
828+
}
829+
coVerify(exactly = 1) {
830+
syncService.sync()
831+
}
832+
}
833+
819834
@Suppress("MaxLineLength")
820835
@Test
821836
fun `sync with syncService Success should unlock the vault for orgs if necessary and update AuthDiskSource and VaultDiskSource`() =

app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ class SearchViewModelTest : BaseViewModelTest() {
100100
private val vaultRepository: VaultRepository = mockk {
101101
every { vaultFilterType } returns VaultFilterType.AllVaults
102102
every { vaultDataStateFlow } returns mutableVaultDataStateFlow
103-
every { sync() } just runs
104103
}
105104
private val mutableUserStateFlow = MutableStateFlow<UserState?>(DEFAULT_USER_STATE)
106105
private val authRepository: AuthRepository = mockk {

app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModelTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class OtherViewModelTest : BaseViewModelTest() {
149149

150150
@Test
151151
fun `on SyncNowButtonClick should sync repo`() = runTest {
152-
every { vaultRepository.sync() } just runs
152+
every { vaultRepository.sync(forced = true) } just runs
153153
val viewModel = createViewModel()
154154
viewModel.stateFlow.test {
155155
assertEquals(DEFAULT_STATE, awaitItem())
@@ -163,7 +163,7 @@ class OtherViewModelTest : BaseViewModelTest() {
163163
awaitItem(),
164164
)
165165
}
166-
verify { vaultRepository.sync() }
166+
verify { vaultRepository.sync(forced = true) }
167167
}
168168

169169
@Test

app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModelTest.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,12 @@ class SendViewModelTest : BaseViewModelTest() {
105105
@Test
106106
fun `RefreshClick should call sync`() {
107107
val viewModel = createViewModel()
108-
every { vaultRepo.sync() } just runs
108+
every { vaultRepo.sync(forced = true) } just runs
109109

110110
viewModel.trySendAction(SendAction.RefreshClick)
111111

112112
verify {
113-
vaultRepo.sync()
113+
vaultRepo.sync(forced = true)
114114
}
115115
}
116116

@@ -223,7 +223,7 @@ class SendViewModelTest : BaseViewModelTest() {
223223
@Test
224224
fun `SyncClick should call sync`() {
225225
val viewModel = createViewModel()
226-
every { vaultRepo.sync() } just runs
226+
every { vaultRepo.sync(forced = true) } just runs
227227

228228
viewModel.trySendAction(SendAction.SyncClick)
229229

@@ -234,7 +234,7 @@ class SendViewModelTest : BaseViewModelTest() {
234234
viewModel.stateFlow.value,
235235
)
236236
verify {
237-
vaultRepo.sync()
237+
vaultRepo.sync(forced = true)
238238
}
239239
}
240240

@@ -419,13 +419,13 @@ class SendViewModelTest : BaseViewModelTest() {
419419

420420
@Test
421421
fun `RefreshPull should call vault repository sync`() {
422-
every { vaultRepo.sync() } just runs
422+
every { vaultRepo.sync(forced = true) } just runs
423423
val viewModel = createViewModel()
424424

425425
viewModel.trySendAction(SendAction.RefreshPull)
426426

427427
verify(exactly = 1) {
428-
vaultRepo.sync()
428+
vaultRepo.sync(forced = true)
429429
}
430430
}
431431

0 commit comments

Comments
 (0)