From 8da7d998271890a68ae648596df89cfbe90b0ead Mon Sep 17 00:00:00 2001 From: mojganii Date: Thu, 18 Jan 2024 10:01:31 +0100 Subject: [PATCH] reusing existing connection for requests --- .../Transport/AccessMethodIterator.swift | 8 ++-- .../ShadowsocksConfiguration.swift | 2 +- .../Socks5/Socks5Configuration.swift | 2 +- .../Transport/TransportProvider.swift | 46 ++++++++++++------- .../Transport/TransportStrategy.swift | 15 +++++- .../AccessMethodRepositoryStub.swift | 4 ++ .../TransportStrategyTests.swift | 17 ------- .../AccessMethodRepository.swift | 4 ++ .../AccessMethodRepositoryProtocol.swift | 2 + 9 files changed, 60 insertions(+), 40 deletions(-) diff --git a/ios/MullvadREST/Transport/AccessMethodIterator.swift b/ios/MullvadREST/Transport/AccessMethodIterator.swift index a91c0fd54633..4afa6c414996 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() + private var enabledConfigurations: [PersistentAccessMethod] { + dataSource.accessMethods.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) diff --git a/ios/MullvadREST/Transport/Shadowsocks/ShadowsocksConfiguration.swift b/ios/MullvadREST/Transport/Shadowsocks/ShadowsocksConfiguration.swift index 710b3ef06b82..4c9b1a7d6bd2 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 4f7ebc06f94b..9bf2eb87dc2b 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 10d2feafeb52..66abb1814494 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 a63891c601b0..acb132acabd3 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 accd7e0f7ce6..e7a699767501 100644 --- a/ios/MullvadRESTTests/AccessMethodRepositoryStub.swift +++ b/ios/MullvadRESTTests/AccessMethodRepositoryStub.swift @@ -17,6 +17,10 @@ struct AccessMethodRepositoryStub: AccessMethodRepositoryDataSource { passthroughSubject.eraseToAnyPublisher() } + var accessMethods: [PersistentAccessMethod] { + passthroughSubject.value + } + let passthroughSubject: CurrentValueSubject<[PersistentAccessMethod], Never> = CurrentValueSubject([]) init(accessMethods: [PersistentAccessMethod]) { diff --git a/ios/MullvadRESTTests/TransportStrategyTests.swift b/ios/MullvadRESTTests/TransportStrategyTests.swift index eee045fec28e..9acc9e443743 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/AccessMethodRepository.swift b/ios/MullvadSettings/AccessMethodRepository.swift index 6f2b253b2f63..845cee8c204a 100644 --- a/ios/MullvadSettings/AccessMethodRepository.swift +++ b/ios/MullvadSettings/AccessMethodRepository.swift @@ -32,6 +32,10 @@ public class AccessMethodRepository: AccessMethodRepositoryProtocol { direct } + public var accessMethods: [PersistentAccessMethod] { + fetchAll() + } + public init() { add([direct, bridge]) } diff --git a/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift b/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift index f956b37848d6..8b0997f53a75 100644 --- a/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift +++ b/ios/MullvadSettings/AccessMethodRepositoryProtocol.swift @@ -14,6 +14,8 @@ public protocol AccessMethodRepositoryDataSource { var publisher: AnyPublisher<[PersistentAccessMethod], Never> { get } var directAccess: PersistentAccessMethod { get } + + var accessMethods: [PersistentAccessMethod] { get } } public protocol AccessMethodRepositoryProtocol: AccessMethodRepositoryDataSource {