diff --git a/.idea/vcs.xml b/.idea/vcs.xml index 08fa18257..3e19a6f0f 100644 --- a/.idea/vcs.xml +++ b/.idea/vcs.xml @@ -1,23 +1,10 @@ - + + + + + diff --git a/app/src/main/java/ch/protonmail/android/api/models/room/messages/Attachment.kt b/app/src/main/java/ch/protonmail/android/api/models/room/messages/Attachment.kt index a8a06c03c..745bc3656 100644 --- a/app/src/main/java/ch/protonmail/android/api/models/room/messages/Attachment.kt +++ b/app/src/main/java/ch/protonmail/android/api/models/room/messages/Attachment.kt @@ -29,6 +29,7 @@ import androidx.room.Index import androidx.room.PrimaryKey import ch.protonmail.android.api.models.AttachmentHeaders import ch.protonmail.android.utils.AppUtil +import ch.protonmail.android.utils.MessageUtils.isLocalAttachmentId import com.google.gson.annotations.Expose import com.google.gson.annotations.SerializedName import java.io.ByteArrayOutputStream @@ -79,7 +80,7 @@ data class Attachment @JvmOverloads constructor( @ColumnInfo(name = COLUMN_ATTACHMENT_MESSAGE_ID) var messageId: String = "", @ColumnInfo(name = COLUMN_ATTACHMENT_UPLOADED) - var isUploaded: Boolean = false, + var isUploaded: Boolean = !isLocalAttachmentId(attachmentId), @ColumnInfo(name = COLUMN_ATTACHMENT_UPLOADING) var isUploading: Boolean = false, @ColumnInfo(name = COLUMN_ATTACHMENT_SIGNATURE) @@ -203,17 +204,16 @@ data class Attachment @JvmOverloads constructor( keyPackets = localAttachment.keyPackets, headers = localAttachment.headers ) - } @Throws(MessagingException::class, NoSuchAlgorithmException::class, IOException::class) - fun fromMimeAttachment(part: Part, message_id: String, count: Int): Attachment { + fun fromMimeAttachment(part: Part, messageId: String, count: Int): Attachment { val data = getBytesFromInputStream(part.inputStream) - return fromMimeAttachment(part, data, message_id, count) + return fromMimeAttachment(part, data, messageId, count) } @Throws(MessagingException::class, NoSuchAlgorithmException::class, IOException::class) - fun fromMimeAttachment(part: Part, data: ByteArray, message_id: String, count: Int): Attachment { + fun fromMimeAttachment(part: Part, data: ByteArray, messageId: String, count: Int): Attachment { val attachment = Attachment() attachment.fileName = part.fileName ?: "default" @@ -237,18 +237,18 @@ data class Attachment @JvmOverloads constructor( md5.digest().forEach { b -> format.format("%02x", b) } - attachment.attachmentId = "PGPAttachment/$message_id/$format/$count" + attachment.attachmentId = "PGPAttachment/$messageId/$format/$count" } attachment.fileSize = data.size.toLong() attachment.mimeType = part.contentType.replace("(\\s*)?[\\r\\n]+(\\s*)".toRegex(), "") attachment.mimeData = data - attachment.messageId = message_id + attachment.messageId = messageId return attachment } @Throws(MessagingException::class, NoSuchAlgorithmException::class, IOException::class) - fun fromMimeAttachment(data: ByteArray, headers: InternetHeaders, message_id: String, count: Int): Attachment { - return fromMimeAttachment(MimeBodyPart(headers, ByteArray(0)), data, message_id, count) + fun fromMimeAttachment(data: ByteArray, headers: InternetHeaders, messageId: String, count: Int): Attachment { + return fromMimeAttachment(MimeBodyPart(headers, ByteArray(0)), data, messageId, count) } @Throws(IOException::class) @@ -273,7 +273,12 @@ data class Attachment @JvmOverloads constructor( useRandomIds: Boolean = true ): List { return localAttachmentList.map { localAttachment -> - fromLocalAttachment(messagesDatabase, localAttachment, localAttachmentList.indexOf(localAttachment).toLong(), useRandomIds) + fromLocalAttachment( + messagesDatabase, + localAttachment, + localAttachmentList.indexOf(localAttachment).toLong(), + useRandomIds + ) } } diff --git a/app/src/main/java/ch/protonmail/android/attachments/UploadAttachments.kt b/app/src/main/java/ch/protonmail/android/attachments/UploadAttachmentsWorker.kt similarity index 82% rename from app/src/main/java/ch/protonmail/android/attachments/UploadAttachments.kt rename to app/src/main/java/ch/protonmail/android/attachments/UploadAttachmentsWorker.kt index d5d37e775..bf7d3853f 100644 --- a/app/src/main/java/ch/protonmail/android/attachments/UploadAttachments.kt +++ b/app/src/main/java/ch/protonmail/android/attachments/UploadAttachmentsWorker.kt @@ -33,6 +33,7 @@ import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkerParameters import androidx.work.workDataOf +import ch.protonmail.android.R import ch.protonmail.android.activities.messageDetails.repository.MessageDetailsRepository import ch.protonmail.android.api.models.room.messages.Message import ch.protonmail.android.api.models.room.pendingActions.PendingActionsDao @@ -57,8 +58,8 @@ internal const val KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR = "keyUploadAttach private const val UPLOAD_ATTACHMENTS_WORK_NAME_PREFIX = "uploadAttachmentUniqueWorkName" private const val UPLOAD_ATTACHMENTS_MAX_RETRIES = 1 -class UploadAttachments @WorkerInject constructor( - @Assisted context: Context, +class UploadAttachmentsWorker @WorkerInject constructor( + @Assisted val context: Context, @Assisted params: WorkerParameters, private val dispatchers: DispatcherProvider, private val attachmentsRepository: AttachmentsRepository, @@ -81,7 +82,14 @@ class UploadAttachments @WorkerInject constructor( Timber.w("Calling upload attachments from inside worker for $messageId.") return@withContext when (val result = upload(newAttachments, message, addressCrypto, isMessageSending)) { is Result.Success -> ListenableWorker.Result.success() - is Result.Failure -> retryOrFail(result.error, messageId = message.messageId) + is Result.Failure.UploadAttachment -> retryOrFail( + result.error, + messageId = message.messageId + ) + is Result.Failure.InvalidAttachment -> failureWithError( + result.error, + messageId = message.messageId + ) } } @@ -111,7 +119,7 @@ class UploadAttachments @WorkerInject constructor( if (result is AttachmentsRepository.Result.Failure) { pendingActionsDao.deletePendingUploadByMessageId(messageId) - return@withContext Result.Failure(result.error) + return@withContext Result.Failure.UploadAttachment(result.error) } } @@ -126,12 +134,20 @@ class UploadAttachments @WorkerInject constructor( messageId: String ): Result.Failure? { attachmentIds.forEach { attachmentId -> - val attachment = messageDetailsRepository.findAttachmentById(attachmentId) + val attachment = messageDetailsRepository.findAttachmentById(attachmentId) ?: return@forEach + + if (!attachment.isUploaded && (attachment.filePath == null || attachment.doesFileExist.not())) { + return Result.Failure.InvalidAttachment( + String.format(context.getString(R.string.attachment_failed_message_drafted), + attachment.fileName + ) + ) + } - if (attachment?.filePath == null || attachment.isUploaded || attachment.doesFileExist.not()) { + if (attachment.isUploaded) { Timber.d( - "Skipping attachment ${attachment?.attachmentId}: " + - "not found, invalid or was already uploaded = ${attachment?.isUploaded}" + "Skipping attachment ${attachment.attachmentId}: " + + "was already uploaded = ${attachment.isUploaded}" ) return@forEach } @@ -149,7 +165,7 @@ class UploadAttachments @WorkerInject constructor( is AttachmentsRepository.Result.Failure -> { Timber.e("UploadAttachment $attachmentId to API for messageId $messageId FAILED.") pendingActionsDao.deletePendingUploadByMessageId(messageId) - return Result.Failure(result.error) + return Result.Failure.UploadAttachment(result.error) } } @@ -188,15 +204,28 @@ class UploadAttachments @WorkerInject constructor( return failureWithError(error, exception, messageId) } - private fun failureWithError(error: String, exception: Throwable? = null, messageId: String?): ListenableWorker.Result { - Timber.e("UploadAttachments Worker failed permanently for $messageId. error = $error, exception = $exception. FAILING") - val errorData = workDataOf(KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to error) + private fun failureWithError( + error: String, + exception: Throwable? = null, + messageId: String? + ): ListenableWorker.Result { + Timber.e("UploadAttachments Worker failed permanently for " + + "$messageId. error = $error, exception = $exception. FAILING") + val errorData = workDataOf( + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to error, + ) + messageId?.let { pendingActionsDao.deletePendingUploadByMessageId(it) } return ListenableWorker.Result.failure(errorData) } sealed class Result { object Success : Result() - data class Failure(val error: String) : Result() + sealed class Failure : Result() { + abstract val error: String + + class UploadAttachment(override val error: String) : Failure() + class InvalidAttachment(override val error: String) : Failure() + } } class Enqueuer @Inject constructor(private val workManager: WorkManager) { @@ -209,7 +238,7 @@ class UploadAttachments @WorkerInject constructor( val constraints = Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) .build() - val uploadAttachmentsRequest = OneTimeWorkRequestBuilder() + val uploadAttachmentsRequest = OneTimeWorkRequestBuilder() .setConstraints(constraints) .setInputData( workDataOf( diff --git a/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorker.kt b/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorker.kt index 2e2fe5e69..0455d1c6b 100644 --- a/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorker.kt +++ b/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorker.kt @@ -55,6 +55,7 @@ import ch.protonmail.android.compose.send.SendMessageWorkerError.FetchSendPrefer import ch.protonmail.android.compose.send.SendMessageWorkerError.MessageAlreadySent import ch.protonmail.android.compose.send.SendMessageWorkerError.MessageNotFound import ch.protonmail.android.compose.send.SendMessageWorkerError.SavedDraftMessageNotFound +import ch.protonmail.android.compose.send.SendMessageWorkerError.UploadAttachmentsFailed import ch.protonmail.android.compose.send.SendMessageWorkerError.UserVerificationNeeded import ch.protonmail.android.core.Constants import ch.protonmail.android.core.Constants.MessageLocationType.ALL_MAIL @@ -172,7 +173,10 @@ class SendMessageWorker @WorkerInject constructor( saveMessageAsSent(message) failureWithError(MessageAlreadySent) } - + is SaveDraftResult.UploadDraftAttachmentsFailed -> { + pendingActionsDao.deletePendingSendByMessageId(message.messageId ?: "") + failureWithError(UploadAttachmentsFailed) + } else -> { pendingActionsDao.deletePendingSendByMessageId(message.messageId ?: "") showSendMessageError(message.subject) diff --git a/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorkerError.kt b/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorkerError.kt index aff102cdf..8d3686736 100644 --- a/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorkerError.kt +++ b/app/src/main/java/ch/protonmail/android/compose/send/SendMessageWorkerError.kt @@ -28,5 +28,6 @@ enum class SendMessageWorkerError { ErrorPerformingApiRequest, UserVerificationNeeded, ApiRequestReturnedBadBodyCode, - MessageAlreadySent + MessageAlreadySent, + UploadAttachmentsFailed } diff --git a/app/src/main/java/ch/protonmail/android/servers/notification/INotificationServer.kt b/app/src/main/java/ch/protonmail/android/servers/notification/INotificationServer.kt index 4b2b9380d..258ae5c7e 100644 --- a/app/src/main/java/ch/protonmail/android/servers/notification/INotificationServer.kt +++ b/app/src/main/java/ch/protonmail/android/servers/notification/INotificationServer.kt @@ -93,4 +93,10 @@ interface INotificationServer { ) fun notifySaveDraftError(errorMessage: String, messageSubject: String?, username: String) + + fun notifyAttachmentUploadError( + errorMessage: String, + messageSubject: String?, + username: String + ) } diff --git a/app/src/main/java/ch/protonmail/android/servers/notification/NotificationServer.kt b/app/src/main/java/ch/protonmail/android/servers/notification/NotificationServer.kt index 428cabe8c..e9b9ceb18 100644 --- a/app/src/main/java/ch/protonmail/android/servers/notification/NotificationServer.kt +++ b/app/src/main/java/ch/protonmail/android/servers/notification/NotificationServer.kt @@ -69,6 +69,7 @@ private const val CHANNEL_ID_ONGOING_OPS = "ongoingOperations" private const val CHANNEL_ID_ACCOUNT = "account" private const val CHANNEL_ID_ATTACHMENTS = "attachments" private const val NOTIFICATION_ID_SAVE_DRAFT_ERROR = 6812 +private const val NOTIFICATION_ID_ATTACHMENT_ERROR = 6813 private const val NOTIFICATION_GROUP_ID_EMAIL = 99 private const val NOTIFICATION_ID_VERIFICATION = 2 private const val NOTIFICATION_ID_LOGGED_OUT = 3 @@ -347,7 +348,7 @@ class NotificationServer @Inject constructor( "com.android.systemui", ringtoneUri, Intent.FLAG_GRANT_READ_URI_PERMISSION ) } - } ?: RingtoneManager.getDefaultUri(TYPE_NOTIFICATION) + } ?: RingtoneManager.getDefaultUri(TYPE_NOTIFICATION) } catch (e: Exception) { Timber.i(e, "Unable to set notification ringtone") RingtoneManager.getDefaultUri(TYPE_NOTIFICATION) @@ -557,41 +558,67 @@ class NotificationServer @Inject constructor( notificationManager.notify(username.hashCode() + NOTIFICATION_ID_SENDING_FAILED, notification) } - override fun notifyMultipleErrorSendingMessage( - unreadSendingFailedNotifications: List, - user: User + private fun notifyErrorWithTitle( + username: String, + title: String, + bigText: String, + summaryText: String = username, + notificationID: Int ) { - - val notificationTitle = context.getString(R.string.message_sending_failures, unreadSendingFailedNotifications.size) - - // Create Notification Style val bigTextStyle = NotificationCompat.BigTextStyle() - .setBigContentTitle(notificationTitle) - .setSummaryText(user.defaultEmail ?: user.username ?: context.getString(R.string.app_name)) - .bigText(createSpannableBigText(unreadSendingFailedNotifications)) + .setBigContentTitle(title) + .setSummaryText(summaryText) + .bigText(bigText) - // Create notification builder - val notificationBuilder = createGenericErrorSendingMessageNotification(user.username) + val notificationBuilder = createGenericErrorSendingMessageNotification(username) .setStyle(bigTextStyle) - // Build and show notification val notification = notificationBuilder.build() - notificationManager.notify(user.username.hashCode() + NOTIFICATION_ID_SENDING_FAILED, notification) + notificationManager.notify(username.hashCode() + notificationID, notification) } - override fun notifySaveDraftError(errorMessage: String, messageSubject: String?, username: String) { - val title = context.getString(R.string.failed_saving_draft_online, messageSubject) - - val bigTextStyle = NotificationCompat.BigTextStyle() - .setBigContentTitle(title) - .setSummaryText(username) - .bigText(errorMessage) + override fun notifyMultipleErrorSendingMessage( + unreadSendingFailedNotifications: List, + user: User + ) { + val notificationTitle = context.getString( + R.string.message_sending_failures, unreadSendingFailedNotifications.size + ) - val notificationBuilder = createGenericErrorSendingMessageNotification(username) - .setStyle(bigTextStyle) + notifyErrorWithTitle( + user.username, + notificationTitle, + createSpannableBigText(unreadSendingFailedNotifications).toString(), + user.defaultEmail ?: user.username ?: context.getString(R.string.app_name), + NOTIFICATION_ID_SENDING_FAILED + ) + } - val notification = notificationBuilder.build() - notificationManager.notify(username.hashCode() + NOTIFICATION_ID_SAVE_DRAFT_ERROR, notification) + override fun notifySaveDraftError( + errorMessage: String, + messageSubject: String?, + username: String + ) { + val title = context.getString(R.string.failed_saving_draft_online, messageSubject) + notifyErrorWithTitle( + username, + title, + errorMessage, + notificationID = NOTIFICATION_ID_SAVE_DRAFT_ERROR + ) } + override fun notifyAttachmentUploadError( + errorMessage: String, + messageSubject: String?, + username: String + ) { + val title = context.getString(R.string.failed_uploading_attachment_online, messageSubject) + notifyErrorWithTitle( + username, + title, + errorMessage, + notificationID = NOTIFICATION_ID_ATTACHMENT_ERROR + ) + } } diff --git a/app/src/main/java/ch/protonmail/android/usecase/compose/SaveDraft.kt b/app/src/main/java/ch/protonmail/android/usecase/compose/SaveDraft.kt index 770a83d78..548863285 100644 --- a/app/src/main/java/ch/protonmail/android/usecase/compose/SaveDraft.kt +++ b/app/src/main/java/ch/protonmail/android/usecase/compose/SaveDraft.kt @@ -24,7 +24,7 @@ import ch.protonmail.android.activities.messageDetails.repository.MessageDetails import ch.protonmail.android.api.models.room.messages.Message import ch.protonmail.android.api.models.room.pendingActions.PendingActionsDao import ch.protonmail.android.attachments.KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR -import ch.protonmail.android.attachments.UploadAttachments +import ch.protonmail.android.attachments.UploadAttachmentsWorker import ch.protonmail.android.core.Constants import ch.protonmail.android.core.Constants.MessageLocationType.ALL_DRAFT import ch.protonmail.android.core.Constants.MessageLocationType.ALL_MAIL @@ -55,7 +55,7 @@ class SaveDraft @Inject constructor( private val pendingActionsDao: PendingActionsDao, private val createDraftWorker: CreateDraftWorker.Enqueuer, @CurrentUsername private val username: String, - private val uploadAttachments: UploadAttachments.Enqueuer, + private val uploadAttachmentsWorker: UploadAttachmentsWorker.Enqueuer, private val userNotifier: UserNotifier ) { @@ -135,14 +135,14 @@ class SaveDraft @Inject constructor( localDraft: Message ): SaveDraftResult { val isMessageSending = params.trigger == SaveDraftTrigger.SendingMessage - return uploadAttachments.enqueue(params.newAttachmentIds, createdDraftId, isMessageSending) + return uploadAttachmentsWorker.enqueue(params.newAttachmentIds, createdDraftId, isMessageSending) .filter { it?.state?.isFinished == true } .map { if (it?.state == WorkInfo.State.FAILED) { val errorMessage = requireNotNull( it.outputData.getString(KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR) ) - userNotifier.showPersistentError(errorMessage, localDraft.subject) + userNotifier.showAttachmentUploadError(errorMessage, localDraft.subject) return@map SaveDraftResult.UploadDraftAttachmentsFailed } if (it?.state == WorkInfo.State.CANCELLED) { diff --git a/app/src/main/java/ch/protonmail/android/utils/MessageUtils.kt b/app/src/main/java/ch/protonmail/android/utils/MessageUtils.kt index 8a40769be..282659295 100644 --- a/app/src/main/java/ch/protonmail/android/utils/MessageUtils.kt +++ b/app/src/main/java/ch/protonmail/android/utils/MessageUtils.kt @@ -211,6 +211,15 @@ object MessageUtils { return valid } + fun isLocalAttachmentId(attachmentId: String?): Boolean { + return try { + attachmentId?.toBigInteger() + true + } catch (e: NumberFormatException) { + false + } + } + fun calculateType(flags: Long): Message.MessageType { val received = flags and MessageFlag.RECEIVED.value == MessageFlag.RECEIVED.value val sent = flags and MessageFlag.SENT.value == MessageFlag.SENT.value diff --git a/app/src/main/java/ch/protonmail/android/utils/notifier/AndroidUserNotifier.kt b/app/src/main/java/ch/protonmail/android/utils/notifier/AndroidUserNotifier.kt index 7a2d098b3..81660eb20 100644 --- a/app/src/main/java/ch/protonmail/android/utils/notifier/AndroidUserNotifier.kt +++ b/app/src/main/java/ch/protonmail/android/utils/notifier/AndroidUserNotifier.kt @@ -53,6 +53,10 @@ class AndroidUserNotifier @Inject constructor( notificationServer.notifySingleErrorSendingMessage(error, userManager.username) } + override fun showAttachmentUploadError(errorMessage: String, messageSubject: String?) { + notificationServer.notifyAttachmentUploadError(errorMessage, messageSubject, userManager.username) + } + override suspend fun showMessageSent() { withContext(dispatchers.Main) { context.showToast(R.string.message_sent) @@ -68,5 +72,4 @@ class AndroidUserNotifier @Inject constructor( message.addressID ) } - } diff --git a/app/src/main/java/ch/protonmail/android/utils/notifier/UserNotifier.kt b/app/src/main/java/ch/protonmail/android/utils/notifier/UserNotifier.kt index a7fc09c34..93213411a 100644 --- a/app/src/main/java/ch/protonmail/android/utils/notifier/UserNotifier.kt +++ b/app/src/main/java/ch/protonmail/android/utils/notifier/UserNotifier.kt @@ -23,6 +23,7 @@ interface UserNotifier { fun showPersistentError(errorMessage: String, messageSubject: String?) fun showError(errorMessage: String) fun showSendMessageError(errorMessage: String, messageSubject: String?) + fun showAttachmentUploadError(errorMessage: String, messageSubject: String?) suspend fun showMessageSent() fun showHumanVerificationNeeded(message: ch.protonmail.android.api.models.room.messages.Message) } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f256e3eb9..60c91f9d8 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -63,6 +63,7 @@ along with ProtonMail. If not, see https://www.gnu.org/licenses/. Email address is a duplicate Code is not valid Message sending failed. Your messages are saved in drafts folder + Please remove and re-attach \"%s\" and send the message again. Your message can be found in the "Drafts" folder. Open Close INBOX @@ -1102,6 +1103,7 @@ along with ProtonMail. If not, see https://www.gnu.org/licenses/. Invalid Firebase API key. Push notifications will not work. Failed saving online draft for message: "%s" + Attachment error for message: \"%s\" Can\'t open this message while it\'s being sent diff --git a/app/src/test/java/ch/protonmail/android/attachments/UploadAttachmentsTest.kt b/app/src/test/java/ch/protonmail/android/attachments/UploadAttachmentsWorkerTest.kt similarity index 88% rename from app/src/test/java/ch/protonmail/android/attachments/UploadAttachmentsTest.kt rename to app/src/test/java/ch/protonmail/android/attachments/UploadAttachmentsWorkerTest.kt index 862e0d1f4..9631250b6 100644 --- a/app/src/test/java/ch/protonmail/android/attachments/UploadAttachmentsTest.kt +++ b/app/src/test/java/ch/protonmail/android/attachments/UploadAttachmentsWorkerTest.kt @@ -28,6 +28,7 @@ import androidx.work.OneTimeWorkRequest import androidx.work.WorkManager import androidx.work.WorkerParameters import androidx.work.workDataOf +import ch.protonmail.android.R import ch.protonmail.android.activities.messageDetails.repository.MessageDetailsRepository import ch.protonmail.android.api.models.room.messages.Attachment import ch.protonmail.android.api.models.room.messages.Message @@ -55,7 +56,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -class UploadAttachmentsTest : CoroutinesTest { +class UploadAttachmentsWorkerTest : CoroutinesTest { @RelaxedMockK private lateinit var context: Context @@ -85,7 +86,7 @@ class UploadAttachmentsTest : CoroutinesTest { private lateinit var crypto: AddressCrypto @InjectMockKs - private lateinit var uploadAttachments: UploadAttachments + private lateinit var uploadAttachmentsWorker: UploadAttachmentsWorker private val currentUsername = "current username" @@ -114,7 +115,7 @@ class UploadAttachmentsTest : CoroutinesTest { every { cryptoFactory.create(Id(addressId), Name(currentUsername)) } returns crypto // When - UploadAttachments.Enqueuer(workManager).enqueue( + UploadAttachmentsWorker.Enqueuer(workManager).enqueue( attachmentIds, messageId, true @@ -154,7 +155,7 @@ class UploadAttachmentsTest : CoroutinesTest { givenFullValidInput(messageId, attachmentIds.toTypedArray()) coEvery { messageDetailsRepository.findMessageById(messageId) } returns message - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() val pendingUpload = PendingUpload(messageId) verify { pendingActionsDao.insertPendingForUpload(pendingUpload) } @@ -169,11 +170,13 @@ class UploadAttachmentsTest : CoroutinesTest { givenFullValidInput(messageId, attachmentIds.toTypedArray()) coEvery { messageDetailsRepository.findMessageById(messageId) } returns null - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() assertEquals( ListenableWorker.Result.failure( - workDataOf(KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to "Message not found") + workDataOf( + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to "Message not found", + ) ), result ) @@ -184,14 +187,25 @@ class UploadAttachmentsTest : CoroutinesTest { @Test fun uploadAttachmentsExecutesNormallyWhenUploadWasLeftOngoingForTheGivenMessage() { runBlockingTest { + val attachment1 = mockk(relaxed = true) { + every { attachmentId } returns "1" + every { filePath } returns "filePath1" + every { isUploaded } returns false + every { doesFileExist } returns true + } val attachmentIds = listOf("1") val messageId = "message-id-123842" val message = Message(messageId = messageId, addressID = "senderAddress1") givenFullValidInput(messageId, attachmentIds.toTypedArray()) + every { cryptoFactory.create(Id("senderAddress1"), Name(currentUsername)) } returns crypto coEvery { messageDetailsRepository.findMessageById(messageId) } returns message every { pendingActionsDao.findPendingUploadByMessageId(messageId) } returns PendingUpload(messageId) + every { messageDetailsRepository.findAttachmentById("1") } returns attachment1 + coEvery { attachmentsRepository.upload(attachment1, crypto) } answers { + AttachmentsRepository.Result.Success("1") + } - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() verify { pendingActionsDao.insertPendingForUpload(any()) } assertEquals(ListenableWorker.Result.success(), result) @@ -222,7 +236,7 @@ class UploadAttachmentsTest : CoroutinesTest { every { messageDetailsRepository.findAttachmentById("1") } returns attachment1 every { messageDetailsRepository.findAttachmentById("2") } returns attachment2 - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() coVerifyOrder { attachment1.setMessage(message) @@ -263,7 +277,7 @@ class UploadAttachmentsTest : CoroutinesTest { } every { parameters.runAttemptCount } returns 0 - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() assertEquals(ListenableWorker.Result.retry(), result) verify { pendingActionsDao.deletePendingUploadByMessageId("messageId8238") } @@ -275,12 +289,14 @@ class UploadAttachmentsTest : CoroutinesTest { runBlockingTest { val attachment1 = mockk(relaxed = true) { every { attachmentId } returns "1" + every { fileName } returns "Attachment_1.jpg" every { filePath } returns "filePath1" every { isUploaded } returns false every { doesFileExist } returns true } val attachment2 = mockk(relaxed = true) { every { attachmentId } returns "2" + every { fileName } returns "Attachment_2.jpg" every { filePath } returns "filePath2" every { isUploaded } returns false every { doesFileExist } returns true @@ -298,11 +314,13 @@ class UploadAttachmentsTest : CoroutinesTest { } every { parameters.runAttemptCount } returns 4 - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() assertEquals( ListenableWorker.Result.failure( - workDataOf(KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to "Failed to upload attachment2") + workDataOf( + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to "Failed to upload attachment2", + ) ), result ) @@ -313,6 +331,12 @@ class UploadAttachmentsTest : CoroutinesTest { @Test fun uploadAttachmentsRetriesIfPublicKeyFailsToBeUploadedAndMaxRetriesWereNotReached() { runBlockingTest { + val attachment1 = mockk(relaxed = true) { + every { attachmentId } returns "1" + every { filePath } returns "filePath1" + every { isUploaded } returns false + every { doesFileExist } returns true + } val attachmentIds = listOf("1") val messageId = "messageId9273584" val username = "username" @@ -321,13 +345,14 @@ class UploadAttachmentsTest : CoroutinesTest { every { cryptoFactory.create(Id("senderAddress13"), Name(username)) } returns crypto coEvery { messageDetailsRepository.findMessageById(messageId) } returns message every { userManager.username } returns username + every { messageDetailsRepository.findAttachmentById("1") } returns attachment1 every { userManager.getMailSettings(username)?.getAttachPublicKey() } returns true coEvery { attachmentsRepository.uploadPublicKey(message, crypto) } answers { AttachmentsRepository.Result.Failure("Failed to upload public key") } every { parameters.runAttemptCount } returns 0 - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() assertEquals(ListenableWorker.Result.retry(), result) verify { pendingActionsDao.deletePendingUploadByMessageId("messageId9273584") } @@ -337,6 +362,12 @@ class UploadAttachmentsTest : CoroutinesTest { @Test fun uploadAttachmentsReturnsFailureIfPublicKeyFailsToBeUploadedAndMaxRetriesWereReached() { runBlockingTest { + val attachment1 = mockk(relaxed = true) { + every { attachmentId } returns "1" + every { filePath } returns "filePath1" + every { isUploaded } returns false + every { doesFileExist } returns true + } val attachmentIds = listOf("1") val messageId = "messageId9273585" val username = "username" @@ -345,17 +376,20 @@ class UploadAttachmentsTest : CoroutinesTest { every { cryptoFactory.create(Id("senderAddress12"), Name(username)) } returns crypto coEvery { messageDetailsRepository.findMessageById(messageId) } returns message every { userManager.username } returns username + every { messageDetailsRepository.findAttachmentById("1") } returns attachment1 every { userManager.getMailSettings(username)?.getAttachPublicKey() } returns true coEvery { attachmentsRepository.uploadPublicKey(message, crypto) } answers { AttachmentsRepository.Result.Failure("Failed to upload public key") } every { parameters.runAttemptCount } returns 4 - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() assertEquals( ListenableWorker.Result.failure( - workDataOf(KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to "Failed to upload public key") + workDataOf( + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to "Failed to upload public key", + ) ), result ) @@ -381,23 +415,25 @@ class UploadAttachmentsTest : CoroutinesTest { every { messageDetailsRepository.findAttachmentById("1") } returns null every { messageDetailsRepository.findAttachmentById("2") } returns attachment2 - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() coVerifySequence { attachmentsRepository.upload(attachment2, crypto) } } } @Test - fun uploadAttachmentsSkipsUploadingIfAttachmentFilePathIsNull() { + fun uploadAttachmentsFailsWorkerIfAttachmentFilePathIsNull() { runBlockingTest { val attachment1 = mockk(relaxed = true) { every { attachmentId } returns "1" + every { fileName } returns "Attachment_1.jpg" every { filePath } returns null every { isUploaded } returns false every { doesFileExist } returns true } val attachment2 = mockk(relaxed = true) { every { attachmentId } returns "2" + every { fileName } returns "Attachment_2.jpg" every { filePath } returns "filePath2" every { isUploaded } returns false every { doesFileExist } returns true @@ -408,13 +444,27 @@ class UploadAttachmentsTest : CoroutinesTest { givenFullValidInput(messageId, attachmentIds.toTypedArray()) every { cryptoFactory.create(Id("senderAddress4"), Name(currentUsername)) } returns crypto coEvery { messageDetailsRepository.findMessageById(messageId) } returns message + every { context.getString(R.string.attachment_failed_message_drafted) } returns + "Please remove and re-attach \"%s\" and send the message again. " + + "Your message can be found in the \"Drafts\" folder." every { messageDetailsRepository.findAttachmentById("1") } returns attachment1 every { messageDetailsRepository.findAttachmentById("2") } returns attachment2 - uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() coVerify(exactly = 0) { attachmentsRepository.upload(attachment1, crypto) } - coVerify { attachmentsRepository.upload(attachment2, crypto) } + coVerify(exactly = 0) { attachmentsRepository.upload(attachment2, crypto) } + assertEquals( + ListenableWorker.Result.failure( + workDataOf( + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to + "Please remove and re-attach \"Attachment_1.jpg\" and send the message again. " + + "Your message can be found in the \"Drafts\" folder.", + ) + ), + result + ) + verify { pendingActionsDao.deletePendingUploadByMessageId("messageId36926543") } } } @@ -442,7 +492,7 @@ class UploadAttachmentsTest : CoroutinesTest { every { messageDetailsRepository.findAttachmentById("1") } returns attachment1 every { messageDetailsRepository.findAttachmentById("2") } returns attachment2 - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() coVerify { attachmentsRepository.upload(attachment1, crypto) } coVerify(exactly = 0) { attachmentsRepository.upload(attachment2, crypto) } @@ -473,7 +523,7 @@ class UploadAttachmentsTest : CoroutinesTest { every { messageDetailsRepository.findAttachmentById("1") } returns attachmentMock1 every { messageDetailsRepository.findAttachmentById("2") } returns attachmentMock2 - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() coVerify { attachmentsRepository.upload(attachmentMock1, crypto) } coVerify(exactly = 0) { attachmentsRepository.upload(attachmentMock2, crypto) } @@ -487,7 +537,7 @@ class UploadAttachmentsTest : CoroutinesTest { val messageId = "messageId823762" val message = Message(messageId = messageId, addressID = "senderAddress6") every { userManager.username } returns username - givenFullValidInput(messageId, isMessageSending = true) + givenFullValidInput(messageId, attachments = emptyArray(), isMessageSending = true) every { cryptoFactory.create(Id("senderAddress6"), Name(username)) } returns crypto coEvery { messageDetailsRepository.findMessageById(messageId) } returns message every { userManager.getMailSettings(username)?.getAttachPublicKey() } returns true @@ -495,7 +545,7 @@ class UploadAttachmentsTest : CoroutinesTest { AttachmentsRepository.Result.Success("23421") } - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() coVerify { attachmentsRepository.uploadPublicKey(message, crypto) } } @@ -531,7 +581,7 @@ class UploadAttachmentsTest : CoroutinesTest { AttachmentsRepository.Result.Failure("failed") } - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() verify { attachmentMock1.deleteLocalFile() } verify(exactly = 0) { attachmentMock2.deleteLocalFile() } @@ -581,8 +631,10 @@ class UploadAttachmentsTest : CoroutinesTest { message.setAttachmentList(listOf(attachmentMock1, attachmentMock2)) every { messageDetailsRepository.findAttachmentById("1") } returns attachmentMock1 every { messageDetailsRepository.findAttachmentById("2") } returns attachmentMock2 - every { messageDetailsRepository.findAttachmentById(uploadedAttachmentMock1Id) } returns uploadedAttachmentMock1 - every { messageDetailsRepository.findAttachmentById(uploadedAttachmentMock2Id) } returns uploadedAttachmentMock2 + every { messageDetailsRepository.findAttachmentById(uploadedAttachmentMock1Id) } returns + uploadedAttachmentMock1 + every { messageDetailsRepository.findAttachmentById(uploadedAttachmentMock2Id) } returns + uploadedAttachmentMock2 coEvery { attachmentsRepository.upload(attachmentMock1, crypto) } answers { AttachmentsRepository.Result.Success(uploadedAttachmentMock1Id) } @@ -590,7 +642,7 @@ class UploadAttachmentsTest : CoroutinesTest { AttachmentsRepository.Result.Success(uploadedAttachmentMock2Id) } - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() val actualMessage = slot() val expectedAttachments = listOf(uploadedAttachmentMock1, uploadedAttachmentMock2) @@ -622,7 +674,7 @@ class UploadAttachmentsTest : CoroutinesTest { AttachmentsRepository.Result.Failure("Failed uploading attachment!") } - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() verify(exactly = 1) { messageDetailsRepository.findAttachmentById("1") } coVerify(exactly = 0) { messageDetailsRepository.saveMessageLocally(any()) } @@ -668,7 +720,7 @@ class UploadAttachmentsTest : CoroutinesTest { AttachmentsRepository.Result.Success(uploadedAttachment2Id) } - uploadAttachments.doWork() + uploadAttachmentsWorker.doWork() val actualMessage = slot() val expectedAttachments = listOf(attachmentMock1) @@ -690,7 +742,7 @@ class UploadAttachmentsTest : CoroutinesTest { coEvery { messageDetailsRepository.findMessageById(messageId) } returns message coEvery { attachmentsRepository.uploadPublicKey(message, crypto) } returns mockk() - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() coVerify(exactly = 0) { attachmentsRepository.uploadPublicKey(any(), any()) } assertEquals(ListenableWorker.Result.success(), result) @@ -713,7 +765,7 @@ class UploadAttachmentsTest : CoroutinesTest { AttachmentsRepository.Result.Success("82384") } - val result = uploadAttachments.doWork() + val result = uploadAttachmentsWorker.doWork() coVerify { attachmentsRepository.uploadPublicKey(any(), any()) } assertEquals(ListenableWorker.Result.success(), result) @@ -725,12 +777,11 @@ class UploadAttachmentsTest : CoroutinesTest { attachments: Array = arrayOf("attId62364"), isMessageSending: Boolean = false ) { - every { parameters.inputData.getStringArray(KEY_INPUT_UPLOAD_ATTACHMENTS_ATTACHMENT_IDS) } answers { attachments } + every { parameters.inputData.getStringArray(KEY_INPUT_UPLOAD_ATTACHMENTS_ATTACHMENT_IDS) } answers + { attachments } every { parameters.inputData.getString(KEY_INPUT_UPLOAD_ATTACHMENTS_MESSAGE_ID) } answers { messageId } every { parameters.inputData.getBoolean(KEY_INPUT_UPLOAD_ATTACHMENTS_IS_MESSAGE_SENDING, false) } answers { isMessageSending } } } - - diff --git a/app/src/test/java/ch/protonmail/android/compose/send/SendMessageWorkerTest.kt b/app/src/test/java/ch/protonmail/android/compose/send/SendMessageWorkerTest.kt index a62b181ff..1c6ac6247 100644 --- a/app/src/test/java/ch/protonmail/android/compose/send/SendMessageWorkerTest.kt +++ b/app/src/test/java/ch/protonmail/android/compose/send/SendMessageWorkerTest.kt @@ -42,6 +42,7 @@ import ch.protonmail.android.api.models.factories.SendPreferencesFactory import ch.protonmail.android.api.models.messages.send.MessageSendBody import ch.protonmail.android.api.models.messages.send.MessageSendKey import ch.protonmail.android.api.models.messages.send.MessageSendPackage +import ch.protonmail.android.api.models.room.messages.Attachment import ch.protonmail.android.api.models.room.messages.Message import ch.protonmail.android.api.models.room.pendingActions.PendingActionsDao import ch.protonmail.android.core.Constants @@ -289,6 +290,35 @@ class SendMessageWorkerTest : CoroutinesTest { ) } + @Test + fun workerFailsWithoutNotifyingUserWhenSaveDraftFailsWithUploadAttachmentError() = runBlockingTest { + // we notify the user from the Save Draft Worker, + // refer to test `SaveDraftTest.notifyUserWithUploadAttachmentErrorWhenAttachmentIsBroken` + val messageDbId = 2834L + val messageId = "823472" + val message = Message().apply { + dbId = messageDbId + this.messageId = messageId + this.subject = "Subject 003" + this.Attachments = listOf(Attachment(attachmentId = "attachmentId234", fileName = "Attachment_234.jpg")) + } + givenFullValidInput(messageDbId, messageId) + coEvery { messageDetailsRepository.findMessageByMessageDbId(messageDbId) } returns message + coEvery { saveDraft(any()) } returns SaveDraftResult.UploadDraftAttachmentsFailed + every { parameters.runAttemptCount } returns 0 + + val result = worker.doWork() + + verify { pendingActionsDao.deletePendingSendByMessageId("823472") } + verify { userNotifier wasNot Called } + assertEquals( + ListenableWorker.Result.failure( + workDataOf(KEY_OUTPUT_RESULT_SEND_MESSAGE_ERROR_ENUM to "UploadAttachmentsFailed") + ), + result + ) + } + @Test fun workerFailsWithoutNotifyingUserWhenSaveDraftFailsWithMessageAlreadySentError() = runBlockingTest { val messageDbId = 2835L diff --git a/app/src/test/java/ch/protonmail/android/usecase/compose/SaveDraftTest.kt b/app/src/test/java/ch/protonmail/android/usecase/compose/SaveDraftTest.kt index 9b7c5c5d1..de2c83410 100644 --- a/app/src/test/java/ch/protonmail/android/usecase/compose/SaveDraftTest.kt +++ b/app/src/test/java/ch/protonmail/android/usecase/compose/SaveDraftTest.kt @@ -23,11 +23,12 @@ import androidx.work.Data import androidx.work.WorkInfo import androidx.work.workDataOf import ch.protonmail.android.activities.messageDetails.repository.MessageDetailsRepository +import ch.protonmail.android.api.models.room.messages.Attachment import ch.protonmail.android.api.models.room.messages.Message import ch.protonmail.android.api.models.room.pendingActions.PendingActionsDao import ch.protonmail.android.api.models.room.pendingActions.PendingSend import ch.protonmail.android.attachments.KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR -import ch.protonmail.android.attachments.UploadAttachments +import ch.protonmail.android.attachments.UploadAttachmentsWorker import ch.protonmail.android.core.Constants.MessageActionType.FORWARD import ch.protonmail.android.core.Constants.MessageActionType.NONE import ch.protonmail.android.core.Constants.MessageActionType.REPLY @@ -72,7 +73,7 @@ class SaveDraftTest : CoroutinesTest { private lateinit var userNotifier: UserNotifier @RelaxedMockK - private lateinit var uploadAttachments: UploadAttachments.Enqueuer + private lateinit var uploadAttachmentsWorker: UploadAttachmentsWorker.Enqueuer @RelaxedMockK private lateinit var createDraftScheduler: Enqueuer @@ -351,7 +352,7 @@ class SaveDraftTest : CoroutinesTest { "previousSenderId132423" ) } answers { workerStatusFlow } - coEvery { uploadAttachments.enqueue(any(), "createdDraftMessageId", false) } returns buildWorkerResponse( + coEvery { uploadAttachmentsWorker.enqueue(any(), "createdDraftMessageId", false) } returns buildWorkerResponse( WorkInfo.State.SUCCEEDED ) @@ -404,7 +405,7 @@ class SaveDraftTest : CoroutinesTest { } answers { workerStatusFlow } val addressCrypto = mockk(relaxed = true) every { addressCryptoFactory.create(Id("addressId"), Name(currentUsername)) } returns addressCrypto - coEvery { uploadAttachments.enqueue(any(), "createdDraftMessageId345", false) } returns buildWorkerResponse( + coEvery { uploadAttachmentsWorker.enqueue(any(), "createdDraftMessageId345", false) } returns buildWorkerResponse( WorkInfo.State.SUCCEEDED ) @@ -421,7 +422,7 @@ class SaveDraftTest : CoroutinesTest { ) // Then - coVerify { uploadAttachments.enqueue(newAttachmentIds, "createdDraftMessageId345", false) } + coVerify { uploadAttachmentsWorker.enqueue(newAttachmentIds, "createdDraftMessageId345", false) } } } @@ -456,7 +457,7 @@ class SaveDraftTest : CoroutinesTest { } answers { workerStatusFlow } val addressCrypto = mockk(relaxed = true) every { addressCryptoFactory.create(Id("addressId"), Name(currentUsername)) } returns addressCrypto - coEvery { uploadAttachments.enqueue(any(), "createdDraftMessageId346", true) } returns buildWorkerResponse( + coEvery { uploadAttachmentsWorker.enqueue(any(), "createdDraftMessageId346", true) } returns buildWorkerResponse( WorkInfo.State.SUCCEEDED ) @@ -473,7 +474,7 @@ class SaveDraftTest : CoroutinesTest { ) // Then - coVerify { uploadAttachments.enqueue(newAttachmentIds, "createdDraftMessageId346", true) } + coVerify { uploadAttachmentsWorker.enqueue(newAttachmentIds, "createdDraftMessageId346", true) } assertEquals(SaveDraftResult.Success(apiDraft.messageId!!), result) } } @@ -523,7 +524,7 @@ class SaveDraftTest : CoroutinesTest { ) // Then - coVerify(exactly = 0) { uploadAttachments.enqueue(any(), any(), any()) } + coVerify(exactly = 0) { uploadAttachmentsWorker.enqueue(any(), any(), any()) } assertEquals(SaveDraftResult.Success(apiDraft.messageId!!), result) } } @@ -619,7 +620,7 @@ class SaveDraftTest : CoroutinesTest { } @Test - fun saveDraftsShowPersistentErrorAndReturnsErrorWhenUploadingNewAttachmentsFails() { + fun saveDraftsShowUploadAttachmentErrorAndReturnsErrorWhenUploadingNewAttachmentsFails() { runBlockingTest { // Given val localDraftId = "8345" @@ -630,6 +631,10 @@ class SaveDraftTest : CoroutinesTest { decryptedBody = "Message body in plain text" localId = localDraftId subject = "Message Subject" + this.Attachments = listOf( + Attachment(attachmentId = "2345", fileName = "Attachment_2345.jpg"), + Attachment(attachmentId = "453", fileName = "Attachment_453.jpg"), + ) } val newAttachmentIds = listOf("2345", "453") val createDraftOutputData = workDataOf( @@ -638,12 +643,12 @@ class SaveDraftTest : CoroutinesTest { val createDraftWorkerResult = buildWorkerResponse(WorkInfo.State.SUCCEEDED, createDraftOutputData) val errorMessage = "Can't upload attachments" val uploadWorkOutputData = workDataOf( - KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to errorMessage + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to errorMessage, ) coEvery { messageDetailsRepository.saveMessageLocally(message) } returns 9833L coEvery { messageDetailsRepository.findMessageById("newDraftId") } returns message.copy(messageId = "newDraftId") coEvery { messageDetailsRepository.findMessageById("45623") } returns message - coEvery { uploadAttachments.enqueue(newAttachmentIds, "newDraftId", false) } returns buildWorkerResponse( + coEvery { uploadAttachmentsWorker.enqueue(newAttachmentIds, "newDraftId", false) } returns buildWorkerResponse( WorkInfo.State.FAILED, uploadWorkOutputData ) @@ -669,7 +674,79 @@ class SaveDraftTest : CoroutinesTest { ) // Then - verify { userNotifier.showPersistentError(errorMessage, "Message Subject") } + verify { userNotifier.showAttachmentUploadError(errorMessage, "Message Subject") } + assertEquals(SaveDraftResult.UploadDraftAttachmentsFailed, result) + } + } + + @Test + fun notifyUserWithUploadAttachmentErrorWhenAttachmentIsBroken() { + runBlockingTest { + // Given + val localDraftId = "8345" + val message = Message().apply { + dbId = 123L + this.messageId = "45623" + addressID = "addressId" + decryptedBody = "Message body in plain text" + localId = localDraftId + subject = "Message Subject" + this.Attachments = listOf( + Attachment( + attachmentId = "2345", + fileName = "Attachment_2345.jpg", + isUploaded = false, + filePath = null + ), + Attachment(attachmentId = "453", fileName = "Attachment_453.jpg"), + ) + } + val newAttachmentIds = listOf("2345", "453") + val createDraftOutputData = workDataOf( + KEY_OUTPUT_RESULT_SAVE_DRAFT_MESSAGE_ID to "newDraftId" + ) + val createDraftWorkerResult = buildWorkerResponse(WorkInfo.State.SUCCEEDED, createDraftOutputData) + val errorMessage = "Invalid attachment." + val uploadWorkOutputData = workDataOf( + KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to errorMessage, + ) + coEvery { messageDetailsRepository.saveMessageLocally(message) } returns 9833L + coEvery { messageDetailsRepository.findMessageById("newDraftId") } returns + message.copy(messageId = "newDraftId") + coEvery { messageDetailsRepository.findMessageById("45623") } returns message + coEvery { + uploadAttachmentsWorker.enqueue( + newAttachmentIds, + "newDraftId", + false + ) + } returns buildWorkerResponse( + WorkInfo.State.FAILED, + uploadWorkOutputData + ) + every { + createDraftScheduler.enqueue( + message, + null, + FORWARD, + "previousSenderId132423" + ) + } answers { createDraftWorkerResult } + + // When + val result = saveDraft.invoke( + SaveDraftParameters( + message, + newAttachmentIds, + null, + FORWARD, + "previousSenderId132423", + SaveDraft.SaveDraftTrigger.UserRequested + ) + ) + + // Then + verify { userNotifier.showAttachmentUploadError(errorMessage, "Message Subject") } assertEquals(SaveDraftResult.UploadDraftAttachmentsFailed, result) } } @@ -697,7 +774,7 @@ class SaveDraftTest : CoroutinesTest { messageDetailsRepository.findMessageById("UploadDraftAttachmentsFailed") } returns message.copy(messageId = "newDraftId2384") coEvery { messageDetailsRepository.findMessageById("45623") } returns message - coEvery { uploadAttachments.enqueue(newAttachmentIds, "newDraftId2384", false) } returns buildWorkerResponse( + coEvery { uploadAttachmentsWorker.enqueue(newAttachmentIds, "newDraftId2384", false) } returns buildWorkerResponse( WorkInfo.State.CANCELLED ) every { @@ -757,7 +834,7 @@ class SaveDraftTest : CoroutinesTest { } answers { workerStatusFlow } val addressCrypto = mockk(relaxed = true) every { addressCryptoFactory.create(Id("addressId"), Name(currentUsername)) } returns addressCrypto - coEvery { uploadAttachments.enqueue(any(), "createdDraftMessageId345", false) } returns buildWorkerResponse( + coEvery { uploadAttachmentsWorker.enqueue(any(), "createdDraftMessageId345", false) } returns buildWorkerResponse( WorkInfo.State.SUCCEEDED ) @@ -774,7 +851,7 @@ class SaveDraftTest : CoroutinesTest { ) // Then - verify { uploadAttachments.enqueue(newAttachmentIds, "createdDraftMessageId345", false) } + verify { uploadAttachmentsWorker.enqueue(newAttachmentIds, "createdDraftMessageId345", false) } assertEquals(SaveDraftResult.Success("createdDraftMessageId345"), result) } } diff --git a/app/src/test/java/ch/protonmail/android/utils/MessageUtilsTest.kt b/app/src/test/java/ch/protonmail/android/utils/MessageUtilsTest.kt new file mode 100644 index 000000000..4e25fd5e4 --- /dev/null +++ b/app/src/test/java/ch/protonmail/android/utils/MessageUtilsTest.kt @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2020 Proton Technologies AG + * + * This file is part of ProtonMail. + * + * ProtonMail is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * ProtonMail is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with ProtonMail. If not, see https://www.gnu.org/licenses/. + */ + +package ch.protonmail.android.utils + +import org.junit.Test +import kotlin.test.assertEquals + +class MessageUtilsTest { + + @Test + fun `isLocalAttachmentId returning true if attachmentId is missing`() { + // given + val attachmentId = null + + // when + val result = MessageUtils.isLocalAttachmentId(attachmentId) + + // then + assertEquals(true, result) + } + + @Test + fun `isLocalAttachmentId returning true if attachmentId is local attachmentId`() { + // given + val attachmentId = "-1234567890" + + // when + val result = MessageUtils.isLocalAttachmentId(attachmentId) + + // then + assertEquals(true, result) + } + + @Test + fun `isLocalAttachmentId returning true if attachmentId is remote attachmentId`() { + // given + val attachmentId = "e-aIFdwkb1WtpBk9rTJY63afcvA9yE7lkjApbGzbdvn-Gbugg56h3LwUrD9b-qXWoMcJuNKecwhiv_mKG1ZrzQ==" + + // when + val result = MessageUtils.isLocalAttachmentId(attachmentId) + + // then + assertEquals(false, result) + } +} diff --git a/buildSrc/src/main/kotlin/ProtonMail.kt b/buildSrc/src/main/kotlin/ProtonMail.kt index 389cf2949..8c7c7237a 100644 --- a/buildSrc/src/main/kotlin/ProtonMail.kt +++ b/buildSrc/src/main/kotlin/ProtonMail.kt @@ -23,8 +23,8 @@ */ object ProtonMail { - const val versionName = "1.13.38" - const val versionCode = 785 + const val versionName = "1.13.39" + const val versionCode = 786 const val targetSdk = 30 const val minSdk = 21