diff --git a/app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt index b076477f..4d0c2d5b 100644 --- a/app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt @@ -311,26 +311,38 @@ class ApkDownloadViewModel internal constructor( private fun checkStatus() { viewModelScope.launch { - // WorkManager is the source of truth for active work. A queued or - // newly started job legitimately has no partial file yet, so never - // infer that it is orphaned from cache contents. - if (_state.value.apkStatus is ApkPreparationStatus.Downloading) { - return@launch - } - val resolvedStatus = resolveApkStatus() _state.update { current -> - // Re-check in case the user started a download while the local - // artifact was being inspected or copied. - if (current.apkStatus is ApkPreparationStatus.Downloading) { - current - } else { - current.copy( + // WorkManager is the source of truth for active work. A queued or newly started + // job legitimately has no partial file yet, so never infer that it is orphaned + // from cache contents, and never let a resolved status overwrite it - the user + // may have started a download while the local artifact was being inspected. + when (val active = current.apkStatus) { + is ApkPreparationStatus.Downloading -> + // Active work still adopts a local artifact it was created without. A + // ViewModel restored onto a running download starts from Loading, so the + // observer had no Ready to carry into shareableFallback, and an installed + // APK - with both sharing actions - would stay hidden for the whole + // transfer. Deciding here covers the observer arriving before this runs + // and during the resolve above, which are different orderings. + if (active.shareableFallback == null) { + current.copy( + apkStatus = active.copy( + shareableFallback = shareableReady(resolvedStatus) + ) + ) + } else { + current + } + else -> current.copy( apkStatus = resolvedStatus, downloadProgress = 0 ) } } + // Metadata is skipped while work is active, as it was before: an in-flight download + // has no use for a freshness check and the API budget is scarce. + if (_state.value.apkStatus is ApkPreparationStatus.Downloading) return@launch // Local availability is resolved and published before this independent network task // starts. Metadata can add a freshness warning, but can never hide sharing. refreshReleaseMetadata() diff --git a/app/src/test/kotlin/com/bitchat/android/ui/ApkDownloadViewModelTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/ApkDownloadViewModelTest.kt index 6e146e6e..4351b7c0 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/ApkDownloadViewModelTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/ApkDownloadViewModelTest.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.test.setMain import kotlinx.coroutines.withTimeout import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertSame import org.junit.Assert.assertTrue import org.junit.Before @@ -143,6 +144,72 @@ class ApkDownloadViewModelTest { assertEquals(emptyList(), failure.messageArgs) } + @Test + fun `a download already running when the ViewModel starts still exposes the local apk`() = + runTest { + // Process death during a transfer leaves WorkManager running and the ViewModel fresh, + // so the observer builds Downloading out of Loading and has no Ready to carry. Without + // a fallback the row and both sharing actions vanish for the rest of the download. + val manager = managerWithLocalApk() + val downloader = FakeDownloader( + ApkDownloader.DownloadState.Downloading( + progressPercent = 30, + phase = ApkDownloader.DownloadPhase.Transferring + ) + ) + + val viewModel = ApkDownloadViewModel( + application, + manager, + downloader, + offlineMetadata() + ) + + val restored = viewModel.state.value.apkStatus as ApkPreparationStatus.Downloading + assertNull(restored.shareableFallback) + + viewModel.onEvent(ApkUiEvent.CheckStatus) + + val adopted = awaitFallback(viewModel) + assertEquals("1.7.5", adopted.version) + assertEquals(UniversalApkManager.ApkSource.INSTALLED, adopted.source) + } + + @Test + fun `adopting a local apk never displaces the fallback a download already carries`() = runTest { + val manager = managerWithLocalApk() + val downloader = FakeDownloader() + val viewModel = ApkDownloadViewModel( + application, + manager, + downloader, + offlineMetadata() + ) + + viewModel.onEvent(ApkUiEvent.CheckStatus) + val originalReady = awaitReady(viewModel) + viewModel.onEvent(ApkUiEvent.PrepareRowClicked) + viewModel.onEvent(ApkUiEvent.ConfirmDownload) + + viewModel.onEvent(ApkUiEvent.CheckStatus) + + val downloading = viewModel.state.value.apkStatus as ApkPreparationStatus.Downloading + assertSame(originalReady, downloading.shareableFallback) + } + + + private suspend fun awaitFallback( + viewModel: ApkDownloadViewModel + ): ApkPreparationStatus.Ready = withTimeout(5_000L) { + while (true) { + (viewModel.state.value.apkStatus as? ApkPreparationStatus.Downloading) + ?.shareableFallback + ?.let { return@withTimeout it } + delay(1L) + } + error("unreachable") + } + private fun offlineMetadata() = object : LatestReleaseProvider { override suspend fun latestRelease(): Result = Result.failure(IllegalStateException("synthetic offline response")) @@ -187,10 +254,10 @@ class ApkDownloadViewModelTest { error("unreachable") } - private class FakeDownloader : ApkDownloader { - private val mutableState = MutableStateFlow( - ApkDownloader.DownloadState.Idle - ) + private class FakeDownloader( + initial: ApkDownloader.DownloadState = ApkDownloader.DownloadState.Idle + ) : ApkDownloader { + private val mutableState = MutableStateFlow(initial) override val downloadState = mutableState.asStateFlow() var startCount = 0