mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-08-29 07:16:08 +00:00
fix(apk): keep the local APK shareable across a restart mid-download
Codex is right about this one. The downloader observer builds Downloading out of whatever status it replaces, reading shareableFallback from an existing Downloading or a current Ready. A ViewModel restored onto work that is already active starts from Loading, so neither cast matches and the fallback is null. WorkManager keeps a download running across process death, so this is the ordinary case: background the app during a 42 MB transfer over Tor, come back, and the row drops to "Prepare App for Sharing" while Share via Hotspot and Share via Quick Share disappear entirely. The installed APK never moved - it was on disk and shareable a moment earlier - and it stays hidden until the download ends. checkStatus() could not repair it because its guard conflated two things: not letting a resolved status overwrite active work, which is right, and not looking at local state at all during a download, which is not. Both orderings lost. If the observer arrived first the guard returned early. If checkStatus() arrived first it suspended on disk IO, the observer flipped the state underneath it, and the re-check discarded the status it had just resolved. Resolves the local artifact either way and decides inside the same state update, where the active download is visible: the download keeps the status it owns, and adopts the artifact only when it is carrying none. Metadata is still skipped while work is active, so this costs no extra API budget. Verified on a Pixel 9a by starting a download, force-stopping mid-transfer and reopening: the row holds "App Ready for Offline Sharing" with both sharing rows present, where it previously showed neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
52f14cc5b5
commit
8f21ad5be3
@ -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()
|
||||
|
||||
@ -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<String>(), 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<GitHubReleaseClient.ReleaseSnapshot> =
|
||||
Result.failure(IllegalStateException("synthetic offline response"))
|
||||
@ -187,10 +254,10 @@ class ApkDownloadViewModelTest {
|
||||
error("unreachable")
|
||||
}
|
||||
|
||||
private class FakeDownloader : ApkDownloader {
|
||||
private val mutableState = MutableStateFlow<ApkDownloader.DownloadState>(
|
||||
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
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user