Add room's update publisher (#952)

* Add room update publisher * Cleanup * Update room’s title * Add refresh logic in RoomDetailsScreenViewModel * Fix UTs * Add UT * Update timeline listener in RoomProxy * Fix caching in LoadableAvatarImage * Cleanup * Update RoomProxy api * Inject room proxy in RoomScreenViewModel * Cleanup * Fix UTs
2026-11-02 21:33:26 +00:00 · 2023-05-25 12:42:58 +02:00
parent c59921103d
commit bf2102e5d1
15 changed files with 169 additions and 105 deletions
@@ -419,6 +419,11 @@ class RoomProxyMock: RoomProxyProtocol {
        set(value) { underlyingMembersPublisher = value }
    }
    var underlyingMembersPublisher: AnyPublisher<[RoomMemberProxyProtocol], Never>!
+    var updatesPublisher: AnyPublisher<TimelineDiff, Never> {
+        get { return underlyingUpdatesPublisher }
+        set(value) { underlyingUpdatesPublisher = value }
+    }
+    var underlyingUpdatesPublisher: AnyPublisher<TimelineDiff, Never>!
    var invitedMembersCount: UInt {
        get { return underlyingInvitedMembersCount }
        set(value) { underlyingInvitedMembersCount = value }
@@ -477,39 +482,23 @@ class RoomProxyMock: RoomProxyProtocol {
            return loadDisplayNameForUserIdReturnValue
        }
    }
-    //MARK: - addTimelineListener
+    //MARK: - registerTimelineListenerIfNeeded

-    var addTimelineListenerListenerCallsCount = 0
-    var addTimelineListenerListenerCalled: Bool {
-        return addTimelineListenerListenerCallsCount > 0
+    var registerTimelineListenerIfNeededCallsCount = 0
+    var registerTimelineListenerIfNeededCalled: Bool {
+        return registerTimelineListenerIfNeededCallsCount > 0
    }
-    var addTimelineListenerListenerReceivedListener: TimelineListener?
-    var addTimelineListenerListenerReceivedInvocations: [TimelineListener] = []
-    var addTimelineListenerListenerReturnValue: Result<[TimelineItem], RoomProxyError>!
-    var addTimelineListenerListenerClosure: ((TimelineListener) -> Result<[TimelineItem], RoomProxyError>)?
+    var registerTimelineListenerIfNeededReturnValue: Result<[TimelineItem], RoomProxyError>!
+    var registerTimelineListenerIfNeededClosure: (() -> Result<[TimelineItem], RoomProxyError>)?

-    func addTimelineListener(listener: TimelineListener) -> Result<[TimelineItem], RoomProxyError> {
-        addTimelineListenerListenerCallsCount += 1
-        addTimelineListenerListenerReceivedListener = listener
-        addTimelineListenerListenerReceivedInvocations.append(listener)
-        if let addTimelineListenerListenerClosure = addTimelineListenerListenerClosure {
-            return addTimelineListenerListenerClosure(listener)
+    func registerTimelineListenerIfNeeded() -> Result<[TimelineItem], RoomProxyError> {
+        registerTimelineListenerIfNeededCallsCount += 1
+        if let registerTimelineListenerIfNeededClosure = registerTimelineListenerIfNeededClosure {
+            return registerTimelineListenerIfNeededClosure()
        } else {
-            return addTimelineListenerListenerReturnValue
+            return registerTimelineListenerIfNeededReturnValue
        }
    }
-    //MARK: - removeTimelineListener
-
-    var removeTimelineListenerCallsCount = 0
-    var removeTimelineListenerCalled: Bool {
-        return removeTimelineListenerCallsCount > 0
-    }
-    var removeTimelineListenerClosure: (() -> Void)?
-
-    func removeTimelineListener() {
-        removeTimelineListenerCallsCount += 1
-        removeTimelineListenerClosure?()
-    }
    //MARK: - paginateBackwards

    var paginateBackwardsRequestSizeUntilNumberOfItemsCallsCount = 0
@@ -73,5 +73,7 @@ extension RoomProxyMock {

        updateMembersClosure = { }
        acceptInvitationClosure = { .success(()) }
+        registerTimelineListenerIfNeededClosure = { .success([]) }
+        underlyingUpdatesPublisher = Empty(completeImmediately: false).eraseToAnyPublisher()
    }
 }
@@ -53,6 +53,9 @@ struct LoadableAvatarImage: View {
            } placeholder: {
                PlaceholderAvatarImage(name: name, contentID: contentID)
            }
+            // Binds the lifecycle of the LoadableImage to the associated URL.
+            // This fixes the problem of the cache returning old values after a change in the URL.
+            .id(url)
        } else {
            PlaceholderAvatarImage(name: name, contentID: contentID)
        }
@@ -36,7 +36,7 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
                                           canonicalAlias: roomProxy.canonicalAlias,
                                           isEncrypted: roomProxy.isEncrypted,
                                           isDirect: roomProxy.isDirect,
-                                           title: roomProxy.displayName ?? roomProxy.name ?? "Unknown Room",
+                                           title: roomProxy.roomTitle,
                                           topic: roomProxy.topic,
                                           avatarURL: roomProxy.avatarURL,
                                           permalink: roomProxy.permalink,
@@ -81,6 +81,13 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
    // MARK: - Private

    private func setupSubscriptions() {
+        switch roomProxy.registerTimelineListenerIfNeeded() {
+        case .success, .failure(.roomListenerAlreadyRegistered):
+            break
+        case .failure:
+            MXLog.error("Failed to register a room listener in room's details for the room \(roomProxy.id)")
+        }
+        
        roomProxy.membersPublisher
            .sink { [weak self] members in
                guard let self else { return }
@@ -102,6 +109,16 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
                }
            }
            .store(in: &cancellables)
+        
+        roomProxy.updatesPublisher
+            .throttle(for: .seconds(1), scheduler: DispatchQueue.main, latest: true)
+            .sink { [weak self] _ in
+                guard let self else { return }
+                self.state.title = self.roomProxy.roomTitle
+                self.state.topic = self.roomProxy.topic
+                self.state.avatarURL = self.roomProxy.avatarURL
+            }
+            .store(in: &cancellables)
    }
    
    private func buildMembersDetails(members: [RoomMemberProxyProtocol]) async -> RoomMembersDetails {
@@ -44,8 +44,7 @@ final class RoomScreenCoordinator: CoordinatorProtocol {
        
        viewModel = RoomScreenViewModel(timelineController: parameters.timelineController,
                                        mediaProvider: parameters.mediaProvider,
-                                        roomName: parameters.roomProxy.displayName ?? parameters.roomProxy.name,
-                                        roomAvatarUrl: parameters.roomProxy.avatarURL)
+                                        roomProxy: parameters.roomProxy)
    }
    
    // MARK: - Public
