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 29b066c1..ed8de52a 100644 --- a/app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ApkDownloadViewModel.kt @@ -340,7 +340,7 @@ class ApkDownloadViewModel(application: Application) : AndroidViewModel(applicat */ private fun failureMessage(state: ApkDownloader.DownloadState.Failed): String = getApplication().getString( - state.messageRes, + state.reason.messageRes, *state.messageArgs.toTypedArray() ) diff --git a/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt b/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt index f8955e60..98e4ca1f 100644 --- a/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt +++ b/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt @@ -58,17 +58,56 @@ internal object DefaultApkDownloadSources { ) } +/** + * Why a download failed, and which string says so. + * + * Crosses a WorkManager `Data` boundary by [name], never by resource id: WorkManager keeps failed + * records in its own database across app updates, and AAPT2 reassigns `R.string` ids on every + * build, so a persisted id would resolve against the wrong resource table after an update. Same + * reasoning as [ApkDownloader.DownloadPhase.fromKey]. + */ +enum class ApkDownloadFailureReason(@StringRes val messageRes: Int) { + Generic(R.string.prepare_apk_error_generic), + Cancelled(R.string.prepare_apk_download_cancelled), + RateLimitedWithWait(R.string.prepare_apk_error_rate_limited_wait), + RateLimited(R.string.prepare_apk_error_rate_limited), + NoUniversalApk(R.string.prepare_apk_error_no_universal), + HttpFailure(R.string.prepare_apk_error_http), + InsufficientStorage(R.string.prepare_apk_error_storage_needed), + NoSources(R.string.prepare_apk_error_no_sources), + TorConnecting(R.string.prepare_apk_error_tor_connecting), + NoUsableUrl(R.string.prepare_apk_error_no_url), + Unreachable(R.string.prepare_apk_error_unreachable), + InsecureRedirect(R.string.prepare_apk_error_insecure_redirect), + ResumeRejected(R.string.prepare_apk_error_resume_rejected), + Incomplete(R.string.prepare_apk_error_incomplete), + InvalidResume(R.string.prepare_apk_error_invalid_resume), + UntrustedKey(R.string.prepare_apk_error_untrusted_key), + NotUniversal(R.string.prepare_apk_error_not_universal), + ApkUnreadable(R.string.prepare_apk_error_apk_unreadable), + NotBitchat(R.string.prepare_apk_error_not_bitchat), + NoVersion(R.string.prepare_apk_error_no_version), + SourceFailed(R.string.prepare_apk_error_source_failed), + AllSourcesFailed(R.string.prepare_apk_error_all_sources); + + companion object { + /** Work enqueued by an older build may name a reason this build no longer has. */ + fun fromKey(key: String?): ApkDownloadFailureReason = + entries.firstOrNull { it.name == key } ?: Generic + } +} + /** * A host-neutral download failure that tells the worker whether backoff can help. * - * [messageRes] and [messageArgs] name what the user should be told without saying it in any + * [reason] and [messageArgs] name what the user should be told without saying it in any * particular language. This layer has no Context by design — that is what keeps its tests plain * JUnit — so the ViewModel resolves them. The inherited [message] stays English for logs and * stack traces, and is never shown. */ class ApkDownloadException( message: String, - @StringRes val messageRes: Int, + val reason: ApkDownloadFailureReason, val messageArgs: List = emptyList(), val retryable: Boolean, val sourceId: String? = null, @@ -120,10 +159,10 @@ internal object ApkDownloadHttpErrors { } return ApkDownloadException( message = "${source.id} rate limited: HTTP $code, retryAt=$retryAt", - messageRes = if (minutes != null) { - R.string.prepare_apk_error_rate_limited_wait + reason = if (minutes != null) { + ApkDownloadFailureReason.RateLimitedWithWait } else { - R.string.prepare_apk_error_rate_limited + ApkDownloadFailureReason.RateLimited }, messageArgs = listOfNotNull(source.displayName, minutes?.toString()), retryable = false, @@ -136,10 +175,10 @@ internal object ApkDownloadHttpErrors { val retryable = code == 408 || code == 425 || code >= 500 return ApkDownloadException( message = "${source.id} failed: HTTP $code $responseMessage", - messageRes = if (code == 404) { - R.string.prepare_apk_error_no_universal + reason = if (code == 404) { + ApkDownloadFailureReason.NoUniversalApk } else { - R.string.prepare_apk_error_http + ApkDownloadFailureReason.HttpFailure }, messageArgs = if (code == 404) { listOf(source.displayName) diff --git a/app/src/main/java/com/bitchat/android/util/ApkDownloadWorker.kt b/app/src/main/java/com/bitchat/android/util/ApkDownloadWorker.kt index 710ec1c1..68c8c487 100644 --- a/app/src/main/java/com/bitchat/android/util/ApkDownloadWorker.kt +++ b/app/src/main/java/com/bitchat/android/util/ApkDownloadWorker.kt @@ -36,7 +36,7 @@ class ApkDownloadWorker( const val KEY_PHASE = "phase" const val KEY_VERSION = "version" const val KEY_SIZE_MB = "size_mb" - const val KEY_ERROR_RES = "error_res" + const val KEY_ERROR_REASON = "error_reason" const val KEY_ERROR_ARGS = "error_args" const val KEY_RESUMABLE_PERCENT = "resumable_percent" @@ -109,9 +109,11 @@ class ApkDownloadWorker( // generic one rather than leaking an untranslated exception string to the user. val failure = error as? ApkDownloadException val outputData = Data.Builder() - .putInt( - KEY_ERROR_RES, - failure?.messageRes ?: R.string.prepare_apk_error_generic + // The reason's name, never its resource id: this record can outlive the build + // that wrote it, and resource ids are reassigned on every build. + .putString( + KEY_ERROR_REASON, + (failure?.reason ?: ApkDownloadFailureReason.Generic).name ) .putStringArray( KEY_ERROR_ARGS, diff --git a/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt b/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt index 615e2357..b3f342c1 100644 --- a/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt +++ b/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt @@ -1,6 +1,5 @@ package com.bitchat.android.util -import androidx.annotation.StringRes import kotlinx.coroutines.flow.Flow /** @@ -36,12 +35,12 @@ interface ApkDownloader { ) : DownloadState() data class Success(val version: String, val sizeMB: Int) : DownloadState() /** - * [messageRes] and [messageArgs] are resolved by the ViewModel, which has a Context. - * Carrying the ids rather than formatted text keeps the failure localizable all the way - * across the WorkManager boundary. + * [reason] and [messageArgs] are resolved by the ViewModel, which has a Context. + * Carrying the reason rather than formatted text keeps the failure localizable all the + * way across the WorkManager boundary. */ data class Failed( - @StringRes val messageRes: Int, + val reason: ApkDownloadFailureReason, val messageArgs: List, val resumablePercent: Int? ) : DownloadState() diff --git a/app/src/main/java/com/bitchat/android/util/UniversalApkManager.kt b/app/src/main/java/com/bitchat/android/util/UniversalApkManager.kt index a50bfd8f..a20f8419 100644 --- a/app/src/main/java/com/bitchat/android/util/UniversalApkManager.kt +++ b/app/src/main/java/com/bitchat/android/util/UniversalApkManager.kt @@ -4,8 +4,6 @@ import android.content.Context import android.content.pm.PackageManager import android.os.Build import android.util.Log -import androidx.annotation.StringRes -import com.bitchat.android.R import com.bitchat.android.BuildConfig import com.bitchat.android.net.ArtiTorManager import com.bitchat.android.net.OkHttpProvider @@ -168,7 +166,7 @@ class UniversalApkManager( Log.e(TAG, error) throw ApkDownloadException( message = error, - messageRes = R.string.prepare_apk_error_storage_needed, + reason = ApkDownloadFailureReason.InsufficientStorage, messageArgs = listOf(requiredMB.toString(), availableMB.toString()), retryable = false ) @@ -187,7 +185,7 @@ class UniversalApkManager( return@withContext Result.failure( ApkDownloadException( message = "No APK download sources are configured.", - messageRes = R.string.prepare_apk_error_no_sources, + reason = ApkDownloadFailureReason.NoSources, retryable = false ) ) @@ -199,7 +197,7 @@ class UniversalApkManager( return@withContext Result.failure( ApkDownloadException( message = "Tor is still connecting.", - messageRes = R.string.prepare_apk_error_tor_connecting, + reason = ApkDownloadFailureReason.TorConnecting, retryable = true ) ) @@ -341,7 +339,7 @@ class UniversalApkManager( throw lastFailure ?: ApkDownloadException( message = "${source.id} has no usable APK URL.", - messageRes = R.string.prepare_apk_error_no_url, + reason = ApkDownloadFailureReason.NoUsableUrl, messageArgs = listOf(source.displayName), retryable = false ) @@ -409,7 +407,7 @@ class UniversalApkManager( ApkDownloadException( message = "${source.id} could not be reached" + (e.message?.let { ": $it" } ?: "."), - messageRes = R.string.prepare_apk_error_unreachable, + reason = ApkDownloadFailureReason.Unreachable, messageArgs = listOf(source.displayName), retryable = true, sourceId = source.id, @@ -424,7 +422,7 @@ class UniversalApkManager( if (!response.request.url.isHttps) { throw ApkDownloadException( message = "${source.id} redirected to an insecure URL.", - messageRes = R.string.prepare_apk_error_insecure_redirect, + reason = ApkDownloadFailureReason.InsecureRedirect, messageArgs = listOf(source.displayName), retryable = false, sourceId = source.id @@ -441,7 +439,7 @@ class UniversalApkManager( clearPartialDownload() throw ApkDownloadException( message = "${source.id} rejected the saved download position.", - messageRes = R.string.prepare_apk_error_resume_rejected, + reason = ApkDownloadFailureReason.ResumeRejected, messageArgs = listOf(source.displayName), retryable = true, sourceId = source.id, @@ -536,7 +534,7 @@ class UniversalApkManager( if (tempFile.length() > expectedSize) clearPartialDownload() throw ApkDownloadException( message = "${source.id} download ended before all bytes arrived.", - messageRes = R.string.prepare_apk_error_incomplete, + reason = ApkDownloadFailureReason.Incomplete, messageArgs = listOf(source.displayName), retryable = true, sourceId = source.id @@ -562,7 +560,7 @@ class UniversalApkManager( progressFile.delete() return ApkDownloadException( message = "${source.id} returned an invalid resume response.", - messageRes = R.string.prepare_apk_error_invalid_resume, + reason = ApkDownloadFailureReason.InvalidResume, messageArgs = listOf(source.displayName), retryable = true, sourceId = source.id @@ -574,7 +572,7 @@ class UniversalApkManager( clearPartialDownload() throw ApkDownloadException( message = "APK from ${source.id} is not signed by a trusted BitChat release key.", - messageRes = R.string.prepare_apk_error_untrusted_key, + reason = ApkDownloadFailureReason.UntrustedKey, messageArgs = listOf(source.displayName), retryable = false, sourceId = source.id @@ -584,7 +582,7 @@ class UniversalApkManager( clearPartialDownload() throw ApkDownloadException( message = "${source.id} returned an architecture-specific APK.", - messageRes = R.string.prepare_apk_error_not_universal, + reason = ApkDownloadFailureReason.NotUniversal, messageArgs = listOf(source.displayName), retryable = false, sourceId = source.id @@ -594,20 +592,23 @@ class UniversalApkManager( private fun downloadedVersionName(apkFile: File): String { val packageInfo = context.packageManager.getPackageArchiveInfo(apkFile.absolutePath, 0) - ?: invalidDownloadedApk(R.string.prepare_apk_error_apk_unreadable, "unreadable APK") + ?: invalidDownloadedApk(ApkDownloadFailureReason.ApkUnreadable, "unreadable APK") if (packageInfo.packageName != context.packageName) { - invalidDownloadedApk(R.string.prepare_apk_error_not_bitchat, "wrong package") + invalidDownloadedApk(ApkDownloadFailureReason.NotBitchat, "wrong package") } return packageInfo.versionName ?.takeIf { it.isNotBlank() } - ?: invalidDownloadedApk(R.string.prepare_apk_error_no_version, "no version name") + ?: invalidDownloadedApk(ApkDownloadFailureReason.NoVersion, "no version name") } - private fun invalidDownloadedApk(@StringRes messageRes: Int, logReason: String): Nothing { + private fun invalidDownloadedApk( + reason: ApkDownloadFailureReason, + logReason: String + ): Nothing { clearPartialDownload() throw ApkDownloadException( message = "Downloaded APK rejected: $logReason", - messageRes = messageRes, + reason = reason, retryable = false ) } @@ -616,7 +617,7 @@ class UniversalApkManager( if (this is ApkDownloadException) return this return ApkDownloadException( message = "${source.id} download failed" + (message?.let { ": $it" } ?: "."), - messageRes = R.string.prepare_apk_error_source_failed, + reason = ApkDownloadFailureReason.SourceFailed, messageArgs = listOf(source.displayName), retryable = this is IOException, sourceId = source.id, @@ -631,7 +632,7 @@ class UniversalApkManager( if (failures.isEmpty()) { return ApkDownloadException( message = "APK download failed with no recorded source failure.", - messageRes = R.string.prepare_apk_error_generic, + reason = ApkDownloadFailureReason.Generic, retryable = false ) } @@ -640,7 +641,7 @@ class UniversalApkManager( return ApkDownloadException( message = "All configured APK sources failed: " + failures.joinToString(" • ") { it.message ?: "Unknown error" }, - messageRes = R.string.prepare_apk_error_all_sources, + reason = ApkDownloadFailureReason.AllSourcesFailed, retryable = failures.any { it.retryable }, cause = failures.last() ) diff --git a/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt b/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt index 0b3e597b..214b8118 100644 --- a/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt +++ b/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt @@ -3,7 +3,6 @@ package com.bitchat.android.util import android.content.Context import androidx.work.Constraints import androidx.work.BackoffPolicy -import com.bitchat.android.R import androidx.work.ExistingWorkPolicy import androidx.work.NetworkType import androidx.work.OneTimeWorkRequestBuilder @@ -79,19 +78,17 @@ class WorkManagerApkDownloader(context: Context) : ApkDownloader { ApkDownloader.DownloadState.Success(version, sizeMB) } WorkInfo.State.FAILED -> { - // Work enqueued by an older build carries no resource id; fall back rather than - // resolve 0 and crash. - val messageRes = workInfo.outputData - .getInt(ApkDownloadWorker.KEY_ERROR_RES, 0) - .takeIf { it != 0 } - ?: R.string.prepare_apk_error_generic + // Tolerates a missing or retired reason from an older build's record. + val reason = ApkDownloadFailureReason.fromKey( + workInfo.outputData.getString(ApkDownloadWorker.KEY_ERROR_REASON) + ) val args = workInfo.outputData .getStringArray(ApkDownloadWorker.KEY_ERROR_ARGS) ?.toList() .orEmpty() val resumable = workInfo.outputData.getInt(ApkDownloadWorker.KEY_RESUMABLE_PERCENT, -1) ApkDownloader.DownloadState.Failed( - messageRes = messageRes, + reason = reason, messageArgs = args, resumablePercent = if (resumable >= 0) resumable else null ) @@ -100,7 +97,7 @@ class WorkManagerApkDownloader(context: Context) : ApkDownloader { val partial = apkManager.getPartialDownloadProgress() if (partial != null) { ApkDownloader.DownloadState.Failed( - messageRes = R.string.prepare_apk_download_cancelled, + reason = ApkDownloadFailureReason.Cancelled, messageArgs = emptyList(), resumablePercent = partial ) diff --git a/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt b/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt index 8caa9c61..db448d59 100644 --- a/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt @@ -1,6 +1,5 @@ package com.bitchat.android.util -import com.bitchat.android.R import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -63,7 +62,7 @@ class ApkDownloadSourceTest { assertFalse(failure.retryable) assertEquals(now + 120_000L, failure.retryAtMillis) // The wait is carried as an argument, not baked into an English sentence. - assertEquals(R.string.prepare_apk_error_rate_limited_wait, failure.messageRes) + assertEquals(ApkDownloadFailureReason.RateLimitedWithWait, failure.reason) assertEquals(listOf(source.displayName, "2"), failure.messageArgs) } @@ -89,13 +88,13 @@ class ApkDownloadSourceTest { ) assertNull(permissionsFailure.retryAtMillis) - assertEquals(R.string.prepare_apk_error_http, permissionsFailure.messageRes) + assertEquals(ApkDownloadFailureReason.HttpFailure, permissionsFailure.reason) assertEquals( listOf(source.displayName, "403", "Forbidden"), permissionsFailure.messageArgs ) assertEquals(now + 300_000L, quotaFailure.retryAtMillis) - assertEquals(R.string.prepare_apk_error_rate_limited_wait, quotaFailure.messageRes) + assertEquals(ApkDownloadFailureReason.RateLimitedWithWait, quotaFailure.reason) } @Test @@ -141,13 +140,35 @@ class ApkDownloadSourceTest { runAttemptCount = 0, ApkDownloadException( message = "invalid APK", - messageRes = R.string.prepare_apk_error_generic, + reason = ApkDownloadFailureReason.Generic, retryable = false ) ) ) } + @Test + fun `a failure reason survives the round trip through its key`() { + // WorkManager keeps failed records across app updates, so the key written by one build is + // read by the next. Resource ids are reassigned per build and would resolve to the wrong + // string; the name does not move. + ApkDownloadFailureReason.entries.forEach { reason -> + assertEquals(reason, ApkDownloadFailureReason.fromKey(reason.name)) + } + } + + @Test + fun `an absent or retired reason falls back instead of resolving nothing`() { + assertEquals( + ApkDownloadFailureReason.Generic, + ApkDownloadFailureReason.fromKey(null) + ) + assertEquals( + ApkDownloadFailureReason.Generic, + ApkDownloadFailureReason.fromKey("ReasonFromAFutureBuild") + ) + } + private fun httpError(code: Int): ApkDownloadException { return ApkDownloadHttpErrors.fromResponse( source = source,