From 3c4ac8b01a36f8088b945156d3ff7685723df1dc Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:18:25 -0500 Subject: [PATCH] [PM-14596] Sync on database scheme change (#4304) --- .../datasource/disk/SettingsDiskSource.kt | 5 +++ .../datasource/disk/SettingsDiskSourceImpl.kt | 11 +++++- .../platform/manager/DatabaseSchemeManager.kt | 6 +++ .../manager/DatabaseSchemeManagerImpl.kt | 17 +++++++++ .../manager/di/PlatformManagerModule.kt | 2 + .../vault/repository/VaultRepositoryImpl.kt | 7 ++++ .../disk/util/FakeSettingsDiskSource.kt | 8 ++++ .../manager/DatabaseSchemeManagerTest.kt | 38 +++++++++++++++++-- .../vault/repository/VaultRepositoryTest.kt | 38 ++++++++++++++++++- 9 files changed, 126 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt index 9ed7f86e6..5f602f0eb 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt @@ -74,6 +74,11 @@ interface SettingsDiskSource { */ var lastDatabaseSchemeChangeInstant: Instant? + /** + * Emits updates that track [lastDatabaseSchemeChangeInstant]. + */ + val lastDatabaseSchemeChangeInstantFlow: Flow + /** * Clears all the settings data for the given user. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt index b65e022fb..3144ac84e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt @@ -75,6 +75,8 @@ class SettingsDiskSourceImpl( private val mutableHasUserLoggedInOrCreatedAccountFlow = bufferedMutableSharedFlow() + private val mutableLastDatabaseSchemeChangeInstantFlow = bufferedMutableSharedFlow() + private val mutableScreenCaptureAllowedFlowMap = mutableMapOf>() @@ -158,7 +160,14 @@ class SettingsDiskSourceImpl( override var lastDatabaseSchemeChangeInstant: Instant? get() = getLong(LAST_SCHEME_CHANGE_INSTANT)?.let { Instant.ofEpochMilli(it) } - set(value) = putLong(LAST_SCHEME_CHANGE_INSTANT, value?.toEpochMilli()) + set(value) { + putLong(LAST_SCHEME_CHANGE_INSTANT, value?.toEpochMilli()) + mutableLastDatabaseSchemeChangeInstantFlow.tryEmit(value) + } + + override val lastDatabaseSchemeChangeInstantFlow: Flow + get() = mutableLastDatabaseSchemeChangeInstantFlow + .onSubscription { emit(lastDatabaseSchemeChangeInstant) } override fun clearData(userId: String) { storeVaultTimeoutInMinutes(userId = userId, vaultTimeoutInMinutes = null) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManager.kt index a88cde55c..e936d7ec2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManager.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.platform.manager +import kotlinx.coroutines.flow.Flow import java.time.Instant /** @@ -14,4 +15,9 @@ interface DatabaseSchemeManager { * that a scheme change to any database will update this value and trigger a sync. */ var lastDatabaseSchemeChangeInstant: Instant? + + /** + * A flow of the last database schema change instant. + */ + val lastDatabaseSchemeChangeInstantFlow: Flow } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerImpl.kt index 2c0fb256e..5b5d7430e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerImpl.kt @@ -1,6 +1,10 @@ package com.x8bit.bitwarden.data.platform.manager import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource +import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.stateIn import java.time.Instant /** @@ -8,10 +12,23 @@ import java.time.Instant */ class DatabaseSchemeManagerImpl( val settingsDiskSource: SettingsDiskSource, + val dispatcherManager: DispatcherManager, ) : DatabaseSchemeManager { + + private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) + override var lastDatabaseSchemeChangeInstant: Instant? get() = settingsDiskSource.lastDatabaseSchemeChangeInstant set(value) { settingsDiskSource.lastDatabaseSchemeChangeInstant = value } + + override val lastDatabaseSchemeChangeInstantFlow = + settingsDiskSource + .lastDatabaseSchemeChangeInstantFlow + .stateIn( + scope = unconfinedScope, + started = SharingStarted.Eagerly, + initialValue = settingsDiskSource.lastDatabaseSchemeChangeInstant, + ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index ea3ffc654..f09e31143 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -308,7 +308,9 @@ object PlatformManagerModule { @Singleton fun provideDatabaseSchemeManager( settingsDiskSource: SettingsDiskSource, + dispatcherManager: DispatcherManager, ): DatabaseSchemeManager = DatabaseSchemeManagerImpl( settingsDiskSource = settingsDiskSource, + dispatcherManager = dispatcherManager, ) } 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 12ac59c11..7c60bbcec 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 @@ -99,6 +99,7 @@ import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.flatMapLatest @@ -323,6 +324,12 @@ class VaultRepositoryImpl( .syncFolderUpsertFlow .onEach(::syncFolderIfNecessary) .launchIn(ioScope) + + databaseSchemeManager + .lastDatabaseSchemeChangeInstantFlow + .filterNotNull() + .onEach { sync() } + .launchIn(ioScope) } private fun clearUnlockedData() { diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt index 286e4963d..a7f44baf1 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt @@ -42,6 +42,9 @@ class FakeSettingsDiskSource : SettingsDiskSource { private val mutableScreenCaptureAllowedFlowMap = mutableMapOf>() + private val mutableLastDatabaseSchemeChangeInstant = + bufferedMutableSharedFlow() + private var storedAppTheme: AppTheme = AppTheme.DEFAULT private val storedLastSyncTime = mutableMapOf() private val storedVaultTimeoutActions = mutableMapOf() @@ -141,6 +144,11 @@ class FakeSettingsDiskSource : SettingsDiskSource { get() = storedLastDatabaseSchemeChangeInstant set(value) { storedLastDatabaseSchemeChangeInstant = value } + override val lastDatabaseSchemeChangeInstantFlow: Flow + get() = mutableLastDatabaseSchemeChangeInstant.onSubscription { + emit(lastDatabaseSchemeChangeInstant) + } + override fun getAccountBiometricIntegrityValidity( userId: String, systemBioIntegrityState: String, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerTest.kt index 3fb01a2f4..c0a469e22 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/DatabaseSchemeManagerTest.kt @@ -1,24 +1,37 @@ package com.x8bit.bitwarden.data.platform.manager +import app.cash.turbine.test +import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import io.mockk.every -import io.mockk.just import io.mockk.mockk -import io.mockk.runs import io.mockk.verify +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest import org.junit.Test +import org.junit.jupiter.api.Assertions.assertEquals import java.time.Clock import java.time.Instant import java.time.ZoneOffset class DatabaseSchemeManagerTest { + private val mutableLastDatabaseSchemeChangeInstantFlow = MutableStateFlow(null) private val mockSettingsDiskSource: SettingsDiskSource = mockk { - every { lastDatabaseSchemeChangeInstant } returns null - every { lastDatabaseSchemeChangeInstant = any() } just runs + every { + lastDatabaseSchemeChangeInstant + } returns mutableLastDatabaseSchemeChangeInstantFlow.value + every { lastDatabaseSchemeChangeInstant = any() } answers { + mutableLastDatabaseSchemeChangeInstantFlow.value = firstArg() + } + every { + lastDatabaseSchemeChangeInstantFlow + } returns mutableLastDatabaseSchemeChangeInstantFlow } + private val dispatcherManager = FakeDispatcherManager() private val databaseSchemeManager = DatabaseSchemeManagerImpl( settingsDiskSource = mockSettingsDiskSource, + dispatcherManager = dispatcherManager, ) @Suppress("MaxLineLength") @@ -30,6 +43,23 @@ class DatabaseSchemeManagerTest { } } + @Test + fun `setLastDatabaseSchemeChangeInstant does emit value`() = runTest { + databaseSchemeManager.lastDatabaseSchemeChangeInstantFlow.test { + // Assert the value is initialized to null + assertEquals( + null, + awaitItem(), + ) + // Assert the new value is emitted + databaseSchemeManager.lastDatabaseSchemeChangeInstant = FIXED_CLOCK.instant() + assertEquals( + FIXED_CLOCK.instant(), + awaitItem(), + ) + } + } + @Test fun `getLastDatabaseSchemeChangeInstant retrieves stored value from settingsDiskSource`() { databaseSchemeManager.lastDatabaseSchemeChangeInstant 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 8c9213a92..1206f4369 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 @@ -193,8 +193,14 @@ class VaultRepositoryTest { mutableUnlockedUserIdsStateFlow.first { userId in it } } } + private val mutableLastDatabaseSchemeChangeInstantFlow = MutableStateFlow(null) private val databaseSchemeManager: DatabaseSchemeManager = mockk { - every { lastDatabaseSchemeChangeInstant } returns null + every { + lastDatabaseSchemeChangeInstant + } returns mutableLastDatabaseSchemeChangeInstantFlow.value + every { + lastDatabaseSchemeChangeInstantFlow + } returns mutableLastDatabaseSchemeChangeInstantFlow } private val mutableFullSyncFlow = bufferedMutableSharedFlow() @@ -780,6 +786,36 @@ class VaultRepositoryTest { } } + @Test + fun `lastDatabaseSchemeChangeInstantFlow should trigger sync when new value is not null`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + every { + databaseSchemeManager.lastDatabaseSchemeChangeInstant + } returns mutableLastDatabaseSchemeChangeInstantFlow.value + coEvery { syncService.sync() } just awaits + + mutableLastDatabaseSchemeChangeInstantFlow.value = clock.instant() + + coVerify(exactly = 1) { syncService.sync() } + } + + @Test + fun `lastDatabaseSchemeChangeInstantFlow should not sync when new value is null`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + + every { + databaseSchemeManager.lastDatabaseSchemeChangeInstant + } returns mutableLastDatabaseSchemeChangeInstantFlow.value + + coEvery { syncService.sync() } just awaits + + mutableLastDatabaseSchemeChangeInstantFlow.value = null + + coVerify(exactly = 0) { syncService.sync() } + } + @Suppress("MaxLineLength") @Test fun `sync with syncService Success should unlock the vault for orgs if necessary and update AuthDiskSource and VaultDiskSource`() =