@@ -76,7 +75,6 @@ final class RoomScreenCoordinator: CoordinatorProtocol {
    }
    
    func stop() {
-        parameters.roomProxy.removeTimelineListener()
        viewModel.context.send(viewAction: .markRoomAsRead)
    }
    
@@ -27,17 +27,18 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
        static let toastErrorID = "RoomScreenToastError"
    }

+    private let roomProxy: RoomProxyProtocol
    private let timelineController: RoomTimelineControllerProtocol
    
    init(timelineController: RoomTimelineControllerProtocol,
         mediaProvider: MediaProviderProtocol,
-         roomName: String?,
-         roomAvatarUrl: URL? = nil) {
+         roomProxy: RoomProxyProtocol) {
+        self.roomProxy = roomProxy
        self.timelineController = timelineController
        
        super.init(initialViewState: RoomScreenViewState(roomId: timelineController.roomID,
-                                                         roomTitle: roomName ?? "Unknown room 💥",
-                                                         roomAvatarURL: roomAvatarUrl,
+                                                         roomTitle: roomProxy.roomTitle,
+                                                         roomAvatarURL: roomProxy.avatarURL,
                                                         timelineStyle: ServiceLocator.shared.settings.timelineStyle,
                                                         bindings: .init(composerText: "", composerFocused: false)),
                   imageProvider: mediaProvider)
@@ -70,6 +71,16 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
            return self.contextMenuActionsForItemId(itemId)
        }
        
+        roomProxy
+            .updatesPublisher
+            .throttle(for: .seconds(1), scheduler: DispatchQueue.main, latest: true)
+            .sink { [weak self] _ in
+                guard let self else { return }
+                self.state.roomTitle = roomProxy.roomTitle
+                self.state.roomAvatarURL = roomProxy.avatarURL
+            }
+            .store(in: &cancellables)
+        
        ServiceLocator.shared.settings.$timelineStyle
            .weakAssign(to: \.state.timelineStyle, on: self)
            .store(in: &cancellables)
@@ -406,5 +417,5 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
 extension RoomScreenViewModel {
    static let mock = RoomScreenViewModel(timelineController: MockRoomTimelineController(),
                                          mediaProvider: MockMediaProvider(),
-                                          roomName: "Preview room")
+                                          roomProxy: RoomProxyMock(with: .init(displayName: "Preview room")))
 }
