-
-
Notifications
You must be signed in to change notification settings - Fork 238
feat: Add data migration and robust system sync #136
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,17 +11,20 @@ import githubstore.composeapp.generated.resources.failed_to_update | |||||||||||||||||||||||||||||||||||||||||||||||
| import githubstore.composeapp.generated.resources.no_updates_available | ||||||||||||||||||||||||||||||||||||||||||||||||
| import githubstore.composeapp.generated.resources.update_all_failed | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.CancellationException | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.Dispatchers | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.Job | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.channels.Channel | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.delay | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.MutableStateFlow | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.SharingStarted | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.first | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.onStart | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.receiveAsFlow | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.stateIn | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.flow.update | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.isActive | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.launch | ||||||||||||||||||||||||||||||||||||||||||||||||
| import kotlinx.coroutines.withContext | ||||||||||||||||||||||||||||||||||||||||||||||||
| import org.jetbrains.compose.resources.getString | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.core.data.services.PackageMonitor | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.core.data.local.db.entities.InstalledApp | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -32,6 +35,8 @@ import zed.rainxch.githubstore.feature.apps.presentation.model.UpdateAllProgress | |||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.feature.apps.presentation.model.UpdateState | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.core.data.services.Downloader | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.core.data.services.Installer | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.core.domain.Platform | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.core.domain.model.PlatformType | ||||||||||||||||||||||||||||||||||||||||||||||||
| import zed.rainxch.githubstore.feature.details.domain.repository.DetailsRepository | ||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.File | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -41,7 +46,8 @@ class AppsViewModel( | |||||||||||||||||||||||||||||||||||||||||||||||
| private val downloader: Downloader, | ||||||||||||||||||||||||||||||||||||||||||||||||
| private val installedAppsRepository: InstalledAppsRepository, | ||||||||||||||||||||||||||||||||||||||||||||||||
| private val packageMonitor: PackageMonitor, | ||||||||||||||||||||||||||||||||||||||||||||||||
| private val detailsRepository: DetailsRepository | ||||||||||||||||||||||||||||||||||||||||||||||||
| private val detailsRepository: DetailsRepository, | ||||||||||||||||||||||||||||||||||||||||||||||||
| private val platform: Platform | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) : ViewModel() { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private var hasLoadedInitialData = false | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -70,6 +76,8 @@ class AppsViewModel( | |||||||||||||||||||||||||||||||||||||||||||||||
| _state.update { it.copy(isLoading = true) } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||
| syncSystemExistenceAndMigrate() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| appsRepository.getApps().collect { apps -> | ||||||||||||||||||||||||||||||||||||||||||||||||
| val appItems = apps.map { app -> | ||||||||||||||||||||||||||||||||||||||||||||||||
| val existing = _state.value.apps.find { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -104,6 +112,67 @@ class AppsViewModel( | |||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private suspend fun syncSystemExistenceAndMigrate() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| withContext(Dispatchers.IO) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||
| val installedPackageNames = packageMonitor.getAllInstalledPackageNames() | ||||||||||||||||||||||||||||||||||||||||||||||||
| val appsInDb = installedAppsRepository.getAllInstalledApps().first() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| appsInDb.forEach { app -> | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (!installedPackageNames.contains(app.packageName)) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.d { "App ${app.packageName} no longer installed (not in system packages), removing from DB" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedAppsRepository.deleteInstalledApp(app.packageName) | ||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (app.installedVersionName == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (platform.type == PlatformType.ANDROID) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| val systemInfo = packageMonitor.getInstalledPackageInfo(app.packageName) | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (systemInfo != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedAppsRepository.updateApp(app.copy( | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedVersionName = systemInfo.versionName, | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedVersionCode = systemInfo.versionCode, | ||||||||||||||||||||||||||||||||||||||||||||||||
| latestVersionName = systemInfo.versionName, | ||||||||||||||||||||||||||||||||||||||||||||||||
| latestVersionCode = systemInfo.versionCode | ||||||||||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.d { "Migrated ${app.packageName}: set versionName/code from system" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedAppsRepository.updateApp(app.copy( | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedVersionName = app.installedVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedVersionCode = 0L, | ||||||||||||||||||||||||||||||||||||||||||||||||
| latestVersionName = app.installedVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||
| latestVersionCode = 0L | ||||||||||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.d { "Migrated ${app.packageName}: fallback to tag as versionName" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedAppsRepository.updateApp(app.copy( | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedVersionName = app.installedVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedVersionCode = 0L, | ||||||||||||||||||||||||||||||||||||||||||||||||
| latestVersionName = app.installedVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||
| latestVersionCode = 0L | ||||||||||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.d { "Migrated ${app.packageName} (desktop): fallback to tag as versionName" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.d { "Robust system existence sync and data migration completed" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e: Exception) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.e { "Failed to sync existence or migrate data: ${e.message}" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private fun checkAllForUpdates() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| viewModelScope.launch { | ||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||
| syncSystemExistenceAndMigrate() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| installedAppsRepository.checkAllForUpdates() | ||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e: Exception) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.e { "Check all for updates failed: ${e.message}" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+164
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same Similar to 🔎 Proposed fix viewModelScope.launch {
try {
syncSystemExistenceAndMigrate()
installedAppsRepository.checkAllForUpdates()
} catch (e: Exception) {
+ if (e is CancellationException) throw e
Logger.e { "Check all for updates failed: ${e.message}" }
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| fun onAction(action: AppsAction) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| when (action) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| AppsAction.OnNavigateBackClick -> { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -395,22 +464,6 @@ class AppsViewModel( | |||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private fun checkAllForUpdates() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| viewModelScope.launch { | ||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||
| _state.value.apps.forEach { appItem -> | ||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||
| installedAppsRepository.checkForUpdates(appItem.installedApp.packageName) | ||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e: Exception) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.w { "Failed to check updates for ${appItem.installedApp.packageName}" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e: Exception) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Logger.e { "Check all for updates failed: ${e.message}" } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private fun updateAppState(packageName: String, state: UpdateState) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| _state.update { currentState -> | ||||||||||||||||||||||||||||||||||||||||||||||||
| currentState.copy( | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider re-throwing
CancellationExceptionto respect coroutine cancellation.The catch block on line 158 catches all exceptions including
CancellationException. This can cause the coroutine to appear successful when it was actually cancelled, potentially leading to inconsistent state during ViewModel cleanup.🔎 Proposed fix
Logger.d { "Robust system existence sync and data migration completed" } } catch (e: Exception) { + if (e is CancellationException) throw e Logger.e { "Failed to sync existence or migrate data: ${e.message}" } }📝 Committable suggestion
🤖 Prompt for AI Agents