github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-03-16 08:33:59 -05:00
Code Issues 144 Packages Projects Releases 116 Wiki Activity

[PM-19613] Save attachment error message update (#5013)

Browse Source
This commit is contained in:
André Bispo
2025-04-14 21:50:59 +01:00
committed by GitHub
parent 8f311861e3
commit d989c61bc2
13 changed files with 264 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/InvalidJsonResponse.kt
View File

@@ -1,5 +1,8 @@
package com.x8bit.bitwarden.data.vault.datasource.network.model
// TODO: Remove this file once all models have been moved to the network module
// This class has already been moved to the network module
/**
* Represents the json body of an invalid send json request.
*/

2
app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersService.kt
View File

@@ -34,7 +34,7 @@ interface CiphersService {
* Attempt to upload an attachment file.
*/
suspend fun uploadAttachment(
attachmentJsonResponse: AttachmentJsonResponse,
attachment: AttachmentJsonResponse.Success,
encryptedFile: File,
): Result<SyncResponseJson.Cipher>

22
app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceImpl.kt
View File

@@ -58,23 +58,31 @@ class CiphersServiceImpl(
body = body,
)
.toResult()
.recoverCatching { throwable ->
throwable.toBitwardenError()
.parseErrorBodyOrNull<AttachmentJsonResponse.Invalid>(
code = NetworkErrorCode.BAD_REQUEST,
json = json,
)
?: throw throwable
}
override suspend fun uploadAttachment(
attachmentJsonResponse: AttachmentJsonResponse,
attachment: AttachmentJsonResponse.Success,
encryptedFile: File,
): Result<SyncResponseJson.Cipher> {
val cipher = attachmentJsonResponse.cipherResponse
return when (attachmentJsonResponse.fileUploadType) {
val cipher = attachment.cipherResponse
return when (attachment.fileUploadType) {
FileUploadType.DIRECT -> {
ciphersApi.uploadAttachment(
cipherId = requireNotNull(cipher.id),
attachmentId = attachmentJsonResponse.attachmentId,
attachmentId = attachment.attachmentId,
body = this
.createMultipartBodyBuilder(
encryptedFile = encryptedFile,
filename = cipher
.attachments
?.find { it.id == attachmentJsonResponse.attachmentId }
?.find { it.id == attachment.attachmentId }
?.fileName,
)
.build(),
@@ -83,11 +91,11 @@ class CiphersServiceImpl(
FileUploadType.AZURE -> {
azureApi.uploadAzureBlob(
url = attachmentJsonResponse.url,
url = attachment.url,
date = DateTimeFormatter
.RFC_1123_DATE_TIME
.format(ZonedDateTime.ofInstant(clock.instant(), ZoneOffset.UTC)),
version = attachmentJsonResponse.url.toUri().getQueryParameter("sv"),
version = attachment.url.toUri().getQueryParameter("sv"),
body = encryptedFile.asRequestBody(),
)
}

34
app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
View File

@@ -5,6 +5,7 @@ import androidx.core.net.toUri
import com.bitwarden.core.data.util.asFailure
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.core.data.util.flatMap
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.ShareCipherJsonRequest
import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
@@ -327,7 +328,15 @@ class CipherManagerImpl(
fileUri = fileUri,
)
.fold(
onFailure = { CreateAttachmentResult.Error(error = it) },
onFailure = {
CreateAttachmentResult.Error(
error = it,
message = when (it) {
is IllegalStateException -> it.message
else -> null
},
)
},
onSuccess = { CreateAttachmentResult.Success(cipherView = it) },
)
@@ -371,11 +380,22 @@ class CipherManagerImpl(
cipherId = cipherId,
body = attachment.toNetworkAttachmentRequest(),
)
.flatMap { attachmentJsonResponse ->
val encryptedFile = File("${cacheFile.absolutePath}.enc")
}
.flatMap { attachmentResponse ->
when (attachmentResponse) {
is AttachmentJsonResponse.Invalid -> {
return IllegalStateException(
attachmentResponse.message,
).asFailure()
}
is AttachmentJsonResponse.Success -> {
val encryptedFile = File(
"${cacheFile.absolutePath}.enc",
)
ciphersService
.uploadAttachment(
attachmentJsonResponse = attachmentJsonResponse,
attachment = attachmentResponse,
encryptedFile = encryptedFile,
)
.onSuccess {
@@ -385,6 +405,7 @@ class CipherManagerImpl(
fileManager.delete(cacheFile, encryptedFile)
}
}
}
}
}
}
@@ -488,7 +509,10 @@ class CipherManagerImpl(
.flatMap { response ->
when (response) {
is UpdateCipherResponseJson.Invalid -> {
IllegalStateException(response.message).asFailure()
IllegalStateException(
response.message,
)
.asFailure()
}
is UpdateCipherResponseJson.Success -> {

5
app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/CreateAttachmentResult.kt
View File

@@ -17,5 +17,8 @@ sealed class CreateAttachmentResult {
/**
* Generic error while creating an attachment.
*/
data class Error(val error: Throwable) : CreateAttachmentResult()
data class Error(
val error: Throwable,
val message: String? = null,
) : CreateAttachmentResult()
}

3
app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/attachments/AttachmentsViewModel.kt
View File

@@ -271,7 +271,8 @@ class AttachmentsViewModel @Inject constructor(
it.copy(
dialogState = AttachmentsState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
message = result.message?.asText()
?: R.string.generic_error_message.asText(),
throwable = result.error,
),
)

46
app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceTest.kt
View File

@@ -4,6 +4,7 @@ import android.net.Uri
import com.bitwarden.network.api.AzureApi
import com.bitwarden.network.api.CiphersApi
import com.bitwarden.network.base.BaseServiceTest
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.FileUploadType
import com.bitwarden.network.model.ImportCiphersJsonRequest
@@ -12,6 +13,7 @@ import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
import com.bitwarden.network.model.createMockAttachment
import com.bitwarden.network.model.createMockAttachmentJsonRequest
import com.bitwarden.network.model.createMockAttachmentJsonResponse
import com.bitwarden.network.model.createMockAttachmentResponse
import com.bitwarden.network.model.createMockCipher
import com.bitwarden.network.model.createMockCipherJsonRequest
import com.x8bit.bitwarden.data.vault.datasource.network.model.ImportCiphersResponseJson
@@ -99,19 +101,37 @@ class CiphersServiceTest : BaseServiceTest() {
)
}
@Test
fun `createAttachment with invalid response should return an Invalid with the correct data`() =
runTest {
server.enqueue(
MockResponse().setResponseCode(400).setBody(CREATE_ATTACHMENT_INVALID_JSON),
)
val result = ciphersService.createAttachment(
cipherId = "mockId-1",
body = createMockAttachmentJsonRequest(number = 1),
)
assertEquals(
AttachmentJsonResponse.Invalid(
message = "You do not have permission.",
validationErrors = null,
),
result.getOrThrow(),
)
}
@Test
fun `uploadAttachment with Azure uploadFile success should return cipher`() = runTest {
setupMockUri(url = "mockUrl-1", queryParams = mapOf("sv" to "2024-04-03"))
val mockCipher = createMockCipher(number = 1)
val attachmentJsonResponse = createMockAttachmentJsonResponse(
number = 1,
fileUploadType = FileUploadType.AZURE,
)
val encryptedFile = File.createTempFile("mockFile", "temp")
server.enqueue(MockResponse().setResponseCode(201))
val result = ciphersService.uploadAttachment(
attachmentJsonResponse = attachmentJsonResponse,
attachment = createMockAttachmentResponse(
number = 1,
fileUploadType = FileUploadType.AZURE,
),
encryptedFile = encryptedFile,
)
@@ -121,15 +141,14 @@ class CiphersServiceTest : BaseServiceTest() {
@Test
fun `uploadAttachment with Direct uploadFile success should return cipher`() = runTest {
val mockCipher = createMockCipher(number = 1)
val attachmentJsonResponse = createMockAttachmentJsonResponse(
number = 1,
fileUploadType = FileUploadType.DIRECT,
)
val encryptedFile = File.createTempFile("mockFile", "temp")
server.enqueue(MockResponse().setResponseCode(201))
val result = ciphersService.uploadAttachment(
attachmentJsonResponse = attachmentJsonResponse,
attachment = createMockAttachmentResponse(
number = 1,
fileUploadType = FileUploadType.DIRECT,
),
encryptedFile = encryptedFile,
)
@@ -484,6 +503,13 @@ private const val CREATE_ATTACHMENT_SUCCESS_JSON = """
}
"""
private const val CREATE_ATTACHMENT_INVALID_JSON = """
{
"message": "You do not have permission.",
"validationErrors": null
}
"""
private const val CREATE_RESTORE_UPDATE_CIPHER_SUCCESS_JSON = """
{
"notes": "mockNotes-1",

21
app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
View File

@@ -11,6 +11,7 @@ import com.bitwarden.network.model.SyncResponseJson
import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
import com.bitwarden.network.model.createMockAttachment
import com.bitwarden.network.model.createMockAttachmentJsonResponse
import com.bitwarden.network.model.createMockAttachmentResponse
import com.bitwarden.network.model.createMockCipher
import com.bitwarden.network.model.createMockCipherJsonRequest
import com.bitwarden.vault.Attachment
@@ -896,7 +897,7 @@ class CipherManagerTest {
} returns mockAttachmentJsonResponse.asSuccess()
coEvery {
ciphersService.uploadAttachment(
attachmentJsonResponse = mockAttachmentJsonResponse,
attachment = createMockAttachmentResponse(number = 1),
encryptedFile = File("${cacheFile.absolutePath}.enc"),
)
} returns mockNetworkCipher.asSuccess()
@@ -1174,7 +1175,13 @@ class CipherManagerTest {
fileUri = mockk(),
)
assertEquals(CreateAttachmentResult.Error(NoActiveUserException()), result)
assertEquals(
CreateAttachmentResult.Error(
error = NoActiveUserException(),
message = "No current active user!",
),
result,
)
}
@Suppress("MaxLineLength")
@@ -1389,7 +1396,7 @@ class CipherManagerTest {
} returns mockAttachmentJsonResponse.asSuccess()
coEvery {
ciphersService.uploadAttachment(
attachmentJsonResponse = mockAttachmentJsonResponse,
attachment = createMockAttachmentResponse(number = 1),
encryptedFile = File("${mockFile.absoluteFile}.enc"),
)
} returns error.asFailure()
@@ -1458,7 +1465,7 @@ class CipherManagerTest {
} returns mockAttachmentJsonResponse.asSuccess()
coEvery {
ciphersService.uploadAttachment(
attachmentJsonResponse = mockAttachmentJsonResponse,
attachment = createMockAttachmentResponse(number = 1),
encryptedFile = File("${mockFile.absoluteFile}.enc"),
)
} returns mockCipherResponse.asSuccess()
@@ -1535,7 +1542,7 @@ class CipherManagerTest {
} returns mockAttachmentJsonResponse.asSuccess()
coEvery {
ciphersService.uploadAttachment(
attachmentJsonResponse = mockAttachmentJsonResponse,
attachment = createMockAttachmentResponse(number = 1),
encryptedFile = File("${mockFile.absolutePath}.enc"),
)
} returns mockCipherResponse.asSuccess()
@@ -1610,7 +1617,7 @@ class CipherManagerTest {
} returns mockAttachmentJsonResponse.asSuccess()
coEvery {
ciphersService.uploadAttachment(
attachmentJsonResponse = mockAttachmentJsonResponse,
attachment = createMockAttachmentResponse(number = 1),
encryptedFile = File("${mockFile.absolutePath}.enc"),
)
} returns mockCipherResponse.asSuccess()
@@ -1683,7 +1690,7 @@ class CipherManagerTest {
} returns mockAttachmentJsonResponse.asSuccess()
coEvery {
ciphersService.uploadAttachment(
attachmentJsonResponse = mockAttachmentJsonResponse,
attachment = createMockAttachmentResponse(number = 1),
encryptedFile = File("${mockFile.absolutePath}.enc"),
)
} returns Throwable("Fail").asFailure()

81
app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/attachments/AttachmentsViewModelTest.kt
View File

@@ -4,6 +4,8 @@ import android.net.Uri
import androidx.lifecycle.SavedStateHandle
import app.cash.turbine.test
import com.bitwarden.core.data.repository.model.DataState
import com.bitwarden.ui.util.asText
import com.bitwarden.ui.util.concat
import com.bitwarden.vault.CipherView
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus
@@ -17,8 +19,6 @@ import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.CreateAttachmentResult
import com.x8bit.bitwarden.data.vault.repository.model.DeleteAttachmentResult
import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
import com.bitwarden.ui.util.asText
import com.bitwarden.ui.util.concat
import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager
import com.x8bit.bitwarden.ui.vault.feature.attachments.util.toViewState
import io.mockk.coEvery
@@ -204,7 +204,82 @@ class AttachmentsViewModelTest : BaseViewModelTest() {
)
mutableVaultItemStateFlow.value = DataState.Loaded(cipherView)
mutableUserStateFlow.value = DEFAULT_USER_STATE
val error = NoActiveUserException()
val error = IllegalStateException("No permissions.")
coEvery {
vaultRepository.createAttachment(
cipherId = state.cipherId,
cipherView = cipherView,
fileSizeBytes = sizeJustRight.toString(),
fileName = fileName,
fileUri = uri,
)
} returns CreateAttachmentResult.Error(
error = error,
message = "No permissions.",
)
val viewModel = createViewModel()
// Need to populate the VM with a file
viewModel.trySendAction(AttachmentsAction.FileChoose(fileData))
viewModel.stateFlow.test {
assertEquals(state, awaitItem())
viewModel.trySendAction(AttachmentsAction.SaveClick)
assertEquals(
state.copy(
dialogState = AttachmentsState.DialogState.Loading(
message = R.string.saving.asText(),
),
),
awaitItem(),
)
assertEquals(
state.copy(
dialogState = AttachmentsState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = error.message!!.asText(),
throwable = error,
),
),
awaitItem(),
)
}
coVerify(exactly = 1) {
vaultRepository.createAttachment(
cipherId = state.cipherId,
cipherView = cipherView,
fileSizeBytes = sizeJustRight.toString(),
fileName = fileName,
fileUri = uri,
)
}
}
@Test
fun `SaveClick should display generic error message dialog when createAttachment fails`() =
runTest {
val cipherView = createMockCipherView(number = 1)
val fileName = "test.png"
val uri = mockk<Uri>()
val sizeJustRight = 104_857_600L
val state = DEFAULT_STATE.copy(
viewState = DEFAULT_CONTENT_WITH_ATTACHMENTS.copy(
newAttachment = AttachmentsState.NewAttachment(
displayName = fileName,
uri = uri,
sizeBytes = sizeJustRight,
),
),
isPremiumUser = true,
)
val fileData = IntentManager.FileData(
fileName = fileName,
uri = uri,
sizeBytes = sizeJustRight,
)
mutableVaultItemStateFlow.value = DataState.Loaded(cipherView)
mutableUserStateFlow.value = DEFAULT_USER_STATE
val error = Exception()
coEvery {
vaultRepository.createAttachment(
cipherId = state.cipherId,

2
network/src/main/kotlin/com/bitwarden/network/api/CiphersApi.kt
View File

@@ -45,7 +45,7 @@ interface CiphersApi {
suspend fun createAttachment(
@Path("cipherId") cipherId: String,
@Body body: AttachmentJsonRequest,
): NetworkResult<AttachmentJsonResponse>
): NetworkResult<AttachmentJsonResponse.Success>
/**
* Uploads the attachment associated with a cipher.

49
network/src/main/kotlin/com/bitwarden/network/model/AttachmentJsonResponse.kt
View File

@@ -6,17 +6,44 @@ import kotlinx.serialization.Serializable
/**
* Represents the JSON response from creating a new attachment.
*/
@Serializable
data class AttachmentJsonResponse(
@SerialName("attachmentId")
val attachmentId: String,
sealed class AttachmentJsonResponse {
/**
* Represents a successful response from create attachment request.
*
* @property attachmentId The ID of the attachment.
* @property url The URL of the attachment.
* @property fileUploadType The type of file upload.
* @property cipherResponse The cipher response associated with the attachment.
*/
@Serializable
data class Success(
@SerialName("attachmentId")
val attachmentId: String,
@SerialName("url")
val url: String,
@SerialName("url")
val url: String,
@SerialName("fileUploadType")
val fileUploadType: FileUploadType,
@SerialName("fileUploadType")
val fileUploadType: FileUploadType,
@SerialName("cipherResponse")
val cipherResponse: SyncResponseJson.Cipher,
)
@SerialName("cipherResponse")
val cipherResponse: SyncResponseJson.Cipher,
) : AttachmentJsonResponse()
/**
* Represents the json body of an invalid create request.
*
* @property message A general, user-displayable error message.
* @property validationErrors a map where each value is a list of error messages for each
* key. The values in the array should be used for display to the user, since the keys tend
* to come back as nonsense. (eg: empty string key)
*/
@Serializable
data class Invalid(
@SerialName("message")
override val message: String,
@SerialName("validationErrors")
override val validationErrors: Map<String, List<String>>?,
) : AttachmentJsonResponse(), InvalidJsonResponse
}

28
network/src/main/kotlin/com/bitwarden/network/model/InvalidJsonResponse.kt Normal file
View File

@@ -0,0 +1,28 @@
package com.bitwarden.network.model
/**
* Represents the json body of an invalid send json request.
*/
sealed interface InvalidJsonResponse {
/**
* A general, user-displayable error message.
*/
val message: String
/**
* a map where each value is a list of error messages for each key.
* The values in the array should be used for display to the user, since the keys tend to come
* back as nonsense. (eg: empty string key)
*/
val validationErrors: Map<String, List<String>>?
/**
* Returns the first error message found in [validationErrors], or [message] if there are no
* [validationErrors] present.
*/
val firstValidationErrorMessage: String?
get() = validationErrors
?.flatMap { it.value }
?.first()
}

16
network/src/testFixtures/kotlin/com/bitwarden/network/model/AttachmentJsonResponseUtil.kt
View File

@@ -7,7 +7,21 @@ fun createMockAttachmentJsonResponse(
number: Int,
fileUploadType: FileUploadType = FileUploadType.AZURE,
): AttachmentJsonResponse =
AttachmentJsonResponse(
AttachmentJsonResponse.Success(
attachmentId = "mockAttachmentId-$number",
url = "mockUrl-$number",
fileUploadType = fileUploadType,
cipherResponse = createMockCipher(number = number),
)
/**
* Create a mock [AttachmentJsonResponse.Success] with a given [number].
*/
fun createMockAttachmentResponse(
number: Int,
fileUploadType: FileUploadType = FileUploadType.AZURE,
): AttachmentJsonResponse.Success =
AttachmentJsonResponse.Success(
attachmentId = "mockAttachmentId-$number",
url = "mockUrl-$number",
fileUploadType = fileUploadType,