[PM-14596] Sync on database scheme change (#4257)

bitwarden · Nov 14, 2024 · f86c100 · f86c100
1 parent 40f33df
commit f86c100
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 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
@@ -74,6 +74,11 @@ interface SettingsDiskSource {
      */
     var lastDatabaseSchemeChangeInstant: Instant?
 
+    /**
+     * Emits updates that track [lastDatabaseSchemeChangeInstant].
+     */
+    val lastDatabaseSchemeChangeInstantFlow: Flow<Instant?>
+
     /**
      * Clears all the settings data for the given user.
      */

diff --git a/...src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt b/...src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt
@@ -75,6 +75,8 @@ class SettingsDiskSourceImpl(
 
     private val mutableHasUserLoggedInOrCreatedAccountFlow = bufferedMutableSharedFlow<Boolean?>()
 
+    private val mutableLastDatabaseSchemeChangeInstantFlow = bufferedMutableSharedFlow<Instant?>()
+
     private val mutableScreenCaptureAllowedFlowMap =
         mutableMapOf<String, MutableSharedFlow<Boolean?>>()
 
@@ -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<Instant?>
+        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
@@ -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<Instant?>
 }
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
@@ -1,17 +1,34 @@
 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
 
 /**
  * Primary implementation of [DatabaseSchemeManager].
  */
 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
@@ -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
@@ -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/...est/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/...est/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt
@@ -42,6 +42,9 @@ class FakeSettingsDiskSource : SettingsDiskSource {
     private val mutableScreenCaptureAllowedFlowMap =
         mutableMapOf<String, MutableSharedFlow<Boolean?>>()
 
+    private val mutableLastDatabaseSchemeChangeInstant =
+        bufferedMutableSharedFlow<Instant?>()
+
     private var storedAppTheme: AppTheme = AppTheme.DEFAULT
     private val storedLastSyncTime = mutableMapOf<String, Instant?>()
     private val storedVaultTimeoutActions = mutableMapOf<String, VaultTimeoutAction?>()
@@ -141,6 +144,11 @@ class FakeSettingsDiskSource : SettingsDiskSource {
         get() = storedLastDatabaseSchemeChangeInstant
         set(value) { storedLastDatabaseSchemeChangeInstant = value }
 
+    override val lastDatabaseSchemeChangeInstantFlow: Flow<Instant?>
+        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
@@ -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<Instant?>(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
@@ -193,8 +193,14 @@ class VaultRepositoryTest {
             mutableUnlockedUserIdsStateFlow.first { userId in it }
         }
     }
+    private val mutableLastDatabaseSchemeChangeInstantFlow = MutableStateFlow<Instant?>(null)
     private val databaseSchemeManager: DatabaseSchemeManager = mockk {
-        every { lastDatabaseSchemeChangeInstant } returns null
+        every {
+            lastDatabaseSchemeChangeInstant
+        } returns mutableLastDatabaseSchemeChangeInstantFlow.value
+        every {
+            lastDatabaseSchemeChangeInstantFlow
+        } returns mutableLastDatabaseSchemeChangeInstantFlow
     }
 
     private val mutableFullSyncFlow = bufferedMutableSharedFlow<Unit>()
@@ -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`() =