Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store the raw JSON payload from the getRelays API call #6767

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions ios/MullvadREST/ApiHandlers/RESTAPIProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,14 @@ extension REST {
switch httpStatus {
case let httpStatus where httpStatus.isSuccess:
return .decoding {
let serverRelays = try self.responseDecoder.decode(
// Discarding result since we're only interested in knowing that it's parseable.
_ = try self.responseDecoder.decode(
ServerRelaysResponse.self,
from: data
)
let newEtag = response.value(forHTTPHeaderField: HTTPHeader.etag)

return .newContent(newEtag, serverRelays)
return .newContent(newEtag, data)
}

case .notModified where etag != nil:
Expand Down Expand Up @@ -284,7 +285,7 @@ extension REST {

public enum ServerRelaysCacheResponse {
case notModified
case newContent(_ etag: String?, _ value: ServerRelaysResponse)
case newContent(_ etag: String?, _ rawData: Data)
}

private struct CreateApplePaymentRequest: Encodable {
Expand Down
2 changes: 1 addition & 1 deletion ios/MullvadREST/Relay/CachedRelays.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import Foundation

/// A struct that represents the relay cache on disk
/// A struct that represents the relay cache in memory
public struct CachedRelays: Codable, Equatable {
/// E-tag returned by server
public let etag: String?
Expand Down
12 changes: 7 additions & 5 deletions ios/MullvadREST/Relay/IPOverrideWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,23 @@ public class IPOverrideWrapper: RelayCacheProtocol {
self.ipOverrideRepository = ipOverrideRepository
}

public func read() throws -> CachedRelays {
public func read() throws -> StoredRelays {
let cache = try relayCache.read()
let relayResponse = apply(overrides: ipOverrideRepository.fetchAll(), to: cache.relays)
let rawData = try REST.Coding.makeJSONEncoder().encode(relayResponse)

return CachedRelays(relays: relayResponse, updatedAt: cache.updatedAt)
return try StoredRelays(etag: cache.etag, rawData: rawData, updatedAt: cache.updatedAt)
}

public func readPrebundledRelays() throws -> CachedRelays {
public func readPrebundledRelays() throws -> StoredRelays {
let prebundledRelays = try relayCache.readPrebundledRelays()
let relayResponse = apply(overrides: ipOverrideRepository.fetchAll(), to: prebundledRelays.relays)
let rawData = try REST.Coding.makeJSONEncoder().encode(relayResponse)

return CachedRelays(relays: relayResponse, updatedAt: prebundledRelays.updatedAt)
return try StoredRelays(etag: prebundledRelays.etag, rawData: rawData, updatedAt: prebundledRelays.updatedAt)
}

public func write(record: CachedRelays) throws {
public func write(record: StoredRelays) throws {
try relayCache.write(record: record)
}

Expand Down
43 changes: 25 additions & 18 deletions ios/MullvadREST/Relay/RelayCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,59 +12,66 @@ import MullvadTypes
public protocol RelayCacheProtocol {
/// Reads from a cached list,
/// which falls back to reading from prebundled relays if there was no cache hit
func read() throws -> CachedRelays
func read() throws -> StoredRelays
/// Reads the relays file that were prebundled with the app installation.
///
/// > Warning: Prefer `read()` over this unless there is an explicit need to read
/// relays from the bundle, because those might contain stale data.
func readPrebundledRelays() throws -> CachedRelays
func write(record: CachedRelays) throws
func readPrebundledRelays() throws -> StoredRelays
func write(record: StoredRelays) throws
}

/// - Warning: `RelayCache` should not be used directly. It should be used through `IPOverrideWrapper` to have
/// ip overrides applied.
public final class RelayCache: RelayCacheProtocol {
private let fileCache: any FileCacheProtocol<CachedRelays>
private let fileURL: URL
private let fileCache: any FileCacheProtocol<StoredRelays>

/// Designated initializer
public init(cacheDirectory: URL) {
fileCache = FileCache(fileURL: cacheDirectory.appendingPathComponent("relays.json", isDirectory: false))
fileURL = cacheDirectory.appendingPathComponent("relays.json", isDirectory: false)
fileCache = FileCache(fileURL: fileURL)
}

/// Initializer that accepts a custom FileCache implementation. Used in tests.
init(fileCache: some FileCacheProtocol<CachedRelays>) {
init(fileCache: some FileCacheProtocol<StoredRelays>) {
fileURL = FileManager.default.temporaryDirectory.appendingPathComponent("relays.json", isDirectory: false)
self.fileCache = fileCache
}

/// Safely read the cache file from disk using file coordinator and fallback to prebundled
/// relays in case if the relay cache file is missing.
public func read() throws -> CachedRelays {
/// Safely read the cache file from disk using file coordinator and fallback in the following manner:
/// 1. If there is a file but it's not decodable, try to parse into the old cache format. If it's still
/// not decodable, read the pre-bundled data.
/// 2. If there is no file, read from the pre-bundled data.
public func read() throws -> StoredRelays {
do {
return try fileCache.read()
} catch {
if error is DecodingError || (error as? CocoaError)?.code == .fileReadNoSuchFile {
} catch is DecodingError {
do {
let oldFormatFileCache = FileCache<CachedRelays>(fileURL: fileURL)
return try StoredRelays(cachedRelays: try oldFormatFileCache.read())
} catch {
return try readPrebundledRelays()
} else {
throw error
}
} catch {
return try readPrebundledRelays()
}
}

/// Safely write the cache file on disk using file coordinator.
public func write(record: CachedRelays) throws {
public func write(record: StoredRelays) throws {
try fileCache.write(record)
}

/// Read pre-bundled relays file from disk.
public func readPrebundledRelays() throws -> CachedRelays {
public func readPrebundledRelays() throws -> StoredRelays {
guard let prebundledRelaysFileURL = Bundle(for: Self.self).url(forResource: "relays", withExtension: "json")
else { throw CocoaError(.fileNoSuchFile) }

let data = try Data(contentsOf: prebundledRelaysFileURL)
let relays = try REST.Coding.makeJSONDecoder().decode(REST.ServerRelaysResponse.self, from: data)

return CachedRelays(
relays: relays,
return try StoredRelays(
rawData: data,
updatedAt: Date(timeIntervalSince1970: 0)
)
}
Expand Down
43 changes: 43 additions & 0 deletions ios/MullvadREST/Relay/StoredRelays.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//
// StoredRelays.swift
// MullvadVPNUITests
//
// Created by Jon Petersson on 2024-09-09.
// Copyright © 2024 Mullvad VPN AB. All rights reserved.
//

import Foundation

/// A struct that represents the relay cache on disk
public struct StoredRelays: Codable, Equatable {
/// E-tag returned by server
public let etag: String?

/// The raw relay JSON data stored within the cache entry
public let rawData: Data

/// The date when this cache was last updated
public let updatedAt: Date

/// Relays parsed from the JSON data
public let relays: REST.ServerRelaysResponse

/// `CachedRelays` representation
public var cachedRelays: CachedRelays {
CachedRelays(etag: etag, relays: relays, updatedAt: updatedAt)
}

public init(etag: String? = nil, rawData: Data, updatedAt: Date) throws {
self.etag = etag
self.rawData = rawData
self.updatedAt = updatedAt
relays = try REST.Coding.makeJSONDecoder().decode(REST.ServerRelaysResponse.self, from: rawData)
}

public init(cachedRelays: CachedRelays) throws {
etag = cachedRelays.etag
rawData = try REST.Coding.makeJSONEncoder().encode(cachedRelays.relays)
updatedAt = cachedRelays.updatedAt
relays = cachedRelays.relays
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@

public func getBridges() throws -> REST.ServerShadowsocks? {
let cachedRelays = try relayCache.read()
return RelaySelector.Shadowsocks.tcpBridge(from: cachedRelays.relays)
return RelaySelector.Shadowsocks.tcpBridge(from: try cachedRelays.relays)

Check warning on line 46 in ios/MullvadREST/Transport/Shadowsocks/ShadowsocksRelaySelector.swift

View workflow job for this annotation

GitHub Actions / Unit tests

no calls to throwing functions occur within 'try' expression
}
}
4 changes: 4 additions & 0 deletions ios/MullvadVPN.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@
7A9FA1422A2E3306000B728D /* CheckboxView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A9FA1412A2E3306000B728D /* CheckboxView.swift */; };
7A9FA1442A2E3FE5000B728D /* CheckableSettingsCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A9FA1432A2E3FE5000B728D /* CheckableSettingsCell.swift */; };
7AA513862BC91C6B00D081A4 /* LogRotationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */; };
7AA7046A2C8EFE2B0045699D /* StoredRelays.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA704682C8EFE050045699D /* StoredRelays.swift */; };
7AB2B6702BA1EB8C00B03E3B /* ListCustomListViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB2B66E2BA1EB8C00B03E3B /* ListCustomListViewController.swift */; };
7AB2B6712BA1EB8C00B03E3B /* ListCustomListCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB2B66F2BA1EB8C00B03E3B /* ListCustomListCoordinator.swift */; };
7AB3BEB52BD7A6CB00E34384 /* LocationViewControllerWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB3BEB42BD7A6CB00E34384 /* LocationViewControllerWrapper.swift */; };
Expand Down Expand Up @@ -1888,6 +1889,7 @@
7A9FA1412A2E3306000B728D /* CheckboxView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CheckboxView.swift; sourceTree = "<group>"; };
7A9FA1432A2E3FE5000B728D /* CheckableSettingsCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CheckableSettingsCell.swift; sourceTree = "<group>"; };
7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LogRotationTests.swift; sourceTree = "<group>"; };
7AA704682C8EFE050045699D /* StoredRelays.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoredRelays.swift; sourceTree = "<group>"; };
7AB2B66E2BA1EB8C00B03E3B /* ListCustomListViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ListCustomListViewController.swift; sourceTree = "<group>"; };
7AB2B66F2BA1EB8C00B03E3B /* ListCustomListCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ListCustomListCoordinator.swift; sourceTree = "<group>"; };
7AB3BEB42BD7A6CB00E34384 /* LocationViewControllerWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationViewControllerWrapper.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4159,6 +4161,7 @@
7AEBA5292C2179F20018BEC5 /* RelaySelectorWrapper.swift */,
F0B894F02BF751E300817A42 /* RelayWithDistance.swift */,
F0B894EE2BF751C500817A42 /* RelayWithLocation.swift */,
7AA704682C8EFE050045699D /* StoredRelays.swift */,
);
path = Relay;
sourceTree = "<group>";
Expand Down Expand Up @@ -5190,6 +5193,7 @@
A90763B32B2857D50045ADF0 /* Socks5Authentication.swift in Sources */,
06799ADB28F98E4800ACD94E /* RESTProxyFactory.swift in Sources */,
F0DDE4182B220458006B57A7 /* ShadowsocksConfiguration.swift in Sources */,
7AA7046A2C8EFE2B0045699D /* StoredRelays.swift in Sources */,
06799AF228F98E4800ACD94E /* RESTAccessTokenManager.swift in Sources */,
A90763B12B2857D50045ADF0 /* Socks5Endpoint.swift in Sources */,
06799AF328F98E4800ACD94E /* RESTAuthenticationProxy.swift in Sources */,
Expand Down
38 changes: 22 additions & 16 deletions ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol {
cache = relayCache

do {
cachedRelays = try cache.read()
cachedRelays = try cache.read().cachedRelays
try hotfixRelaysThatDoNotHaveDaita()
} catch {
logger.error(
Expand All @@ -89,8 +89,8 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol {
// If the cached relays already have daita information, this fix is not necessary
guard daitaPropertyMissing else { return }

let preBundledRelays = try cache.readPrebundledRelays()
let preBundledDaitaRelays = preBundledRelays.relays.wireguard.relays.filter { $0.daita == true }
let preBundledRelays = try cache.readPrebundledRelays().relays
let preBundledDaitaRelays = preBundledRelays.wireguard.relays.filter { $0.daita == true }
var cachedRelaysWithFixedDaita = cachedRelays.relays.wireguard.relays

// For each daita enabled relay in the prebundled relays
Expand Down Expand Up @@ -119,9 +119,19 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol {
bridge: cachedRelays.relays.bridge
)

let updatedCachedRelays = CachedRelays(relays: updatedRelays, updatedAt: Date())
let updatedRawRelayData = try REST.Coding.makeJSONEncoder().encode(updatedRelays)
let updatedCachedRelays = try StoredRelays(
etag: cachedRelays.etag,
rawData: updatedRawRelayData,
updatedAt: cachedRelays.updatedAt
)

try cache.write(record: updatedCachedRelays)
self.cachedRelays = updatedCachedRelays
self.cachedRelays = CachedRelays(
etag: cachedRelays.etag,
relays: updatedRelays,
updatedAt: cachedRelays.updatedAt
)
}

func startPeriodicUpdates() {
Expand Down Expand Up @@ -196,7 +206,7 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol {
}

func refreshCachedRelays() throws {
let newCachedRelays = try cache.read()
let newCachedRelays = try cache.read().cachedRelays

relayCacheLock.lock()
cachedRelays = newCachedRelays
Expand Down Expand Up @@ -244,8 +254,8 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol {
-> Result<RelaysFetchResult, Error> {
result.tryMap { response -> RelaysFetchResult in
switch response {
case let .newContent(etag, relays):
try self.storeResponse(etag: etag, relays: relays)
case let .newContent(etag, rawData):
try self.storeResponse(etag: etag, rawData: rawData)

return .newContent

Expand All @@ -262,18 +272,14 @@ final class RelayCacheTracker: RelayCacheTrackerProtocol {
}
}

private func storeResponse(etag: String?, relays: REST.ServerRelaysResponse) throws {
let numRelays = relays.wireguard.relays.count

logger.info("Downloaded \(numRelays) relays.")

let newCachedRelays = CachedRelays(
private func storeResponse(etag: String?, rawData: Data) throws {
let newCachedData = try StoredRelays(
etag: etag,
relays: relays,
rawData: rawData,
updatedAt: Date()
)

try cache.write(record: newCachedRelays)
try cache.write(record: newCachedData)
try refreshCachedRelays()
}

Expand Down
22 changes: 16 additions & 6 deletions ios/MullvadVPNTests/MullvadREST/Relay/RelayCacheTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import XCTest
final class RelayCacheTests: XCTestCase {
func testReadCache() throws {
let fileCache = MockFileCache(
initialState: .exists(CachedRelays(relays: .mock(), updatedAt: .distantPast))
initialState: .exists(try StoredRelays(rawData: try .mock(), updatedAt: .distantPast))
)
let cache = RelayCache(fileCache: fileCache)
let relays = try XCTUnwrap(cache.read())
Expand All @@ -22,17 +22,17 @@ final class RelayCacheTests: XCTestCase {

func testWriteCache() throws {
let fileCache = MockFileCache(
initialState: .exists(CachedRelays(relays: .mock(), updatedAt: .distantPast))
initialState: .exists(try StoredRelays(rawData: try .mock(), updatedAt: .distantPast))
)
let cache = RelayCache(fileCache: fileCache)
let newCachedRelays = CachedRelays(relays: .mock(), updatedAt: Date())
let newCachedRelays = try StoredRelays(rawData: try .mock(), updatedAt: Date())

try cache.write(record: newCachedRelays)
XCTAssertEqual(fileCache.getState(), .exists(newCachedRelays))
}

func testReadPrebundledRelaysWhenNoCacheIsStored() throws {
let fileCache = MockFileCache<CachedRelays>(initialState: .fileNotFound)
let fileCache = MockFileCache<StoredRelays>(initialState: .fileNotFound)
let cache = RelayCache(fileCache: fileCache)

XCTAssertNoThrow(try cache.read())
Expand All @@ -42,7 +42,7 @@ final class RelayCacheTests: XCTestCase {
extension REST.ServerRelaysResponse {
static func mock(
serverRelays: [REST.ServerRelay] = [],
brideRelays: [REST.BridgeRelay] = []
bridgeRelays: [REST.BridgeRelay] = []
) -> Self {
REST.ServerRelaysResponse(
locations: [:],
Expand All @@ -52,7 +52,17 @@ extension REST.ServerRelaysResponse {
portRanges: [],
relays: serverRelays
),
bridge: REST.ServerBridges(shadowsocks: [], relays: brideRelays)
bridge: REST.ServerBridges(shadowsocks: [], relays: bridgeRelays)
)
}
}

extension Data {
static func mock(
serverRelays: [REST.ServerRelay] = [],
bridgeRelays: [REST.BridgeRelay] = []
) throws -> Data {
let relays = REST.ServerRelaysResponse.mock(serverRelays: serverRelays, bridgeRelays: bridgeRelays)
return try REST.Coding.makeJSONEncoder().encode(relays)
}
}
Loading
Loading