Do not instantiate tunnel settings in RelaySelectorWrapper

mullvad · Sep 3, 2024 · b405566 · b405566
1 parent 74c81ec
commit b405566
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 99 deletions.
diff --git a/ios/MullvadMockData/MullvadREST/RelaySelectorStub.swift b/ios/MullvadMockData/MullvadREST/RelaySelectorStub.swift
@@ -7,6 +7,7 @@
 //
 
 import MullvadREST
+import MullvadSettings
 import MullvadTypes
 import WireGuardKitTypes
 
@@ -18,7 +19,10 @@ public final class RelaySelectorStub: RelaySelectorProtocol {
         self.selectedRelaysResult = selectedRelaysResult
     }
 
-    public func selectRelays(connectionAttemptCount: UInt) throws -> SelectedRelays {
+    public func selectRelays(
+        tunnelSettings: LatestTunnelSettings,
+        connectionAttemptCount: UInt
+    ) throws -> SelectedRelays {
         return try selectedRelaysResult(connectionAttemptCount)
     }
 }

diff --git a/ios/MullvadREST/Relay/RelaySelectorProtocol.swift b/ios/MullvadREST/Relay/RelaySelectorProtocol.swift
@@ -12,7 +12,10 @@ import MullvadTypes
 
 /// Protocol describing a type that can select a relay.
 public protocol RelaySelectorProtocol {
-    func selectRelays(connectionAttemptCount: UInt) throws -> SelectedRelays
+    func selectRelays(
+        tunnelSettings: LatestTunnelSettings,
+        connectionAttemptCount: UInt
+    ) throws -> SelectedRelays
 }
 
 /// Struct describing the selected relay.

diff --git a/ios/MullvadREST/Relay/RelaySelectorWrapper.swift b/ios/MullvadREST/Relay/RelaySelectorWrapper.swift
@@ -12,25 +12,12 @@ import MullvadTypes
 public final class RelaySelectorWrapper: RelaySelectorProtocol {
     let relayCache: RelayCacheProtocol
 
-    let tunnelSettingsUpdater: SettingsUpdater
-    private var tunnelSettings = LatestTunnelSettings()
-    private var observer: SettingsObserverBlock!
-
-    deinit {
-        self.tunnelSettingsUpdater.removeObserver(observer)
-    }
-
-    public init(
-        relayCache: RelayCacheProtocol,
-        tunnelSettingsUpdater: SettingsUpdater
-    ) {
+    public init(relayCache: RelayCacheProtocol) {
         self.relayCache = relayCache
-        self.tunnelSettingsUpdater = tunnelSettingsUpdater
-
-        self.addObserver()
     }
 
     public func selectRelays(
+        tunnelSettings: LatestTunnelSettings,
         connectionAttemptCount: UInt
     ) throws -> SelectedRelays {
         let relays = try relayCache.read().relays
@@ -52,12 +39,4 @@ public final class RelaySelectorWrapper: RelaySelectorProtocol {
             ).pick()
         }
     }
-
-    private func addObserver() {
-        self.observer = SettingsObserverBlock(didUpdateSettings: { [weak self] latestTunnelSettings in
-            self?.tunnelSettings = latestTunnelSettings
-        })
-
-        tunnelSettingsUpdater.addObserver(observer)
-    }
 }
diff --git a/ios/MullvadVPN/AppDelegate.swift b/ios/MullvadVPN/AppDelegate.swift
@@ -93,8 +93,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
         tunnelStore = TunnelStore(application: application)
 
         let relaySelector = RelaySelectorWrapper(
-            relayCache: ipOverrideWrapper,
-            tunnelSettingsUpdater: tunnelSettingsUpdater
+            relayCache: ipOverrideWrapper
         )
         tunnelManager = createTunnelManager(application: application, relaySelector: relaySelector)
 

