Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PM-15177: Improve destructive fallback logic #4372

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ 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

/**
Expand Down Expand Up @@ -74,7 +73,6 @@ object PlatformDiskModule {
fun provideEventDatabase(
app: Application,
databaseSchemeManager: DatabaseSchemeManager,
clock: Clock,
): PlatformDatabase =
Room
.databaseBuilder(
Expand All @@ -84,12 +82,7 @@ object PlatformDiskModule {
)
.fallbackToDestructiveMigration()
.addTypeConverter(ZonedDateTimeTypeConverter())
.addCallback(
DatabaseSchemeCallback(
databaseSchemeManager = databaseSchemeManager,
clock = clock,
),
)
.addCallback(DatabaseSchemeCallback(databaseSchemeManager = databaseSchemeManager))
.build()

@Provides
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
package com.x8bit.bitwarden.data.platform.manager

import kotlinx.coroutines.flow.Flow
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.
* Clears the sync state for all users and emits on the [databaseSchemeChangeFlow].
*/
var lastDatabaseSchemeChangeInstant: Instant?
fun clearSyncState()

/**
* A flow of the last database schema change instant.
* Emits whenever the sync state hs been cleared.
*/
val lastDatabaseSchemeChangeInstantFlow: Flow<Instant?>
val databaseSchemeChangeFlow: Flow<Unit>
}
Original file line number Diff line number Diff line change
@@ -1,34 +1,27 @@
package com.x8bit.bitwarden.data.platform.manager

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 kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.stateIn
import java.time.Instant
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow

/**
* Primary implementation of [DatabaseSchemeManager].
*/
class DatabaseSchemeManagerImpl(
val authDiskSource: AuthDiskSource,
val settingsDiskSource: SettingsDiskSource,
val dispatcherManager: DispatcherManager,
) : DatabaseSchemeManager {
private val mutableSharedFlow: MutableSharedFlow<Unit> = bufferedMutableSharedFlow()

private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined)

override var lastDatabaseSchemeChangeInstant: Instant?
get() = settingsDiskSource.lastDatabaseSchemeChangeInstant
set(value) {
settingsDiskSource.lastDatabaseSchemeChangeInstant = value
override fun clearSyncState() {
authDiskSource.userState?.accounts?.forEach { (userId, _) ->
settingsDiskSource.storeLastSyncTime(userId = userId, lastSyncTime = null)
}
mutableSharedFlow.tryEmit(Unit)
}

override val lastDatabaseSchemeChangeInstantFlow =
settingsDiskSource
.lastDatabaseSchemeChangeInstantFlow
.stateIn(
scope = unconfinedScope,
started = SharingStarted.Eagerly,
initialValue = settingsDiskSource.lastDatabaseSchemeChangeInstant,
)
override val databaseSchemeChangeFlow: Flow<Unit> = mutableSharedFlow.asSharedFlow()
}
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,10 @@ object PlatformManagerModule {
@Provides
@Singleton
fun provideDatabaseSchemeManager(
authDiskSource: AuthDiskSource,
settingsDiskSource: SettingsDiskSource,
dispatcherManager: DispatcherManager,
): DatabaseSchemeManager = DatabaseSchemeManagerImpl(
authDiskSource = authDiskSource,
settingsDiskSource = settingsDiskSource,
dispatcherManager = dispatcherManager,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ 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

/**
Expand Down Expand Up @@ -51,20 +50,14 @@ object GeneratorDiskModule {
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,
),
)
.addCallback(DatabaseSchemeCallback(databaseSchemeManager = databaseSchemeManager))
.build()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ 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()
databaseSchemeManager.clearSyncState()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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

/**
Expand All @@ -34,7 +33,6 @@ class VaultDiskModule {
fun provideVaultDatabase(
app: Application,
databaseSchemeManager: DatabaseSchemeManager,
clock: Clock,
): VaultDatabase =
Room
.databaseBuilder(
Expand All @@ -43,7 +41,7 @@ class VaultDiskModule {
name = "vault_database",
)
.fallbackToDestructiveMigration()
.addCallback(DatabaseSchemeCallback(databaseSchemeManager, clock))
.addCallback(DatabaseSchemeCallback(databaseSchemeManager = databaseSchemeManager))
.addTypeConverter(ZonedDateTimeTypeConverter())
.build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ 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
Expand Down Expand Up @@ -326,9 +325,8 @@ class VaultRepositoryImpl(
.launchIn(ioScope)

databaseSchemeManager
.lastDatabaseSchemeChangeInstantFlow
.filterNotNull()
.onEach { sync() }
.databaseSchemeChangeFlow
.onEach { sync(forced = true) }
.launchIn(ioScope)
}

Expand Down Expand Up @@ -361,13 +359,11 @@ 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, 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)) ||
lastDatabaseSchemeChangeInstant?.isAfter(lastSyncInstant) == true
currentInstant.isAfter(lastSyncInstant.plus(30, ChronoUnit.MINUTES))
) {
sync()
}
Expand Down Expand Up @@ -1347,37 +1343,33 @@ class VaultRepositoryImpl(
val lastSyncInstant = settingsDiskSource
.getLastSyncTime(userId = userId)
?.toEpochMilli()
?: 0
val lastDatabaseSchemeChangeInstant = databaseSchemeManager
.lastDatabaseSchemeChangeInstant
?.toEpochMilli()
?: 0
syncService
.getAccountRevisionDateMillis()
.fold(
onSuccess = { serverRevisionDate ->
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 = userId)
settingsDiskSource.storeLastSyncTime(
userId = userId,
lastSyncTime = clock.instant(),
)
val itemsAvailable = vaultDiskSource
.getCiphers(userId)
.firstOrNull()
?.isNotEmpty() == true
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
}
},
onFailure = {
updateVaultStateFlowsToError(throwable = it)
return SyncVaultDataResult.Error(throwable = it)
},
)
lastSyncInstant?.let { lastSyncTimeMs ->
// If the lasSyncState is null we just sync, no checks required.
syncService
.getAccountRevisionDateMillis()
.fold(
onSuccess = { serverRevisionDate ->
if (serverRevisionDate < lastSyncTimeMs) {
// We can skip the actual sync call if there is no new data or
// database scheme changes since the last sync.
vaultDiskSource.resyncVaultData(userId = userId)
settingsDiskSource.storeLastSyncTime(
userId = userId,
lastSyncTime = clock.instant(),
)
val itemsAvailable = vaultDiskSource
.getCiphers(userId)
.firstOrNull()
?.isNotEmpty() == true
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
}
},
onFailure = {
updateVaultStateFlowsToError(throwable = it)
return SyncVaultDataResult.Error(throwable = it)
},
)
}
}

return syncService
Expand Down
Loading