@@ -59,8 +59,7 @@ struct RoomHeaderView_Previews: PreviewProvider {
    static var bodyPlain: some View {
        let viewModel = RoomScreenViewModel(timelineController: MockRoomTimelineController(),
                                            mediaProvider: MockMediaProvider(),
-                                            roomName: "Some Room name",
-                                            roomAvatarUrl: URL.picturesDirectory)
+                                            roomProxy: RoomProxyMock(with: .init(displayName: "Some Room name", avatarURL: URL.picturesDirectory)))

        RoomHeaderView(context: viewModel.context)
            .previewLayout(.sizeThatFits)
@@ -71,8 +70,7 @@ struct RoomHeaderView_Previews: PreviewProvider {
    static var bodyEncrypted: some View {
        let viewModel = RoomScreenViewModel(timelineController: MockRoomTimelineController(),
                                            mediaProvider: MockMediaProvider(),
-                                            roomName: "Some Room name",
-                                            roomAvatarUrl: nil)
+                                            roomProxy: RoomProxyMock(with: .init(displayName: "Some Room name")))

        RoomHeaderView(context: viewModel.context)
            .previewLayout(.sizeThatFits)
@@ -133,7 +133,7 @@ struct RoomScreen: View {
 struct RoomScreen_Previews: PreviewProvider {
    static let viewModel = RoomScreenViewModel(timelineController: MockRoomTimelineController(),
                                               mediaProvider: MockMediaProvider(),
-                                               roomName: "Preview room")
+                                               roomProxy: RoomProxyMock(with: .init(displayName: "Preview room")))
    
    static var previews: some View {
        NavigationStack {
@@ -84,7 +84,7 @@ struct TimelineView: UIViewControllerRepresentable {
 struct TimelineTableView_Previews: PreviewProvider {
    static let viewModel = RoomScreenViewModel(timelineController: MockRoomTimelineController(),
                                               mediaProvider: MockMediaProvider(),
-                                               roomName: "Preview room")
+                                               roomProxy: RoomProxyMock(with: .init(displayName: "Preview room")))
    
    static var previews: some View {
        NavigationStack {
@@ -41,7 +41,29 @@ class RoomProxy: RoomProxyProtocol {
    var membersPublisher: AnyPublisher<[RoomMemberProxyProtocol], Never> {
        membersSubject.eraseToAnyPublisher()
    }
-        
+    
+    private var timelineListener: RoomTimelineListener?
+    private let updatesSubject = PassthroughSubject<TimelineDiff, Never>()
+    var updatesPublisher: AnyPublisher<TimelineDiff, Never> {
+        updatesSubject.eraseToAnyPublisher()
+    }
+    
+    deinit {
+        Task { @MainActor [roomTimelineObservationToken, slidingSyncRoom] in
+            roomTimelineObservationToken?.cancel()
+            
+            let task = ExpiringTaskRunner {
+                let unsubscribeTask = slidingSyncRoom.unsubscribeFromRoom()
+                
+                while !unsubscribeTask.isFinished() {
+                    try await Task.sleep(for: .seconds(2))
+                }
+            }
+            
+            try? await task.run(timeout: .seconds(30))
+        }
+    }
+
    init(slidingSyncRoom: SlidingSyncRoomProtocol,
         room: RoomProtocol,
         backgroundTaskService: BackgroundTaskServiceProtocol) {
@@ -139,15 +161,27 @@ class RoomProxy: RoomProxyProtocol {
        }
    }
        
-    func addTimelineListener(listener: TimelineListener) -> Result<[TimelineItem], RoomProxyError> {
-        let settings = RoomSubscription(requiredState: [RequiredState(key: "m.room.topic", value: ""),
+    func registerTimelineListenerIfNeeded() -> Result<[TimelineItem], RoomProxyError> {
+        guard timelineListener == nil else {
+            return .failure(.roomListenerAlreadyRegistered)
+        }
+        
+        let settings = RoomSubscription(requiredState: [RequiredState(key: "m.room.name", value: ""),
+                                                        RequiredState(key: "m.room.topic", value: ""),
+                                                        RequiredState(key: "m.room.avatar", value: ""),
                                                        RequiredState(key: "m.room.canonical_alias", value: ""),
                                                        RequiredState(key: "m.room.join_rules", value: "")],
                                        timelineLimit: UInt32(SlidingSyncConstants.timelinePrecachingTimelineLimit))
        roomSubscriptionObservationToken = slidingSyncRoom.subscribeToRoom(settings: settings)
        
+        let listener = RoomTimelineListener { [weak self] timelineDiff in
+            self?.updatesSubject.send(timelineDiff)
+        }
+        
        if let result = try? slidingSyncRoom.addTimelineListener(listener: listener) {
            roomTimelineObservationToken = result.taskHandle
+            timelineListener = listener
+            
            Task {
                await fetchMembers()
                await updateMembers()
@@ -158,15 +192,6 @@ class RoomProxy: RoomProxyProtocol {
        }
    }
    
-    func removeTimelineListener() {
-        roomTimelineObservationToken?.cancel()
-        roomTimelineObservationToken = nil
-        
-        roomSubscriptionObservationToken = nil
-        
-        roomUnsubscriptionObservationToken = slidingSyncRoom.unsubscribeFromRoom()
-    }
-    
    func paginateBackwards(requestSize: UInt, untilNumberOfItems: UInt) async -> Result<Void, RoomProxyError> {
        do {
            try await Task.dispatch(on: .global()) {
@@ -487,3 +512,15 @@ class RoomProxy: RoomProxyProtocol {
        self.displayName = displayName
    }
 }
+
+private class RoomTimelineListener: TimelineListener {
+    private let onUpdateClosure: (TimelineDiff) -> Void
+   
+    init(_ onUpdateClosure: @escaping (TimelineDiff) -> Void) {
+        self.onUpdateClosure = onUpdateClosure
+    }
+    
+    func onUpdate(update: TimelineDiff) {
+        onUpdateClosure(update)
+    }
+}
@@ -20,6 +20,7 @@ import MatrixRustSDK

 enum RoomProxyError: Error {
    case noMoreMessagesToBackPaginate
+    case roomListenerAlreadyRegistered
    case failedPaginatingBackwards
    case failedRetrievingMemberAvatarURL
    case failedRetrievingMemberDisplayName
@@ -59,14 +60,19 @@ protocol RoomProxyProtocol {
    var avatarURL: URL? { get }

    var membersPublisher: AnyPublisher<[RoomMemberProxyProtocol], Never> { get }
+    
+    /// Publishes the room's updates.
+    /// The publisher starts publishing after the first call to `registerTimelineListenerIfNeeded()`
+    /// The thread on which this publisher sends the output isn't defined.
+    var updatesPublisher: AnyPublisher<TimelineDiff, Never> { get }

    func loadAvatarURLForUserId(_ userId: String) async -> Result<URL?, RoomProxyError>
    
    func loadDisplayNameForUserId(_ userId: String) async -> Result<String?, RoomProxyError>
    
-    func addTimelineListener(listener: TimelineListener) -> Result<[TimelineItem], RoomProxyError>
-    
-    func removeTimelineListener()
+    /// Registers a timeline listener if not registered already.
+    /// Updates for this object will be published on the `updatesPublisher` publisher.
+    func registerTimelineListenerIfNeeded() -> Result<[TimelineItem], RoomProxyError>
    
    func paginateBackwards(requestSize: UInt, untilNumberOfItems: UInt) async -> Result<Void, RoomProxyError>
    
@@ -130,4 +136,10 @@ extension RoomProxyProtocol {
    func sendMessage(_ message: String) async -> Result<Void, RoomProxyError> {
        await sendMessage(message, inReplyTo: nil)
    }
+    
+    // Avoids to duplicate the same logic around in the app
+    // Probably this should be done in rust.
+    var roomTitle: String {
+        displayName ?? name ?? "Unknown room 💥"
+    }
 }
@@ -18,14 +18,6 @@ import Combine
 import Foundation
 import MatrixRustSDK

-private class RoomTimelineListener: TimelineListener {
-    let itemsUpdatePublisher = PassthroughSubject<TimelineDiff, Never>()
-    
-    func onUpdate(update: TimelineDiff) {
-        itemsUpdatePublisher.send(update)
-    }
-}
-
 class RoomTimelineProvider: RoomTimelineProviderProtocol {
    private let roomProxy: RoomProxyProtocol
    private var cancellables = Set<AnyCancellable>()
@@ -48,21 +40,21 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol {
        serialDispatchQueue = DispatchQueue(label: "io.element.elementx.roomtimelineprovider", qos: .utility)
        itemProxies = []

-        Task {
-            let roomTimelineListener = RoomTimelineListener()
-            
-            roomTimelineListener
-                .itemsUpdatePublisher
+        Task { @MainActor in
+            roomProxy
+                .updatesPublisher
                .collect(.byTime(serialDispatchQueue, 0.1))
                .sink { [weak self] in self?.updateItemsWithDiffs($0) }
                .store(in: &cancellables)
            
-            switch await roomProxy.addTimelineListener(listener: roomTimelineListener) {
-            case .success(let items):
+            switch roomProxy.registerTimelineListenerIfNeeded() {
+            case let .success(items):
                itemProxies = items.map(TimelineItemProxy.init)
                MXLog.info("Added timeline listener, current items (\(items.count)) : \(items.map(\.debugIdentifier))")
+            case .failure(.roomListenerAlreadyRegistered):
+                MXLog.info("Listener already registered for the room: \(roomProxy.id)")
            case .failure:
-                let roomID = await roomProxy.id
+                let roomID = roomProxy.id
                MXLog.error("Failed adding timeline listener on room with identifier: \(roomID)")
            }
        }