diff --git a/ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProviderHost.swift b/ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProviderHost.swift
@@ -160,7 +160,11 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
     }
 
     private func pickRelays() throws -> SelectedRelays {
-        return try relaySelector.selectRelays(connectionAttemptCount: 0)
+        let settings = try SettingsManager.readSettings()
+        return try relaySelector.selectRelays(
+            tunnelSettings: settings,
+            connectionAttemptCount: 0
+        )
     }
 
     private func setInternalStateConnected(with selectedRelays: SelectedRelays?) {

diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
@@ -785,7 +785,10 @@ final class TunnelManager: StorePaymentObserver {
     fileprivate func selectRelays() throws -> SelectedRelays {
         let retryAttempts = tunnelStatus.observedState.connectionState?.connectionAttemptCount ?? 0
 
-        return try relaySelector.selectRelays(connectionAttemptCount: retryAttempts)
+        return try relaySelector.selectRelays(
+            tunnelSettings: settings,
+            connectionAttemptCount: retryAttempts
+        )
     }
 
     fileprivate func prepareForVPNConfigurationDeletion() {

diff --git a/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorWrapperTests.swift b/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorWrapperTests.swift
@@ -18,124 +18,100 @@ class RelaySelectorWrapperTests: XCTestCase {
             updatedAt: .distantPast
         ))
     )
+    let multihopWithDaitaConstraints = RelayConstraints(
+        entryLocations: .only(UserSelectedRelays(locations: [.country("es")])), // Relay with DAITA.
+        exitLocations: .only(UserSelectedRelays(locations: [.country("us")]))
+    )
 
-    var relayCache: RelayCache!
-    var settingsUpdater: SettingsUpdater!
-    var settingsListener: TunnelSettingsListener!
+    let multihopWithoutDaitaConstraints = RelayConstraints(
+        entryLocations: .only(UserSelectedRelays(locations: [.country("se")])), // Relay without DAITA.
+        exitLocations: .only(UserSelectedRelays(locations: [.country("us")]))
+    )
 
+    let singlehopConstraints = RelayConstraints(
+        exitLocations: .only(UserSelectedRelays(locations: [.country("se")])) // Relay without DAITA.
+    )
+
+    var relayCache: RelayCache!
     override func setUp() {
         relayCache = RelayCache(fileCache: fileCache)
-        settingsListener = TunnelSettingsListener()
-        settingsUpdater = SettingsUpdater(listener: settingsListener)
     }
 
     func testSelectRelayWithMultihopOff() throws {
-        let wrapper = RelaySelectorWrapper(
-            relayCache: relayCache,
-            tunnelSettingsUpdater: settingsUpdater
-        )
+        let wrapper = RelaySelectorWrapper(relayCache: relayCache)
 
-        settingsListener.onNewSettings?(LatestTunnelSettings(tunnelMultihopState: .off))
+        let settings = LatestTunnelSettings(
+            relayConstraints: singlehopConstraints,
+            tunnelMultihopState: .off,
+            daita: DAITASettings(state: .off)
+        )
 
-        let selectedRelays = try wrapper.selectRelays(connectionAttemptCount: 0)
+        let selectedRelays = try wrapper.selectRelays(tunnelSettings: settings, connectionAttemptCount: 0)
         XCTAssertNil(selectedRelays.entry)
     }
 
     func testSelectRelayWithMultihopOn() throws {
-        let wrapper = RelaySelectorWrapper(
-            relayCache: relayCache,
-            tunnelSettingsUpdater: settingsUpdater
-        )
+        let wrapper = RelaySelectorWrapper(relayCache: relayCache)
 
-        settingsListener.onNewSettings?(LatestTunnelSettings(tunnelMultihopState: .on))
+        let settings = LatestTunnelSettings(
+            relayConstraints: multihopWithDaitaConstraints,
+            tunnelMultihopState: .on,
+            daita: DAITASettings(state: .off)
+        )
 
-        let selectedRelays = try wrapper.selectRelays(connectionAttemptCount: 0)
+        let selectedRelays = try wrapper.selectRelays(tunnelSettings: settings, connectionAttemptCount: 0)
         XCTAssertNotNil(selectedRelays.entry)
     }
 
     func testCanSelectRelayWithMultihopOnAndDaitaOn() throws {
-        let wrapper = RelaySelectorWrapper(
-            relayCache: relayCache,
-            tunnelSettingsUpdater: settingsUpdater
-        )
-
-        let constraints = RelayConstraints(
-            entryLocations: .only(UserSelectedRelays(locations: [.country("es")])), // Relay with DAITA.
-            exitLocations: .only(UserSelectedRelays(locations: [.country("us")]))
-        )
+        let wrapper = RelaySelectorWrapper(relayCache: relayCache)
 
         let settings = LatestTunnelSettings(
-            relayConstraints: constraints,
+            relayConstraints: multihopWithDaitaConstraints,
             tunnelMultihopState: .on,
             daita: DAITASettings(state: .on)
         )
-        settingsListener.onNewSettings?(settings)
 
-        XCTAssertNoThrow(try wrapper.selectRelays(connectionAttemptCount: 0))
+        XCTAssertNoThrow(try wrapper.selectRelays(tunnelSettings: settings, connectionAttemptCount: 0))
     }
 
     func testCannotSelectRelayWithMultihopOnAndDaitaOn() throws {
-        let wrapper = RelaySelectorWrapper(
-            relayCache: relayCache,
-            tunnelSettingsUpdater: settingsUpdater
-        )
-
-        let constraints = RelayConstraints(
-            entryLocations: .only(UserSelectedRelays(locations: [.country("se")])), // Relay without DAITA.
-            exitLocations: .only(UserSelectedRelays(locations: [.country("us")]))
-        )
+        let wrapper = RelaySelectorWrapper(relayCache: relayCache)
 
         let settings = LatestTunnelSettings(
-            relayConstraints: constraints,
+            relayConstraints: multihopWithoutDaitaConstraints,
             tunnelMultihopState: .on,
             daita: DAITASettings(state: .on)
         )
-        settingsListener.onNewSettings?(settings)
 
-        XCTAssertThrowsError(try wrapper.selectRelays(connectionAttemptCount: 0))
+        XCTAssertThrowsError(try wrapper.selectRelays(tunnelSettings: settings, connectionAttemptCount: 0))
     }
 
     func testCanSelectRelayWithMultihopOffAndDaitaOn() throws {
-        let wrapper = RelaySelectorWrapper(
-            relayCache: relayCache,
-            tunnelSettingsUpdater: settingsUpdater
-        )
-
-        let constraints = RelayConstraints(
-            exitLocations: .only(UserSelectedRelays(locations: [.country("es")])) // Relay with DAITA.
-        )
+        let wrapper = RelaySelectorWrapper(relayCache: relayCache)
 
         let settings = LatestTunnelSettings(
-            relayConstraints: constraints,
+            relayConstraints: multihopWithoutDaitaConstraints,
             tunnelMultihopState: .off,
             daita: DAITASettings(state: .on)
         )
-        settingsListener.onNewSettings?(settings)
 
-        let selectedRelays = try wrapper.selectRelays(connectionAttemptCount: 0)
+        let selectedRelays = try wrapper.selectRelays(tunnelSettings: settings, connectionAttemptCount: 0)
         XCTAssertNil(selectedRelays.entry)
     }
 
     // If DAITA is enabled and no supported relays are found, we should try to find the nearest
     // available relay that supports DAITA and use it as entry in a multihop selection.
     func testCanSelectRelayWithMultihopOffAndDaitaOnThroughMultihop() throws {
-        let wrapper = RelaySelectorWrapper(
-            relayCache: relayCache,
-            tunnelSettingsUpdater: settingsUpdater
-        )
-
-        let constraints = RelayConstraints(
-            exitLocations: .only(UserSelectedRelays(locations: [.country("se")])) // Relay without DAITA.
-        )
+        let wrapper = RelaySelectorWrapper(relayCache: relayCache)
 
         let settings = LatestTunnelSettings(
-            relayConstraints: constraints,
+            relayConstraints: singlehopConstraints,
             tunnelMultihopState: .off,
             daita: DAITASettings(state: .on)
         )
-        settingsListener.onNewSettings?(settings)
 
-        let selectedRelays = try wrapper.selectRelays(connectionAttemptCount: 0)
+        let selectedRelays = try wrapper.selectRelays(tunnelSettings: settings, connectionAttemptCount: 0)
         XCTAssertNotNil(selectedRelays.entry)
     }
 }
