Soft logout removal (#672)

* Remove isSoftLogout from everywhere except the client delegate
- bump the RustSDK to v1.0.43
- cleanup the app coordinator state machine and login/logout flows

* Merge `succeededSigningIn` and `succeededRestoringSession` into `createdUserSession` in order to simplify the logic
This commit is contained in:
Stefan Ceriu
2023-03-06 14:58:50 +02:00
committed by GitHub
parent 1fe2e7e882
commit e4117e3205
13 changed files with 34 additions and 122 deletions
@@ -95,8 +95,8 @@
"kind" : "remoteSourceControl", "kind" : "remoteSourceControl",
"location" : "https://github.com/matrix-org/matrix-rust-components-swift", "location" : "https://github.com/matrix-org/matrix-rust-components-swift",
"state" : { "state" : {
"revision" : "852bb356fbd2b5053f17b3c6c27c8c098cf5f055", "revision" : "32645162ea39badc586d610d2ada6bbc52f398ba",
"version" : "1.0.42-alpha" "version" : "1.0.43-alpha"
} }
}, },
{ {
@@ -154,7 +154,6 @@ class AppCoordinator: AppCoordinatorProtocol {
userSessionStore.reset() userSessionStore.reset()
} }
// swiftlint:disable:next cyclomatic_complexity
private func setupStateMachine() { private func setupStateMachine() {
stateMachine.addTransitionHandler { [weak self] context in stateMachine.addTransitionHandler { [weak self] context in
guard let self else { return } guard let self else { return }
@@ -162,25 +161,19 @@ class AppCoordinator: AppCoordinatorProtocol {
switch (context.fromState, context.event, context.toState) { switch (context.fromState, context.event, context.toState) {
case (.initial, .startWithAuthentication, .signedOut): case (.initial, .startWithAuthentication, .signedOut):
self.startAuthentication() self.startAuthentication()
case (.signedOut, .succeededSigningIn, .signedIn): case (.signedOut, .createdUserSession, .signedIn):
self.setupUserSession() self.setupUserSession()
case (.initial, .startWithExistingSession, .restoringSession): case (.initial, .startWithExistingSession, .restoringSession):
self.restoreUserSession() self.restoreUserSession()
case (.restoringSession, .failedRestoringSession, .signedOut): case (.restoringSession, .failedRestoringSession, .signedOut):
self.showLoginErrorToast() self.showLoginErrorToast()
self.presentSplashScreen() self.presentSplashScreen()
case (.restoringSession, .succeededRestoringSession, .signedIn): case (.restoringSession, .createdUserSession, .signedIn):
self.setupUserSession() self.setupUserSession()
case (_, .signOut, .signingOut): case (_, .signOut(let isSoft), .signingOut):
self.logout(isSoftLogout: false) self.logout(isSoft: isSoft)
case (.signingOut, .completedSigningOut, .signedOut): case (.signingOut, .completedSigningOut(let isSoft), .signedOut):
self.tearDownUserSession() self.presentSplashScreen(isSoftLogout: isSoft)
self.presentSplashScreen()
case (_, .remoteSignOut(let isSoftLogout), .remoteSigningOut):
self.logout(isSoftLogout: isSoftLogout)
case (.remoteSigningOut(let isSoftLogout), .completedSigningOut, .signedOut):
self.tearDownUserSession()
self.presentSplashScreen(isSoftLogout: isSoftLogout)
default: default:
fatalError("Unknown transition: \(context)") fatalError("Unknown transition: \(context)")
} }
@@ -196,11 +189,7 @@ class AppCoordinator: AppCoordinatorProtocol {
switch await userSessionStore.restoreUserSession() { switch await userSessionStore.restoreUserSession() {
case .success(let userSession): case .success(let userSession):
self.userSession = userSession self.userSession = userSession
if userSession.isSoftLogout { stateMachine.processEvent(.createdUserSession)
stateMachine.processEvent(.remoteSignOut(isSoft: true))
} else {
stateMachine.processEvent(.succeededRestoringSession)
}
case .failure: case .failure:
MXLog.error("Failed to restore an existing session.") MXLog.error("Failed to restore an existing session.")
stateMachine.processEvent(.failedRestoringSession) stateMachine.processEvent(.failedRestoringSession)
@@ -243,12 +232,9 @@ class AppCoordinator: AppCoordinatorProtocol {
switch result { switch result {
case .signedIn(let session): case .signedIn(let session):
self.userSession = session self.userSession = session
self.stateMachine.processEvent(.succeededSigningIn) self.stateMachine.processEvent(.createdUserSession)
case .clearAllData: case .clearAllData:
// clear user data self.stateMachine.processEvent(.signOut(isSoft: false))
self.userSessionStore.logout(userSession: self.userSession)
self.userSession = nil
self.startAuthentication()
} }
} }
@@ -266,7 +252,7 @@ class AppCoordinator: AppCoordinatorProtocol {
userSessionFlowCoordinator.callback = { [weak self] action in userSessionFlowCoordinator.callback = { [weak self] action in
switch action { switch action {
case .signOut: case .signOut:
self?.stateMachine.processEvent(.signOut) self?.stateMachine.processEvent(.signOut(isSoft: false))
} }
} }
@@ -277,7 +263,7 @@ class AppCoordinator: AppCoordinatorProtocol {
navigationRootCoordinator.setRootCoordinator(navigationSplitCoordinator) navigationRootCoordinator.setRootCoordinator(navigationSplitCoordinator)
} }
private func logout(isSoftLogout: Bool) { private func logout(isSoft: Bool) {
showLoadingIndicator() showLoadingIndicator()
defer { defer {
@@ -287,8 +273,8 @@ class AppCoordinator: AppCoordinatorProtocol {
userSession.clientProxy.stopSync() userSession.clientProxy.stopSync()
userSessionFlowCoordinator?.stop() userSessionFlowCoordinator?.stop()
guard !isSoftLogout else { guard !isSoft else {
stateMachine.processEvent(.completedSigningOut) stateMachine.processEvent(.completedSigningOut(isSoft: isSoft))
return return
} }
@@ -298,8 +284,9 @@ class AppCoordinator: AppCoordinatorProtocol {
// regardless of the result, clear user data // regardless of the result, clear user data
userSessionStore.logout(userSession: userSession) userSessionStore.logout(userSession: userSession)
tearDownUserSession()
stateMachine.processEvent(.completedSigningOut) stateMachine.processEvent(.completedSigningOut(isSoft: isSoft))
} }
} }
@@ -361,11 +348,7 @@ class AppCoordinator: AppCoordinatorProtocol {
guard let self else { return } guard let self else { return }
switch callback { switch callback {
case .didReceiveAuthError(let isSoftLogout): case .didReceiveAuthError(let isSoftLogout):
self.stateMachine.processEvent(.remoteSignOut(isSoft: isSoftLogout)) self.stateMachine.processEvent(.signOut(isSoft: isSoftLogout))
case .updateRestoreTokenNeeded:
if let userSession = self.userSession {
_ = self.userSessionStore.refreshRestorationToken(for: userSession)
}
default: default:
break break
} }
@@ -463,7 +446,7 @@ class AppCoordinator: AppCoordinatorProtocol {
extension AppCoordinator: AuthenticationCoordinatorDelegate { extension AppCoordinator: AuthenticationCoordinatorDelegate {
func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator, didLoginWithSession userSession: UserSessionProtocol) { func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator, didLoginWithSession userSession: UserSessionProtocol) {
self.userSession = userSession self.userSession = userSession
stateMachine.processEvent(.succeededSigningIn) stateMachine.processEvent(.createdUserSession)
} }
} }
@@ -31,32 +31,27 @@ class AppCoordinatorStateMachine {
case signedIn case signedIn
/// Processing a sign out request /// Processing a sign out request
case signingOut case signingOut(isSoft: Bool)
/// Processing a remote sign out
case remoteSigningOut(isSoft: Bool)
} }
/// Events that can be triggered on the AppCoordinator state machine /// Events that can be triggered on the AppCoordinator state machine
enum Event: EventType { enum Event: EventType {
/// Start the `AppCoordinator` by showing authentication. /// Start the `AppCoordinator` by showing authentication.
case startWithAuthentication case startWithAuthentication
/// Signing in succeeded
case succeededSigningIn
/// Start the `AppCoordinator` by restoring an existing account. /// Start the `AppCoordinator` by restoring an existing account.
case startWithExistingSession case startWithExistingSession
/// Restoring session succeeded.
case succeededRestoringSession
/// Restoring session failed. /// Restoring session failed.
case failedRestoringSession case failedRestoringSession
/// A session has been created
case createdUserSession
/// Request sign out /// Request sign out
case signOut case signOut(isSoft: Bool)
/// Remote sign out.
case remoteSignOut(isSoft: Bool)
/// Signing out completed /// Signing out completed
case completedSigningOut case completedSigningOut(isSoft: Bool)
} }
private let stateMachine: StateMachine<State, Event> private let stateMachine: StateMachine<State, Event>
@@ -72,21 +67,18 @@ class AppCoordinatorStateMachine {
private func configure() { private func configure() {
stateMachine.addRoutes(event: .startWithAuthentication, transitions: [.initial => .signedOut]) stateMachine.addRoutes(event: .startWithAuthentication, transitions: [.initial => .signedOut])
stateMachine.addRoutes(event: .succeededSigningIn, transitions: [.signedOut => .signedIn]) stateMachine.addRoutes(event: .createdUserSession, transitions: [.signedOut => .signedIn])
stateMachine.addRoutes(event: .startWithExistingSession, transitions: [.initial => .restoringSession]) stateMachine.addRoutes(event: .startWithExistingSession, transitions: [.initial => .restoringSession])
stateMachine.addRoutes(event: .succeededRestoringSession, transitions: [.restoringSession => .signedIn]) stateMachine.addRoutes(event: .createdUserSession, transitions: [.restoringSession => .signedIn])
stateMachine.addRoutes(event: .failedRestoringSession, transitions: [.restoringSession => .signedOut]) stateMachine.addRoutes(event: .failedRestoringSession, transitions: [.restoringSession => .signedOut])
stateMachine.addRoutes(event: .signOut, transitions: [.any => .signingOut])
stateMachine.addRoutes(event: .completedSigningOut, transitions: [.signingOut => .signedOut])
// Transitions with associated values need to be handled through `addRouteMapping` // Transitions with associated values need to be handled through `addRouteMapping`
stateMachine.addRouteMapping { event, fromState, _ in stateMachine.addRouteMapping { event, fromState, _ in
switch (event, fromState) { switch (event, fromState) {
case (.remoteSignOut(let isSoft), _): case (.signOut(let isSoft), _):
return .remoteSigningOut(isSoft: isSoft) return .signingOut(isSoft: isSoft)
case (.completedSigningOut, .remoteSigningOut): case (.completedSigningOut, .signingOut):
return .signedOut return .signedOut
default: default:
return nil return nil
@@ -35,11 +35,6 @@ private class WeakClientProxyWrapper: ClientDelegate, SlidingSyncObserver {
clientProxy?.didReceiveAuthError(isSoftLogout: isSoftLogout) clientProxy?.didReceiveAuthError(isSoftLogout: isSoftLogout)
} }
func didUpdateRestoreToken() {
MXLog.info("Did update restoration token")
clientProxy?.didUpdateRestoreToken()
}
// MARK: - SlidingSyncDelegate // MARK: - SlidingSyncDelegate
func didReceiveSyncUpdate(summary: UpdateSummary) { func didReceiveSyncUpdate(summary: UpdateSummary) {
@@ -98,10 +93,6 @@ class ClientProxy: ClientProxyProtocol {
} }
} }
var isSoftLogout: Bool {
client.isSoftLogout()
}
var deviceId: String? { var deviceId: String? {
do { do {
return try client.deviceId() return try client.deviceId()
@@ -126,7 +117,7 @@ class ClientProxy: ClientProxyProtocol {
func startSync() { func startSync() {
MXLog.info("Starting sync") MXLog.info("Starting sync")
guard !client.isSoftLogout(), slidingSyncObserverToken == nil else { guard slidingSyncObserverToken == nil else {
return return
} }
@@ -382,10 +373,6 @@ class ClientProxy: ClientProxyProtocol {
fileprivate func didReceiveAuthError(isSoftLogout: Bool) { fileprivate func didReceiveAuthError(isSoftLogout: Bool) {
callbacks.send(.receivedAuthError(isSoftLogout: isSoftLogout)) callbacks.send(.receivedAuthError(isSoftLogout: isSoftLogout))
} }
fileprivate func didUpdateRestoreToken() {
callbacks.send(.updatedRestoreToken)
}
fileprivate func didReceiveSlidingSyncUpdate(summary: UpdateSummary) { fileprivate func didReceiveSlidingSyncUpdate(summary: UpdateSummary) {
callbacks.send(.receivedSyncUpdate) callbacks.send(.receivedSyncUpdate)
@@ -21,7 +21,6 @@ import MatrixRustSDK
enum ClientProxyCallback { enum ClientProxyCallback {
case receivedSyncUpdate case receivedSyncUpdate
case receivedAuthError(isSoftLogout: Bool) case receivedAuthError(isSoftLogout: Bool)
case updatedRestoreToken
} }
enum ClientProxyError: Error { enum ClientProxyError: Error {
@@ -69,8 +68,6 @@ protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol {
var userID: String { get } var userID: String { get }
var isSoftLogout: Bool { get }
var deviceId: String? { get } var deviceId: String? { get }
var homeserver: String { get } var homeserver: String { get }
@@ -22,7 +22,6 @@ class MockClientProxy: ClientProxyProtocol {
let callbacks = PassthroughSubject<ClientProxyCallback, Never>() let callbacks = PassthroughSubject<ClientProxyCallback, Never>()
let userID: String let userID: String
let isSoftLogout = false
let deviceId: String? = nil let deviceId: String? = nil
let homeserver = "" let homeserver = ""
let restorationToken: RestorationToken? = nil let restorationToken: RestorationToken? = nil
@@ -20,7 +20,6 @@ struct MockUserSession: UserSessionProtocol {
let callbacks = PassthroughSubject<UserSessionCallback, Never>() let callbacks = PassthroughSubject<UserSessionCallback, Never>()
let sessionVerificationController: SessionVerificationControllerProxyProtocol? = nil let sessionVerificationController: SessionVerificationControllerProxyProtocol? = nil
var userID: String { clientProxy.userID } var userID: String { clientProxy.userID }
var isSoftLogout: Bool { clientProxy.isSoftLogout }
var deviceID: String? { clientProxy.deviceId } var deviceID: String? { clientProxy.deviceId }
var homeserver: String { clientProxy.homeserver } var homeserver: String { clientProxy.homeserver }
let clientProxy: ClientProxyProtocol let clientProxy: ClientProxyProtocol
@@ -21,10 +21,8 @@ class UserSession: UserSessionProtocol {
private var cancellables = Set<AnyCancellable>() private var cancellables = Set<AnyCancellable>()
private var checkForSessionVerificationControllerCancellable: AnyCancellable? private var checkForSessionVerificationControllerCancellable: AnyCancellable?
private var authErrorCancellable: AnyCancellable? private var authErrorCancellable: AnyCancellable?
private var restoreTokenUpdateCancellable: AnyCancellable?
var userID: String { clientProxy.userID } var userID: String { clientProxy.userID }
var isSoftLogout: Bool { clientProxy.isSoftLogout }
var deviceID: String? { clientProxy.deviceId } var deviceID: String? { clientProxy.deviceId }
var homeserver: String { clientProxy.homeserver } var homeserver: String { clientProxy.homeserver }
@@ -39,7 +37,6 @@ class UserSession: UserSessionProtocol {
setupSessionVerificationWatchdog() setupSessionVerificationWatchdog()
setupAuthErrorWatchdog() setupAuthErrorWatchdog()
setupRestoreTokenUpdateWatchdog()
} }
// MARK: - Private // MARK: - Private
@@ -102,21 +99,4 @@ class UserSession: UserSessionProtocol {
private func tearDownAuthErrorWatchdog() { private func tearDownAuthErrorWatchdog() {
authErrorCancellable = nil authErrorCancellable = nil
} }
// MARK: Restore Token Update Watchdog
private func setupRestoreTokenUpdateWatchdog() {
restoreTokenUpdateCancellable = clientProxy.callbacks
.receive(on: DispatchQueue.main)
.sink { [weak self] callback in
if case .updatedRestoreToken = callback {
self?.callbacks.send(.updateRestoreTokenNeeded)
self?.tearDownRestoreTokenUpdateWatchdog()
}
}
}
private func tearDownRestoreTokenUpdateWatchdog() {
restoreTokenUpdateCancellable = nil
}
} }
@@ -21,7 +21,6 @@ enum UserSessionCallback {
case sessionVerificationNeeded case sessionVerificationNeeded
case didVerifySession case didVerifySession
case didReceiveAuthError(isSoftLogout: Bool) case didReceiveAuthError(isSoftLogout: Bool)
case updateRestoreTokenNeeded
} }
protocol UserSessionProtocol { protocol UserSessionProtocol {
@@ -29,8 +28,6 @@ protocol UserSessionProtocol {
var userID: String { get } var userID: String { get }
var deviceID: String? { get } var deviceID: String? { get }
var isSoftLogout: Bool { get }
var clientProxy: ClientProxyProtocol { get } var clientProxy: ClientProxyProtocol { get }
var mediaProvider: MediaProviderProtocol { get } var mediaProvider: MediaProviderProtocol { get }
@@ -29,7 +29,6 @@ extension MatrixRustSDK.Session: Codable {
userId: try container.decode(String.self, forKey: .userId), userId: try container.decode(String.self, forKey: .userId),
deviceId: try container.decode(String.self, forKey: .deviceId), deviceId: try container.decode(String.self, forKey: .deviceId),
homeserverUrl: try container.decode(String.self, forKey: .homeserverUrl), homeserverUrl: try container.decode(String.self, forKey: .homeserverUrl),
isSoftLogout: try container.decode(Bool.self, forKey: .isSoftLogout),
slidingSyncProxy: try container.decode(String.self, forKey: .slidingSyncProxy)) slidingSyncProxy: try container.decode(String.self, forKey: .slidingSyncProxy))
} }
@@ -40,11 +39,10 @@ extension MatrixRustSDK.Session: Codable {
try container.encode(userId, forKey: .userId) try container.encode(userId, forKey: .userId)
try container.encode(deviceId, forKey: .deviceId) try container.encode(deviceId, forKey: .deviceId)
try container.encode(homeserverUrl, forKey: .homeserverUrl) try container.encode(homeserverUrl, forKey: .homeserverUrl)
try container.encode(isSoftLogout, forKey: .isSoftLogout)
try container.encode(slidingSyncProxy, forKey: .slidingSyncProxy) try container.encode(slidingSyncProxy, forKey: .slidingSyncProxy)
} }
enum CodingKeys: String, CodingKey { enum CodingKeys: String, CodingKey {
case accessToken, refreshToken, userId, deviceId, homeserverUrl, isSoftLogout, slidingSyncProxy case accessToken, refreshToken, userId, deviceId, homeserverUrl, slidingSyncProxy
} }
} }
@@ -37,7 +37,6 @@ class KeychainControllerTests: XCTestCase {
userId: "userId", userId: "userId",
deviceId: "deviceId", deviceId: "deviceId",
homeserverUrl: "homeserverUrl", homeserverUrl: "homeserverUrl",
isSoftLogout: false,
slidingSyncProxy: "https://my.sync.proxy")) slidingSyncProxy: "https://my.sync.proxy"))
keychain.setRestorationToken(restorationToken, forUsername: username) keychain.setRestorationToken(restorationToken, forUsername: username)
@@ -53,7 +52,6 @@ class KeychainControllerTests: XCTestCase {
userId: "userId", userId: "userId",
deviceId: "deviceId", deviceId: "deviceId",
homeserverUrl: "homeserverUrl", homeserverUrl: "homeserverUrl",
isSoftLogout: false,
slidingSyncProxy: "https://my.sync.proxy")) slidingSyncProxy: "https://my.sync.proxy"))
keychain.setRestorationToken(restorationToken, forUsername: username) keychain.setRestorationToken(restorationToken, forUsername: username)
XCTAssertEqual(keychain.restorationTokens().count, 1, "The keychain should have 1 restoration token.") XCTAssertEqual(keychain.restorationTokens().count, 1, "The keychain should have 1 restoration token.")
@@ -75,7 +73,6 @@ class KeychainControllerTests: XCTestCase {
userId: "userId", userId: "userId",
deviceId: "deviceId", deviceId: "deviceId",
homeserverUrl: "homeserverUrl", homeserverUrl: "homeserverUrl",
isSoftLogout: false,
slidingSyncProxy: "https://my.sync.proxy")) slidingSyncProxy: "https://my.sync.proxy"))
keychain.setRestorationToken(restorationToken, forUsername: "@test\(index):example.com") keychain.setRestorationToken(restorationToken, forUsername: "@test\(index):example.com")
} }
@@ -96,7 +93,6 @@ class KeychainControllerTests: XCTestCase {
userId: "userId", userId: "userId",
deviceId: "deviceId", deviceId: "deviceId",
homeserverUrl: "homeserverUrl", homeserverUrl: "homeserverUrl",
isSoftLogout: false,
slidingSyncProxy: "https://my.sync.proxy")) slidingSyncProxy: "https://my.sync.proxy"))
keychain.setRestorationToken(restorationToken, forUsername: "@test\(index):example.com") keychain.setRestorationToken(restorationToken, forUsername: "@test\(index):example.com")
} }
@@ -69,20 +69,4 @@ final class UserSessionTests: XCTestCase {
controller.callbacks.send(.finished) controller.callbacks.send(.finished)
waitForExpectations(timeout: 1.0) waitForExpectations(timeout: 1.0)
} }
func test_whenUserSessionReceivesUpdatedRestoreToken_updateRestoreTokenNeededEventReceived() throws {
let expectation = expectation(description: "UpdatedRestoreToken expectation")
userSession.callbacks.sink { callback in
switch callback {
case .updateRestoreTokenNeeded:
expectation.fulfill()
default:
break
}
}
.store(in: &cancellables)
clientProxy.callbacks.send(.updatedRestoreToken)
waitForExpectations(timeout: 1.0)
}
} }
+1 -1
View File
@@ -40,7 +40,7 @@ include:
packages: packages:
MatrixRustSDK: MatrixRustSDK:
url: https://github.com/matrix-org/matrix-rust-components-swift url: https://github.com/matrix-org/matrix-rust-components-swift
exactVersion: 1.0.42-alpha exactVersion: 1.0.43-alpha
# path: ../matrix-rust-sdk # path: ../matrix-rust-sdk
DesignKit: DesignKit:
path: DesignKit path: DesignKit