Refetches the pgp mime messages to get the attachments

As the pgp/mime messages are encrypted together with the message body,
the handling of those attachments needs to be different. The
LocalAttachment objects will contain all the attachment data and can't
be passed around between the activities.

The blocking call is far from perfect but making it async lead to race
conditions- the attachments sometimes would fail to be uploaded
correctly and not show in the attachments screen. As this is a rather
rare type of message, it should not have too much of an impact.

MAILAND-2894
This commit is contained in:
Maciej Surmacz 2022-04-29 19:23:44 +02:00 committed by Davide Farella
parent 4b4a92737b
commit 990b6d3b1a
7 changed files with 207 additions and 53 deletions

View File

@ -50,7 +50,6 @@ import android.view.Gravity;
import android.view.KeyEvent;
import android.view.Menu;
import android.view.MenuItem;
import android.view.MotionEvent;
import android.view.View;
import android.view.ViewTreeObserver;
import android.webkit.WebSettings;
@ -204,7 +203,6 @@ public class ComposeMessageActivity
public static final String EXTRA_LOAD_IMAGES = "load_images";
public static final String EXTRA_LOAD_REMOTE_CONTENT = "load_remote_content";
private static final int REQUEST_CODE_ADD_ATTACHMENTS = 1;
private static final String STATE_ATTACHMENT_LIST = "attachment_list";
private static final String STATE_ADDITIONAL_ROWS_VISIBLE = "additional_rows_visible";
private static final String STATE_DRAFT_ID = "draft_id";
private static final String STATE_ADDED_CONTENT = "added_content";
@ -948,7 +946,6 @@ public class ComposeMessageActivity
@Override
public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
outState.putParcelableArrayList(STATE_ATTACHMENT_LIST, new ArrayList<>(composeMessageViewModel.getMessageDataResult().getAttachmentList()));
outState.putBoolean(STATE_ADDITIONAL_ROWS_VISIBLE, mAreAdditionalRowsVisible);
outState.putString(STATE_DRAFT_ID, composeMessageViewModel.getDraftId());
if (largeBody) {
@ -966,7 +963,6 @@ public class ComposeMessageActivity
@Override
public void onRestoreInstanceState(@NonNull final Bundle savedInstanceState) {
super.onRestoreInstanceState(savedInstanceState);
composeMessageViewModel.setAttachmentList(savedInstanceState.getParcelableArrayList(STATE_ATTACHMENT_LIST));
mAreAdditionalRowsVisible = savedInstanceState.getBoolean(STATE_ADDITIONAL_ROWS_VISIBLE);
String draftId = savedInstanceState.getString(STATE_DRAFT_ID);
composeMessageViewModel.setDraftId(!TextUtils.isEmpty(draftId) ? draftId : "");
@ -1785,16 +1781,6 @@ public class ComposeMessageActivity
}
}
private class ParentViewScrollListener implements View.OnTouchListener {
@Override
public boolean onTouch(View v, MotionEvent event) {
messageBodyEditText.setFocusableInTouchMode(true);
messageBodyEditText.requestFocus();
return false;
}
}
private class RespondInlineButtonClickListener implements View.OnClickListener {
@Override
public void onClick(View v) {

View File

@ -18,6 +18,7 @@
*/
package ch.protonmail.android.activities.messageDetails
import android.net.Uri
import ch.protonmail.android.api.models.User
import ch.protonmail.android.api.models.address.Address
import ch.protonmail.android.core.BigContentHolder
@ -29,7 +30,6 @@ import ch.protonmail.android.data.local.model.Message
class IntentExtrasData(
val user: User,
val userAddresses: List<Address>,
val message: Message,
val toRecipientListString: String,
val messageCcList: String,
val includeCCList: Boolean,
@ -176,6 +176,9 @@ class IntentExtrasData(
ArrayList(LocalAttachment.createLocalAttachmentList(embeddedImagesAttachments))
this.attachments = att
this.embeddedImagesAttachmentsExist = true
} else if (message.isPGPMime) {
attachments.forEach { it.uri = Uri.EMPTY }
this.attachments = attachments
} else {
this.attachments = attachments
}
@ -184,7 +187,6 @@ class IntentExtrasData(
fun build() = IntentExtrasData(
user,
userAddresses,
message,
toRecipientListString,
messageCcList,
includeCCList,

View File

@ -21,6 +21,7 @@ package ch.protonmail.android.compose
import android.annotation.SuppressLint
import android.content.Context
import android.graphics.Color
import android.net.Uri
import android.text.Spanned
import android.text.TextUtils
import android.webkit.WebView
@ -28,6 +29,7 @@ import androidx.core.net.MailTo
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.liveData
import androidx.lifecycle.map
import androidx.lifecycle.switchMap
import androidx.lifecycle.viewModelScope
import ch.protonmail.android.R
@ -51,8 +53,6 @@ import ch.protonmail.android.contacts.details.presentation.model.ContactLabelUiM
import ch.protonmail.android.core.Constants
import ch.protonmail.android.core.Constants.MessageLocationType
import ch.protonmail.android.core.UserManager
import ch.protonmail.android.crypto.CipherText
import ch.protonmail.android.crypto.Crypto.Companion.forAddress
import ch.protonmail.android.data.local.model.Attachment
import ch.protonmail.android.data.local.model.LocalAttachment
import ch.protonmail.android.data.local.model.Message
@ -67,6 +67,7 @@ import ch.protonmail.android.usecase.compose.SaveDraft
import ch.protonmail.android.usecase.compose.SaveDraftResult
import ch.protonmail.android.usecase.delete.DeleteMessage
import ch.protonmail.android.usecase.fetch.FetchPublicKeys
import ch.protonmail.android.usecase.message.GetDecryptedMessageById
import ch.protonmail.android.usecase.model.FetchPublicKeysRequest
import ch.protonmail.android.usecase.model.FetchPublicKeysResult
import ch.protonmail.android.utils.Event
@ -82,6 +83,7 @@ import io.reactivex.Single
import io.reactivex.disposables.CompositeDisposable
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
@ -92,10 +94,10 @@ import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import me.proton.core.accountmanager.domain.AccountManager
import me.proton.core.domain.entity.UserId
import me.proton.core.user.domain.entity.AddressId
import me.proton.core.util.kotlin.DispatcherProvider
import me.proton.core.util.kotlin.EMPTY_STRING
import timber.log.Timber
@ -128,7 +130,8 @@ class ComposeMessageViewModel @Inject constructor(
networkConfigurator: NetworkConfigurator,
private val htmlToSpanned: HtmlToSpanned,
private val addExpirationTimeToMessage: AddExpirationTimeToMessage,
private val setUpWebViewDarkModeHandlingIfSupported: SetUpWebViewDarkModeHandlingIfSupported
private val setUpWebViewDarkModeHandlingIfSupported: SetUpWebViewDarkModeHandlingIfSupported,
private val getDecryptedMessageById: GetDecryptedMessageById
) : ConnectivityBaseViewModel(verifyConnection, networkConfigurator) {
// region events data
@ -210,7 +213,7 @@ class ComposeMessageViewModel @Inject constructor(
val loadingDraftResult: LiveData<Message>
get() = _loadingDraftResult
val openAttachmentsScreenResult: LiveData<List<LocalAttachment>>
get() = _openAttachmentsScreenResult
get() = _openAttachmentsScreenResult.map { it.withoutPgpData() }
val buildingMessageCompleted: LiveData<Event<Message>>
get() = _buildingMessageCompleted
val dbIdWatcher: LiveData<Long?>
@ -325,12 +328,33 @@ class ComposeMessageViewModel @Inject constructor(
getSenderEmailAddresses(addressEmailAlias)
}
fun prepareMessageData(messageTitle: String, attachments: ArrayList<LocalAttachment>) {
_messageDataResult = composeMessageRepository.prepareMessageData(
_messageDataResult,
messageTitle,
attachments
)
fun prepareMessageData(
messageTitle: String,
attachments: ArrayList<LocalAttachment>
) {
fun Message.restoredPgpMimeAttachments(): List<LocalAttachment> =
LocalAttachment.createLocalAttachmentList(this.attachments).onEach {
it.messageId = EMPTY_STRING
it.attachmentId = EMPTY_STRING
}
if (_messageDataResult.isPGPMime) {
runBlocking {
parentMessageAsync().await()?.let { parentMessage ->
_messageDataResult = composeMessageRepository.prepareMessageData(
_messageDataResult,
messageTitle,
ArrayList(parentMessage.restoredPgpMimeAttachments())
)
}
}
} else {
_messageDataResult = composeMessageRepository.prepareMessageData(
_messageDataResult,
messageTitle,
attachments
)
}
}
@SuppressLint("CheckResult")
@ -692,7 +716,7 @@ class ComposeMessageViewModel @Inject constructor(
if (_draftId.get().isNotEmpty()) {
deleteMessage(
listOf(_draftId.get()),
Constants.MessageLocationType.DRAFT.messageLocationTypeValue.toString(),
MessageLocationType.DRAFT.messageLocationTypeValue.toString(),
userId
)
}
@ -1080,27 +1104,13 @@ class ComposeMessageViewModel @Inject constructor(
// Initial message content is the content that in the end is sent as the quoted text when replying/forwarding
// We need this to be the clean HTML message body, instead of the styled one that we show in the composer
viewModelScope.launch {
_parentId?.let { parentMessageId ->
val message = messageDetailsRepository.findMessageById(parentMessageId).first()
val decryptedMessageBody = message?.messageBody?.let { messageBody ->
try {
val crypto = forAddress(
userManager, userManager.requireCurrentUserId(), AddressId(message.addressID!!)
)
crypto.decrypt(CipherText(messageBody)).decryptedData
} catch (e: Exception) {
Timber.w(e, "Decryption error")
null
}
}
decryptedMessageBody?.let {
val builder = StringBuilder()
builder.append("<blockquote class=\"protonmail_quote\">")
builder.append(NEW_LINE)
builder.append(it)
builder.append("</div>")
setInitialMessageContent(builder.toString())
}
parentMessageAsync().await()?.decryptedBody?.let {
val builder = StringBuilder()
builder.append("<blockquote class=\"protonmail_quote\">")
builder.append(NEW_LINE)
builder.append(it)
builder.append("</div>")
setInitialMessageContent(builder.toString())
}
}
}
@ -1353,4 +1363,17 @@ class ComposeMessageViewModel @Inject constructor(
_messageDataResult.message.subject == context.getString(R.string.empty_subject) &&
_messageDataResult.content.isEmpty() &&
_messageDataResult.attachmentList.isEmpty()
private fun parentMessageAsync() = viewModelScope.async(dispatchers.Io) {
val parentId = _parentId
if (parentId != null) {
getDecryptedMessageById.orNull(parentId)
} else {
null
}
}
private fun List<LocalAttachment>.withoutPgpData() = map {
if (it.isPgpAttachment) it.apply { uri = Uri.EMPTY } else it
}
}

View File

@ -36,7 +36,8 @@ class LocalAttachment @JvmOverloads constructor(
val keyPackets: String? = null,
val headers: AttachmentHeaders? = null,
val isUploaded: Boolean = false,
var doSaveInDB: Boolean = true
var doSaveInDB: Boolean = true,
var isPgpAttachment: Boolean = false
) : Parcelable {
override fun describeContents() = 0
@ -54,6 +55,7 @@ class LocalAttachment @JvmOverloads constructor(
dest.writeString(headers?.toString() ?: "")
dest.writeInt(if (isUploaded) 1 else 0)
dest.writeInt(if (doSaveInDB) 1 else 0)
dest.writeInt(if (isPgpAttachment) 1 else 0)
}
companion object {
@ -80,7 +82,8 @@ class LocalAttachment @JvmOverloads constructor(
headers = attachment.headers,
keyPackets = attachment.keyPackets,
isUploading = attachment.isUploading,
isUploaded = attachment.isUploaded
isUploaded = attachment.isUploaded,
isPgpAttachment = attachment.isPGPAttachment
)
}
@ -105,6 +108,7 @@ class LocalAttachment @JvmOverloads constructor(
else AttachmentHeaders.fromString(serializedHeaders)
val isUploaded = parcel.readInt() == 1
val doSaveInDB = parcel.readInt() == 1
val isPgpAttachment = parcel.readInt() == 1
return LocalAttachment(
uri,
@ -118,7 +122,8 @@ class LocalAttachment @JvmOverloads constructor(
keyPackets,
headers,
isUploaded,
doSaveInDB
doSaveInDB,
isPgpAttachment
)
}

View File

@ -0,0 +1,44 @@
/*
* Copyright (c) 2022 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.usecase.message
import ch.protonmail.android.core.UserManager
import ch.protonmail.android.data.local.model.Message
import ch.protonmail.android.repository.MessageRepository
import kotlinx.coroutines.flow.firstOrNull
import me.proton.core.accountmanager.domain.AccountManager
import javax.inject.Inject
class GetDecryptedMessageById @Inject constructor(
val accountManager: AccountManager,
val userManager: UserManager,
val messageRepository: MessageRepository
) {
suspend fun orNull(messageId: String): Message? {
return accountManager.getPrimaryUserId()
.firstOrNull()
?.let { userId ->
messageRepository.getMessage(userId, messageId)?.apply {
decrypt(userManager, userId)
}
}
}
}

View File

@ -36,6 +36,7 @@ import ch.protonmail.android.usecase.compose.SaveDraft
import ch.protonmail.android.usecase.compose.SaveDraftResult
import ch.protonmail.android.usecase.delete.DeleteMessage
import ch.protonmail.android.usecase.fetch.FetchPublicKeys
import ch.protonmail.android.usecase.message.GetDecryptedMessageById
import ch.protonmail.android.utils.UiUtil
import ch.protonmail.android.utils.resources.StringResourceResolver
import ch.protonmail.android.utils.webview.SetUpWebViewDarkModeHandlingIfSupported
@ -98,6 +99,8 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
private val setUpWebViewDarkModeHandlingIfSupported: SetUpWebViewDarkModeHandlingIfSupported = mockk()
private val getDecryptedMessageById: GetDecryptedMessageById = mockk()
private val viewModel = ComposeMessageViewModel(
isAppInDarkMode = isAppInDarkMode,
composeMessageRepository = composeMessageRepository,
@ -114,7 +117,8 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
networkConfigurator = networkConfigurator,
htmlToSpanned = htmlToSpanned,
addExpirationTimeToMessage = addExpirationTimeToMessage,
setUpWebViewDarkModeHandlingIfSupported = setUpWebViewDarkModeHandlingIfSupported
setUpWebViewDarkModeHandlingIfSupported = setUpWebViewDarkModeHandlingIfSupported,
getDecryptedMessageById = getDecryptedMessageById
)
@BeforeTest

View File

@ -0,0 +1,90 @@
/*
* Copyright (c) 2022 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.usecase.message
import ch.protonmail.android.core.UserManager
import ch.protonmail.android.repository.MessageRepository
import ch.protonmail.android.testdata.MessageTestData
import ch.protonmail.android.testdata.UserIdTestData
import io.mockk.coEvery
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.runBlockingTest
import me.proton.core.accountmanager.domain.AccountManager
import org.junit.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull
internal class GetDecryptedMessageByIdTest {
private val accountManagerMock = mockk<AccountManager> {
coEvery { getPrimaryUserId() } returns flowOf(UserIdTestData.userId)
}
private val userManagerMock = mockk<UserManager>()
private val messageRepositoryMock = mockk<MessageRepository>()
private val getDecryptedMessageById = GetDecryptedMessageById(
accountManagerMock,
userManagerMock,
messageRepositoryMock
)
@Test
fun `should return null when primary user not found`() = runBlockingTest {
// given
coEvery { accountManagerMock.getPrimaryUserId() } returns flowOf(null)
// when
val decryptedMessage = getDecryptedMessageById.orNull(MessageTestData.MESSAGE_ID_RAW)
// then
assertNull(decryptedMessage)
}
@Test
fun `should return null when message not found`() = runBlockingTest {
// given
coEvery {
messageRepositoryMock.getMessage(UserIdTestData.userId, MessageTestData.MESSAGE_ID_RAW)
} returns null
// when
val decryptedMessage = getDecryptedMessageById.orNull(MessageTestData.MESSAGE_ID_RAW)
// then
assertNull(decryptedMessage)
}
@Test
fun `should return the decrypted message when message found`() = runBlockingTest {
// given
val messageSpy = MessageTestData.messageSpy()
coEvery {
messageRepositoryMock.getMessage(UserIdTestData.userId, MessageTestData.MESSAGE_ID_RAW)
} returns messageSpy
// when
val decryptedMessage = getDecryptedMessageById.orNull(MessageTestData.MESSAGE_ID_RAW)
// then
assertEquals(messageSpy, decryptedMessage)
verify { messageSpy.decrypt(userManagerMock, UserIdTestData.userId) }
}
}