From 4fb7eef8b864455c595378230e3490d1fd490256 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Thu, 23 Jan 2025 19:35:54 +0100 Subject: [PATCH] Change AnyRelay Location from a String to a custom type --- .../ApiHandlers/ServerRelaysResponse.swift | 4 +- ios/MullvadREST/Relay/AnyRelay.swift | 2 +- .../Relay/LocationIdentifier.swift | 73 +++++++++++++++++++ ios/MullvadREST/Relay/RelaySelector.swift | 7 +- ios/MullvadREST/Relay/RelayWithLocation.swift | 8 +- ios/MullvadVPN.xcodeproj/project.pbxproj | 5 +- .../AllLocationDataSource.swift | 11 ++- .../Relay/RelaySelectorTests.swift | 9 +-- .../IPOverrideWrapperTests.swift | 4 +- 9 files changed, 98 insertions(+), 25 deletions(-) create mode 100644 ios/MullvadREST/Relay/LocationIdentifier.swift diff --git a/ios/MullvadREST/ApiHandlers/ServerRelaysResponse.swift b/ios/MullvadREST/ApiHandlers/ServerRelaysResponse.swift index fbc9c5660418..370a803fd7f0 100644 --- a/ios/MullvadREST/ApiHandlers/ServerRelaysResponse.swift +++ b/ios/MullvadREST/ApiHandlers/ServerRelaysResponse.swift @@ -29,7 +29,7 @@ extension REST { public let hostname: String public let active: Bool public let owned: Bool - public let location: String + public let location: LocationIdentifier public let provider: String public let ipv4AddrIn: IPv4Address public let weight: UInt64 @@ -54,7 +54,7 @@ extension REST { public let hostname: String public let active: Bool public let owned: Bool - public let location: String + public let location: LocationIdentifier public let provider: String public let weight: UInt64 public let ipv4AddrIn: IPv4Address diff --git a/ios/MullvadREST/Relay/AnyRelay.swift b/ios/MullvadREST/Relay/AnyRelay.swift index 13f10029b2af..a38e5076c7e6 100644 --- a/ios/MullvadREST/Relay/AnyRelay.swift +++ b/ios/MullvadREST/Relay/AnyRelay.swift @@ -12,7 +12,7 @@ import Network public protocol AnyRelay { var hostname: String { get } var owned: Bool { get } - var location: String { get } + var location: REST.LocationIdentifier { get } var provider: String { get } var weight: UInt64 { get } var active: Bool { get } diff --git a/ios/MullvadREST/Relay/LocationIdentifier.swift b/ios/MullvadREST/Relay/LocationIdentifier.swift new file mode 100644 index 000000000000..53e753277ce4 --- /dev/null +++ b/ios/MullvadREST/Relay/LocationIdentifier.swift @@ -0,0 +1,73 @@ +// +// LocationIdentifier.swift +// MullvadVPN +// +// Created by Andrew Bulhak on 2025-01-24. +// Copyright © 2025 Mullvad VPN AB. All rights reserved. +// + +extension REST { + // locations are currently always "aa-bbb" for some country code aa and city code bbb. Should this change, this type can be extended. + public struct LocationIdentifier: Sendable { + public let country: Substring + public let city: Substring + + fileprivate static func parse(_ input: String) -> (Substring, Substring)? { + let components = input.split(separator: "-") + guard components.count == 2 else { return nil } + return (components[0], components[1]) + } + } +} + +extension REST.LocationIdentifier: RawRepresentable { + public var rawValue: String { country.base } + + public init?(rawValue: String) { + guard let parsed = Self.parse(rawValue) else { return nil } + (country, city) = parsed + } +} + +extension REST.LocationIdentifier: ExpressibleByStringLiteral { + public init(stringLiteral value: StringLiteralType) { + guard let parsed = Self.parse(value) else { + // this is ugly, but it will only ever be called for + // code initialised from a literal in code, and + // never from real-world input, so it'll have to do. + fatalError("Invalid LocationIdentifier: \(value)") + } + (country, city) = parsed + } +} + +// Allow LocationIdentifier to code to/from JSON Strings +extension REST.LocationIdentifier: Codable { + enum ParsingError: Error { + case malformed + } + + public init(from decoder: any Decoder) throws { + let container = try decoder.singleValueContainer() + guard let parsed = Self.parse(try container.decode(String.self)) else { + throw ParsingError.malformed + } + (country, city) = parsed + } + + public func encode(to encoder: any Encoder) throws { + var container = encoder.singleValueContainer() + try container.encode(rawValue) + } +} + +// As the location's values are Substrings of the same String, to which they maintain references, we use the base String for holistic operations such as equality and hashing +extension REST.LocationIdentifier: Hashable { + public static func == (lhs: REST.LocationIdentifier, rhs: REST.LocationIdentifier) -> Bool { + lhs.rawValue == rhs.rawValue + } + + public func hash(into hasher: inout Hasher) { + hasher.combine(rawValue) + } +} diff --git a/ios/MullvadREST/Relay/RelaySelector.swift b/ios/MullvadREST/Relay/RelaySelector.swift index b36729999ef2..008012182877 100644 --- a/ios/MullvadREST/Relay/RelaySelector.swift +++ b/ios/MullvadREST/Relay/RelaySelector.swift @@ -141,14 +141,11 @@ public enum RelaySelector { _ serverLocation: REST.ServerLocation, relay: T ) -> RelayWithLocation? { - let locationComponents = relay.location.split(separator: "-") - guard locationComponents.count > 1 else { return nil } - let location = Location( country: serverLocation.country, - countryCode: String(locationComponents[0]), + countryCode: String(relay.location.country), city: serverLocation.city, - cityCode: String(locationComponents[1]), + cityCode: String(relay.location.city), latitude: serverLocation.latitude, longitude: serverLocation.longitude ) diff --git a/ios/MullvadREST/Relay/RelayWithLocation.swift b/ios/MullvadREST/Relay/RelayWithLocation.swift index d1b8320f57b2..e06016eaedd2 100644 --- a/ios/MullvadREST/Relay/RelayWithLocation.swift +++ b/ios/MullvadREST/Relay/RelayWithLocation.swift @@ -35,18 +35,16 @@ public struct RelayWithLocation { } init?(_ relay: T, locations: [String: REST.ServerLocation]) { - let locationComponents = relay.location.split(separator: "-") guard - locationComponents.count > 1, - let serverLocation = locations[relay.location] + let serverLocation = locations[relay.location.rawValue] else { return nil } self.relay = relay self.serverLocation = Location( country: serverLocation.country, - countryCode: String(locationComponents[0]), + countryCode: String(relay.location.country), city: serverLocation.city, - cityCode: String(locationComponents[1]), + cityCode: String(relay.location.city), latitude: serverLocation.latitude, longitude: serverLocation.longitude ) diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index fe253f586cd4..aa6c57456280 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -65,6 +65,7 @@ 44BB5F972BE527F4002520EB /* TunnelState+UI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44BB5F962BE527F4002520EB /* TunnelState+UI.swift */; }; 44BB5F982BE527F4002520EB /* TunnelState+UI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44BB5F962BE527F4002520EB /* TunnelState+UI.swift */; }; 44C18DE32C74DF93009BE3E1 /* TunnelPinger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 449275432C3C3029000526DE /* TunnelPinger.swift */; }; + 44CAEAA12D442F5E004A8E65 /* LocationIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44CAEAA02D442F56004A8E65 /* LocationIdentifier.swift */; }; 44DD7D242B6CFFD70005F67F /* StartTunnelOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44DD7D232B6CFFD70005F67F /* StartTunnelOperationTests.swift */; }; 44DD7D272B6D18FB0005F67F /* MockTunnelInteractor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44DD7D262B6D18FB0005F67F /* MockTunnelInteractor.swift */; }; 44DD7D292B7113CA0005F67F /* MockTunnel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44DD7D282B7113CA0005F67F /* MockTunnel.swift */; }; @@ -1481,6 +1482,7 @@ 44B3C43C2C00CBBC0079782C /* PacketTunnelActorReducerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PacketTunnelActorReducerTests.swift; sourceTree = ""; }; 44BB5F962BE527F4002520EB /* TunnelState+UI.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TunnelState+UI.swift"; sourceTree = ""; }; 44BB5F992BE529FE002520EB /* TunnelStateTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TunnelStateTests.swift; sourceTree = ""; }; + 44CAEAA02D442F56004A8E65 /* LocationIdentifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationIdentifier.swift; sourceTree = ""; }; 44DD7D232B6CFFD70005F67F /* StartTunnelOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StartTunnelOperationTests.swift; sourceTree = ""; }; 44DD7D262B6D18FB0005F67F /* MockTunnelInteractor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTunnelInteractor.swift; sourceTree = ""; }; 44DD7D282B7113CA0005F67F /* MockTunnel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTunnel.swift; sourceTree = ""; }; @@ -4468,6 +4470,7 @@ 585DA87626B024A600B8C587 /* CachedRelays.swift */, F0DDE4272B220A15006B57A7 /* Haversine.swift */, 7A516C392B7111A700BBD33D /* IPOverrideWrapper.swift */, + 44CAEAA02D442F56004A8E65 /* LocationIdentifier.swift */, F0DDE4292B220A15006B57A7 /* Midpoint.swift */, 7ACE19102C1C349200260BB6 /* MultihopDecisionFlow.swift */, F0F3161A2BF358590078DBCF /* NoRelaysSatisfyingConstraintsError.swift */, @@ -5569,6 +5572,7 @@ A90763C32B2858630045ADF0 /* Socks5Configuration.swift in Sources */, 06799AE428F98E4800ACD94E /* RESTAccountsProxy.swift in Sources */, 5897F1742913EAF800AF5695 /* ExponentialBackoff.swift in Sources */, + 44CAEAA12D442F5E004A8E65 /* LocationIdentifier.swift in Sources */, 06799AE328F98E4800ACD94E /* RESTNetworkOperation.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -6106,7 +6110,6 @@ 5868585524054096000B8131 /* CustomButton.swift in Sources */, 58E25F812837BBBB002CFB2C /* SceneDelegate.swift in Sources */, 7A1A26492A29D48A00B978AA /* RelayFilterCellFactory.swift in Sources */, - 5867771629097C5B006F721F /* ProductState.swift in Sources */, F0D5591E2D38051C0072B63F /* LatestChangesNotificationProvider.swift in Sources */, 7A28826A2BA8336600FD9F20 /* VPNSettingsCoordinator.swift in Sources */, 7A6389DE2B7E3BD6008E77E1 /* CustomListItemIdentifier.swift in Sources */, diff --git a/ios/MullvadVPN/View controllers/SelectLocation/AllLocationDataSource.swift b/ios/MullvadVPN/View controllers/SelectLocation/AllLocationDataSource.swift index ac3c9a3b63ef..ee85954aff25 100644 --- a/ios/MullvadVPN/View controllers/SelectLocation/AllLocationDataSource.swift +++ b/ios/MullvadVPN/View controllers/SelectLocation/AllLocationDataSource.swift @@ -25,12 +25,15 @@ class AllLocationDataSource: LocationDataSourceProtocol { let expandedRelays = nodes.flatMap { [$0] + $0.flattened }.filter { $0.showsChildren }.map { $0.code } for relay in relays.relays { - guard case - let .city(countryCode, cityCode) = RelayLocation(dashSeparatedString: relay.location), - let serverLocation = relays.locations[relay.location] + guard + let serverLocation = relays.locations[relay.location.rawValue] else { continue } - let relayLocation = RelayLocation.hostname(countryCode, cityCode, relay.hostname) + let relayLocation = RelayLocation.hostname( + String(relay.location.country), + String(relay.location.city), + relay.hostname + ) for ancestorOrSelf in relayLocation.ancestors + [relayLocation] { addLocation( diff --git a/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift b/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift index 0385cc5df046..630662bc3753 100644 --- a/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift +++ b/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift @@ -55,16 +55,15 @@ class RelaySelectorTests: XCTestCase { ) let relayWithLocations = sampleRelays.wireguard.relays.map { - let location = sampleRelays.locations[$0.location]! - let locationComponents = $0.location.split(separator: "-") + let location = sampleRelays.locations[$0.location.rawValue]! return RelayWithLocation( relay: $0, serverLocation: Location( country: location.country, - countryCode: String(locationComponents[0]), + countryCode: String($0.location.country), city: location.city, - cityCode: String(locationComponents[1]), + cityCode: String($0.location.city), latitude: location.latitude, longitude: location.longitude ) @@ -138,7 +137,7 @@ class RelaySelectorTests: XCTestCase { func testClosestRelay() throws { let relayWithLocations = try sampleRelays.wireguard.relays.map { - let serverLocation = try XCTUnwrap(sampleRelays.locations[$0.location]) + let serverLocation = try XCTUnwrap(sampleRelays.locations[$0.location.rawValue]) let location = Location( country: serverLocation.country, countryCode: serverLocation.country, diff --git a/ios/MullvadVPNTests/MullvadSettings/IPOverrideWrapperTests.swift b/ios/MullvadVPNTests/MullvadSettings/IPOverrideWrapperTests.swift index 8a71f1b0fc86..6cafd931c2f1 100644 --- a/ios/MullvadVPNTests/MullvadSettings/IPOverrideWrapperTests.swift +++ b/ios/MullvadVPNTests/MullvadSettings/IPOverrideWrapperTests.swift @@ -77,7 +77,7 @@ extension IPOverrideWrapperTests { hostname: "", active: true, owned: true, - location: "", + location: "xx-yyy", provider: "", weight: 0, ipv4AddrIn: .any, @@ -94,7 +94,7 @@ extension IPOverrideWrapperTests { hostname: "", active: true, owned: true, - location: "", + location: "xx-yyy", provider: "", ipv4AddrIn: .any, weight: 0,