Try to make all collections used in Compose code immutable (#1922)

* Try to make all collections used in Compose code immutable.

Mark sealed interfaces as `@Immutable` too

* Add gradle code to check the compose compiler reports

* Fix some more unstable classes
This commit is contained in:
Jorge Martin Espinosa 2023-11-29 16:02:08 +01:00 committed by GitHub
parent c697baf585
commit c739ebc0ee
55 changed files with 213 additions and 107 deletions

View file

@ -133,10 +133,10 @@ class MessagesPresenter @AssistedInject constructor(
val syncUpdateFlow = room.syncUpdateFlow.collectAsState()
val userHasPermissionToSendMessage by room.canSendMessageAsState(type = MessageEventType.ROOM_MESSAGE, updateKey = syncUpdateFlow.value)
val userHasPermissionToRedact by room.canRedactAsState(updateKey = syncUpdateFlow.value)
val roomName: Async<String> by remember(roomInfo?.name) {
val roomName: Async<String> by remember {
derivedStateOf { roomInfo?.name?.let { Async.Success(it) } ?: Async.Uninitialized }
}
val roomAvatar: Async<AvatarData> by remember(roomInfo?.avatarUrl) {
val roomAvatar: Async<AvatarData> by remember {
derivedStateOf { roomInfo?.avatarData()?.let { Async.Success(it) } ?: Async.Uninitialized }
}

View file

@ -45,6 +45,8 @@ import io.element.android.libraries.matrix.api.timeline.item.event.PollContent
import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType
import io.element.android.libraries.matrix.api.timeline.item.event.VideoMessageType
import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageType
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.persistentMapOf
@PreviewsDayNight
@Composable
@ -127,8 +129,8 @@ class InReplyToDetailsProvider : PreviewParameterProvider<InReplyToDetails> {
question = "Poll which are being replied.",
kind = PollKind.Disclosed,
maxSelections = 1u,
answers = emptyList(),
votes = emptyMap(),
answers = persistentListOf(),
votes = persistentMapOf(),
endTime = null
),
).map {

View file

@ -17,7 +17,6 @@
package io.element.android.features.messages.impl.timeline.components.reactionsummary
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.derivedStateOf
@ -38,10 +37,6 @@ class ReactionSummaryPresenter @Inject constructor(
) : Presenter<ReactionSummaryState> {
@Composable
override fun present(): ReactionSummaryState {
LaunchedEffect(Unit) {
room.updateMembers()
}
val membersState by room.membersStateFlow.collectAsState()
val target: MutableState<ReactionSummaryState.Summary?> = remember {

View file

@ -92,6 +92,7 @@ import io.element.android.tests.testutils.consumeItemsUntilTimeout
import io.element.android.tests.testutils.testCoroutineDispatchers
import io.element.android.tests.testutils.waitForPredicate
import io.mockk.mockk
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.Rule
@ -462,7 +463,7 @@ class MessagesPresenterTest {
val room = FakeMatrixRoom(sessionId = A_SESSION_ID)
room.givenRoomMembersState(
MatrixRoomMembersState.Ready(
listOf(
persistentListOf(
aRoomMember(userId = A_SESSION_ID, membership = RoomMembershipState.JOIN),
aRoomMember(userId = A_SESSION_ID_2, membership = RoomMembershipState.LEAVE),
)
@ -489,7 +490,7 @@ class MessagesPresenterTest {
room.givenRoomMembersState(
MatrixRoomMembersState.Error(
failure = Throwable(),
prevRoomMembers = listOf(
prevRoomMembers = persistentListOf(
aRoomMember(userId = A_SESSION_ID, membership = RoomMembershipState.JOIN),
aRoomMember(userId = A_SESSION_ID_2, membership = RoomMembershipState.LEAVE),
)
@ -535,7 +536,7 @@ class MessagesPresenterTest {
val room = FakeMatrixRoom(sessionId = A_SESSION_ID)
room.givenRoomMembersState(
MatrixRoomMembersState.Ready(
listOf(
persistentListOf(
aRoomMember(userId = A_SESSION_ID, membership = RoomMembershipState.JOIN),
aRoomMember(userId = A_SESSION_ID_2, membership = RoomMembershipState.LEAVE),
)

View file

@ -78,11 +78,11 @@ import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.waitForPredicate
import io.mockk.mockk
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import okhttp3.internal.immutableListOf
import org.junit.Rule
import org.junit.Test
import uniffi.wysiwyg_composer.MentionsState
@ -734,7 +734,7 @@ class MessageComposerPresenterTest {
isOneToOne = false,
).apply {
givenRoomMembersState(MatrixRoomMembersState.Ready(
immutableListOf(currentUser, invitedUser, bob, david),
persistentListOf(currentUser, invitedUser, bob, david),
))
givenCanTriggerRoomNotification(Result.success(true))
}
@ -798,7 +798,7 @@ class MessageComposerPresenterTest {
isOneToOne = true,
).apply {
givenRoomMembersState(MatrixRoomMembersState.Ready(
immutableListOf(currentUser, invitedUser, bob, david),
persistentListOf(currentUser, invitedUser, bob, david),
))
givenCanTriggerRoomNotification(Result.success(true))
}

View file

@ -55,6 +55,7 @@ import io.element.android.tests.testutils.awaitWithLatch
import io.element.android.tests.testutils.consumeItemsUntilPredicate
import io.element.android.tests.testutils.testCoroutineDispatchers
import io.element.android.tests.testutils.waitForPredicate
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.delay
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
@ -254,18 +255,18 @@ class TimelinePresenterTest {
val (alice, bob, charlie) = aMatrixUserList().take(3).mapIndexed { i, user ->
ReactionSender(senderId = user.userId, timestamp = now + i * minuteInMillis)
}
val oneReaction = listOf(
val oneReaction = persistentListOf(
EventReaction(
key = "❤️",
senders = listOf(alice, charlie)
senders = persistentListOf(alice, charlie)
),
EventReaction(
key = "👍",
senders = listOf(alice, bob)
senders = persistentListOf(alice, bob)
),
EventReaction(
key = "🐶",
senders = listOf(charlie)
senders = persistentListOf(charlie)
),
)
timeline.updateTimelineItems {

View file

@ -29,6 +29,7 @@ import io.element.android.libraries.matrix.test.A_USER_NAME
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
import io.element.android.libraries.matrix.test.room.aRoomMember
import io.element.android.tests.testutils.WarmUpRule
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
@ -42,7 +43,7 @@ class ReactionSummaryPresenterTests {
private val roomMember = aRoomMember(userId = A_USER_ID, avatarUrl = AN_AVATAR_URL, displayName = A_USER_NAME)
private val summaryEvent = ReactionSummaryEvents.ShowReactionSummary(AN_EVENT_ID, listOf(aggregatedReaction), aggregatedReaction.key)
private val room = FakeMatrixRoom().apply {
givenRoomMembersState(MatrixRoomMembersState.Ready(listOf(roomMember)))
givenRoomMembersState(MatrixRoomMembersState.Ready(persistentListOf(roomMember)))
}
private val presenter = ReactionSummaryPresenter(room)

View file

@ -55,6 +55,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.VoiceMessageT
import io.element.android.libraries.matrix.test.AN_EVENT_ID
import io.element.android.libraries.matrix.ui.components.A_BLUR_HASH
import io.element.android.libraries.mediaviewer.api.util.FileExtensionExtractorWithoutValidation
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.test.runTest
import org.junit.Test
@ -280,7 +281,7 @@ class TimelineItemContentMessageFactoryTest {
),
details = AudioDetails(
duration = 1.minutes,
waveform = listOf(1f, 2f),
waveform = persistentListOf(1f, 2f),
),
)
),
@ -293,7 +294,7 @@ class TimelineItemContentMessageFactoryTest {
duration = 1.minutes,
mediaSource = MediaSource(url = "url", json = null),
mimeType = MimeTypes.Ogg,
waveform = listOf(1f, 2f).toImmutableList()
waveform = persistentListOf(1f, 2f)
)
assertThat(result).isEqualTo(expected)
}

View file

@ -38,6 +38,11 @@ import io.element.android.libraries.matrix.test.A_USER_ID_7
import io.element.android.libraries.matrix.test.A_USER_ID_8
import io.element.android.libraries.matrix.test.A_USER_ID_9
import io.element.android.libraries.matrix.test.FakeMatrixClient
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.ImmutableMap
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.persistentMapOf
import kotlinx.collections.immutable.toImmutableMap
import kotlinx.coroutines.test.runTest
import org.junit.Test
@ -55,7 +60,7 @@ internal class TimelineItemContentPollFactoryTest {
@Test
fun `Disclosed poll - not ended, some votes, including one from current user`() = runTest {
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }.toImmutableMap()
Truth.assertThat(
factory.create(aPollContent(votes = votes), eventId = null)
)
@ -87,7 +92,7 @@ internal class TimelineItemContentPollFactoryTest {
@Test
fun `Disclosed poll - ended, some votes, including one from current user (winner)`() = runTest {
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }.toImmutableMap()
Truth.assertThat(
factory.create(aPollContent(votes = votes, endTime = 1UL), eventId = null)
)
@ -106,7 +111,7 @@ internal class TimelineItemContentPollFactoryTest {
@Test
fun `Disclosed poll - ended, some votes, including one from current user (not winner) and two winning votes`() = runTest {
val votes = OTHER_WINNING_VOTES.mapKeys { it.key.id }
val votes = OTHER_WINNING_VOTES.mapKeys { it.key.id }.toImmutableMap()
Truth.assertThat(
factory.create(aPollContent(votes = votes, endTime = 1UL), eventId = null)
)
@ -136,7 +141,7 @@ internal class TimelineItemContentPollFactoryTest {
@Test
fun `Undisclosed poll - not ended, some votes, including one from current user`() = runTest {
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }.toImmutableMap()
Truth.assertThat(
factory.create(aPollContent(pollKind = PollKind.Undisclosed, votes = votes), eventId = null)
)
@ -172,7 +177,7 @@ internal class TimelineItemContentPollFactoryTest {
@Test
fun `Undisclosed poll - ended, some votes, including one from current user (winner)`() = runTest {
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }
val votes = MY_USER_WINNING_VOTES.mapKeys { it.key.id }.toImmutableMap()
Truth.assertThat(
factory.create(aPollContent(pollKind = PollKind.Undisclosed, votes = votes, endTime = 1UL), eventId = null)
)
@ -192,7 +197,7 @@ internal class TimelineItemContentPollFactoryTest {
@Test
fun `Undisclosed poll - ended, some votes, including one from current user (not winner) and two winning votes`() = runTest {
val votes = OTHER_WINNING_VOTES.mapKeys { it.key.id }
val votes = OTHER_WINNING_VOTES.mapKeys { it.key.id }.toImmutableMap()
Truth.assertThat(
factory.create(aPollContent(PollKind.Undisclosed).copy(votes = votes, endTime = 1UL), eventId = null)
)
@ -221,13 +226,13 @@ internal class TimelineItemContentPollFactoryTest {
private fun aPollContent(
pollKind: PollKind = PollKind.Disclosed,
votes: Map<String, List<UserId>> = emptyMap(),
votes: ImmutableMap<String, ImmutableList<UserId>> = persistentMapOf(),
endTime: ULong? = null,
): PollContent = PollContent(
question = A_POLL_QUESTION,
kind = pollKind,
maxSelections = 1UL,
answers = listOf(A_POLL_ANSWER_1, A_POLL_ANSWER_2, A_POLL_ANSWER_3, A_POLL_ANSWER_4),
answers = persistentListOf(A_POLL_ANSWER_1, A_POLL_ANSWER_2, A_POLL_ANSWER_3, A_POLL_ANSWER_4),
votes = votes,
endTime = endTime,
)
@ -275,17 +280,17 @@ internal class TimelineItemContentPollFactoryTest {
private val A_POLL_ANSWER_3 = PollAnswer("id_3", "French Fries")
private val A_POLL_ANSWER_4 = PollAnswer("id_4", "Hamburger")
private val MY_USER_WINNING_VOTES = mapOf(
A_POLL_ANSWER_1 to listOf(A_USER_ID_2, A_USER_ID_3, A_USER_ID_4),
A_POLL_ANSWER_2 to listOf(A_USER_ID /* my vote */, A_USER_ID_5, A_USER_ID_6, A_USER_ID_7, A_USER_ID_8, A_USER_ID_9), // winner
A_POLL_ANSWER_3 to emptyList(),
A_POLL_ANSWER_4 to listOf(A_USER_ID_10),
private val MY_USER_WINNING_VOTES = persistentMapOf(
A_POLL_ANSWER_1 to persistentListOf(A_USER_ID_2, A_USER_ID_3, A_USER_ID_4),
A_POLL_ANSWER_2 to persistentListOf(A_USER_ID /* my vote */, A_USER_ID_5, A_USER_ID_6, A_USER_ID_7, A_USER_ID_8, A_USER_ID_9), // winner
A_POLL_ANSWER_3 to persistentListOf(),
A_POLL_ANSWER_4 to persistentListOf(A_USER_ID_10),
)
private val OTHER_WINNING_VOTES = mapOf(
A_POLL_ANSWER_1 to listOf(A_USER_ID_2, A_USER_ID_3, A_USER_ID_4, A_USER_ID_5), // winner
A_POLL_ANSWER_2 to listOf(A_USER_ID /* my vote */, A_USER_ID_6),
A_POLL_ANSWER_3 to emptyList(),
A_POLL_ANSWER_4 to listOf(A_USER_ID_7, A_USER_ID_8, A_USER_ID_9, A_USER_ID_10), // winner
private val OTHER_WINNING_VOTES = persistentMapOf(
A_POLL_ANSWER_1 to persistentListOf(A_USER_ID_2, A_USER_ID_3, A_USER_ID_4, A_USER_ID_5), // winner
A_POLL_ANSWER_2 to persistentListOf(A_USER_ID /* my vote */, A_USER_ID_6),
A_POLL_ANSWER_3 to persistentListOf(),
A_POLL_ANSWER_4 to persistentListOf(A_USER_ID_7, A_USER_ID_8, A_USER_ID_9, A_USER_ID_10), // winner
)
}
}

View file

@ -30,6 +30,7 @@ import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.mediaplayer.api.MediaPlayer
import io.element.android.libraries.mediaplayer.test.FakeMediaPlayer
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.Test
@ -88,8 +89,8 @@ fun aRedactedMatrixTimeline(eventId: EventId) = listOf<MatrixTimelineItem>(
isOwn = false,
isRemote = false,
localSendState = null,
reactions = listOf(),
receipts = listOf(),
reactions = persistentListOf(),
receipts = persistentListOf(),
sender = A_USER_ID,
senderProfile = ProfileTimelineDetails.Unavailable,
timestamp = 9442,