From 46c6545cd54567a8beaf25faf70bfe2c8ea447b6 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 11 Jan 2024 21:23:04 -0600 Subject: [PATCH] Add the Delete Send API (#584) --- .../vault/datasource/disk/VaultDiskSource.kt | 6 ++ .../datasource/disk/VaultDiskSourceImpl.kt | 5 ++ .../vault/datasource/disk/dao/SendsDao.kt | 7 ++ .../data/vault/repository/VaultRepository.kt | 6 ++ .../vault/repository/VaultRepositoryImpl.kt | 12 ++++ .../repository/model/DeleteSendResult.kt | 17 +++++ .../feature/send/addsend/AddSendViewModel.kt | 72 ++++++++++++++----- .../datasource/disk/VaultDiskSourceTest.kt | 12 ++++ .../vault/datasource/disk/dao/FakeSendsDao.kt | 9 +++ .../vault/repository/VaultRepositoryTest.kt | 30 +++++++- .../send/addsend/AddSendViewModelTest.kt | 71 ++++++++++++++---- 11 files changed, 217 insertions(+), 30 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/DeleteSendResult.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt index 04adb32d18..44504add93 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt @@ -6,6 +6,7 @@ import kotlinx.coroutines.flow.Flow /** * Primary access point for disk information related to vault data. */ +@Suppress("TooManyFunctions") interface VaultDiskSource { /** @@ -43,6 +44,11 @@ interface VaultDiskSource { */ suspend fun saveSend(userId: String, send: SyncResponseJson.Send) + /** + * Deletes a send from the data source for the given [userId] and [sendId]. + */ + suspend fun deleteSend(userId: String, sendId: String) + /** * Retrieves all sends from the data source for a given [userId]. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt index a32c8cf767..cba8efcfac 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt @@ -22,6 +22,7 @@ import kotlinx.serialization.json.Json /** * Default implementation of [VaultDiskSource]. */ +@Suppress("TooManyFunctions") class VaultDiskSourceImpl( private val ciphersDao: CiphersDao, private val collectionsDao: CollectionsDao, @@ -140,6 +141,10 @@ class VaultDiskSourceImpl( ) } + override suspend fun deleteSend(userId: String, sendId: String) { + sendsDao.deleteSend(userId, sendId) + } + override fun getSends( userId: String, ): Flow> = diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/SendsDao.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/SendsDao.kt index 5a85f67ad6..ef40e86810 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/SendsDao.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/SendsDao.kt @@ -29,6 +29,13 @@ interface SendsDao { userId: String, ): Flow> + /** + * Deletes the specified send associated with the given [userId] and [sendId]. This will return + * the number of rows deleted by this query. + */ + @Query("DELETE FROM sends WHERE user_id = :userId AND id = :sendId") + suspend fun deleteSend(userId: String, sendId: String): Int + /** * Deletes all the stored sends associated with the given [userId]. This will return the * number of rows deleted by this query. diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt index 172c5f3513..5c7b720d36 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt @@ -9,6 +9,7 @@ import com.x8bit.bitwarden.data.platform.repository.model.DataState import com.x8bit.bitwarden.data.vault.manager.VaultLockManager import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult +import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.SendData import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult @@ -157,4 +158,9 @@ interface VaultRepository : VaultLockManager { * Attempt to remove the password from a send. */ suspend fun removePasswordSend(sendId: String): RemovePasswordSendResult + + /** + * Attempt to delete a send. + */ + suspend fun deleteSend(sendId: String): DeleteSendResult } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 8dc26c89ba..f03b0bfcfd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -30,6 +30,7 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource import com.x8bit.bitwarden.data.vault.manager.VaultLockManager import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult +import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.SendData import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult @@ -472,6 +473,17 @@ class VaultRepositoryImpl( ) } + override suspend fun deleteSend(sendId: String): DeleteSendResult { + val userId = requireNotNull(activeUserId) + return sendsService + .deleteSend(sendId) + .onSuccess { vaultDiskSource.deleteSend(userId, sendId) } + .fold( + onSuccess = { DeleteSendResult.Success }, + onFailure = { DeleteSendResult.Error }, + ) + } + private fun storeProfileData( syncResponse: SyncResponseJson, ) { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/DeleteSendResult.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/DeleteSendResult.kt new file mode 100644 index 0000000000..fe297923ec --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/DeleteSendResult.kt @@ -0,0 +1,17 @@ +package com.x8bit.bitwarden.data.vault.repository.model + +/** + * Models result of deleting a send. + */ +sealed class DeleteSendResult { + + /** + * Send delete successfully. + */ + data object Success : DeleteSendResult() + + /** + * Generic error while deleting a send. + */ + data object Error : DeleteSendResult() +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt index 209fd63c21..f6025bc04e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt @@ -13,6 +13,7 @@ import com.x8bit.bitwarden.data.platform.repository.util.baseWebSendUrl import com.x8bit.bitwarden.data.platform.repository.util.takeUntilLoaded import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult +import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModel @@ -141,6 +142,7 @@ class AddSendViewModel @Inject constructor( private fun handleInternalAction(action: AddSendAction.Internal): Unit = when (action) { is AddSendAction.Internal.CreateSendResultReceive -> handleCreateSendResultReceive(action) is AddSendAction.Internal.UpdateSendResultReceive -> handleUpdateSendResultReceive(action) + is AddSendAction.Internal.DeleteSendResultReceive -> handleDeleteSendResultReceive(action) is AddSendAction.Internal.RemovePasswordResultReceive -> handleRemovePasswordResultReceive( action, ) @@ -206,6 +208,29 @@ class AddSendViewModel @Inject constructor( } } + private fun handleDeleteSendResultReceive( + action: AddSendAction.Internal.DeleteSendResultReceive, + ) { + when (action.result) { + is DeleteSendResult.Error -> { + mutableStateFlow.update { + it.copy( + dialogState = AddSendState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), + ), + ) + } + } + + is DeleteSendResult.Success -> { + mutableStateFlow.update { it.copy(dialogState = null) } + sendEvent(AddSendEvent.NavigateBack) + sendEvent(AddSendEvent.ShowToast(message = R.string.send_deleted.asText())) + } + } + } + private fun handleRemovePasswordResultReceive( action: AddSendAction.Internal.RemovePasswordResultReceive, ) { @@ -315,25 +340,29 @@ class AddSendViewModel @Inject constructor( } private fun handleDeleteClick() { - // TODO Add deletion support (BIT-1435) - sendEvent(AddSendEvent.ShowToast("Not yet implemented".asText())) + onEdit { + mutableStateFlow.update { + it.copy(dialogState = AddSendState.DialogState.Loading(R.string.deleting.asText())) + } + viewModelScope.launch { + val result = vaultRepo.deleteSend(it.sendItemId) + sendAction(AddSendAction.Internal.DeleteSendResultReceive(result)) + } + } } private fun handleRemovePasswordClick() { - when (val addSendType = state.addSendType) { - AddSendType.AddItem -> Unit - is AddSendType.EditItem -> { - mutableStateFlow.update { - it.copy( - dialogState = AddSendState.DialogState.Loading( - message = R.string.removing_send_password.asText(), - ), - ) - } - viewModelScope.launch { - val result = vaultRepo.removePasswordSend(addSendType.sendItemId) - sendAction(AddSendAction.Internal.RemovePasswordResultReceive(result)) - } + onEdit { + mutableStateFlow.update { + it.copy( + dialogState = AddSendState.DialogState.Loading( + message = R.string.removing_send_password.asText(), + ), + ) + } + viewModelScope.launch { + val result = vaultRepo.removePasswordSend(it.sendItemId) + sendAction(AddSendAction.Internal.RemovePasswordResultReceive(result)) } } } @@ -493,6 +522,12 @@ class AddSendViewModel @Inject constructor( (state.viewState as? AddSendState.ViewState.Content)?.let(block) } + private inline fun onEdit( + crossinline block: (AddSendType.EditItem) -> Unit, + ) { + (state.addSendType as? AddSendType.EditItem)?.let(block) + } + private inline fun updateContent( crossinline block: ( AddSendState.ViewState.Content, @@ -825,6 +860,11 @@ sealed class AddSendAction { */ data class RemovePasswordResultReceive(val result: RemovePasswordSendResult) : Internal() + /** + * Indicates a result for deleting the send has been received. + */ + data class DeleteSendResultReceive(val result: DeleteSendResult) : Internal() + /** * Indicates that the send item data has been received. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceTest.kt index 3e6ffd9fb0..bc9021bd6a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceTest.kt @@ -149,6 +149,18 @@ class VaultDiskSourceTest { assertJsonEquals(SEND_ENTITY.sendJson, storedSendEntity.sendJson) } + @Test + fun `DeleteSend should call deleteSend`() = runTest { + assertFalse(sendsDao.deleteSendCalled) + sendsDao.storedSends.add(SEND_ENTITY) + assertEquals(1, sendsDao.storedSends.size) + + vaultDiskSource.deleteSend(USER_ID, SEND_1.id) + + assertTrue(sendsDao.deleteSendCalled) + assertEquals(emptyList(), sendsDao.storedSends) + } + @Test fun `getSends should emit all SendsDao updates`() = runTest { val sendEntities = listOf(SEND_ENTITY) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FakeSendsDao.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FakeSendsDao.kt index 43451f1b54..9d4f9f14e5 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FakeSendsDao.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FakeSendsDao.kt @@ -10,6 +10,7 @@ class FakeSendsDao : SendsDao { val storedSends = mutableListOf() var deleteSendsCalled: Boolean = false + var deleteSendCalled: Boolean = false var insertSendsCalled: Boolean = false private val sendsFlow = bufferedMutableSharedFlow>(replay = 1) @@ -26,6 +27,14 @@ class FakeSendsDao : SendsDao { return count } + override suspend fun deleteSend(userId: String, sendId: String): Int { + deleteSendCalled = true + val count = storedSends.count { it.userId == userId && it.id == sendId } + storedSends.removeAll { it.userId == userId && it.id == sendId } + sendsFlow.tryEmit(storedSends.toList()) + return count + } + override fun getAllSends(userId: String): Flow> = sendsFlow.map { ciphers -> ciphers.filter { it.userId == userId } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index 3ef2fc41f6..714c6f7ef2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -49,6 +49,7 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView import com.x8bit.bitwarden.data.vault.manager.VaultLockManager import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult +import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.SendData import com.x8bit.bitwarden.data.vault.repository.model.UpdateCipherResult @@ -1598,7 +1599,6 @@ class VaultRepositoryTest { runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE val sendId = "sendId1234" - val mockSendView = createMockSendView(number = 1) coEvery { sendsService.removeSendPassword(sendId = sendId) } returns Throwable("Fail").asFailure() @@ -1651,6 +1651,34 @@ class VaultRepositoryTest { assertEquals(RemovePasswordSendResult.Success(mockSendView), result) } + @Test + fun `deleteSend with sendsService deleteSend failure should return DeleteSendResult Error`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + val sendId = "mockId-1" + coEvery { + sendsService.deleteSend(sendId = sendId) + } returns Throwable("Fail").asFailure() + + val result = vaultRepository.deleteSend(sendId) + + assertEquals(DeleteSendResult.Error, result) + } + + @Test + fun `deleteSend with sendsService deleteSend success should return DeleteSendResult success`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + val userId = "mockId-1" + val sendId = "mockId-1" + coEvery { sendsService.deleteSend(sendId = sendId) } returns Unit.asSuccess() + coEvery { vaultDiskSource.deleteSend(userId, sendId) } just runs + + val result = vaultRepository.deleteSend(sendId) + + assertEquals(DeleteSendResult.Success, result) + } + //region Helper functions /** diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt index db69e90e47..f057de9c63 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt @@ -12,6 +12,7 @@ import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult +import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest @@ -37,6 +38,7 @@ import java.time.Instant import java.time.ZoneOffset import java.time.ZonedDateTime +@Suppress("LargeClass") class AddSendViewModelTest : BaseViewModelTest() { private val clock = Clock.fixed( @@ -279,19 +281,6 @@ class AddSendViewModelTest : BaseViewModelTest() { } } - @Test - fun `DeleteClick should send ShowToast`() = runTest { - val viewModel = createViewModel( - state = DEFAULT_STATE.copy(addSendType = AddSendType.EditItem("sendId")), - addSendType = AddSendType.EditItem("sendId"), - ) - - viewModel.eventFlow.test { - viewModel.trySendAction(AddSendAction.DeleteClick) - assertEquals(AddSendEvent.ShowToast("Not yet implemented".asText()), awaitItem()) - } - } - @Test fun `in add item state, RemovePasswordClick should do nothing`() = runTest { val viewModel = createViewModel() @@ -414,6 +403,62 @@ class AddSendViewModelTest : BaseViewModelTest() { } } + @Test + fun `DeleteClick vaultRepository deleteSend Error should show error dialog`() = runTest { + val sendId = "mockId-1" + coEvery { vaultRepository.deleteSend(sendId) } returns DeleteSendResult.Error + val initialState = DEFAULT_STATE.copy( + addSendType = AddSendType.EditItem(sendItemId = sendId), + ) + val mockSendView = createMockSendView(number = 1) + every { + mockSendView.toViewState(clock, DEFAULT_ENVIRONMENT_URL) + } returns DEFAULT_VIEW_STATE + mutableSendDataStateFlow.value = DataState.Loaded(mockSendView) + val viewModel = createViewModel( + state = initialState, + addSendType = AddSendType.EditItem(sendItemId = sendId), + ) + + viewModel.stateFlow.test { + assertEquals(initialState, awaitItem()) + viewModel.trySendAction(AddSendAction.DeleteClick) + assertEquals( + initialState.copy( + dialogState = AddSendState.DialogState.Loading( + message = R.string.deleting.asText(), + ), + ), + awaitItem(), + ) + assertEquals( + initialState.copy( + dialogState = AddSendState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), + ), + ), + awaitItem(), + ) + } + } + + @Test + fun `DeleteClick vaultRepository deleteSend Success should show toast`() = runTest { + val sendId = "mockId-1" + coEvery { vaultRepository.deleteSend(sendId) } returns DeleteSendResult.Success + val viewModel = createViewModel( + state = DEFAULT_STATE.copy(addSendType = AddSendType.EditItem(sendItemId = sendId)), + addSendType = AddSendType.EditItem(sendItemId = sendId), + ) + + viewModel.eventFlow.test { + viewModel.trySendAction(AddSendAction.DeleteClick) + assertEquals(AddSendEvent.NavigateBack, awaitItem()) + assertEquals(AddSendEvent.ShowToast(R.string.send_deleted.asText()), awaitItem()) + } + } + @Test fun `ShareLinkClick should send ShowToast`() = runTest { val viewModel = createViewModel(