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

Refactor account deletion in TunnelManager #5520

Merged
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: 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
Loading