From de99c36b20db949c816a65509862c2ae11f2ed11 Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 23 Jan 2024 19:47:18 -0600 Subject: [PATCH] BIT-1282: Add UI for Vault Sync (#740) --- .../platform/repository/SettingsRepository.kt | 11 ++++ .../repository/SettingsRepositoryImpl.kt | 21 +++++++ .../vault/repository/VaultRepositoryImpl.kt | 5 ++ .../repository/di/VaultRepositoryModule.kt | 6 ++ .../feature/settings/other/OtherViewModel.kt | 58 ++++++++++++++++++- .../repository/SettingsRepositoryTest.kt | 31 ++++++++++ .../vault/repository/VaultRepositoryTest.kt | 20 ++++++- .../settings/other/OtherViewModelTest.kt | 45 +++++++++++++- 8 files changed, 191 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt index 7bdfa3d7d..162af71e3 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt @@ -6,6 +6,7 @@ import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppLang import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppTheme import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow +import java.time.Instant /** * Provides an API for observing and modifying settings state. @@ -27,6 +28,16 @@ interface SettingsRepository { */ val appThemeStateFlow: StateFlow + /** + * The currently stored last time the vault was synced. + */ + var vaultLastSync: Instant? + + /** + * Tracks changes to the [vaultLastSync]. + */ + val vaultLastSyncStateFlow: StateFlow + /** * The current setting for getting login item icons. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt index 43a6a6941..761367d76 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import java.time.Instant /** * Primary implementation of [SettingsRepository]. @@ -61,6 +62,26 @@ class SettingsRepositoryImpl( initialValue = settingsDiskSource.appTheme, ) + override var vaultLastSync: Instant? + get() = vaultLastSyncStateFlow.value + set(value) { + val userId = activeUserId ?: return + settingsDiskSource.storeLastSyncTime(userId = userId, lastSyncTime = value) + } + + override val vaultLastSyncStateFlow: StateFlow + get() = activeUserId + ?.let { + settingsDiskSource + .getLastSyncTimeFlow(userId = it) + .stateIn( + scope = unconfinedScope, + started = SharingStarted.Eagerly, + initialValue = settingsDiskSource.getLastSyncTime(userId = it), + ) + } + ?: MutableStateFlow(value = null) + override var isIconLoadingDisabled: Boolean get() = settingsDiskSource.isIconLoadingDisabled ?: false set(value) { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 0dd9201d4..cbd7e17cf 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -14,6 +14,7 @@ import com.bitwarden.crypto.Kdf import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams import com.x8bit.bitwarden.data.auth.repository.util.toUpdatedUserStateJson +import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.datasource.network.util.isNoConnectionError import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.repository.model.DataState @@ -78,6 +79,7 @@ import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import java.time.Clock import java.time.Instant /** @@ -97,9 +99,11 @@ class VaultRepositoryImpl( private val vaultDiskSource: VaultDiskSource, private val vaultSdkSource: VaultSdkSource, private val authDiskSource: AuthDiskSource, + private val settingsDiskSource: SettingsDiskSource, private val fileManager: FileManager, private val vaultLockManager: VaultLockManager, private val totpCodeManager: TotpCodeManager, + private val clock: Clock, dispatcherManager: DispatcherManager, ) : VaultRepository, VaultLockManager by vaultLockManager { @@ -230,6 +234,7 @@ class VaultRepositoryImpl( unlockVaultForOrganizationsIfNecessary(syncResponse = syncResponse) storeProfileData(syncResponse = syncResponse) vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse) + settingsDiskSource.storeLastSyncTime(userId = userId, clock.instant()) }, onFailure = { throwable -> mutableCiphersStateFlow.update { currentState -> diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt index 11432a38d..d034da103 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/di/VaultRepositoryModule.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.data.vault.repository.di import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource +import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource import com.x8bit.bitwarden.data.vault.datasource.network.service.CiphersService @@ -16,6 +17,7 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import java.time.Clock import javax.inject.Singleton /** @@ -34,10 +36,12 @@ object VaultRepositoryModule { vaultDiskSource: VaultDiskSource, vaultSdkSource: VaultSdkSource, authDiskSource: AuthDiskSource, + settingsDiskSource: SettingsDiskSource, fileManager: FileManager, vaultLockManager: VaultLockManager, dispatcherManager: DispatcherManager, totpCodeManager: TotpCodeManager, + clock: Clock, ): VaultRepository = VaultRepositoryImpl( syncService = syncService, sendsService = sendsService, @@ -45,9 +49,11 @@ object VaultRepositoryModule { vaultDiskSource = vaultDiskSource, vaultSdkSource = vaultSdkSource, authDiskSource = authDiskSource, + settingsDiskSource = settingsDiskSource, fileManager = fileManager, vaultLockManager = vaultLockManager, dispatcherManager = dispatcherManager, totpCodeManager = totpCodeManager, + clock = clock, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModel.kt index c57dee115..be56159d1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModel.kt @@ -2,24 +2,34 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.other import android.os.Parcelable import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.ui.platform.base.BaseViewModel import com.x8bit.bitwarden.ui.platform.base.util.Text import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.util.toFormattedPattern import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update import kotlinx.parcelize.Parcelize +import java.time.Clock +import java.time.Instant import javax.inject.Inject private const val KEY_STATE = "state" +private const val VAULT_LAST_SYNC_TIME_PATTERN: String = "M/d/yyyy h:mm a" + /** * View model for the other screen. */ @HiltViewModel class OtherViewModel @Inject constructor( + private val clock: Clock, private val settingsRepo: SettingsRepository, private val vaultRepo: VaultRepository, savedStateHandle: SavedStateHandle, @@ -29,16 +39,28 @@ class OtherViewModel @Inject constructor( allowScreenCapture = false, allowSyncOnRefresh = settingsRepo.getPullToRefreshEnabledFlow().value, clearClipboardFrequency = OtherState.ClearClipboardFrequency.DEFAULT, - lastSyncTime = "5/14/2023 4:52 PM", + lastSyncTime = settingsRepo + .vaultLastSync + ?.toFormattedPattern(VAULT_LAST_SYNC_TIME_PATTERN, clock.zone) + .orEmpty(), dialogState = null, ), ) { + init { + settingsRepo + .vaultLastSyncStateFlow + .map { OtherAction.Internal.VaultLastSyncReceive(it) } + .onEach(::sendAction) + .launchIn(viewModelScope) + } + override fun handleAction(action: OtherAction): Unit = when (action) { is OtherAction.AllowScreenCaptureToggle -> handleAllowScreenCaptureToggled(action) is OtherAction.AllowSyncToggle -> handleAllowSyncToggled(action) OtherAction.BackClick -> handleBackClicked() is OtherAction.ClearClipboardFrequencyChange -> handleClearClipboardFrequencyChanged(action) OtherAction.SyncNowButtonClick -> handleSyncNowButtonClicked() + is OtherAction.Internal -> handleInternalAction(action) } private fun handleAllowScreenCaptureToggled(action: OtherAction.AllowScreenCaptureToggle) { @@ -67,9 +89,29 @@ class OtherViewModel @Inject constructor( } private fun handleSyncNowButtonClicked() { - // TODO BIT-1282 add full support and visual feedback + mutableStateFlow.update { + it.copy(dialogState = OtherState.DialogState.Loading(R.string.syncing.asText())) + } vaultRepo.sync() } + + private fun handleInternalAction(action: OtherAction.Internal) { + when (action) { + is OtherAction.Internal.VaultLastSyncReceive -> handleVaultDataReceive(action) + } + } + + private fun handleVaultDataReceive(action: OtherAction.Internal.VaultLastSyncReceive) { + mutableStateFlow.update { + it.copy( + lastSyncTime = action + .vaultLastSyncTime + ?.toFormattedPattern(VAULT_LAST_SYNC_TIME_PATTERN, clock.zone) + .orEmpty(), + dialogState = null, + ) + } + } } /** @@ -154,4 +196,16 @@ sealed class OtherAction { * Indicates that the user clicked the Sync Now button. */ data object SyncNowButtonClick : OtherAction() + + /** + * Models actions that the [OtherViewModel] itself might send. + */ + sealed class Internal : OtherAction() { + /** + * Indicates last sync time of the vault has been received. + */ + data class VaultLastSyncReceive( + val vaultLastSyncTime: Instant?, + ) : Internal() + } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt index 4b1df135f..fc947f77c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt @@ -29,6 +29,7 @@ import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test +import java.time.Instant class SettingsRepositoryTest { private val autofillManager: AutofillManager = mockk { @@ -119,6 +120,36 @@ class SettingsRepositoryTest { ) } + @Test + fun `vaultLastSync should pull from and update SettingsDiskSource`() { + fakeAuthDiskSource.userState = MOCK_USER_STATE + assertNull(settingsRepository.vaultLastSync) + val instant = Instant.ofEpochMilli(1_698_408_000_000L) + + // Updates to the disk source change the repository value. + fakeSettingsDiskSource.storeLastSyncTime( + userId = MOCK_USER_STATE.activeUserId, + lastSyncTime = instant, + ) + assertEquals(instant, settingsRepository.vaultLastSync) + } + + @Test + fun `vaultLastSyncStateFlow should react to changes in SettingsDiskSource`() = runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + val instant = Instant.ofEpochMilli(1_698_408_000_000L) + settingsRepository + .vaultLastSyncStateFlow + .test { + assertNull(awaitItem()) + fakeSettingsDiskSource.storeLastSyncTime( + userId = MOCK_USER_STATE.activeUserId, + lastSyncTime = instant, + ) + assertEquals(instant, awaitItem()) + } + } + @Test fun `isIconLoadingDisabled should pull from and update SettingsDiskSource`() { assertFalse(settingsRepository.isIconLoadingDisabled) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index 1695a6f47..38d28bbe9 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -20,6 +20,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams import com.x8bit.bitwarden.data.auth.util.KdfParamsConstants.DEFAULT_PBKDF2_ITERATIONS import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager +import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.repository.model.DataState import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow @@ -95,14 +96,20 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import java.net.UnknownHostException +import java.time.Clock import java.time.Instant +import java.time.ZoneOffset @Suppress("LargeClass") class VaultRepositoryTest { - + private val clock: Clock = Clock.fixed( + Instant.parse("2023-10-27T12:00:00Z"), + ZoneOffset.UTC, + ) private val dispatcherManager: DispatcherManager = FakeDispatcherManager() private val fileManager: FileManager = mockk() private val fakeAuthDiskSource = FakeAuthDiskSource() + private val settingsDiskSource = mockk() private val syncService: SyncService = mockk() private val sendsService: SendsService = mockk() private val ciphersService: CiphersService = mockk() @@ -132,10 +139,12 @@ class VaultRepositoryTest { vaultDiskSource = vaultDiskSource, vaultSdkSource = vaultSdkSource, authDiskSource = fakeAuthDiskSource, + settingsDiskSource = settingsDiskSource, vaultLockManager = vaultLockManager, dispatcherManager = dispatcherManager, totpCodeManager = totpCodeManager, fileManager = fileManager, + clock = clock, ) @BeforeEach @@ -412,6 +421,9 @@ class VaultRepositoryTest { vault = mockSyncResponse, ) } just runs + every { + settingsDiskSource.storeLastSyncTime(MOCK_USER_STATE.activeUserId, clock.instant()) + } just runs vaultRepository.sync() @@ -1051,6 +1063,9 @@ class VaultRepositoryTest { vault = mockSyncResponse, ) } just runs + every { + settingsDiskSource.storeLastSyncTime(MOCK_USER_STATE.activeUserId, clock.instant()) + } just runs coEvery { vaultSdkSource.decryptSendList( userId = userId, @@ -2217,6 +2232,9 @@ class VaultRepositoryTest { vault = mockSyncResponse, ) } just runs + every { + settingsDiskSource.storeLastSyncTime(MOCK_USER_STATE.activeUserId, clock.instant()) + } just runs val stateFlow = MutableStateFlow>>( DataState.Loading, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModelTest.kt index 37b3a2d31..495e3c54d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/other/OtherViewModelTest.kt @@ -2,9 +2,11 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.other import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import com.x8bit.bitwarden.ui.platform.base.util.asText import io.mockk.every import io.mockk.just import io.mockk.mockk @@ -14,11 +16,22 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test +import java.time.Clock +import java.time.Instant +import java.time.ZoneOffset class OtherViewModelTest : BaseViewModelTest() { + private val clock: Clock = Clock.fixed( + Instant.parse("2023-10-27T12:00:00Z"), + ZoneOffset.UTC, + ) private val mutablePullToRefreshStateFlow = MutableStateFlow(false) + private val instant: Instant = Instant.parse("2023-10-26T12:00:00Z") + private val mutableVaultLastSyncStateFlow = MutableStateFlow(instant) private val settingsRepository = mockk { every { getPullToRefreshEnabledFlow() } returns mutablePullToRefreshStateFlow + every { vaultLastSyncStateFlow } returns mutableVaultLastSyncStateFlow + every { vaultLastSync } returns instant } private val vaultRepository = mockk() @@ -103,13 +116,38 @@ class OtherViewModelTest : BaseViewModelTest() { } } + @Test + fun `on VaultLastSyncReceive should sync repo`() = runTest { + val newSyncTime = Instant.parse("2023-10-27T12:00:00Z") + val viewModel = createViewModel() + viewModel.stateFlow.test { + assertEquals(DEFAULT_STATE, awaitItem()) + mutableVaultLastSyncStateFlow.tryEmit(newSyncTime) + assertEquals( + DEFAULT_STATE.copy( + lastSyncTime = "10/27/2023 12:00 PM", + dialogState = null, + ), + awaitItem(), + ) + } + } + @Test fun `on SyncNowButtonClick should sync repo`() = runTest { every { vaultRepository.sync() } just runs val viewModel = createViewModel() - viewModel.eventFlow.test { + viewModel.stateFlow.test { + assertEquals(DEFAULT_STATE, awaitItem()) viewModel.trySendAction(OtherAction.SyncNowButtonClick) - expectNoEvents() + assertEquals( + DEFAULT_STATE.copy( + dialogState = OtherState.DialogState.Loading( + message = R.string.syncing.asText(), + ), + ), + awaitItem(), + ) } verify { vaultRepository.sync() } } @@ -117,6 +155,7 @@ class OtherViewModelTest : BaseViewModelTest() { private fun createViewModel( state: OtherState? = null, ) = OtherViewModel( + clock = clock, settingsRepo = settingsRepository, vaultRepo = vaultRepository, savedStateHandle = SavedStateHandle().apply { @@ -129,7 +168,7 @@ class OtherViewModelTest : BaseViewModelTest() { allowScreenCapture = false, allowSyncOnRefresh = false, clearClipboardFrequency = OtherState.ClearClipboardFrequency.DEFAULT, - lastSyncTime = "5/14/2023 4:52 PM", + lastSyncTime = "10/26/2023 12:00 PM", dialogState = null, ) }