From f6a599a51babde1a71f6b803d8f30493dabe4685 Mon Sep 17 00:00:00 2001 From: ismailgulek Date: Wed, 2 Nov 2022 18:25:46 +0300 Subject: [PATCH] Concurrency Fixes (#283) * Review places where MainActor used * Adapt to MainActor changes, avoid redundant task detachments * Move Rust client operations into a dedicated concurrent queue, make init async * Add changelog * Fixed small working for awaiting non-async method * Remove some redundant tasks * Put back some main actors * Fix tests * Fixed timeline updates when the number of items doesn't change. Keeps more previous home screen room data between reloads Co-authored-by: Stefan Ceriu --- .../ViewModel/StateStoreViewModel.swift | 13 +- .../HomeScreen/HomeScreenViewModel.swift | 81 +++++----- .../RoomScreen/View/TimelineItemList.swift | 5 + .../AuthenticationServiceProxyProtocol.swift | 1 - .../BackgroundTaskServiceProtocol.swift | 1 + .../Sources/Services/Client/ClientProxy.swift | 140 ++++++++++-------- .../Services/Client/ClientProxyProtocol.swift | 3 +- .../Services/Client/MockClientProxy.swift | 2 +- .../Services/Media/MediaProvider.swift | 55 ++++--- .../Media/MediaProviderProtocol.swift | 1 - .../Sources/Services/Media/MediaSource.swift | 4 + .../RoomSummary/RoomSummaryProvider.swift | 54 ++++--- .../Session/UserSessionProtocol.swift | 1 - .../Timeline/RoomTimelineProvider.swift | 4 +- .../UserSessionStore/UserSessionStore.swift | 2 +- .../UserSessionStoreProtocol.swift | 1 - .../UserSessionFlowCoordinator.swift | 88 +++++------ UnitTests/Sources/BackgroundTaskTests.swift | 1 + changelog.d/pr-283.change | 1 + 19 files changed, 240 insertions(+), 218 deletions(-) create mode 100644 changelog.d/pr-283.change diff --git a/ElementX/Sources/Other/SwiftUI/ViewModel/StateStoreViewModel.swift b/ElementX/Sources/Other/SwiftUI/ViewModel/StateStoreViewModel.swift index f2bf1f332f..b932eb2d59 100644 --- a/ElementX/Sources/Other/SwiftUI/ViewModel/StateStoreViewModel.swift +++ b/ElementX/Sources/Other/SwiftUI/ViewModel/StateStoreViewModel.swift @@ -96,12 +96,13 @@ class StateStoreViewModel { init(initialViewState: State) { context = Context(initialViewState: initialViewState) - context.viewActions.sink { [weak self] action in - guard let self else { return } - - Task { await self.process(viewAction: action) } - } - .store(in: &cancellables) + context.viewActions + .sink { [weak self] action in + guard let self else { return } + + Task { await self.process(viewAction: action) } + } + .store(in: &cancellables) } /// Override to handles incoming `ViewAction`s from the `ViewModel`. diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index 63634759e9..57e1776d79 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -23,6 +23,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol private let userSession: UserSessionProtocol private let roomSummaryProvider: RoomSummaryProviderProtocol private let attributedStringBuilder: AttributedStringBuilderProtocol + private var roomsForIdentifiers = [String: HomeScreenRoom]() var callback: ((HomeScreenViewModelAction) -> Void)? @@ -72,9 +73,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol roomSummaryProvider.roomListPublisher .receive(on: DispatchQueue.main) .sink { [weak self] _ in - Task { - await self?.updateRooms() - } + self?.updateRooms() } .store(in: &cancellables) @@ -91,10 +90,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol state.userDisplayName = userDisplayName } } - - Task { - await updateRooms() - } + + updateRooms() } // MARK: - Public @@ -117,52 +114,58 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol // MARK: - Private private func loadDataForRoomIdentifier(_ identifier: String) { - guard let summary = roomSummaryProvider.roomListPublisher.value.first(where: { $0.asFilled?.id == identifier })?.asFilled, - let homeRoomIndex = state.rooms.firstIndex(where: { $0.id == identifier }) else { + guard let roomSummary = roomSummaryProvider.roomListPublisher.value.first(where: { $0.asFilled?.id == identifier })?.asFilled, + let roomIndex = state.rooms.firstIndex(where: { $0.id == identifier }) else { return } - var details = state.rooms[homeRoomIndex] + var room = state.rooms[roomIndex] - guard details.avatar == nil, - let avatarURLString = summary.avatarURLString else { + guard room.avatar == nil, + let avatarURLString = roomSummary.avatarURLString else { return } Task { if case let .success(image) = await userSession.mediaProvider.loadImageFromURLString(avatarURLString, avatarSize: .room(on: .home)) { - details.avatar = image - state.rooms[homeRoomIndex] = details + room.avatar = image + state.rooms[roomIndex] = room + roomsForIdentifiers[roomSummary.id] = room } } } - private func updateRooms() async { - state.rooms = await Task.detached { - var rooms = [HomeScreenRoom]() - - for summary in self.roomSummaryProvider.roomListPublisher.value { - switch summary { - case .empty(let id): - rooms.append(HomeScreenRoom.placeholder(id: id)) - case .filled(let summary): - let avatarImage = await self.userSession.mediaProvider.imageFromURLString(summary.avatarURLString, avatarSize: .room(on: .home)) - - var timestamp: String? - if let lastMessageTimestamp = summary.lastMessageTimestamp { - timestamp = lastMessageTimestamp.formatted(date: .omitted, time: .shortened) - } - - rooms.append(HomeScreenRoom(id: summary.id, - name: summary.name, - hasUnreads: summary.unreadNotificationCount > 0, - timestamp: timestamp, - lastMessage: summary.lastMessage, - avatar: avatarImage)) + private func updateRooms() { + var rooms = [HomeScreenRoom]() + var newRoomsForIdentifiers = [String: HomeScreenRoom]() + + for summary in roomSummaryProvider.roomListPublisher.value { + switch summary { + case .empty(let id): + rooms.append(HomeScreenRoom.placeholder(id: id)) + case .filled(let summary): + let oldRoom = roomsForIdentifiers[summary.id] + + let avatarImage = userSession.mediaProvider.imageFromURLString(summary.avatarURLString, avatarSize: .room(on: .home)) + + var timestamp: String? + if let lastMessageTimestamp = summary.lastMessageTimestamp { + timestamp = lastMessageTimestamp.formatted(date: .omitted, time: .shortened) } + + let room = HomeScreenRoom(id: summary.id, + name: summary.name, + hasUnreads: summary.unreadNotificationCount > 0, + timestamp: timestamp ?? oldRoom?.timestamp, + lastMessage: summary.lastMessage ?? oldRoom?.lastMessage, + avatar: avatarImage ?? oldRoom?.avatar) + + rooms.append(room) + newRoomsForIdentifiers[summary.id] = room } - - return rooms - }.value + } + + state.rooms = rooms + roomsForIdentifiers = newRoomsForIdentifiers } } diff --git a/ElementX/Sources/Screens/RoomScreen/View/TimelineItemList.swift b/ElementX/Sources/Screens/RoomScreen/View/TimelineItemList.swift index 480daef577..ba78affbe0 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/TimelineItemList.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/TimelineItemList.swift @@ -152,6 +152,11 @@ struct TimelineItemList: View { // Otherwise just update the items timelineItems = context.viewState.items } + .onChange(of: context.viewState.items, perform: { items in + if timelineItems != items { + timelineItems = items + } + }) .background(GeometryReader { geo in Color.clear.preference(key: ViewFramePreferenceKey.self, value: [geo.frame(in: .global)]) }) diff --git a/ElementX/Sources/Services/Authentication/AuthenticationServiceProxyProtocol.swift b/ElementX/Sources/Services/Authentication/AuthenticationServiceProxyProtocol.swift index 8c4570a251..b57ec10692 100644 --- a/ElementX/Sources/Services/Authentication/AuthenticationServiceProxyProtocol.swift +++ b/ElementX/Sources/Services/Authentication/AuthenticationServiceProxyProtocol.swift @@ -27,7 +27,6 @@ enum AuthenticationServiceError: Error { case failedLoggingIn } -@MainActor protocol AuthenticationServiceProxyProtocol { var homeserver: LoginHomeserver { get } diff --git a/ElementX/Sources/Services/Background/BackgroundTaskServiceProtocol.swift b/ElementX/Sources/Services/Background/BackgroundTaskServiceProtocol.swift index aa194eddc7..568fcee14c 100644 --- a/ElementX/Sources/Services/Background/BackgroundTaskServiceProtocol.swift +++ b/ElementX/Sources/Services/Background/BackgroundTaskServiceProtocol.swift @@ -16,6 +16,7 @@ import Foundation +@MainActor protocol BackgroundTaskServiceProtocol { func startBackgroundTask(withName name: String, isReusable: Bool, diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index 5aaed4577e..7ec910a0f8 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -31,23 +31,17 @@ private class WeakClientProxyWrapper: ClientDelegate, SlidingSyncObserver { func didReceiveSyncUpdate() { } func didReceiveAuthError(isSoftLogout: Bool) { - Task { - await clientProxy?.didReceiveAuthError(isSoftLogout: isSoftLogout) - } + clientProxy?.didReceiveAuthError(isSoftLogout: isSoftLogout) } func didUpdateRestoreToken() { - Task { - await clientProxy?.didUpdateRestoreToken() - } + clientProxy?.didUpdateRestoreToken() } // MARK: - SlidingSyncDelegate func didReceiveSyncUpdate(summary: UpdateSummary) { - Task { - await self.clientProxy?.didReceiveSlidingSyncUpdate(summary: summary) - } + clientProxy?.didReceiveSlidingSyncUpdate(summary: summary) } } @@ -58,11 +52,18 @@ class ClientProxy: ClientProxyProtocol { private let client: ClientProtocol private let backgroundTaskService: BackgroundTaskServiceProtocol private var sessionVerificationControllerProxy: SessionVerificationControllerProxy? + private let clientQueue: DispatchQueue private var slidingSyncObserverToken: StoppableSpawn? - private let slidingSync: SlidingSync + private var slidingSync: SlidingSync! - let roomSummaryProvider: RoomSummaryProviderProtocol + var roomSummaryProviderInternal: RoomSummaryProviderProtocol! + var roomSummaryProvider: RoomSummaryProviderProtocol { + guard let roomSummaryProviderInternal else { + fatalError("There is an issue with ClientProxy object initialization") + } + return roomSummaryProviderInternal + } deinit { // These need to be inlined instead of using stopSync() @@ -75,33 +76,37 @@ class ClientProxy: ClientProxyProtocol { let callbacks = PassthroughSubject() init(client: ClientProtocol, - backgroundTaskService: BackgroundTaskServiceProtocol) { + backgroundTaskService: BackgroundTaskServiceProtocol) async { self.client = client self.backgroundTaskService = backgroundTaskService - - do { - let slidingSyncBuilder = try client.slidingSync().homeserver(url: BuildSettings.slidingSyncProxyBaseURL.absoluteString) - - let slidingSyncView = try SlidingSyncViewBuilder() - .timelineLimit(limit: 10) - .requiredState(requiredState: [RequiredState(key: "m.room.avatar", value: ""), - RequiredState(key: "m.room.encryption", value: "")]) - .name(name: "HomeScreenView") - .syncMode(mode: .fullSync) - .build() - - slidingSync = try slidingSyncBuilder - .addView(view: slidingSyncView) - .withCommonExtensions() - .build() - - roomSummaryProvider = RoomSummaryProvider(slidingSyncController: slidingSync, - slidingSyncView: slidingSyncView, - roomMessageFactory: RoomMessageFactory()) - } catch { - fatalError("Failed configuring sliding sync") + clientQueue = .init(label: "ClientProxyQueue", + attributes: .concurrent) + + await Task.dispatch(on: clientQueue) { + do { + let slidingSyncBuilder = try client.slidingSync().homeserver(url: BuildSettings.slidingSyncProxyBaseURL.absoluteString) + + let slidingSyncView = try SlidingSyncViewBuilder() + .timelineLimit(limit: 10) + .requiredState(requiredState: [RequiredState(key: "m.room.avatar", value: ""), + RequiredState(key: "m.room.encryption", value: "")]) + .name(name: "HomeScreenView") + .syncMode(mode: .fullSync) + .build() + + self.slidingSync = try slidingSyncBuilder + .addView(view: slidingSyncView) + .withCommonExtensions() + .build() + + self.roomSummaryProviderInternal = RoomSummaryProvider(slidingSyncController: self.slidingSync, + slidingSyncView: slidingSyncView, + roomMessageFactory: RoomMessageFactory()) + } catch { + fatalError("Failed configuring sliding sync") + } } - + client.setDelegate(delegate: WeakClientProxyWrapper(clientProxy: self)) } @@ -156,27 +161,22 @@ class ClientProxy: ClientProxyProtocol { slidingSync.setObserver(observer: nil) } - func roomForIdentifier(_ identifier: String) -> RoomProxyProtocol? { - do { - guard let slidingSyncRoom = try slidingSync.getRoom(roomId: identifier), - let room = slidingSyncRoom.fullRoom() else { - MXLog.error("Failed retrieving room with identifier: \(identifier)") - return nil - } - - let roomProxy = RoomProxy(slidingSyncRoom: slidingSyncRoom, - room: room, - backgroundTaskService: backgroundTaskService) - - return roomProxy - } catch { - MXLog.error("Failed retrieving room with identifier: \(identifier)") + func roomForIdentifier(_ identifier: String) async -> RoomProxyProtocol? { + let (slidingSyncRoom, room) = await Task.dispatch(on: clientQueue) { + self.roomTupleForIdentifier(identifier) + } + + guard let slidingSyncRoom, let room else { return nil } + + return await RoomProxy(slidingSyncRoom: slidingSyncRoom, + room: room, + backgroundTaskService: backgroundTaskService) } func loadUserDisplayName() async -> Result { - await Task.dispatch(on: .global()) { + await Task.dispatch(on: clientQueue) { do { let displayName = try self.client.displayName() return .success(displayName) @@ -187,7 +187,7 @@ class ClientProxy: ClientProxyProtocol { } func loadUserAvatarURLString() async -> Result { - await Task.dispatch(on: .global()) { + await Task.dispatch(on: clientQueue) { do { let avatarURL = try self.client.avatarUrl() return .success(avatarURL) @@ -198,11 +198,15 @@ class ClientProxy: ClientProxyProtocol { } func accountDataEvent(type: String) async -> Result where Content: Decodable { - .failure(.failedRetrievingAccountData) + await Task.dispatch(on: clientQueue) { + .failure(.failedRetrievingAccountData) + } } func setAccountData(content: Content, type: String) async -> Result { - .failure(.failedSettingAccountData) + await Task.dispatch(on: clientQueue) { + .failure(.failedSettingAccountData) + } } func mediaSourceForURLString(_ urlString: String) -> MatrixRustSDK.MediaSource { @@ -210,21 +214,21 @@ class ClientProxy: ClientProxyProtocol { } func loadMediaContentForSource(_ source: MatrixRustSDK.MediaSource) async throws -> Data { - try await Task.dispatch(on: .global()) { + try await Task.dispatch(on: clientQueue) { let bytes = try self.client.getMediaContent(source: source) return Data(bytes: bytes, count: bytes.count) } } func loadMediaThumbnailForSource(_ source: MatrixRustSDK.MediaSource, width: UInt, height: UInt) async throws -> Data { - try await Task.dispatch(on: .global()) { + try await Task.dispatch(on: clientQueue) { let bytes = try self.client.getMediaThumbnail(source: source, width: UInt64(width), height: UInt64(height)) return Data(bytes: bytes, count: bytes.count) } } func sessionVerificationControllerProxy() async -> Result { - await Task.dispatch(on: .global()) { + await Task.dispatch(on: clientQueue) { do { let sessionVerificationController = try self.client.getSessionVerificationController() return .success(SessionVerificationControllerProxy(sessionVerificationController: sessionVerificationController)) @@ -235,14 +239,28 @@ class ClientProxy: ClientProxyProtocol { } func logout() async { - do { - try client.logout() - } catch { - MXLog.error("Failed logging out with error: \(error)") + await Task.dispatch(on: clientQueue) { + do { + try self.client.logout() + } catch { + MXLog.error("Failed logging out with error: \(error)") + } } } // MARK: Private + + private func roomTupleForIdentifier(_ identifier: String) -> (SlidingSyncRoom?, Room?) { + do { + let slidingSyncRoom = try slidingSync.getRoom(roomId: identifier) + let fullRoom = slidingSyncRoom?.fullRoom() + + return (slidingSyncRoom, fullRoom) + } catch { + MXLog.error("Failed retrieving room with identifier: \(identifier)") + return (nil, nil) + } + } fileprivate func didReceiveAuthError(isSoftLogout: Bool) { callbacks.send(.receivedAuthError(isSoftLogout: isSoftLogout)) diff --git a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift index 12c55b867d..30f694c20e 100644 --- a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift +++ b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift @@ -33,7 +33,6 @@ enum ClientProxyError: Error { case failedLoadingMedia } -@MainActor protocol ClientProxyProtocol { var callbacks: PassthroughSubject { get } @@ -53,7 +52,7 @@ protocol ClientProxyProtocol { func stopSync() - func roomForIdentifier(_ identifier: String) -> RoomProxyProtocol? + func roomForIdentifier(_ identifier: String) async -> RoomProxyProtocol? func loadUserDisplayName() async -> Result diff --git a/ElementX/Sources/Services/Client/MockClientProxy.swift b/ElementX/Sources/Services/Client/MockClientProxy.swift index 41a8b6d56e..d13e445ac4 100644 --- a/ElementX/Sources/Services/Client/MockClientProxy.swift +++ b/ElementX/Sources/Services/Client/MockClientProxy.swift @@ -32,7 +32,7 @@ struct MockClientProxy: ClientProxyProtocol { func stopSync() { } - func roomForIdentifier(_ identifier: String) -> RoomProxyProtocol? { + func roomForIdentifier(_ identifier: String) async -> RoomProxyProtocol? { nil } diff --git a/ElementX/Sources/Services/Media/MediaProvider.swift b/ElementX/Sources/Services/Media/MediaProvider.swift index 69c6bfffa4..71b723a521 100644 --- a/ElementX/Sources/Services/Media/MediaProvider.swift +++ b/ElementX/Sources/Services/Media/MediaProvider.swift @@ -55,43 +55,38 @@ struct MediaProvider: MediaProviderProtocol { return .success(image) } - let loadImageBgTask = backgroundTaskService.startBackgroundTask(withName: "LoadImage: \(source.underlyingSource.url().hashValue)") + let loadImageBgTask = await backgroundTaskService.startBackgroundTask(withName: "LoadImage: \(source.url.hashValue)") defer { loadImageBgTask?.stop() } - let cacheKey = cacheKeyForURLString(source.underlyingSource.url(), avatarSize: avatarSize) - - return await Task.detached { () -> Result in - if case let .success(cacheResult) = await imageCache.retrieveImage(forKey: cacheKey), - let image = cacheResult.image { - return .success(image) + let cacheKey = cacheKeyForURLString(source.url, avatarSize: avatarSize) + + if case let .success(cacheResult) = await imageCache.retrieveImage(forKey: cacheKey), + let image = cacheResult.image { + return .success(image) + } + + do { + let imageData: Data + if let avatarSize { + imageData = try await clientProxy.loadMediaThumbnailForSource(source.underlyingSource, width: UInt(avatarSize.scaledValue), height: UInt(avatarSize.scaledValue)) + } else { + imageData = try await clientProxy.loadMediaContentForSource(source.underlyingSource) } - - do { - let imageData = try await Task.detached { () -> Data in - if let avatarSize { - return try await clientProxy.loadMediaThumbnailForSource(source.underlyingSource, width: UInt(avatarSize.scaledValue), height: UInt(avatarSize.scaledValue)) - } else { - return try await clientProxy.loadMediaContentForSource(source.underlyingSource) - } - - }.value - - guard let image = UIImage(data: imageData) else { - MXLog.error("Invalid image data") - return .failure(.invalidImageData) - } - - imageCache.store(image, forKey: cacheKey) - - return .success(image) - } catch { - MXLog.error("Failed retrieving image with error: \(error)") - return .failure(.failedRetrievingImage) + + guard let image = UIImage(data: imageData) else { + MXLog.error("Invalid image data") + return .failure(.invalidImageData) } + + imageCache.store(image, forKey: cacheKey) + + return .success(image) + } catch { + MXLog.error("Failed retrieving image with error: \(error)") + return .failure(.failedRetrievingImage) } - .value } // MARK: - Private diff --git a/ElementX/Sources/Services/Media/MediaProviderProtocol.swift b/ElementX/Sources/Services/Media/MediaProviderProtocol.swift index cad940ab29..bd9d3d84eb 100644 --- a/ElementX/Sources/Services/Media/MediaProviderProtocol.swift +++ b/ElementX/Sources/Services/Media/MediaProviderProtocol.swift @@ -22,7 +22,6 @@ enum MediaProviderError: Error { case invalidImageData } -@MainActor protocol MediaProviderProtocol { func imageFromSource(_ source: MediaSource?, avatarSize: AvatarSize?) -> UIImage? diff --git a/ElementX/Sources/Services/Media/MediaSource.swift b/ElementX/Sources/Services/Media/MediaSource.swift index d03dd68f55..7220433e7e 100644 --- a/ElementX/Sources/Services/Media/MediaSource.swift +++ b/ElementX/Sources/Services/Media/MediaSource.swift @@ -19,6 +19,10 @@ import MatrixRustSDK struct MediaSource: Equatable { let underlyingSource: MatrixRustSDK.MediaSource + + var url: String { + underlyingSource.url() + } init(source: MatrixRustSDK.MediaSource) { underlyingSource = source diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index f3f7c948d1..cc1bfad09b 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -101,36 +101,34 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { } func updateRoomsWithIdentifiers(_ identifiers: [String]) { - Task.detached { - guard self.statePublisher.value == .live else { - return - } - - var changes = [CollectionDifference.Change]() - for identifier in identifiers { - guard let index = self.rooms.firstIndex(where: { $0.id == identifier }) else { - continue - } - - let oldRoom = self.rooms[index] - let newRoom = self.buildRoomSummaryForIdentifier(identifier) - - changes.append(.remove(offset: index, element: oldRoom, associatedWith: nil)) - changes.append(.insert(offset: index, element: newRoom, associatedWith: nil)) - } - - guard let diff = CollectionDifference(changes) else { - MXLog.error("Failed creating diff from changes: \(changes)") - return - } - - guard let newSummaries = self.rooms.applying(diff) else { - MXLog.error("Failed applying diff: \(diff)") - return + guard statePublisher.value == .live else { + return + } + + var changes = [CollectionDifference.Change]() + for identifier in identifiers { + guard let index = rooms.firstIndex(where: { $0.id == identifier }) else { + continue } - - self.rooms = newSummaries + + let oldRoom = rooms[index] + let newRoom = buildRoomSummaryForIdentifier(identifier) + + changes.append(.remove(offset: index, element: oldRoom, associatedWith: nil)) + changes.append(.insert(offset: index, element: newRoom, associatedWith: nil)) } + + guard let diff = CollectionDifference(changes) else { + MXLog.error("Failed creating diff from changes: \(changes)") + return + } + + guard let newSummaries = rooms.applying(diff) else { + MXLog.error("Failed applying diff: \(diff)") + return + } + + rooms = newSummaries } // MARK: - Private diff --git a/ElementX/Sources/Services/Session/UserSessionProtocol.swift b/ElementX/Sources/Services/Session/UserSessionProtocol.swift index 94f84bbf1a..da32bc626f 100644 --- a/ElementX/Sources/Services/Session/UserSessionProtocol.swift +++ b/ElementX/Sources/Services/Session/UserSessionProtocol.swift @@ -24,7 +24,6 @@ enum UserSessionCallback { case updateRestoreTokenNeeded } -@MainActor protocol UserSessionProtocol { var userID: String { get } var isSoftLogout: Bool { get } diff --git a/ElementX/Sources/Services/Timeline/RoomTimelineProvider.swift b/ElementX/Sources/Services/Timeline/RoomTimelineProvider.swift index 8c3de24729..7c6f077dad 100644 --- a/ElementX/Sources/Services/Timeline/RoomTimelineProvider.swift +++ b/ElementX/Sources/Services/Timeline/RoomTimelineProvider.swift @@ -45,11 +45,11 @@ class RoomTimelineProvider: RoomTimelineProviderProtocol { init(roomProxy: RoomProxyProtocol) { self.roomProxy = roomProxy itemProxies = [] - + Task { let roomTimelineListener = RoomTimelineListener() await roomProxy.addTimelineListener(listener: roomTimelineListener) - + roomTimelineListener .itemsUpdatePublisher .collect(.byTime(DispatchQueue.global(qos: .background), 0.5)) diff --git a/ElementX/Sources/Services/UserSessionStore/UserSessionStore.swift b/ElementX/Sources/Services/UserSessionStore/UserSessionStore.swift index a4b0a164fe..02af03d812 100644 --- a/ElementX/Sources/Services/UserSessionStore/UserSessionStore.swift +++ b/ElementX/Sources/Services/UserSessionStore/UserSessionStore.swift @@ -135,7 +135,7 @@ class UserSessionStore: UserSessionStoreProtocol { return .failure(.failedSettingUpSession) } - let clientProxy = ClientProxy(client: client, backgroundTaskService: backgroundTaskService) + let clientProxy = await ClientProxy(client: client, backgroundTaskService: backgroundTaskService) return .success(clientProxy) } diff --git a/ElementX/Sources/Services/UserSessionStore/UserSessionStoreProtocol.swift b/ElementX/Sources/Services/UserSessionStore/UserSessionStoreProtocol.swift index 89b8a452a9..f6facad681 100644 --- a/ElementX/Sources/Services/UserSessionStore/UserSessionStoreProtocol.swift +++ b/ElementX/Sources/Services/UserSessionStore/UserSessionStoreProtocol.swift @@ -24,7 +24,6 @@ enum UserSessionStoreError: Error { case failedRefreshingRestoreToken } -@MainActor protocol UserSessionStoreProtocol { /// Whether or not there are sessions in the store. var hasSessions: Bool { get } diff --git a/ElementX/Sources/UserSession/UserSessionFlowCoordinator.swift b/ElementX/Sources/UserSession/UserSessionFlowCoordinator.swift index 3e2b97af0f..2cbe3918f7 100644 --- a/ElementX/Sources/UserSession/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/UserSession/UserSessionFlowCoordinator.swift @@ -139,34 +139,36 @@ class UserSessionFlowCoordinator: Coordinator { // MARK: Rooms private func presentRoomWithIdentifier(_ roomIdentifier: String) { - guard let roomProxy = userSession.clientProxy.roomForIdentifier(roomIdentifier) else { - MXLog.error("Invalid room identifier: \(roomIdentifier)") - return - } - let userId = userSession.clientProxy.userIdentifier - - let timelineItemFactory = RoomTimelineItemFactory(userID: userId, - mediaProvider: userSession.mediaProvider, - roomProxy: roomProxy, - attributedStringBuilder: AttributedStringBuilder()) - - let timelineController = RoomTimelineController(userId: userId, - roomId: roomIdentifier, - timelineProvider: RoomTimelineProvider(roomProxy: roomProxy), - timelineItemFactory: timelineItemFactory, - mediaProvider: userSession.mediaProvider, - roomProxy: roomProxy) + Task { @MainActor in + guard let roomProxy = await userSession.clientProxy.roomForIdentifier(roomIdentifier) else { + MXLog.error("Invalid room identifier: \(roomIdentifier)") + return + } + let userId = userSession.clientProxy.userIdentifier - let parameters = RoomScreenCoordinatorParameters(timelineController: timelineController, - mediaProvider: userSession.mediaProvider, - roomName: roomProxy.displayName ?? roomProxy.name, - roomAvatarUrl: roomProxy.avatarURL) - let coordinator = RoomScreenCoordinator(parameters: parameters) + let timelineItemFactory = RoomTimelineItemFactory(userID: userId, + mediaProvider: userSession.mediaProvider, + roomProxy: roomProxy, + attributedStringBuilder: AttributedStringBuilder()) - add(childCoordinator: coordinator) - navigationRouter.push(coordinator) { [weak self] in - guard let self else { return } - self.stateMachine.processEvent(.dismissedRoomScreen) + let timelineController = RoomTimelineController(userId: userId, + roomId: roomIdentifier, + timelineProvider: RoomTimelineProvider(roomProxy: roomProxy), + timelineItemFactory: timelineItemFactory, + mediaProvider: userSession.mediaProvider, + roomProxy: roomProxy) + + let parameters = RoomScreenCoordinatorParameters(timelineController: timelineController, + mediaProvider: userSession.mediaProvider, + roomName: roomProxy.displayName ?? roomProxy.name, + roomAvatarUrl: roomProxy.avatarURL) + let coordinator = RoomScreenCoordinator(parameters: parameters) + + add(childCoordinator: coordinator) + navigationRouter.push(coordinator) { [weak self] in + guard let self else { return } + self.stateMachine.processEvent(.dismissedRoomScreen) + } } } @@ -222,25 +224,23 @@ class UserSessionFlowCoordinator: Coordinator { // MARK: Session verification private func presentSessionVerification() { - Task { - guard let sessionVerificationController = userSession.sessionVerificationController else { - fatalError("The sessionVerificationController should aways be valid at this point") - } - - let parameters = SessionVerificationCoordinatorParameters(sessionVerificationControllerProxy: sessionVerificationController) - - let coordinator = SessionVerificationCoordinator(parameters: parameters) - - coordinator.callback = { [weak self] in - self?.navigationRouter.dismissModule() - self?.stateMachine.processEvent(.dismissedSessionVerificationScreen) - } - - add(childCoordinator: coordinator) - navigationRouter.present(coordinator) - - coordinator.start() + guard let sessionVerificationController = userSession.sessionVerificationController else { + fatalError("The sessionVerificationController should aways be valid at this point") } + + let parameters = SessionVerificationCoordinatorParameters(sessionVerificationControllerProxy: sessionVerificationController) + + let coordinator = SessionVerificationCoordinator(parameters: parameters) + + coordinator.callback = { [weak self] in + self?.navigationRouter.dismissModule() + self?.stateMachine.processEvent(.dismissedSessionVerificationScreen) + } + + add(childCoordinator: coordinator) + navigationRouter.present(coordinator) + + coordinator.start() } private func tearDownDismissedSessionVerificationScreen() { diff --git a/UnitTests/Sources/BackgroundTaskTests.swift b/UnitTests/Sources/BackgroundTaskTests.swift index 403dbb0844..073bf07734 100644 --- a/UnitTests/Sources/BackgroundTaskTests.swift +++ b/UnitTests/Sources/BackgroundTaskTests.swift @@ -18,6 +18,7 @@ import XCTest @testable import ElementX +@MainActor class BackgroundTaskTests: XCTestCase { private enum Constants { static let bgTaskName = "test" diff --git a/changelog.d/pr-283.change b/changelog.d/pr-283.change new file mode 100644 index 0000000000..4d27b2ec9e --- /dev/null +++ b/changelog.d/pr-283.change @@ -0,0 +1 @@ +Move Rust client operations into a dedicated concurrent queue, make sure not used on main thread.