diff --git a/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt b/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt index 5e75e555..b9a33c3e 100644 --- a/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt +++ b/app/src/main/java/com/m57/hermescontrol/HermesControlApp.kt @@ -13,7 +13,6 @@ import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.local.SessionListCacheStore import com.m57.hermescontrol.data.remote.NetworkMonitor import com.m57.hermescontrol.data.remote.OkHttpProvider -import com.m57.hermescontrol.data.update.UpdateNoticeManager import com.m57.hermescontrol.notification.TurnCorrelationTracker import com.m57.hermescontrol.ui.analytics.AnalyticsPreloader import kotlinx.coroutines.CoroutineScope @@ -41,9 +40,6 @@ class HermesControlApp : // so the tab renders instantly when opened (the usage endpoint is slow on a // cold backend). Fire-and-forget; never blocks UI startup. AnalyticsPreloader.preload(this@HermesControlApp) - // Issue #890: silent once-per-version update check right after launch, - // so the chat screen can show an update banner without user interaction. - UpdateNoticeManager.checkOnLaunch() } } diff --git a/app/src/main/java/com/m57/hermescontrol/MainActivity.kt b/app/src/main/java/com/m57/hermescontrol/MainActivity.kt index 5523362d..08e3b710 100644 --- a/app/src/main/java/com/m57/hermescontrol/MainActivity.kt +++ b/app/src/main/java/com/m57/hermescontrol/MainActivity.kt @@ -19,6 +19,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle import com.m57.hermescontrol.data.local.AuthManager +import com.m57.hermescontrol.data.update.UpdateNoticeManager import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.notification.NotificationHelper import com.m57.hermescontrol.notification.NotificationReplyReceiver @@ -26,6 +27,7 @@ import com.m57.hermescontrol.theme.HermesControlTheme import com.m57.hermescontrol.ui.common.ErrorState import com.m57.hermescontrol.ui.common.LoadingState import com.m57.hermescontrol.util.LocaleContextWrapper +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch class MainActivity : ComponentActivity() { @@ -131,6 +133,10 @@ class MainActivity : ComponentActivity() { override fun onResume() { super.onResume() + lifecycleScope.launch { + AuthManager.initializationState.first { it == AuthManager.InitializationState.Ready } + UpdateNoticeManager.checkOnLaunch() + } NotificationHelper.setAppForeground(this, true) ExternalActivityLifecycleGuard.onHostResumed() NotificationHelper.stop(this) diff --git a/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateCache.kt b/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateCache.kt index d7331790..c5bef22f 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateCache.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateCache.kt @@ -21,8 +21,9 @@ object AppUpdateCache { * Session-only dismissal of the chat banner. Snapshot-backed so the chat * screen recomposes when it flips; dies with the process. */ - var dismissed by mutableStateOf(false) - private set + private var dismissedTags by mutableStateOf>(emptySet()) + + fun isDismissed(tag: String): Boolean = tag in dismissedTags /** Whether the full-screen / in-place update dialog should be visible. */ var isDialogVisible by mutableStateOf(false) @@ -32,8 +33,8 @@ object AppUpdateCache { _state.value = state } - fun dismiss() { - dismissed = true + fun dismiss(tag: String) { + dismissedTags = dismissedTags + tag } fun showDialog() { @@ -46,7 +47,7 @@ object AppUpdateCache { /** Test hook: clear state and dismissal between tests. */ internal fun reset() { - dismissed = false + dismissedTags = emptySet() isDialogVisible = false _state.value = AppUpdateState.Idle } diff --git a/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateChecker.kt b/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateChecker.kt index a6887ba3..fb261acd 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateChecker.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateChecker.kt @@ -2,10 +2,15 @@ package com.m57.hermescontrol.data.update import com.m57.hermescontrol.data.remote.OkHttpProvider import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.suspendCancellableCoroutine import kotlinx.coroutines.withContext import kotlinx.serialization.Serializable +import okhttp3.Call +import okhttp3.Callback +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.Response import java.io.File import java.io.IOException @@ -79,17 +84,28 @@ fun selectLatestUpdate( includeReleaseCandidates: Boolean = false, ): UpdateInfo? = releases - .filter { !it.draft && it.apkAsset != null } - .filter { - if (!it.prerelease) { - true - } else { - includeReleaseCandidates && isReleaseCandidateVersion(it.tagName) - } - }.reduceOrNull { best, candidate -> + .filter(::hasUsableApk) + .filter { isEligibleRelease(it, includeReleaseCandidates) } + .reduceOrNull { best, candidate -> if (isNewerVersion(candidate.tagName, best.tagName)) candidate else best } +private fun hasUsableApk(release: UpdateInfo): Boolean = + !release.draft && release.apkAsset?.browserDownloadUrl?.toHttpUrlOrNull()?.let { + it.scheme == "http" || it.scheme == "https" + } == true + +private fun isEligibleRelease( + release: UpdateInfo, + includeReleaseCandidates: Boolean, +): Boolean { + val version = parseVersion(release.tagName) ?: return false + val isRc = version.prerelease.firstOrNull()?.let(RC_IDENTIFIER::matches) == true + val isStable = version.prerelease.isEmpty() && !release.prerelease + if (isStable) return true + return includeReleaseCandidates && isRc +} + private data class ParsedVersion( val core: List, val prerelease: List, @@ -161,17 +177,16 @@ open class AppUpdateChecker( */ open suspend fun fetchLatestRelease(includeReleaseCandidates: Boolean = false): UpdateInfo? = withContext(Dispatchers.IO) { - if (includeReleaseCandidates) { - val body = get("$apiBaseUrl/releases?per_page=$RELEASE_LIST_PAGE_SIZE") ?: return@withContext null - selectLatestUpdate(parseReleaseList(body).orEmpty(), includeReleaseCandidates = true) - } else { - val body = get("$apiBaseUrl/releases/latest") ?: return@withContext null - parseUpdateInfo(body) + val latest = get("$apiBaseUrl/releases/latest")?.let(::parseRelease) + if (!includeReleaseCandidates && latest != null && selectLatestUpdate(listOf(latest)) != null) { + return@withContext latest } + val releases = fetchReleasePages() ?: return@withContext null + selectLatestUpdate(releases + listOfNotNull(latest), includeReleaseCandidates) } - /** GET [url] as JSON text, or null on any non-2xx response. */ - private fun get(url: String): String? { + /** GET a GitHub URL. 404 means absent; other HTTP/network failures stay visible. */ + private fun get(url: String): ReleasePage? { val request = Request .Builder() @@ -179,11 +194,45 @@ open class AppUpdateChecker( .header("Accept", "application/vnd.github+json") .build() client.newCall(request).execute().use { response -> - if (!response.isSuccessful) return null - return response.body.string().orEmpty() + if (response.code == HTTP_NOT_FOUND) return null + if (!response.isSuccessful) throw IOException("GitHub releases request failed (${response.code})") + return ReleasePage(response.body.string().orEmpty(), response.nextPageUrl()) } } + private fun fetchReleasePages(): List? { + var nextUrl: String? = "$apiBaseUrl/releases?per_page=$RELEASE_LIST_PAGE_SIZE&page=1" + val releases = mutableListOf() + while (nextUrl != null) { + val page = + get(nextUrl) + ?: return if (releases.isEmpty()) null else throw IOException("Incomplete GitHub releases list") + val parsed = parseReleaseList(page.body) ?: throw IOException("GitHub returned malformed release metadata") + releases += parsed + nextUrl = page.nextUrl + } + return releases + } + + private fun parseRelease(page: ReleasePage): UpdateInfo = + parseUpdateInfo(page.body) ?: throw IOException("GitHub returned malformed release metadata") + + private fun Response.nextPageUrl(): String? { + val link = header("Link") ?: return null + val url = NEXT_LINK.find(link)?.groupValues?.get(1) ?: return null + val next = url.toHttpUrlOrNull() ?: throw IOException("GitHub returned an invalid pagination URL") + val base = apiBaseUrl.toHttpUrlOrNull() ?: throw IOException("Invalid GitHub API base URL") + if (next.host != base.host || next.encodedPath != base.encodedPath.trimEnd('/') + "/releases") { + throw IOException("GitHub returned an untrusted pagination URL") + } + return next.toString() + } + + private data class ReleasePage( + val body: String, + val nextUrl: String?, + ) + /** * Stream a release APK asset to [dest], reporting progress 0..1 via * [onProgress]. True on success; false (with the partial file deleted) @@ -194,41 +243,103 @@ open class AppUpdateChecker( dest: File, onProgress: (Float) -> Unit, ): Boolean = - withContext(Dispatchers.IO) { - val request = Request.Builder().url(url).build() - try { - client.newCall(request).execute().use { response -> - if (!response.isSuccessful) return@withContext false - val body = response.body - val total = body.contentLength() - dest.outputStream().use { out -> - val buffer = ByteArray(DEFAULT_BUFFER_SIZE) - var read = 0L - body.byteStream().use { input -> - while (true) { - val n = input.read(buffer) - if (n == -1) break - out.write(buffer, 0, n) - read += n - if (total > 0) { - onProgress((read.toFloat() / total.toFloat()).coerceIn(0f, 1f)) + suspendCancellableCoroutine { continuation -> + val request = + try { + val httpUrl = + url.toHttpUrlOrNull()?.takeIf { it.scheme == "http" || it.scheme == "https" } + ?: run { + failDownload(continuation, dest) + return@suspendCancellableCoroutine + } + Request.Builder().url(httpUrl).build() + } catch (_: IllegalArgumentException) { + failDownload(continuation, dest) + return@suspendCancellableCoroutine + } + val call = + try { + client.newCall(request) + } catch (_: Exception) { + failDownload(continuation, dest) + return@suspendCancellableCoroutine + } + continuation.invokeOnCancellation { + call.cancel() + deletePartial(dest) + } + val callback = + object : Callback { + override fun onFailure( + call: Call, + e: IOException, + ) { + failDownload(continuation, dest) + } + + override fun onResponse( + call: Call, + response: Response, + ) { + var succeeded = false + try { + response.use { + if (response.isSuccessful) { + val body = response.body + val total = body.contentLength() + dest.outputStream().use { out -> + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + var read = 0L + body.byteStream().use { input -> + while (true) { + val n = input.read(buffer) + if (n == -1) break + out.write(buffer, 0, n) + read += n + if (total > 0) { + onProgress((read.toFloat() / total.toFloat()).coerceIn(0f, 1f)) + } + } + } + } + succeeded = true } } + } catch (_: Exception) { + // Includes response/body close failures; never report success before close. + } + if (succeeded) { + if (continuation.isActive) continuation.resume(true) { _, _, _ -> } + } else { + failDownload(continuation, dest) } } - true } - } catch (e: Exception) { - dest.delete() - false + try { + call.enqueue(callback) + } catch (_: Exception) { + failDownload(continuation, dest) } } + private fun failDownload( + continuation: kotlinx.coroutines.CancellableContinuation, + dest: File, + ) { + deletePartial(dest) + if (continuation.isActive) continuation.resume(false) { _, _, _ -> } + } + + private fun deletePartial(dest: File) { + runCatching { dest.delete() } + } + private companion object { const val DEFAULT_BUFFER_SIZE = 8192 + const val HTTP_NOT_FOUND = 404 - /** How many recent releases the opt-in RC scan inspects. */ - const val RELEASE_LIST_PAGE_SIZE = 20 + const val RELEASE_LIST_PAGE_SIZE = 100 + val NEXT_LINK = Regex("<([^>]+)>;\\s*rel=\"next\"") } } diff --git a/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateState.kt b/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateState.kt index 49b3bd4c..bf19a1c7 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateState.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/update/AppUpdateState.kt @@ -40,6 +40,7 @@ sealed interface AppUpdateState { data class Error( val message: String, + val isCheckError: Boolean = false, ) : AppUpdateState } diff --git a/app/src/main/java/com/m57/hermescontrol/data/update/UpdateNoticeManager.kt b/app/src/main/java/com/m57/hermescontrol/data/update/UpdateNoticeManager.kt index b4ae9020..9047cd37 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/update/UpdateNoticeManager.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/update/UpdateNoticeManager.kt @@ -6,23 +6,31 @@ import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import java.io.IOException /** - * Startup update check (issue #890): one silent GitHub ping per installed - * version, fired right after launch, so the chat screen can show an update - * banner without the user ever opening the About tab. + * Shared release check (issue #890): checks after launch and on foreground + * return, with a short process-local debounce so the chat screen can show an + * update banner without the user ever opening the About tab. * * The result lands in [AppUpdateCache] (consumed by the chat banner and * adopted by the About tab) and the latest tag is persisted via * [AuthManager.setLastKnownLatestTag] so a dismissed banner can return on a - * later launch. Failures leave the cache Idle — the About tab still has its - * own manual retry path. + * later launch. Failed checks remain visible and can be retried from the About + * tab or dialog without persisting a false successful-check marker. */ object UpdateNoticeManager { private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + private val checkLock = Any() + private var activeCheck: Job? = null + private var lastSuccessfulCheckAt: Long? = null + private var lastFailedCheckAt: Long? = null + private var channelGeneration = 0 /** * Release-only feature (issue #890): the launch check and chat banner are @@ -32,57 +40,153 @@ object UpdateNoticeManager { var enabled: Boolean = !BuildConfig.DEBUG internal set - /** Release check interval (24 hours). */ - const val CHECK_INTERVAL_MS: Long = 24 * 60 * 60 * 1000L + /** Avoid duplicate foreground checks in one process; a cold launch always rechecks. */ + const val CHECK_INTERVAL_MS: Long = 5 * 60 * 1000L + const val FAILURE_BACKOFF_MS: Long = 30 * 1000L - /** Run from Application.onCreate, after AuthManager.init. Checks at most once every 24h. */ + /** Run after preferences are ready; failed checks never suppress the next foreground retry. */ fun checkOnLaunch( checker: AppUpdateChecker = AppUpdateChecker(), currentVersion: String = BuildConfig.VERSION_NAME, ioDispatcher: CoroutineDispatcher = Dispatchers.IO, now: Long = System.currentTimeMillis(), + force: Boolean = false, ) { - if (!enabled) return - val lastCheck = AuthManager.getLastUpdateCheckTimestamp() - val versionChanged = AuthManager.getUpdateCheckDoneForVersion() != currentVersion - if (!versionChanged && (now - lastCheck < CHECK_INTERVAL_MS)) return - - // Captured once so the check and the notice path agree on the channel. - val includeReleaseCandidates = AuthManager.isCheckingReleaseCandidateUpdates() - - scope.launch(ioDispatcher) { - AuthManager.setUpdateCheckDoneForVersion(currentVersion) - AuthManager.setLastUpdateCheckTimestamp(now) - val result = - try { - checker.fetchLatestRelease(includeReleaseCandidates) - } catch (e: CancellationException) { - throw e - } catch (e: IOException) { - return@launch - } catch (e: Exception) { - return@launch - } - val info = result ?: return@launch - // The user may have switched back to stable while this request was in - // flight — an RC result must never be published on the stable channel. - if (isReleaseCandidateVersion(info.tagName) && !AuthManager.isCheckingReleaseCandidateUpdates()) { - return@launch + if (!enabled && !force) return + synchronized(checkLock) { + if (activeCheck?.isActive == true) return + if (!force && lastSuccessfulCheckAt?.let { now - it < CHECK_INTERVAL_MS } == true) return + if (!force && lastFailedCheckAt?.let { now - it < FAILURE_BACKOFF_MS } == true) return + val includeReleaseCandidates = AuthManager.isCheckingReleaseCandidateUpdates() + val generation = channelGeneration + val previousState = AppUpdateCache.state.value + if (force || + previousState !is AppUpdateState.UpdateAvailable + ) { + AppUpdateCache.update(AppUpdateState.Checking) } - val apk = info.apkAsset ?: return@launch - val state = - if (isNewerVersion(info.tagName, currentVersion)) { - AppUpdateState.UpdateAvailable( - latestTag = info.tagName, - apkUrl = apk.browserDownloadUrl, - sizeBytes = apk.size, - releaseNotes = info.body, - ) - } else { - AppUpdateState.UpToDate(latestTag = info.tagName) + activeCheck = + scope.launch(ioDispatcher) { + val thisJob = currentCoroutineContext()[Job] + var successful = false + var failed = false + try { + val info = checker.fetchLatestRelease(includeReleaseCandidates) + val apk = info?.apkAsset + val state = + if (info == null) { + AppUpdateState.Error(NO_RELEASE_ERROR, isCheckError = true) + } else if (apk == null) { + AppUpdateState.Error(NO_APK_ERROR, isCheckError = true) + } else if (isNewerVersion(info.tagName, currentVersion)) { + AppUpdateState.UpdateAvailable( + latestTag = info.tagName, + apkUrl = apk.browserDownloadUrl, + sizeBytes = apk.size, + releaseNotes = info.body, + ) + } else { + AppUpdateState.UpToDate(latestTag = info.tagName) + } + var resultRejected = false + synchronized(checkLock) { + if (activeCheck !== thisJob || generation != channelGeneration) return@synchronized + val currentChannel = AuthManager.isCheckingReleaseCandidateUpdates() + if (includeReleaseCandidates != currentChannel || + (info != null && isReleaseCandidateVersion(info.tagName) && !currentChannel) + ) { + if (AppUpdateCache.state.value is AppUpdateState.Checking) { + AppUpdateCache.update(previousState) + } + resultRejected = true + return@synchronized + } + AppUpdateCache.update(state) + if (state is AppUpdateState.UpdateAvailable || state is AppUpdateState.UpToDate) { + state.releaseTag()?.let { AuthManager.setLastKnownLatestTag(it) } + AuthManager.setUpdateCheckDoneForVersion(currentVersion) + AuthManager.setLastUpdateCheckTimestamp(System.currentTimeMillis()) + successful = true + } else { + failed = true + } + } + if (resultRejected) return@launch + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { + synchronized(checkLock) { + if (activeCheck === thisJob && generation == channelGeneration) { + failed = true + if (AppUpdateCache.state.value is AppUpdateState.Checking) { + AppUpdateCache.update(AppUpdateState.Error(NETWORK_ERROR, isCheckError = true)) + } + } + } + } catch (e: Exception) { + synchronized(checkLock) { + if (activeCheck === thisJob && generation == channelGeneration) { + failed = true + if (AppUpdateCache.state.value is AppUpdateState.Checking) { + AppUpdateCache.update( + AppUpdateState.Error(GENERIC_CHECK_ERROR, isCheckError = true), + ) + } + } + } + } finally { + synchronized(checkLock) { + if (activeCheck === thisJob) { + if (successful) lastSuccessfulCheckAt = now + if (failed) lastFailedCheckAt = now + activeCheck = null + } + } + } } - AppUpdateCache.update(state) - state.releaseTag()?.let { AuthManager.setLastKnownLatestTag(it) } + } + } + + private const val NETWORK_ERROR = "Network error — check your connection" + private const val GENERIC_CHECK_ERROR = "Couldn't check for updates" + private const val NO_RELEASE_ERROR = "No release found yet" + private const val NO_APK_ERROR = "Release has no APK asset" + + /** Explicit check action; shares in-flight work but bypasses the local debounce. */ + fun checkNow( + checker: AppUpdateChecker, + currentVersion: String, + ioDispatcher: CoroutineDispatcher, + ) { + checkOnLaunch(checker, currentVersion, ioDispatcher, force = true) + } + + /** Persist the channel choice and invalidate in-flight checks as one operation. */ + fun channelChanged(enabled: Boolean) { + synchronized(checkLock) { + AuthManager.setCheckReleaseCandidateUpdates(enabled) + channelGeneration++ + if (AppUpdateCache.state.value is AppUpdateState.Checking) { + AppUpdateCache.update(AppUpdateState.Idle) + } + activeCheck?.cancel() + activeCheck = null + lastSuccessfulCheckAt = null + lastFailedCheckAt = null + } + } + + /** Test hook: reset process-local admission state between tests. */ + internal fun resetForTests() { + synchronized(checkLock) { + activeCheck?.cancel() + activeCheck = null + if (AppUpdateCache.state.value is AppUpdateState.Checking) { + AppUpdateCache.update(AppUpdateState.Idle) + } + lastSuccessfulCheckAt = null + lastFailedCheckAt = null + channelGeneration++ } } @@ -94,12 +198,9 @@ object UpdateNoticeManager { fun noticeTag(currentVersion: String = BuildConfig.VERSION_NAME): String? { if (!enabled) return null val dismissed = AuthManager.getDismissedUpdateTag() - val candidateTag = - (AppUpdateCache.state.value as? AppUpdateState.UpdateAvailable)?.latestTag - ?: AuthManager.getLastKnownLatestTag() - ?: return null + val candidateTag = (AppUpdateCache.state.value as? AppUpdateState.UpdateAvailable)?.latestTag ?: return null - if (candidateTag == dismissed) return null + if (candidateTag == dismissed || AppUpdateCache.isDismissed(candidateTag)) return null if (isReleaseCandidateVersion(candidateTag) && !AuthManager.isCheckingReleaseCandidateUpdates()) { return null } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatAppUpdateSection.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatAppUpdateSection.kt index 2ec2e26b..c05fd7d0 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatAppUpdateSection.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatAppUpdateSection.kt @@ -7,6 +7,7 @@ import android.provider.Settings import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.platform.LocalContext import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleEventObserver @@ -17,6 +18,7 @@ import androidx.lifecycle.viewmodel.compose.viewModel import com.m57.hermescontrol.data.update.AppUpdateCache import com.m57.hermescontrol.data.update.AppUpdateState import com.m57.hermescontrol.data.update.UpdateNoticeManager +import com.m57.hermescontrol.data.update.releaseTag import com.m57.hermescontrol.ui.common.AppUpdateDialog import com.m57.hermescontrol.ui.common.UpdateNoticeBanner import com.m57.hermescontrol.ui.settings.AppUpdateViewModel @@ -32,14 +34,17 @@ fun ChatAppUpdateSection() { AppUpdateViewModel(app) } val appUpdateState by appUpdateViewModel.state.collectAsStateWithLifecycle() + val currentAppUpdateState by rememberUpdatedState(appUpdateState) val updateLifecycleOwner = LocalLifecycleOwner.current DisposableEffect(updateLifecycleOwner) { val observer = LifecycleEventObserver { _, event -> if (event == Lifecycle.Event.ON_RESUME) { - if (appUpdateState is AppUpdateState.NeedsUnknownSourcesPermission) { + if (currentAppUpdateState is AppUpdateState.NeedsUnknownSourcesPermission) { appUpdateViewModel.resumeInstallAfterPermission() + } else { + appUpdateViewModel.reconcileInstallerReturn() } } } @@ -49,15 +54,13 @@ fun ChatAppUpdateSection() { } } - if (UpdateNoticeManager.enabled && !AppUpdateCache.dismissed) { - val noticeTag = - (updateNotice as? AppUpdateState.UpdateAvailable)?.latestTag - ?: UpdateNoticeManager.noticeTag() + if (UpdateNoticeManager.enabled) { + val noticeTag = UpdateNoticeManager.noticeTag() if (noticeTag != null) { UpdateNoticeBanner( latestTag = noticeTag, onUpdate = { AppUpdateCache.showDialog() }, - onDismiss = { AppUpdateCache.dismiss() }, + onDismiss = { AppUpdateCache.dismiss(noticeTag) }, ) } } @@ -73,12 +76,17 @@ fun ChatAppUpdateSection() { AppUpdateDialog( state = dialogState, onDismiss = { AppUpdateCache.hideDialog() }, + onLater = { + dialogState.releaseTag()?.let(AppUpdateCache::dismiss) + AppUpdateCache.hideDialog() + }, onStartUpdate = { appUpdateViewModel.startUpdate() }, onCancelDownload = { appUpdateViewModel.cancelDownload() }, onNeverAskAgain = { appUpdateViewModel.dismissCurrentUpdate() }, onOpenSettings = { launchUnknownAppSourcesSettings(context) }, + onCheckUpdate = { appUpdateViewModel.checkForUpdate() }, ) } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/common/AppUpdateDialog.kt b/app/src/main/java/com/m57/hermescontrol/ui/common/AppUpdateDialog.kt index e69c0c21..5106ff4e 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/common/AppUpdateDialog.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/common/AppUpdateDialog.kt @@ -48,10 +48,12 @@ import com.m57.hermescontrol.data.update.releaseTag fun AppUpdateDialog( state: AppUpdateState, onDismiss: () -> Unit, + onLater: () -> Unit = onDismiss, onStartUpdate: () -> Unit, onCancelDownload: () -> Unit, onNeverAskAgain: () -> Unit, onOpenSettings: () -> Unit, + onCheckUpdate: () -> Unit = {}, ) { val available = state as? AppUpdateState.UpdateAvailable var cachedAvailable by remember { mutableStateOf(available) } @@ -160,6 +162,26 @@ fun AppUpdateDialog( // Progress / Action States when (state) { + AppUpdateState.Checking -> { + Column(modifier = Modifier.fillMaxWidth()) { + Text( + text = stringResource(R.string.settings_about_update_checking), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Spacer(modifier = Modifier.height(8.dp)) + LinearProgressIndicator(modifier = Modifier.fillMaxWidth()) + } + } + + is AppUpdateState.UpToDate -> { + Text( + text = stringResource(R.string.settings_about_update_uptodate, state.latestTag), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + is AppUpdateState.Downloading -> { Column(modifier = Modifier.fillMaxWidth()) { Row( @@ -260,7 +282,7 @@ fun AppUpdateDialog( ) Spacer(modifier = Modifier.height(8.dp)) Button( - onClick = onStartUpdate, + onClick = if (state.isCheckError) onCheckUpdate else onStartUpdate, modifier = Modifier.fillMaxWidth(), ) { Text(stringResource(R.string.settings_about_update_retry)) @@ -268,8 +290,7 @@ fun AppUpdateDialog( } } - else -> { - // UpdateAvailable or Idle + is AppUpdateState.UpdateAvailable -> { Column(modifier = Modifier.fillMaxWidth()) { Button( onClick = onStartUpdate, @@ -291,7 +312,7 @@ fun AppUpdateDialog( modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.SpaceBetween, ) { - TextButton(onClick = onDismiss) { + TextButton(onClick = onLater) { Text(stringResource(R.string.update_dialog_action_later)) } TextButton(onClick = onNeverAskAgain) { @@ -303,6 +324,12 @@ fun AppUpdateDialog( } } } + + AppUpdateState.Idle -> { + Button(onClick = onCheckUpdate, modifier = Modifier.fillMaxWidth()) { + Text(stringResource(R.string.settings_about_update_check)) + } + } } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModel.kt index b02aa97b..d7cf2d45 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModel.kt @@ -11,19 +11,19 @@ import com.m57.hermescontrol.data.local.AuthManager import com.m57.hermescontrol.data.update.AppUpdateCache import com.m57.hermescontrol.data.update.AppUpdateChecker import com.m57.hermescontrol.data.update.AppUpdateState -import com.m57.hermescontrol.data.update.isNewerVersion +import com.m57.hermescontrol.data.update.UpdateNoticeManager import com.m57.hermescontrol.data.update.isReleaseCandidateVersion import com.m57.hermescontrol.data.update.releaseTag import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch import java.io.File -import java.io.IOException +import java.util.UUID /** * Drives the in-app self-update flow (issue #867): check the GitHub @@ -31,9 +31,8 @@ import java.io.IOException * installer. The installer launch uses the application context * (FLAG_ACTIVITY_NEW_TASK), so no Activity is required. * - * A silent check runs once per installed version (guarded by the - * `updateCheckDoneForVersion` app pref) — that's what puts the "Update - * available" badge on the About row without the user tapping anything. + * The ViewModel adopts the process-shared launch/foreground check result, while + * manual checks, downloads, and system-installer handoff stay screen-owned. */ class AppUpdateViewModel( application: Application, @@ -51,16 +50,11 @@ class AppUpdateViewModel( /** Whether the RC update channel is enabled (issue: RC update toggle). */ val checkReleaseCandidateUpdates: StateFlow = _checkReleaseCandidateUpdates.asStateFlow() - private var checkJob: kotlinx.coroutines.Job? = null private var downloadJob: kotlinx.coroutines.Job? = null private var lastAvailable: AppUpdateState.UpdateAvailable? = null - - /** - * Bumped by every check. The blocking OkHttp call underneath cannot be - * interrupted, so a cancelled check may still be running; results are only - * published when this token is unchanged (and the channel still matches). - */ - private var checkGeneration = 0 + private var downloadGeneration = 0 + private var downloadPartialFile: File? = null + private var installerReturnPending = false init { val cached = AppUpdateCache.state.value @@ -70,13 +64,25 @@ class AppUpdateViewModel( !_checkReleaseCandidateUpdates.value && cached.releaseTag()?.let(::isReleaseCandidateVersion) == true if (blocked) AppUpdateCache.reset() + viewModelScope.launch { + AppUpdateCache.state.collect { sharedState -> + if (_state.value !is AppUpdateState.Downloading && + _state.value !is AppUpdateState.Installing && + _state.value !is AppUpdateState.NeedsUnknownSourcesPermission + ) { + _state.value = sharedState + if (sharedState is AppUpdateState.UpdateAvailable) lastAvailable = sharedState + } + } + } if (cached is AppUpdateState.UpdateAvailable && !blocked) { lastAvailable = cached _state.value = cached } else if (cached is AppUpdateState.UpToDate && !blocked) { _state.value = cached } else { - checkForUpdate() + _state.value = cached + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, ioDispatcher) } } @@ -94,92 +100,20 @@ class AppUpdateViewModel( */ fun setCheckReleaseCandidateUpdates(enabled: Boolean) { if (_checkReleaseCandidateUpdates.value == enabled) return - AuthManager.setCheckReleaseCandidateUpdates(enabled) + UpdateNoticeManager.channelChanged(enabled) _checkReleaseCandidateUpdates.value = enabled if (!enabled) { + if (lastAvailable?.latestTag?.let(::isReleaseCandidateVersion) == true) cancelDownload() lastAvailable = null AppUpdateCache.reset() } - // A check already in flight captured the previous channel — cancel it and - // bump the generation so a late result can never be published. - checkJob?.cancel() runCheck() } private fun runCheck() { - val generation = ++checkGeneration - val includeReleaseCandidates = _checkReleaseCandidateUpdates.value - _state.value = AppUpdateState.Checking - checkJob = - viewModelScope.launch(ioDispatcher) { - val now = System.currentTimeMillis() - AuthManager.setUpdateCheckDoneForVersion(currentVersion) - AuthManager.setLastUpdateCheckTimestamp(now) - val result = - try { - checker.fetchLatestRelease(includeReleaseCandidates) - } catch (e: CancellationException) { - // Cancellation is control flow — it must never be reported - // as a failed check. - throw e - } catch (e: IOException) { - if (isCurrentCheck(generation, includeReleaseCandidates)) { - _state.value = AppUpdateState.Error(NETWORK_ERROR) - } - return@launch - } catch (e: Exception) { - if (isCurrentCheck(generation, includeReleaseCandidates)) { - _state.value = AppUpdateState.Error(GENERIC_CHECK_ERROR) - } - return@launch - } - - // The OkHttp call underneath blocks and cannot be interrupted, so a - // newer check — or a channel switch — may have happened while it ran. - ensureActive() - if (!isCurrentCheck(generation, includeReleaseCandidates)) return@launch - - val info = - result ?: run { - _state.value = AppUpdateState.Error(NO_RELEASE_ERROR) - return@launch - } - val apk = - info.apkAsset ?: run { - _state.value = AppUpdateState.Error(NO_APK_ERROR) - return@launch - } - val state = - if (isNewerVersion(info.tagName, currentVersion)) { - AppUpdateState.UpdateAvailable( - latestTag = info.tagName, - apkUrl = apk.browserDownloadUrl, - sizeBytes = apk.size, - releaseNotes = info.body, - ) - } else { - AppUpdateState.UpToDate(latestTag = info.tagName) - } - // Fail closed: a release-candidate result is never published once - // the RC channel is off, however it reached this point. - if (!isAdmissibleOnCurrentChannel(state.releaseTag())) return@launch - if (state is AppUpdateState.UpdateAvailable) lastAvailable = state - _state.value = state - // Keep the launch notice (issue #890) in sync with manual checks. - AppUpdateCache.update(state) - state.releaseTag()?.let { AuthManager.setLastKnownLatestTag(it) } - } + UpdateNoticeManager.checkNow(checker, currentVersion, ioDispatcher) } - /** - * True when [generation] is still the newest check *and* the channel that - * check was started for is still the selected one. - */ - private fun isCurrentCheck( - generation: Int, - includeReleaseCandidates: Boolean, - ): Boolean = generation == checkGeneration && includeReleaseCandidates == _checkReleaseCandidateUpdates.value - /** A release-candidate tag may only be used while the RC channel is on. */ private fun isAdmissibleOnCurrentChannel(tag: String?): Boolean = tag == null || !isReleaseCandidateVersion(tag) || _checkReleaseCandidateUpdates.value @@ -208,36 +142,70 @@ class AppUpdateViewModel( /** Download the release APK and launch the system installer. */ fun startUpdate() { + if (_state.value is AppUpdateState.Downloading || _state.value is AppUpdateState.Installing) return val available = updateAvailableForInstall() ?: return lastAvailable = available if (!canRequestInstalls()) { _state.value = AppUpdateState.NeedsUnknownSourcesPermission return } - val dest = File(getApplication().cacheDir, APK_FILE_NAME) - _state.value = AppUpdateState.Downloading(0f) - downloadJob?.cancel() - downloadJob = - viewModelScope.launch(ioDispatcher) { - val downloaded = - checker.downloadApk(available.apkUrl, dest) { progress -> - _state.value = AppUpdateState.Downloading(progress) + val dest = updateApkFile(available.latestTag) + val generation = ++downloadGeneration + val partial = File(dest.parentFile, "${dest.name}.${UUID.randomUUID()}.$generation.part") + downloadPartialFile = partial + try { + partial.delete() + _state.value = AppUpdateState.Downloading(0f) + downloadJob?.cancel() + downloadJob = + viewModelScope.launch(ioDispatcher) { + try { + val downloaded = + checker.downloadApk(available.apkUrl, partial) { progress -> + if (generation == + downloadGeneration + ) { + _state.value = AppUpdateState.Downloading(progress) + } + } + if (generation != downloadGeneration) return@launch + if (!downloaded || !partial.exists() || partial.length() == 0L) { + _state.value = AppUpdateState.Error(DOWNLOAD_ERROR) + return@launch + } + if ((dest.exists() && !dest.delete()) || !partial.renameTo(dest)) { + _state.value = AppUpdateState.Error(DOWNLOAD_ERROR) + return@launch + } + _state.value = AppUpdateState.Installing(available.latestTag) + launchInstaller(dest) + } catch (e: CancellationException) { + throw e + } catch (_: Exception) { + if (generation == downloadGeneration) _state.value = AppUpdateState.Error(DOWNLOAD_ERROR) + } finally { + if (generation == downloadGeneration) { + partial.delete() + downloadPartialFile = null + } } - if (!downloaded) { - if (downloadJob?.isCancelled != true) { - _state.value = AppUpdateState.Error(DOWNLOAD_ERROR) - } - return@launch } - _state.value = AppUpdateState.Installing(available.latestTag) - launchInstaller(dest) + } catch (_: Exception) { + if (generation == downloadGeneration) { + partial.delete() + downloadPartialFile = null + _state.value = AppUpdateState.Error(DOWNLOAD_ERROR) } + } } /** Cancel in-flight download and return to UpdateAvailable. */ fun cancelDownload() { + downloadGeneration++ downloadJob?.cancel() downloadJob = null + downloadPartialFile?.delete() + downloadPartialFile = null val available = lastAvailable ?: (AppUpdateCache.state.value as? AppUpdateState.UpdateAvailable) @@ -257,16 +225,15 @@ class AppUpdateViewModel( if (tag != null) { AuthManager.setDismissedUpdateTag(tag) } - AppUpdateCache.dismiss() AppUpdateCache.hideDialog() } /** Resume install when returning from Unknown Sources permission screen. */ fun resumeInstallAfterPermission() { if (canRequestInstalls()) { - val dest = File(getApplication().cacheDir, APK_FILE_NAME) val available = updateAvailableForInstall() - if (dest.exists() && dest.length() > 0 && available != null) { + val dest = available?.let { updateApkFile(it.latestTag) } + if (dest != null && dest.exists() && dest.length() > 0L) { _state.value = AppUpdateState.Installing(available.latestTag) launchInstaller(dest) } else { @@ -275,6 +242,18 @@ class AppUpdateViewModel( } } + /** Returning from the external installer is not proof that installation succeeded. */ + fun reconcileInstallerReturn() { + if (_state.value !is AppUpdateState.Installing || !installerReturnPending) return + installerReturnPending = false + _state.value = lastAvailable ?: AppUpdateCache.state.value + } + + private fun updateApkFile(tag: String): File { + val safeTag = tag.replace(Regex("[^A-Za-z0-9._-]"), "_") + return File(getApplication().cacheDir, "hermes-update-$safeTag.apk") + } + private fun canRequestInstalls(): Boolean = try { getApplication().packageManager.canRequestPackageInstalls() @@ -292,18 +271,14 @@ class AppUpdateViewModel( apkFile, ) context.startActivity(installIntentFactory(uri)) + installerReturnPending = true } catch (e: Exception) { + installerReturnPending = false _state.value = AppUpdateState.Error(INSTALLER_ERROR) } } private companion object { - const val APK_FILE_NAME = "hermes-update.apk" - - val NETWORK_ERROR = "Network error — check your connection" - val GENERIC_CHECK_ERROR = "Couldn't check for updates" - val NO_RELEASE_ERROR = "No release found yet" - val NO_APK_ERROR = "Release has no APK asset" val DOWNLOAD_ERROR = "Download failed — tap to retry" val INSTALLER_ERROR = "Couldn't open the installer" } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsSubPages.kt b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsSubPages.kt index 8d30bfaf..75344ee2 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsSubPages.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/settings/SettingsSubPages.kt @@ -262,6 +262,7 @@ internal fun SettingsAboutPage( }, ) { val updateState by viewModel.state.collectAsStateWithLifecycle() + val currentUpdateState by androidx.compose.runtime.rememberUpdatedState(updateState) val checkReleaseCandidateUpdates by viewModel.checkReleaseCandidateUpdates.collectAsStateWithLifecycle() val context = LocalContext.current val lifecycleOwner = androidx.lifecycle.compose.LocalLifecycleOwner.current @@ -270,8 +271,12 @@ internal fun SettingsAboutPage( val observer = androidx.lifecycle.LifecycleEventObserver { _, event -> if (event == androidx.lifecycle.Lifecycle.Event.ON_RESUME) { - if (updateState is com.m57.hermescontrol.data.update.AppUpdateState.NeedsUnknownSourcesPermission) { + if (currentUpdateState is + com.m57.hermescontrol.data.update.AppUpdateState.NeedsUnknownSourcesPermission + ) { viewModel.resumeInstallAfterPermission() + } else { + viewModel.reconcileInstallerReturn() } } } diff --git a/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerNetworkTest.kt b/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerNetworkTest.kt index 4bd6a811..e149cfcc 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerNetworkTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerNetworkTest.kt @@ -1,14 +1,21 @@ package com.m57.hermescontrol.data.update +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import okhttp3.OkHttpClient import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.SocketPolicy import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test +import java.io.File +import java.util.concurrent.TimeUnit /** * Endpoint contract for the opt-in release-candidate channel: the stable path @@ -38,8 +45,11 @@ class AppUpdateCheckerNetworkTest { private fun enqueue( body: String, code: Int = 200, + headers: Map = emptyMap(), ) { - server.enqueue(MockResponse().setResponseCode(code).setBody(body)) + val response = MockResponse().setResponseCode(code).setBody(body) + headers.forEach(response::addHeader) + server.enqueue(response) } @Test @@ -56,26 +66,62 @@ class AppUpdateCheckerNetworkTest { @Test fun fetchLatestRelease_releaseCandidatesScanTheReleasesList() = runTest { + enqueue(STABLE_BODY) enqueue(RELEASES_BODY) val info = checker().fetchLatestRelease(includeReleaseCandidates = true) assertEquals("v1.25.0-rc.3", info?.tagName) - assertEquals("/repos/Hy4ri/hermes-mobile/releases?per_page=20", server.takeRequest().path) + assertEquals("/repos/Hy4ri/hermes-mobile/releases/latest", server.takeRequest().path) + assertEquals("/repos/Hy4ri/hermes-mobile/releases?per_page=100&page=1", server.takeRequest().path) } @Test - fun fetchLatestRelease_releaseCandidatesStillIgnoreAlphaAndBeta() = + fun fetchLatestRelease_stableFallsBackToReleasesWhenLatestIsNotUsable() = runTest { + enqueue("{}") + enqueue(RELEASES_BODY) + + val info = checker().fetchLatestRelease() + + assertEquals("v1.24.2", info?.tagName) + assertEquals("/repos/Hy4ri/hermes-mobile/releases/latest", server.takeRequest().path) + assertEquals("/repos/Hy4ri/hermes-mobile/releases?per_page=100&page=1", server.takeRequest().path) + } + + @Test + fun fetchLatestRelease_rcScanFollowsGithubPagination() = + runTest { + enqueue(STABLE_BODY) + val nextPage = server.url("/repos/Hy4ri/hermes-mobile/releases?per_page=100&page=2") + enqueue(RELEASES_BODY, headers = mapOf("Link" to "<$nextPage>; rel=\"next\"")) + enqueue( + """[{"tag_name":"v1.26.0-rc.1","prerelease":true,"assets":[{"name":"release.apk","size":5,"browser_download_url":"https://example.com/new.apk"}]}]""", + ) + + val info = checker().fetchLatestRelease(includeReleaseCandidates = true) + + assertEquals("v1.26.0-rc.1", info?.tagName) + assertEquals("/repos/Hy4ri/hermes-mobile/releases/latest", server.takeRequest().path) + assertEquals("/repos/Hy4ri/hermes-mobile/releases?per_page=100&page=1", server.takeRequest().path) + assertEquals("/repos/Hy4ri/hermes-mobile/releases?per_page=100&page=2", server.takeRequest().path) + } + + @Test + fun fetchLatestRelease_releaseCandidatesStillIgnoreAlphaAndBetaButKeepStable() = + runTest { + enqueue(STABLE_BODY) enqueue(ALPHA_BETA_BODY) - assertNull(checker().fetchLatestRelease(includeReleaseCandidates = true)) + assertEquals("v1.24.2", checker().fetchLatestRelease(includeReleaseCandidates = true)?.tagName) } @Test fun fetchLatestRelease_nonSuccessYieldsNull() = runTest { - // One queued response per request: stable path, then RC path. + // Each channel checks the stable endpoint and falls back to the list on 404. + enqueue("", code = 404) + enqueue("", code = 404) enqueue("", code = 404) enqueue("", code = 404) @@ -83,6 +129,35 @@ class AppUpdateCheckerNetworkTest { assertNull(checker().fetchLatestRelease(includeReleaseCandidates = true)) } + @Test + fun downloadApk_cancellingCoroutineCancelsNetworkAndDeletesPartialFile() = + runTest { + server.enqueue(MockResponse().setSocketPolicy(SocketPolicy.NO_RESPONSE)) + val destination = File.createTempFile("hermes-update", ".apk") + destination.writeText("partial") + val download = + launch(Dispatchers.IO) { + checker().downloadApk(server.url("/slow.apk").toString(), destination) {} + } + + assertEquals("/slow.apk", server.takeRequest(5, TimeUnit.SECONDS)?.path) + download.cancelAndJoin() + + assertFalse(destination.exists()) + } + + @Test + fun downloadApk_invalidUrlReturnsFailureAndDeletesDestination() = + runTest { + val destination = File.createTempFile("hermes-update", ".apk") + destination.writeText("partial") + + val downloaded = checker().downloadApk("file:///tmp/release.apk", destination) {} + + assertFalse(downloaded) + assertFalse(destination.exists()) + } + private companion object { const val STABLE_BODY = """ { diff --git a/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerTest.kt b/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerTest.kt index 7efd13f5..9d130e10 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/update/AppUpdateCheckerTest.kt @@ -183,6 +183,7 @@ class AppUpdateCheckerTest { draft: Boolean = false, prerelease: Boolean = false, withApk: Boolean = true, + apkUrl: String = "https://example.com/$tag.apk", ): UpdateInfo = UpdateInfo( tagName = tag, @@ -194,7 +195,7 @@ class AppUpdateCheckerTest { UpdateInfo.Asset( name = "hermes-mobile-$tag.apk", size = 1L, - browserDownloadUrl = "https://example.com/$tag.apk", + browserDownloadUrl = apkUrl, ), ) } else { @@ -289,6 +290,55 @@ class AppUpdateCheckerTest { assertNull(selectLatestUpdate(listOf(release("v1.25.0", draft = true), release("v1.24.0", withApk = false)))) } + @Test + fun selectLatestUpdate_rejectsPrereleaseTagsMislabelledAsStable() { + val releases = + listOf( + release("v1.30.0-rc.1"), + release("v1.29.0-alpha.1"), + release("v1.28.0-beta.2"), + release("v1.27.0"), + ) + + assertEquals("v1.27.0", selectLatestUpdate(releases)?.tagName) + assertEquals("v1.30.0-rc.1", selectLatestUpdate(releases, includeReleaseCandidates = true)?.tagName) + } + + @Test + fun selectLatestUpdate_ignoresInvalidTagsAndUnusableApkUrls() { + val releases = + listOf( + release("not-a-version"), + release("v1.30.0", apkUrl = " "), + release("v1.29.0"), + ) + + assertEquals("v1.29.0", selectLatestUpdate(releases)?.tagName) + } + + @Test + fun selectLatestUpdate_rejectsMalformedAndUnsupportedApkUrlsButAllowsHttp() { + val releases = + listOf( + release("v1.32.0", apkUrl = "https://bad host/release.apk"), + release("v1.31.0", apkUrl = "file:///tmp/release.apk"), + release("v1.30.0", apkUrl = "http://localhost/release.apk"), + ) + + assertEquals("v1.30.0", selectLatestUpdate(releases)?.tagName) + } + + @Test + fun selectLatestUpdate_stableOutranksRcForSameVersion() { + val releases = + listOf( + release("v1.25.0-rc.10", prerelease = true), + release("v1.25.0"), + ) + + assertEquals("v1.25.0", selectLatestUpdate(releases, includeReleaseCandidates = true)?.tagName) + } + @Test fun parseReleaseList_readsDraftAndPrereleaseFlags() { val json = diff --git a/app/src/test/java/com/m57/hermescontrol/data/update/UpdateNoticeManagerTest.kt b/app/src/test/java/com/m57/hermescontrol/data/update/UpdateNoticeManagerTest.kt index 06195d26..a8f2cabf 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/update/UpdateNoticeManagerTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/update/UpdateNoticeManagerTest.kt @@ -49,6 +49,7 @@ class UpdateNoticeManagerTest { @Before fun setUp() { Dispatchers.setMain(testDispatcher) + UpdateNoticeManager.resetForTests() AppUpdateCache.reset() // Unit tests run against the debug variant where the feature is // disabled by default — flip the knob on so the behavior is testable. @@ -68,6 +69,7 @@ class UpdateNoticeManagerTest { @After fun tearDown() { + UpdateNoticeManager.resetForTests() UpdateNoticeManager.enabled = false unmockkAll() Dispatchers.resetMain() @@ -92,7 +94,7 @@ class UpdateNoticeManagerTest { } @Test - fun checkOnLaunch_skipsWhenAlreadyCheckedWithin24Hours() = + fun checkOnLaunch_ignoresPersistedDailyCheckTimestampAfterColdStart() = runTest { val now = 1000000000000L every { AuthManager.getUpdateCheckDoneForVersion() } returns currentVersion @@ -103,9 +105,24 @@ class UpdateNoticeManagerTest { UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher, now = now) advanceUntilIdle() - coVerify(exactly = 0) { checker.fetchLatestRelease() } - assertEquals(AppUpdateState.Idle, AppUpdateCache.state.value) - verify(exactly = 0) { AuthManager.setUpdateCheckDoneForVersion(currentVersion) } + coVerify(exactly = 1) { checker.fetchLatestRelease(false) } + assertTrue(AppUpdateCache.state.value is AppUpdateState.UpdateAvailable) + verify(exactly = 1) { AuthManager.setUpdateCheckDoneForVersion(currentVersion) } + } + + @Test + fun checkOnLaunch_debouncesRepeatedForegroundChecksAfterSuccess() = + runTest { + val checker = mockk() + coEvery { checker.fetchLatestRelease(false) } returns updateInfo() + val now = 1_000_000L + + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher, now) + advanceUntilIdle() + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher, now + 1_000L) + advanceUntilIdle() + + coVerify(exactly = 1) { checker.fetchLatestRelease(false) } } @Test @@ -148,9 +165,9 @@ class UpdateNoticeManagerTest { } @Test - fun noticeTag_stableReleasePromptsDotRcFromPersistedCache() { + fun noticeTag_doesNotUsePersistedTagForStableInstall() { every { AuthManager.getLastKnownLatestTag() } returns "v1.25" - assertEquals("v1.25", UpdateNoticeManager.noticeTag("1.25.rc.1")) + assertNull(UpdateNoticeManager.noticeTag("1.25.rc.1")) assertNull(UpdateNoticeManager.noticeTag("1.25")) } @@ -176,9 +193,11 @@ class UpdateNoticeManagerTest { UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher) advanceUntilIdle() - assertEquals(AppUpdateState.Idle, AppUpdateCache.state.value) - // Still marked done so a dead network can't spam the API per launch. - verify(exactly = 1) { AuthManager.setUpdateCheckDoneForVersion(currentVersion) } + assertEquals( + AppUpdateState.Error("Network error — check your connection", isCheckError = true), + AppUpdateCache.state.value, + ) + verify(exactly = 0) { AuthManager.setUpdateCheckDoneForVersion(currentVersion) } } @Test @@ -190,8 +209,41 @@ class UpdateNoticeManagerTest { UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher) advanceUntilIdle() + assertEquals(AppUpdateState.Error("No release found yet", isCheckError = true), AppUpdateCache.state.value) + assertFalse(AppUpdateCache.isDismissed("v1.22.0")) + } + + @Test + fun checkOnLaunch_noReleaseDoesNotPersistSuccessfulCheck() = + runTest { + val checker = mockk() + coEvery { checker.fetchLatestRelease(false) } returns null + + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher) + advanceUntilIdle() + + verify(exactly = 0) { AuthManager.setUpdateCheckDoneForVersion(any()) } + verify(exactly = 0) { AuthManager.setLastUpdateCheckTimestamp(any()) } + } + + @Test + fun checkOnLaunch_channelInvalidationDuringFetchPreservesNewChannelState() = + runTest { + every { AuthManager.isCheckingReleaseCandidateUpdates() } returns true + val checker = mockk() + coEvery { checker.fetchLatestRelease(true) } coAnswers { + // A channel change invalidates the captured request while it is completing. + UpdateNoticeManager.channelChanged(enabled = false) + every { AuthManager.isCheckingReleaseCandidateUpdates() } returns false + updateInfo(tag = "v1.25.0-rc.3") + } + + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher) + advanceUntilIdle() + assertEquals(AppUpdateState.Idle, AppUpdateCache.state.value) - assertFalse(AppUpdateCache.dismissed) + verify(exactly = 0) { AuthManager.setLastKnownLatestTag(any()) } + verify(exactly = 0) { AuthManager.setUpdateCheckDoneForVersion(any()) } } // ── noticeTag (dismissed/restart banner return) ───────────────────── @@ -211,10 +263,10 @@ class UpdateNoticeManagerTest { } @Test - fun noticeTag_returnsPersistedNewerTagWhenCacheIdle() { + fun noticeTag_doesNotAdvertisePersistedTagWithoutFreshApkMetadata() { every { AuthManager.getLastKnownLatestTag() } returns "v1.22.0" - assertEquals("v1.22.0", UpdateNoticeManager.noticeTag(currentVersion)) + assertNull(UpdateNoticeManager.noticeTag(currentVersion)) } @Test @@ -303,11 +355,11 @@ class UpdateNoticeManagerTest { } @Test - fun noticeTag_advertisesPersistedRcTagWhenOptedIn() { + fun noticeTag_doesNotUsePersistedRcTagWithoutApkMetadata() { every { AuthManager.getLastKnownLatestTag() } returns "v1.25.0-rc.3" every { AuthManager.isCheckingReleaseCandidateUpdates() } returns true - assertEquals("v1.25.0-rc.3", UpdateNoticeManager.noticeTag(currentVersion)) + assertNull(UpdateNoticeManager.noticeTag(currentVersion)) } @Test @@ -336,10 +388,50 @@ class UpdateNoticeManagerTest { } UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher) + UpdateNoticeManager.channelChanged(enabled = false) + every { AuthManager.isCheckingReleaseCandidateUpdates() } returns false advanceUntilIdle() - // The late RC result must not reach the cache or the persisted tag. + // A genuine manager-mediated opt-out invalidates the in-flight RC result. assertEquals(AppUpdateState.Idle, AppUpdateCache.state.value) verify(exactly = 0) { AuthManager.setLastKnownLatestTag(any()) } } + + @Test + fun checkOnLaunch_retriesAfterFailureBackoffInsteadOfPersistingFailureAsDone() = + runTest { + val checker = mockk() + coEvery { checker.fetchLatestRelease(false) } throws IOException("offline") andThen updateInfo() + val now = 100_000L + + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher, now) + advanceUntilIdle() + UpdateNoticeManager.checkOnLaunch(checker, currentVersion, testDispatcher, now + 1_000L) + advanceUntilIdle() + UpdateNoticeManager.checkOnLaunch( + checker, + currentVersion, + testDispatcher, + now + UpdateNoticeManager.FAILURE_BACKOFF_MS + 1L, + ) + advanceUntilIdle() + + coVerify(exactly = 2) { checker.fetchLatestRelease(false) } + assertTrue(AppUpdateCache.state.value is AppUpdateState.UpdateAvailable) + } + + @Test + fun noticeTag_dismissalAppliesOnlyToThatRelease() { + AppUpdateCache.update( + AppUpdateState.UpdateAvailable("v1.22.0", "https://example.com/1.apk", 1L), + ) + AppUpdateCache.dismiss("v1.22.0") + + assertNull(UpdateNoticeManager.noticeTag(currentVersion)) + + AppUpdateCache.update( + AppUpdateState.UpdateAvailable("v1.23.0", "https://example.com/2.apk", 1L), + ) + assertEquals("v1.23.0", UpdateNoticeManager.noticeTag(currentVersion)) + } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModelTest.kt index 3c0079bd..40a0f377 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/settings/AppUpdateViewModelTest.kt @@ -10,12 +10,14 @@ import com.m57.hermescontrol.data.update.AppUpdateCache import com.m57.hermescontrol.data.update.AppUpdateChecker import com.m57.hermescontrol.data.update.AppUpdateState import com.m57.hermescontrol.data.update.UpdateInfo +import com.m57.hermescontrol.data.update.UpdateNoticeManager import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.mockkObject import io.mockk.mockkStatic +import io.mockk.slot import io.mockk.unmockkAll import io.mockk.verify import kotlinx.coroutines.CompletableDeferred @@ -48,6 +50,7 @@ class AppUpdateViewModelTest { private lateinit var app: Application private lateinit var packageManager: PackageManager private lateinit var checker: AppUpdateChecker + private var rcUpdatesEnabled = false private val currentVersion = "1.21.0" @@ -70,6 +73,8 @@ class AppUpdateViewModelTest { @Before fun setUp() { Dispatchers.setMain(testDispatcher) + UpdateNoticeManager.resetForTests() + UpdateNoticeManager.enabled = true AppUpdateCache.reset() mockkObject(AuthManager) every { AuthManager.getUpdateCheckDoneForVersion() } returns null @@ -79,8 +84,12 @@ class AppUpdateViewModelTest { every { AuthManager.setLastUpdateCheckTimestamp(any()) } returns Unit every { AuthManager.getDismissedUpdateTag() } returns null every { AuthManager.setDismissedUpdateTag(any()) } returns Unit - every { AuthManager.isCheckingReleaseCandidateUpdates() } returns false - every { AuthManager.setCheckReleaseCandidateUpdates(any()) } returns Unit + rcUpdatesEnabled = false + every { AuthManager.isCheckingReleaseCandidateUpdates() } answers { rcUpdatesEnabled } + every { AuthManager.setCheckReleaseCandidateUpdates(any()) } answers { + rcUpdatesEnabled = firstArg() + Unit + } app = mockk(relaxed = true) every { app.cacheDir } returns File(System.getProperty("java.io.tmpdir")) @@ -91,11 +100,17 @@ class AppUpdateViewModelTest { checker = mockk() coEvery { checker.fetchLatestRelease() } returns updateInfo() - coEvery { checker.downloadApk(any(), any(), any()) } returns true + val downloadDestination = slot() + coEvery { checker.downloadApk(any(), capture(downloadDestination), any()) } coAnswers { + downloadDestination.captured.writeBytes(byteArrayOf(1)) + true + } } @After fun tearDown() { + UpdateNoticeManager.resetForTests() + UpdateNoticeManager.enabled = false unmockkAll() Dispatchers.resetMain() } @@ -237,14 +252,14 @@ class AppUpdateViewModelTest { } @Test - fun checkForUpdate_marksDoneEvenWhenCheckFails() = + fun checkForUpdate_marksDoneOnlyAfterSuccessfulDiscovery() = runTest { coEvery { checker.fetchLatestRelease() } throws IOException("boom") val vm = createViewModel() advanceUntilIdle() - coVerify(exactly = 1) { AuthManager.setUpdateCheckDoneForVersion(currentVersion) } + coVerify(exactly = 0) { AuthManager.setUpdateCheckDoneForVersion(currentVersion) } } // ── Update flow ───────────────────────────────────────────────────── @@ -351,7 +366,7 @@ class AppUpdateViewModelTest { advanceUntilIdle() verify(exactly = 1) { AuthManager.setDismissedUpdateTag("v1.22.0") } - assertTrue(AppUpdateCache.dismissed) + assertFalse(AppUpdateCache.isDismissed("v1.22.0")) assertFalse(AppUpdateCache.isDialogVisible) } @@ -392,7 +407,7 @@ class AppUpdateViewModelTest { @Test fun init_adoptsCachedRcOfferWhenOptedIn() = runTest { - every { AuthManager.isCheckingReleaseCandidateUpdates() } returns true + rcUpdatesEnabled = true AppUpdateCache.update( AppUpdateState.UpdateAvailable("v1.25.0-rc.3", "https://example.com/rc.apk", 1L), ) @@ -419,6 +434,7 @@ class AppUpdateViewModelTest { advanceUntilIdle() assertTrue(vm.checkReleaseCandidateUpdates.value) + verify(exactly = 1) { UpdateNoticeManager.channelChanged(true) } verify(exactly = 1) { AuthManager.setCheckReleaseCandidateUpdates(true) } coVerify(exactly = 1) { checker.fetchLatestRelease(true) } assertEquals("v1.25.0-rc.3", (vm.state.value as AppUpdateState.UpdateAvailable).latestTag) @@ -435,13 +451,14 @@ class AppUpdateViewModelTest { advanceUntilIdle() verify(exactly = 0) { AuthManager.setCheckReleaseCandidateUpdates(any()) } + verify(exactly = 0) { UpdateNoticeManager.channelChanged(any()) } coVerify(exactly = 1) { checker.fetchLatestRelease(false) } } @Test fun setCheckReleaseCandidateUpdates_offDropsStaleRcBeforeCheckingStable() = runTest { - every { AuthManager.isCheckingReleaseCandidateUpdates() } returns true + rcUpdatesEnabled = true coEvery { checker.fetchLatestRelease(true) } returns updateInfo(tag = "v1.25.0-rc.3") val vm = createViewModel() @@ -487,7 +504,7 @@ class AppUpdateViewModelTest { @Test fun lateRcResultAfterOptOut_neverPublishesRcOrInstalls() = runTest { - every { AuthManager.isCheckingReleaseCandidateUpdates() } returns true + rcUpdatesEnabled = true val rcGate = CompletableDeferred() coEvery { checker.fetchLatestRelease(true) } coAnswers { withContext(NonCancellable) { rcGate.await() }