From e3f45cf7fbf472ec315ccb04c569463aac6c2aac Mon Sep 17 00:00:00 2001 From: Titouan Thibaud Date: Wed, 27 May 2026 00:50:26 +0200 Subject: [PATCH 1/2] Add Treeherder install history Add a History screen reachable from the home top bar that lists Treeherder APKs whose installer was opened. History entries persist the try metadata, job metadata, artifact URL, and cache-relative path so the screen can show install context and navigate back to the revision search. Record installer launches from both Treeherder search and Treeherder user flows. The History screen resolves each entry against the local Treeherder cache so cached APKs show an Install action immediately, while missing APKs can be downloaded from the stored artifact URL and then installed. Move APK downloads into a files-backed download-cache with legacy cache migration and FileProvider support. Exclude the new download cache from backup/transfer. Harden APK downloads by writing to .part files, preserving an existing final APK with backup files during replacement, restoring backups after interrupted replacements, and ignoring unmanaged backup suffixes. Tests cover history state resolution, installer launch recording, legacy cache migration, and the download replacement/recovery paths. --- .../tryfox/data/FakeHistoryRepository.kt | 27 ++ .../tryfox/ui/screens/HistoryScreenTest.kt | 81 +++++ .../tryfox/ui/screens/ProfileScreenTest.kt | 4 + .../ui/screens/TreeherderApksScreenTest.kt | 2 + .../main/java/org/mozilla/tryfox/AppRoutes.kt | 1 + .../java/org/mozilla/tryfox/MainActivity.kt | 18 ++ .../org/mozilla/tryfox/TryFoxViewModel.kt | 145 ++++++++- .../data/TreeherderInstallHistoryEntry.kt | 24 ++ .../data/managers/DefaultCacheManager.kt | 54 +++- .../DefaultDownloadFileRepository.kt | 133 ++++++++- .../repositories/DefaultHistoryRepository.kt | 193 ++++++++++++ .../data/repositories/HistoryRepository.kt | 12 + .../java/org/mozilla/tryfox/di/AppModule.kt | 12 +- .../tryfox/ui/models/HistoryItemUiModel.kt | 9 + .../tryfox/ui/screens/HistoryScreen.kt | 277 ++++++++++++++++++ .../tryfox/ui/screens/HistoryViewModel.kt | 230 +++++++++++++++ .../mozilla/tryfox/ui/screens/HomeScreen.kt | 9 + .../tryfox/ui/screens/ProfileViewModel.kt | 72 ++++- .../tryfox/ui/screens/TreeherderApksScreen.kt | 17 ++ app/src/main/res/values/strings.xml | 6 + app/src/main/res/xml/backup_rules.xml | 3 +- .../main/res/xml/data_extraction_rules.xml | 8 +- app/src/main/res/xml/provider_paths.xml | 3 +- .../org/mozilla/tryfox/TryFoxViewModelTest.kt | 74 ++++- .../tryfox/data/FakeHistoryRepository.kt | 34 +++ .../data/managers/DefaultCacheManagerTest.kt | 38 +++ .../DefaultDownloadFileRepositoryTest.kt | 231 +++++++++++++++ .../tryfox/ui/screens/HistoryViewModelTest.kt | 246 ++++++++++++++++ .../tryfox/ui/screens/ProfileViewModelTest.kt | 14 +- 29 files changed, 1943 insertions(+), 34 deletions(-) create mode 100644 app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt create mode 100644 app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt create mode 100644 app/src/main/java/org/mozilla/tryfox/data/TreeherderInstallHistoryEntry.kt create mode 100644 app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt create mode 100644 app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt create mode 100644 app/src/main/java/org/mozilla/tryfox/ui/models/HistoryItemUiModel.kt create mode 100644 app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt create mode 100644 app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt create mode 100644 app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt create mode 100644 app/src/test/java/org/mozilla/tryfox/data/managers/DefaultCacheManagerTest.kt create mode 100644 app/src/test/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepositoryTest.kt create mode 100644 app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt diff --git a/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt b/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt new file mode 100644 index 0000000..0b4bde4 --- /dev/null +++ b/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt @@ -0,0 +1,27 @@ +package org.mozilla.tryfox.data + +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import org.mozilla.tryfox.data.repositories.HistoryRepository + +class FakeHistoryRepository : HistoryRepository { + private val _historyEntries = MutableStateFlow>(emptyList()) + override val historyEntries: StateFlow> = _historyEntries.asStateFlow() + + val recordedEntries = mutableListOf() + + override suspend fun refresh() = Unit + + override suspend fun recordInstallerLaunch(entry: TreeherderInstallHistoryEntry) { + recordedEntries.removeAll { it.uniqueKey == entry.uniqueKey } + recordedEntries.add(entry) + _historyEntries.value = recordedEntries.sortedByDescending { it.lastInstallerLaunchTimestamp } + } + + fun setEntries(entries: List) { + recordedEntries.clear() + recordedEntries.addAll(entries) + _historyEntries.value = entries + } +} diff --git a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt new file mode 100644 index 0000000..4378180 --- /dev/null +++ b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt @@ -0,0 +1,81 @@ +package org.mozilla.tryfox.ui.screens + +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.tryfox.data.FakeCacheManager +import org.mozilla.tryfox.data.FakeDownloadFileRepository +import org.mozilla.tryfox.data.FakeHistoryRepository +import org.mozilla.tryfox.data.FakeIntentManager +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry +import org.mozilla.tryfox.ui.theme.TryFoxTheme + +@RunWith(AndroidJUnit4::class) +class HistoryScreenTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @Test + fun clickingRevisionInvokesTreeherderRevisionNavigation() { + val revision = "abcdef1234567890" + val historyRepository = FakeHistoryRepository().apply { + setEntries(listOf(historyEntry(project = "mozilla-central", revision = revision))) + } + val historyViewModel = HistoryViewModel( + historyRepository = historyRepository, + downloadFileRepository = FakeDownloadFileRepository(), + cacheManager = FakeCacheManager(), + intentManager = FakeIntentManager(), + ) + var selectedProject: String? = null + var selectedRevision: String? = null + + composeTestRule.setContent { + TryFoxTheme { + HistoryScreen( + onNavigateUp = {}, + onNavigateToTreeherderRevision = { project, clickedRevision -> + selectedProject = project + selectedRevision = clickedRevision + }, + historyViewModel = historyViewModel, + ) + } + } + + composeTestRule.onNodeWithText("Revision: ${revision.take(12)}").performClick() + + assertEquals("mozilla-central", selectedProject) + assertEquals(revision, selectedRevision) + } + + private fun historyEntry( + project: String, + revision: String, + ): TreeherderInstallHistoryEntry = + TreeherderInstallHistoryEntry( + project = project, + revision = revision, + commitMessage = "Bug 123 - Test history", + author = "author@mozilla.com", + pushTimestamp = 1_716_460_800L, + appName = "fenix", + jobName = "signing-apk-fenix-nightly", + jobSymbol = "Bfs", + taskId = "task-id", + artifactName = "public/build/target.arm64-v8a.apk", + artifactFileName = "target.arm64-v8a.apk", + downloadUrl = "https://example.com/task/artifact", + abiName = "arm64-v8a", + abiSupported = true, + expires = "2026-01-01T00:00:00.000Z", + cacheRelativePath = "treeherder/task-id/target.arm64-v8a.apk", + lastInstallerLaunchTimestamp = 123L, + ) +} diff --git a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/ProfileScreenTest.kt b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/ProfileScreenTest.kt index 1c9ca10..228dd82 100644 --- a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/ProfileScreenTest.kt +++ b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/ProfileScreenTest.kt @@ -15,6 +15,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.tryfox.data.FakeCacheManager import org.mozilla.tryfox.data.FakeDownloadFileRepository +import org.mozilla.tryfox.data.FakeHistoryRepository import org.mozilla.tryfox.data.FakeIntentManager import org.mozilla.tryfox.data.FakeTreeherderRepository import org.mozilla.tryfox.data.FakeUserDataRepository @@ -32,6 +33,7 @@ class ProfileScreenTest { private val userDataRepository: UserDataRepository = FakeUserDataRepository() private val cacheManager: CacheManager = FakeCacheManager() private val intentManager = FakeIntentManager() + private val historyRepository = FakeHistoryRepository() private val emailInputTag = "profile_email_input" private val emailClearButtonTag = "profile_email_clear_button" private val searchButtonTag = "profile_search_button" @@ -50,6 +52,7 @@ class ProfileScreenTest { userDataRepository = userDataRepository, cacheManager = cacheManager, intentManager = intentManager, + historyRepository = historyRepository, authorEmail = null, ) @@ -109,6 +112,7 @@ class ProfileScreenTest { userDataRepository = userDataRepository, cacheManager = cacheManager, intentManager = intentManager, + historyRepository = historyRepository, authorEmail = initialEmail, ) diff --git a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreenTest.kt b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreenTest.kt index ba01065..5feba89 100644 --- a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreenTest.kt +++ b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreenTest.kt @@ -17,6 +17,7 @@ import org.mozilla.tryfox.data.Artifact import org.mozilla.tryfox.data.ArtifactsResponse import org.mozilla.tryfox.data.FakeCacheManager import org.mozilla.tryfox.data.FakeDownloadFileRepository +import org.mozilla.tryfox.data.FakeHistoryRepository import org.mozilla.tryfox.data.FakeIntentManager import org.mozilla.tryfox.data.JobDetails import org.mozilla.tryfox.data.NetworkResult @@ -42,6 +43,7 @@ class TreeherderApksScreenTest { downloadFileRepository = FakeDownloadFileRepository(), cacheManager = FakeCacheManager(), intentManager = FakeIntentManager(), + historyRepository = FakeHistoryRepository(), project = "mozilla-central", revision = "ed209aa2136b241686ff20489c5cb622348e2ecf", supportedAbis = listOf("arm64-v8a"), diff --git a/app/src/main/java/org/mozilla/tryfox/AppRoutes.kt b/app/src/main/java/org/mozilla/tryfox/AppRoutes.kt index 56719f4..4f1ce05 100644 --- a/app/src/main/java/org/mozilla/tryfox/AppRoutes.kt +++ b/app/src/main/java/org/mozilla/tryfox/AppRoutes.kt @@ -4,6 +4,7 @@ import java.net.URLEncoder object AppRoutes { const val HOME = "home" + const val HISTORY = "history" const val QR_SCANNER = "qr_scanner" const val TREEHERDER_SEARCH = "treeherder_search" const val TREEHERDER_SEARCH_WITH_ARGS = "treeherder_search/{project}/{revision}" diff --git a/app/src/main/java/org/mozilla/tryfox/MainActivity.kt b/app/src/main/java/org/mozilla/tryfox/MainActivity.kt index 9f55510..a4bc088 100644 --- a/app/src/main/java/org/mozilla/tryfox/MainActivity.kt +++ b/app/src/main/java/org/mozilla/tryfox/MainActivity.kt @@ -16,6 +16,7 @@ import androidx.navigation.compose.rememberNavController import androidx.navigation.navArgument import org.koin.androidx.compose.koinViewModel import org.koin.core.parameter.parametersOf +import org.mozilla.tryfox.ui.screens.HistoryScreen import org.mozilla.tryfox.ui.screens.HomeScreen import org.mozilla.tryfox.ui.screens.ProfileScreen import org.mozilla.tryfox.ui.screens.QrCodeScannerScreen @@ -32,6 +33,11 @@ sealed class NavScreen(val route: String) { */ data object Home : NavScreen(AppRoutes.HOME) + /** + * Represents the History screen. + */ + data object History : NavScreen(AppRoutes.HISTORY) + /** * Represents the QR code scanner screen. */ @@ -118,9 +124,21 @@ class MainActivity : ComponentActivity() { onNavigateToTreeherder = { localNavController.navigate(NavScreen.TreeherderSearch.route) }, onNavigateToProfile = { localNavController.navigate(NavScreen.Profile.route) }, onNavigateToQrScanner = { localNavController.navigate(NavScreen.QrScanner.route) }, + onNavigateToHistory = { localNavController.navigate(NavScreen.History.route) }, homeViewModel = koinViewModel(), ) } + composable(NavScreen.History.route) { + HistoryScreen( + onNavigateUp = { localNavController.popBackStack() }, + onNavigateToTreeherderRevision = { project, revision -> + localNavController.navigate( + NavScreen.TreeherderSearchWithArgs.createRoute(project, revision), + ) + }, + historyViewModel = koinViewModel(), + ) + } composable(NavScreen.QrScanner.route) { QrCodeScannerScreen( onNavigateUp = { localNavController.popBackStack() }, diff --git a/app/src/main/java/org/mozilla/tryfox/TryFoxViewModel.kt b/app/src/main/java/org/mozilla/tryfox/TryFoxViewModel.kt index 54bc01b..41c281a 100644 --- a/app/src/main/java/org/mozilla/tryfox/TryFoxViewModel.kt +++ b/app/src/main/java/org/mozilla/tryfox/TryFoxViewModel.kt @@ -25,14 +25,17 @@ import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withContext import org.mozilla.tryfox.data.DownloadState import org.mozilla.tryfox.data.NetworkResult +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry import org.mozilla.tryfox.data.managers.CacheManager import org.mozilla.tryfox.data.managers.IntentManager import org.mozilla.tryfox.data.repositories.DownloadFileRepository +import org.mozilla.tryfox.data.repositories.HistoryRepository import org.mozilla.tryfox.data.repositories.TreeherderRepository import org.mozilla.tryfox.model.CacheManagementState import org.mozilla.tryfox.ui.models.AbiUiModel import org.mozilla.tryfox.ui.models.ArtifactUiModel import org.mozilla.tryfox.ui.models.JobDetailsUiModel +import org.mozilla.tryfox.util.TREEHERDER import java.io.File /** @@ -49,11 +52,13 @@ class TryFoxViewModel( private val downloadFileRepository: DownloadFileRepository, private val cacheManager: CacheManager, private val intentManager: IntentManager, + private val historyRepository: HistoryRepository, project: String?, revision: String?, private val supportedAbis: List = Build.SUPPORTED_ABIS.toList(), private val elapsedRealtimeProvider: () -> Long = SystemClock::elapsedRealtime, - private val infoLogger: (String, String) -> Int = Log::i, + private val currentTimeMillisProvider: () -> Long = System::currentTimeMillis, + private val infoLogger: (String, String) -> Int = Log::d, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, private val mainDispatcher: CoroutineDispatcher = Dispatchers.Main, ) : ViewModel() { @@ -128,19 +133,9 @@ class TryFoxViewModel( searchJobsAndArtifacts() } cacheManager.cacheState.onEach { state -> - if (state is CacheManagementState.IdleEmpty) { - // Reset download states for artifacts in this ViewModel - // This logic was moved from clearAppCache - val updatedSelectedJobs = selectedJobs.map { - val updatedArtifacts = it.artifacts.map { artifact -> - artifact.copy(downloadState = DownloadState.NotDownloaded) - } - it.copy(artifacts = updatedArtifacts) - } - selectedJobs = updatedSelectedJobs - checkAndUpdateDownloadingStatus() // Downloads are effectively cancelled + if (state !is CacheManagementState.Clearing) { + refreshArtifactDownloadStatesFromCache() } - // Potentially update _isDownloadingAnyFile or other states if they depend on cache state changes }.launchIn(viewModelScope) } @@ -176,11 +171,17 @@ class TryFoxViewModel( // The cache directory for Treeherder artifacts is now under a "treeherder" subdirectory val taskSpecificDir = File(cacheManager.getCacheDir("treeherder"), taskId) val outputFile = File(taskSpecificDir, artifactName) + infoLogger( + TAG, + "getDownloadedFile artifactName=$artifactName, taskId=$taskId, " + + "path=${outputFile.absolutePath}, exists=${outputFile.exists()}", + ) return if (outputFile.exists()) outputFile else null } fun checkCacheStatus() { cacheManager.checkCacheStatus() + refreshArtifactDownloadStatesFromCache() } fun clearAppCache() { @@ -220,6 +221,9 @@ class TryFoxViewModel( break } } + if (foundComment == null) { + foundComment = firstPushResult.revisions.firstOrNull()?.comments ?: "No comment" + } relevantPushAuthor = firstPushResult.author relevantPushTimestamp = firstPushResult.pushTimestamp } else { @@ -479,7 +483,14 @@ class TryFoxViewModel( expires = artifact.expires, downloadState = downloadState, uniqueKey = "$taskId/${artifact.name.substringAfterLast('/')}", - ) + ).also { + infoLogger( + TAG, + "fetchArtifacts resolved artifact taskId=$taskId, artifactName=${artifact.name}, " + + "artifactFileName=$artifactFileName, uniqueKey=${it.uniqueKey}, " + + "downloadState=${downloadState.javaClass.simpleName}", + ) + } } } is NetworkResult.Error -> emptyList() @@ -512,6 +523,11 @@ class TryFoxViewModel( outputDir.mkdirs() } val outputFile = File(outputDir, artifactFileName) + infoLogger( + TAG, + "downloadArtifact output taskId=$taskId, artifactName=${artifactUiModel.name}, " + + "artifactFileName=$artifactFileName, outputPath=${outputFile.absolutePath}", + ) val result = downloadFileRepository.downloadFile( downloadUrl = downloadUrl, @@ -530,7 +546,14 @@ class TryFoxViewModel( is NetworkResult.Success -> { updateArtifactDownloadState(taskId, artifactUiModel.name, DownloadState.Downloaded(result.data)) cacheManager.checkCacheStatus() // Update cache status via CacheManager - onInstallApk?.invoke(result.data) + onInstallApk?.let { installCallback -> + try { + recordInstallerLaunch(job = findJob(taskId), artifact = artifactUiModel) + } catch (_: Exception) { + // History is best-effort; never block installation. + } + installCallback(result.data) + } } is NetworkResult.Error -> { updateArtifactDownloadState(taskId, artifactUiModel.name, DownloadState.DownloadFailed(result.message)) @@ -559,7 +582,97 @@ class TryFoxViewModel( checkAndUpdateDownloadingStatus() } + private fun refreshArtifactDownloadStatesFromCache() { + if (selectedJobs.isEmpty()) { + checkAndUpdateDownloadingStatus() + return + } + + selectedJobs = selectedJobs.map { job -> + job.copy( + artifacts = job.artifacts.map { artifact -> + when (artifact.downloadState) { + is DownloadState.InProgress, + is DownloadState.DownloadFailed, + -> artifact + else -> { + val artifactFileName = artifact.name.substringAfterLast('/') + val downloadedFile = getDownloadedFile(artifactFileName, artifact.taskId) + val refreshedState = downloadedFile?.let { DownloadState.Downloaded(it) } + ?: DownloadState.NotDownloaded + artifact.copy(downloadState = refreshedState) + } + } + }, + ) + } + checkAndUpdateDownloadingStatus() + } + fun installApk(file: File) { - intentManager.installApk(file) + val downloadedArtifact = findDownloadedArtifact(file) + if (downloadedArtifact == null) { + intentManager.installApk(file) + return + } + + viewModelScope.launch { + try { + recordInstallerLaunch( + job = downloadedArtifact.job, + artifact = downloadedArtifact.artifact, + ) + } catch (_: Exception) { + // History is best-effort; never block installation. + } + intentManager.installApk(file) + } } + + private fun findJob(taskId: String): JobDetailsUiModel? = + selectedJobs.firstOrNull { it.taskId == taskId } + + private fun findDownloadedArtifact(file: File): DownloadedArtifact? = + selectedJobs.firstNotNullOfOrNull { job -> + job.artifacts.firstOrNull { artifact -> + val downloadState = artifact.downloadState + downloadState is DownloadState.Downloaded && downloadState.file.absolutePath == file.absolutePath + }?.let { artifact -> DownloadedArtifact(job, artifact) } + } + + private suspend fun recordInstallerLaunch( + job: JobDetailsUiModel?, + artifact: ArtifactUiModel, + ) { + if (job == null || relevantPushTimestamp == null) { + return + } + val artifactFileName = artifact.name.substringAfterLast('/') + historyRepository.recordInstallerLaunch( + TreeherderInstallHistoryEntry( + project = selectedProject, + revision = revision, + commitMessage = relevantPushComment ?: "No comment", + author = relevantPushAuthor, + pushTimestamp = relevantPushTimestamp ?: 0L, + appName = job.appName, + jobName = job.jobName, + jobSymbol = job.jobSymbol, + taskId = artifact.taskId, + artifactName = artifact.name, + artifactFileName = artifactFileName, + downloadUrl = artifact.downloadUrl, + abiName = artifact.abi.name, + abiSupported = artifact.abi.isSupported, + expires = artifact.expires, + cacheRelativePath = "$TREEHERDER/${artifact.taskId}/$artifactFileName", + lastInstallerLaunchTimestamp = currentTimeMillisProvider(), + ), + ) + } + + private data class DownloadedArtifact( + val job: JobDetailsUiModel, + val artifact: ArtifactUiModel, + ) } diff --git a/app/src/main/java/org/mozilla/tryfox/data/TreeherderInstallHistoryEntry.kt b/app/src/main/java/org/mozilla/tryfox/data/TreeherderInstallHistoryEntry.kt new file mode 100644 index 0000000..b309fc4 --- /dev/null +++ b/app/src/main/java/org/mozilla/tryfox/data/TreeherderInstallHistoryEntry.kt @@ -0,0 +1,24 @@ +package org.mozilla.tryfox.data + +data class TreeherderInstallHistoryEntry( + val project: String, + val revision: String, + val commitMessage: String, + val author: String?, + val pushTimestamp: Long, + val appName: String, + val jobName: String, + val jobSymbol: String, + val taskId: String, + val artifactName: String, + val artifactFileName: String, + val downloadUrl: String, + val abiName: String?, + val abiSupported: Boolean, + val expires: String, + val cacheRelativePath: String, + val lastInstallerLaunchTimestamp: Long, +) { + val uniqueKey: String + get() = "$taskId/$artifactFileName" +} diff --git a/app/src/main/java/org/mozilla/tryfox/data/managers/DefaultCacheManager.kt b/app/src/main/java/org/mozilla/tryfox/data/managers/DefaultCacheManager.kt index 94deae7..8480df6 100644 --- a/app/src/main/java/org/mozilla/tryfox/data/managers/DefaultCacheManager.kt +++ b/app/src/main/java/org/mozilla/tryfox/data/managers/DefaultCacheManager.kt @@ -21,11 +21,16 @@ import java.io.File class DefaultCacheManager( private val cacheDir: File, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, + legacyCacheDir: File? = null, ) : CacheManager { private val _cacheState = MutableStateFlow(CacheManagementState.IdleEmpty) override val cacheState: StateFlow = _cacheState.asStateFlow() + init { + migrateLegacyCache(legacyCacheDir) + } + override fun getCacheDir(appName: String): File { return File(cacheDir, appName) } @@ -45,7 +50,7 @@ class DefaultCacheManager( } private fun determineCacheState(): CacheManagementState { - val cacheIsNotEmpty = listOf(FENIX, FOCUS, FOCUS_RELEASE, REFERENCE_BROWSER, TREEHERDER, TRYFOX).any { isAppCachePopulated(it) } + val cacheIsNotEmpty = MANAGED_CACHE_NAMES.any { isAppCachePopulated(it) } return if (cacheIsNotEmpty) CacheManagementState.IdleNonEmpty else CacheManagementState.IdleEmpty } @@ -61,11 +66,14 @@ class DefaultCacheManager( withContext(ioDispatcher) { cacheDir.listFiles()?.forEach { if (it.isDirectory) { + logcat(LogPriority.DEBUG, TAG) { + "Deleting cache directory path=${it.absolutePath}" + } it.deleteRecursively() } } } - logcat(TAG) { "Cache cleared successfully." } + logcat(LogPriority.DEBUG, TAG) { "Cache cleared successfully." } } catch (e: Exception) { logcat( LogPriority.ERROR, @@ -76,7 +84,49 @@ class DefaultCacheManager( } } + private fun migrateLegacyCache(legacyCacheDir: File?) { + if (legacyCacheDir == null || legacyCacheDir == cacheDir || !legacyCacheDir.isDirectory) { + return + } + + MANAGED_CACHE_NAMES.forEach { cacheName -> + val legacyEntry = File(legacyCacheDir, cacheName) + val target = File(cacheDir, cacheName) + try { + if (legacyEntry.isDirectory) { + migrateDirectory(legacyEntry, target) + } else if (legacyEntry.isFile && !target.exists()) { + target.parentFile?.mkdirs() + legacyEntry.renameTo(target) + } + } catch (e: Exception) { + logcat(LogPriority.WARN, TAG) { + "Failed to migrate legacy cache path=${legacyEntry.absolutePath}: ${e.message}" + } + } + } + } + + private fun migrateDirectory(source: File, target: File) { + if (!target.exists() && source.renameTo(target)) { + return + } + + target.mkdirs() + source.listFiles()?.forEach { child -> + val childTarget = File(target, child.name) + if (child.isDirectory) { + migrateDirectory(child, childTarget) + } else if (child.isFile && !childTarget.exists()) { + childTarget.parentFile?.mkdirs() + child.renameTo(childTarget) + } + } + source.delete() + } + companion object { private const val TAG = "DefaultCacheManager" + private val MANAGED_CACHE_NAMES = listOf(FENIX, FOCUS, FOCUS_RELEASE, REFERENCE_BROWSER, TREEHERDER, TRYFOX) } } diff --git a/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepository.kt b/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepository.kt index 14fdc54..c9e1752 100644 --- a/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepository.kt +++ b/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepository.kt @@ -3,6 +3,8 @@ package org.mozilla.tryfox.data.repositories import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import logcat.LogPriority +import logcat.logcat import org.mozilla.tryfox.data.NetworkResult import org.mozilla.tryfox.network.DownloadApiService import java.io.File @@ -17,27 +19,156 @@ class DefaultDownloadFileRepository( ) : DownloadFileRepository { override suspend fun downloadFile(downloadUrl: String, outputFile: File, onProgress: (Long, Long) -> Unit): NetworkResult { return withContext(ioDispatcher) { + val partialFile = File(outputFile.parentFile, "${outputFile.name}.part") + var backupFile = File(outputFile.parentFile, "${outputFile.name}.bak") try { + outputFile.parentFile?.mkdirs() + if (partialFile.exists()) { + partialFile.delete() + } + if (!outputFile.exists()) { + val latestBackupFile = findLatestBackupFile(outputFile) + if (latestBackupFile != null && !latestBackupFile.renameTo(outputFile)) { + return@withContext NetworkResult.Error( + "Failed to restore backup file: ${latestBackupFile.absolutePath}", + null, + ) + } + } + if (outputFile.exists() && backupFile.exists() && !backupFile.delete()) { + backupFile = createReplacementBackupFile(outputFile) + logcat(LogPriority.WARN, TAG) { + "Could not delete stale backup, using replacement backup path=${backupFile.absolutePath}" + } + } + logcat(LogPriority.DEBUG, TAG) { + "downloadFile started url=$downloadUrl, outputPath=${outputFile.absolutePath}, " + + "parentExists=${outputFile.parentFile?.exists()}, existing=${outputFile.exists()}, " + + "existingLength=${outputFile.length()}, partialPath=${partialFile.absolutePath}" + } val response = downloadApiService.downloadFile(downloadUrl) val body = response.byteStream() val totalBytes = response.contentLength() var bytesCopied: Long = 0 + val progressLogStepBytes = if (totalBytes > 0) { + (totalBytes / PROGRESS_LOG_SLICES).coerceAtLeast(1L) + } else { + 0L + } + var nextProgressLogBytes = progressLogStepBytes + logcat(LogPriority.DEBUG, TAG) { + "downloadFile response url=$downloadUrl, contentLength=$totalBytes" + } body.use { inputStream -> - FileOutputStream(outputFile).use { outputStream -> + FileOutputStream(partialFile).use { outputStream -> val buffer = ByteArray(4 * 1024) var read: Int while (inputStream.read(buffer).also { read = it } != -1) { outputStream.write(buffer, 0, read) bytesCopied += read onProgress(bytesCopied, totalBytes) + if (totalBytes > 0 && bytesCopied >= nextProgressLogBytes) { + logcat(LogPriority.DEBUG, TAG) { + "downloadFile progress partialPath=${partialFile.absolutePath}, " + + "bytesCopied=$bytesCopied, totalBytes=$totalBytes, " + + "partialExists=${partialFile.exists()}, partialLength=${partialFile.length()}" + } + nextProgressLogBytes += progressLogStepBytes + } } } } + + if (totalBytes >= 0 && bytesCopied != totalBytes) { + partialFile.delete() + return@withContext NetworkResult.Error( + "Failed to download file: expected $totalBytes bytes but received $bytesCopied", + null, + ) + } + + val hadExistingOutput = outputFile.exists() + if (hadExistingOutput && !outputFile.renameTo(backupFile)) { + partialFile.delete() + return@withContext NetworkResult.Error( + "Failed to prepare existing file for replacement: ${outputFile.absolutePath}", + null, + ) + } + + if (!partialFile.renameTo(outputFile)) { + partialFile.delete() + if (backupFile.exists()) { + backupFile.renameTo(outputFile) + } + return@withContext NetworkResult.Error( + "Failed to move downloaded file into place: ${outputFile.absolutePath}", + null, + ) + } + if (backupFile.exists() && !backupFile.delete()) { + logcat(LogPriority.WARN, TAG) { + "Failed to delete backup after successful replacement path=${backupFile.absolutePath}" + } + } + + logcat(LogPriority.DEBUG, TAG) { + "downloadFile finished outputPath=${outputFile.absolutePath}, bytesCopied=$bytesCopied, " + + "totalBytes=$totalBytes, exists=${outputFile.exists()}, length=${outputFile.length()}" + } NetworkResult.Success(outputFile) } catch (e: Exception) { + partialFile.delete() + if (backupFile.exists() && !outputFile.exists()) { + backupFile.renameTo(outputFile) + } + logcat(LogPriority.ERROR, TAG) { + "downloadFile failed outputPath=${outputFile.absolutePath}, partialPath=${partialFile.absolutePath}, " + + "partialExists=${partialFile.exists()}, message=${e.message}" + } NetworkResult.Error("Failed to download file: ${e.message}", e) } } } + + private companion object { + const val TAG = "DefaultDownloadFileRepository" + const val PROGRESS_LOG_SLICES = 10 + + fun createReplacementBackupFile(outputFile: File): File { + var index = 1 + while (true) { + val candidate = File(outputFile.parentFile, "${outputFile.name}.bak.$index") + if (!candidate.exists()) { + return candidate + } + index += 1 + } + } + + fun findLatestBackupFile(outputFile: File): File? = + outputFile.parentFile + ?.listFiles { file -> file.isFile && file.isManagedBackupFile(outputFile) } + ?.maxWithOrNull( + compareBy { it.backupSequence(outputFile) } + .thenBy { it.lastModified() }, + ) + + fun File.isManagedBackupFile(outputFile: File): Boolean = backupSequence(outputFile) >= 0 + + fun File.backupSequence(outputFile: File): Int { + val baseBackupName = "${outputFile.name}.bak" + if (name == baseBackupName) { + return 0 + } + if (!name.startsWith("$baseBackupName.")) { + return -1 + } + return name + .removePrefix("$baseBackupName.") + .toIntOrNull() + ?: -1 + } + } } diff --git a/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt b/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt new file mode 100644 index 0000000..28bf3d5 --- /dev/null +++ b/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt @@ -0,0 +1,193 @@ +package org.mozilla.tryfox.data.repositories + +import android.content.ContentValues +import android.content.Context +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase +import android.database.sqlite.SQLiteOpenHelper +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.withContext +import logcat.LogPriority +import logcat.logcat +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry + +class DefaultHistoryRepository( + context: Context, + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, +) : HistoryRepository { + + private val dbHelper = HistoryDatabaseHelper(context.applicationContext) + private val _historyEntries = MutableStateFlow>(emptyList()) + override val historyEntries: StateFlow> = _historyEntries.asStateFlow() + + override suspend fun refresh() { + val entries = loadEntries() + logcat(LogPriority.DEBUG, TAG) { + "refresh loaded count=${entries.size}, keys=${entries.joinToString { it.uniqueKey }}" + } + _historyEntries.value = entries + } + + override suspend fun recordInstallerLaunch(entry: TreeherderInstallHistoryEntry) { + logcat(LogPriority.DEBUG, TAG) { + "recordInstallerLaunch uniqueKey=${entry.uniqueKey}, taskId=${entry.taskId}, " + + "artifactFileName=${entry.artifactFileName}, jobSymbol=${entry.jobSymbol}, " + + "cacheRelativePath=${entry.cacheRelativePath}" + } + withContext(ioDispatcher) { + dbHelper.writableDatabase.insertWithOnConflict( + TABLE_HISTORY, + null, + entry.toContentValues(), + SQLiteDatabase.CONFLICT_REPLACE, + ) + } + refresh() + } + + private suspend fun loadEntries(): List = withContext(ioDispatcher) { + dbHelper.readableDatabase.query( + TABLE_HISTORY, + null, + null, + null, + null, + null, + "$COLUMN_LAST_INSTALLER_LAUNCH_TIMESTAMP DESC", + ).use { cursor -> + buildList { + while (cursor.moveToNext()) { + add(cursor.toHistoryEntry()) + } + } + } + } + + private fun TreeherderInstallHistoryEntry.toContentValues(): ContentValues = + ContentValues().apply { + put(COLUMN_ID, uniqueKey) + put(COLUMN_PROJECT, project) + put(COLUMN_REVISION, revision) + put(COLUMN_COMMIT_MESSAGE, commitMessage) + put(COLUMN_AUTHOR, author) + put(COLUMN_PUSH_TIMESTAMP, pushTimestamp) + put(COLUMN_APP_NAME, appName) + put(COLUMN_JOB_NAME, jobName) + put(COLUMN_JOB_SYMBOL, jobSymbol) + put(COLUMN_TASK_ID, taskId) + put(COLUMN_ARTIFACT_NAME, artifactName) + put(COLUMN_ARTIFACT_FILE_NAME, artifactFileName) + put(COLUMN_DOWNLOAD_URL, downloadUrl) + put(COLUMN_ABI_NAME, abiName) + put(COLUMN_ABI_SUPPORTED, if (abiSupported) 1 else 0) + put(COLUMN_EXPIRES, expires) + put(COLUMN_CACHE_RELATIVE_PATH, cacheRelativePath) + put(COLUMN_LAST_INSTALLER_LAUNCH_TIMESTAMP, lastInstallerLaunchTimestamp) + } + + private fun Cursor.toHistoryEntry(): TreeherderInstallHistoryEntry = + TreeherderInstallHistoryEntry( + project = getStringValue(COLUMN_PROJECT), + revision = getStringValue(COLUMN_REVISION), + commitMessage = getStringValue(COLUMN_COMMIT_MESSAGE), + author = getNullableStringValue(COLUMN_AUTHOR), + pushTimestamp = getLongValue(COLUMN_PUSH_TIMESTAMP), + appName = getStringValue(COLUMN_APP_NAME), + jobName = getStringValue(COLUMN_JOB_NAME), + jobSymbol = getStringValue(COLUMN_JOB_SYMBOL), + taskId = getStringValue(COLUMN_TASK_ID), + artifactName = getStringValue(COLUMN_ARTIFACT_NAME), + artifactFileName = getStringValue(COLUMN_ARTIFACT_FILE_NAME), + downloadUrl = getStringValue(COLUMN_DOWNLOAD_URL), + abiName = getNullableStringValue(COLUMN_ABI_NAME), + abiSupported = getIntValue(COLUMN_ABI_SUPPORTED) == 1, + expires = getStringValue(COLUMN_EXPIRES), + cacheRelativePath = getStringValue(COLUMN_CACHE_RELATIVE_PATH), + lastInstallerLaunchTimestamp = getLongValue(COLUMN_LAST_INSTALLER_LAUNCH_TIMESTAMP), + ) + + private fun Cursor.getStringValue(columnName: String): String = + getString(getColumnIndexOrThrow(columnName)) + + private fun Cursor.getNullableStringValue(columnName: String): String? = + if (isNull(getColumnIndexOrThrow(columnName))) { + null + } else { + getString(getColumnIndexOrThrow(columnName)) + } + + private fun Cursor.getLongValue(columnName: String): Long = + getLong(getColumnIndexOrThrow(columnName)) + + private fun Cursor.getIntValue(columnName: String): Int = + getInt(getColumnIndexOrThrow(columnName)) + + private class HistoryDatabaseHelper(context: Context) : SQLiteOpenHelper( + context, + DATABASE_NAME, + null, + DATABASE_VERSION, + ) { + override fun onCreate(db: SQLiteDatabase) { + db.execSQL( + """ + CREATE TABLE $TABLE_HISTORY ( + $COLUMN_ID TEXT PRIMARY KEY NOT NULL, + $COLUMN_PROJECT TEXT NOT NULL, + $COLUMN_REVISION TEXT NOT NULL, + $COLUMN_COMMIT_MESSAGE TEXT NOT NULL, + $COLUMN_AUTHOR TEXT, + $COLUMN_PUSH_TIMESTAMP INTEGER NOT NULL, + $COLUMN_APP_NAME TEXT NOT NULL, + $COLUMN_JOB_NAME TEXT NOT NULL, + $COLUMN_JOB_SYMBOL TEXT NOT NULL, + $COLUMN_TASK_ID TEXT NOT NULL, + $COLUMN_ARTIFACT_NAME TEXT NOT NULL, + $COLUMN_ARTIFACT_FILE_NAME TEXT NOT NULL, + $COLUMN_DOWNLOAD_URL TEXT NOT NULL, + $COLUMN_ABI_NAME TEXT, + $COLUMN_ABI_SUPPORTED INTEGER NOT NULL, + $COLUMN_EXPIRES TEXT NOT NULL, + $COLUMN_CACHE_RELATIVE_PATH TEXT NOT NULL, + $COLUMN_LAST_INSTALLER_LAUNCH_TIMESTAMP INTEGER NOT NULL + ) + """.trimIndent(), + ) + } + + override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + db.execSQL("DROP TABLE IF EXISTS $TABLE_HISTORY") + onCreate(db) + } + } + + private companion object { + const val DATABASE_NAME = "tryfox_history.db" + const val DATABASE_VERSION = 1 + + const val TABLE_HISTORY = "treeherder_install_history" + const val COLUMN_ID = "id" + const val COLUMN_PROJECT = "project" + const val COLUMN_REVISION = "revision" + const val COLUMN_COMMIT_MESSAGE = "commit_message" + const val COLUMN_AUTHOR = "author" + const val COLUMN_PUSH_TIMESTAMP = "push_timestamp" + const val COLUMN_APP_NAME = "app_name" + const val COLUMN_JOB_NAME = "job_name" + const val COLUMN_JOB_SYMBOL = "job_symbol" + const val COLUMN_TASK_ID = "task_id" + const val COLUMN_ARTIFACT_NAME = "artifact_name" + const val COLUMN_ARTIFACT_FILE_NAME = "artifact_file_name" + const val COLUMN_DOWNLOAD_URL = "download_url" + const val COLUMN_ABI_NAME = "abi_name" + const val COLUMN_ABI_SUPPORTED = "abi_supported" + const val COLUMN_EXPIRES = "expires" + const val COLUMN_CACHE_RELATIVE_PATH = "cache_relative_path" + const val COLUMN_LAST_INSTALLER_LAUNCH_TIMESTAMP = "last_installer_launch_timestamp" + const val TAG = "DefaultHistoryRepository" + } +} diff --git a/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt b/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt new file mode 100644 index 0000000..a7dccdd --- /dev/null +++ b/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt @@ -0,0 +1,12 @@ +package org.mozilla.tryfox.data.repositories + +import kotlinx.coroutines.flow.StateFlow +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry + +interface HistoryRepository { + val historyEntries: StateFlow> + + suspend fun refresh() + + suspend fun recordInstallerLaunch(entry: TreeherderInstallHistoryEntry) +} diff --git a/app/src/main/java/org/mozilla/tryfox/di/AppModule.kt b/app/src/main/java/org/mozilla/tryfox/di/AppModule.kt index 0cb384b..2f2c23d 100644 --- a/app/src/main/java/org/mozilla/tryfox/di/AppModule.kt +++ b/app/src/main/java/org/mozilla/tryfox/di/AppModule.kt @@ -20,6 +20,7 @@ import org.mozilla.tryfox.data.managers.DefaultCacheManager import org.mozilla.tryfox.data.managers.DefaultIntentManager import org.mozilla.tryfox.data.managers.IntentManager import org.mozilla.tryfox.data.repositories.DefaultDownloadFileRepository +import org.mozilla.tryfox.data.repositories.DefaultHistoryRepository import org.mozilla.tryfox.data.repositories.DefaultMozillaArchiveRepository import org.mozilla.tryfox.data.repositories.DefaultTreeherderRepository import org.mozilla.tryfox.data.repositories.DefaultUserDataRepository @@ -29,6 +30,7 @@ import org.mozilla.tryfox.data.repositories.FenixReleaseReleaseRepository import org.mozilla.tryfox.data.repositories.FenixReleaseRepository import org.mozilla.tryfox.data.repositories.FocusNightlyRepository import org.mozilla.tryfox.data.repositories.FocusReleaseRepository +import org.mozilla.tryfox.data.repositories.HistoryRepository import org.mozilla.tryfox.data.repositories.MozillaArchiveRepository import org.mozilla.tryfox.data.repositories.ReferenceBrowserReleaseRepository import org.mozilla.tryfox.data.repositories.ReleaseRepository @@ -39,6 +41,7 @@ import org.mozilla.tryfox.network.DownloadApiService import org.mozilla.tryfox.network.GithubApiService import org.mozilla.tryfox.network.MozillaArchivesApiService import org.mozilla.tryfox.network.TreeherderApiService +import org.mozilla.tryfox.ui.screens.HistoryViewModel import org.mozilla.tryfox.ui.screens.HomeViewModel import org.mozilla.tryfox.ui.screens.ProfileViewModel import org.mozilla.tryfox.util.FENIX @@ -50,6 +53,7 @@ import org.mozilla.tryfox.util.REFERENCE_BROWSER import org.mozilla.tryfox.util.TRYFOX import retrofit2.Retrofit import retrofit2.converter.scalars.ScalarsConverterFactory +import java.io.File const val TREEHERDER_BASE_URL = "https://treeherder.mozilla.org/api/" const val GITHUB_BASE_URL = "https://api.github.com/" @@ -136,11 +140,13 @@ val repositoryModule = module { single { DefaultTreeherderRepository(get()) } single { DefaultMozillaArchiveRepository(get()) } single { DefaultUserDataRepository(androidContext()) } + single { DefaultHistoryRepository(androidContext(), get(named("IODispatcher"))) } single { DefaultMozillaPackageManager(androidContext()) } single { DefaultCacheManager( - androidContext().cacheDir, + File(androidContext().filesDir, "download-cache"), get(named("IODispatcher")), + androidContext().cacheDir, ) } single { DefaultIntentManager(androidContext()) } @@ -161,10 +167,12 @@ val viewModelModule = module { get(), get(), get(), + get(), params.getOrNull(), params.getOrNull(), ) } + viewModel { HistoryViewModel(get(), get(), get(), get(), get(named("IODispatcher"))) } viewModel { val releaseRepositories = listOf( get(named(FENIX)), @@ -184,7 +192,7 @@ val viewModelModule = module { get(named("IODispatcher")), ) } - viewModel { params -> ProfileViewModel(get(), get(), get(), get(), get(), params.getOrNull()) } + viewModel { params -> ProfileViewModel(get(), get(), get(), get(), get(), get(), params.getOrNull()) } } val appModules = listOf(dispatchersModule, networkModule, repositoryModule, viewModelModule) diff --git a/app/src/main/java/org/mozilla/tryfox/ui/models/HistoryItemUiModel.kt b/app/src/main/java/org/mozilla/tryfox/ui/models/HistoryItemUiModel.kt new file mode 100644 index 0000000..324a435 --- /dev/null +++ b/app/src/main/java/org/mozilla/tryfox/ui/models/HistoryItemUiModel.kt @@ -0,0 +1,9 @@ +package org.mozilla.tryfox.ui.models + +import org.mozilla.tryfox.data.DownloadState +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry + +data class HistoryItemUiModel( + val entry: TreeherderInstallHistoryEntry, + val downloadState: DownloadState, +) diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt new file mode 100644 index 0000000..9f6e690 --- /dev/null +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt @@ -0,0 +1,277 @@ +package org.mozilla.tryfox.ui.screens + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material3.CardDefaults +import androidx.compose.material3.ElevatedCard +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.material3.TopAppBarDefaults +import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalLifecycleOwner +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver +import kotlinx.datetime.Instant +import kotlinx.datetime.LocalDateTime +import kotlinx.datetime.TimeZone +import kotlinx.datetime.format.FormatStringsInDatetimeFormats +import kotlinx.datetime.format.byUnicodePattern +import kotlinx.datetime.toLocalDateTime +import logcat.LogPriority +import logcat.logcat +import org.mozilla.tryfox.R +import org.mozilla.tryfox.data.DownloadState +import org.mozilla.tryfox.ui.composables.AppIcon +import org.mozilla.tryfox.ui.composables.DownloadButton +import org.mozilla.tryfox.ui.composables.ErrorState +import org.mozilla.tryfox.ui.composables.PushCommentCard +import org.mozilla.tryfox.ui.models.HistoryItemUiModel + +internal const val HISTORY_EMPTY_STATE_TAG = "history_empty_state" +internal const val HISTORY_LIST_TAG = "history_list" +private const val HISTORY_SCREEN_LOG_TAG = "HistoryScreen" + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun HistoryScreen( + modifier: Modifier = Modifier, + onNavigateUp: () -> Unit, + onNavigateToTreeherderRevision: (project: String, revision: String) -> Unit, + historyViewModel: HistoryViewModel, +) { + val historyItems by historyViewModel.historyItems.collectAsState() + val lifecycleOwner = LocalLifecycleOwner.current + + LaunchedEffect(historyItems.size) { + logcat(LogPriority.DEBUG, HISTORY_SCREEN_LOG_TAG) { + "rendered with itemCount=${historyItems.size}" + } + } + + DisposableEffect(lifecycleOwner, historyViewModel) { + val observer = LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_RESUME) { + logcat(LogPriority.DEBUG, HISTORY_SCREEN_LOG_TAG) { "ON_RESUME" } + historyViewModel.refreshCachedDownloadStates() + } + } + lifecycleOwner.lifecycle.addObserver(observer) + onDispose { + lifecycleOwner.lifecycle.removeObserver(observer) + } + } + + Scaffold( + modifier = modifier.fillMaxSize(), + topBar = { + TopAppBar( + title = { Text(stringResource(id = R.string.history_screen_title)) }, + navigationIcon = { + IconButton(onClick = onNavigateUp) { + Icon( + imageVector = Icons.AutoMirrored.Filled.ArrowBack, + contentDescription = stringResource(id = R.string.common_back_button_description), + ) + } + }, + colors = TopAppBarDefaults.topAppBarColors( + containerColor = MaterialTheme.colorScheme.primaryContainer, + titleContentColor = MaterialTheme.colorScheme.onPrimaryContainer, + navigationIconContentColor = MaterialTheme.colorScheme.onPrimaryContainer, + ), + ) + }, + ) { innerPadding -> + if (historyItems.isEmpty()) { + Column( + modifier = Modifier + .fillMaxSize() + .padding(innerPadding) + .padding(24.dp) + .testTag(HISTORY_EMPTY_STATE_TAG), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + Text( + text = stringResource(id = R.string.history_screen_empty_title), + style = MaterialTheme.typography.titleLarge, + fontWeight = FontWeight.Bold, + ) + Text( + text = stringResource(id = R.string.history_screen_empty_message), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 8.dp), + ) + } + } else { + LazyColumn( + modifier = Modifier + .fillMaxSize() + .padding(innerPadding) + .testTag(HISTORY_LIST_TAG), + contentPadding = PaddingValues(16.dp), + verticalArrangement = Arrangement.spacedBy(16.dp), + ) { + items(historyItems, key = { it.entry.uniqueKey }) { historyItem -> + HistoryCard( + historyItem = historyItem, + onRevisionClick = onNavigateToTreeherderRevision, + onDownloadClick = { historyViewModel.download(historyItem) }, + onInstallClick = { file -> historyViewModel.install(historyItem, file) }, + ) + } + } + } + } +} + +@Composable +private fun HistoryCard( + historyItem: HistoryItemUiModel, + onRevisionClick: (project: String, revision: String) -> Unit, + onDownloadClick: () -> Unit, + onInstallClick: (java.io.File) -> Unit, +) { + val entry = historyItem.entry + + ElevatedCard( + modifier = Modifier.fillMaxWidth(), + elevation = CardDefaults.cardElevation(defaultElevation = 4.dp), + ) { + Column( + modifier = Modifier.padding(16.dp), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically, + ) { + AppIcon(appName = entry.appName, modifier = Modifier.size(36.dp)) + Spacer(modifier = Modifier.width(10.dp)) + Column(modifier = Modifier.weight(1f)) { + Row( + horizontalArrangement = Arrangement.spacedBy(6.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = entry.jobName, + modifier = Modifier.weight(1f, fill = false), + style = MaterialTheme.typography.titleMedium, + fontWeight = FontWeight.SemiBold, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + JobSymbolChip(symbol = entry.jobSymbol) + } + Text( + text = stringResource(id = R.string.history_screen_revision_label, entry.revision.take(12)), + modifier = Modifier.clickable { + onRevisionClick(entry.project, entry.revision) + }, + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.primary, + ) + } + } + + PushCommentCard( + comment = entry.commitMessage, + author = entry.author, + revision = entry.revision, + pushTimestamp = entry.pushTimestamp, + ) + + if (historyItem.downloadState is DownloadState.DownloadFailed) { + ErrorState( + errorMessage = stringResource( + id = R.string.app_card_download_failed_message, + historyItem.downloadState.message ?: stringResource(id = R.string.common_unknown_error), + ), + ) + } + + Text( + text = stringResource( + id = R.string.history_screen_last_installed_label, + formatInstallTimestamp(entry.lastInstallerLaunchTimestamp), + ), + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.End, + ) { + DownloadButton( + downloadState = historyItem.downloadState, + onDownloadClick = onDownloadClick, + onInstallClick = onInstallClick, + ) + } + } + } +} + +@Composable +private fun JobSymbolChip(symbol: String) { + Surface( + shape = RoundedCornerShape(10.dp), + color = MaterialTheme.colorScheme.tertiaryContainer, + contentColor = MaterialTheme.colorScheme.onTertiaryContainer, + ) { + Text( + text = symbol, + modifier = Modifier.padding(horizontal = 6.dp, vertical = 2.dp), + style = MaterialTheme.typography.labelSmall, + maxLines = 1, + ) + } +} + +@OptIn(FormatStringsInDatetimeFormats::class) +@Composable +private fun formatInstallTimestamp(timestampMillis: Long): String { + val format = remember { + LocalDateTime.Format { byUnicodePattern("yyyy-MM-dd HH:mm") } + } + return remember(timestampMillis) { + format.format( + Instant.fromEpochMilliseconds(timestampMillis) + .toLocalDateTime(TimeZone.currentSystemDefault()), + ) + } +} diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt new file mode 100644 index 0000000..4822749 --- /dev/null +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt @@ -0,0 +1,230 @@ +package org.mozilla.tryfox.ui.screens + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import logcat.LogPriority +import logcat.logcat +import org.mozilla.tryfox.data.DownloadState +import org.mozilla.tryfox.data.NetworkResult +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry +import org.mozilla.tryfox.data.managers.CacheManager +import org.mozilla.tryfox.data.managers.IntentManager +import org.mozilla.tryfox.data.repositories.DownloadFileRepository +import org.mozilla.tryfox.data.repositories.HistoryRepository +import org.mozilla.tryfox.ui.models.HistoryItemUiModel +import org.mozilla.tryfox.util.TREEHERDER +import java.io.File + +class HistoryViewModel( + private val historyRepository: HistoryRepository, + private val downloadFileRepository: DownloadFileRepository, + private val cacheManager: CacheManager, + private val intentManager: IntentManager, + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, + private val currentTimeMillisProvider: () -> Long = System::currentTimeMillis, +) : ViewModel() { + + private companion object { + const val TAG = "HistoryViewModel" + } + + private val downloadStates = MutableStateFlow>(emptyMap()) + private val cacheRefreshEvents = MutableStateFlow(0) + + private val _historyItems = MutableStateFlow>(emptyList()) + val historyItems: StateFlow> = _historyItems.asStateFlow() + + init { + logcat(LogPriority.DEBUG, TAG) { "init" } + viewModelScope.launch { + historyRepository.refresh() + cacheManager.checkCacheStatus() + } + + historyRepository.historyEntries + .combine(cacheManager.cacheState) { entries, _ -> entries } + .combine(cacheRefreshEvents) { entries, _ -> entries } + .combine(downloadStates) { entries, states -> entries.toUiModels(states) } + .onEach { _historyItems.value = it } + .launchIn(viewModelScope) + } + + fun refreshCachedDownloadStates() { + logcat(LogPriority.DEBUG, TAG) { "refreshCachedDownloadStates called" } + cacheManager.checkCacheStatus() + cacheRefreshEvents.update { it + 1 } + } + + fun download(historyItem: HistoryItemUiModel) { + val entry = historyItem.entry + val currentState = downloadStates.value[entry.uniqueKey] ?: historyItem.downloadState + logcat(LogPriority.DEBUG, TAG) { + "download requested uniqueKey=${entry.uniqueKey}, currentState=${currentState.javaClass.simpleName}, " + + "historyItemState=${historyItem.downloadState.javaClass.simpleName}" + } + when (currentState) { + is DownloadState.InProgress -> { + logcat(LogPriority.DEBUG, TAG) { + "download ignored because already in progress uniqueKey=${entry.uniqueKey}" + } + return + } + is DownloadState.Downloaded -> { + if (currentState.file.exists()) { + logcat(LogPriority.DEBUG, TAG) { + "download ignored because remembered file exists uniqueKey=${entry.uniqueKey}, " + + "path=${currentState.file.absolutePath}, length=${currentState.file.length()}" + } + return + } + logcat(LogPriority.DEBUG, TAG) { + "download retrying because remembered file is missing uniqueKey=${entry.uniqueKey}, " + + "path=${currentState.file.absolutePath}" + } + updateDownloadState(entry.uniqueKey, DownloadState.NotDownloaded) + } + else -> Unit + } + + viewModelScope.launch(ioDispatcher) { + updateDownloadState(entry.uniqueKey, DownloadState.InProgress(0f)) + val outputFile = getCachedFile(entry).selectedFile + outputFile.parentFile?.mkdirs() + logcat(LogPriority.DEBUG, TAG) { + "download started uniqueKey=${entry.uniqueKey}, url=${entry.downloadUrl}, " + + "outputPath=${outputFile.absolutePath}, parentExists=${outputFile.parentFile?.exists()}, " + + "preExisting=${outputFile.exists()}, preExistingLength=${outputFile.length()}" + } + + when ( + val result = downloadFileRepository.downloadFile( + downloadUrl = entry.downloadUrl, + outputFile = outputFile, + onProgress = { bytesDownloaded, totalBytes -> + val progress = if (totalBytes > 0) { + bytesDownloaded.toFloat() / totalBytes.toFloat() + } else { + 0f + } + updateDownloadState(entry.uniqueKey, DownloadState.InProgress(progress)) + }, + ) + ) { + is NetworkResult.Success -> { + logcat(LogPriority.DEBUG, TAG) { + "download repository success uniqueKey=${entry.uniqueKey}, " + + "resultPath=${result.data.absolutePath}, resultExists=${result.data.exists()}, " + + "resultLength=${result.data.length()}, outputExists=${outputFile.exists()}, " + + "outputLength=${outputFile.length()}, parentExists=${outputFile.parentFile?.exists()}" + } + val downloadedFile = result.data.takeIf { it.exists() } ?: outputFile.takeIf { it.exists() } + if (downloadedFile == null) { + logcat(LogPriority.ERROR, TAG) { + "download success but file is missing uniqueKey=${entry.uniqueKey}, " + + "resultPath=${result.data.absolutePath}, outputPath=${outputFile.absolutePath}" + } + updateDownloadState(entry.uniqueKey, DownloadState.DownloadFailed("Downloaded file is missing")) + } else { + logcat(LogPriority.DEBUG, TAG) { + "download marked downloaded uniqueKey=${entry.uniqueKey}, " + + "path=${downloadedFile.absolutePath}, length=${downloadedFile.length()}" + } + updateDownloadState(entry.uniqueKey, DownloadState.Downloaded(downloadedFile)) + } + cacheManager.checkCacheStatus() + cacheRefreshEvents.update { it + 1 } + } + is NetworkResult.Error -> { + logcat(LogPriority.ERROR, TAG) { + "download repository error uniqueKey=${entry.uniqueKey}, message=${result.message}" + } + updateDownloadState(entry.uniqueKey, DownloadState.DownloadFailed(result.message)) + cacheManager.checkCacheStatus() + cacheRefreshEvents.update { it + 1 } + } + } + } + } + + fun install(historyItem: HistoryItemUiModel, file: File) { + viewModelScope.launch { + try { + historyRepository.recordInstallerLaunch( + historyItem.entry.copy(lastInstallerLaunchTimestamp = currentTimeMillisProvider()), + ) + } catch (_: Exception) { + // History is best-effort; never block installation. + } + intentManager.installApk(file) + } + } + + private fun updateDownloadState(uniqueKey: String, downloadState: DownloadState) { + downloadStates.update { it + (uniqueKey to downloadState) } + } + + private fun List.toUiModels( + states: Map, + ): List = + map { entry -> + val cacheResolution = getCachedFile(entry) + val rememberedState = states[entry.uniqueKey] + val downloadState = when { + rememberedState is DownloadState.InProgress -> rememberedState + rememberedState is DownloadState.DownloadFailed -> rememberedState + + rememberedState is DownloadState.Downloaded && rememberedState.file.exists() -> rememberedState + cacheResolution.selectedFile.exists() -> DownloadState.Downloaded(cacheResolution.selectedFile) + else -> DownloadState.NotDownloaded + } + logcat(LogPriority.DEBUG, TAG) { + "history item resolved uniqueKey=${entry.uniqueKey}, taskId=${entry.taskId}, " + + "artifactName=${entry.artifactName}, artifactFileName=${entry.artifactFileName}, " + + "jobSymbol=${entry.jobSymbol}, cacheRelativePath=${entry.cacheRelativePath}, " + + "relativePath=${cacheResolution.relativePathFile.absolutePath}, " + + "relativeExists=${cacheResolution.relativePathFile.exists()}, " + + "fallbackPath=${cacheResolution.fallbackFile.absolutePath}, " + + "fallbackExists=${cacheResolution.fallbackFile.exists()}, " + + "selectedPath=${cacheResolution.selectedFile.absolutePath}, " + + "rememberedState=${rememberedState?.javaClass?.simpleName}, " + + "resolvedState=${downloadState.javaClass.simpleName}" + } + + HistoryItemUiModel( + entry = entry, + downloadState = downloadState, + ) + } + + private fun getCachedFile(entry: TreeherderInstallHistoryEntry): CacheResolution { + val treeherderCacheDir = cacheManager.getCacheDir(TREEHERDER) + val relativePathFile = File(treeherderCacheDir.parentFile, entry.cacheRelativePath) + val fallbackFile = File(treeherderCacheDir, "${entry.taskId}/${entry.artifactFileName}") + val selectedFile = if (entry.cacheRelativePath.isNotBlank() && relativePathFile.exists()) { + relativePathFile + } else { + fallbackFile + } + return CacheResolution( + relativePathFile = relativePathFile, + fallbackFile = fallbackFile, + selectedFile = selectedFile, + ) + } + + private data class CacheResolution( + val relativePathFile: File, + val fallbackFile: File, + val selectedFile: File, + ) +} diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/HomeScreen.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/HomeScreen.kt index 7d1a18f..60c80de 100644 --- a/app/src/main/java/org/mozilla/tryfox/ui/screens/HomeScreen.kt +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/HomeScreen.kt @@ -13,6 +13,7 @@ import androidx.compose.material.ExperimentalMaterialApi import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.AccountCircle import androidx.compose.material.icons.filled.CameraAlt +import androidx.compose.material.icons.filled.History import androidx.compose.material.icons.filled.Search import androidx.compose.material.pullrefresh.PullRefreshIndicator import androidx.compose.material.pullrefresh.pullRefresh @@ -60,6 +61,7 @@ import java.io.File * @param modifier The modifier to be applied to the component. * @param onNavigateToTreeherder Callback to navigate to the Treeherder search screen. * @param onNavigateToProfile Callback to navigate to the Profile screen. + * @param onNavigateToHistory Callback to navigate to the History screen. * @param homeViewModel The ViewModel for the Home screen. */ @OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterialApi::class) @@ -69,6 +71,7 @@ fun HomeScreen( onNavigateToTreeherder: () -> Unit, onNavigateToProfile: () -> Unit, onNavigateToQrScanner: () -> Unit, + onNavigateToHistory: () -> Unit, homeViewModel: HomeViewModel = viewModel(), ) { val screenState by homeViewModel.homeScreenState.collectAsState() @@ -97,6 +100,12 @@ fun HomeScreen( actionIconContentColor = MaterialTheme.colorScheme.onPrimaryContainer, ), actions = { + IconButton(onClick = onNavigateToHistory) { + Icon( + imageVector = Icons.Filled.History, + contentDescription = stringResource(id = R.string.home_history_button_description), + ) + } IconButton(onClick = onNavigateToQrScanner) { Icon( imageVector = Icons.Filled.CameraAlt, diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/ProfileViewModel.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/ProfileViewModel.kt index c0fed7d..b18cba3 100644 --- a/app/src/main/java/org/mozilla/tryfox/ui/screens/ProfileViewModel.kt +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/ProfileViewModel.kt @@ -16,9 +16,11 @@ import logcat.LogPriority import logcat.logcat import org.mozilla.tryfox.data.DownloadState import org.mozilla.tryfox.data.NetworkResult +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry import org.mozilla.tryfox.data.managers.CacheManager import org.mozilla.tryfox.data.managers.IntentManager import org.mozilla.tryfox.data.repositories.DownloadFileRepository +import org.mozilla.tryfox.data.repositories.HistoryRepository import org.mozilla.tryfox.data.repositories.TreeherderRepository import org.mozilla.tryfox.data.repositories.UserDataRepository import org.mozilla.tryfox.model.CacheManagementState @@ -26,6 +28,7 @@ import org.mozilla.tryfox.ui.models.AbiUiModel import org.mozilla.tryfox.ui.models.ArtifactUiModel import org.mozilla.tryfox.ui.models.JobDetailsUiModel import org.mozilla.tryfox.ui.models.PushUiModel +import org.mozilla.tryfox.util.TREEHERDER import java.io.File /** @@ -43,7 +46,9 @@ class ProfileViewModel( private val userDataRepository: UserDataRepository, private val cacheManager: CacheManager, private val intentManager: IntentManager, + private val historyRepository: HistoryRepository, authorEmail: String?, + private val currentTimeMillisProvider: () -> Long = System::currentTimeMillis, ) : ViewModel() { companion object { @@ -268,9 +273,12 @@ class ProfileViewModel( val outputFile = File(taskSpecificDir, artifactName) val exists = outputFile.exists() logcat( - LogPriority.VERBOSE, + LogPriority.DEBUG, TAG, - ) { "getDownloadedFile for $artifactName in $taskId: exists=$exists" } + ) { + "getDownloadedFile artifactName=$artifactName, taskId=$taskId, " + + "path=${outputFile.absolutePath}, exists=$exists" + } return if (exists) outputFile else null } @@ -403,7 +411,20 @@ class ProfileViewModel( } fun installApk(file: File) { - intentManager.installApk(file) + val downloadedArtifact = findDownloadedArtifact(file) + if (downloadedArtifact == null) { + intentManager.installApk(file) + return + } + + viewModelScope.launch { + try { + recordInstallerLaunch(downloadedArtifact) + } catch (_: Exception) { + // History is best-effort; never block installation. + } + intentManager.installApk(file) + } } private fun updateArtifactDownloadState( @@ -445,4 +466,49 @@ class ProfileViewModel( } }, ) + + private fun findDownloadedArtifact(file: File): DownloadedArtifact? = + _pushes.value.firstNotNullOfOrNull { push -> + push.jobs.firstNotNullOfOrNull { job -> + job.artifacts.firstOrNull { artifact -> + val downloadState = artifact.downloadState + downloadState is DownloadState.Downloaded && downloadState.file.absolutePath == file.absolutePath + }?.let { artifact -> DownloadedArtifact(push, job, artifact) } + } + } + + private suspend fun recordInstallerLaunch(downloadedArtifact: DownloadedArtifact) { + val push = downloadedArtifact.push + val job = downloadedArtifact.job + val artifact = downloadedArtifact.artifact + val artifactFileName = artifact.name.substringAfterLast('/') + + historyRepository.recordInstallerLaunch( + TreeherderInstallHistoryEntry( + project = "try", + revision = push.revision ?: "unknown_revision", + commitMessage = push.pushComment, + author = push.author, + pushTimestamp = push.pushTimestamp, + appName = job.appName, + jobName = job.jobName, + jobSymbol = job.jobSymbol, + taskId = artifact.taskId, + artifactName = artifact.name, + artifactFileName = artifactFileName, + downloadUrl = artifact.downloadUrl, + abiName = artifact.abi.name, + abiSupported = artifact.abi.isSupported, + expires = artifact.expires, + cacheRelativePath = "$TREEHERDER/${artifact.taskId}/$artifactFileName", + lastInstallerLaunchTimestamp = currentTimeMillisProvider(), + ), + ) + } + + private data class DownloadedArtifact( + val push: PushUiModel, + val job: JobDetailsUiModel, + val artifact: ArtifactUiModel, + ) } diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreen.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreen.kt index 9d43bab..13eb97b 100644 --- a/app/src/main/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreen.kt +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/TreeherderApksScreen.kt @@ -45,6 +45,7 @@ import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.rememberTooltipState import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue @@ -53,10 +54,13 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalLifecycleOwner import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver import org.mozilla.tryfox.R import org.mozilla.tryfox.TryFoxViewModel import org.mozilla.tryfox.model.CacheManagementState @@ -86,11 +90,24 @@ fun TryFoxMainScreen( ) { val cacheState by tryFoxViewModel.cacheState.collectAsState() val isDownloading by tryFoxViewModel.isDownloadingAnyFile.collectAsState() + val lifecycleOwner = LocalLifecycleOwner.current LaunchedEffect(Unit) { tryFoxViewModel.checkCacheStatus() } + DisposableEffect(lifecycleOwner, tryFoxViewModel) { + val observer = LifecycleEventObserver { _, event -> + if (event == Lifecycle.Event.ON_RESUME) { + tryFoxViewModel.checkCacheStatus() + } + } + lifecycleOwner.lifecycle.addObserver(observer) + onDispose { + lifecycleOwner.lifecycle.removeObserver(observer) + } + } + LaunchedEffect(deepLinkProject, deepLinkRevision) { if (!deepLinkRevision.isNullOrBlank()) { val resolvedProject = deepLinkProject ?: "try" diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 14f86ee..65b4b43 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -2,6 +2,7 @@ TryFox Profile Search Treeherder + History Scan QR code Fetching latest nightly builds... Loading initial app data... @@ -54,6 +55,11 @@ Search pushes by email Enter a user email and tap search to find pushes. Clear email field + History + No installs yet + Treeherder APKs appear here after TryFox opens the installer. + Revision: %1$s + Last installed: %1$s Fenix Fenix Release diff --git a/app/src/main/res/xml/backup_rules.xml b/app/src/main/res/xml/backup_rules.xml index 4df9255..0511290 100644 --- a/app/src/main/res/xml/backup_rules.xml +++ b/app/src/main/res/xml/backup_rules.xml @@ -6,8 +6,9 @@ See https://developer.android.com/about/versions/12/backup-restore --> + - \ No newline at end of file + diff --git a/app/src/main/res/xml/data_extraction_rules.xml b/app/src/main/res/xml/data_extraction_rules.xml index 9ee9997..1f33e8e 100644 --- a/app/src/main/res/xml/data_extraction_rules.xml +++ b/app/src/main/res/xml/data_extraction_rules.xml @@ -5,15 +5,13 @@ --> + - - \ No newline at end of file + diff --git a/app/src/main/res/xml/provider_paths.xml b/app/src/main/res/xml/provider_paths.xml index 4d4a513..0d76511 100644 --- a/app/src/main/res/xml/provider_paths.xml +++ b/app/src/main/res/xml/provider_paths.xml @@ -1,4 +1,5 @@ - \ No newline at end of file + + diff --git a/app/src/test/java/org/mozilla/tryfox/TryFoxViewModelTest.kt b/app/src/test/java/org/mozilla/tryfox/TryFoxViewModelTest.kt index d1466f7..78f0e4c 100644 --- a/app/src/test/java/org/mozilla/tryfox/TryFoxViewModelTest.kt +++ b/app/src/test/java/org/mozilla/tryfox/TryFoxViewModelTest.kt @@ -16,6 +16,9 @@ import org.junit.jupiter.api.extension.RegisterExtension import org.junit.jupiter.api.io.TempDir import org.mozilla.tryfox.data.Artifact import org.mozilla.tryfox.data.ArtifactsResponse +import org.mozilla.tryfox.data.DownloadState +import org.mozilla.tryfox.data.FakeDownloadFileRepository +import org.mozilla.tryfox.data.FakeHistoryRepository import org.mozilla.tryfox.data.JobDetails import org.mozilla.tryfox.data.NetworkResult import org.mozilla.tryfox.data.RevisionDetail @@ -243,17 +246,84 @@ class TryFoxViewModelTest { assertFalse(artifacts.single { it.abi.name == "x86_64" }.abi.isSupported) } + @Test + fun `installApk records Treeherder history entry for downloaded artifact`() = runTest { + val job = signedJob(taskId = "history-task", jobName = "signing-apk-fenix-nightly", appName = "fenix") + val repository = FakeTestTreeherderRepository( + pages = mapOf(1 to listOf(job)), + artifactsByTaskId = mapOf("history-task" to listOf(apkArtifact("public/build/target.arm64-v8a.apk"))), + ) + val historyRepository = FakeHistoryRepository() + val intentManager = FakeIntentManager() + val viewModel = createViewModel( + repository = repository, + historyRepository = historyRepository, + intentManager = intentManager, + currentTimeMillisProvider = { 123L }, + ) + + viewModel.updateRevision("ed209aa2136b241686ff20489c5cb622348e2ecf") + viewModel.searchJobsAndArtifacts() + advanceUntilIdle() + viewModel.downloadArtifact(viewModel.selectedJobs.single().artifacts.single()) + advanceUntilIdle() + + val downloadedArtifact = viewModel.selectedJobs.single().artifacts.single() + val downloadedFile = (downloadedArtifact.downloadState as DownloadState.Downloaded).file + viewModel.installApk(downloadedFile) + advanceUntilIdle() + + val historyEntry = historyRepository.recordedEntries.single() + assertEquals("signing-apk-fenix-nightly", historyEntry.jobName) + assertEquals("Bug 2001527: test patch", historyEntry.commitMessage) + assertEquals("arm64-v8a", historyEntry.abiName) + assertEquals(123L, historyEntry.lastInstallerLaunchTimestamp) + assertTrue(intentManager.wasInstallApkCalled) + } + + @Test + fun `checkCacheStatus demotes downloaded Treeherder artifact when cached apk is missing`() = runTest { + val job = signedJob(taskId = "history-task", jobName = "signing-apk-fenix-nightly", appName = "fenix") + val repository = FakeTestTreeherderRepository( + pages = mapOf(1 to listOf(job)), + artifactsByTaskId = mapOf("history-task" to listOf(apkArtifact("public/build/target.arm64-v8a.apk"))), + ) + val viewModel = createViewModel(repository = repository) + + viewModel.updateRevision("ed209aa2136b241686ff20489c5cb622348e2ecf") + viewModel.searchJobsAndArtifacts() + advanceUntilIdle() + viewModel.downloadArtifact(viewModel.selectedJobs.single().artifacts.single()) + advanceUntilIdle() + val downloadedArtifact = viewModel.selectedJobs.single().artifacts.single() + val downloadedFile = (downloadedArtifact.downloadState as DownloadState.Downloaded).file + assertTrue(downloadedFile.delete()) + + viewModel.checkCacheStatus() + advanceUntilIdle() + + assertTrue(viewModel.selectedJobs.single().artifacts.single().downloadState is DownloadState.NotDownloaded) + } + private fun createViewModel( repository: TreeherderRepository, + historyRepository: FakeHistoryRepository = FakeHistoryRepository(), + intentManager: FakeIntentManager = FakeIntentManager(), + downloadFileRepository: FakeDownloadFileRepository = FakeDownloadFileRepository( + downloadProgressDelayMillis = 0, + ), + currentTimeMillisProvider: () -> Long = { 0L }, ): TryFoxViewModel = TryFoxViewModel( fenixRepository = repository, - downloadFileRepository = org.mozilla.tryfox.data.FakeDownloadFileRepository(), + downloadFileRepository = downloadFileRepository, cacheManager = cacheManager, - intentManager = FakeIntentManager(), + intentManager = intentManager, + historyRepository = historyRepository, project = "mozilla-central", revision = null, supportedAbis = listOf("arm64-v8a"), elapsedRealtimeProvider = { 0L }, + currentTimeMillisProvider = currentTimeMillisProvider, infoLogger = { _, _ -> 0 }, ioDispatcher = mainCoroutineRule.testDispatcher, mainDispatcher = mainCoroutineRule.testDispatcher, diff --git a/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt b/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt new file mode 100644 index 0000000..cba8940 --- /dev/null +++ b/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt @@ -0,0 +1,34 @@ +package org.mozilla.tryfox.data + +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import org.mozilla.tryfox.data.repositories.HistoryRepository + +class FakeHistoryRepository : HistoryRepository { + private val _historyEntries = MutableStateFlow>(emptyList()) + override val historyEntries: StateFlow> = _historyEntries.asStateFlow() + + val recordedEntries = mutableListOf() + var refreshCalled = false + var failRecordInstallerLaunch = false + + override suspend fun refresh() { + refreshCalled = true + } + + override suspend fun recordInstallerLaunch(entry: TreeherderInstallHistoryEntry) { + if (failRecordInstallerLaunch) { + error("Failed to record history") + } + recordedEntries.removeAll { it.uniqueKey == entry.uniqueKey } + recordedEntries.add(entry) + _historyEntries.value = recordedEntries.sortedByDescending { it.lastInstallerLaunchTimestamp } + } + + fun setEntries(entries: List) { + recordedEntries.clear() + recordedEntries.addAll(entries) + _historyEntries.value = entries + } +} diff --git a/app/src/test/java/org/mozilla/tryfox/data/managers/DefaultCacheManagerTest.kt b/app/src/test/java/org/mozilla/tryfox/data/managers/DefaultCacheManagerTest.kt new file mode 100644 index 0000000..52d5bdb --- /dev/null +++ b/app/src/test/java/org/mozilla/tryfox/data/managers/DefaultCacheManagerTest.kt @@ -0,0 +1,38 @@ +package org.mozilla.tryfox.data.managers + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File + +class DefaultCacheManagerTest { + + @TempDir + lateinit var tempDir: File + + @Test + fun `initialization migrates legacy cache directories to managed cache root`() { + val legacyRoot = File(tempDir, "legacy-cache") + val managedRoot = File(tempDir, "download-cache") + val legacyApk = File(legacyRoot, "treeherder/task-id/target.apk") + val unrelatedCacheFile = File(legacyRoot, "image_cache/cached-image") + legacyApk.parentFile?.mkdirs() + legacyApk.writeText("cached apk") + unrelatedCacheFile.parentFile?.mkdirs() + unrelatedCacheFile.writeText("unrelated cache") + + DefaultCacheManager( + cacheDir = managedRoot, + legacyCacheDir = legacyRoot, + ) + + val migratedApk = File(managedRoot, "treeherder/task-id/target.apk") + assertTrue(migratedApk.exists()) + assertEquals("cached apk", migratedApk.readText()) + assertFalse(legacyApk.exists()) + assertTrue(unrelatedCacheFile.exists()) + assertFalse(File(managedRoot, "image_cache/cached-image").exists()) + } +} diff --git a/app/src/test/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepositoryTest.kt b/app/src/test/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepositoryTest.kt new file mode 100644 index 0000000..c02f252 --- /dev/null +++ b/app/src/test/java/org/mozilla/tryfox/data/repositories/DefaultDownloadFileRepositoryTest.kt @@ -0,0 +1,231 @@ +package org.mozilla.tryfox.data.repositories + +import kotlinx.coroutines.test.runTest +import okhttp3.MediaType +import okhttp3.ResponseBody +import okhttp3.ResponseBody.Companion.toResponseBody +import okio.buffer +import okio.source +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import org.mozilla.tryfox.data.NetworkResult +import org.mozilla.tryfox.network.DownloadApiService +import java.io.File + +class DefaultDownloadFileRepositoryTest { + + @TempDir + lateinit var tempDir: File + + @Test + fun `successful download moves complete partial file into final path`() = runTest { + val outputFile = File(tempDir, "target.apk") + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService("complete apk".toResponseBody()), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Success) + assertEquals("complete apk", outputFile.readText()) + assertFalse(File(tempDir, "target.apk.part").exists()) + } + + @Test + fun `incomplete download removes partial file and does not create final path`() = runTest { + val outputFile = File(tempDir, "target.apk") + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService( + IncompleteResponseBody( + declaredLength = 10L, + bytes = "short".toByteArray(), + ), + ), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Error) + assertFalse(outputFile.exists()) + assertFalse(File(tempDir, "target.apk.part").exists()) + } + + @Test + fun `successful download replaces existing final file and removes backup`() = runTest { + val outputFile = File(tempDir, "target.apk") + outputFile.writeText("old apk") + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService("new apk".toResponseBody()), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Success) + assertEquals("new apk", outputFile.readText()) + assertFalse(File(tempDir, "target.apk.part").exists()) + assertFalse(File(tempDir, "target.apk.bak").exists()) + } + + @Test + fun `missing final file is restored from backup before retrying download`() = runTest { + val outputFile = File(tempDir, "target.apk") + val backupFile = File(tempDir, "target.apk.bak") + backupFile.writeText("old apk") + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService( + IncompleteResponseBody( + declaredLength = 10L, + bytes = "short".toByteArray(), + ), + ), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Error) + assertEquals("old apk", outputFile.readText()) + assertFalse(File(tempDir, "target.apk.part").exists()) + assertFalse(backupFile.exists()) + } + + @Test + fun `missing final file is restored from latest backup when alternate backups exist`() = runTest { + val outputFile = File(tempDir, "target.apk") + val staleBackupFile = File(tempDir, "target.apk.bak") + val latestBackupFile = File(tempDir, "target.apk.bak.1") + staleBackupFile.writeText("stale apk") + latestBackupFile.writeText("latest apk") + staleBackupFile.setLastModified(1_000L) + latestBackupFile.setLastModified(2_000L) + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService( + IncompleteResponseBody( + declaredLength = 10L, + bytes = "short".toByteArray(), + ), + ), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Error) + assertEquals("latest apk", outputFile.readText()) + assertFalse(staleBackupFile.exists()) + assertFalse(latestBackupFile.exists()) + } + + @Test + fun `missing final file prefers numbered backup over newer plain stale backup`() = runTest { + val outputFile = File(tempDir, "target.apk") + val staleBackupFile = File(tempDir, "target.apk.bak") + val latestBackupFile = File(tempDir, "target.apk.bak.1") + staleBackupFile.writeText("stale apk") + latestBackupFile.writeText("latest apk") + staleBackupFile.setLastModified(2_000L) + latestBackupFile.setLastModified(1_000L) + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService( + IncompleteResponseBody( + declaredLength = 10L, + bytes = "short".toByteArray(), + ), + ), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Error) + assertEquals("latest apk", outputFile.readText()) + assertFalse(staleBackupFile.exists()) + assertFalse(latestBackupFile.exists()) + } + + @Test + fun `missing final file ignores non managed backup suffixes`() = runTest { + val outputFile = File(tempDir, "target.apk") + val nonManagedBackupFile = File(tempDir, "target.apk.bak.tmp") + nonManagedBackupFile.writeText("not managed") + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService( + IncompleteResponseBody( + declaredLength = 10L, + bytes = "short".toByteArray(), + ), + ), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Error) + assertFalse(outputFile.exists()) + assertTrue(nonManagedBackupFile.exists()) + } + + @Test + fun `successful download tolerates stale backup when final file exists`() = runTest { + val outputFile = File(tempDir, "target.apk") + val backupFile = File(tempDir, "target.apk.bak") + outputFile.writeText("current apk") + backupFile.writeText("stale apk") + val repository = DefaultDownloadFileRepository( + downloadApiService = FakeDownloadApiService("new apk".toResponseBody()), + ) + + val result = repository.downloadFile( + downloadUrl = "https://example.com/target.apk", + outputFile = outputFile, + onProgress = { _, _ -> }, + ) + + assertTrue(result is NetworkResult.Success) + assertEquals("new apk", outputFile.readText()) + assertFalse(File(tempDir, "target.apk.part").exists()) + assertFalse(backupFile.exists()) + } + + private class FakeDownloadApiService( + private val responseBody: ResponseBody, + ) : DownloadApiService { + override suspend fun downloadFile(downloadUrl: String): ResponseBody = responseBody + } + + private class IncompleteResponseBody( + private val declaredLength: Long, + private val bytes: ByteArray, + ) : ResponseBody() { + override fun contentType(): MediaType? = null + override fun contentLength(): Long = declaredLength + override fun source() = bytes.inputStream().source().buffer() + } +} diff --git a/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt b/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt new file mode 100644 index 0000000..7fab159 --- /dev/null +++ b/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt @@ -0,0 +1,246 @@ +package org.mozilla.tryfox.ui.screens + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import org.junit.jupiter.api.io.TempDir +import org.mozilla.tryfox.data.DownloadState +import org.mozilla.tryfox.data.FakeDownloadFileRepository +import org.mozilla.tryfox.data.FakeHistoryRepository +import org.mozilla.tryfox.data.TreeherderInstallHistoryEntry +import org.mozilla.tryfox.data.managers.FakeCacheManager +import org.mozilla.tryfox.data.managers.FakeIntentManager +import java.io.File + +@OptIn(ExperimentalCoroutinesApi::class) +class HistoryViewModelTest { + + @JvmField + @RegisterExtension + val mainCoroutineRule = MainCoroutineRule() + + @TempDir + lateinit var tempCacheDir: File + + @Test + fun `history item uses downloaded state when apk exists in cache`() = runTest { + val entry = historyEntry() + val cacheManager = FakeCacheManager(tempCacheDir) + val cachedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + cachedFile.parentFile?.mkdirs() + cachedFile.writeText("cached apk") + + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) }, + ) + advanceUntilIdle() + + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.Downloaded) + } + + @Test + fun `refreshing cache state marks history item downloaded when apk was downloaded elsewhere`() = runTest { + val entry = historyEntry() + val cacheManager = FakeCacheManager(tempCacheDir) + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) }, + ) + advanceUntilIdle() + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.NotDownloaded) + + val cachedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + cachedFile.parentFile?.mkdirs() + cachedFile.writeText("cached apk") + viewModel.refreshCachedDownloadStates() + advanceUntilIdle() + + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.Downloaded) + } + + @Test + fun `download uses stored url and writes apk to treeherder cache`() = runTest { + val entry = historyEntry(downloadUrl = "https://example.com/artifact.apk") + val cacheManager = FakeCacheManager(tempCacheDir) + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) }, + ) + advanceUntilIdle() + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + + val downloadedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + assertTrue(downloadedFile.exists()) + assertTrue(downloadedFile.readText().contains(entry.downloadUrl)) + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.Downloaded) + } + + @Test + fun `download retries when rendered item is not downloaded but remembered downloaded file is missing`() = runTest { + val entry = historyEntry(downloadUrl = "https://example.com/artifact.apk") + val cacheManager = FakeCacheManager(tempCacheDir) + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) }, + ) + advanceUntilIdle() + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + val downloadedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + assertTrue(downloadedFile.delete()) + viewModel.refreshCachedDownloadStates() + advanceUntilIdle() + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.NotDownloaded) + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + + assertTrue(downloadedFile.exists()) + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.Downloaded) + } + + @Test + fun `in progress download state is kept even if output file already exists`() = runTest { + val entry = historyEntry() + val cacheManager = FakeCacheManager(tempCacheDir) + val blockingDownloadRepository = BlockingDownloadFileRepository() + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) }, + downloadFileRepository = blockingDownloadRepository, + ) + advanceUntilIdle() + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + + val cachedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + assertTrue(cachedFile.exists()) + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.InProgress) + + blockingDownloadRepository.complete() + advanceUntilIdle() + + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.Downloaded) + } + + @Test + fun `install records a fresh installer launch timestamp before launching installer`() = runTest { + val entry = historyEntry(lastInstallerLaunchTimestamp = 1L) + val historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) } + val intentManager = FakeIntentManager() + val cacheManager = FakeCacheManager(tempCacheDir) + val cachedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + cachedFile.parentFile?.mkdirs() + cachedFile.writeText("cached apk") + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = historyRepository, + intentManager = intentManager, + currentTimeMillisProvider = { 123L }, + ) + advanceUntilIdle() + + viewModel.install(viewModel.historyItems.value.single(), cachedFile) + advanceUntilIdle() + + assertEquals(123L, historyRepository.recordedEntries.single().lastInstallerLaunchTimestamp) + assertTrue(intentManager.wasInstallApkCalled) + } + + @Test + fun `install still launches installer when history recording fails`() = runTest { + val entry = historyEntry() + val historyRepository = FakeHistoryRepository().apply { + setEntries(listOf(entry)) + failRecordInstallerLaunch = true + } + val intentManager = FakeIntentManager() + val cacheManager = FakeCacheManager(tempCacheDir) + val cachedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + cachedFile.parentFile?.mkdirs() + cachedFile.writeText("cached apk") + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = historyRepository, + intentManager = intentManager, + ) + advanceUntilIdle() + + viewModel.install(viewModel.historyItems.value.single(), cachedFile) + advanceUntilIdle() + + assertTrue(intentManager.wasInstallApkCalled) + } + + private fun createViewModel( + cacheManager: FakeCacheManager, + historyRepository: FakeHistoryRepository, + downloadFileRepository: org.mozilla.tryfox.data.repositories.DownloadFileRepository = + FakeDownloadFileRepository(downloadProgressDelayMillis = 0), + intentManager: FakeIntentManager = FakeIntentManager(), + currentTimeMillisProvider: () -> Long = { 0L }, + ): HistoryViewModel = + HistoryViewModel( + historyRepository = historyRepository, + downloadFileRepository = downloadFileRepository, + cacheManager = cacheManager, + intentManager = intentManager, + ioDispatcher = mainCoroutineRule.testDispatcher, + currentTimeMillisProvider = currentTimeMillisProvider, + ) + + private fun historyEntry( + downloadUrl: String = "https://example.com/task/artifact", + lastInstallerLaunchTimestamp: Long = 10L, + ): TreeherderInstallHistoryEntry = + TreeherderInstallHistoryEntry( + project = "try", + revision = "abcdef123456", + commitMessage = "Bug 123 - Test history", + author = "author@mozilla.com", + pushTimestamp = 1_716_460_800L, + appName = "fenix", + jobName = "signing-apk-fenix-nightly", + jobSymbol = "B", + taskId = "task-id", + artifactName = "public/build/target.arm64-v8a.apk", + artifactFileName = "target.arm64-v8a.apk", + downloadUrl = downloadUrl, + abiName = "arm64-v8a", + abiSupported = true, + expires = "2026-01-01T00:00:00.000Z", + cacheRelativePath = "treeherder/task-id/target.arm64-v8a.apk", + lastInstallerLaunchTimestamp = lastInstallerLaunchTimestamp, + ) + + private class BlockingDownloadFileRepository : org.mozilla.tryfox.data.repositories.DownloadFileRepository { + private val completion = kotlinx.coroutines.CompletableDeferred() + + override suspend fun downloadFile( + downloadUrl: String, + outputFile: File, + onProgress: (bytesDownloaded: Long, totalBytes: Long) -> Unit, + ): org.mozilla.tryfox.data.NetworkResult { + outputFile.parentFile?.mkdirs() + outputFile.writeText("partial") + onProgress(1L, 10L) + completion.await() + outputFile.writeText("complete") + onProgress(10L, 10L) + return org.mozilla.tryfox.data.NetworkResult.Success(outputFile) + } + + fun complete() { + completion.complete(Unit) + } + } +} diff --git a/app/src/test/java/org/mozilla/tryfox/ui/screens/ProfileViewModelTest.kt b/app/src/test/java/org/mozilla/tryfox/ui/screens/ProfileViewModelTest.kt index 2682f1d..4ba91b3 100644 --- a/app/src/test/java/org/mozilla/tryfox/ui/screens/ProfileViewModelTest.kt +++ b/app/src/test/java/org/mozilla/tryfox/ui/screens/ProfileViewModelTest.kt @@ -12,6 +12,7 @@ import org.junit.jupiter.api.io.TempDir import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension import org.mozilla.tryfox.data.FakeDownloadFileRepository +import org.mozilla.tryfox.data.FakeHistoryRepository import org.mozilla.tryfox.data.managers.FakeCacheManager import org.mozilla.tryfox.data.managers.FakeIntentManager import org.mozilla.tryfox.data.managers.FakeUserDataRepository @@ -34,6 +35,8 @@ class ProfileViewModelTest { private val intentManager = FakeIntentManager() + private val historyRepository = FakeHistoryRepository() + @TempDir lateinit var tempCacheDir: File @@ -47,6 +50,7 @@ class ProfileViewModelTest { userDataRepository = userDataRepository, cacheManager = cacheManager, intentManager = intentManager, + historyRepository = historyRepository, authorEmail = null, ) } @@ -59,7 +63,15 @@ class ProfileViewModelTest { @Test fun `updateAuthorEmail should update the authorEmail state`() = runTest { // Given - val viewModel = ProfileViewModel(fenixRepository, downloadFileRepository, userDataRepository, cacheManager, intentManager, null) + val viewModel = ProfileViewModel( + fenixRepository, + downloadFileRepository, + userDataRepository, + cacheManager, + intentManager, + historyRepository, + null, + ) val newEmail = "test@example.com" viewModel.authorEmail.test { From 8e6821fc2aad39f0a1e414884c43fc2df0908abf Mon Sep 17 00:00:00 2001 From: Titouan Thibaud Date: Wed, 27 May 2026 16:04:11 +0200 Subject: [PATCH 2/2] Add history entry removal Add a remove action to History cards and persist deletion through the HistoryRepository so removing an entry drops the card and deletes the corresponding database row. Cancel active History downloads when their entry is removed, clear related APK cache files, and guard late callbacks with per-download generations. Reinserted same-key entries render as an indeterminate in-progress state while the old canceled download is still finishing so the Download button cannot start a competing writer. Tests cover repository-backed deletion, card removal, active-download cancellation, stale callback suppression, managed cache file cleanup, and same-key retry behavior after cancellation completes. --- .../tryfox/data/FakeHistoryRepository.kt | 5 + .../tryfox/ui/screens/HistoryScreenTest.kt | 40 ++++ .../repositories/DefaultHistoryRepository.kt | 12 ++ .../data/repositories/HistoryRepository.kt | 2 + .../tryfox/ui/screens/HistoryScreen.kt | 34 ++++ .../tryfox/ui/screens/HistoryViewModel.kt | 158 ++++++++++++++- app/src/main/res/values/strings.xml | 1 + .../tryfox/data/FakeHistoryRepository.kt | 5 + .../tryfox/ui/screens/HistoryViewModelTest.kt | 187 ++++++++++++++++++ 9 files changed, 438 insertions(+), 6 deletions(-) diff --git a/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt b/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt index 0b4bde4..3ea840f 100644 --- a/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt +++ b/app/src/androidTest/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt @@ -19,6 +19,11 @@ class FakeHistoryRepository : HistoryRepository { _historyEntries.value = recordedEntries.sortedByDescending { it.lastInstallerLaunchTimestamp } } + override suspend fun delete(uniqueKey: String) { + recordedEntries.removeAll { it.uniqueKey == uniqueKey } + _historyEntries.value = recordedEntries.sortedByDescending { it.lastInstallerLaunchTimestamp } + } + fun setEntries(entries: List) { recordedEntries.clear() recordedEntries.addAll(entries) diff --git a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt index 4378180..8b507a7 100644 --- a/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt +++ b/app/src/androidTest/java/org/mozilla/tryfox/ui/screens/HistoryScreenTest.kt @@ -1,13 +1,18 @@ package org.mozilla.tryfox.ui.screens +import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.tryfox.R import org.mozilla.tryfox.data.FakeCacheManager import org.mozilla.tryfox.data.FakeDownloadFileRepository import org.mozilla.tryfox.data.FakeHistoryRepository @@ -55,6 +60,41 @@ class HistoryScreenTest { assertEquals(revision, selectedRevision) } + @Test + fun clickingRemoveHistoryEntryRemovesCard() { + val revision = "abcdef1234567890" + val historyRepository = FakeHistoryRepository().apply { + setEntries(listOf(historyEntry(project = "mozilla-central", revision = revision))) + } + val historyViewModel = HistoryViewModel( + historyRepository = historyRepository, + downloadFileRepository = FakeDownloadFileRepository(), + cacheManager = FakeCacheManager(), + intentManager = FakeIntentManager(), + ) + + composeTestRule.setContent { + TryFoxTheme { + HistoryScreen( + onNavigateUp = {}, + onNavigateToTreeherderRevision = { _, _ -> }, + historyViewModel = historyViewModel, + ) + } + } + + composeTestRule + .onNodeWithContentDescription( + InstrumentationRegistry.getInstrumentation().targetContext.getString( + R.string.history_screen_delete_entry_description, + ), + ) + .performClick() + + composeTestRule.onAllNodesWithText("Revision: ${revision.take(12)}").assertCountEquals(0) + assertEquals(emptyList(), historyRepository.recordedEntries) + } + private fun historyEntry( project: String, revision: String, diff --git a/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt b/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt index 28bf3d5..d2ed334 100644 --- a/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt +++ b/app/src/main/java/org/mozilla/tryfox/data/repositories/DefaultHistoryRepository.kt @@ -49,6 +49,18 @@ class DefaultHistoryRepository( refresh() } + override suspend fun delete(uniqueKey: String) { + logcat(LogPriority.DEBUG, TAG) { "delete uniqueKey=$uniqueKey" } + withContext(ioDispatcher) { + dbHelper.writableDatabase.delete( + TABLE_HISTORY, + "$COLUMN_ID = ?", + arrayOf(uniqueKey), + ) + } + refresh() + } + private suspend fun loadEntries(): List = withContext(ioDispatcher) { dbHelper.readableDatabase.query( TABLE_HISTORY, diff --git a/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt b/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt index a7dccdd..6fb2aa8 100644 --- a/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt +++ b/app/src/main/java/org/mozilla/tryfox/data/repositories/HistoryRepository.kt @@ -9,4 +9,6 @@ interface HistoryRepository { suspend fun refresh() suspend fun recordInstallerLaunch(entry: TreeherderInstallHistoryEntry) + + suspend fun delete(uniqueKey: String) } diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt index 9f6e690..d1952b7 100644 --- a/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryScreen.kt @@ -2,6 +2,7 @@ package org.mozilla.tryfox.ui.screens import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row @@ -13,9 +14,11 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.filled.Close import androidx.compose.material3.CardDefaults import androidx.compose.material3.ElevatedCard import androidx.compose.material3.ExperimentalMaterial3Api @@ -35,6 +38,7 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalLifecycleOwner import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource @@ -61,6 +65,7 @@ import org.mozilla.tryfox.ui.models.HistoryItemUiModel internal const val HISTORY_EMPTY_STATE_TAG = "history_empty_state" internal const val HISTORY_LIST_TAG = "history_list" +internal const val HISTORY_DELETE_BUTTON_TAG = "history_delete_button" private const val HISTORY_SCREEN_LOG_TAG = "HistoryScreen" @OptIn(ExperimentalMaterial3Api::class) @@ -151,6 +156,7 @@ fun HistoryScreen( onRevisionClick = onNavigateToTreeherderRevision, onDownloadClick = { historyViewModel.download(historyItem) }, onInstallClick = { file -> historyViewModel.install(historyItem, file) }, + onDeleteClick = { historyViewModel.delete(historyItem) }, ) } } @@ -164,6 +170,7 @@ private fun HistoryCard( onRevisionClick: (project: String, revision: String) -> Unit, onDownloadClick: () -> Unit, onInstallClick: (java.io.File) -> Unit, + onDeleteClick: () -> Unit, ) { val entry = historyItem.entry @@ -205,6 +212,8 @@ private fun HistoryCard( color = MaterialTheme.colorScheme.primary, ) } + Spacer(modifier = Modifier.width(8.dp)) + DeleteHistoryButton(onClick = onDeleteClick) } PushCommentCard( @@ -246,6 +255,31 @@ private fun HistoryCard( } } +@Composable +private fun DeleteHistoryButton(onClick: () -> Unit) { + IconButton( + onClick = onClick, + modifier = Modifier + .size(32.dp) + .testTag(HISTORY_DELETE_BUTTON_TAG), + ) { + Surface( + modifier = Modifier.size(24.dp), + shape = CircleShape, + color = Color(0xFFD32F2F), + contentColor = Color.White, + ) { + Box(contentAlignment = Alignment.Center) { + Icon( + imageVector = Icons.Filled.Close, + contentDescription = stringResource(id = R.string.history_screen_delete_entry_description), + modifier = Modifier.size(16.dp), + ) + } + } + } +} + @Composable private fun JobSymbolChip(symbol: String) { Surface( diff --git a/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt index 4822749..ac5469d 100644 --- a/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt +++ b/app/src/main/java/org/mozilla/tryfox/ui/screens/HistoryViewModel.kt @@ -3,7 +3,9 @@ package org.mozilla.tryfox.ui.screens import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -39,7 +41,10 @@ class HistoryViewModel( } private val downloadStates = MutableStateFlow>(emptyMap()) + private val activeDownloads = MutableStateFlow>(emptyMap()) + private val canceledDownloads = MutableStateFlow>(emptyMap()) private val cacheRefreshEvents = MutableStateFlow(0) + private var nextDownloadGeneration = 0L private val _historyItems = MutableStateFlow>(emptyList()) val historyItems: StateFlow> = _historyItems.asStateFlow() @@ -72,6 +77,12 @@ class HistoryViewModel( "download requested uniqueKey=${entry.uniqueKey}, currentState=${currentState.javaClass.simpleName}, " + "historyItemState=${historyItem.downloadState.javaClass.simpleName}" } + if (canceledDownloads.value.keys.any { it.uniqueKey == entry.uniqueKey }) { + logcat(LogPriority.DEBUG, TAG) { + "download ignored because a canceled download is still finishing uniqueKey=${entry.uniqueKey}" + } + return + } when (currentState) { is DownloadState.InProgress -> { logcat(LogPriority.DEBUG, TAG) { @@ -96,8 +107,10 @@ class HistoryViewModel( else -> Unit } - viewModelScope.launch(ioDispatcher) { - updateDownloadState(entry.uniqueKey, DownloadState.InProgress(0f)) + val generation = nextDownloadGeneration++ + lateinit var downloadJob: Job + downloadJob = viewModelScope.launch(ioDispatcher, start = CoroutineStart.LAZY) { + updateDownloadStateIfActive(entry.uniqueKey, generation, DownloadState.InProgress(0f)) val outputFile = getCachedFile(entry).selectedFile outputFile.parentFile?.mkdirs() logcat(LogPriority.DEBUG, TAG) { @@ -116,7 +129,11 @@ class HistoryViewModel( } else { 0f } - updateDownloadState(entry.uniqueKey, DownloadState.InProgress(progress)) + updateDownloadStateIfActive( + entry.uniqueKey, + generation, + DownloadState.InProgress(progress), + ) }, ) ) { @@ -133,13 +150,21 @@ class HistoryViewModel( "download success but file is missing uniqueKey=${entry.uniqueKey}, " + "resultPath=${result.data.absolutePath}, outputPath=${outputFile.absolutePath}" } - updateDownloadState(entry.uniqueKey, DownloadState.DownloadFailed("Downloaded file is missing")) + updateDownloadStateIfActive( + entry.uniqueKey, + generation, + DownloadState.DownloadFailed("Downloaded file is missing"), + ) } else { logcat(LogPriority.DEBUG, TAG) { "download marked downloaded uniqueKey=${entry.uniqueKey}, " + "path=${downloadedFile.absolutePath}, length=${downloadedFile.length()}" } - updateDownloadState(entry.uniqueKey, DownloadState.Downloaded(downloadedFile)) + updateDownloadStateIfActive( + entry.uniqueKey, + generation, + DownloadState.Downloaded(downloadedFile), + ) } cacheManager.checkCacheStatus() cacheRefreshEvents.update { it + 1 } @@ -148,12 +173,41 @@ class HistoryViewModel( logcat(LogPriority.ERROR, TAG) { "download repository error uniqueKey=${entry.uniqueKey}, message=${result.message}" } - updateDownloadState(entry.uniqueKey, DownloadState.DownloadFailed(result.message)) + updateDownloadStateIfActive( + entry.uniqueKey, + generation, + DownloadState.DownloadFailed(result.message), + ) + cacheManager.checkCacheStatus() + cacheRefreshEvents.update { it + 1 } + } + } + } + val activeDownload = ActiveDownload( + job = downloadJob, + generation = generation, + entry = entry, + ) + val downloadIdentity = DownloadIdentity(entry.uniqueKey, generation) + activeDownloads.update { it + (entry.uniqueKey to activeDownload) } + downloadJob.invokeOnCompletion { + activeDownloads.update { downloads -> + if (downloads[entry.uniqueKey]?.generation == generation) { + downloads - entry.uniqueKey + } else { + downloads + } + } + canceledDownloads.value[downloadIdentity]?.let { canceledEntry -> + if (activeDownloads.value[canceledEntry.uniqueKey] == null) { + deleteDownloadFiles(canceledEntry) cacheManager.checkCacheStatus() cacheRefreshEvents.update { it + 1 } } + canceledDownloads.update { it - downloadIdentity } } } + downloadJob.start() } fun install(historyItem: HistoryItemUiModel, file: File) { @@ -169,17 +223,63 @@ class HistoryViewModel( } } + fun delete(historyItem: HistoryItemUiModel) { + val uniqueKey = historyItem.entry.uniqueKey + viewModelScope.launch(ioDispatcher) { + try { + activeDownloads.value[uniqueKey]?.let { activeDownload -> + val downloadIdentity = DownloadIdentity(uniqueKey, activeDownload.generation) + canceledDownloads.update { it + (downloadIdentity to activeDownload.entry) } + activeDownloads.update { downloads -> + if (downloads[uniqueKey]?.generation == activeDownload.generation) { + downloads - uniqueKey + } else { + downloads + } + } + activeDownload.job.cancel() + deleteDownloadFiles(activeDownload.entry) + } + historyRepository.delete(uniqueKey) + downloadStates.update { it - uniqueKey } + cacheManager.checkCacheStatus() + cacheRefreshEvents.update { it + 1 } + } catch (exception: Exception) { + logcat(LogPriority.ERROR, TAG) { + "delete failed uniqueKey=$uniqueKey\n${exception.stackTraceToString()}" + } + } + } + } + private fun updateDownloadState(uniqueKey: String, downloadState: DownloadState) { downloadStates.update { it + (uniqueKey to downloadState) } } + private fun updateDownloadStateIfActive( + uniqueKey: String, + generation: Long, + downloadState: DownloadState, + ) { + if (activeDownloads.value[uniqueKey]?.generation == generation) { + updateDownloadState(uniqueKey, downloadState) + } else { + logcat(LogPriority.DEBUG, TAG) { + "ignored stale download state uniqueKey=$uniqueKey, generation=$generation, " + + "state=${downloadState.javaClass.simpleName}" + } + } + } + private fun List.toUiModels( states: Map, ): List = map { entry -> val cacheResolution = getCachedFile(entry) val rememberedState = states[entry.uniqueKey] + val isCanceledDownloadFinishing = canceledDownloads.value.keys.any { it.uniqueKey == entry.uniqueKey } val downloadState = when { + isCanceledDownloadFinishing -> DownloadState.InProgress(0f, isIndeterminate = true) rememberedState is DownloadState.InProgress -> rememberedState rememberedState is DownloadState.DownloadFailed -> rememberedState @@ -227,4 +327,50 @@ class HistoryViewModel( val fallbackFile: File, val selectedFile: File, ) + + private data class ActiveDownload( + val job: Job, + val generation: Long, + val entry: TreeherderInstallHistoryEntry, + ) + + private data class DownloadIdentity( + val uniqueKey: String, + val generation: Long, + ) + + private fun deleteDownloadFiles(entry: TreeherderInstallHistoryEntry) { + val cacheResolution = getCachedFile(entry) + setOf( + cacheResolution.relativePathFile, + cacheResolution.fallbackFile, + cacheResolution.selectedFile, + ).forEach(::deleteDownloadFilesForOutput) + } + + private fun deleteDownloadFilesForOutput(outputFile: File) { + val relatedFiles = buildList { + add(outputFile) + add(File(outputFile.parentFile, "${outputFile.name}.part")) + add(File(outputFile.parentFile, "${outputFile.name}.bak")) + outputFile.parentFile?.listFiles { file -> + file.isFile && file.isManagedNumberedBackupFile(outputFile) + }?.let(::addAll) + } + relatedFiles.forEach { file -> + if (file.exists() && !file.delete()) { + logcat(LogPriority.WARN, TAG) { + "failed to delete history download file path=${file.absolutePath}" + } + } + } + } + + private fun File.isManagedNumberedBackupFile(outputFile: File): Boolean { + val backupPrefix = "${outputFile.name}.bak." + if (!name.startsWith(backupPrefix)) { + return false + } + return name.removePrefix(backupPrefix).toIntOrNull() != null + } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 65b4b43..503e72f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -60,6 +60,7 @@ Treeherder APKs appear here after TryFox opens the installer. Revision: %1$s Last installed: %1$s + Remove history entry Fenix Fenix Release diff --git a/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt b/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt index cba8940..04a15cc 100644 --- a/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt +++ b/app/src/test/java/org/mozilla/tryfox/data/FakeHistoryRepository.kt @@ -26,6 +26,11 @@ class FakeHistoryRepository : HistoryRepository { _historyEntries.value = recordedEntries.sortedByDescending { it.lastInstallerLaunchTimestamp } } + override suspend fun delete(uniqueKey: String) { + recordedEntries.removeAll { it.uniqueKey == uniqueKey } + _historyEntries.value = recordedEntries.sortedByDescending { it.lastInstallerLaunchTimestamp } + } + fun setEntries(entries: List) { recordedEntries.clear() recordedEntries.addAll(entries) diff --git a/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt b/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt index 7fab159..dea50b7 100644 --- a/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt +++ b/app/src/test/java/org/mozilla/tryfox/ui/screens/HistoryViewModelTest.kt @@ -4,6 +4,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -181,6 +182,118 @@ class HistoryViewModelTest { assertTrue(intentManager.wasInstallApkCalled) } + @Test + fun `delete removes history item from repository and rendered state`() = runTest { + val firstEntry = historyEntry() + val secondEntry = historyEntry(downloadUrl = "https://example.com/second.apk") + .copy(taskId = "second-task", artifactFileName = "second.apk", artifactName = "public/build/second.apk") + val historyRepository = FakeHistoryRepository().apply { + setEntries(listOf(firstEntry, secondEntry)) + } + val viewModel = createViewModel( + cacheManager = FakeCacheManager(tempCacheDir), + historyRepository = historyRepository, + ) + advanceUntilIdle() + + viewModel.delete(viewModel.historyItems.value.first { it.entry.uniqueKey == firstEntry.uniqueKey }) + advanceUntilIdle() + + assertEquals(listOf(secondEntry.uniqueKey), historyRepository.recordedEntries.map { it.uniqueKey }) + assertEquals(listOf(secondEntry.uniqueKey), viewModel.historyItems.value.map { it.entry.uniqueKey }) + } + + @Test + fun `delete cancels active download removes files and ignores late download callbacks`() = runTest { + val entry = historyEntry() + val cacheManager = FakeCacheManager(tempCacheDir) + val historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) } + val downloadFileRepository = CancellationIgnoringFailingDownloadFileRepository() + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = historyRepository, + downloadFileRepository = downloadFileRepository, + ) + advanceUntilIdle() + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.InProgress) + val downloadedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + val partialFile = File(downloadedFile.parentFile, "${downloadedFile.name}.part") + val managedBackupFile = File(downloadedFile.parentFile, "${downloadedFile.name}.bak.1") + val unmanagedBackupLikeFile = File(downloadedFile.parentFile, "${downloadedFile.name}.bak.tmp") + managedBackupFile.writeText("backup") + unmanagedBackupLikeFile.writeText("not managed by downloader") + + viewModel.delete(viewModel.historyItems.value.single()) + advanceUntilIdle() + + assertTrue(downloadFileRepository.wasCanceled) + assertEquals(emptyList(), historyRepository.recordedEntries) + assertTrue(viewModel.historyItems.value.isEmpty()) + assertFalse(downloadedFile.exists()) + assertFalse(partialFile.exists()) + assertFalse(managedBackupFile.exists()) + assertTrue(unmanagedBackupLikeFile.exists()) + + historyRepository.setEntries(listOf(entry)) + advanceUntilIdle() + + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.NotDownloaded) + } + + @Test + fun `same key download waits until canceled download finishes`() = runTest { + val entry = historyEntry() + val cacheManager = FakeCacheManager(tempCacheDir) + val historyRepository = FakeHistoryRepository().apply { setEntries(listOf(entry)) } + val downloadFileRepository = DelayedCanceledThenBlockingDownloadFileRepository() + val viewModel = createViewModel( + cacheManager = cacheManager, + historyRepository = historyRepository, + downloadFileRepository = downloadFileRepository, + ) + advanceUntilIdle() + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + viewModel.delete(viewModel.historyItems.value.single()) + advanceUntilIdle() + + historyRepository.setEntries(listOf(entry)) + advanceUntilIdle() + assertTrue( + (viewModel.historyItems.value.single().downloadState as DownloadState.InProgress) + .isIndeterminate, + ) + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + + val downloadedFile = File(cacheManager.getCacheDir("treeherder"), "${entry.taskId}/${entry.artifactFileName}") + val partialFile = File(downloadedFile.parentFile, "${downloadedFile.name}.part") + assertEquals(1, downloadFileRepository.startedDownloads) + assertFalse(partialFile.exists()) + + downloadFileRepository.completeCanceledDownload() + advanceUntilIdle() + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.NotDownloaded) + + viewModel.download(viewModel.historyItems.value.single()) + advanceUntilIdle() + + assertTrue(partialFile.exists()) + assertEquals(2, downloadFileRepository.startedDownloads) + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.InProgress) + + downloadFileRepository.completeSecondDownload() + advanceUntilIdle() + + assertTrue(downloadedFile.exists()) + assertTrue(viewModel.historyItems.value.single().downloadState is DownloadState.Downloaded) + } + private fun createViewModel( cacheManager: FakeCacheManager, historyRepository: FakeHistoryRepository, @@ -243,4 +356,78 @@ class HistoryViewModelTest { completion.complete(Unit) } } + + private class CancellationIgnoringFailingDownloadFileRepository : org.mozilla.tryfox.data.repositories.DownloadFileRepository { + var wasCanceled = false + private set + + override suspend fun downloadFile( + downloadUrl: String, + outputFile: File, + onProgress: (bytesDownloaded: Long, totalBytes: Long) -> Unit, + ): org.mozilla.tryfox.data.NetworkResult { + outputFile.parentFile?.mkdirs() + outputFile.writeText("partial") + File(outputFile.parentFile, "${outputFile.name}.part").writeText("partial") + onProgress(1L, 10L) + + try { + kotlinx.coroutines.awaitCancellation() + } catch (_: kotlinx.coroutines.CancellationException) { + wasCanceled = true + } + + outputFile.writeText("late complete") + File(outputFile.parentFile, "${outputFile.name}.part").writeText("late partial") + onProgress(10L, 10L) + return org.mozilla.tryfox.data.NetworkResult.Error("late failure after cancellation", null) + } + } + + private class DelayedCanceledThenBlockingDownloadFileRepository : org.mozilla.tryfox.data.repositories.DownloadFileRepository { + private val canceledDownloadCanComplete = kotlinx.coroutines.CompletableDeferred() + private val secondDownloadCanComplete = kotlinx.coroutines.CompletableDeferred() + + var startedDownloads = 0 + private set + + override suspend fun downloadFile( + downloadUrl: String, + outputFile: File, + onProgress: (bytesDownloaded: Long, totalBytes: Long) -> Unit, + ): org.mozilla.tryfox.data.NetworkResult { + startedDownloads += 1 + outputFile.parentFile?.mkdirs() + val partialFile = File(outputFile.parentFile, "${outputFile.name}.part") + + return if (startedDownloads == 1) { + partialFile.writeText("first partial") + onProgress(1L, 10L) + try { + kotlinx.coroutines.awaitCancellation() + } catch (_: kotlinx.coroutines.CancellationException) { + kotlinx.coroutines.withContext(kotlinx.coroutines.NonCancellable) { + canceledDownloadCanComplete.await() + } + } + org.mozilla.tryfox.data.NetworkResult.Error("first download canceled", null) + } else { + partialFile.writeText("second partial") + onProgress(1L, 10L) + secondDownloadCanComplete.await() + partialFile.delete() + outputFile.writeText("second complete") + onProgress(10L, 10L) + org.mozilla.tryfox.data.NetworkResult.Success(outputFile) + } + } + + fun completeCanceledDownload() { + canceledDownloadCanComplete.complete(Unit) + } + + fun completeSecondDownload() { + secondDownloadCanComplete.complete(Unit) + } + } }