Skip to content
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 @@ -247,7 +247,8 @@ val appsModule: Module = module {
installer = get(),
downloader = get(),
packageMonitor = get(),
detailsRepository = get()
detailsRepository = get(),
platform = get()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}" }
}
}
}
Comment on lines +115 to +162
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider re-throwing CancellationException to 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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 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) {
if (e is CancellationException) throw e
Logger.e { "Failed to sync existence or migrate data: ${e.message}" }
}
}
}
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.kt
around lines 115 to 162, the catch block currently swallows all Exceptions
including CancellationException; update the handler so that if the caught
exception is a CancellationException it is re-thrown (or not caught) to allow
coroutine cancellation to propagate, otherwise handle/log the error as before.
Ensure you import or reference kotlinx.coroutines.CancellationException if
needed and rethrow immediately when detected before logging or swallowing other
exceptions.


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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Same CancellationException handling issue.

Similar to syncSystemExistenceAndMigrate(), this catch block swallows cancellation. For consistency and proper coroutine behavior, re-throw CancellationException.

🔎 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private fun checkAllForUpdates() {
viewModelScope.launch {
try {
syncSystemExistenceAndMigrate()
installedAppsRepository.checkAllForUpdates()
} catch (e: Exception) {
Logger.e { "Check all for updates failed: ${e.message}" }
}
}
}
private fun checkAllForUpdates() {
viewModelScope.launch {
try {
syncSystemExistenceAndMigrate()
installedAppsRepository.checkAllForUpdates()
} catch (e: Exception) {
if (e is CancellationException) throw e
Logger.e { "Check all for updates failed: ${e.message}" }
}
}
}
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/feature/apps/presentation/AppsViewModel.kt
around lines 164 to 174, the catch block for Exception swallows
CancellationException; change the catch to re-throw CancellationException and
only log other exceptions — i.e., inside the catch check if the caught exception
is a CancellationException and throw it, otherwise log the error message as
before.


fun onAction(action: AppsAction) {
when (action) {
AppsAction.OnNavigateBackClick -> {
Expand Down Expand Up @@ -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(
Expand Down
Loading