Skip to content
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

Rename completion to callback and simplify actor usage. #6141

Merged
merged 1 commit into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 2 additions & 4 deletions Riot/Modules/Onboarding/AuthenticationCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ final class AuthenticationCoordinator: NSObject, AuthenticationCoordinatorProtoc
let parameters = AuthenticationServerSelectionCoordinatorParameters(authenticationService: authenticationService,
hasModalPresentation: false)
let coordinator = AuthenticationServerSelectionCoordinator(parameters: parameters)
coordinator.completion = { [weak self, weak coordinator] result in
coordinator.callback = { [weak self, weak coordinator] result in
guard let self = self, let coordinator = coordinator else { return }
self.serverSelectionCoordinator(coordinator, didCompleteWith: result)
}
Expand Down Expand Up @@ -168,7 +168,7 @@ final class AuthenticationCoordinator: NSObject, AuthenticationCoordinatorProtoc
registrationFlow: homeserver.registrationFlow,
loginMode: homeserver.preferredLoginMode)
let coordinator = AuthenticationRegistrationCoordinator(parameters: parameters)
coordinator.completion = { [weak self, weak coordinator] result in
coordinator.callback = { [weak self, weak coordinator] result in
guard let self = self, let coordinator = coordinator else { return }
self.registrationCoordinator(coordinator, didCompleteWith: result)
}
Expand All @@ -189,8 +189,6 @@ final class AuthenticationCoordinator: NSObject, AuthenticationCoordinatorProtoc
@MainActor private func registrationCoordinator(_ coordinator: AuthenticationRegistrationCoordinator,
didCompleteWith result: AuthenticationRegistrationCoordinatorResult) {
switch result {
case .selectServer:
showServerSelectionScreen()
case .completed(let result):
handleRegistrationResult(result)
}
Expand Down
2 changes: 1 addition & 1 deletion Riot/Modules/Onboarding/OnboardingCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ final class OnboardingCoordinator: NSObject, OnboardingCoordinatorProtocol {
let parameters = OnboardingAvatarCoordinatorParameters(userSession: userSession, avatar: selectedAvatar)
let coordinator = OnboardingAvatarCoordinator(parameters: parameters)

coordinator.completion = { [weak self, weak coordinator] result in
coordinator.callback = { [weak self, weak coordinator] result in
guard let self = self, let coordinator = coordinator else { return }

switch result {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class AuthenticationRegistrationViewModel: AuthenticationRegistrationViewModelTy

// MARK: Public

@MainActor var completion: ((AuthenticationRegistrationViewModelResult) -> Void)?
@MainActor var callback: ((AuthenticationRegistrationViewModelResult) -> Void)?

// MARK: - Setup

Expand All @@ -44,25 +44,19 @@ class AuthenticationRegistrationViewModel: AuthenticationRegistrationViewModelTy
// MARK: - Public

override func process(viewAction: AuthenticationRegistrationViewAction) {
Task {
await MainActor.run {
switch viewAction {
case .selectServer:
completion?(.selectServer)
case .validateUsername:
state.hasEditedUsername = true
completion?(.validateUsername(state.bindings.username))
case .enablePasswordValidation:
state.hasEditedPassword = true
case .clearUsernameError:
guard state.usernameErrorMessage != nil else { return }
state.usernameErrorMessage = nil
case .next:
completion?(.createAccount(username: state.bindings.username, password: state.bindings.password))
case .continueWithSSO(let id):
break
}
}
switch viewAction {
case .selectServer:
Task { await callback?(.selectServer) }
case .validateUsername:
Task { await validateUsername() }
case .enablePasswordValidation:
Task { await enablePasswordValidation() }
case .clearUsernameError:
Task { await clearUsernameError() }
case .next:
Task { await callback?(.createAccount(username: state.bindings.username, password: state.bindings.password)) }
case .continueWithSSO(let id):
break
}
}

Expand Down Expand Up @@ -92,4 +86,27 @@ class AuthenticationRegistrationViewModel: AuthenticationRegistrationViewModelTy
state.bindings.alertInfo = AlertInfo(id: type)
}
}

// MARK: - Private

/// Validate the supplied username with the homeserver.
@MainActor private func validateUsername() {
if !state.hasEditedUsername {
state.hasEditedUsername = true
}

callback?(.validateUsername(state.bindings.username))
}

/// Allows password validation to take place.
@MainActor private func enablePasswordValidation() {
guard !state.hasEditedPassword else { return }
state.hasEditedPassword = true
}

/// Clear any errors being shown in the username text field footer.
@MainActor private func clearUsernameError() {
guard state.usernameErrorMessage != nil else { return }
state.usernameErrorMessage = nil
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import Foundation

protocol AuthenticationRegistrationViewModelProtocol {

@MainActor var completion: ((AuthenticationRegistrationViewModelResult) -> Void)? { get set }
@MainActor var callback: ((AuthenticationRegistrationViewModelResult) -> Void)? { get set }
var context: AuthenticationRegistrationViewModelType.Context { get }

/// Update the view with new homeserver information.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ struct AuthenticationRegistrationCoordinatorParameters {
}

enum AuthenticationRegistrationCoordinatorResult {
/// The user would like to select another server.
case selectServer
/// The screen completed with the associated registration result.
case completed(RegistrationResult)
}
Expand Down Expand Up @@ -63,7 +61,7 @@ final class AuthenticationRegistrationCoordinator: Coordinator, Presentable {

// Must be used only internally
var childCoordinators: [Coordinator] = []
@MainActor var completion: ((AuthenticationRegistrationCoordinatorResult) -> Void)?
@MainActor var callback: ((AuthenticationRegistrationCoordinatorResult) -> Void)?

// MARK: - Setup

Expand All @@ -87,23 +85,8 @@ final class AuthenticationRegistrationCoordinator: Coordinator, Presentable {

// MARK: - Public
func start() {
Task {
await MainActor.run {
MXLog.debug("[AuthenticationRegistrationCoordinator] did start.")
authenticationRegistrationViewModel.completion = { [weak self] result in
guard let self = self else { return }
MXLog.debug("[AuthenticationRegistrationCoordinator] AuthenticationRegistrationViewModel did complete with result: \(result).")
switch result {
case .selectServer:
self.presentServerSelectionScreen()
case.validateUsername(let username):
self.validateUsername(username)
case .createAccount(let username, let password):
self.createAccount(username: username, password: password)
}
}
}
}
MXLog.debug("[AuthenticationRegistrationCoordinator] did start.")
Task { await setupViewModel() }
}

func toPresentable() -> UIViewController {
Expand All @@ -112,6 +95,22 @@ final class AuthenticationRegistrationCoordinator: Coordinator, Presentable {

// MARK: - Private

/// Set up the view model. This method is extracted from `start()` so it can run on the `MainActor`.
@MainActor private func setupViewModel() {
authenticationRegistrationViewModel.callback = { [weak self] result in
guard let self = self else { return }
MXLog.debug("[AuthenticationRegistrationCoordinator] AuthenticationRegistrationViewModel did complete with result: \(result).")
switch result {
case .selectServer:
self.presentServerSelectionScreen()
case.validateUsername(let username):
self.validateUsername(username)
case .createAccount(let username, let password):
self.createAccount(username: username, password: password)
}
}
}

/// Show a blocking activity indicator whilst saving.
@MainActor private func startLoading(label: String? = nil) {
waitingIndicator = indicatorPresenter.present(.loading(label: label ?? VectorL10n.loading, isInteractionBlocking: true))
Expand Down Expand Up @@ -160,7 +159,7 @@ final class AuthenticationRegistrationCoordinator: Coordinator, Presentable {
let result = try await registrationWizard.createAccount(username: username, password: password, initialDeviceDisplayName: deviceDisplayName)

guard !Task.isCancelled else { return }
completion?(.completed(result))
callback?(.completed(result))

self?.stopLoading()
} catch {
Expand Down Expand Up @@ -211,7 +210,7 @@ final class AuthenticationRegistrationCoordinator: Coordinator, Presentable {
let parameters = AuthenticationServerSelectionCoordinatorParameters(authenticationService: authenticationService,
hasModalPresentation: true)
let coordinator = AuthenticationServerSelectionCoordinator(parameters: parameters)
coordinator.completion = { [weak self, weak coordinator] result in
coordinator.callback = { [weak self, weak coordinator] result in
guard let self = self, let coordinator = coordinator else { return }
self.serverSelectionCoordinator(coordinator, didCompleteWith: result)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class AuthenticationServerSelectionViewModel: AuthenticationServerSelectionViewM

// MARK: Public

var completion: ((AuthenticationServerSelectionViewModelResult) -> Void)?
@MainActor var callback: ((AuthenticationServerSelectionViewModelResult) -> Void)?

// MARK: - Setup

Expand All @@ -41,20 +41,15 @@ class AuthenticationServerSelectionViewModel: AuthenticationServerSelectionViewM
// MARK: - Public

override func process(viewAction: AuthenticationServerSelectionViewAction) {
Task {
await MainActor.run {
switch viewAction {
case .confirm:
completion?(.confirm(homeserverAddress: state.bindings.homeserverAddress))
case .dismiss:
completion?(.dismiss)
case .getInTouch:
getInTouch()
case .clearFooterError:
guard state.footerErrorMessage != nil else { return }
withAnimation { state.footerErrorMessage = nil }
}
}
switch viewAction {
case .confirm:
Task { await callback?(.confirm(homeserverAddress: state.bindings.homeserverAddress)) }
case .dismiss:
Task { await callback?(.dismiss) }
case .getInTouch:
Task { await getInTouch() }
case .clearFooterError:
Task { await clearFooterError() }
}
}

Expand All @@ -71,6 +66,12 @@ class AuthenticationServerSelectionViewModel: AuthenticationServerSelectionViewM

// MARK: - Private

/// Clear any errors shown in the text field footer.
@MainActor private func clearFooterError() {
guard state.footerErrorMessage != nil else { return }
withAnimation { state.footerErrorMessage = nil }
}

/// Opens the EMS link in the user's browser.
@MainActor private func getInTouch() {
let url = BuildSettings.onboardingHostYourOwnServerLink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import Foundation

protocol AuthenticationServerSelectionViewModelProtocol {

@MainActor var completion: ((AuthenticationServerSelectionViewModelResult) -> Void)? { get set }
@MainActor var callback: ((AuthenticationServerSelectionViewModelResult) -> Void)? { get set }
var context: AuthenticationServerSelectionViewModelType.Context { get }

/// Displays an error to the user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ final class AuthenticationServerSelectionCoordinator: Coordinator, Presentable {

// Must be used only internally
var childCoordinators: [Coordinator] = []
@MainActor var completion: ((AuthenticationServerSelectionCoordinatorResult) -> Void)?
@MainActor var callback: ((AuthenticationServerSelectionCoordinatorResult) -> Void)?

// MARK: - Setup

Expand All @@ -70,22 +70,8 @@ final class AuthenticationServerSelectionCoordinator: Coordinator, Presentable {
// MARK: - Public

func start() {
Task {
await MainActor.run {
MXLog.debug("[AuthenticationServerSelectionCoordinator] did start.")
authenticationServerSelectionViewModel.completion = { [weak self] result in
guard let self = self else { return }
MXLog.debug("[AuthenticationServerSelectionCoordinator] AuthenticationServerSelectionViewModel did complete with result: \(result).")

switch result {
case .confirm(let homeserverAddress):
self.useHomeserver(homeserverAddress)
case .dismiss:
self.completion?(.dismiss)
}
}
}
}
MXLog.debug("[AuthenticationServerSelectionCoordinator] did start.")
Task { await setupViewModel() }
}

func toPresentable() -> UIViewController {
Expand All @@ -94,6 +80,21 @@ final class AuthenticationServerSelectionCoordinator: Coordinator, Presentable {

// MARK: - Private

/// Set up the view model. This method is extracted from `start()` so it can run on the `MainActor`.
@MainActor private func setupViewModel() {
authenticationServerSelectionViewModel.callback = { [weak self] result in
guard let self = self else { return }
MXLog.debug("[AuthenticationServerSelectionCoordinator] AuthenticationServerSelectionViewModel did complete with result: \(result).")

switch result {
case .confirm(let homeserverAddress):
self.useHomeserver(homeserverAddress)
case .dismiss:
self.callback?(.dismiss)
}
}
}

/// Show an activity indicator whilst loading.
/// - Parameters:
/// - label: The label to show on the indicator.
Expand All @@ -120,7 +121,7 @@ final class AuthenticationServerSelectionCoordinator: Coordinator, Presentable {
try await authenticationService.startFlow(.register, for: homeserverAddress)
stopLoading()

completion?(.updated)
callback?(.updated)
} catch {
stopLoading()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ struct OnboardingAvatarCoordinatorParameters {
}

enum OnboardingAvatarCoordinatorResult {
/// The user has chosen an image (but hasn't yet saved it).
/// The user has chosen an image (but it won't be uploaded until `.complete` is sent).
/// This result is to cache the image in the flow coordinator so it can be restored if the user was to navigate backwards.
case selectedAvatar(UIImage?)
/// The screen is finished and the next one can be shown.
case complete(UserSession)
Expand Down Expand Up @@ -63,7 +64,7 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {

// Must be used only internally
var childCoordinators: [Coordinator] = []
var completion: ((OnboardingAvatarCoordinatorResult) -> Void)?
var callback: ((OnboardingAvatarCoordinatorResult) -> Void)?

// MARK: - Setup

Expand All @@ -88,7 +89,7 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {

func start() {
MXLog.debug("[OnboardingAvatarCoordinator] did start.")
onboardingAvatarViewModel.completion = { [weak self] result in
onboardingAvatarViewModel.callback = { [weak self] result in
guard let self = self else { return }
MXLog.debug("[OnboardingAvatarCoordinator] OnboardingAvatarViewModel did complete with result: \(result).")
switch result {
Expand All @@ -99,7 +100,7 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {
case .save(let avatar):
self.setAvatar(avatar)
case .skip:
self.completion?(.complete(self.parameters.userSession))
self.callback?(.complete(self.parameters.userSession))
}
}
}
Expand Down Expand Up @@ -161,7 +162,7 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {
self.parameters.userSession.account.setUserAvatarUrl(urlString) { [weak self] in
guard let self = self else { return }
self.stopWaiting()
self.completion?(.complete(self.parameters.userSession))
self.callback?(.complete(self.parameters.userSession))
} failure: { [weak self] error in
guard let self = self else { return }
self.stopWaiting()
Expand All @@ -182,7 +183,7 @@ extension OnboardingAvatarCoordinator: MediaPickerPresenterDelegate {
/// so whilst this method may not appear to be called, everything works fine when run on a device.
func mediaPickerPresenter(_ presenter: MediaPickerPresenter, didPickImage image: UIImage) {
onboardingAvatarViewModel.updateAvatarImage(with: image)
completion?(.selectedAvatar(image))
callback?(.selectedAvatar(image))
presenter.dismiss(animated: true, completion: nil)
}

Expand All @@ -196,7 +197,7 @@ extension OnboardingAvatarCoordinator: MediaPickerPresenterDelegate {
extension OnboardingAvatarCoordinator: CameraPresenterDelegate {
func cameraPresenter(_ presenter: CameraPresenter, didSelectImage image: UIImage) {
onboardingAvatarViewModel.updateAvatarImage(with: image)
completion?(.selectedAvatar(image))
callback?(.selectedAvatar(image))
presenter.dismiss(animated: true, completion: nil)
}

Expand Down
Loading