vector-im/element-x-ios/issues/53 - Adopt MainActors, dispatch heavy operations do detached tasks and ensure combine publisher call back on the right queue

This commit is contained in:
Stefan Ceriu 2022-05-25 13:28:19 +03:00 committed by Stefan Ceriu
parent dde7786152
commit f5890b68a2
29 changed files with 100 additions and 90 deletions

View File

@ -19,6 +19,7 @@ import UIKit
/// Protocol describing a [Coordinator](http://khanlou.com/2015/10/coordinators-redux/). /// Protocol describing a [Coordinator](http://khanlou.com/2015/10/coordinators-redux/).
/// Coordinators are the objects which control the navigation flow of the application. /// Coordinators are the objects which control the navigation flow of the application.
/// It helps to isolate and reuse view controllers and pass dependencies down the navigation hierarchy. /// It helps to isolate and reuse view controllers and pass dependencies down the navigation hierarchy.
@MainActor
protocol Coordinator: AnyObject { protocol Coordinator: AnyObject {
/// Starts job of the coordinator. /// Starts job of the coordinator.

View File

@ -17,6 +17,7 @@
import UIKit import UIKit
/// `NavigationRouterStoreProtocol` describes a structure that enables to get a NavigationRouter from a UINavigationController instance. /// `NavigationRouterStoreProtocol` describes a structure that enables to get a NavigationRouter from a UINavigationController instance.
@MainActor
protocol NavigationRouterStoreProtocol { protocol NavigationRouterStoreProtocol {
/// Gets the existing navigation router for the supplied controller, creating a new one if it doesn't yet exist. /// Gets the existing navigation router for the supplied controller, creating a new one if it doesn't yet exist.

View File

@ -18,6 +18,7 @@ import UIKit
/// Protocol describing a router that wraps a UINavigationController and add convenient completion handlers. Completions are called when a Presentable is removed. /// Protocol describing a router that wraps a UINavigationController and add convenient completion handlers. Completions are called when a Presentable is removed.
/// Routers are used to be passed between coordinators. They handles only `physical` navigation. /// Routers are used to be passed between coordinators. They handles only `physical` navigation.
@MainActor
protocol NavigationRouterType: AnyObject, Presentable { protocol NavigationRouterType: AnyObject, Presentable {
/// Present modally a view controller on the navigation controller /// Present modally a view controller on the navigation controller

View File

@ -17,6 +17,7 @@
import UIKit import UIKit
/// Protocol used to pass UIViewControllers to routers /// Protocol used to pass UIViewControllers to routers
@MainActor
protocol Presentable { protocol Presentable {
func toPresentable() -> UIViewController func toPresentable() -> UIViewController
} }

View File

@ -18,6 +18,7 @@ import UIKit
/// Protocol describing a router that wraps the root navigation of the application. /// Protocol describing a router that wraps the root navigation of the application.
/// Routers are used to be passed between coordinators. They handles only `physical` navigation. /// Routers are used to be passed between coordinators. They handles only `physical` navigation.
@MainActor
protocol RootRouterType: AnyObject { protocol RootRouterType: AnyObject {
/// Update the root view controller /// Update the root view controller

View File

@ -30,6 +30,7 @@ import Combine
/// It provides a nice layer of consistency and also safety. As we are not passing the `ViewModel` to the view directly, shortcuts/hacks /// It provides a nice layer of consistency and also safety. As we are not passing the `ViewModel` to the view directly, shortcuts/hacks
/// can't be made into the `ViewModel`. /// can't be made into the `ViewModel`.
@dynamicMemberLookup @dynamicMemberLookup
@MainActor
class ViewModelContext<ViewState: BindableState, ViewAction>: ObservableObject { class ViewModelContext<ViewState: BindableState, ViewAction>: ObservableObject {
// MARK: - Properties // MARK: - Properties
@ -70,6 +71,7 @@ class ViewModelContext<ViewState: BindableState, ViewAction>: ObservableObject {
/// a specific portion of state that can be safely bound to. /// a specific portion of state that can be safely bound to.
/// If we decide to add more features to our state management (like doing state processing off the main thread) /// If we decide to add more features to our state management (like doing state processing off the main thread)
/// we can do it in this centralised place. /// we can do it in this centralised place.
@MainActor
class StateStoreViewModel<State: BindableState, ViewAction> { class StateStoreViewModel<State: BindableState, ViewAction> {
typealias Context = ViewModelContext<State, ViewAction> typealias Context = ViewModelContext<State, ViewAction>

View File

@ -85,19 +85,15 @@ final class HomeScreenCoordinator: Coordinator, Presentable {
Task { Task {
if case let .success(userAvatarURL) = await parameters.userSession.loadUserAvatarURL() { if case let .success(userAvatarURL) = await parameters.userSession.loadUserAvatarURL() {
if case let .success(avatar) = await parameters.mediaProvider.loadImageFromURL(userAvatarURL) { if case let .success(avatar) = await parameters.mediaProvider.loadImageFromURL(userAvatarURL) {
await MainActor.run {
self.viewModel.updateWithUserAvatar(avatar) self.viewModel.updateWithUserAvatar(avatar)
} }
} }
}
if case let .success(userDisplayName) = await parameters.userSession.loadUserDisplayName() { if case let .success(userDisplayName) = await parameters.userSession.loadUserDisplayName() {
await MainActor.run {
self.viewModel.updateWithUserDisplayName(userDisplayName) self.viewModel.updateWithUserDisplayName(userDisplayName)
} }
} }
} }
}
// MARK: - Public // MARK: - Public
func start() { func start() {

View File

@ -47,11 +47,11 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
override func process(viewAction: HomeScreenViewAction) { override func process(viewAction: HomeScreenViewAction) {
switch viewAction { switch viewAction {
case .logout: case .logout:
self.completion?(.logout) completion?(.logout)
case .loadRoomData(let roomIdentifier): case .loadRoomData(let roomIdentifier):
self.loadRoomDataForIdentifier(roomIdentifier) loadRoomDataForIdentifier(roomIdentifier)
case .selectRoom(let roomIdentifier): case .selectRoom(let roomIdentifier):
self.completion?(.selectRoom(roomIdentifier: roomIdentifier)) completion?(.selectRoom(roomIdentifier: roomIdentifier))
} }
} }

View File

@ -17,6 +17,7 @@
import Foundation import Foundation
import UIKit import UIKit
@MainActor
protocol HomeScreenViewModelProtocol { protocol HomeScreenViewModelProtocol {
var completion: ((HomeScreenViewModelResult) -> Void)? { get set } var completion: ((HomeScreenViewModelResult) -> Void)? { get set }

View File

@ -16,6 +16,7 @@
import Foundation import Foundation
@MainActor
protocol LoginScreenViewModelProtocol { protocol LoginScreenViewModelProtocol {
var completion: ((LoginScreenViewModelResult) -> Void)? { get set } var completion: ((LoginScreenViewModelResult) -> Void)? { get set }
var context: LoginScreenViewModelType.Context { get } var context: LoginScreenViewModelType.Context { get }

View File

@ -39,7 +39,7 @@ struct RoomScreenViewState: BindableState {
var isBackPaginating = false var isBackPaginating = false
var bindings: RoomScreenViewStateBindings var bindings: RoomScreenViewStateBindings
var contextMenuBuilder: ((_ itemId: String) -> TimelineItemContextMenu)? var contextMenuBuilder: (@MainActor (_ itemId: String) -> TimelineItemContextMenu)?
var sendButtonDisabled: Bool { var sendButtonDisabled: Bool {
bindings.composerText.count == 0 bindings.composerText.count == 0

View File

@ -67,16 +67,12 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
Task { Task {
switch viewAction { switch viewAction {
case .loadPreviousPage: case .loadPreviousPage:
await MainActor.run {
state.isBackPaginating = true state.isBackPaginating = true
}
switch await timelineController.paginateBackwards(Constants.backPaginationPageSize) { switch await timelineController.paginateBackwards(Constants.backPaginationPageSize) {
default: default:
await MainActor.run {
state.isBackPaginating = false state.isBackPaginating = false
} }
}
case .itemAppeared(let id): case .itemAppeared(let id):
await timelineController.processItemAppearance(id) await timelineController.processItemAppearance(id)
@ -90,13 +86,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
} }
await timelineController.sendMessage(state.bindings.composerText) await timelineController.sendMessage(state.bindings.composerText)
await MainActor.run {
state.bindings.composerText = "" state.bindings.composerText = ""
} }
} }
} }
}
// MARK: - Private // MARK: - Private

View File

@ -16,6 +16,7 @@
import Foundation import Foundation
@MainActor
protocol RoomScreenViewModelProtocol { protocol RoomScreenViewModelProtocol {
var context: RoomScreenViewModelType.Context { get } var context: RoomScreenViewModelType.Context { get }
} }

View File

@ -14,6 +14,7 @@ enum AuthenticationCoordinatorError: Error {
case failedSettingUpSession case failedSettingUpSession
} }
@MainActor
protocol AuthenticationCoordinatorDelegate: AnyObject { protocol AuthenticationCoordinatorDelegate: AnyObject {
func authenticationCoordinatorDidStartLoading(_ authenticationCoordinator: AuthenticationCoordinator) func authenticationCoordinatorDidStartLoading(_ authenticationCoordinator: AuthenticationCoordinator)

View File

@ -27,12 +27,13 @@ private class WeakUserSessionWrapper: ClientDelegate {
self.userSession = userSession self.userSession = userSession
} }
func didReceiveSyncUpdate() { @MainActor func didReceiveSyncUpdate() {
self.userSession?.didReceiveSyncUpdate() self.userSession?.didReceiveSyncUpdate()
} }
} }
class UserSession: ClientDelegate { @MainActor
class UserSession {
private let client: Client private let client: Client
@ -74,26 +75,26 @@ class UserSession: ClientDelegate {
} }
func loadUserDisplayName() async -> Result<String, UserSessionError> { func loadUserDisplayName() async -> Result<String, UserSessionError> {
await withCheckedContinuation { continuation in await Task.detached { () -> Result<String, UserSessionError> in
do { do {
let displayName = try self.client.displayName() let displayName = try self.client.displayName()
continuation.resume(returning: .success(displayName)) return .success(displayName)
} catch { } catch {
continuation.resume(returning: .failure(.failedRetrievingDisplayName)) return .failure(.failedRetrievingDisplayName)
} }
} }.value
} }
func loadUserAvatarURL() async -> Result<String, UserSessionError> { func loadUserAvatarURL() async -> Result<String, UserSessionError> {
await withCheckedContinuation { continuation in await Task.detached { () -> Result<String, UserSessionError> in
do { do {
let avatarURL = try self.client.avatarUrl() let avatarURL = try self.client.avatarUrl()
continuation.resume(returning: .success(avatarURL)) return .success(avatarURL)
} catch { } catch {
continuation.resume(returning: .failure(.failedRetrievingDisplayName)) return .failure(.failedRetrievingDisplayName)
}
} }
}.value
} }
// MARK: ClientDelegate // MARK: ClientDelegate
@ -101,8 +102,8 @@ class UserSession: ClientDelegate {
func didReceiveSyncUpdate() { func didReceiveSyncUpdate() {
Benchmark.logElapsedDurationForIdentifier("ClientSync", message: "Received sync update") Benchmark.logElapsedDurationForIdentifier("ClientSync", message: "Received sync update")
Task { Task.detached {
await updateRooms() await self.updateRooms()
} }
} }

View File

@ -46,33 +46,33 @@ struct MediaProvider: MediaProviderProtocol {
return .success(image) return .success(image)
} }
return await withCheckedContinuation { continuation in let cachedImageLoadResult = await withCheckedContinuation({ continuation in
imageCache.retrieveImage(forKey: source.underlyingSource.url()) { result in imageCache.retrieveImage(forKey: source.underlyingSource.url()) { result in
if case let .success(cacheResult) = result, continuation.resume(returning: result)
}
})
if case let .success(cacheResult) = cachedImageLoadResult,
let image = cacheResult.image { let image = cacheResult.image {
continuation.resume(returning: .success(image)) return .success(image)
return
} }
processingQueue.async { return await Task.detached { () -> Result<UIImage, MediaProviderError> in
do { do {
let imageData = try client.getMediaContent(source: source.underlyingSource) let imageData = try client.getMediaContent(source: source.underlyingSource)
guard let image = UIImage(data: Data(bytes: imageData, count: imageData.count)) else { guard let image = UIImage(data: Data(bytes: imageData, count: imageData.count)) else {
MXLog.error("Invalid image data") MXLog.error("Invalid image data")
continuation.resume(returning: .failure(.invalidImageData)) return .failure(.invalidImageData)
return
} }
imageCache.store(image, forKey: source.underlyingSource.url()) imageCache.store(image, forKey: source.underlyingSource.url())
continuation.resume(returning: .success(image)) return .success(image)
} catch { } catch {
MXLog.error("Failed retrieving image with error: \(error)") MXLog.error("Failed retrieving image with error: \(error)")
continuation.resume(returning: .failure(.failedRetrievingImage)) return .failure(.failedRetrievingImage)
}
}
}
} }
}.value
} }
} }

View File

@ -14,6 +14,7 @@ enum MediaProviderError: Error {
case invalidImageData case invalidImageData
} }
@MainActor
protocol MediaProviderProtocol { protocol MediaProviderProtocol {
func imageFromSource(_ source: MediaSource?) -> UIImage? func imageFromSource(_ source: MediaSource?) -> UIImage?

View File

@ -8,6 +8,7 @@
import Foundation import Foundation
@MainActor
class MemberDetailProviderManager { class MemberDetailProviderManager {
private var memberDetailProviders: [String: MemberDetailProviderProtocol] = [:] private var memberDetailProviders: [String: MemberDetailProviderProtocol] = [:]

View File

@ -14,6 +14,7 @@ enum MemberDetailProviderError: Error {
case failedRetrievingUserDisplayName case failedRetrievingUserDisplayName
} }
@MainActor
protocol MemberDetailProviderProtocol { protocol MemberDetailProviderProtocol {
func avatarURLForUserId(_ userId: String) -> String? func avatarURLForUserId(_ userId: String) -> String?
func loadAvatarURLForUserId(_ userId: String) async -> Result<String?, MemberDetailProviderError> func loadAvatarURLForUserId(_ userId: String) async -> Result<String?, MemberDetailProviderError>

View File

@ -44,10 +44,8 @@ class RoomProxy: RoomProxyProtocol {
room.setDelegate(delegate: WeakRoomProxyWrapper(roomProxy: self)) room.setDelegate(delegate: WeakRoomProxyWrapper(roomProxy: self))
Task {
backwardStream = room.startLiveEventListener() backwardStream = room.startLiveEventListener()
} }
}
var id: String { var id: String {
room.id() room.id()
@ -86,50 +84,48 @@ class RoomProxy: RoomProxyProtocol {
} }
func loadAvatarURLForUserId(_ userId: String) async -> Result<String?, RoomProxyError> { func loadAvatarURLForUserId(_ userId: String) async -> Result<String?, RoomProxyError> {
await withCheckedContinuation({ continuation in await Task.detached { () -> Result<String?, RoomProxyError> in
do { do {
let avatarURL = try self.room.memberAvatarUrl(userId: userId) let avatarURL = try self.room.memberAvatarUrl(userId: userId)
continuation.resume(returning: .success(avatarURL)) return .success(avatarURL)
} catch { } catch {
continuation.resume(returning: .failure(.failedRetrievingMemberAvatarURL)) return .failure(.failedRetrievingMemberAvatarURL)
} }
}) }.value
} }
func loadDisplayNameForUserId(_ userId: String) async -> Result<String?, RoomProxyError> { func loadDisplayNameForUserId(_ userId: String) async -> Result<String?, RoomProxyError> {
await withCheckedContinuation({ continuation in await Task.detached { () -> Result<String?, RoomProxyError> in
do { do {
let displayName = try self.room.memberDisplayName(userId: userId) let displayName = try self.room.memberDisplayName(userId: userId)
continuation.resume(returning: .success(displayName)) return .success(displayName)
} catch { } catch {
continuation.resume(returning: .failure(.failedRetrievingMemberDisplayName)) return .failure(.failedRetrievingMemberDisplayName)
} }
}) }.value
} }
func loadDisplayName() async -> Result<String, RoomProxyError> { func loadDisplayName() async -> Result<String, RoomProxyError> {
await withCheckedContinuation({ continuation in await Task.detached { () -> Result<String, RoomProxyError> in
if let displayName = displayName { if let displayName = self.displayName {
continuation.resume(returning: .success(displayName)) return .success(displayName)
return
} }
do { do {
let displayName = try self.room.displayName() let displayName = try self.room.displayName()
self.displayName = displayName self.displayName = displayName
continuation.resume(returning: .success(displayName)) return .success(displayName)
} catch { } catch {
continuation.resume(returning: .failure(.failedRetrievingDisplayName)) return .failure(.failedRetrievingDisplayName)
} }
}) }.value
} }
func paginateBackwards(count: UInt) async -> Result<Void, RoomProxyError> { func paginateBackwards(count: UInt) async -> Result<Void, RoomProxyError> {
await withCheckedContinuation { continuation in await Task.detached { () -> Result<Void, RoomProxyError> in
guard let backwardStream = self.backwardStream else { guard let backwardStream = self.backwardStream else {
continuation.resume(returning: .failure(.backwardStreamNotAvailable)) return .failure(RoomProxyError.backwardStreamNotAvailable)
return
} }
Benchmark.startTrackingForIdentifier("BackPagination \(self.id)", message: "Backpaginating \(count) message(s) in room \(self.id)") Benchmark.startTrackingForIdentifier("BackPagination \(self.id)", message: "Backpaginating \(count) message(s) in room \(self.id)")
@ -142,22 +138,22 @@ class RoomProxy: RoomProxyProtocol {
self.messages.insert(contentsOf: messages, at: 0) self.messages.insert(contentsOf: messages, at: 0)
continuation.resume(returning: .success(())) return .success(())
} }.value
} }
func sendMessage(_ message: String) async -> Result<Void, RoomProxyError> { func sendMessage(_ message: String) async -> Result<Void, RoomProxyError> {
let messageContent = messageEventContentFromMarkdown(md: message) let messageContent = messageEventContentFromMarkdown(md: message)
let transactionId = genTransactionId() let transactionId = genTransactionId()
return await withCheckedContinuation { continuation in return await Task(priority: .high) { () -> Result<Void, RoomProxyError> in
do { do {
try self.room.send(msg: messageContent, txnId: transactionId) try self.room.send(msg: messageContent, txnId: transactionId)
continuation.resume(returning: .success(())) return .success(())
} catch { } catch {
continuation.resume(returning: .failure(.failedSendingMessage)) return .failure(.failedSendingMessage)
}
} }
}.value
} }
// MARK: - Private // MARK: - Private

View File

@ -8,6 +8,7 @@
import Foundation import Foundation
@MainActor
protocol EventBriefFactoryProtocol { protocol EventBriefFactoryProtocol {
func eventBriefForMessage(_ message: RoomMessageProtocol?) async -> EventBrief? func eventBriefForMessage(_ message: RoomMessageProtocol?) async -> EventBrief?
} }

View File

@ -98,13 +98,13 @@ class RoomSummary: RoomSummaryProtocol {
} }
await withTaskGroup(of: Void.self) { group in await withTaskGroup(of: Void.self) { group in
group.addTask { group.addTask(priority: .medium) {
await self.loadDisplayName() await self.loadDisplayName()
} }
group.addTask { group.addTask(priority: .medium) {
await self.loadAvatar() await self.loadAvatar()
} }
group.addTask { group.addTask(priority: .medium) {
await self.loadLastMessage() await self.loadLastMessage()
} }
} }

View File

@ -13,6 +13,7 @@ enum RoomSummaryCallback {
case updatedData case updatedData
} }
@MainActor
protocol RoomSummaryProtocol { protocol RoomSummaryProtocol {
var id: String { get } var id: String { get }
var name: String? { get } var name: String? { get }

View File

@ -31,7 +31,10 @@ class RoomTimelineController: RoomTimelineControllerProtocol {
self.mediaProvider = mediaProvider self.mediaProvider = mediaProvider
self.memberDetailProvider = memberDetailProvider self.memberDetailProvider = memberDetailProvider
self.timelineProvider.callbacks.sink { [weak self] callback in self.timelineProvider
.callbacks
.receive(on: DispatchQueue.main)
.sink { [weak self] callback in
guard let self = self else { return } guard let self = self else { return }
switch callback { switch callback {

View File

@ -18,6 +18,7 @@ enum RoomTimelineControllerError: Error {
case generic case generic
} }
@MainActor
protocol RoomTimelineControllerProtocol { protocol RoomTimelineControllerProtocol {
var timelineItems: [RoomTimelineItemProtocol] { get } var timelineItems: [RoomTimelineItemProtocol] { get }
var callbacks: PassthroughSubject<RoomTimelineControllerCallback, Never> { get } var callbacks: PassthroughSubject<RoomTimelineControllerCallback, Never> { get }

View File

@ -18,6 +18,7 @@ enum RoomTimelineProviderError: Error {
case generic case generic
} }
@MainActor
protocol RoomTimelineProviderProtocol { protocol RoomTimelineProviderProtocol {
var callbacks: PassthroughSubject<RoomTimelineProviderCallback, Never> { get } var callbacks: PassthroughSubject<RoomTimelineProviderCallback, Never> { get }

View File

@ -9,6 +9,7 @@
import Foundation import Foundation
import UIKit import UIKit
@MainActor
struct RoomTimelineItemFactory { struct RoomTimelineItemFactory {
private let mediaProvider: MediaProviderProtocol private let mediaProvider: MediaProviderProtocol
private let memberDetailProvider: MemberDetailProviderProtocol private let memberDetailProvider: MemberDetailProviderProtocol

View File

@ -8,6 +8,7 @@
import Foundation import Foundation
@MainActor
struct RoomTimelineViewFactory { struct RoomTimelineViewFactory {
func buildTimelineViewFor(_ timelineItem: RoomTimelineItemProtocol) -> RoomTimelineViewProvider { func buildTimelineViewFor(_ timelineItem: RoomTimelineItemProtocol) -> RoomTimelineViewProvider {
switch timelineItem { switch timelineItem {

View File

@ -16,6 +16,7 @@
import Foundation import Foundation
@MainActor
protocol TemplateSimpleScreenViewModelProtocol { protocol TemplateSimpleScreenViewModelProtocol {
var completion: ((TemplateSimpleScreenViewModelResult) -> Void)? { get set } var completion: ((TemplateSimpleScreenViewModelResult) -> Void)? { get set }
var context: TemplateSimpleScreenViewModelType.Context { get } var context: TemplateSimpleScreenViewModelType.Context { get }