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

This commit is contained in:
Patrick Honkonen 2024-11-14 14:18:25 -05:00 committed by GitHub
parent 0a888a72c8
commit 3c4ac8b01a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 126 additions and 6 deletions

View file

@ -74,6 +74,11 @@ interface SettingsDiskSource {
*/ */
var lastDatabaseSchemeChangeInstant: Instant? var lastDatabaseSchemeChangeInstant: Instant?
/**
* Emits updates that track [lastDatabaseSchemeChangeInstant].
*/
val lastDatabaseSchemeChangeInstantFlow: Flow<Instant?>
/** /**
* Clears all the settings data for the given user. * Clears all the settings data for the given user.
*/ */

View file

@ -75,6 +75,8 @@ class SettingsDiskSourceImpl(
private val mutableHasUserLoggedInOrCreatedAccountFlow = bufferedMutableSharedFlow<Boolean?>() private val mutableHasUserLoggedInOrCreatedAccountFlow = bufferedMutableSharedFlow<Boolean?>()
private val mutableLastDatabaseSchemeChangeInstantFlow = bufferedMutableSharedFlow<Instant?>()
private val mutableScreenCaptureAllowedFlowMap = private val mutableScreenCaptureAllowedFlowMap =
mutableMapOf<String, MutableSharedFlow<Boolean?>>() mutableMapOf<String, MutableSharedFlow<Boolean?>>()
@ -158,7 +160,14 @@ class SettingsDiskSourceImpl(
override var lastDatabaseSchemeChangeInstant: Instant? override var lastDatabaseSchemeChangeInstant: Instant?
get() = getLong(LAST_SCHEME_CHANGE_INSTANT)?.let { Instant.ofEpochMilli(it) } 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) { override fun clearData(userId: String) {
storeVaultTimeoutInMinutes(userId = userId, vaultTimeoutInMinutes = null) storeVaultTimeoutInMinutes(userId = userId, vaultTimeoutInMinutes = null)

View file

@ -1,5 +1,6 @@
package com.x8bit.bitwarden.data.platform.manager package com.x8bit.bitwarden.data.platform.manager
import kotlinx.coroutines.flow.Flow
import java.time.Instant 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. * that a scheme change to any database will update this value and trigger a sync.
*/ */
var lastDatabaseSchemeChangeInstant: Instant? var lastDatabaseSchemeChangeInstant: Instant?
/**
* A flow of the last database schema change instant.
*/
val lastDatabaseSchemeChangeInstantFlow: Flow<Instant?>
} }

View file

@ -1,6 +1,10 @@
package com.x8bit.bitwarden.data.platform.manager package com.x8bit.bitwarden.data.platform.manager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource 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 import java.time.Instant
/** /**
@ -8,10 +12,23 @@ import java.time.Instant
*/ */
class DatabaseSchemeManagerImpl( class DatabaseSchemeManagerImpl(
val settingsDiskSource: SettingsDiskSource, val settingsDiskSource: SettingsDiskSource,
val dispatcherManager: DispatcherManager,
) : DatabaseSchemeManager { ) : DatabaseSchemeManager {
private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined)
override var lastDatabaseSchemeChangeInstant: Instant? override var lastDatabaseSchemeChangeInstant: Instant?
get() = settingsDiskSource.lastDatabaseSchemeChangeInstant get() = settingsDiskSource.lastDatabaseSchemeChangeInstant
set(value) { set(value) {
settingsDiskSource.lastDatabaseSchemeChangeInstant = value settingsDiskSource.lastDatabaseSchemeChangeInstant = value
} }
override val lastDatabaseSchemeChangeInstantFlow =
settingsDiskSource
.lastDatabaseSchemeChangeInstantFlow
.stateIn(
scope = unconfinedScope,
started = SharingStarted.Eagerly,
initialValue = settingsDiskSource.lastDatabaseSchemeChangeInstant,
)
} }

View file

@ -308,7 +308,9 @@ object PlatformManagerModule {
@Singleton @Singleton
fun provideDatabaseSchemeManager( fun provideDatabaseSchemeManager(
settingsDiskSource: SettingsDiskSource, settingsDiskSource: SettingsDiskSource,
dispatcherManager: DispatcherManager,
): DatabaseSchemeManager = DatabaseSchemeManagerImpl( ): DatabaseSchemeManager = DatabaseSchemeManagerImpl(
settingsDiskSource = settingsDiskSource, settingsDiskSource = settingsDiskSource,
dispatcherManager = dispatcherManager,
) )
} }

View file

