Skip to content

Commit

Permalink
Merge branch 'refactor-account-deletion-in-tunnelmanager-ios-310'
Browse files Browse the repository at this point in the history
  • Loading branch information
buggmagnet committed Dec 4, 2023
2 parents ddc095a + 7963de8 commit f929009
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 113 deletions.
6 changes: 0 additions & 6 deletions ios/MullvadVPN.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,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 */; };
Expand Down Expand Up @@ -672,7 +671,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 */; };
Expand Down Expand Up @@ -1683,7 +1681,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>"; };
Expand Down Expand Up @@ -2014,7 +2011,6 @@
5823FA5726CE4A4100283BF8 /* TunnelManager */ = {
isa = PBXGroup;
children = (
F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */,
588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */,
58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */,
F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */,
Expand Down Expand Up @@ -4217,7 +4213,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 */,
Expand Down Expand Up @@ -4519,7 +4514,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 */,
Expand Down
61 changes: 0 additions & 61 deletions ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift

This file was deleted.

54 changes: 54 additions & 0 deletions ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
Expand Down Expand Up @@ -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 })
}
}
}
}
Expand Down Expand Up @@ -141,6 +150,27 @@ 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)
}
}

/**
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:
Expand Down Expand Up @@ -264,6 +294,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...")
Expand Down Expand Up @@ -398,3 +450,5 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
var device: Device
}
}

// swiftlint:disable:this file_length
1 change: 1 addition & 0 deletions ios/MullvadVPN/TunnelManager/TunnelInteractor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
56 changes: 15 additions & 41 deletions ios/MullvadVPN/TunnelManager/TunnelManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)) { 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) {
Expand Down Expand Up @@ -1092,7 +1062,7 @@ final class TunnelManager: StorePaymentObserver {
isRunningPeriodicPrivateKeyRotation = false
}

private func wipeAllUserData() {
fileprivate func removeLastUsedAccount() {
do {
try SettingsManager.setLastUsedAccount(nil)
} catch {
Expand Down Expand Up @@ -1121,7 +1091,7 @@ final class TunnelManager: StorePaymentObserver {
unsetTunnelConfiguration {
self.setDeviceState(.revoked, persist: true)
self.operationQueue.cancelAllOperations()
self.wipeAllUserData()
self.removeLastUsedAccount()
}
default:
break
Expand Down Expand Up @@ -1307,6 +1277,10 @@ private struct TunnelInteractorProxy: TunnelInteractor {
tunnelManager.setDeviceState(deviceState, persist: persist)
}

func removeLastUsedAccount() {
tunnelManager.removeLastUsedAccount()
}

func startTunnel() {
tunnelManager.startTunnel()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down

0 comments on commit f929009

Please sign in to comment.