diff --git a/app/src/androidTest/java/com/android/messaging/ui/photoviewer/screen/PhotoViewerScreenContentTest.kt b/app/src/androidTest/java/com/android/messaging/ui/photoviewer/screen/PhotoViewerScreenContentTest.kt index 538ae856f..7ad7b34ca 100644 --- a/app/src/androidTest/java/com/android/messaging/ui/photoviewer/screen/PhotoViewerScreenContentTest.kt +++ b/app/src/androidTest/java/com/android/messaging/ui/photoviewer/screen/PhotoViewerScreenContentTest.kt @@ -619,6 +619,7 @@ internal class PhotoViewerScreenContentTest { isDraft: Boolean = false, ): PhotoViewerItem { return PhotoViewerItem( + partId = "part-$index", contentUri = photoViewerImageUri(), contentType = IMAGE_JPEG, isIncoming = isIncoming, diff --git a/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryDirectLookupTest.kt b/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryDirectLookupTest.kt index 25437fc67..615143b9a 100644 --- a/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryDirectLookupTest.kt +++ b/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryDirectLookupTest.kt @@ -23,7 +23,6 @@ import io.mockk.every import io.mockk.mockk import io.mockk.slot import io.mockk.verify -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -32,7 +31,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -@OptIn(ExperimentalCoroutinesApi::class) @RunWith(RobolectricTestRunner::class) internal class ConversationsRepositoryDirectLookupTest : BaseConversationsRepositoryTest() { @@ -59,7 +57,7 @@ internal class ConversationsRepositoryDirectLookupTest : BaseConversationsReposi context = mainDispatcherRule.testDispatcher, ) { val metadataUri = MessagingContentProvider.buildConversationMetadataUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, ) val participantsUri = MessagingContentProvider .buildConversationParticipantsUri(CONVERSATION_ID.value) @@ -137,7 +135,7 @@ internal class ConversationsRepositoryDirectLookupTest : BaseConversationsReposi context = mainDispatcherRule.testDispatcher, ) { val metadataUri = MessagingContentProvider.buildConversationMetadataUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, ) val participantsUri = MessagingContentProvider .buildConversationParticipantsUri(CONVERSATION_ID.value) @@ -196,8 +194,9 @@ internal class ConversationsRepositoryDirectLookupTest : BaseConversationsReposi runTest( context = mainDispatcherRule.testDispatcher, ) { - val messagesUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + val messagesUri = MessagingContentProvider.buildConversationMessageUri( + CONVERSATION_ID.value, + "message-1", ) val participantsUri = MessagingContentProvider .buildConversationParticipantsUri(CONVERSATION_ID.value) diff --git a/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryMessagesTest.kt b/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryMessagesTest.kt index c808f3cea..405792faf 100644 --- a/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryMessagesTest.kt +++ b/app/src/test/kotlin/com/android/messaging/data/conversation/repository/conversations/ConversationsRepositoryMessagesTest.kt @@ -16,6 +16,11 @@ import io.mockk.mockk import io.mockk.runs import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue @@ -30,13 +35,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_registersAndUnregistersObserverForCollection() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) stubObserverRegistration( @@ -44,13 +53,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = emptyList()), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - assertTrue(awaitItem().isEmpty()) + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + assertTrue(awaitItem().messages.isEmpty()) cancelAndIgnoreRemainingEvents() } @@ -74,13 +86,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_emitsMessagesInUiOrderWithLegacyClusteringRules() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val messagesInUiOrder = listOf( messageRow( @@ -146,13 +162,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = messagesInUiOrder.asReversed()), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - val messages = awaitItem() + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + val messages = awaitItem().messages assertEquals( messagesInUiOrder.map { it.messageId }, @@ -202,7 +221,7 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository cancelAndIgnoreRemainingEvents() } - verify(exactly = 1) { contentResolver.query(expectedUri, any(), null, null, null) } + verify(exactly = 1) { contentResolver.query(expectedQueryUri, any(), null, null, null) } assertEquals( ConversationMessageData.getProjection().toList(), capturedProjections.single()?.toList(), @@ -213,13 +232,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_requeriesWhenObserverChanges() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val firstMessage = messageRow( messageId = "first", @@ -244,7 +267,7 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository ) every { contentResolver.query( - expectedUri, + expectedQueryUri, any(), null, null, @@ -261,14 +284,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository } } - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - assertEquals(listOf("first"), awaitItem().map { it.messageId }) + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + assertEquals(listOf("first"), awaitItem().messages.map { it.messageId }) registeredObservers.single().onChange(false) assertEquals( listOf("first", "second"), - awaitItem().map { it.messageId }, + awaitItem().messages.map { it.messageId }, ) cancelAndIgnoreRemainingEvents() @@ -276,7 +302,7 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository verify(exactly = 2) { contentResolver.query( - expectedUri, + expectedQueryUri, any(), null, null, @@ -296,13 +322,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_singleMessageHasNoClustering() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val singleMessage = messageRow( messageId = "only", @@ -318,13 +348,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = listOf(singleMessage)), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - val messages = awaitItem() + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + val messages = awaitItem().messages assertEquals(1, messages.size) assertEquals("only", messages[0].messageId) @@ -342,13 +375,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_clustersAtExactlyOneMinuteButNotOneMillisOver() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val messagesInUiOrder = listOf( messageRow( @@ -382,13 +419,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = messagesInUiOrder.asReversed()), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - val messages = awaitItem() + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + val messages = awaitItem().messages assertEquals(3, messages.size) @@ -416,13 +456,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_doesNotClusterFailedMessageWithDeliveredNeighbours() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val messagesInUiOrder = listOf( messageRow( @@ -464,13 +508,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = messagesInUiOrder.asReversed()), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - val messages = awaitItem() + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + val messages = awaitItem().messages assertEquals( messagesInUiOrder.map { it.messageId }, @@ -509,13 +556,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_doesNotClusterConsecutiveFailedMessages() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val messagesInUiOrder = listOf( messageRow( @@ -549,13 +600,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = messagesInUiOrder.asReversed()), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - val messages = awaitItem() + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + val messages = awaitItem().messages assertEquals(3, messages.size) @@ -577,13 +631,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_stillClustersOutgoingMessagesWithDifferentSuccessStatuses() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) val messagesInUiOrder = listOf( messageRow( @@ -617,13 +675,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = createConversationMessagesCursor(rows = messagesInUiOrder.asReversed()), ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - val messages = awaitItem() + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + val messages = awaitItem().messages assertEquals(3, messages.size) @@ -655,13 +716,17 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository @Test fun getConversationMessages_returnsEmptyListWhenQueryReturnsNull() { runTest( - context = mainDispatcherRule.testDispatcher + context = mainDispatcherRule.testDispatcher, ) { val registeredObservers = mutableListOf() val capturedProjections = mutableListOf?>() val repository = createRepository() val expectedUri = MessagingContentProvider.buildConversationMessagesUri( - CONVERSATION_ID.value + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, ) stubObserverRegistration( @@ -669,13 +734,16 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository expectedUri = expectedUri, ) stubQuery( - expectedUri = expectedUri, + expectedUri = expectedQueryUri, capturedProjections = capturedProjections, result = null, ) - repository.getConversationMessages(conversationId = CONVERSATION_ID).test { - assertTrue(awaitItem().isEmpty()) + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).test { + assertTrue(awaitItem().messages.isEmpty()) cancelAndIgnoreRemainingEvents() } @@ -686,7 +754,240 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository } } - private fun createConversationMessagesCursor(rows: List): Cursor { + @Test + fun getConversationMessages_reportsMoreWhenTheWindowIsFull() { + runTest( + context = mainDispatcherRule.testDispatcher, + ) { + val registeredObservers = mutableListOf() + val capturedProjections = mutableListOf?>() + val repository = createRepository() + val expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + WINDOW_SIZE_OF_TWO, + ) + + stubObserverRegistration( + registeredObservers = registeredObservers, + expectedUri = expectedUri, + ) + stubQuery( + expectedUri = expectedQueryUri, + capturedProjections = capturedProjections, + result = createConversationMessagesCursor( + rows = listOf( + messageRow( + messageId = "newest", + participantId = "participant-a", + selfParticipantId = "self-1", + receivedTimestamp = 2_000L, + status = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + text = "Newest", + ), + messageRow( + messageId = "older", + participantId = "participant-a", + selfParticipantId = "self-1", + receivedTimestamp = 1_000L, + status = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + text = "Older", + ), + ), + ), + ) + + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(WINDOW_SIZE_OF_TWO), + ).test { + assertTrue(awaitItem().hasMore) + cancelAndIgnoreRemainingEvents() + } + } + } + + @Test + fun getConversationMessages_reportsNoMoreWhenTheWindowIsNotFull() { + runTest( + context = mainDispatcherRule.testDispatcher, + ) { + val registeredObservers = mutableListOf() + val capturedProjections = mutableListOf?>() + val repository = createRepository() + val expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + ) + val expectedQueryUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + WINDOW_SIZE_OF_TWO, + ) + + stubObserverRegistration( + registeredObservers = registeredObservers, + expectedUri = expectedUri, + ) + stubQuery( + expectedUri = expectedQueryUri, + capturedProjections = capturedProjections, + result = createConversationMessagesCursor( + rows = listOf( + messageRow( + messageId = "only", + participantId = "participant-a", + selfParticipantId = "self-1", + receivedTimestamp = 1_000L, + status = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + text = "Only", + ), + ), + ), + ) + + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(WINDOW_SIZE_OF_TWO), + ).test { + assertEquals(false, awaitItem().hasMore) + cancelAndIgnoreRemainingEvents() + } + } + } + + @Test + fun getConversationMessages_requeriesWithTheLargerWindowWhenItGrows() { + runTest( + context = mainDispatcherRule.testDispatcher, + ) { + val registeredObservers = mutableListOf() + val capturedProjections = mutableListOf?>() + val repository = createRepository() + val expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + ) + val newestMessage = messageRow( + messageId = "newest", + participantId = "participant-a", + selfParticipantId = "self-1", + receivedTimestamp = 2_000L, + status = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + text = "Newest", + ) + val olderMessage = messageRow( + messageId = "older", + participantId = "participant-a", + selfParticipantId = "self-1", + receivedTimestamp = 1_000L, + status = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + text = "Older", + ) + val windowSizes = MutableStateFlow(value = 1) + + stubObserverRegistration( + registeredObservers = registeredObservers, + expectedUri = expectedUri, + ) + stubQuery( + expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + 1, + ), + capturedProjections = capturedProjections, + result = createConversationMessagesCursor(rows = listOf(newestMessage)), + ) + stubQuery( + expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + WINDOW_SIZE_OF_TWO, + ), + capturedProjections = capturedProjections, + result = createConversationMessagesCursor( + rows = listOf(newestMessage, olderMessage), + ), + ) + + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = windowSizes, + ).test { + assertEquals(listOf("newest"), awaitItem().messages.map { it.messageId }) + + windowSizes.value = WINDOW_SIZE_OF_TWO + + // The query is newest first and the repository walks the cursor backwards, so the + // window reaches the ui oldest first. + assertEquals( + listOf("older", "newest"), + awaitItem().messages.map { it.messageId }, + ) + cancelAndIgnoreRemainingEvents() + } + } + } + + @Test + fun getConversationMessages_whenTheCollectorLeavesMidWalk_stopsWalkingTheCursor() { + runTest( + context = mainDispatcherRule.testDispatcher, + ) { + val repository = createRepository() + var cursorMoves = 0 + var collection: Job? = null + val cursor = createConversationMessagesCursor( + rows = List(ROW_COUNT) { index -> + messageRow( + messageId = "message-$index", + participantId = "participant-1", + selfParticipantId = "self-1", + receivedTimestamp = index.toLong(), + status = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + text = "message $index", + ) + }, + onCursorMove = { + cursorMoves += 1 + if (cursorMoves == CANCEL_AFTER_CURSOR_MOVES) { + collection?.cancel() + } + }, + ) + + stubObserverRegistration( + registeredObservers = mutableListOf(), + expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + ), + ) + stubQuery( + expectedUri = MessagingContentProvider.buildConversationMessagesUri( + CONVERSATION_ID.value, + TEST_WINDOW_SIZE, + ), + capturedProjections = mutableListOf(), + result = cursor, + ) + + collection = backgroundScope.launch { + repository.getConversationMessages( + conversationId = CONVERSATION_ID, + windowSizes = flowOf(TEST_WINDOW_SIZE), + ).collect { } + } + runCurrent() + + assertTrue( + "kept walking after the screen left: $cursorMoves moves for $ROW_COUNT rows", + cursorMoves < ROW_COUNT, + ) + } + } + + private fun createConversationMessagesCursor( + rows: List, + onCursorMove: () -> Unit = {}, + ): Cursor { val projection = ConversationMessageData.getProjection() val rowsByColumn = rows.map { it.toColumnValues() } var position = -1 @@ -718,6 +1019,7 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository else -> { position = positionToMove + onCursorMove() true } } @@ -871,5 +1173,9 @@ internal class ConversationsRepositoryMessagesTest : BaseConversationsRepository private companion object { private const val TEXT_CONTENT_TYPE = "text/plain" + private const val TEST_WINDOW_SIZE = 500 + private const val WINDOW_SIZE_OF_TWO = 2 + private const val ROW_COUNT = 10 + private const val CANCEL_AFTER_CURSOR_MOVES = 2 } } diff --git a/app/src/test/kotlin/com/android/messaging/data/media/repository/PhotoViewerRepositoryImplTest.kt b/app/src/test/kotlin/com/android/messaging/data/media/repository/PhotoViewerRepositoryImplTest.kt index 613bb0c95..7653f2e74 100644 --- a/app/src/test/kotlin/com/android/messaging/data/media/repository/PhotoViewerRepositoryImplTest.kt +++ b/app/src/test/kotlin/com/android/messaging/data/media/repository/PhotoViewerRepositoryImplTest.kt @@ -79,6 +79,7 @@ internal class PhotoViewerRepositoryImplTest { assertEquals(true, result.items[0].isIncoming) assertEquals( PhotoViewerItem( + partId = "part-2", contentUri = Uri.parse("content://example/content/2?updated=true"), contentType = IMAGE_JPEG, isIncoming = false, @@ -93,7 +94,7 @@ internal class PhotoViewerRepositoryImplTest { } @Test - fun getPhotoViewerItems_whenInitialUriHasDuplicates_usesOccurrenceIndex() { + fun getPhotoViewerItems_whenInitialUriHasDuplicates_usesPartId() { runTest(context = testDispatcher) { val duplicateContentUri = "content://example/content/shared" val cursor = MatrixCursor(ConversationImagePartsView.PhotoViewQuery.PROJECTION).apply { @@ -130,7 +131,7 @@ internal class PhotoViewerRepositoryImplTest { val result = repository.getPhotoViewerItems( photosUri = photosUri, initialPhotoUri = Uri.parse(duplicateContentUri), - initialPhotoOccurrenceIndex = 2, + initialPartId = "part-3", ).firstLoadedForTest() assertEquals(2, result.initialIndex) @@ -387,6 +388,7 @@ internal class PhotoViewerRepositoryImplTest { senderDestination: String, receivedTimestampMillis: Long, status: Int, + partId: String? = "part-${count + 1}", ) { addRow( arrayOf( @@ -398,6 +400,7 @@ internal class PhotoViewerRepositoryImplTest { senderDestination, receivedTimestampMillis, status, + partId, ), ) } diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/ConversationMessagesWindowedQueryTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/ConversationMessagesWindowedQueryTest.kt new file mode 100644 index 000000000..28c06bf8b --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/datamodel/ConversationMessagesWindowedQueryTest.kt @@ -0,0 +1,362 @@ +package com.android.messaging.datamodel + +import android.content.pm.ProviderInfo +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase +import android.net.Uri +import androidx.core.content.contentValuesOf +import com.android.messaging.FactoryTestAccess +import com.android.messaging.datamodel.DatabaseHelper.ConversationColumns +import com.android.messaging.datamodel.DatabaseHelper.MessageColumns +import com.android.messaging.datamodel.DatabaseHelper.PartColumns +import com.android.messaging.datamodel.DatabaseHelper.ParticipantColumns +import com.android.messaging.datamodel.data.MessageData +import com.android.messaging.testutil.installTestFactory +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.shadows.ShadowContentResolver +import org.robolectric.util.ReflectionHelpers + +@RunWith(RobolectricTestRunner::class) +class ConversationMessagesWindowedQueryTest { + + private val context get() = RuntimeEnvironment.getApplication().applicationContext + + @Before + fun setUp() { + installTestFactory(context = context) + } + + @After + fun tearDown() { + unmockkAll() + FactoryTestAccess.reset() + } + + @Test + fun windowedQuery_returnsTheNewestRowsOfTheUnboundedQuery() { + withPopulatedDatabase { conversationId, _ -> + val unbounded = unboundedMessageTexts(conversationId) + + assertEquals(MESSAGE_COUNT, unbounded.size) + for (limit in listOf(1, 2, 10, MESSAGE_COUNT - 1)) { + assertEquals( + "window of $limit is not the head of the unbounded query", + unbounded.take(limit), + windowedMessageTexts(limit, conversationId), + ) + } + } + } + + @Test + fun windowedQuery_withAWindowLargerThanTheConversation_returnsEverything() { + withPopulatedDatabase { conversationId, _ -> + val unbounded = unboundedMessageTexts(conversationId) + + assertEquals(unbounded, windowedMessageTexts(MESSAGE_COUNT, conversationId)) + assertEquals(unbounded, windowedMessageTexts(MESSAGE_COUNT * 10, conversationId)) + } + } + + /** + * The draft and the other conversation's messages are newer than everything else, so a window + * that let either through would put them at the top of the conversation. + */ + @Test + fun windowedQuery_excludesDraftsAndOtherConversations() { + withPopulatedDatabase { conversationId, _ -> + assertEquals( + listOf("message 24", "message 23"), + windowedMessageTexts(limit = 2, conversationId = conversationId), + ) + } + } + + /** + * A message can have no parts at all, since an outgoing mms carrying only a subject never gets + * one, and the left join gives every one of those the same null part id. Grouped by that id + * they collapse into a single row, so the window comes back shorter than it is, the caller + * reads that as the whole conversation and stops asking for more, and the messages that were + * folded away are not in the window either. + */ + @Test + fun windowedQuery_withMessagesWithoutParts_returnsEachOfThem() { + withPopulatedDatabase(messagesWithoutParts = MESSAGES_WITHOUT_PARTS) { conversationId, _ -> + val unbounded = unboundedMessageIds(conversationId = conversationId) + val window = windowedMessageIds( + limit = WINDOW_OVER_THE_MESSAGES_WITHOUT_PARTS, + conversationId = conversationId, + ) + + assertEquals( + "the window is short, so the conversation looks fully loaded", + WINDOW_OVER_THE_MESSAGES_WITHOUT_PARTS, + window.size, + ) + assertEquals(MESSAGE_COUNT + MESSAGES_WITHOUT_PARTS, unbounded.size) + assertEquals( + "the window is not the newest messages of the conversation", + unbounded.take(WINDOW_OVER_THE_MESSAGES_WITHOUT_PARTS), + window, + ) + } + } + + /** + * Grouping moved from the part id to the message id, and those are the same key only as long + * as every part joins back to its own message. A message carrying several parts still has to + * come back as the single row that holds all of them, or every multipart message in the + * conversation would be torn into one row per part. + */ + @Test + fun windowedQuery_withAMultipartMessage_returnsItsPartsInOneRow() { + withPopulatedDatabase(multipartTexts = MULTIPART_TEXTS) { conversationId, _ -> + val window = windowedMessageTexts(limit = 2, conversationId = conversationId) + + assertEquals("the parts of one message are one row", 2, window.size) + assertEquals("message ${MESSAGE_COUNT - 1}", window.last()) + // The projection quotes every part and joins them once a message has more than one, + // in an order group_concat does not promise. + assertEquals( + MULTIPART_TEXTS.map { "'$it'" }.toSet(), + window.first().split(PARTS_DIVIDER).toSet(), + ) + } + } + + /** + * The single message uri is the other half of not walking the conversation: the repository + * uses it to refresh one message, and it has to come back alone. + */ + @Test + fun messageQuery_returnsOnlyThatMessage() { + withPopulatedDatabase { conversationId, messageIds -> + assertEquals( + listOf("message 7"), + messageTexts( + MessagingContentProvider.buildConversationMessageUri( + conversationId, + messageIds[7], + ), + conversationId, + ), + ) + } + } + + private fun withPopulatedDatabase( + messagesWithoutParts: Int = 0, + multipartTexts: List = emptyList(), + block: (String, List) -> Unit, + ) { + SQLiteDatabase.create(null).use { db -> + DatabaseHelper.rebuildTables(db) + val senderId = db.insertParticipant() + val conversationId = db.insertConversation() + val otherConversationId = db.insertConversation() + + val messageIds = List(MESSAGE_COUNT) { index -> + db.insertMessageWithPart( + conversationId = conversationId, + senderId = senderId, + receivedTimestamp = FIRST_TIMESTAMP + index, + text = "message $index", + ) + } + // Newer than every message above, so that a window small enough to be worth + // growing is the one that holds them. + var timestamp = FIRST_TIMESTAMP + MESSAGE_COUNT + repeat(messagesWithoutParts) { + db.insertMessage( + conversationId = conversationId, + senderId = senderId, + receivedTimestamp = timestamp++, + ) + } + if (multipartTexts.isNotEmpty()) { + val multipartMessageId = db.insertMessageWithPart( + conversationId = conversationId, + senderId = senderId, + receivedTimestamp = timestamp++, + text = multipartTexts.first(), + ) + multipartTexts.drop(1).forEach { text -> + db.insertPart( + messageId = multipartMessageId, + conversationId = conversationId, + text = text, + ) + } + } + db.insertMessageWithPart( + conversationId = conversationId, + senderId = senderId, + receivedTimestamp = timestamp++, + text = "draft", + status = MessageData.BUGLE_STATUS_OUTGOING_DRAFT, + ) + db.insertMessageWithPart( + conversationId = otherConversationId, + senderId = senderId, + receivedTimestamp = timestamp, + text = "other conversation", + ) + + installProvider(db = db) + + block(conversationId, messageIds) + } + } + + /** The real provider, serving [db] instead of the on-disk database. */ + private fun installProvider(db: SQLiteDatabase) { + val provider = MessagingContentProvider() + provider.attachInfo( + context, + ProviderInfo().apply { authority = MessagingContentProvider.AUTHORITY }, + ) + // setDatabaseForTest asserts on a flag that only the instrumentation application sets, so + // the field it writes is written here instead. Nothing else in the provider is replaced. + ReflectionHelpers.setField(provider, "mDatabaseWrapper", DatabaseWrapper(context, db)) + ShadowContentResolver.registerProviderInternal(MessagingContentProvider.AUTHORITY, provider) + } + + private fun windowedMessageTexts(limit: Int, conversationId: String): List { + return messageTexts( + MessagingContentProvider.buildConversationMessagesUri(conversationId, limit), + conversationId, + ) + } + + private fun unboundedMessageTexts(conversationId: String): List { + return messageTexts( + MessagingContentProvider.buildConversationMessagesUri(conversationId), + conversationId, + ) + } + + private fun windowedMessageIds(limit: Int, conversationId: String): List { + return messageRows( + MessagingContentProvider.buildConversationMessagesUri(conversationId, limit), + conversationId, + MessageColumns._ID, + ) + } + + private fun unboundedMessageIds(conversationId: String): List { + return messageRows( + MessagingContentProvider.buildConversationMessagesUri(conversationId), + conversationId, + MessageColumns._ID, + ) + } + + private fun messageTexts(uri: Uri, conversationId: String): List { + return messageRows(uri, conversationId, column = "parts_texts") + } + + private fun messageRows(uri: Uri, conversationId: String, column: String): List { + return checkNotNull(context.contentResolver.query(uri, null, null, null, null)) + .use { cursor -> + // The window and the message id travel as query parameters, so the cursor still + // has to watch the bare uri that notifyMessagesChanged posts changes on. + assertEquals( + MessagingContentProvider.buildConversationMessagesUri(conversationId), + cursor.notificationUri, + ) + + generateSequence { cursor.takeIf(Cursor::moveToNext) } + .map { it.getString(it.getColumnIndexOrThrow(column)) } + .toList() + } + } + + private fun SQLiteDatabase.insertParticipant(): String { + return insertOrThrow( + DatabaseHelper.PARTICIPANTS_TABLE, + null, + contentValuesOf(ParticipantColumns.NORMALIZED_DESTINATION to "+15550001"), + ).toString() + } + + private fun SQLiteDatabase.insertConversation(): String { + return insertOrThrow( + DatabaseHelper.CONVERSATIONS_TABLE, + null, + contentValuesOf(ConversationColumns.NAME to "Conversation"), + ).toString() + } + + private fun SQLiteDatabase.insertMessageWithPart( + conversationId: String, + senderId: String, + receivedTimestamp: Long, + text: String, + status: Int = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + ): String { + val messageId = insertMessage( + conversationId = conversationId, + senderId = senderId, + receivedTimestamp = receivedTimestamp, + status = status, + ) + insertPart(messageId = messageId, conversationId = conversationId, text = text) + return messageId + } + + private fun SQLiteDatabase.insertPart( + messageId: String, + conversationId: String, + text: String, + ) { + insertOrThrow( + DatabaseHelper.PARTS_TABLE, + null, + contentValuesOf( + PartColumns.MESSAGE_ID to messageId, + PartColumns.CONVERSATION_ID to conversationId, + PartColumns.TEXT to text, + ), + ) + } + + /** A message with no rows in the parts table, the way a subject only mms is stored. */ + private fun SQLiteDatabase.insertMessage( + conversationId: String, + senderId: String, + receivedTimestamp: Long, + status: Int = MessageData.BUGLE_STATUS_INCOMING_COMPLETE, + ): String { + return insertOrThrow( + DatabaseHelper.MESSAGES_TABLE, + null, + contentValuesOf( + MessageColumns.CONVERSATION_ID to conversationId, + MessageColumns.SENDER_PARTICIPANT_ID to senderId, + MessageColumns.RECEIVED_TIMESTAMP to receivedTimestamp, + MessageColumns.STATUS to status, + ), + ).toString() + } + + private companion object { + private const val MESSAGE_COUNT = 25 + private const val FIRST_TIMESTAMP = 1_700_000_000_000L + + /** More than one, because a single message without parts is still a single row. */ + private const val MESSAGES_WITHOUT_PARTS = 2 + + /** Both of the messages without parts, and enough messages around them to tell. */ + private const val WINDOW_OVER_THE_MESSAGES_WITHOUT_PARTS = 5 + + private val MULTIPART_TEXTS = listOf("first part", "second part") + private const val PARTS_DIVIDER = '|' + } +} diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt index 266335523..552ae918b 100644 --- a/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt +++ b/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt @@ -6,11 +6,13 @@ import androidx.core.content.contentValuesOf import com.android.messaging.FactoryTestAccess import com.android.messaging.R import com.android.messaging.datamodel.DatabaseHelper.ConversationColumns +import com.android.messaging.datamodel.DatabaseHelper.MessageColumns import com.android.messaging.datamodel.data.ConversationListItemData import com.android.messaging.testutil.installTestFactory import io.mockk.unmockkAll import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -83,6 +85,35 @@ class DatabaseUpgradeHelperTest { } } + @Test + fun upgradeToVersion5_createsConversationTimestampIndex() { + SQLiteDatabase.create(null).use { db -> + db.execSQL( + "CREATE TABLE ${DatabaseHelper.MESSAGES_TABLE} (" + + "_id INTEGER PRIMARY KEY, " + + "${MessageColumns.CONVERSATION_ID} INTEGER, " + + "${MessageColumns.RECEIVED_TIMESTAMP} INTEGER)", + ) + + DatabaseUpgradeHelper().upgradeToVersion5(db) + + assertTrue( + db.hasIndex("index_${DatabaseHelper.MESSAGES_TABLE}_conversation_timestamp"), + ) + } + } + + @Test + fun upgradeToVersion5_whenTheIndexCannotBeCreated_stillReachesVersion5() { + SQLiteDatabase.create(null).use { db -> + // No messages table: execSQL throws exactly as it would with no room left on the disk. + assertEquals(5, DatabaseUpgradeHelper().upgradeToVersion5(db)) + assertFalse( + db.hasIndex("index_${DatabaseHelper.MESSAGES_TABLE}_conversation_timestamp"), + ) + } + } + private fun SQLiteDatabase.hasColumn(table: String, column: String): Boolean { return rawQuery("SELECT * FROM $table LIMIT 0", null).use { cursor -> cursor.getColumnIndex(column) != -1 diff --git a/app/src/test/kotlin/com/android/messaging/domain/photoviewer/usecase/ResolveConversationPhotoViewerInitialOccurrenceIndexTest.kt b/app/src/test/kotlin/com/android/messaging/domain/photoviewer/usecase/ResolveConversationPhotoViewerInitialOccurrenceIndexTest.kt deleted file mode 100644 index aed60424d..000000000 --- a/app/src/test/kotlin/com/android/messaging/domain/photoviewer/usecase/ResolveConversationPhotoViewerInitialOccurrenceIndexTest.kt +++ /dev/null @@ -1,123 +0,0 @@ -package com.android.messaging.domain.photoviewer.usecase - -import androidx.core.net.toUri -import com.android.messaging.domain.photoviewer.model.ConversationPhotoViewerAttachment -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@RunWith(RobolectricTestRunner::class) -internal class ResolveConversationPhotoViewerInitialOccurrenceIndexTest { - - private val resolveIndex = ResolveConversationPhotoViewerInitialOccurrenceIndexImpl( - normalizePhotoViewerUri = NormalizePhotoViewerUriImpl(), - ) - - @Test - fun invoke_whenClickedUriHasDuplicates_returnsClickedOccurrenceIndex() { - val result = resolveIndex( - partId = "part-3", - contentUri = DUPLICATE_URI.toUri(), - attachments = sequenceOf( - photoAttachment(partId = "part-1", contentUri = DUPLICATE_URI), - photoAttachment(partId = "part-2", contentUri = DUPLICATE_URI), - photoAttachment(partId = "part-3", contentUri = DUPLICATE_URI), - ), - ) - - assertEquals(2, result) - } - - @Test - fun invoke_whenClickedUriIsDistinct_returnsZero() { - val result = resolveIndex( - partId = "part-2", - contentUri = "content://example/content/2".toUri(), - attachments = sequenceOf( - photoAttachment(partId = "part-1", contentUri = "content://example/content/1"), - photoAttachment(partId = "part-2", contentUri = "content://example/content/2"), - photoAttachment(partId = "part-3", contentUri = "content://example/content/3"), - ), - ) - - assertEquals(0, result) - } - - @Test - fun invoke_whenUrisOnlyDifferByQueryOrFragment_treatsUrisAsDuplicates() { - val result = resolveIndex( - partId = "part-3", - contentUri = "$DUPLICATE_URI#preview".toUri(), - attachments = sequenceOf( - photoAttachment(partId = "part-1", contentUri = DUPLICATE_URI), - photoAttachment(partId = "part-2", contentUri = "$DUPLICATE_URI?version=2"), - photoAttachment(partId = "part-3", contentUri = "$DUPLICATE_URI#preview"), - ), - ) - - assertEquals(2, result) - } - - @Test - fun invoke_whenPartIdIsBlank_returnsZero() { - val result = resolveIndex( - partId = "", - contentUri = DUPLICATE_URI.toUri(), - attachments = sequenceOf( - photoAttachment(partId = "part-1", contentUri = DUPLICATE_URI), - photoAttachment(partId = "part-2", contentUri = DUPLICATE_URI), - ), - ) - - assertEquals(0, result) - } - - @Test - fun invoke_whenEarlierPartIdIsBlank_countsEarlierOccurrence() { - val result = resolveIndex( - partId = "part-2", - contentUri = DUPLICATE_URI.toUri(), - attachments = sequenceOf( - photoAttachment(partId = "", contentUri = DUPLICATE_URI), - photoAttachment(partId = "part-2", contentUri = DUPLICATE_URI), - ), - ) - - assertEquals(1, result) - } - - @Test - fun invoke_whenClickedPartIsReached_doesNotConsumeLaterAttachments() { - var laterAttachmentConsumed = false - - val result = resolveIndex( - partId = "part-2", - contentUri = DUPLICATE_URI.toUri(), - attachments = sequence { - yield(photoAttachment(partId = "part-1", contentUri = DUPLICATE_URI)) - yield(photoAttachment(partId = "part-2", contentUri = DUPLICATE_URI)) - laterAttachmentConsumed = true - yield(photoAttachment(partId = "part-3", contentUri = DUPLICATE_URI)) - }, - ) - - assertEquals(1, result) - assertFalse(laterAttachmentConsumed) - } - - private fun photoAttachment( - partId: String, - contentUri: String, - ): ConversationPhotoViewerAttachment { - return ConversationPhotoViewerAttachment( - partId = partId, - contentUri = contentUri.toUri(), - ) - } - - private companion object { - private const val DUPLICATE_URI = "content://example/content/shared" - } -} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/ConversationMessagesDelegateImplTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/ConversationMessagesDelegateImplTest.kt deleted file mode 100644 index 9db76435e..000000000 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/ConversationMessagesDelegateImplTest.kt +++ /dev/null @@ -1,87 +0,0 @@ -package com.android.messaging.ui.conversation.messages.delegate - -import androidx.core.net.toUri -import com.android.messaging.data.appsettings.repository.AppSettingsRepository -import com.android.messaging.data.conversation.repository.ConversationVCardMetadataRepository -import com.android.messaging.data.conversation.repository.ConversationsRepository -import com.android.messaging.domain.media.usecase.ResolveAudioDurationMillis -import com.android.messaging.domain.photoviewer.usecase.ResolveConversationPhotoViewerInitialOccurrenceIndex -import com.android.messaging.ui.conversation.attachment.mapper.ConversationVCardAttachmentUiModelMapper -import com.android.messaging.ui.conversation.messages.mapper.ConversationMessageUiModelMapper -import com.android.messaging.util.ContentType -import io.mockk.every -import io.mockk.mockk -import io.mockk.verify -import kotlinx.coroutines.test.StandardTestDispatcher -import org.junit.Assert.assertEquals -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@RunWith(RobolectricTestRunner::class) -internal class ConversationMessagesDelegateImplTest { - - private val resolveInitialPhotoOccurrenceIndex = - mockk() - private val delegate = ConversationMessagesDelegateImpl( - conversationsRepository = mockk(), - appSettingsRepository = mockk(), - resolveAudioDurationMillis = mockk(), - resolveInitialPhotoOccurrenceIndex = resolveInitialPhotoOccurrenceIndex, - conversationMessageUiModelMapper = mockk(), - conversationVCardAttachmentUiModelMapper = - mockk(), - conversationVCardMetadataRepository = mockk(), - defaultDispatcher = StandardTestDispatcher(), - ) - - @Test - fun resolvePhotoViewerInitialOccurrenceIndex_whenContentTypeIsImage_usesPhotoResolver() { - every { - resolveInitialPhotoOccurrenceIndex.invoke( - partId = PART_ID, - contentUri = ATTACHMENT_URI.toUri(), - attachments = any(), - ) - } returns SECOND_OCCURRENCE_INDEX - - val result = delegate.resolvePhotoViewerInitialOccurrenceIndex( - contentType = ContentType.IMAGE_JPEG, - partId = PART_ID, - contentUri = ATTACHMENT_URI, - ) - - assertEquals(SECOND_OCCURRENCE_INDEX, result) - verify(exactly = 1) { - resolveInitialPhotoOccurrenceIndex.invoke( - partId = PART_ID, - contentUri = ATTACHMENT_URI.toUri(), - attachments = any(), - ) - } - } - - @Test - fun resolvePhotoViewerInitialOccurrenceIndex_whenContentTypeIsNotImage_skipsPhotoResolver() { - val result = delegate.resolvePhotoViewerInitialOccurrenceIndex( - contentType = ContentType.VIDEO_MP4, - partId = PART_ID, - contentUri = ATTACHMENT_URI, - ) - - assertEquals(0, result) - verify(exactly = 0) { - resolveInitialPhotoOccurrenceIndex.invoke( - partId = any(), - contentUri = any(), - attachments = any(), - ) - } - } - - private companion object { - private const val ATTACHMENT_URI = "content://example/attachment/1" - private const val PART_ID = "part-1" - private const val SECOND_OCCURRENCE_INDEX = 1 - } -} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/BaseConversationMessagesDelegateTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/BaseConversationMessagesDelegateTest.kt index c03e9a46b..c6611a1ad 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/BaseConversationMessagesDelegateTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/BaseConversationMessagesDelegateTest.kt @@ -1,16 +1,18 @@ package com.android.messaging.ui.conversation.messages.delegate.conversationmessagesdelegate import android.net.Uri +import android.os.Parcel +import androidx.lifecycle.SavedStateHandle import com.android.messaging.data.appsettings.repository.AppSettingsRepository import com.android.messaging.data.conversation.model.ConversationId import com.android.messaging.data.conversation.model.MessageId import com.android.messaging.data.conversation.model.attachment.ConversationVCardAttachmentMetadata import com.android.messaging.data.conversation.model.attachment.ConversationVCardAttachmentType +import com.android.messaging.data.conversation.model.message.ConversationMessagesWindow import com.android.messaging.data.conversation.repository.ConversationVCardMetadataRepository import com.android.messaging.data.conversation.repository.ConversationsRepository import com.android.messaging.datamodel.data.ConversationMessageData import com.android.messaging.domain.media.usecase.ResolveAudioDurationMillis -import com.android.messaging.domain.photoviewer.usecase.ResolveConversationPhotoViewerInitialOccurrenceIndex import com.android.messaging.testutil.MainDispatcherRule import com.android.messaging.testutil.TEST_CONVERSATION_ID as CONVERSATION_ID import com.android.messaging.ui.conversation.attachment.mapper.ConversationVCardAttachmentUiModelMapper @@ -23,13 +25,13 @@ import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import kotlinx.collections.immutable.toImmutableList -import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.map import kotlinx.coroutines.test.TestScope import org.junit.Rule -@OptIn(ExperimentalCoroutinesApi::class) internal abstract class BaseConversationMessagesDelegateTest { @get:Rule @@ -44,36 +46,64 @@ internal abstract class BaseConversationMessagesDelegateTest { protected val messageUiModelMapper = mockk() protected val vCardUiModelMapper = mockk() protected val vCardMetadataRepository = mockk() + protected val savedStateHandle = SavedStateHandle() - protected fun createDelegate(): ConversationMessagesDelegateImpl { + protected fun createDelegate( + savedStateHandle: SavedStateHandle = this.savedStateHandle, + defaultDispatcher: CoroutineDispatcher = mainDispatcherRule.testDispatcher, + ): ConversationMessagesDelegateImpl { return ConversationMessagesDelegateImpl( conversationsRepository = conversationsRepository, appSettingsRepository = appSettingsRepository, resolveAudioDurationMillis = resolveAudioDurationMillis, - resolveInitialPhotoOccurrenceIndex = - mockk(relaxed = true), conversationMessageUiModelMapper = messageUiModelMapper, conversationVCardAttachmentUiModelMapper = vCardUiModelMapper, conversationVCardMetadataRepository = vCardMetadataRepository, - defaultDispatcher = mainDispatcherRule.testDispatcher, + savedStateHandle = savedStateHandle, + defaultDispatcher = defaultDispatcher, ) } protected fun TestScope.createBoundDelegate( conversationIdFlow: StateFlow, + savedStateHandle: SavedStateHandle = + this@BaseConversationMessagesDelegateTest.savedStateHandle, + defaultDispatcher: CoroutineDispatcher = mainDispatcherRule.testDispatcher, ): ConversationMessagesDelegateImpl { - return createDelegate().also { delegate -> + return createDelegate( + savedStateHandle = savedStateHandle, + defaultDispatcher = defaultDispatcher, + ).also { delegate -> delegate.bind(scope = backgroundScope, conversationIdFlow = conversationIdFlow) } } + /** What a restored process is handed: these values, through what the framework saves. */ + protected fun SavedStateHandle.afterProcessDeath(): SavedStateHandle { + val parcel = Parcel.obtain() + + return try { + parcel.writeBundle(savedStateProvider().saveState()) + parcel.setDataPosition(0) + SavedStateHandle.createHandle(parcel.readBundle(), null) + } finally { + parcel.recycle() + } + } + protected fun givenConversationMessages( messages: Flow>, conversationId: ConversationId = CONVERSATION_ID, + hasMore: Boolean = false, ) { every { - conversationsRepository.getConversationMessages(conversationId = conversationId) - } returns messages + conversationsRepository.getConversationMessages( + conversationId = conversationId, + windowSizes = any(), + ) + } returns messages.map { currentMessages -> + ConversationMessagesWindow(messages = currentMessages, hasMore = hasMore) + } } protected fun givenVCardMetadata( diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateBindingTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateBindingTest.kt index 8e2165fda..f7731dd44 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateBindingTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateBindingTest.kt @@ -56,7 +56,10 @@ internal class ConversationMessagesDelegateBindingTest : BaseConversationMessage assertEquals(ConversationMessagesUiState.Loading, delegate.state.value) verify(exactly = 0) { @Suppress("UnusedFlow") - conversationsRepository.getConversationMessages(conversationId = any()) + conversationsRepository.getConversationMessages( + conversationId = any(), + windowSizes = any(), + ) } } } @@ -89,6 +92,7 @@ internal class ConversationMessagesDelegateBindingTest : BaseConversationMessage @Suppress("UnusedFlow") conversationsRepository.getConversationMessages( conversationId = ConversationId("conversation-rebound"), + windowSizes = any(), ) } } @@ -134,13 +138,15 @@ internal class ConversationMessagesDelegateBindingTest : BaseConversationMessage verify(exactly = 1) { @Suppress("UnusedFlow") conversationsRepository.getConversationMessages( - conversationId = CONVERSATION_ID + conversationId = CONVERSATION_ID, + windowSizes = any(), ) } verify(exactly = 1) { @Suppress("UnusedFlow") conversationsRepository.getConversationMessages( - conversationId = ConversationId("conversation-2") + conversationId = ConversationId("conversation-2"), + windowSizes = any(), ) } } diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateWindowConcurrencyTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateWindowConcurrencyTest.kt new file mode 100644 index 000000000..4d9c4229a --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateWindowConcurrencyTest.kt @@ -0,0 +1,264 @@ +package com.android.messaging.ui.conversation.messages.delegate.conversationmessagesdelegate + +import android.os.Bundle +import androidx.lifecycle.SavedStateHandle +import com.android.messaging.data.conversation.model.message.ConversationMessagesWindow +import com.android.messaging.datamodel.data.ConversationMessageData +import com.android.messaging.testutil.TEST_CONVERSATION_ID as CONVERSATION_ID +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.spyk +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.coroutines.CoroutineContext +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.ExperimentalForInheritanceCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.util.ReflectionHelpers + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +internal class ConversationMessagesDelegateWindowConcurrencyTest : + BaseConversationMessagesDelegateTest() { + + @Test + fun growingTheWindow_neverTouchesTheSavedStateFromTheLoadingDispatcher() { + runTest(context = mainDispatcherRule.testDispatcher) { + val loadingDispatcher = LoadingDispatcher(mainDispatcherRule.testDispatcher) + val calls = mutableListOf>() + val hasOlderMessages = MutableStateFlow(value = false) + val windowSizes = givenConversationMessagesFillingTheWindow( + hasOlderMessages = hasOlderMessages, + ) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + savedStateHandle = recordingItsCalls(calls, loadingDispatcher), + defaultDispatcher = loadingDispatcher, + ) + runCurrent() + delegate.loadOlderMessages() + runCurrent() + + // The request found everything loaded, so the window is grown by the observer, on the + // loading dispatcher, once older messages turn up. + hasOlderMessages.value = true + runCurrent() + + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + assertFalse(calls.toString(), calls.any { (_, dispatcher) -> dispatcher == LOADING }) + } + } + + @Test + fun savingTheState_leavesTheGrownWindowAlone() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenConversationMessagesFillingTheWindow() + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + + // What saving reads, taken before the window grows underneath it. + val readWhileSaving = savedStateHandle.savedStateProvider().saveState() + delegate.loadOlderMessages() + runCurrent() + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + + writeBackWhatSavingRead(readWhileSaving) + runCurrent() + + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + } + } + + private fun writeBackWhatSavingRead(readWhileSaving: Bundle) { + val readValues = SavedStateHandle.createHandle(readWhileSaving, null) + + readValues.keys().forEach { key -> + savedStateHandle[key] = readValues.get(key) + } + } + + @Test + fun loadOlderMessages_whileAWindowArrives_growsTheWindowOnce() { + runTest(context = mainDispatcherRule.testDispatcher) { + val newMessages = MutableStateFlow(value = 0) + val windowSizes = givenConversationMessagesFillingTheWindow( + newMessages = newMessages, + ) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + + val askedForOlderMessages = CountDownLatch(1) + val letTheRequestFinish = CountDownLatch(1) + pauseTheRequestBeforeItGrowsTheWindow( + delegate = delegate, + paused = askedForOlderMessages, + resumed = letTheRequestFinish, + ) + val request = Thread({ delegate.loadOlderMessages() }, REQUEST_THREAD_NAME) + request.start() + + try { + assertTrue(askedForOlderMessages.await(AWAIT_SECONDS, TimeUnit.SECONDS)) + + // A message arrives while the request is pending, and the window observer finds it. + newMessages.value += 1 + runCurrent() + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + } finally { + letTheRequestFinish.countDown() + request.join(TimeUnit.SECONDS.toMillis(AWAIT_SECONDS)) + } + runCurrent() + + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + } + } + + @OptIn(ExperimentalForInheritanceCoroutinesApi::class) + private fun pauseTheRequestBeforeItGrowsTheWindow( + delegate: Any, + paused: CountDownLatch, + resumed: CountDownLatch, + ) { + val hasOlderMessages = ReflectionHelpers + .getField>(delegate, HAS_OLDER_MESSAGES) + + val pauseOnce = AtomicBoolean(true) + val pausingRead = object : MutableStateFlow by hasOlderMessages { + override var value: Boolean + get() { + val currentValue = hasOlderMessages.value + + if (Thread.currentThread().name == REQUEST_THREAD_NAME && + pauseOnce.compareAndSet(true, false) + ) { + paused.countDown() + check(resumed.await(AWAIT_SECONDS, TimeUnit.SECONDS)) + } + + return currentValue + } + set(value) { + hasOlderMessages.value = value + } + } + + ReflectionHelpers.setField(delegate, HAS_OLDER_MESSAGES, pausingRead) + } + + private fun recordingItsCalls( + calls: MutableList>, + loadingDispatcher: LoadingDispatcher? = null, + ): SavedStateHandle { + fun called(call: String) { + calls += call to when (loadingDispatcher?.isRunning) { + true -> LOADING + else -> MAIN + } + } + + return spyk(SavedStateHandle()) { + every { get(any()) } answers { + called("get") + callOriginal() + } + every { set(any(), any()) } answers { + called("set") + callOriginal() + } + every { getMutableStateFlow(any(), any()) } answers { + called(GET_MUTABLE_STATE_FLOW) + callOriginal() + } + every { remove(any()) } answers { + called("remove") + callOriginal() + } + every { setSavedStateProvider(any(), any()) } answers { + called("setSavedStateProvider") + callOriginal() + } + } + } + + private fun givenConversationMessagesFillingTheWindow( + newMessages: Flow = MutableStateFlow(value = 0), + hasOlderMessages: Flow = MutableStateFlow(value = true), + ): () -> Flow { + val message = mockk(relaxed = true) + every { messageUiModelMapper.map(data = message) } returns + messageUiModel(messageId = "message") + val capturedWindowSizes = slot>() + + every { + conversationsRepository.getConversationMessages( + conversationId = any(), + windowSizes = capture(capturedWindowSizes), + ) + } answers { + combine( + capturedWindowSizes.captured, + newMessages, + hasOlderMessages, + ) { windowSize, _, hasMore -> + val loadedMessageCount = minOf(windowSize, MESSAGE_COUNT) + + ConversationMessagesWindow( + messages = List(loadedMessageCount) { message }, + hasMore = hasMore && loadedMessageCount < MESSAGE_COUNT, + ) + } + } + + return { capturedWindowSizes.captured } + } + + private class LoadingDispatcher( + private val testDispatcher: CoroutineDispatcher, + ) : CoroutineDispatcher() { + + var isRunning = false + private set + + override fun dispatch(context: CoroutineContext, block: Runnable) { + testDispatcher.dispatch(context) { + isRunning = true + + try { + block.run() + } finally { + isRunning = false + } + } + } + } + + private companion object { + private const val DEFAULT_WINDOW = 500 + private const val MESSAGE_COUNT = DEFAULT_WINDOW * 4 + private const val AWAIT_SECONDS = 5L + private const val MAIN = "main" + private const val LOADING = "loading dispatcher" + private const val GET_MUTABLE_STATE_FLOW = "getMutableStateFlow" + private const val HAS_OLDER_MESSAGES = "hasOlderMessages" + private const val REQUEST_THREAD_NAME = "test-load-older-messages" + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateWindowTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateWindowTest.kt new file mode 100644 index 000000000..e072d1dd4 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/delegate/conversationmessagesdelegate/ConversationMessagesDelegateWindowTest.kt @@ -0,0 +1,267 @@ +package com.android.messaging.ui.conversation.messages.delegate.conversationmessagesdelegate + +import com.android.messaging.data.conversation.model.ConversationId +import com.android.messaging.data.conversation.model.MessageId +import com.android.messaging.data.conversation.model.message.ConversationMessagesWindow +import com.android.messaging.datamodel.data.ConversationMessageData +import com.android.messaging.testutil.TEST_CONVERSATION_ID as CONVERSATION_ID +import com.android.messaging.ui.conversation.messages.model.message.ConversationMessagesUiState +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +internal class ConversationMessagesDelegateWindowTest : BaseConversationMessagesDelegateTest() { + + @Test + fun bind_startsWithTheDefaultWindow() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenWindowedConversationMessages(loadedMessageCount = 10) + createBoundDelegate(conversationIdFlow = MutableStateFlow(CONVERSATION_ID)) + runCurrent() + + assertEquals(DEFAULT_WINDOW, windowSizes().first()) + } + } + + @Test + fun loadOlderMessages_withOlderMessages_doublesWhatWasLoaded() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenWindowedConversationMessages( + loadedMessageCount = DEFAULT_WINDOW, + hasMore = true, + ) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + + delegate.loadOlderMessages() + runCurrent() + + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + } + } + + @Test + fun loadOlderMessages_calledTwiceBeforeTheLargerWindowArrives_growsOnce() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenWindowedConversationMessages( + loadedMessageCount = DEFAULT_WINDOW, + hasMore = true, + ) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + + delegate.loadOlderMessages() + delegate.loadOlderMessages() + runCurrent() + + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + } + } + + @Test + fun loadOlderMessages_withoutOlderMessages_doesNotGrowTheWindow() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenWindowedConversationMessages(loadedMessageCount = 10) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + + delegate.loadOlderMessages() + runCurrent() + + assertEquals(DEFAULT_WINDOW, windowSizes().first()) + } + } + + @Test + fun bind_afterProcessDeath_restoresTheGrownWindowAndItsMessages() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenConversationMessagesUpTo(messageCount = LARGE_WINDOW) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + // 500, 1000, 2000, 4000, 8000: each request doubles what the one before it loaded, + // and past 4000 so that a window capped at the step this started from fails here. + repeat(4) { + delegate.loadOlderMessages() + runCurrent() + } + assertEquals(LARGE_WINDOW, windowSizes().first()) + + // A brand new delegate, handed the state the framework would have saved and restored. + val restored = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + savedStateHandle = savedStateHandle.afterProcessDeath(), + ) + runCurrent() + + assertEquals(LARGE_WINDOW, windowSizes().first()) + val messages = (restored.state.value as ConversationMessagesUiState.Present).messages + assertEquals(LARGE_WINDOW, messages.size) + assertEquals(MessageId(OLDEST_MESSAGE_ID), messages.first().messageId) + } + } + + @Test + fun bind_toAnotherConversation_startsFromTheDefaultWindowAgain() { + runTest(context = mainDispatcherRule.testDispatcher) { + val windowSizes = givenWindowedConversationMessages( + loadedMessageCount = DEFAULT_WINDOW, + hasMore = true, + ) + val conversationIdFlow = MutableStateFlow(CONVERSATION_ID) + val delegate = createBoundDelegate(conversationIdFlow = conversationIdFlow) + runCurrent() + delegate.loadOlderMessages() + runCurrent() + + conversationIdFlow.value = OTHER_CONVERSATION_ID + runCurrent() + + assertEquals(DEFAULT_WINDOW, windowSizes().first()) + } + } + + @Test + fun newMessages_afterARequestThatFoundEverythingLoaded_growTheWindow() { + runTest(context = mainDispatcherRule.testDispatcher) { + // Short enough to fit in the window, so the request from the oldest message is ignored. + val windows = MutableStateFlow( + value = messagesWindow(loadedMessageCount = DEFAULT_WINDOW - 1, hasMore = false), + ) + val windowSizes = givenChangingConversationMessages(windows = windows) + val delegate = createBoundDelegate( + conversationIdFlow = MutableStateFlow(CONVERSATION_ID), + ) + runCurrent() + delegate.loadOlderMessages() + runCurrent() + assertEquals(DEFAULT_WINDOW, windowSizes().first()) + + // Two more arrive and push the oldest message out of the window, while the user is + // still sitting at the oldest edge and will not ask for more from there. + windows.value = messagesWindow(loadedMessageCount = DEFAULT_WINDOW, hasMore = true) + runCurrent() + + assertEquals(DEFAULT_WINDOW * 2, windowSizes().first()) + } + } + + /** + * Answers every conversation with the newest [messageCount] messages at most, as many of them + * as the window asks for, and hands back the window sizes the delegate asked for. + */ + private fun givenConversationMessagesUpTo(messageCount: Int): () -> Flow { + val message = message(messageId = "message") + val oldestMessage = message(messageId = OLDEST_MESSAGE_ID) + val capturedWindowSizes = slot>() + + every { + conversationsRepository.getConversationMessages( + conversationId = any(), + windowSizes = capture(capturedWindowSizes), + ) + } answers { + capturedWindowSizes.captured.map { windowSize -> + val loadedMessageCount = minOf(windowSize, messageCount) + + ConversationMessagesWindow( + // Oldest first, the way the repository hands the window over. + messages = List(loadedMessageCount) { index -> + when (index) { + 0 -> oldestMessage + else -> message + } + }, + hasMore = loadedMessageCount < messageCount, + ) + } + } + + return { capturedWindowSizes.captured } + } + + /** Answers every conversation with the same fixed window. */ + private fun givenWindowedConversationMessages( + loadedMessageCount: Int, + hasMore: Boolean = false, + ): () -> Flow { + return givenChangingConversationMessages( + windows = flowOf( + messagesWindow(loadedMessageCount = loadedMessageCount, hasMore = hasMore), + ), + ) + } + + /** + * Answers every conversation with [windows], re-reading it whenever the delegate asks for + * another size, and hands back the window sizes the delegate asked for. + */ + private fun givenChangingConversationMessages( + windows: Flow, + ): () -> Flow { + val capturedWindowSizes = slot>() + + every { + conversationsRepository.getConversationMessages( + conversationId = any(), + windowSizes = capture(capturedWindowSizes), + ) + } answers { + combine(capturedWindowSizes.captured, windows) { _, window -> window } + } + + return { capturedWindowSizes.captured } + } + + private fun messagesWindow( + loadedMessageCount: Int, + hasMore: Boolean, + ): ConversationMessagesWindow { + val message = message(messageId = "message") + + return ConversationMessagesWindow( + messages = List(loadedMessageCount) { message }, + hasMore = hasMore, + ) + } + + /** + * A message to repeat across a whole window: only its size and its oldest message matter here, + * and a relaxed mock per message is enough to exhaust the test heap. + */ + private fun message(messageId: String): ConversationMessageData { + return mockk(relaxed = true).also { data -> + every { messageUiModelMapper.map(data = data) } returns + messageUiModel(messageId = messageId) + } + } + + private companion object { + private const val DEFAULT_WINDOW = 500 + private const val LARGE_WINDOW = DEFAULT_WINDOW * 16 + private const val OLDEST_MESSAGE_ID = "message-oldest" + private val OTHER_CONVERSATION_ID = ConversationId("conversation-other") + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/ConversationScreenScrollBehaviorTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/ConversationScreenScrollBehaviorTest.kt index bad7ff3e2..50cf48229 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/ConversationScreenScrollBehaviorTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/ConversationScreenScrollBehaviorTest.kt @@ -118,8 +118,9 @@ internal class ConversationScreenScrollBehaviorTest : BaseConversationScreenTest composeTestRule.waitForIdle() + // The widget hands over a newest first position, so 5 is the sixth message from the end. composeTestRule - .onNodeWithTag(conversationMessageItemTestTag(messageId = MessageId("message-6"))) + .onNodeWithTag(conversationMessageItemTestTag(messageId = MessageId("message-45"))) .assertIsDisplayed() composeTestRule.runOnIdle { assertEquals(1, consumedCount) diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/MessagePositionToDisplayIndexTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/MessagePositionToDisplayIndexTest.kt index f25e0c16c..524dbefef 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/MessagePositionToDisplayIndexTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/MessagePositionToDisplayIndexTest.kt @@ -6,19 +6,24 @@ import org.junit.Test class MessagePositionToDisplayIndexTest { @Test - fun position0_inFiveItemList_mapsToLastDisplayIndex() { - assertEquals(4, messagePositionToDisplayIndex(position = 0, size = 5)) + fun position0_inFiveItemList_mapsToNewestDisplayIndex() { + assertEquals(0, messagePositionToDisplayIndex(position = 0, size = 5)) } @Test - fun lastPosition_mapsToFirstDisplayIndex() { - assertEquals(0, messagePositionToDisplayIndex(position = 4, size = 5)) + fun lastPosition_mapsToOldestDisplayIndex() { + assertEquals(4, messagePositionToDisplayIndex(position = 4, size = 5)) } @Test - fun positionAtOrBeyondSize_clampsToZero() { - assertEquals(0, messagePositionToDisplayIndex(position = 5, size = 5)) - assertEquals(0, messagePositionToDisplayIndex(position = 100, size = 5)) + fun positionAtOrBeyondSize_clampsToLastDisplayIndex() { + assertEquals(4, messagePositionToDisplayIndex(position = 5, size = 5)) + assertEquals(4, messagePositionToDisplayIndex(position = 100, size = 5)) + } + + @Test + fun negativePosition_clampsToZero() { + assertEquals(0, messagePositionToDisplayIndex(position = -1, size = 5)) } @Test diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/effects/ConversationAttachmentPreviewEffectTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/effects/ConversationAttachmentPreviewEffectTest.kt index 264f17b2b..b87a3519d 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/effects/ConversationAttachmentPreviewEffectTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/effects/ConversationAttachmentPreviewEffectTest.kt @@ -25,7 +25,6 @@ import io.mockk.runs import io.mockk.slot import io.mockk.unmockkAll import io.mockk.verify -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals @@ -35,7 +34,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -@OptIn(ExperimentalCoroutinesApi::class) @RunWith(RobolectricTestRunner::class) internal class ConversationAttachmentPreviewEffectTest { @@ -96,7 +94,7 @@ internal class ConversationAttachmentPreviewEffectTest { right = 7, bottom = 9, ), - initialPhotoOccurrenceIndex = 0, + initialPartId = null, ), ), launchRequests, diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/BaseConversationViewModelTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/BaseConversationViewModelTest.kt index 641b6a15d..974aa219d 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/BaseConversationViewModelTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/BaseConversationViewModelTest.kt @@ -56,7 +56,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import org.junit.Rule -@OptIn(ExperimentalCoroutinesApi::class) internal abstract class BaseConversationViewModelTest { @get:Rule @@ -269,9 +268,6 @@ internal abstract class BaseConversationViewModelTest { ) val mock = mockk(relaxed = true) every { mock.state } returns stateFlow - every { - mock.resolvePhotoViewerInitialOccurrenceIndex(any(), any(), any()) - } returns 0 every { mock.bind(any(), any()) } answers { diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelAttachmentPreviewTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelAttachmentPreviewTest.kt index 5b250a1b0..adeec49cc 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelAttachmentPreviewTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelAttachmentPreviewTest.kt @@ -90,6 +90,7 @@ internal class ConversationViewModelAttachmentPreviewTest : BaseConversationView imageCollectionUri = MessagingContentProvider .buildConversationImagesUri(CONVERSATION_ID.value) .toString(), + initialPartId = "part-1", ), awaitItem(), ) diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelSimSelectionTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelSimSelectionTest.kt index 6de4b831e..269f2d1c2 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelSimSelectionTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/screen/viewmodel/ConversationViewModelSimSelectionTest.kt @@ -103,14 +103,6 @@ internal class ConversationViewModelSimSelectionTest { ConversationMessagesUiState.Loading, ) every { conversationMessagesDelegate.bind(any(), any()) } just runs - every { - conversationMessagesDelegate.resolvePhotoViewerInitialOccurrenceIndex( - contentType = any(), - partId = any(), - contentUri = any(), - ) - } returns 0 - every { conversationMessageSelectionDelegate.state } returns MutableStateFlow( ConversationMessageSelectionUiState(), ) @@ -161,44 +153,6 @@ internal class ConversationViewModelSimSelectionTest { } just runs } - @Test - fun onMessageAttachmentClicked_whenImageAttachment_resolvesPhotoOccurrenceIndex() { - val viewModel = createViewModel() - - viewModel.onMessageAttachmentClicked( - contentType = ContentType.IMAGE_JPEG, - contentUri = ATTACHMENT_URI, - partId = ATTACHMENT_PART_ID, - ) - - verify(exactly = 1) { - conversationMessagesDelegate.resolvePhotoViewerInitialOccurrenceIndex( - contentType = ContentType.IMAGE_JPEG, - partId = ATTACHMENT_PART_ID, - contentUri = ATTACHMENT_URI, - ) - } - } - - @Test - fun onMessageAttachmentClicked_whenNonImageAttachment_delegatesOccurrenceResolution() { - val viewModel = createViewModel() - - viewModel.onMessageAttachmentClicked( - contentType = ContentType.VIDEO_MP4, - contentUri = ATTACHMENT_URI, - partId = ATTACHMENT_PART_ID, - ) - - verify(exactly = 1) { - conversationMessagesDelegate.resolvePhotoViewerInitialOccurrenceIndex( - contentType = ContentType.VIDEO_MP4, - partId = ATTACHMENT_PART_ID, - contentUri = ATTACHMENT_URI, - ) - } - } - @Test fun onSimSelected_withConversationId_forwardsSelectionToDraftDelegate() { val viewModel = createViewModel() diff --git a/app/src/test/kotlin/com/android/messaging/ui/navigation/NavKeySerializationTest.kt b/app/src/test/kotlin/com/android/messaging/ui/navigation/NavKeySerializationTest.kt index 8d0ab8cdd..41b51e671 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/navigation/NavKeySerializationTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/navigation/NavKeySerializationTest.kt @@ -101,7 +101,7 @@ class NavKeySerializationTest { right = 110, bottom = 220, ), - initialPhotoOccurrenceIndex = 2, + initialPartId = "part-2", ), ), ) diff --git a/app/src/test/kotlin/com/android/messaging/ui/photoviewer/screen/PhotoViewerViewModelTest.kt b/app/src/test/kotlin/com/android/messaging/ui/photoviewer/screen/PhotoViewerViewModelTest.kt index e4949d892..51fe1e451 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/photoviewer/screen/PhotoViewerViewModelTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/photoviewer/screen/PhotoViewerViewModelTest.kt @@ -516,7 +516,7 @@ internal class PhotoViewerViewModelTest { repository.getPhotoViewerItems( photosUri = any(), initialPhotoUri = any(), - initialPhotoOccurrenceIndex = any(), + initialPartId = any(), ) } returns results @@ -554,6 +554,7 @@ internal class PhotoViewerViewModelTest { fun photoViewerItem(index: Int): PhotoViewerItem { return PhotoViewerItem( + partId = "part-$index", contentUri = Uri.parse("content://example/content/$index"), contentType = IMAGE_JPEG, isIncoming = true, diff --git a/res/values/versions.xml b/res/values/versions.xml index 8334be238..33fa1d347 100644 --- a/res/values/versions.xml +++ b/res/values/versions.xml @@ -16,7 +16,7 @@ --> - 4 + 5