From e46165119fefc8169f6543279320926166cf19e0 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Mon, 14 Jul 2025 12:20:41 +0100 Subject: [PATCH] Use the timeline when marking a room as read (#4319) * Forward JoinedRoomProxy.markAsRead to the timeline The room version was building a fresh timeline even though we already had one. * Run swiftformat 0.57.0. --- .../Mocks/Generated/GeneratedMocks.swift | 70 +++++++++++++++++++ .../MediaPickerScreen/DocumentPicker.swift | 2 +- .../PhotoLibraryPicker.swift | 2 +- .../Services/Room/JoinedRoomProxy.swift | 16 +++-- .../Services/Room/RoomProxyProtocol.swift | 1 + .../Services/Timeline/TimelineProxy.swift | 13 ++++ .../Timeline/TimelineProxyProtocol.swift | 1 + UnitTests/Sources/RestorationTokenTests.swift | 4 +- 8 files changed, 98 insertions(+), 11 deletions(-) diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index 042f7f85a..c5d75192a 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -17520,6 +17520,76 @@ class TimelineProxyMock: TimelineProxyProtocol, @unchecked Sendable { return sendReadReceiptForTypeReturnValue } } + //MARK: - markAsRead + + var markAsReadReceiptTypeUnderlyingCallsCount = 0 + var markAsReadReceiptTypeCallsCount: Int { + get { + if Thread.isMainThread { + return markAsReadReceiptTypeUnderlyingCallsCount + } else { + var returnValue: Int? = nil + DispatchQueue.main.sync { + returnValue = markAsReadReceiptTypeUnderlyingCallsCount + } + + return returnValue! + } + } + set { + if Thread.isMainThread { + markAsReadReceiptTypeUnderlyingCallsCount = newValue + } else { + DispatchQueue.main.sync { + markAsReadReceiptTypeUnderlyingCallsCount = newValue + } + } + } + } + var markAsReadReceiptTypeCalled: Bool { + return markAsReadReceiptTypeCallsCount > 0 + } + var markAsReadReceiptTypeReceivedReceiptType: ReceiptType? + var markAsReadReceiptTypeReceivedInvocations: [ReceiptType] = [] + + var markAsReadReceiptTypeUnderlyingReturnValue: Result! + var markAsReadReceiptTypeReturnValue: Result! { + get { + if Thread.isMainThread { + return markAsReadReceiptTypeUnderlyingReturnValue + } else { + var returnValue: Result? = nil + DispatchQueue.main.sync { + returnValue = markAsReadReceiptTypeUnderlyingReturnValue + } + + return returnValue! + } + } + set { + if Thread.isMainThread { + markAsReadReceiptTypeUnderlyingReturnValue = newValue + } else { + DispatchQueue.main.sync { + markAsReadReceiptTypeUnderlyingReturnValue = newValue + } + } + } + } + var markAsReadReceiptTypeClosure: ((ReceiptType) async -> Result)? + + func markAsRead(receiptType: ReceiptType) async -> Result { + markAsReadReceiptTypeCallsCount += 1 + markAsReadReceiptTypeReceivedReceiptType = receiptType + DispatchQueue.main.async { + self.markAsReadReceiptTypeReceivedInvocations.append(receiptType) + } + if let markAsReadReceiptTypeClosure = markAsReadReceiptTypeClosure { + return await markAsReadReceiptTypeClosure(receiptType) + } else { + return markAsReadReceiptTypeReturnValue + } + } //MARK: - sendMessageEventContent var sendMessageEventContentUnderlyingCallsCount = 0 diff --git a/ElementX/Sources/Screens/MediaPickerScreen/DocumentPicker.swift b/ElementX/Sources/Screens/MediaPickerScreen/DocumentPicker.swift index 045292503..a50f3a256 100644 --- a/ElementX/Sources/Screens/MediaPickerScreen/DocumentPicker.swift +++ b/ElementX/Sources/Screens/MediaPickerScreen/DocumentPicker.swift @@ -69,7 +69,7 @@ struct DocumentPicker: UIViewControllerRepresentable { } do { - let _ = url.startAccessingSecurityScopedResource() + _ = url.startAccessingSecurityScopedResource() let newURL = try FileManager.default.copyFileToTemporaryDirectory(file: url) url.stopAccessingSecurityScopedResource() documentPicker.callback(.selectFile(newURL)) diff --git a/ElementX/Sources/Screens/MediaPickerScreen/PhotoLibraryPicker.swift b/ElementX/Sources/Screens/MediaPickerScreen/PhotoLibraryPicker.swift index a6c7d4f42..1f6726b5c 100644 --- a/ElementX/Sources/Screens/MediaPickerScreen/PhotoLibraryPicker.swift +++ b/ElementX/Sources/Screens/MediaPickerScreen/PhotoLibraryPicker.swift @@ -78,7 +78,7 @@ struct PhotoLibraryPicker: UIViewControllerRepresentable { } do { - let _ = url.startAccessingSecurityScopedResource() + _ = url.startAccessingSecurityScopedResource() let newURL = try FileManager.default.copyFileToTemporaryDirectory(file: url) url.stopAccessingSecurityScopedResource() diff --git a/ElementX/Sources/Services/Room/JoinedRoomProxy.swift b/ElementX/Sources/Services/Room/JoinedRoomProxy.swift index 6f61fec26..062f5da44 100644 --- a/ElementX/Sources/Services/Room/JoinedRoomProxy.swift +++ b/ElementX/Sources/Services/Room/JoinedRoomProxy.swift @@ -402,14 +402,16 @@ class JoinedRoomProxy: JoinedRoomProxyProtocol { return .failure(.sdkError(error)) } } - + func markAsRead(receiptType: ReceiptType) async -> Result { - do { - try await room.markAsRead(receiptType: receiptType) - return .success(()) - } catch { - MXLog.error("Failed marking room \(id) as read with error: \(error)") - return .failure(.sdkError(error)) + // Defer to the timeline here as room.markAsRead will build a fresh timeline. + switch await timeline.markAsRead(receiptType: receiptType) { + case .success: + .success(()) + case .failure(.sdkError(let error)): + .failure(.sdkError(error)) + case .failure(let error): + .failure(.timelineError(error)) } } diff --git a/ElementX/Sources/Services/Room/RoomProxyProtocol.swift b/ElementX/Sources/Services/Room/RoomProxyProtocol.swift index bae679dd3..df3a95b43 100644 --- a/ElementX/Sources/Services/Room/RoomProxyProtocol.swift +++ b/ElementX/Sources/Services/Room/RoomProxyProtocol.swift @@ -17,6 +17,7 @@ enum RoomProxyError: Error { case eventNotFound case missingTransactionID case failedCreatingPinnedTimeline + case timelineError(TimelineProxyError) } /// An enum that describes the relationship between the current user and the room, and contains a reference to the specific implementation of the `RoomProxy`. diff --git a/ElementX/Sources/Services/Timeline/TimelineProxy.swift b/ElementX/Sources/Services/Timeline/TimelineProxy.swift index 3b1543155..ea4d9b550 100644 --- a/ElementX/Sources/Services/Timeline/TimelineProxy.swift +++ b/ElementX/Sources/Services/Timeline/TimelineProxy.swift @@ -514,6 +514,19 @@ final class TimelineProxy: TimelineProxyProtocol { } } + func markAsRead(receiptType: ReceiptType) async -> Result { + MXLog.info("Marking as \(receiptType)") + + do { + try await timeline.markAsRead(receiptType: receiptType) + MXLog.info("Finished marking as read") + return .success(()) + } catch { + MXLog.error("Failed marking as \(receiptType) with error: \(error)") + return .failure(.sdkError(error)) + } + } + func toggleReaction(_ reaction: String, to eventOrTransactionID: TimelineItemIdentifier.EventOrTransactionID) async -> Result { MXLog.info("Toggling reaction \(reaction) for event: \(eventOrTransactionID)") diff --git a/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift b/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift index 5176d6047..fa24e8781 100644 --- a/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift +++ b/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift @@ -119,6 +119,7 @@ protocol TimelineProxyProtocol { requestHandle: @MainActor (SendAttachmentJoinHandleProtocol) -> Void) async -> Result func sendReadReceipt(for eventID: String, type: ReceiptType) async -> Result + func markAsRead(receiptType: ReceiptType) async -> Result func sendMessageEventContent(_ messageContent: RoomMessageEventContentWithoutRelation) async -> Result diff --git a/UnitTests/Sources/RestorationTokenTests.swift b/UnitTests/Sources/RestorationTokenTests.swift index 95fb2b943..c9281ad9a 100644 --- a/UnitTests/Sources/RestorationTokenTests.swift +++ b/UnitTests/Sources/RestorationTokenTests.swift @@ -182,7 +182,7 @@ enum SlidingSyncVersionV1: Equatable { } extension SessionV1: Codable { - public init(from decoder: Decoder) throws { + init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) let slidingSyncProxy = try container.decodeIfPresent(String.self, forKey: .slidingSyncProxy) self = try .init(accessToken: container.decode(String.self, forKey: .accessToken), @@ -194,7 +194,7 @@ extension SessionV1: Codable { slidingSyncVersion: slidingSyncProxy.map { .proxy(url: $0) } ?? .native) } - public func encode(to encoder: Encoder) throws { + func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(accessToken, forKey: .accessToken) try container.encode(refreshToken, forKey: .refreshToken)