PR Feedback part 3

mullvad · Oct 16, 2023 · fab18b6 · fab18b6
1 parent 95f66b1
commit fab18b6
Show file tree

Hide file tree

Showing 11 changed files with 176 additions and 247 deletions.
diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -494,6 +494,8 @@
 		A97F1F472A1F4E1A00ECEFDE /* MullvadTransport.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = A97F1F412A1F4E1A00ECEFDE /* MullvadTransport.framework */; };
 		A97F1F482A1F4E1A00ECEFDE /* MullvadTransport.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = A97F1F412A1F4E1A00ECEFDE /* MullvadTransport.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		A97FF5502A0D2FFC00900996 /* NSFileCoordinator+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = A97FF54F2A0D2FFC00900996 /* NSFileCoordinator+Extensions.swift */; };
+		A988DF212ADD293D00D807EF /* RESTTransportStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = A9A1DE782AD5708E0073F689 /* RESTTransportStrategy.swift */; };
+		A988DF242ADD307200D807EF /* libRelaySelector.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 5898D29829017DAC00EB5EBA /* libRelaySelector.a */; };
 		A9A1DE792AD5708E0073F689 /* RESTTransportStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = A9A1DE782AD5708E0073F689 /* RESTTransportStrategy.swift */; };
 		A9A5F9E12ACB05160083449F /* AddressCacheTracker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 06AC114028F841390037AF9A /* AddressCacheTracker.swift */; };
 		A9A5F9E22ACB05160083449F /* BackgroundTask.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58C76A0A2A338E4300100D75 /* BackgroundTask.swift */; };
@@ -948,6 +950,13 @@
 			remoteGlobalIDString = A97F1F402A1F4E1A00ECEFDE;
 			remoteInfo = MullvadTransport;
 		};
+		A988DF222ADD305300D807EF /* PBXContainerItemProxy */ = {
+			isa = PBXContainerItemProxy;
+			containerPortal = 58CE5E58224146200008646E /* Project object */;
+			proxyType = 1;
+			remoteGlobalIDString = 5898D29729017DAC00EB5EBA;
+			remoteInfo = RelaySelector;
+		};
 		A9B2CF702A1F64B20013CC6C /* PBXContainerItemProxy */ = {
 			isa = PBXContainerItemProxy;
 			containerPortal = 58CE5E58224146200008646E /* Project object */;
@@ -1540,7 +1549,6 @@
 		A900E9BB2ACC609200C95F67 /* DevicesProxy+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DevicesProxy+Stubs.swift"; sourceTree = "<group>"; };
 		A900E9BD2ACC654100C95F67 /* APIProxy+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "APIProxy+Stubs.swift"; sourceTree = "<group>"; };
 		A900E9BF2ACC661900C95F67 /* AccessTokenManager+Stubs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AccessTokenManager+Stubs.swift"; sourceTree = "<group>"; };
-		A917351E29FAA9C400D5DCFD /* RESTTransportStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RESTTransportStrategy.swift; sourceTree = "<group>"; };
 		A917352029FAAA5200D5DCFD /* TransportStrategyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TransportStrategyTests.swift; sourceTree = "<group>"; };
 		A92ECC202A77FFAF0052F1B1 /* TunnelSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelSettings.swift; sourceTree = "<group>"; };
 		A92ECC232A7802520052F1B1 /* StoredAccountData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoredAccountData.swift; sourceTree = "<group>"; };
@@ -1691,6 +1699,7 @@
 			isa = PBXFrameworksBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				A988DF242ADD307200D807EF /* libRelaySelector.a in Frameworks */,
 				A94D691B2ABAD66700413DD4 /* WireGuardKitTypes in Frameworks */,
 				58C7A43E2A863F470060C66F /* PacketTunnelCore.framework in Frameworks */,
 			);
@@ -3283,6 +3292,7 @@
 			buildRules = (
 			);
 			dependencies = (
+				A988DF232ADD305300D807EF /* PBXTargetDependency */,
 				58C7A4402A863F470060C66F /* PBXTargetDependency */,
 				58C7A4722A864B860060C66F /* PBXTargetDependency */,
 			);