@ -99,6 +99,7 @@ import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapLatest
@ -323,6 +324,12 @@ class VaultRepositoryImpl(
.syncFolderUpsertFlow .syncFolderUpsertFlow
.onEach(::syncFolderIfNecessary) .onEach(::syncFolderIfNecessary)
.launchIn(ioScope) .launchIn(ioScope)
databaseSchemeManager
.lastDatabaseSchemeChangeInstantFlow
.filterNotNull()
.onEach { sync() }
.launchIn(ioScope)
} }
private fun clearUnlockedData() { private fun clearUnlockedData() {

View file

@ -42,6 +42,9 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val mutableScreenCaptureAllowedFlowMap = private val mutableScreenCaptureAllowedFlowMap =
mutableMapOf<String, MutableSharedFlow<Boolean?>>() mutableMapOf<String, MutableSharedFlow<Boolean?>>()
private val mutableLastDatabaseSchemeChangeInstant =
bufferedMutableSharedFlow<Instant?>()
private var storedAppTheme: AppTheme = AppTheme.DEFAULT private var storedAppTheme: AppTheme = AppTheme.DEFAULT
private val storedLastSyncTime = mutableMapOf<String, Instant?>() private val storedLastSyncTime = mutableMapOf<String, Instant?>()
private val storedVaultTimeoutActions = mutableMapOf<String, VaultTimeoutAction?>() private val storedVaultTimeoutActions = mutableMapOf<String, VaultTimeoutAction?>()
@ -141,6 +144,11 @@ class FakeSettingsDiskSource : SettingsDiskSource {
get() = storedLastDatabaseSchemeChangeInstant get() = storedLastDatabaseSchemeChangeInstant
set(value) { storedLastDatabaseSchemeChangeInstant = value } set(value) { storedLastDatabaseSchemeChangeInstant = value }
override val lastDatabaseSchemeChangeInstantFlow: Flow<Instant?>
get() = mutableLastDatabaseSchemeChangeInstant.onSubscription {
emit(lastDatabaseSchemeChangeInstant)
}
override fun getAccountBiometricIntegrityValidity( override fun getAccountBiometricIntegrityValidity(
userId: String, userId: String,
systemBioIntegrityState: String, systemBioIntegrityState: String,

View file

@ -1,24 +1,37 @@
package com.x8bit.bitwarden.data.platform.manager 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 com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import io.mockk.every import io.mockk.every
import io.mockk.just
import io.mockk.mockk import io.mockk.mockk
import io.mockk.runs
import io.mockk.verify import io.mockk.verify
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import org.junit.Test import org.junit.Test
import org.junit.jupiter.api.Assertions.assertEquals
import java.time.Clock import java.time.Clock
import java.time.Instant import java.time.Instant
import java.time.ZoneOffset import java.time.ZoneOffset
class DatabaseSchemeManagerTest { class DatabaseSchemeManagerTest {
private val mutableLastDatabaseSchemeChangeInstantFlow = MutableStateFlow<Instant?>(null)
private val mockSettingsDiskSource: SettingsDiskSource = mockk { private val mockSettingsDiskSource: SettingsDiskSource = mockk {
every { lastDatabaseSchemeChangeInstant } returns null every {
every { lastDatabaseSchemeChangeInstant = any() } just runs lastDatabaseSchemeChangeInstant
} returns mutableLastDatabaseSchemeChangeInstantFlow.value
every { lastDatabaseSchemeChangeInstant = any() } answers {
mutableLastDatabaseSchemeChangeInstantFlow.value = firstArg()
} }
every {
lastDatabaseSchemeChangeInstantFlow
} returns mutableLastDatabaseSchemeChangeInstantFlow
}
private val dispatcherManager = FakeDispatcherManager()
private val databaseSchemeManager = DatabaseSchemeManagerImpl( private val databaseSchemeManager = DatabaseSchemeManagerImpl(
settingsDiskSource = mockSettingsDiskSource, settingsDiskSource = mockSettingsDiskSource,
dispatcherManager = dispatcherManager,
) )
@Suppress("MaxLineLength") @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 @Test
fun `getLastDatabaseSchemeChangeInstant retrieves stored value from settingsDiskSource`() { fun `getLastDatabaseSchemeChangeInstant retrieves stored value from settingsDiskSource`() {
databaseSchemeManager.lastDatabaseSchemeChangeInstant databaseSchemeManager.lastDatabaseSchemeChangeInstant

View file

@ -193,8 +193,14 @@ class VaultRepositoryTest {
mutableUnlockedUserIdsStateFlow.first { userId in it } mutableUnlockedUserIdsStateFlow.first { userId in it }
} }
} }
private val mutableLastDatabaseSchemeChangeInstantFlow = MutableStateFlow<Instant?>(null)
private val databaseSchemeManager: DatabaseSchemeManager = mockk { private val databaseSchemeManager: DatabaseSchemeManager = mockk {
every { lastDatabaseSchemeChangeInstant } returns null every {
lastDatabaseSchemeChangeInstant
} returns mutableLastDatabaseSchemeChangeInstantFlow.value
every {
lastDatabaseSchemeChangeInstantFlow
} returns mutableLastDatabaseSchemeChangeInstantFlow
} }
private val mutableFullSyncFlow = bufferedMutableSharedFlow<Unit>() 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") @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`() =