Refactor account deletion in TunnelManager

mullvad · Dec 4, 2023 · 94a925c · 94a925c
1 parent 5c8d672
commit 94a925c
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 113 deletions.
diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -570,7 +570,6 @@
 		A9A5FA0F2ACB05160083449F /* StoreSubscription.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5846227026E229F20035F7C2 /* StoreSubscription.swift */; };
 		A9A5FA102ACB05160083449F /* PacketTunnelTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 063687B928EB234F00BE7161 /* PacketTunnelTransport.swift */; };
 		A9A5FA112ACB05160083449F /* TransportMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0697D6E628F01513007A9E99 /* TransportMonitor.swift */; };
-		A9A5FA122ACB05160083449F /* DeleteAccountOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */; };
 		A9A5FA132ACB05160083449F /* LoadTunnelConfigurationOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */; };
 		A9A5FA142ACB05160083449F /* MapConnectionStatusOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */; };
 		A9A5FA152ACB05160083449F /* RedeemVoucherOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */; };
@@ -666,7 +665,6 @@
 		F0B0E6972AFE6E7E001DC66B /* XCTest+Async.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */; };
 		F0C2AEFD2A0BB5CC00986207 /* NotificationProviderIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C2AEFC2A0BB5CC00986207 /* NotificationProviderIdentifier.swift */; };
 		F0C6A8432AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6A8422AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift */; };
-		F0C6FA812A66E23300F521F0 /* DeleteAccountOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */; };
 		F0C6FA852A6A733700F521F0 /* InAppPurchaseInteractor.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA842A6A733700F521F0 /* InAppPurchaseInteractor.swift */; };
 		F0D8825B2B04F53600D3EF9A /* OutgoingConnectionData.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */; };
 		F0D8825C2B04F70E00D3EF9A /* OutgoingConnectionData.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */; };
@@ -1666,7 +1664,6 @@
 		F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "XCTest+Async.swift"; sourceTree = "<group>"; };
 		F0C2AEFC2A0BB5CC00986207 /* NotificationProviderIdentifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationProviderIdentifier.swift; sourceTree = "<group>"; };
 		F0C6A8422AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RedeemVoucherViewConfiguration.swift; sourceTree = "<group>"; };
-		F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeleteAccountOperation.swift; sourceTree = "<group>"; };
 		F0C6FA842A6A733700F521F0 /* InAppPurchaseInteractor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InAppPurchaseInteractor.swift; sourceTree = "<group>"; };
 		F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OutgoingConnectionData.swift; sourceTree = "<group>"; };
 		F0DA87462A9CB9A2006044F1 /* AccountExpiryRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountExpiryRow.swift; sourceTree = "<group>"; };
@@ -1995,7 +1992,6 @@
 		5823FA5726CE4A4100283BF8 /* TunnelManager */ = {
 			isa = PBXGroup;
 			children = (
-				F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */,
 				588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */,
 				58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */,
 				F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */,
@@ -4193,7 +4189,6 @@
 				A9A5FA0F2ACB05160083449F /* StoreSubscription.swift in Sources */,
 				A9A5FA102ACB05160083449F /* PacketTunnelTransport.swift in Sources */,
 				A9A5FA112ACB05160083449F /* TransportMonitor.swift in Sources */,
-				A9A5FA122ACB05160083449F /* DeleteAccountOperation.swift in Sources */,
 				A9B6AC1A2ADE8FBB00F7802A /* InMemorySettingsStore.swift in Sources */,
 				A9A5FA132ACB05160083449F /* LoadTunnelConfigurationOperation.swift in Sources */,
 				A9A5FA142ACB05160083449F /* MapConnectionStatusOperation.swift in Sources */,
@@ -4491,7 +4486,6 @@
 				7AF9BE8E2A331C7B00DBFEDB /* RelayFilterViewModel.swift in Sources */,
 				58F3C0A4249CB069003E76BE /* HeaderBarView.swift in Sources */,
 				5864AF0829C78849005B0CD9 /* CellFactoryProtocol.swift in Sources */,
-				F0C6FA812A66E23300F521F0 /* DeleteAccountOperation.swift in Sources */,
 				F07CFF2029F2720E008C0343 /* RegisteredDeviceInAppNotificationProvider.swift in Sources */,
 				587A01FC23F1F0BE00B68763 /* SimulatorTunnelProviderHost.swift in Sources */,
 				7A6F2FA72AFBB9AE006D0856 /* AccountExpiry.swift in Sources */,

diff --git a/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift b/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift
diff --git a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
@@ -24,11 +24,15 @@ enum SetAccountAction {
     /// Unset account.
     case unset
 
+    /// Delete account.
+    case delete(String)
+
     var taskName: String {
         switch self {
         case .new: "Set new account"
         case .existing: "Set existing account"
         case .unset: "Unset account"
+        case .delete: "Delete account"
         }
     }
 }
@@ -78,6 +82,11 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
 
             case .unset:
                 finish(result: .success(nil))
+
+            case let .delete(accountNumber):
+                startDeleteAccountFlow(accountNumber: accountNumber) { [self] result in
+                    finish(result: result.map { .none })
+                }
             }
         }
     }