@@ -4118,7 +4128,6 @@
 				58C7A4522A863FB50060C66F /* Pinger.swift in Sources */,
 				580D6B8C2AB3369300B2D6E0 /* BlockedStateErrorMapperProtocol.swift in Sources */,
 				58C7AF172ABD84AA007EDD7A /* ProxyURLRequest.swift in Sources */,
-				58C7AF142ABD8480007EDD7A /* RelaySelectorResult+PacketTunnelRelay.swift in Sources */,
 				5838321F2AC3160A00EA2071 /* PacketTunnelActor+KeyPolicy.swift in Sources */,
 				58C7AF122ABD8480007EDD7A /* TunnelProviderReply.swift in Sources */,
 				58C7A4572A863FB90060C66F /* TunnelDeviceInfoProtocol.swift in Sources */,
@@ -4531,7 +4540,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				58B465702A98C53300467203 /* RequestExecutorTests.swift in Sources */,
-				A9A1DE792AD5708E0073F689 /* RESTTransportStrategy.swift in Sources */,
+				A988DF212ADD293D00D807EF /* RESTTransportStrategy.swift in Sources */,
 				A917352129FAAA5200D5DCFD /* TransportStrategyTests.swift in Sources */,
 				58FBFBE9291622580020E046 /* ExponentialBackoffTests.swift in Sources */,
 				58BDEB9D2A98F69E00F578F2 /* MemoryCache.swift in Sources */,
@@ -4802,6 +4811,11 @@
 			target = A97F1F402A1F4E1A00ECEFDE /* MullvadTransport */;
 			targetProxy = A97F1F452A1F4E1A00ECEFDE /* PBXContainerItemProxy */;
 		};
+		A988DF232ADD305300D807EF /* PBXTargetDependency */ = {
+			isa = PBXTargetDependency;
+			target = 5898D29729017DAC00EB5EBA /* RelaySelector */;
+			targetProxy = A988DF222ADD305300D807EF /* PBXContainerItemProxy */;
+		};
 		A9B2CF712A1F64B20013CC6C /* PBXTargetDependency */ = {
 			isa = PBXTargetDependency;
 			target = 06799ABB28F98E1D00ACD94E /* MullvadREST */;

diff --git a/ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift b/ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift
@@ -85,7 +85,7 @@ class StartTunnelOperation: ResultOperation<Void> {
         }
     }
 
