Replace usages of MessageRenderer.kt with new API

MAILAND-2332
This commit is contained in:
Davide Farella 2021-09-24 10:29:35 +02:00 committed by Davide Giuseppe Farella
parent bb1d8e665f
commit 34d3058406
4 changed files with 62 additions and 42 deletions

View File

@ -85,7 +85,7 @@ internal class MessageRenderer(
"Use 'setMessageBody' with relative messageId. This will be removed",
ReplaceWith("setMessageBody(messageId, messageBody!!)")
)
var messageBody: String? = null
private var messageBody: String? = null
set(value) {
// Return if body is already set
if (field != null) return
@ -101,13 +101,15 @@ internal class MessageRenderer(
/** reference to the [Document] */
private val document by lazy { documentParser(messageBody!!) }
private val messagesBodiesById = mutableMapOf<String, String>()
// region Actors
/** A [Channel] for receive new [EmbeddedImage] images to inline in [document] */
@Deprecated(
"Use 'setImagesAndStartProcess' with relative messageId. This will be private",
ReplaceWith("setImagesAndStartProcess(messageId, embeddedImages)")
)
val images = actor<List<EmbeddedImage>> {
private val images = actor<List<EmbeddedImage>> {
for (embeddedImages in channel) {
imageCompressor.send(embeddedImages)
// Workaround that ignore values for the next half second, since ViewModel is emitting
@ -121,7 +123,7 @@ internal class MessageRenderer(
"Use 'results'. This will be private",
ReplaceWith("results")
)
val renderedMessage = Channel<RenderedMessage>()
private val renderedMessage = Channel<RenderedMessage>()
/** [List] for keep track of ids of the already inlined images across the threads */
private val inlinedImageIds = mutableListOf<String>()
@ -253,18 +255,20 @@ internal class MessageRenderer(
* @param messageBody [String] representation of the HTML message's body
*/
fun setMessageBody(messageId: String, messageBody: String) {
messagesBodiesById[messageId] = messageBody
this.messageBody = messageBody
}
/**
* Set [EmbeddedImage]s to be inlined in the message with the given [messageId] and start the inlining process
* Result will be delivered through [renderedMessage]
* Result will be delivered through [results]
*
* @throws IllegalStateException if no message body has been set for the message
* @see setMessageBody
*/
fun setImagesAndStartProcess(messageId: String, images: List<EmbeddedImage>) {
checkNotNull(messagesBodiesById[messageId]) { "No message body set for id: $messageId" }
this.images.trySend(images)
}
/** @return [File] directory for the current message */

View File

@ -55,8 +55,8 @@ import ch.protonmail.android.data.local.model.Message
import ch.protonmail.android.details.data.toConversationUiModel
import ch.protonmail.android.details.presentation.MessageDetailsActivity
import ch.protonmail.android.details.presentation.model.ConversationUiModel
import ch.protonmail.android.domain.entity.LabelId
import ch.protonmail.android.details.presentation.model.MessageBodyState
import ch.protonmail.android.domain.entity.LabelId
import ch.protonmail.android.domain.entity.Name
import ch.protonmail.android.events.DownloadEmbeddedImagesEvent
import ch.protonmail.android.events.Status
@ -224,17 +224,15 @@ internal class MessageDetailsViewModel @Inject constructor(
}
.launchIn(viewModelScope)
viewModelScope.launch {
for (renderedMessage in messageRenderer.renderedMessage) {
val updatedMessage = updateUiModelMessageWithFormattedHtml(
renderedMessage.messageId,
renderedMessage.renderedHtmlBody
)
Timber.v("Update rendered HTML message id: ${updatedMessage?.messageId}")
_messageRenderedWithImages.value = updatedMessage
areImagesDisplayed = true
}
}
messageRenderer.results.onEach { renderedMessage ->
val updatedMessage = updateUiModelMessageWithFormattedHtml(
renderedMessage.messageId,
renderedMessage.renderedHtmlBody
)
Timber.v("Update rendered HTML message id: ${updatedMessage?.messageId}")
_messageRenderedWithImages.value = updatedMessage
areImagesDisplayed = true
}.launchIn(viewModelScope)
}
private fun getMessageFlow(userId: UserId): Flow<ConversationUiModel?> =
@ -489,7 +487,8 @@ internal class MessageDetailsViewModel @Inject constructor(
fun onEmbeddedImagesDownloaded(event: DownloadEmbeddedImagesEvent) {
Timber.v("onEmbeddedImagesDownloaded status: ${event.status} images size: ${event.images.size}")
messageRenderer.images.offer(event.images)
val messageId = event.images.first().messageId
messageRenderer.setImagesAndStartProcess(messageId, event.images)
}
@ -675,6 +674,7 @@ internal class MessageDetailsViewModel @Inject constructor(
css: String,
defaultErrorMessage: String
): String {
val messageId = requireNotNull(message.messageId) { "message id is null" }
val formattedHtml = try {
val contentTransformer = DefaultTransformer()
.pipe(ViewportTransformer(windowWidth, css))
@ -687,7 +687,7 @@ internal class MessageDetailsViewModel @Inject constructor(
updateUiModelMessageWithFormattedHtml(message.messageId, formattedHtml, message.decryptedBody)
// Set the body of the message currently being displayed in messageRenderer to allow embedded images loading
messageRenderer.messageBody = formattedHtml
messageRenderer.setMessageBody(messageId, formattedHtml)
return formattedHtml
}

View File

@ -31,7 +31,7 @@ import io.mockk.unmockkStatic
import io.mockk.verify
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.consumeAsFlow
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.plus
import me.proton.core.test.kotlin.CoroutinesTest
import me.proton.core.util.kotlin.EMPTY_STRING
@ -46,6 +46,9 @@ import kotlin.test.assertEquals
internal class MessageRendererTest : CoroutinesTest {
private val testMessageId = "message id"
private val testMessageBody = "Message body"
@get:Rule
val folder: TemporaryFolder = TemporaryFolder()
.also { it.create() }
@ -65,7 +68,9 @@ internal class MessageRendererTest : CoroutinesTest {
bitmapImageDecoder = mockImageDecoder,
attachmentsDirectory = folder.root,
scope = this + Job()
).apply { messageBody = "" }
).apply {
setMessageBody(testMessageId, testMessageBody)
}
@BeforeTest
fun setUp() {
@ -87,13 +92,13 @@ internal class MessageRendererTest : CoroutinesTest {
createFilesFor(imageSet1, imageSet2)
// when
messageRenderer.renderedMessage.consumeAsFlow().test {
messageRenderer.results.test {
// then
messageRenderer.images.trySend(imageSet1)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet1)
expectItem()
messageRenderer.images.trySend(imageSet2)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet2)
expectNoEvents()
}
}
@ -107,15 +112,15 @@ internal class MessageRendererTest : CoroutinesTest {
createFilesFor(imageSet1, imageSet2)
// when
messageRenderer.renderedMessage.consumeAsFlow().test {
messageRenderer.results.test {
// then
messageRenderer.images.trySend(imageSet1)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet1)
expectItem()
advanceUntilIdle()
messageRenderer.images.trySend(imageSet2)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet2)
expectItem()
}
}
@ -146,9 +151,9 @@ internal class MessageRendererTest : CoroutinesTest {
val expected = RenderedMessage(messageId, expectedMessageBody)
// when
messageRenderer.messageBody = messageBody
messageRenderer.renderedMessage.consumeAsFlow().test {
messageRenderer.images.send(imageSet)
messageRenderer.setMessageBody(testMessageId, messageBody)
messageRenderer.results.test {
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet)
// then
assertEquals(expected, expectItem())
@ -166,7 +171,7 @@ internal class MessageRendererTest : CoroutinesTest {
every { mockImageDecoder(any(), any()) } returns mockBitmap
// when
messageRenderer.images.send(imageSet)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet)
// then
verify(exactly = imageSet.size) { mockBitmap.compress(any(), any(), any()) }
@ -184,14 +189,26 @@ internal class MessageRendererTest : CoroutinesTest {
every { mockImageDecoder(any(), any()) } returns mockBitmap
// when
messageRenderer.images.send(imageSet1)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet1)
advanceUntilIdle()
messageRenderer.images.send(imageSet2)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet2)
// then
verify(exactly = imageSet2.size) { mockBitmap.compress(any(), any(), any()) }
}
@Test(expected = IllegalStateException::class)
fun setImagesAndStartProcessThrowsExceptionIfNoMessageBodySetForGivenMessageId() = coroutinesTest {
// given
val messageRenderer = buildRenderer()
val messageId1 = "message 1"
val messageId2 = "message 2"
// when
messageRenderer.setMessageBody(messageId1, EMPTY_STRING)
messageRenderer.setImagesAndStartProcess(messageId2, emptyList())
}
@Test
@Ignore("To be refactored")
fun messageRendererRendersImagesForDifferentMessagesByChangingTheMessageBody() = coroutinesTest {
@ -202,19 +219,19 @@ internal class MessageRendererTest : CoroutinesTest {
val secondMessageBody = "second message body"
val imageSet = buildEmbeddedImages(idsRange = 1..10)
createFilesFor(imageSet)
messageRenderer.messageBody = firstMessageBody
messageRenderer.setMessageBody(testMessageId, firstMessageBody)
every { Base64.encodeToString(any(), any()) } returns "base64EncodedImageData"
every { mockDocumentParser.invoke(firstMessageBody) } returns mockk(relaxed = true) {
every { this@mockk.toString() } returns "First message body with inlined images"
}
// When
messageRenderer.images.trySend(imageSet)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet)
advanceTimeBy(500)
// Then
val expected = RenderedMessage("messageId-1", "First message body with inlined images")
val actual = messageRenderer.renderedMessage.tryReceive().getOrNull()
val actual = messageRenderer.results.firstOrNull()
assertEquals(expected, actual)
// Render images for a second message
@ -222,14 +239,14 @@ internal class MessageRendererTest : CoroutinesTest {
every { mockDocumentParser.invoke(secondMessageBody) } returns mockk(relaxed = true) {
every { this@mockk.toString() } returns "Second message body with inlined images"
}
messageRenderer.messageBody = secondMessageBody
messageRenderer.setMessageBody(testMessageId, secondMessageBody)
// When
messageRenderer.images.trySend(imageSet)
messageRenderer.setImagesAndStartProcess(testMessageId, imageSet)
// Then
val secondMessageExpected = RenderedMessage("messageId-1", "Second message body with inlined images")
val secondMessageActual = messageRenderer.renderedMessage.tryReceive().getOrNull()
val secondMessageActual = messageRenderer.results.firstOrNull()
assertEquals(secondMessageExpected, secondMessageActual)
}

View File

@ -72,7 +72,6 @@ import io.mockk.runs
import io.mockk.spyk
import io.mockk.verify
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.first
@ -172,7 +171,7 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
private var messageRendererFactory = mockk<MessageRenderer.Factory> {
every { create(any()) } returns mockk(relaxed = true) {
every { renderedMessage } returns Channel()
every { results } returns flowOf()
}
}