[PM-13900] Track last database scheme change (#4124)

This commit is contained in:
Patrick Honkonen 2024-10-24 09:15:54 -04:00 committed by GitHub
parent 6217532237
commit 2d9451cc34
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 320 additions and 7 deletions

View file

@ -68,6 +68,12 @@ interface SettingsDiskSource {
*/
val hasUserLoggedInOrCreatedAccountFlow: Flow<Boolean?>
/**
* The instant when the last database scheme change was applied. `null` if no scheme changes
* have been applied yet.
*/
var lastDatabaseSchemeChangeInstant: Instant?
/**
* Clears all the settings data for the given user.
*/

View file

@ -35,6 +35,7 @@ private const val INITIAL_AUTOFILL_DIALOG_SHOWN = "addSitePromptShown"
private const val HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY = "hasUserLoggedInOrCreatedAccount"
private const val SHOW_AUTOFILL_SETTING_BADGE = "showAutofillSettingBadge"
private const val SHOW_UNLOCK_SETTING_BADGE = "showUnlockSettingBadge"
private const val LAST_SCHEME_CHANGE_INSTANT = "lastDatabaseSchemeChangeInstant"
/**
* Primary implementation of [SettingsDiskSource].
@ -151,6 +152,10 @@ class SettingsDiskSourceImpl(
get() = mutableHasUserLoggedInOrCreatedAccountFlow
.onSubscription { emit(getBoolean(HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY)) }
override var lastDatabaseSchemeChangeInstant: Instant?
get() = getLong(LAST_SCHEME_CHANGE_INSTANT)?.let { Instant.ofEpochMilli(it) }
set(value) = putLong(LAST_SCHEME_CHANGE_INSTANT, value?.toEpochMilli())
override fun clearData(userId: String) {
storeVaultTimeoutInMinutes(userId = userId, vaultTimeoutInMinutes = null)
storeVaultTimeoutAction(userId = userId, vaultTimeoutAction = null)

View file

@ -26,8 +26,10 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacySecureStor
import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacySecureStorageImpl
import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacySecureStorageMigrator
import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacySecureStorageMigratorImpl
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
import com.x8bit.bitwarden.data.vault.datasource.disk.callback.DatabaseSchemeCallback
import com.x8bit.bitwarden.data.vault.datasource.disk.convertor.ZonedDateTimeTypeConverter
import dagger.Module
import dagger.Provides
@ -35,6 +37,7 @@ import dagger.hilt.InstallIn
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import kotlinx.serialization.json.Json
import java.time.Clock
import javax.inject.Singleton
/**
@ -68,7 +71,11 @@ object PlatformDiskModule {
@Provides
@Singleton
fun provideEventDatabase(app: Application): PlatformDatabase =
fun provideEventDatabase(
app: Application,
databaseSchemeManager: DatabaseSchemeManager,
clock: Clock,
): PlatformDatabase =
Room
.databaseBuilder(
context = app,
@ -77,6 +84,12 @@ object PlatformDiskModule {
)
.fallbackToDestructiveMigration()
.addTypeConverter(ZonedDateTimeTypeConverter())
.addCallback(
DatabaseSchemeCallback(
databaseSchemeManager = databaseSchemeManager,
clock = clock,
),
)
.build()
@Provides

View file

@ -0,0 +1,17 @@
package com.x8bit.bitwarden.data.platform.manager
import java.time.Instant
/**
* Manager for tracking changes to database scheme(s).
*/
interface DatabaseSchemeManager {
/**
* The instant of the last database schema change performed on the database, if any.
*
* There is only a single scheme change instant tracked for all database schemes. It is expected
* that a scheme change to any database will update this value and trigger a sync.
*/
var lastDatabaseSchemeChangeInstant: Instant?
}

View file

@ -0,0 +1,17 @@
package com.x8bit.bitwarden.data.platform.manager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import java.time.Instant
/**
* Primary implementation of [DatabaseSchemeManager].
*/
class DatabaseSchemeManagerImpl(
val settingsDiskSource: SettingsDiskSource,
) : DatabaseSchemeManager {
override var lastDatabaseSchemeChangeInstant: Instant?
get() = settingsDiskSource.lastDatabaseSchemeChangeInstant
set(value) {
settingsDiskSource.lastDatabaseSchemeChangeInstant = value
}
}

View file

@ -20,6 +20,8 @@ import com.x8bit.bitwarden.data.platform.manager.AssetManager
import com.x8bit.bitwarden.data.platform.manager.AssetManagerImpl
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManagerImpl
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManagerImpl
import com.x8bit.bitwarden.data.platform.manager.DebugMenuFeatureFlagManagerImpl
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManagerImpl
@ -297,4 +299,12 @@ object PlatformManagerModule {
dispatcherManager = dispatcherManager,
featureFlagManager = featureFlagManager,
)
@Provides
@Singleton
fun provideDatabaseSchemeManager(
settingsDiskSource: SettingsDiskSource,
): DatabaseSchemeManager = DatabaseSchemeManagerImpl(
settingsDiskSource = settingsDiskSource,
)
}

View file

@ -4,17 +4,20 @@ import android.app.Application
import android.content.SharedPreferences
import androidx.room.Room
import com.x8bit.bitwarden.data.platform.datasource.di.UnencryptedPreferences
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSource
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSourceImpl
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.PasswordHistoryDiskSource
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.PasswordHistoryDiskSourceImpl
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.dao.PasswordHistoryDao
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.database.PasswordHistoryDatabase
import com.x8bit.bitwarden.data.vault.datasource.disk.callback.DatabaseSchemeCallback
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import kotlinx.serialization.json.Json
import java.time.Clock
import javax.inject.Singleton
/**
@ -45,13 +48,23 @@ object GeneratorDiskModule {
@Provides
@Singleton
fun providePasswordHistoryDatabase(app: Application): PasswordHistoryDatabase {
fun providePasswordHistoryDatabase(
app: Application,
databaseSchemeManager: DatabaseSchemeManager,
clock: Clock,
): PasswordHistoryDatabase {
return Room
.databaseBuilder(
context = app,
klass = PasswordHistoryDatabase::class.java,
name = "passcode_history_database",
)
.addCallback(
DatabaseSchemeCallback(
databaseSchemeManager = databaseSchemeManager,
clock = clock,
),
)
.build()
}

View file

@ -0,0 +1,18 @@
package com.x8bit.bitwarden.data.vault.datasource.disk.callback
import androidx.room.RoomDatabase
import androidx.sqlite.db.SupportSQLiteDatabase
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import java.time.Clock
/**
* A [RoomDatabase.Callback] for tracking database scheme changes.
*/
class DatabaseSchemeCallback(
private val databaseSchemeManager: DatabaseSchemeManager,
private val clock: Clock,
) : RoomDatabase.Callback() {
override fun onDestructiveMigration(db: SupportSQLiteDatabase) {
databaseSchemeManager.lastDatabaseSchemeChangeInstant = clock.instant()
}
}

View file

@ -2,9 +2,11 @@ package com.x8bit.bitwarden.data.vault.datasource.disk.di
import android.app.Application
import androidx.room.Room
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
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.disk.VaultDiskSourceImpl
import com.x8bit.bitwarden.data.vault.datasource.disk.callback.DatabaseSchemeCallback
import com.x8bit.bitwarden.data.vault.datasource.disk.convertor.ZonedDateTimeTypeConverter
import com.x8bit.bitwarden.data.vault.datasource.disk.dao.CiphersDao
import com.x8bit.bitwarden.data.vault.datasource.disk.dao.CollectionsDao
@ -17,6 +19,7 @@ import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import kotlinx.serialization.json.Json
import java.time.Clock
import javax.inject.Singleton
/**
@ -28,7 +31,11 @@ class VaultDiskModule {
@Provides
@Singleton
fun provideVaultDatabase(app: Application): VaultDatabase =
fun provideVaultDatabase(
app: Application,
databaseSchemeManager: DatabaseSchemeManager,
clock: Clock,
): VaultDatabase =
Room
.databaseBuilder(
context = app,
@ -36,6 +43,7 @@ class VaultDiskModule {
name = "vault_database",
)
.fallbackToDestructiveMigration()
.addCallback(DatabaseSchemeCallback(databaseSchemeManager, clock))
.addTypeConverter(ZonedDateTimeTypeConverter())
.build()

View file

@ -21,6 +21,7 @@ import com.x8bit.bitwarden.data.auth.repository.util.toUpdatedUserStateJson
import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow
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.DatabaseSchemeManager
import com.x8bit.bitwarden.data.platform.manager.PushManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.manager.model.SyncCipherDeleteData
@ -138,6 +139,7 @@ class VaultRepositoryImpl(
private val vaultLockManager: VaultLockManager,
private val totpCodeManager: TotpCodeManager,
private val userLogoutManager: UserLogoutManager,
private val databaseSchemeManager: DatabaseSchemeManager,
pushManager: PushManager,
private val clock: Clock,
dispatcherManager: DispatcherManager,
@ -331,10 +333,13 @@ class VaultRepositoryImpl(
val userId = activeUserId ?: return
val currentInstant = clock.instant()
val lastSyncInstant = settingsDiskSource.getLastSyncTime(userId = userId)
val lastDatabaseSchemeChangeInstant = databaseSchemeManager.lastDatabaseSchemeChangeInstant
// Sync if we have never done so or the last time was at last 30 minutes ago
// Sync if we have never done so, the last time was at last 30 minutes ago, or the database
// scheme changed since the last sync.
if (lastSyncInstant == null ||
currentInstant.isAfter(lastSyncInstant.plus(30, ChronoUnit.MINUTES))
currentInstant.isAfter(lastSyncInstant.plus(30, ChronoUnit.MINUTES)) ||
lastDatabaseSchemeChangeInstant?.isAfter(lastSyncInstant) == true
) {
sync()
}
@ -1312,12 +1317,20 @@ class VaultRepositoryImpl(
?.toEpochMilli()
?: 0
val lastDatabaseSchemeChangeInstant = databaseSchemeManager
.lastDatabaseSchemeChangeInstant
?.toEpochMilli()
?: 0
syncService
.getAccountRevisionDateMillis()
.fold(
onSuccess = { serverRevisionDate ->
if (serverRevisionDate < lastSyncInstant) {
// We can skip the actual sync call if there is no new data
if (serverRevisionDate < lastSyncInstant &&
lastDatabaseSchemeChangeInstant < lastSyncInstant
) {
// We can skip the actual sync call if there is no new data or database
// scheme changes since the last sync.
vaultDiskSource.resyncVaultData(userId)
settingsDiskSource.storeLastSyncTime(
userId = userId,

View file

@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.vault.repository.di
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.auth.manager.UserLogoutManager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import com.x8bit.bitwarden.data.platform.manager.PushManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
@ -49,6 +50,7 @@ object VaultRepositoryModule {
totpCodeManager: TotpCodeManager,
pushManager: PushManager,
userLogoutManager: UserLogoutManager,
databaseSchemeManager: DatabaseSchemeManager,
clock: Clock,
): VaultRepository = VaultRepositoryImpl(
syncService = syncService,
@ -66,6 +68,7 @@ object VaultRepositoryModule {
totpCodeManager = totpCodeManager,
pushManager = pushManager,
userLogoutManager = userLogoutManager,
databaseSchemeManager = databaseSchemeManager,
clock = clock,
)
}

View file

@ -1116,4 +1116,53 @@ class SettingsDiskSourceTest {
assertFalse(awaitItem() ?: true)
}
}
@Test
fun `lastDatabaseSchemeChangeInstant should pull from SharedPreferences`() {
val schemeChangeKey = "bwPreferencesStorage:lastDatabaseSchemeChangeInstant"
val expected: Long = Instant.now().toEpochMilli()
fakeSharedPreferences
.edit {
remove(schemeChangeKey)
}
assertEquals(0, fakeSharedPreferences.getLong(schemeChangeKey, 0))
assertNull(settingsDiskSource.lastDatabaseSchemeChangeInstant)
// Updating the shared preferences should update disk source.
fakeSharedPreferences
.edit {
putLong(
schemeChangeKey,
expected,
)
}
val actual = settingsDiskSource.lastDatabaseSchemeChangeInstant
assertEquals(
expected,
actual?.toEpochMilli(),
)
}
@Test
fun `setting lastDatabaseSchemeChangeInstant should update SharedPreferences`() {
val schemeChangeKey = "bwPreferencesStorage:lastDatabaseSchemeChangeInstant"
val schemeChangeInstant = Instant.now()
// Setting to null should update disk source
settingsDiskSource.lastDatabaseSchemeChangeInstant = null
assertEquals(0, fakeSharedPreferences.getLong(schemeChangeKey, 0))
assertNull(settingsDiskSource.lastDatabaseSchemeChangeInstant)
// Setting to value should update disk source
settingsDiskSource.lastDatabaseSchemeChangeInstant = schemeChangeInstant
val actual = fakeSharedPreferences.getLong(
schemeChangeKey,
0,
)
assertEquals(
schemeChangeInstant.toEpochMilli(),
actual,
)
}
}

View file

@ -63,6 +63,7 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val userSignIns = mutableMapOf<String, Boolean>()
private val userShowAutoFillBadge = mutableMapOf<String, Boolean?>()
private val userShowUnlockBadge = mutableMapOf<String, Boolean?>()
private var storedLastDatabaseSchemeChangeInstant: Instant? = null
private val mutableShowAutoFillSettingBadgeFlowMap =
mutableMapOf<String, MutableSharedFlow<Boolean?>>()
@ -132,6 +133,10 @@ class FakeSettingsDiskSource : SettingsDiskSource {
emit(hasUserLoggedInOrCreatedAccount)
}
override var lastDatabaseSchemeChangeInstant: Instant?
get() = storedLastDatabaseSchemeChangeInstant
set(value) { storedLastDatabaseSchemeChangeInstant = value }
override fun getAccountBiometricIntegrityValidity(
userId: String,
systemBioIntegrityState: String,

View file

@ -0,0 +1,45 @@
package com.x8bit.bitwarden.data.platform.manager
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 org.junit.Test
import java.time.Clock
import java.time.Instant
import java.time.ZoneOffset
class DatabaseSchemeManagerTest {
private val mockSettingsDiskSource: SettingsDiskSource = mockk {
every { lastDatabaseSchemeChangeInstant } returns null
every { lastDatabaseSchemeChangeInstant = any() } just runs
}
private val databaseSchemeManager = DatabaseSchemeManagerImpl(
settingsDiskSource = mockSettingsDiskSource,
)
@Suppress("MaxLineLength")
@Test
fun `setLastDatabaseSchemeChangeInstant persists value in settingsDiskSource`() {
databaseSchemeManager.lastDatabaseSchemeChangeInstant = FIXED_CLOCK.instant()
verify {
mockSettingsDiskSource.lastDatabaseSchemeChangeInstant = FIXED_CLOCK.instant()
}
}
@Test
fun `getLastDatabaseSchemeChangeInstant retrieves stored value from settingsDiskSource`() {
databaseSchemeManager.lastDatabaseSchemeChangeInstant
verify {
mockSettingsDiskSource.lastDatabaseSchemeChangeInstant
}
}
}
private val FIXED_CLOCK: Clock = Clock.fixed(
Instant.parse("2023-10-27T12:00:00Z"),
ZoneOffset.UTC,
)

View file

@ -0,0 +1,32 @@
package com.x8bit.bitwarden.data.vault.datasource.disk.callback
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import io.mockk.verify
import org.junit.Test
import java.time.Clock
import java.time.Instant
import java.time.ZoneOffset
class DatabaseSchemeCallbackTest {
private val databaseSchemeManager: DatabaseSchemeManager = mockk {
every { lastDatabaseSchemeChangeInstant = any() } just runs
}
private val callback = DatabaseSchemeCallback(databaseSchemeManager, FIXED_CLOCK)
@Test
fun `onDestructiveMigration updates lastDatabaseSchemeChangeInstant`() {
callback.onDestructiveMigration(mockk())
verify { databaseSchemeManager.lastDatabaseSchemeChangeInstant = FIXED_CLOCK.instant() }
}
}
private val FIXED_CLOCK: Clock = Clock.fixed(
Instant.parse("2023-10-27T12:00:00Z"),
ZoneOffset.UTC,
)

View file

@ -25,6 +25,7 @@ import com.x8bit.bitwarden.data.auth.manager.UserLogoutManager
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.DatabaseSchemeManager
import com.x8bit.bitwarden.data.platform.manager.PushManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.manager.model.SyncCipherDeleteData
@ -189,6 +190,9 @@ class VaultRepositoryTest {
mutableUnlockedUserIdsStateFlow.first { userId in it }
}
}
private val databaseSchemeManager: DatabaseSchemeManager = mockk {
every { lastDatabaseSchemeChangeInstant } returns null
}
private val mutableFullSyncFlow = bufferedMutableSharedFlow<Unit>()
private val mutableSyncCipherDeleteFlow = bufferedMutableSharedFlow<SyncCipherDeleteData>()
@ -224,6 +228,7 @@ class VaultRepositoryTest {
fileManager = fileManager,
clock = clock,
userLogoutManager = userLogoutManager,
databaseSchemeManager = databaseSchemeManager,
)
@BeforeEach
@ -1057,6 +1062,60 @@ class VaultRepositoryTest {
coVerify(exactly = 0) { syncService.sync() }
}
@Test
fun `syncIfNecessary when there is no last scheme change should not sync the vault`() {
val userId = "mockId-1"
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getLastSyncTime(userId)
} returns clock.instant().minus(1, ChronoUnit.MINUTES)
every {
databaseSchemeManager.lastDatabaseSchemeChangeInstant
} returns null
coEvery { syncService.sync() } just awaits
vaultRepository.syncIfNecessary()
coVerify(exactly = 0) { syncService.sync() }
}
@Suppress("MaxLineLength")
@Test
fun `syncIfNecessary when the last scheme change is before the last sync time should not sync the vault`() {
val userId = "mockId-1"
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getLastSyncTime(userId)
} returns clock.instant().plus(1, ChronoUnit.MINUTES)
every {
databaseSchemeManager.lastDatabaseSchemeChangeInstant
} returns clock.instant().minus(1, ChronoUnit.MINUTES)
coEvery { syncService.sync() } just awaits
vaultRepository.syncIfNecessary()
coVerify(exactly = 0) { syncService.sync() }
}
@Suppress("MaxLineLength")
@Test
fun `syncIfNecessary when the last scheme change is after the last sync time should sync the vault`() {
val userId = "mockId-1"
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getLastSyncTime(userId)
} returns clock.instant().minus(1, ChronoUnit.MINUTES)
every {
databaseSchemeManager.lastDatabaseSchemeChangeInstant
} returns clock.instant().plus(1, ChronoUnit.MINUTES)
coEvery { syncService.sync() } just awaits
vaultRepository.syncIfNecessary()
coVerify { syncService.sync() }
}
@Test
fun `sync when the last sync time is older than the revision date should sync the vault`() {
val userId = "mockId-1"