Introduce MessageRenderer.setImagesAndProcess
This is a suspend function that returns the `RenderedMessage`, instead of `setImagesAndStartProcess`, which is a fire-and-forget, with the result returned on a separate stream. This will allow us to have more consistency in the class. MAILAND-2332
This commit is contained in:
parent
56d99f313e
commit
b23e133fbd
|
@ -38,9 +38,11 @@ import kotlinx.coroutines.channels.Channel
|
|||
import kotlinx.coroutines.channels.SendChannel
|
||||
import kotlinx.coroutines.channels.actor
|
||||
import kotlinx.coroutines.channels.toList
|
||||
import kotlinx.coroutines.coroutineScope
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.consumeAsFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.newSingleThreadContext
|
||||
import kotlinx.coroutines.plus
|
||||
import me.proton.core.util.kotlin.DispatcherProvider
|
||||
|
@ -87,6 +89,8 @@ internal class MessageRenderer(
|
|||
val results: Flow<RenderedMessage> get() =
|
||||
resultsChannel.consumeAsFlow()
|
||||
|
||||
private val renderedMessagesCache = mutableMapOf<String, RenderedMessage>()
|
||||
|
||||
private val messagesBodiesById = mutableMapOf<String, String>()
|
||||
// keep track of ids of the already inlined images across the threads
|
||||
private val inlinedImagesIdsByMessageId = mutableMapOf<String, MutableList<String>>()
|
||||
|
@ -243,6 +247,36 @@ internal class MessageRenderer(
|
|||
addToQueue(MessageEmbeddedImages(messageId, images))
|
||||
}
|
||||
|
||||
/**
|
||||
* Set [EmbeddedImage]s to be inlined in the message with the given [messageId]
|
||||
*
|
||||
* @throws IllegalStateException if no message body has been set for the message
|
||||
* @see setMessageBody
|
||||
*/
|
||||
suspend fun setImagesAndProcess(messageId: String, images: List<EmbeddedImage>): RenderedMessage {
|
||||
return coroutineScope {
|
||||
val fromCache = renderedMessagesCache.remove(messageId)
|
||||
|
||||
if (fromCache != null) {
|
||||
return@coroutineScope fromCache
|
||||
} else {
|
||||
val cacheJob = launch {
|
||||
setImagesAndStartProcess(messageId, images)
|
||||
for (result in resultsChannel) {
|
||||
renderedMessagesCache[result.messageId] = result
|
||||
}
|
||||
}
|
||||
var renderedMessage: RenderedMessage? = renderedMessagesCache.remove(messageId)
|
||||
while (renderedMessage == null) {
|
||||
renderedMessage = renderedMessagesCache.remove(messageId)
|
||||
delay(1)
|
||||
}
|
||||
cacheJob.cancel()
|
||||
return@coroutineScope renderedMessage
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun addToQueue(messageImages: MessageEmbeddedImages) {
|
||||
queues.getOrPut(messageImages.messageId) {
|
||||
actor {
|
||||
|
@ -330,14 +364,14 @@ private fun Document.findImageElements(id: String): Elements? {
|
|||
* Parses a document as [String] and returns a [Document] model
|
||||
*/
|
||||
internal interface DocumentParser {
|
||||
operator fun invoke(body: String): Document
|
||||
suspend operator fun invoke(body: String): Document
|
||||
}
|
||||
|
||||
/**
|
||||
* Default implementation of [DocumentParser]
|
||||
*/
|
||||
internal class DefaultDocumentParser @Inject constructor() : DocumentParser {
|
||||
override fun invoke(body: String): Document = Jsoup.parse(body).flatten()
|
||||
override suspend fun invoke(body: String): Document = Jsoup.parse(body).flatten()
|
||||
}
|
||||
// endregion
|
||||
|
||||
|
|
|
@ -24,6 +24,7 @@ import android.util.Base64
|
|||
import app.cash.turbine.test
|
||||
import ch.protonmail.android.details.presentation.model.RenderedMessage
|
||||
import ch.protonmail.android.jobs.helper.EmbeddedImage
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.mockkStatic
|
||||
|
@ -31,6 +32,8 @@ import io.mockk.unmockkStatic
|
|||
import io.mockk.verify
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.async
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.plus
|
||||
import me.proton.core.test.kotlin.CoroutinesTest
|
||||
import me.proton.core.util.kotlin.EMPTY_STRING
|
||||
|
@ -61,7 +64,7 @@ internal class MessageRendererTest : CoroutinesTest {
|
|||
}
|
||||
|
||||
private val mockDocumentParser: DocumentParser = mockk {
|
||||
every { this@mockk(any()) } returns buildMockDocument()
|
||||
coEvery { this@mockk(any()) } returns buildMockDocument()
|
||||
}
|
||||
|
||||
private fun CoroutineScope.buildRenderer() =
|
||||
|
@ -86,6 +89,38 @@ internal class MessageRendererTest : CoroutinesTest {
|
|||
unmockkStatic(Base64::class)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun emitsResultForASingleImagesSetSent() = coroutinesTest {
|
||||
// given
|
||||
val messageRenderer = buildRenderer()
|
||||
val imageSet = buildEmbeddedImages(idsRange = 1..3)
|
||||
createFilesFor(imageSet)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID, EMPTY_STRING)
|
||||
|
||||
// when
|
||||
messageRenderer.results.test {
|
||||
|
||||
// then
|
||||
messageRenderer.setImagesAndStartProcess(TEST_MESSAGE_ID, imageSet)
|
||||
expectItem()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun returnsResultForASingleImagesSetSent() = coroutinesTest {
|
||||
// given
|
||||
val messageRenderer = buildRenderer()
|
||||
val imageSet = buildEmbeddedImages(idsRange = 1..3)
|
||||
createFilesFor(imageSet)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID, EMPTY_STRING)
|
||||
|
||||
// when
|
||||
val result = messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID, imageSet)
|
||||
|
||||
// then
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID, TEST_DOCUMENT_CONTENT), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun doesNotEmitResultForImagesSentWithTooShortDelayForTheSameMessage() = coroutinesTest {
|
||||
// given
|
||||
|
@ -128,6 +163,91 @@ internal class MessageRendererTest : CoroutinesTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun returnsResultForEachMessageSequentially() = coroutinesTest {
|
||||
// given
|
||||
val messageRenderer = buildRenderer()
|
||||
val imageSet1 = buildEmbeddedImages(idsRange = 1..3)
|
||||
val imageSet2 = buildEmbeddedImages(idsRange = 4..7)
|
||||
createFilesFor(imageSet1, imageSet2)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_1, EMPTY_STRING)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_2, EMPTY_STRING)
|
||||
|
||||
// when
|
||||
val result1 = messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_1, imageSet1)
|
||||
val result2 = messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_2, imageSet2)
|
||||
|
||||
// then
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_1, TEST_DOCUMENT_CONTENT), result1)
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_2, TEST_DOCUMENT_CONTENT), result2)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun returnsResultForEachMessageInParallel() = coroutinesTest {
|
||||
// given
|
||||
val messageRenderer = buildRenderer()
|
||||
val imageSet1 = buildEmbeddedImages(idsRange = 1..3)
|
||||
val imageSet2 = buildEmbeddedImages(idsRange = 4..7)
|
||||
createFilesFor(imageSet1, imageSet2)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_1, EMPTY_STRING)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_2, EMPTY_STRING)
|
||||
|
||||
// when
|
||||
val result1Deferred = async { messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_1, imageSet1) }
|
||||
val result2Deferred = async { messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_2, imageSet2) }
|
||||
|
||||
// then
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_1, TEST_DOCUMENT_CONTENT), result1Deferred.await())
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_2, TEST_DOCUMENT_CONTENT), result2Deferred.await())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun returnsResultForEachMessageInParallelWithDifferentExecutionTimes() = coroutinesTest {
|
||||
// given
|
||||
val messageRenderer = buildRenderer()
|
||||
val imageSet1 = buildEmbeddedImages(idsRange = 1..3)
|
||||
val imageSet2 = buildEmbeddedImages(idsRange = 4..7)
|
||||
createFilesFor(imageSet1, imageSet2)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_1, TEST_MESSAGE_BODY_1)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_2, TEST_MESSAGE_BODY_2)
|
||||
|
||||
coEvery { mockDocumentParser(TEST_MESSAGE_BODY_1) } coAnswers {
|
||||
delay(500)
|
||||
buildMockDocument()
|
||||
}
|
||||
coEvery { mockDocumentParser(TEST_MESSAGE_BODY_2) } coAnswers {
|
||||
delay(1)
|
||||
buildMockDocument()
|
||||
}
|
||||
|
||||
// when
|
||||
val result1Deferred = async { messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_1, imageSet1) }
|
||||
val result2Deferred = async { messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_2, imageSet2) }
|
||||
|
||||
// then
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_1, TEST_DOCUMENT_CONTENT), result1Deferred.await())
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_2, TEST_DOCUMENT_CONTENT), result2Deferred.await())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun returnsResultForEachMessageWithReversedOrder() = coroutinesTest {
|
||||
// given
|
||||
val messageRenderer = buildRenderer()
|
||||
val imageSet1 = buildEmbeddedImages(idsRange = 1..3)
|
||||
val imageSet2 = buildEmbeddedImages(idsRange = 4..7)
|
||||
createFilesFor(imageSet1, imageSet2)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_1, EMPTY_STRING)
|
||||
messageRenderer.setMessageBody(TEST_MESSAGE_ID_2, EMPTY_STRING)
|
||||
|
||||
// when
|
||||
val result2 = messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_2, imageSet2)
|
||||
val result1 = messageRenderer.setImagesAndProcess(TEST_MESSAGE_ID_1, imageSet1)
|
||||
|
||||
// then
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_1, TEST_DOCUMENT_CONTENT), result1)
|
||||
assertEquals(RenderedMessage(TEST_MESSAGE_ID_2, TEST_DOCUMENT_CONTENT), result2)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun emitsResultForEveryImageSentWithRightDelay() = coroutinesTest {
|
||||
// given
|
||||
|
@ -264,9 +384,9 @@ internal class MessageRendererTest : CoroutinesTest {
|
|||
val firstMessageBodyWithInlinedImages = "$TEST_MESSAGE_BODY_1 with inlined images"
|
||||
val secondMessageBodyWithInlinedImages = "$TEST_MESSAGE_BODY_2 with inlined images"
|
||||
|
||||
every { mockDocumentParser(TEST_MESSAGE_BODY_1) } returns
|
||||
coEvery { mockDocumentParser(TEST_MESSAGE_BODY_1) } returns
|
||||
buildMockDocument(content = firstMessageBodyWithInlinedImages)
|
||||
every { mockDocumentParser(TEST_MESSAGE_BODY_2) } returns
|
||||
coEvery { mockDocumentParser(TEST_MESSAGE_BODY_2) } returns
|
||||
buildMockDocument(content = secondMessageBodyWithInlinedImages)
|
||||
|
||||
val expectedFirstRenderedMessage = RenderedMessage(TEST_MESSAGE_ID_1, firstMessageBodyWithInlinedImages)
|
||||
|
@ -354,12 +474,13 @@ internal class MessageRendererTest : CoroutinesTest {
|
|||
|
||||
private fun <K, V> Collection<Map<K, List<V>>>.mergeMaps(): Map<K, List<V>> {
|
||||
val keys = flatMap { map -> map.keys }
|
||||
return keys.map { key ->
|
||||
// Take values of each maps, for the given key
|
||||
return keys.associateWith { key ->
|
||||
// Take values of each maps, for the given key
|
||||
key to flatMap { map ->
|
||||
flatMap { map ->
|
||||
map[key] ?: emptyList()
|
||||
}
|
||||
}.toMap()
|
||||
}
|
||||
}
|
||||
|
||||
private fun setBase64EncodeToStringToIncrementalResult() {
|
||||
|
@ -368,6 +489,6 @@ internal class MessageRendererTest : CoroutinesTest {
|
|||
}
|
||||
|
||||
private fun setMockDocumentParserToReplaceStringsInMessage(message: String) {
|
||||
every { mockDocumentParser(any()) } returns buildMockDocumentWithReplaceFeature(message)
|
||||
coEvery { mockDocumentParser(any()) } returns buildMockDocumentWithReplaceFeature(message)
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue