Simplify the restoreCipher API (#1441)

2024-11-25 02:46:00 +03:00 · 2024-06-10 17:13:28 -05:00 · 2024-06-10 17:13:28 -05:00 · 746fb42188
commit 746fb42188
parent 8b5210e27a
9 changed files with 41 additions and 122 deletions
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/api/CiphersApi.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/api/CiphersApi.kt
@ -112,7 +112,7 @@ interface CiphersApi {
    @PUT("ciphers/{cipherId}/restore")
    suspend fun restoreCipher(
        @Path("cipherId") cipherId: String,
-    ): Result<Unit>
+    ): Result<SyncResponseJson.Cipher>

    /**
     * Gets a cipher.
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersService.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersService.kt
@ -88,7 +88,7 @@ interface CiphersService {
    /**
     * Attempt to restore a cipher.
     */
-    suspend fun restoreCipher(cipherId: String): Result<Unit>
+    suspend fun restoreCipher(cipherId: String): Result<SyncResponseJson.Cipher>

    /**
     * Attempt to retrieve a cipher.
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceImpl.kt
@ -144,7 +144,7 @@ class CiphersServiceImpl(
            attachmentId = attachmentId,
        )

-    override suspend fun restoreCipher(cipherId: String): Result<Unit> =
+    override suspend fun restoreCipher(cipherId: String): Result<SyncResponseJson.Cipher> =
        ciphersApi.restoreCipher(cipherId = cipherId)

    override suspend fun getCipher(
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManager.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManager.kt
@ -80,7 +80,6 @@ interface CipherManager {
     */
    suspend fun restoreCipher(
        cipherId: String,
-        cipherView: CipherView,
    ): RestoreCipherResult

    /**
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
@ -172,23 +172,11 @@ class CipherManagerImpl(

    override suspend fun restoreCipher(
        cipherId: String,
-        cipherView: CipherView,
    ): RestoreCipherResult {
        val userId = activeUserId ?: return RestoreCipherResult.Error
        return ciphersService
            .restoreCipher(cipherId = cipherId)
-            .flatMap {
-                vaultSdkSource.encryptCipher(
-                    userId = userId,
-                    cipherView = cipherView.copy(deletedDate = null),
-                )
-            }
-            .onSuccess { cipher ->
-                vaultDiskSource.saveCipher(
-                    userId = userId,
-                    cipher = cipher.toEncryptedNetworkCipherResponse(),
-                )
-            }
+            .onSuccess { vaultDiskSource.saveCipher(userId = userId, cipher = it) }
            .fold(
                onSuccess = { RestoreCipherResult.Success },
                onFailure = { RestoreCipherResult.Error },
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt
@ -486,22 +486,14 @@ class VaultItemViewModel @Inject constructor(
                ),
            )
        }
-        onContent { content ->
-            content
-                .common
-                .currentCipher
-                ?.let { cipher ->
-                    viewModelScope.launch {
-                        trySendAction(
-                            VaultItemAction.Internal.RestoreCipherReceive(
-                                result = vaultRepository.restoreCipher(
-                                    cipherId = state.vaultItemId,
-                                    cipherView = cipher,
-                                ),
-                            ),
-                        )
-                    }
-                }
+        viewModelScope.launch {
+            trySendAction(
+                VaultItemAction.Internal.RestoreCipherReceive(
+                    result = vaultRepository.restoreCipher(
+                        cipherId = state.vaultItemId,
+                    ),
+                ),
+            )
        }
    }

--- a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceTest.kt
@ -58,7 +58,7 @@ class CiphersServiceTest : BaseServiceTest() {

    @Test
    fun `createCipher should return the correct response`() = runTest {
-        server.enqueue(MockResponse().setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON))
+        server.enqueue(MockResponse().setBody(CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON))
        val result = ciphersService.createCipher(
            body = createMockCipherJsonRequest(number = 1),
        )
@ -70,7 +70,7 @@ class CiphersServiceTest : BaseServiceTest() {

    @Test
    fun `createCipherInOrganization should return the correct response`() = runTest {
-        server.enqueue(MockResponse().setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON))
+        server.enqueue(MockResponse().setBody(CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON))
        val result = ciphersService.createCipherInOrganization(
            body = CreateCipherInOrganizationJsonRequest(
                cipher = createMockCipherJsonRequest(number = 1),
@ -136,7 +136,7 @@ class CiphersServiceTest : BaseServiceTest() {
    @Test
    fun `updateCipher with success response should return a Success with the correct cipher`() =
        runTest {
-            server.enqueue(MockResponse().setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON))
+            server.enqueue(MockResponse().setBody(CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON))
            val result = ciphersService.updateCipher(
                cipherId = "cipher-id-1",
                body = createMockCipherJsonRequest(number = 1),
@ -199,7 +199,7 @@ class CiphersServiceTest : BaseServiceTest() {
        server.enqueue(
            MockResponse()
                .setResponseCode(200)
-                .setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON),
+                .setBody(CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON),
        )

        val result = ciphersService.shareCipher(
@ -233,15 +233,15 @@ class CiphersServiceTest : BaseServiceTest() {

    @Test
    fun `restoreCipher should execute the restoreCipher API`() = runTest {
-        server.enqueue(MockResponse().setResponseCode(200))
+        server.enqueue(MockResponse().setBody(CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON))
        val cipherId = "cipherId"
        val result = ciphersService.restoreCipher(cipherId = cipherId)
-        assertEquals(Unit, result.getOrThrow())
+        assertEquals(createMockCipher(number = 1), result.getOrThrow())
    }

    @Test
    fun `getCipher should return the correct response`() = runTest {
-        server.enqueue(MockResponse().setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON))
+        server.enqueue(MockResponse().setBody(CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON))
        val result = ciphersService.getCipher(cipherId = "mockId-1")
        assertEquals(
            createMockCipher(number = 1),
@ -398,7 +398,7 @@ private const val CREATE_ATTACHMENT_SUCCESS_JSON = """
 }
 """

-private const val CREATE_UPDATE_CIPHER_SUCCESS_JSON = """
+private const val CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON = """
 {
  "notes": "mockNotes-1",
  "attachments": [
--- a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
@ -603,10 +603,7 @@ class CipherManagerTest {
    fun `restoreCipher with no active user should return RestoreCipherResult Error`() = runTest {
        fakeAuthDiskSource.userState = null

-        val result = cipherManager.restoreCipher(
-            cipherId = "cipherId",
-            cipherView = mockk(),
-        )
+        val result = cipherManager.restoreCipher(cipherId = "cipherId")

        assertEquals(RestoreCipherResult.Error, result)
    }
@ -621,10 +618,7 @@ class CipherManagerTest {
                ciphersService.restoreCipher(cipherId = cipherId)
            } returns Throwable("Fail").asFailure()

-            val result = cipherManager.restoreCipher(
-                cipherId = cipherId,
-                cipherView = createMockCipherView(number = 1),
-            )
+            val result = cipherManager.restoreCipher(cipherId = cipherId)

            assertEquals(RestoreCipherResult.Error, result)
        }
@ -633,35 +627,14 @@ class CipherManagerTest {
    @Test
    fun `restoreCipher with ciphersService restoreCipher success should return RestoreCipherResult success`() =
        runTest {
-            mockkStatic(Cipher::toEncryptedNetworkCipherResponse)
-            every {
-                createMockSdkCipher(number = 1, clock = clock).toEncryptedNetworkCipherResponse()
-            } returns createMockCipher(number = 1)
-            val fixedInstant = Instant.parse("2021-01-01T00:00:00Z")
            val userId = "mockId-1"
            val cipherId = "mockId-1"
-            coEvery {
-                vaultSdkSource.encryptCipher(
-                    userId = userId,
-                    cipherView = createMockCipherView(number = 1).copy(deletedDate = null),
-                )
-            } returns createMockSdkCipher(number = 1, clock = clock).asSuccess()
+            val cipher = createMockCipher(number = 1)
            fakeAuthDiskSource.userState = MOCK_USER_STATE
-            coEvery { ciphersService.restoreCipher(cipherId = cipherId) } returns Unit.asSuccess()
-            coEvery {
-                vaultDiskSource.saveCipher(
-                    userId = userId,
-                    cipher = createMockCipher(number = 1),
-                )
-            } just runs
-            val cipherView = createMockCipherView(number = 1)
-            mockkStatic(Instant::class)
-            every { Instant.now() } returns fixedInstant
+            coEvery { ciphersService.restoreCipher(cipherId = cipherId) } returns cipher.asSuccess()
+            coEvery { vaultDiskSource.saveCipher(userId = userId, cipher = cipher) } just runs

-            val result = cipherManager.restoreCipher(
-                cipherId = cipherId,
-                cipherView = cipherView,
-            )
+            val result = cipherManager.restoreCipher(cipherId = cipherId)

            assertEquals(RestoreCipherResult.Success, result)
        }
--- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt
@ -377,25 +377,9 @@ class VaultItemViewModelTest : BaseViewModelTest() {
        @Suppress("MaxLineLength")
        fun `ConfirmRestoreClick with RestoreCipherResult Success should should ShowToast and NavigateBack`() =
            runTest {
-                val mockCipherView = mockk<CipherView> {
-                    every {
-                        toViewState(
-                            isPremiumUser = true,
-                            hasMasterPassword = true,
-                            totpCodeItemData = createTotpCodeData(),
-                        )
-                    } returns DEFAULT_VIEW_STATE
-                }
-                mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
-                mutableAuthCodeItemFlow.value =
-                    DataState.Loaded(data = createVerificationCodeItem())
-
                val viewModel = createViewModel(state = DEFAULT_STATE)
                coEvery {
-                    vaultRepo.restoreCipher(
-                        cipherId = VAULT_ITEM_ID,
-                        cipherView = createMockCipherView(number = 1),
-                    )
+                    vaultRepo.restoreCipher(cipherId = VAULT_ITEM_ID)
                } returns RestoreCipherResult.Success

                viewModel.trySendAction(VaultItemAction.Common.ConfirmRestoreClick)
@ -414,40 +398,23 @@ class VaultItemViewModelTest : BaseViewModelTest() {

        @Test
        @Suppress("MaxLineLength")
-        fun `ConfirmRestoreClick with RestoreCipherResult Failure should should Show generic error`() =
-            runTest {
-                val mockCipherView = mockk<CipherView> {
-                    every {
-                        toViewState(
-                            isPremiumUser = true,
-                            hasMasterPassword = true,
-                            totpCodeItemData = createTotpCodeData(),
-                        )
-                    } returns DEFAULT_VIEW_STATE
-                }
-                mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
-                mutableAuthCodeItemFlow.value =
-                    DataState.Loaded(data = createVerificationCodeItem())
-                val viewModel = createViewModel(state = DEFAULT_STATE)
-                coEvery {
-                    vaultRepo.restoreCipher(
-                        cipherId = VAULT_ITEM_ID,
-                        cipherView = createMockCipherView(number = 1),
-                    )
-                } returns RestoreCipherResult.Error
+        fun `ConfirmRestoreClick with RestoreCipherResult Failure should should Show generic error`() {
+            val viewModel = createViewModel(state = DEFAULT_STATE)
+            coEvery {
+                vaultRepo.restoreCipher(cipherId = VAULT_ITEM_ID)
+            } returns RestoreCipherResult.Error

-                viewModel.trySendAction(VaultItemAction.Common.ConfirmRestoreClick)
+            viewModel.trySendAction(VaultItemAction.Common.ConfirmRestoreClick)

-                assertEquals(
-                    DEFAULT_STATE.copy(
-                        viewState = DEFAULT_VIEW_STATE,
-                        dialog = VaultItemState.DialogState.Generic(
-                            message = R.string.generic_error_message.asText(),
-                        ),
+            assertEquals(
+                DEFAULT_STATE.copy(
+                    dialog = VaultItemState.DialogState.Generic(
+                        message = R.string.generic_error_message.asText(),
                    ),
-                    viewModel.stateFlow.value,
-                )
-            }
+                ),
+                viewModel.stateFlow.value,
+            )
+        }

        @Test
        fun `on EditClick should do nothing when ViewState is not Content`() = runTest {