diff --git a/ios/MullvadVPNTests/MullvadVPN/PacketTunnelCore/PacketTunnelActorReducerTests.swift b/ios/MullvadVPNTests/MullvadVPN/PacketTunnelCore/PacketTunnelActorReducerTests.swift
@@ -7,14 +7,23 @@
 //
 
 import MullvadMockData
+@testable import MullvadREST
+@testable import MullvadSettings
 import MullvadTypes
 @testable import PacketTunnelCore
 import WireGuardKitTypes
 import XCTest
 
 final class PacketTunnelActorReducerTests: XCTestCase {
-    // swiftlint:disable:next force_try
-    let selectedRelays = try! RelaySelectorStub.nonFallible().selectRelays(connectionAttemptCount: 0)
+    var selectedRelays: SelectedRelays!
+
+    override func setUpWithError() throws {
+        let settings = LatestTunnelSettings()
+        selectedRelays = try RelaySelectorStub.nonFallible().selectRelays(
+            tunnelSettings: settings,
+            connectionAttemptCount: 0
+        )
+    }
 
     func makeConnectionData(keyPolicy: State.KeyPolicy = .useCurrent) -> State.ConnectionData {
         State.ConnectionData(

diff --git a/ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelManagerTests.swift b/ios/MullvadVPNTests/MullvadVPN/TunnelManager/TunnelManagerTests.swift
@@ -122,7 +122,10 @@ class TunnelManagerTests: XCTestCase {
         accountProxy.createAccountResult = .success(REST.NewAccountData.mockValue())
 
         let relaySelector = RelaySelectorStub { _ in
-            try RelaySelectorStub.unsatisfied().selectRelays(connectionAttemptCount: 0)
+            try RelaySelectorStub.unsatisfied().selectRelays(
+                tunnelSettings: LatestTunnelSettings(),
+                connectionAttemptCount: 0
+            )
         }
 
         let tunnelManager = TunnelManager(
@@ -148,7 +151,10 @@ class TunnelManagerTests: XCTestCase {
                 case let .error(blockedStateReason) where blockedStateReason == .noRelaysSatisfyingConstraints:
                     blockedExpectation.fulfill()
                     relaySelector.selectedRelaysResult = { connectionAttemptCount in
-                        try RelaySelectorStub.nonFallible().selectRelays(connectionAttemptCount: connectionAttemptCount)
+                        try RelaySelectorStub.nonFallible().selectRelays(
+                            tunnelSettings: LatestTunnelSettings(),
+                            connectionAttemptCount: connectionAttemptCount
+                        )
                     }
                     tunnelManager.reconnectTunnel(selectNewRelay: true)
 

diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift
@@ -75,8 +75,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
 
         deviceChecker = DeviceChecker(accountsProxy: accountsProxy, devicesProxy: devicesProxy)
         relaySelector = RelaySelectorWrapper(
-            relayCache: ipOverrideWrapper,
-            tunnelSettingsUpdater: tunnelSettingsUpdater
+            relayCache: ipOverrideWrapper
         )
 
         actor = PacketTunnelActor(

diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift
@@ -10,6 +10,7 @@ import Foundation
 import MullvadLogging
 import MullvadREST
 import MullvadRustRuntime
+import MullvadSettings
 import MullvadTypes
 import NetworkExtension
 import WireGuardKitTypes
@@ -370,6 +371,7 @@ extension PacketTunnelActor {
                 nextRelays: nextRelays,
                 relayConstraints: settings.relayConstraints,
                 currentRelays: maybeCurrentRelays,
+                tunnelSettings: settings.tunnelSettings,
                 connectionAttemptCount: connectionCount
             )
         }
@@ -488,6 +490,7 @@ extension PacketTunnelActor {
         nextRelays: NextRelays,
         relayConstraints: RelayConstraints,
         currentRelays: SelectedRelays?,
+        tunnelSettings: LatestTunnelSettings,
         connectionAttemptCount: UInt
     ) throws -> SelectedRelays {
         switch nextRelays {
@@ -500,7 +503,10 @@ extension PacketTunnelActor {
             }
 
         case .random:
-            return try relaySelector.selectRelays(connectionAttemptCount: connectionAttemptCount)
+            return try relaySelector.selectRelays(
+                tunnelSettings: tunnelSettings,
+                connectionAttemptCount: connectionAttemptCount
+            )
 
         case let .preSelected(selectedRelays):
             return selectedRelays