reusing existing connection for upcoming requests if it is still valid

8 files changed, 65 insertions, 50 deletions

diff --git a/ios/MullvadREST/Transport/AccessMethodIterator.swift b/ios/MullvadREST/Transport/AccessMethodIterator.swift
index a91c0fd546..34ec668c64 100644
--- a/ios/MullvadREST/Transport/AccessMethodIterator.swift
+++ b/ios/MullvadREST/Transport/AccessMethodIterator.swift
@@ -15,9 +15,12 @@ class AccessMethodIterator {
     private let dataSource: AccessMethodRepositoryDataSource
 
     private var index = 0
-    private var enabledConfigurations: [PersistentAccessMethod] = []
     private var cancellables = Set<Combine.AnyCancellable>()
 
+    private var enabledConfigurations: [PersistentAccessMethod] {
+        dataSource.fetchAll().filter { $0.isEnabled }
+    }
+
     private var lastReachableApiAccessId: UUID {
         lastReachableApiAccessCache.id
     }
@@ -31,9 +34,8 @@ class AccessMethodIterator {
 
         self.dataSource
             .publisher
-            .sink { [weak self] configurations in
+            .sink { [weak self] _ in
                 guard let self else { return }
-                self.enabledConfigurations = configurations.filter { $0.isEnabled }
                 self.refreshCacheIfNeeded()
             }
             .store(in: &cancellables)
@@ -45,7 +47,7 @@ class AccessMethodIterator {
             index = firstIndex
         } else {
             /// When `firstIndex` is `nil`, that means the current configuration is not valid anymore
-            /// Invalidating cache by replacing the `current`  to the next enabled access method
+            /// Invalidating cache by replacing the `current` to the next enabled access method
             lastReachableApiAccessCache.id = pick().id
         }
     }
@@ -57,14 +59,15 @@ class AccessMethodIterator {
     }
 
     func pick() -> PersistentAccessMethod {
-        if enabledConfigurations.isEmpty {
-            /// Returning  `Default` strategy  when  all is disabled
+        let configurations = enabledConfigurations
+        if configurations.isEmpty {
+            /// Returning `Default` strategy  when  all is disabled
             return dataSource.directAccess
         } else {
             /// Picking the next `Enabled` configuration in order they are added
             /// And starting from the beginning when it reaches end
-            let circularIndex = index % enabledConfigurations.count
-            return enabledConfigurations[circularIndex]
+            let circularIndex = index % configurations.count
+            return configurations[circularIndex]
         }
     }
 }
diff --git a/ios/MullvadREST/Transport/Shadowsocks/ShadowsocksConfiguration.swift b/ios/MullvadREST/Transport/Shadowsocks/ShadowsocksConfiguration.swift
index 710b3ef06b..4c9b1a7d6b 100644
--- a/ios/MullvadREST/Transport/Shadowsocks/ShadowsocksConfiguration.swift
+++ b/ios/MullvadREST/Transport/Shadowsocks/ShadowsocksConfiguration.swift
@@ -10,7 +10,7 @@ import Foundation
 import MullvadTypes
 import Network
 
-public struct ShadowsocksConfiguration: Codable {
+public struct ShadowsocksConfiguration: Codable, Equatable {
     public let address: AnyIPAddress
     public let port: UInt16
     public let password: String
diff --git a/ios/MullvadREST/Transport/Socks5/Socks5Configuration.swift b/ios/MullvadREST/Transport/Socks5/Socks5Configuration.swift
index 4f7ebc06f9..9bf2eb87dc 100644
--- a/ios/MullvadREST/Transport/Socks5/Socks5Configuration.swift
+++ b/ios/MullvadREST/Transport/Socks5/Socks5Configuration.swift
@@ -11,7 +11,7 @@ import MullvadTypes
 
 /// Socks5 configuration.
 /// - See: ``URLSessionSocks5Transport``
-public struct Socks5Configuration {
+public struct Socks5Configuration: Equatable {
     /// The socks proxy endpoint.
     public var proxyEndpoint: AnyIPEndpoint
 
diff --git a/ios/MullvadREST/Transport/TransportProvider.swift b/ios/MullvadREST/Transport/TransportProvider.swift
index 10d2feafeb..06aaf6395c 100644
--- a/ios/MullvadREST/Transport/TransportProvider.swift
+++ b/ios/MullvadREST/Transport/TransportProvider.swift
@@ -15,6 +15,7 @@ public final class TransportProvider: RESTTransportProvider {
     private let addressCache: REST.AddressCache
     private var transportStrategy: TransportStrategy
     private var currentTransport: RESTTransport?
+    private var currentTransportType: TransportStrategy.Transport
     private let parallelRequestsMutex = NSLock()
 
     public init(
@@ -25,6 +26,7 @@ public final class TransportProvider: RESTTransportProvider {
         self.urlSessionTransport = urlSessionTransport
         self.addressCache = addressCache
         self.transportStrategy = transportStrategy
+        self.currentTransportType = transportStrategy.connectionTransport()
     }
 
     public func makeTransport() -> RESTTransport? {
@@ -62,26 +64,36 @@ public final class TransportProvider: RESTTransportProvider {
     ///
     /// - Returns: A `RESTTransport` object to make a connection
     private func makeTransportInner() -> RESTTransport? {
-        switch transportStrategy.connectionTransport() {
-        case .direct:
-            currentTransport = urlSessionTransport
-        case let .shadowsocks(configuration):
-            currentTransport = ShadowsocksTransport(
-                urlSession: urlSessionTransport.urlSession,
-                configuration: configuration,
-                addressCache: addressCache
-            )
-        case let .socks5(configuration):
-            currentTransport = URLSessionSocks5Transport(
-                urlSession: urlSessionTransport.urlSession,
-                configuration: configuration,
-                addressCache: addressCache
-            )
-        case .none:
-            currentTransport = nil
+        if currentTransport == nil || shouldNotReuseCurrentTransport {
+            currentTransportType = transportStrategy.connectionTransport()
+            switch currentTransportType {
+            case .direct:
+                currentTransport = urlSessionTransport
+            case let .shadowsocks(configuration):
+                currentTransport = ShadowsocksTransport(
+                    urlSession: urlSessionTransport.urlSession,
+                    configuration: configuration,
+                    addressCache: addressCache
+                )
+            case let .socks5(configuration):
+                currentTransport = URLSessionSocks5Transport(
+                    urlSession: urlSessionTransport.urlSession,
+                    configuration: configuration,
+                    addressCache: addressCache
+                )
+            case .none:
+                currentTransport = nil
+            }
         }
         return currentTransport
     }
+
+    /// The `Main` allows modifications to access methods through the UI.
+    /// The `TransportProvider` relies on a `CurrentTransport` value set during build time or network error.
+    /// To ensure  both process `Packet Tunnel` and `Main` uses the latest changes, the `TransportProvider` compares the `transportType` with the latest value in the cache and reuse it if it's still valid .
+    private var shouldNotReuseCurrentTransport: Bool {
+        currentTransportType != transportStrategy.connectionTransport()
+    }
 }
 
 private extension URLError {
diff --git a/ios/MullvadREST/Transport/TransportStrategy.swift b/ios/MullvadREST/Transport/TransportStrategy.swift
index a63891c601..acb132acab 100644
--- a/ios/MullvadREST/Transport/TransportStrategy.swift
+++ b/ios/MullvadREST/Transport/TransportStrategy.swift
@@ -13,7 +13,7 @@ import MullvadTypes
 
 public class TransportStrategy: Equatable {
     /// The different transports suggested by the strategy
-    public enum Transport {
+    public enum Transport: Equatable {
         /// Connecting a direct connection
         case direct
 
@@ -25,6 +25,19 @@ public class TransportStrategy: Equatable {
 
         /// Failing  to retrive transport
         case none
+
+        public static func == (lhs: Self, rhs: Self) -> Bool {
+            switch (lhs, rhs) {
+            case(.direct, .direct), (.none, .none):
+                return true
+            case let (.shadowsocks(lhsConfiguration), .shadowsocks(rhsConfiguration)):
+                return lhsConfiguration == rhsConfiguration
+            case let (.socks5(lhsConfiguration), .socks5(rhsConfiguration)):
+                return lhsConfiguration == rhsConfiguration
+            default:
+                return false
+            }
+        }
     }
 
     private let shadowsocksLoader: ShadowsocksLoaderProtocol
diff --git a/ios/MullvadRESTTests/AccessMethodRepositoryStub.swift b/ios/MullvadRESTTests/AccessMethodRepositoryStub.swift
index accd7e0f7c..bd8b341654 100644
--- a/ios/MullvadRESTTests/AccessMethodRepositoryStub.swift
+++ b/ios/MullvadRESTTests/AccessMethodRepositoryStub.swift
@@ -12,7 +12,6 @@ import MullvadSettings
 typealias PersistentAccessMethod = MullvadSettings.PersistentAccessMethod
 struct AccessMethodRepositoryStub: AccessMethodRepositoryDataSource {
     var directAccess: MullvadSettings.PersistentAccessMethod
-
     var publisher: AnyPublisher<[MullvadSettings.PersistentAccessMethod], Never> {
         passthroughSubject.eraseToAnyPublisher()
     }
@@ -23,4 +22,8 @@ struct AccessMethodRepositoryStub: AccessMethodRepositoryDataSource {
         directAccess = accessMethods.first(where: { $0.kind == .direct })!
         passthroughSubject.send(accessMethods)
     }
+
+    func fetchAll() -> [MullvadSettings.PersistentAccessMethod] {
+        passthroughSubject.value
+    }
 }
diff --git a/ios/MullvadRESTTests/TransportStrategyTests.swift b/ios/MullvadRESTTests/TransportStrategyTests.swift
index eee045fec2..9acc9e4437 100644
--- a/ios/MullvadRESTTests/TransportStrategyTests.swift
+++ b/ios/MullvadRESTTests/TransportStrategyTests.swift
@@ -238,23 +238,6 @@ class TransportStrategyTests: XCTestCase {
     }
 }
 
-extension TransportStrategy.Transport: Equatable {
-    public static func == (lhs: Self, rhs: Self) -> Bool {
-        switch (lhs, rhs) {
-        case(.direct, .direct), (.none, .none):
-            return true
-        case let (.shadowsocks(config1), .shadowsocks(config2)):
-            return config1.port == config2.port && config1.cipher == config2.cipher && config1.password == config2
-                .password
-        case let (.socks5(config1), .socks5(config2)):
-            return config1.proxyEndpoint == config2.proxyEndpoint && config1.username == config2.username && config1
-                .password == config2.password
-        default:
-            return false
-        }
-    }
-}
-
 private enum IOError: Error {
     case fileNotFound
 }
diff --git a/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift b/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift
index f956b37848..9b0a255b99 100644
--- a/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift
+++ b/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift
@@ -13,7 +13,12 @@ public protocol AccessMethodRepositoryDataSource {
     /// Publisher that propagates a snapshot of persistent store upon modifications.
     var publisher: AnyPublisher<[PersistentAccessMethod], Never> { get }
 
+    /// - Returns: the default strategy.
     var directAccess: PersistentAccessMethod { get }
+
+    /// Fetch all access method from the persistent store.
+    /// - Returns: an array of all persistent access method.
+    func fetchAll() -> [PersistentAccessMethod]
 }
 
 public protocol AccessMethodRepositoryProtocol: AccessMethodRepositoryDataSource {
@@ -34,10 +39,6 @@ public protocol AccessMethodRepositoryProtocol: AccessMethodRepositoryDataSource
     /// - Returns: a persistent access method model upon success, otherwise `nil`.
     func fetch(by id: UUID) -> PersistentAccessMethod?
 
-    /// Fetch all access method from the persistent store.
-    /// - Returns: an array of all persistent access method.
-    func fetchAll() -> [PersistentAccessMethod]
-
     ///  Refreshes the storage with default values.
     func reloadWithDefaultsAfterDataRemoval()
 }