@@ -141,6 +150,42 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
         }
     }
 
+    /**
+     Begin delete flow of an existing account by performing the following steps:
+
+     1. Delete existing account with the API.
+     2. Reset tunnel settings to default and remove last used account.
+     */
+    private func startDeleteAccountFlow(
+        accountNumber: String,
+        completion: @escaping (Result<Void, Error>) -> Void
+    ) {
+        deleteAccount(accountNumber: accountNumber) { [self] result in
+            interactor.setSettings(LatestTunnelSettings(), persist: true)
+
+            if result.isSuccess {
+                interactor.removeLastUsedAccount()
+            }
+
+            completion(result)
+        }
+    }
+
+//    operation.completionHandler = { [weak self] result in
+//        switch result {
+//        case .success:
+//            self?.unsetTunnelConfiguration {
+//                self?.operationQueue.cancelAllOperations()
+//                self?.wipeAllUserData()
+//                self?.setDeviceState(.loggedOut, persist: true)
+//                DispatchQueue.main.async {
+//                    completion?(nil)
+//                }
+//            }
+//        case let .failure(error):
+//            completion?(error)
+//        }
+
     /**
      Continue login flow after receiving account data as a part of creating new or retrieving existing account from
      the API by performing the following steps:
@@ -264,6 +309,28 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
         tasks.append(task)
     }
 
+    /// Delete account.
+    private func deleteAccount(accountNumber: String, completion: @escaping (Result<Void, Error>) -> Void) {
+        logger.debug("Delete account...")
+
+        let task = accountsProxy.deleteAccount(
+            accountNumber: accountNumber,
+            retryStrategy: .default
+        ) { [self] result in
+            dispatchQueue.async { [self] in
+                let result = result.inspectError { error in
+                    guard !error.isOperationCancellationError else { return }
+
+                    logger.error(error: error, message: "Failed to delete account.")
+                }
+
+                completion(result)
+            }
+        }
+
+        tasks.append(task)
+    }
+
     /// Delete device from API.
     private func deleteDevice(accountNumber: String, deviceIdentifier: String, completion: @escaping (Error?) -> Void) {
         logger.debug("Delete current device...")
@@ -398,3 +465,5 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
         var device: Device
     }
 }
+
+// swiftlint:disable:this file_length
diff --git a/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift b/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift
@@ -33,6 +33,7 @@ protocol TunnelInteractor {
     func setConfigurationLoaded()
     func setSettings(_ settings: LatestTunnelSettings, persist: Bool)
     func setDeviceState(_ deviceState: DeviceState, persist: Bool)
+    func removeLastUsedAccount()
     func handleRestError(_ error: Error)
 
     func startTunnel()

diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
@@ -364,10 +364,13 @@ final class TunnelManager: StorePaymentObserver {
             MutuallyExclusive(category: OperationCategory.settingsUpdate.category)
         )
 
-        // Unsetting the account (ie. logging out) should cancel all other currently ongoing
-        // activity.
-        if case .unset = action {
+        // Unsetting (ie. logging out) or deleting the account should cancel all other
+        // currently ongoing activity.
+        switch action {
+        case .unset, .delete:
             operationQueue.cancelAllOperations()
+        default:
+            break
         }
 
         operationQueue.addOperation(operation)
@@ -437,43 +440,10 @@ final class TunnelManager: StorePaymentObserver {
     func deleteAccount(
         accountNumber: String,
         completion: ((Error?) -> Void)? = nil
-    ) -> Cancellable {
-        let operation = DeleteAccountOperation(
-            dispatchQueue: internalQueue,
-            accountsProxy: accountsProxy,
-            accessTokenManager: accessTokenManager,
-            accountNumber: accountNumber
-        )
-
-        operation.completionQueue = .main
-        operation.completionHandler = { [weak self] result in
-            switch result {
-            case .success:
-                self?.unsetTunnelConfiguration {
-                    self?.operationQueue.cancelAllOperations()
-                    self?.wipeAllUserData()
-                    self?.setDeviceState(.loggedOut, persist: true)
-                    DispatchQueue.main.async {
-                        completion?(nil)
-                    }
-                }
-            case let .failure(error):
-                completion?(error)
-            }
+    ) {
+        setAccount(action: .delete(accountNumber)) { [weak self] result in
+            completion?(result.error)
         }
-
-        operation.addObserver(
-            BackgroundObserver(
-                application: application,
-                name: "Delete account",
-                cancelUponExpiration: true
-            )
-        )
-
-        operation.addCondition(MutuallyExclusive(category: OperationCategory.deviceStateUpdate.category))
-
-        operationQueue.addOperation(operation)
-        return operation
     }
 
     func updateDeviceData(_ completionHandler: ((Error?) -> Void)? = nil) {
@@ -1091,7 +1061,7 @@ final class TunnelManager: StorePaymentObserver {
         isRunningPeriodicPrivateKeyRotation = false
     }
 
-    private func wipeAllUserData() {
+    fileprivate func removeLastUsedAccount() {
         do {
             try SettingsManager.setLastUsedAccount(nil)
         } catch {
@@ -1120,7 +1090,7 @@ final class TunnelManager: StorePaymentObserver {
             unsetTunnelConfiguration {
                 self.setDeviceState(.revoked, persist: true)
                 self.operationQueue.cancelAllOperations()
-                self.wipeAllUserData()
+                self.removeLastUsedAccount()
             }
         default:
             break
@@ -1306,6 +1276,10 @@ private struct TunnelInteractorProxy: TunnelInteractor {
         tunnelManager.setDeviceState(deviceState, persist: persist)
     }
 
+    func removeLastUsedAccount() {
+        tunnelManager.removeLastUsedAccount()
+    }
+
     func startTunnel() {
         tunnelManager.startTunnel()
     }

diff --git a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift
@@ -48,7 +48,7 @@ class AccountDeletionInteractor {
         }
     }
 
-    func delete(accountNumber: String, completionHandler: @escaping (Error?) -> Void) -> Cancellable {
-        return tunnelManager.deleteAccount(accountNumber: accountNumber, completion: completionHandler)
+    func delete(accountNumber: String, completionHandler: @escaping (Error?) -> Void) {
+        tunnelManager.deleteAccount(accountNumber: accountNumber, completion: completionHandler)
     }
 }
diff --git a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift
@@ -15,7 +15,6 @@ protocol AccountDeletionViewControllerDelegate: AnyObject {
 }
 
 class AccountDeletionViewController: UIViewController {
-    private var task: Cancellable?
     private lazy var contentView: AccountDeletionContentView = {
         let view = AccountDeletionContentView()
         view.delegate = self
@@ -63,7 +62,7 @@ class AccountDeletionViewController: UIViewController {
 
     private func submit(accountNumber: String) {
         contentView.state = .loading
-        task = interactor.delete(accountNumber: accountNumber) { [weak self] error in
+        interactor.delete(accountNumber: accountNumber) { [weak self] error in
             guard let self else { return }
             guard let error else {
                 self.contentView.state = .initial
@@ -78,7 +77,6 @@ class AccountDeletionViewController: UIViewController {
 extension AccountDeletionViewController: AccountDeletionContentViewDelegate {
     func didTapCancelButton(contentView: AccountDeletionContentView, button: AppButton) {
         contentView.isEditing = false
-        task?.cancel()
         delegate?.deleteAccountDidCancel(controller: self)
     }