-    private func startTunnel(tunnel: Tunnel, selectedRelay: SelectedRelay) throws {
+    private func startTunnel(tunnel: any TunnelProtocol, selectedRelay: SelectedRelay) throws {
         var tunnelOptions = PacketTunnelOptions()
 
         do {

diff --git a/ios/MullvadVPN/TunnelManager/Tunnel.swift b/ios/MullvadVPN/TunnelManager/Tunnel.swift
@@ -25,8 +25,10 @@ protocol TunnelProtocol: AnyObject {
     var isOnDemandEnabled: Bool { get set }
     var startDate: Date? { get }
 
-    func addObserver(_ observer: TunnelStatusObserver)
-    func removeObserver(_ observer: TunnelStatusObserver)
+    init(tunnelProvider: TunnelProviderManagerType)
+
+    func addObserver(_ observer: any TunnelStatusObserver)
+    func removeObserver(_ observer: any TunnelStatusObserver)
     func addBlockObserver(
         queue: DispatchQueue?,
         handler: @escaping (any TunnelProtocol, NEVPNStatus) -> Void
@@ -105,7 +107,7 @@ final class Tunnel: TunnelProtocol, Equatable {
     }
 
     private let lock = NSLock()
-    private var observerList = ObserverList<TunnelStatusObserver>()
+    private var observerList = ObserverList<any TunnelStatusObserver>()
 
     private var _startDate: Date?
     internal let tunnelProvider: TunnelProviderManagerType
@@ -169,11 +171,11 @@ final class Tunnel: TunnelProtocol, Equatable {
         return observer
     }
 
-    func addObserver(_ observer: TunnelStatusObserver) {
+    func addObserver(_ observer: any TunnelStatusObserver) {
         observerList.append(observer)
     }
 
-    func removeObserver(_ observer: TunnelStatusObserver) {
+    func removeObserver(_ observer: any TunnelStatusObserver) {
         observerList.remove(observer)
     }
 
@@ -185,7 +187,7 @@ final class Tunnel: TunnelProtocol, Equatable {
         handleVPNStatus(newStatus)
 
         observerList.forEach { observer in
-            observer.tunnel(self, didReceiveStatus: newStatus)
+//            observer.tunnel(self, didReceiveStatus: newStatus)
         }
     }
 

diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
@@ -45,7 +45,7 @@ final class TunnelManager: StorePaymentObserver {
     // MARK: - Internal variables
 
     private let application: BackgroundTaskProvider
-    fileprivate let tunnelStore: TunnelStoreProtocol
+    fileprivate let tunnelStore: any TunnelStoreProtocol
     private let relayCacheTracker: RelayCacheTrackerProtocol
     private let accountsProxy: RESTAccountHandling
     private let devicesProxy: DeviceHandling
@@ -82,7 +82,7 @@ final class TunnelManager: StorePaymentObserver {
 
     init(
         application: BackgroundTaskProvider,
-        tunnelStore: TunnelStoreProtocol,
+        tunnelStore: any TunnelStoreProtocol,
         relayCacheTracker: RelayCacheTrackerProtocol,
         accountsProxy: RESTAccountHandling,
         devicesProxy: DeviceHandling,

diff --git a/ios/MullvadVPN/TunnelManager/TunnelStore.swift b/ios/MullvadVPN/TunnelManager/TunnelStore.swift
@@ -12,20 +12,22 @@ import NetworkExtension
 import UIKit
 
 protocol TunnelStoreProtocol {
-    func getPersistentTunnels() -> [any TunnelProtocol]
-    func createNewTunnel() -> any TunnelProtocol
+    associatedtype TunnelType: TunnelProtocol, Equatable
+    func getPersistentTunnels() -> [TunnelType]
+    func createNewTunnel() -> TunnelType
 }
 
 /// Wrapper around system VPN tunnels.
 final class TunnelStore: TunnelStoreProtocol, TunnelStatusObserver {
+    typealias TunnelType = Tunnel
     private let logger = Logger(label: "TunnelStore")
     private let lock = NSLock()
 
     /// Persistent tunnels registered with the system.
-    private var persistentTunnels: [any TunnelProtocol] = []
+    private var persistentTunnels: [TunnelType] = []
 
     /// Newly created tunnels, stored as collection of weak boxes.
-    private var newTunnels: [WeakBox<any TunnelProtocol>] = []
+    private var newTunnels: [WeakBox<TunnelType>] = []
 
     init(application: UIApplication) {
         NotificationCenter.default.addObserver(
@@ -36,7 +38,7 @@ final class TunnelStore: TunnelStoreProtocol, TunnelStatusObserver {
         )
     }
 
-    func getPersistentTunnels() -> [any TunnelProtocol] {
+    func getPersistentTunnels() -> [TunnelType] {
         lock.lock()
         defer { lock.unlock() }
 
@@ -71,12 +73,12 @@ final class TunnelStore: TunnelStoreProtocol, TunnelStatusObserver {
         }
     }
 
-    func createNewTunnel() -> any TunnelProtocol {
+    func createNewTunnel() -> TunnelType {
         lock.lock()
         defer { lock.unlock() }
 
         let tunnelProviderManager = TunnelProviderManagerType()
-        let tunnel = Tunnel(tunnelProvider: tunnelProviderManager)
+        let tunnel = TunnelType(tunnelProvider: tunnelProviderManager)
         tunnel.addObserver(self)
 
         newTunnels = newTunnels.filter { $0.value != nil }
@@ -91,20 +93,19 @@ final class TunnelStore: TunnelStoreProtocol, TunnelStatusObserver {
         lock.lock()
         defer { lock.unlock() }
 
-        handleTunnelStatus(tunnel: tunnel, status: status)
+        // swiftlint:disable:next force_cast
+        handleTunnelStatus(tunnel: tunnel as! TunnelType, status: status)
     }
 
-    private func handleTunnelStatus(tunnel: any TunnelProtocol, status: NEVPNStatus) {
-        guard let tunnel = tunnel as? Tunnel else { return }
-
+    private func handleTunnelStatus(tunnel: TunnelType, status: NEVPNStatus) {
         if status == .invalid,
-           let index = persistentTunnels.map({ $0 as? Tunnel }).firstIndex(of: tunnel) {
+           let index = persistentTunnels.firstIndex(of: tunnel) {
             persistentTunnels.remove(at: index)
             logger.debug("Persistent tunnel was removed: \(tunnel.logFormat()).")
         }
 
         if status != .invalid,
-           let index = newTunnels.map({ $0.value as? Tunnel }).firstIndex(where: { $0 == tunnel }) {
+           let index = newTunnels.compactMap({ $0.value }).firstIndex(where: { $0 == tunnel }) {
             newTunnels.remove(at: index)
             persistentTunnels.append(tunnel)
             logger.debug("New tunnel became persistent: \(tunnel.logFormat()).")

diff --git a/ios/MullvadVPNTests/AddressCacheTests.swift b/ios/MullvadVPNTests/AddressCacheTests.swift
@@ -42,10 +42,14 @@ final class AddressCacheTests: XCTestCase {
         addressCache.setEndpoints([apiEndpoint])
 
         let dateBeforeUpdate = addressCache.getLastUpdateDate()
+        // Calling `Date()` several times in a row can result in the same Date object being returned.
+        // Force a sleep before setting the next endpoint to avoid getting the same Date object twice in a row.
+        Thread.sleep(forTimeInterval: Duration.milliseconds(10).timeInterval)
         addressCache.setEndpoints([apiEndpoint])
         let dateAfterUpdate = addressCache.getLastUpdateDate()
 
-        XCTAssertNotEqual(dateBeforeUpdate, dateAfterUpdate)
+        let timeDifference = dateAfterUpdate.timeIntervalSince(dateBeforeUpdate)
+        XCTAssertNotEqual(0.0, timeDifference)
     }
 
     func testSetEndpointsDoesNotDoAnythingIfSettingEmptyEndpoints() throws {

diff --git a/ios/MullvadVPNTests/TunnelStore+Stubs.swift b/ios/MullvadVPNTests/TunnelStore+Stubs.swift
@@ -10,11 +10,12 @@ import Foundation
 import NetworkExtension
 
 struct TunnelStoreStub: TunnelStoreProtocol {
-    func getPersistentTunnels() -> [any TunnelProtocol] {
+    typealias TunnelType = TunnelStub
+    func getPersistentTunnels() -> [TunnelType] {
         []
     }
 
-    func createNewTunnel() -> any TunnelProtocol {
+    func createNewTunnel() -> TunnelType {
         TunnelStub(status: .invalid, isOnDemandEnabled: false)
     }
 }
@@ -23,7 +24,11 @@ class DummyTunnelStatusObserver: TunnelStatusObserver {
     func tunnel(_ tunnel: any TunnelProtocol, didReceiveStatus status: NEVPNStatus) {}
 }
 
-class TunnelStub: TunnelProtocol {
+final class TunnelStub: TunnelProtocol, Equatable {
+    convenience init(tunnelProvider: TunnelProviderManagerType) {
+        self.init(status: .invalid, isOnDemandEnabled: false)
+    }
+
     static func == (lhs: TunnelStub, rhs: TunnelStub) -> Bool {
         ObjectIdentifier(lhs) == ObjectIdentifier(rhs)
     }

diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift
@@ -1,5 +1,5 @@
 //
-//  State+.swift
+//  State+Extensions.swift
 //  PacketTunnelCore
 //
 //  Created by pronebird on 08/09/2023.
-Original file line number
+Diff line change
@@ Expand Up / @@ -85,7 +85,7 @@ class StartTunnelOperation: ResultOperation<Void> { @@
             }
         }
-        private func startTunnel(tunnel: Tunnel, selectedRelay: SelectedRelay) throws {
+        private func startTunnel(tunnel: any TunnelProtocol, selectedRelay: SelectedRelay) throws {
             var tunnelOptions = PacketTunnelOptions()
             do {
@@ Expand Down @@