Skip to content

Use the right live timeline instance in RustRoomFactory #4745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import io.element.android.appconfig.TimelineConfig
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.RoomId
Expand All @@ -21,6 +20,7 @@
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.awaitLoaded
import io.element.android.libraries.matrix.impl.room.preview.RoomPreviewInfoMapper
import io.element.android.libraries.matrix.impl.roomlist.fullRoomWithTimeline
import io.element.android.libraries.matrix.impl.roomlist.roomOrNull
import io.element.android.services.toolbox.api.systemclock.SystemClock
import kotlinx.coroutines.CoroutineScope
Expand All @@ -30,10 +30,10 @@
import kotlinx.coroutines.withContext
import org.matrix.rustcomponents.sdk.Client
import org.matrix.rustcomponents.sdk.Membership
import org.matrix.rustcomponents.sdk.Room
import org.matrix.rustcomponents.sdk.RoomListItem
import timber.log.Timber
import java.util.concurrent.atomic.AtomicBoolean
import org.matrix.rustcomponents.sdk.Room as SdkRoom
import org.matrix.rustcomponents.sdk.RoomListService as InnerRoomListService

class RustRoomFactory(
Expand All @@ -56,11 +56,6 @@
private val mutex = Mutex()
private val isDestroyed: AtomicBoolean = AtomicBoolean(false)

private data class RustRoomReferences(
val roomListItem: RoomListItem,
val room: SdkRoom,
)

private val roomInfoMapper = RoomInfoMapper()

private val eventFilters = TimelineConfig.excludedEvents
Expand All @@ -84,17 +79,22 @@
Timber.d("Room factory is destroyed, returning null for $roomId")
return@withContext null
}
val roomReferences = awaitRoomReferences(roomId) ?: return@withContext null
getBaseRoom(roomReferences)
val roomListItem = awaitRoomListItem(roomId) ?: return@withContext null
getBaseRoom(roomListItem)

Check warning on line 83 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L83

Added line #L83 was not covered by tests
}
}

private suspend fun getBaseRoom(roomReferences: RustRoomReferences): RustBaseRoom? {
val initialRoomInfo = roomReferences.room.roomInfo()
private suspend fun getBaseRoom(roomListItem: RoomListItem): RustBaseRoom? {
val sdkRoom = innerClient.getRoom(roomListItem.id()) ?: return null
return getBaseRoom(sdkRoom)

Check warning on line 89 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L89

Added line #L89 was not covered by tests
}

private suspend fun getBaseRoom(sdkRoom: Room): RustBaseRoom {
val initialRoomInfo = sdkRoom.roomInfo()

Check warning on line 93 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L93

Added line #L93 was not covered by tests
return RustBaseRoom(
sessionId = sessionId,
deviceId = deviceId,
innerRoom = roomReferences.room,
innerRoom = sdkRoom,

Check warning on line 97 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L97

Added line #L97 was not covered by tests
coroutineDispatchers = dispatchers,
roomSyncSubscriber = roomSyncSubscriber,
roomMembershipObserver = roomMembershipObserver,
Expand All @@ -110,30 +110,30 @@
Timber.d("Room factory is destroyed, returning null for $roomId")
return@withContext null
}
val roomReferences = awaitRoomReferences(roomId) ?: return@withContext null

if (roomReferences.room.membership() == Membership.JOINED) {
val baseRoom = getBaseRoom(roomReferences) ?: return@withContext null
val roomListItem = awaitRoomListItem(roomId) ?: return@withContext null

if (roomListItem.membership() == Membership.JOINED) {
// Init the live timeline in the SDK from the RoomListItem
if (!roomReferences.roomListItem.isTimelineInitialized()) {
roomReferences.roomListItem.initTimeline(eventFilters, "LIVE")
val sdkRoom = if (!roomListItem.isTimelineInitialized()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fullRoomWithTimeline is already checking if it's initialized, so I think we can safely remove the check here

roomListItem.fullRoomWithTimeline(eventFilters)

Check warning on line 118 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L118

Added line #L118 was not covered by tests
} else {
roomListItem.fullRoom()

Check warning on line 120 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L120

Added line #L120 was not covered by tests
}

GetRoomResult.Joined(
JoinedRustRoom(
baseRoom = baseRoom,
baseRoom = getBaseRoom(sdkRoom),

Check warning on line 125 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L125

Added line #L125 was not covered by tests
notificationSettingsService = notificationSettingsService,
roomContentForwarder = roomContentForwarder,
liveInnerTimeline = roomReferences.room.timeline(),
liveInnerTimeline = sdkRoom.timeline(),

Check warning on line 128 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L128

Added line #L128 was not covered by tests
coroutineDispatchers = dispatchers,
systemClock = systemClock,
featureFlagService = featureFlagService,
)
)
} else {
val preview = try {
roomReferences.roomListItem.previewRoom(via = emptyList())
roomListItem.previewRoom(via = emptyList())

Check warning on line 136 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L136

Added line #L136 was not covered by tests
} catch (e: Exception) {
Timber.e(e, "Failed to get room preview for $roomId")
return@withContext null
Expand All @@ -142,44 +142,31 @@
GetRoomResult.NotJoined(
NotJoinedRustRoom(
sessionId = sessionId,
localRoom = getBaseRoom(roomReferences),
localRoom = getBaseRoom(roomListItem),

Check warning on line 145 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L145

Added line #L145 was not covered by tests
previewInfo = RoomPreviewInfoMapper.map(preview.info()),
)
)
}
}
}

private fun getRoomReferences(roomId: RoomId): RustRoomReferences? {
val roomListItem = innerRoomListService.roomOrNull(roomId.value)
if (roomListItem == null) {
Timber.d("Room not found for $roomId")
return null
}
val room = tryOrNull {
innerClient.getRoom(roomId.value)
} ?: error("Failed to get room for room id: $roomId")

Timber.d("Got room for $roomId")
return RustRoomReferences(
roomListItem = roomListItem,
room = room,
)
}

/**
* Get the Rust room references for a room, retrying after the room list is loaded if necessary.
* Get the Rust room list item for a room, retrying after the room list is loaded if necessary.
*/
private suspend fun awaitRoomReferences(roomId: RoomId): RustRoomReferences? {
var roomReferences = getRoomReferences(roomId)

if (roomReferences == null) {
private suspend fun awaitRoomListItem(roomId: RoomId): RoomListItem? {
var roomListItem = innerRoomListService.roomOrNull(roomId.value)

Check warning on line 157 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L157

Added line #L157 was not covered by tests
if (roomListItem == null) {
// ... otherwise, lets wait for the SS to load all rooms and check again.
roomListService.allRooms.awaitLoaded()
roomReferences = getRoomReferences(roomId)
roomListItem = innerRoomListService.roomOrNull(roomId.value)

Check warning on line 161 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L161

Added line #L161 was not covered by tests
}

if (roomListItem == null) {
Timber.d("Room not found for $roomId")
return null

Check warning on line 166 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L165-L166

Added lines #L165 - L166 were not covered by tests
}

return roomReferences
return roomListItem

Check warning on line 169 in libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt

View check run for this annotation

Codecov / codecov/patch

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt#L169

Added line #L169 was not covered by tests
}
}

Expand Down
Loading