diff --git a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt index ee497b0106..9966e238a2 100644 --- a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt +++ b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt @@ -1,6 +1,7 @@ package com.itsaky.androidide.repositories import com.itsaky.androidide.templates.manager.models.CgtFileItem +import java.io.IOException /** * Repository interface for template (`.cgt`) file operations. @@ -20,9 +21,22 @@ interface TemplateRepository { /** Moves [item]'s file from Downloads into the templates directory and reloads templates. */ suspend fun installTemplate(item: CgtFileItem): Result - /** Restores a copy of [item]'s file to Downloads, removes it from the templates directory, and reloads templates. */ - suspend fun uninstallTemplate(item: CgtFileItem): Result + /** + * Restores a copy of [item]'s file to Downloads, removes it from the templates directory, and + * reloads templates. Fails with [TemplateReplaceConflictException] if a file of the same name + * already exists in Downloads, unless [overwrite] is true. + */ + suspend fun uninstallTemplate( + item: CgtFileItem, + overwrite: Boolean = false, + ): Result /** Deletes a not-installed [item]'s file from Downloads. */ suspend fun deleteDownloadFile(item: CgtFileItem): Result } + +/** Thrown by [TemplateRepository.uninstallTemplate] when restoring to Downloads would silently + * overwrite an existing file there and the caller didn't opt into replacing it. */ +class TemplateReplaceConflictException( + val fileName: String, +) : IOException("A file named '$fileName' already exists in Downloads") diff --git a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt index 21662008d1..cb2301eae2 100644 --- a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt +++ b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt @@ -137,7 +137,15 @@ class TemplateRepositoryImpl( } } - override suspend fun uninstallTemplate(item: CgtFileItem): Result = + /** + * On a post-copy delete failure: rolls back (deletes the Downloads copy this call created) when + * `overwrite` was false, but leaves the Downloads copy in place when `overwrite` was true, since + * the pre-existing content there was already replaced and can't be recovered either way. + */ + override suspend fun uninstallTemplate( + item: CgtFileItem, + overwrite: Boolean, + ): Result = withContext(Dispatchers.IO) { try { check(item.installed) { "'${item.name}' is not installed" } @@ -146,11 +154,31 @@ class TemplateRepositoryImpl( // Restore a copy to Downloads BEFORE removing it from the store: if the restore // throws, the store copy below is never touched, so the user's only copy survives. val restored = File(downloadDir, item.file.name) - check(!restored.exists()) { "A download named '${restored.name}' already exists in $downloadDir" } - item.file.copyTo(restored, overwrite = false) + val hadExistingDownload = restored.exists() + if (hadExistingDownload && !overwrite) { + // Not thrown, so it skips the catch blocks below - log it here instead, or a + // repeated replace-conflict leaves no trace for support to find. + logger.warn( + "Uninstall of '{}' would overwrite an existing Downloads file; asking for confirmation", + item.name, + ) + return@withContext Result.failure(TemplateReplaceConflictException(restored.name)) + } + item.file.copyTo(restored, overwrite = overwrite) if (!item.file.delete()) { - restored.delete() - throw IOException("Failed to delete source file after copying: ${item.file.absolutePath}") + // Only roll back a copy this call created itself. When `overwrite` replaced a + // pre-existing Downloads file, that original content is already gone - deleting + // `restored` here would destroy the new copy too and leave the user with nothing, + // whereas the still-installed source (its delete just failed) means leaving the + // new copy in place costs nothing and loses no data. + if (!hadExistingDownload) { + restored.delete() + throw IOException("Failed to delete source file after copying: ${item.file.absolutePath}") + } + throw IOException( + "Replaced '${restored.name}' in Downloads, but failed to delete the installed copy at " + + "${item.file.absolutePath} - the template now exists in both places", + ) } ITemplateProvider.getInstance(reload = true) Result.success(Unit) diff --git a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt index dcdb2f54e3..07266a1585 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt @@ -44,6 +44,46 @@ fun DeleteTemplateConfirmationDialog( ) } +/** Confirm before uninstalling [item], since it moves the file back to Downloads. */ +@Composable +fun UninstallTemplateConfirmationDialog( + item: CgtFileItem, + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(stringResource(R.string.title_uninstall_template)) }, + text = { Text(stringResource(R.string.msg_uninstall_template_confirm, item.displayName)) }, + confirmButton = { + TextButton(onClick = onConfirm) { Text(stringResource(R.string.action_uninstall_template)) } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text(stringResource(android.R.string.cancel)) } + }, + ) +} + +/** Shown when uninstalling [item] would silently overwrite an existing Downloads file of the same name. */ +@Composable +fun ReplaceTemplateInDownloadsDialog( + item: CgtFileItem, + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(stringResource(R.string.title_replace_template)) }, + text = { Text(stringResource(R.string.msg_replace_template_confirm, item.displayName)) }, + confirmButton = { + TextButton(onClick = onConfirm) { Text(stringResource(R.string.replace)) } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text(stringResource(android.R.string.cancel)) } + }, + ) +} + /** File-level details for a single-template .cgt (multi-template files use [TemplateListDialog]). */ @Composable fun TemplateFileDetailsDialog( diff --git a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt index 4e6da44a51..cd9bd56903 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt @@ -67,6 +67,16 @@ private sealed interface TemplateManagerDialogState : Parcelable { val path: String, ) : TemplateManagerDialogState + @Parcelize + data class UninstallConfirm( + val path: String, + ) : TemplateManagerDialogState + + @Parcelize + data class ReplaceConfirm( + val path: String, + ) : TemplateManagerDialogState + @Parcelize data class FileDetails( val path: String, @@ -153,6 +163,14 @@ fun TemplateManagerScreen( dialogState = TemplateManagerDialogState.DeleteConfirm(effect.item.file.absolutePath) } + is TemplateManagerUiEffect.ShowUninstallConfirmation -> { + dialogState = TemplateManagerDialogState.UninstallConfirm(effect.item.file.absolutePath) + } + + is TemplateManagerUiEffect.ShowReplaceConfirmation -> { + dialogState = TemplateManagerDialogState.ReplaceConfirm(effect.item.file.absolutePath) + } + is TemplateManagerUiEffect.ShowTemplateDetails -> { dialogState = TemplateManagerDialogState.FileDetails(effect.item.file.absolutePath) } @@ -215,6 +233,34 @@ fun TemplateManagerScreen( } } + is TemplateManagerDialogState.UninstallConfirm -> { + val item = uiState.items.firstOrNull { it.file.absolutePath == dialog.path } + if (item != null) { + UninstallTemplateConfirmationDialog( + item = item, + onConfirm = { + viewModel.confirmUninstallTemplate(item) + dialogState = TemplateManagerDialogState.None + }, + onDismiss = { dialogState = TemplateManagerDialogState.None }, + ) + } + } + + is TemplateManagerDialogState.ReplaceConfirm -> { + val item = uiState.items.firstOrNull { it.file.absolutePath == dialog.path } + if (item != null) { + ReplaceTemplateInDownloadsDialog( + item = item, + onConfirm = { + viewModel.confirmUninstallTemplate(item, overwrite = true) + dialogState = TemplateManagerDialogState.None + }, + onDismiss = { dialogState = TemplateManagerDialogState.None }, + ) + } + } + is TemplateManagerDialogState.FileDetails -> { val item = uiState.items.firstOrNull { it.file.absolutePath == dialog.path } if (item != null) { diff --git a/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt b/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt index b47f16115e..ae236fb5bb 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt @@ -50,6 +50,16 @@ sealed class TemplateManagerUiEffect { val item: CgtFileItem, ) : TemplateManagerUiEffect() + /** Ask before uninstalling [item], mirroring [ShowDeleteConfirmation]'s pattern for a Downloads file. */ + data class ShowUninstallConfirmation( + val item: CgtFileItem, + ) : TemplateManagerUiEffect() + + /** A Downloads file with the same name already exists; ask before [uninstallTemplate][com.itsaky.androidide.repositories.TemplateRepository.uninstallTemplate] overwrites it. */ + data class ShowReplaceConfirmation( + val item: CgtFileItem, + ) : TemplateManagerUiEffect() + data class ShowTemplateDetails( val item: CgtFileItem, ) : TemplateManagerUiEffect() diff --git a/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt index 13df7bca84..bb60d1e76f 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt @@ -3,6 +3,7 @@ package com.itsaky.androidide.viewmodels import android.util.Log import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.itsaky.androidide.repositories.TemplateReplaceConflictException import com.itsaky.androidide.repositories.TemplateRepository import com.itsaky.androidide.resources.R import com.itsaky.androidide.templates.manager.models.CgtFileItem @@ -42,7 +43,7 @@ class TemplateManagerViewModel( when (event) { is TemplateManagerUiEvent.LoadTemplates -> loadTemplates() is TemplateManagerUiEvent.InstallTemplate -> installTemplate(event.item) - is TemplateManagerUiEvent.UninstallTemplate -> uninstallTemplate(event.item) + is TemplateManagerUiEvent.UninstallTemplate -> showUninstallConfirmation(event.item) is TemplateManagerUiEvent.DeleteDownloadFile -> showDeleteConfirmation(event.item) is TemplateManagerUiEvent.ShowTemplateDetails -> showTemplateDetails(event.item) is TemplateManagerUiEvent.ShowTemplateList -> showTemplateList(event.item) @@ -101,11 +102,26 @@ class TemplateManagerViewModel( } } - private fun uninstallTemplate(item: CgtFileItem) { + private fun showUninstallConfirmation(item: CgtFileItem) { + viewModelScope.launch { + _uiEffect.send(TemplateManagerUiEffect.ShowUninstallConfirmation(item)) + } + } + + /** + * Uninstalls [item] (called after [TemplateManagerUiEffect.ShowUninstallConfirmation] is + * confirmed). A [TemplateReplaceConflictException] means a same-named file already sits in + * Downloads; [ShowReplaceConfirmation][TemplateManagerUiEffect.ShowReplaceConfirmation] asks + * whether to replace it, and a confirmed replace re-enters this with [overwrite] set. + */ + fun confirmUninstallTemplate( + item: CgtFileItem, + overwrite: Boolean = false, + ) { viewModelScope.launch { _uiState.update { it.copy(isLoading = true) } templateRepository - .uninstallTemplate(item) + .uninstallTemplate(item, overwrite) .onSuccess { Log.d(TAG, "Template uninstalled successfully: ${item.name}") _uiEffect.send(TemplateManagerUiEffect.ShowSuccess(R.string.msg_template_uninstalled)) @@ -113,12 +129,16 @@ class TemplateManagerViewModel( }.onFailure { exception -> Log.e(TAG, "Failed to uninstall template: ${item.name}", exception) _uiState.update { it.copy(isLoading = false) } - _uiEffect.send( - TemplateManagerUiEffect.ShowError( - R.string.msg_template_uninstall_failed, - listOf(exception.message ?: ""), - ), - ) + if (exception is TemplateReplaceConflictException) { + _uiEffect.send(TemplateManagerUiEffect.ShowReplaceConfirmation(item)) + } else { + _uiEffect.send( + TemplateManagerUiEffect.ShowError( + R.string.msg_template_uninstall_failed, + listOf(exception.message ?: ""), + ), + ) + } } } } diff --git a/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt b/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt index c1c0048630..6d0c810a6c 100644 --- a/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt +++ b/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt @@ -98,7 +98,7 @@ class TemplateRepositoryImplTest { } @Test - fun uninstallTemplate_nameCollision_failsWithoutTouchingEitherCopy() = + fun uninstallTemplate_nameCollision_failsWithReplaceConflict_withoutTouchingEitherCopy() = runTest { val source = File(templatesDir, "dup.cgt").apply { writeText("installed") } val existingDownload = File(downloadDir, "dup.cgt").apply { writeText("already in downloads") } @@ -106,6 +106,7 @@ class TemplateRepositoryImplTest { val result = repository.uninstallTemplate(item(source, installed = true)) assertThat(result.isFailure).isTrue() + assertThat(result.exceptionOrNull()).isInstanceOf(TemplateReplaceConflictException::class.java) assertThat(source.exists()).isTrue() assertThat(source.readText()).isEqualTo("installed") assertThat(existingDownload.readText()).isEqualTo("already in downloads") @@ -127,6 +128,29 @@ class TemplateRepositoryImplTest { assertThat(restored.exists()).isFalse() } + @Test + fun uninstallTemplate_overwriteTrue_deleteFails_leavesTheReplacementCopyInDownloads() = + runTest { + val source = File(templatesDir, "uninstall.cgt").apply { writeText("installed") } + val restored = File(downloadDir, "uninstall.cgt").apply { writeText("stale") } + + check(templatesDir.setWritable(false)) { "test setup: could not make templatesDir read-only" } + + val result = repository.uninstallTemplate(item(source, installed = true), overwrite = true) + + assertThat(result.isFailure).isTrue() + assertThat(result.exceptionOrNull()).isInstanceOf(IOException::class.java) + // The source is still installed (its delete failed) - the pre-existing Downloads + // content was already overwritten and can't be recovered either way, so the new copy + // is left in place rather than deleted for nothing. + assertThat(source.exists()).isTrue() + assertThat(restored.readText()).isEqualTo("installed") + // The message must say the file now exists in both places, not the plain "failed to + // delete" wording the non-overwrite rollback case uses - a user reading that after an + // overwrite would wrongly assume their prior Downloads content is still intact. + assertThat(result.exceptionOrNull()?.message).contains("now exists in both places") + } + @Test fun deleteDownloadFile_succeeds_whenNotInstalled() = runTest { diff --git a/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt b/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt index d84bb62bc5..5c66369dfc 100644 --- a/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt +++ b/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt @@ -2,6 +2,7 @@ package com.itsaky.androidide.viewmodels import android.util.Log import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.repositories.TemplateReplaceConflictException import com.itsaky.androidide.repositories.TemplateRepository import com.itsaky.androidide.templates.manager.models.CgtFileItem import com.itsaky.androidide.templates.manager.models.TemplateMetadata @@ -44,6 +45,15 @@ class TemplateManagerViewModelTest { provenance = TemplateProvenance.USER, ) + private val installedItem = + CgtFileItem( + file = File("/tmp/uninstall.cgt"), + name = "uninstall.cgt", + templates = listOf(TemplateMetadata("T", "d", "1.0")), + installed = true, + provenance = TemplateProvenance.USER, + ) + @Before fun stubAndroidLog() { // TemplateManagerViewModel calls android.util.Log.d/Log.e directly; under plain JVM @@ -113,4 +123,89 @@ class TemplateManagerViewModelTest { coVerify(exactly = 1) { repository.listTemplateFiles() } assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowError).isTrue() } + + @Test + fun uninstallTemplate_event_asksForConfirmation_withoutCallingRepository() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.onEvent(TemplateManagerUiEvent.UninstallTemplate(installedItem)) + advanceUntilIdle() + + coVerify(exactly = 0) { repository.uninstallTemplate(any(), any()) } + val effect = viewModel.uiEffect.first() + assertThat(effect).isInstanceOf(TemplateManagerUiEffect.ShowUninstallConfirmation::class.java) + assertThat((effect as TemplateManagerUiEffect.ShowUninstallConfirmation).item).isEqualTo(installedItem) + } + + @Test + fun confirmUninstallTemplate_onSuccess_reloadsAndSendsShowSuccessEffect() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, false) } returns Result.success(Unit) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem) + advanceUntilIdle() + + coVerify(exactly = 2) { repository.listTemplateFiles() } + assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowSuccess).isTrue() + } + + @Test + fun confirmUninstallTemplate_onReplaceConflict_sendsShowReplaceConfirmation_notShowError() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, false) } returns + Result.failure(TemplateReplaceConflictException(installedItem.name)) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem) + advanceUntilIdle() + + // A conflict isn't a reload-worthy outcome and must not be reported as a generic error. + coVerify(exactly = 1) { repository.listTemplateFiles() } + val effect = viewModel.uiEffect.first() + assertThat(effect).isInstanceOf(TemplateManagerUiEffect.ShowReplaceConfirmation::class.java) + assertThat((effect as TemplateManagerUiEffect.ShowReplaceConfirmation).item).isEqualTo(installedItem) + } + + @Test + fun confirmUninstallTemplate_withOverwrite_passesOverwriteThrough_onReplaceConfirm() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, true) } returns Result.success(Unit) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem, overwrite = true) + advanceUntilIdle() + + coVerify(exactly = 1) { repository.uninstallTemplate(installedItem, true) } + assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowSuccess).isTrue() + } + + @Test + fun confirmUninstallTemplate_onOtherFailure_sendsShowErrorEffect_withoutReloading() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, false) } returns Result.failure(java.io.IOException("boom")) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem) + advanceUntilIdle() + + coVerify(exactly = 1) { repository.listTemplateFiles() } + assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowError).isTrue() + } } diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index bb7730ff20..be266ce729 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1521,6 +1521,10 @@ Templates in %1$s Delete template? This permanently deletes \'%1$s\' from Downloads. + Uninstall template? + This moves \'%1$s\' back to Downloads. + Replace file in Downloads? + A file named \'%1$s\' already exists in Downloads. Replace it? File Status Location