From 2197c96bd94211e1ca89a9fdc2557ce8ba1015cb Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 1 Nov 2018 13:19:35 +0100 Subject: [PATCH 1/4] Move serialization URLs to ConnectionService Keep ProfileKey a bare struct. --- .../ConnectionService+Configurations.swift | 3 +- .../Sources/Model/ConnectionService.swift | 48 +++++++++---------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/Passepartout/Sources/Model/ConnectionService+Configurations.swift b/Passepartout/Sources/Model/ConnectionService+Configurations.swift index 625da0d3..c45a04bb 100644 --- a/Passepartout/Sources/Model/ConnectionService+Configurations.swift +++ b/Passepartout/Sources/Model/ConnectionService+Configurations.swift @@ -54,8 +54,7 @@ extension ConnectionService { } private func targetConfigurationURL(for key: ProfileKey) -> URL { - let contextURL = key.contextURL(in: self) - return contextURL.appendingPathComponent(key.id).appendingPathExtension("ovpn") + return contextURL(key).appendingPathComponent(key.id).appendingPathExtension("ovpn") } func pendingConfigurationURLs() -> [URL] { diff --git a/Passepartout/Sources/Model/ConnectionService.swift b/Passepartout/Sources/Model/ConnectionService.swift index 26969aa4..5bf30af4 100644 --- a/Passepartout/Sources/Model/ConnectionService.swift +++ b/Passepartout/Sources/Model/ConnectionService.swift @@ -64,24 +64,6 @@ class ConnectionService: Codable { id = profile.id } - func contextURL(in service: ConnectionService) -> URL { - switch context { - case .provider: - return service.providersURL - - case .host: - return service.hostsURL - } - } - - func profileURL(in service: ConnectionService) -> URL { - return ConnectionService.url(in: contextURL(in: service), forProfileId: id) - } - - func profileData(in service: ConnectionService) throws -> Data { - return try Data(contentsOf: profileURL(in: service)) - } - // MARK: RawRepresentable var rawValue: String { @@ -246,7 +228,7 @@ class ConnectionService: Codable { try? fm.createDirectory(at: hostsURL, withIntermediateDirectories: false, attributes: nil) for key in pendingRemoval { - let url = key.profileURL(in: self) + let url = profileURL(key) try? fm.removeItem(at: url) if let cfg = configurationURL(for: key) { try? fm.removeItem(at: cfg) @@ -255,7 +237,7 @@ class ConnectionService: Codable { for entry in cache.values { if let profile = entry as? ProviderConnectionProfile { do { - let url = ConnectionService.url(in: providersURL, forProfileId: entry.id) + let url = profileURL(ProfileKey(.provider, entry.id)) let data = try encoder.encode(profile) try data.write(to: url) log.debug("Saved provider '\(profile.id)'") @@ -265,7 +247,7 @@ class ConnectionService: Codable { } } else if let profile = entry as? HostConnectionProfile { do { - let url = ConnectionService.url(in: hostsURL, forProfileId: entry.id) + let url = profileURL(ProfileKey(.host, entry.id)) let data = try encoder.encode(profile) try data.write(to: url) log.debug("Saved host '\(profile.id)'") @@ -285,7 +267,7 @@ class ConnectionService: Codable { if let _ = profile as? PlaceholderConnectionProfile { let decoder = JSONDecoder() do { - let data = try key.profileData(in: self) + let data = try profileData(key) switch context { case .provider: profile = try decoder.decode(ProviderConnectionProfile.self, from: data) @@ -306,6 +288,24 @@ class ConnectionService: Codable { return cache.keys.filter { $0.context == context }.map { $0.id } } + func contextURL(_ key: ProfileKey) -> URL { + switch key.context { + case .provider: + return providersURL + + case .host: + return hostsURL + } + } + + func profileURL(_ key: ProfileKey) -> URL { + return contextURL(key).appendingPathComponent(key.id).appendingPathExtension("json") + } + + func profileData(_ key: ProfileKey) throws -> Data { + return try Data(contentsOf: profileURL(key)) + } + private static func profileId(fromURL url: URL) -> String? { guard url.pathExtension == "json" else { return nil @@ -313,10 +313,6 @@ class ConnectionService: Codable { return url.deletingPathExtension().lastPathComponent } - private static func url(in directory: URL, forProfileId profileId: String) -> URL { - return directory.appendingPathComponent(profileId).appendingPathExtension("json") - } - // MARK: Profiles func addProfile(_ profile: ConnectionProfile, credentials: Credentials?) -> Bool { From 36995e089b07268bd0149e582b9733671bf32e07 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 1 Nov 2018 13:25:33 +0100 Subject: [PATCH 2/4] Locate profiles via relative URLs Avoid absolute URLs, profiles directory is always relative to documents. Assume profiles to be based in the documents root unless directory is != nil. Given that, starting from iOS 8, the documents location can change from time to time, could this fix #19? --- .../Model/ConnectionService+Configurations.swift | 2 +- Passepartout/Sources/Model/ConnectionService.swift | 10 +++++++--- Passepartout/Sources/Model/TransientStore.swift | 11 ++--------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/Passepartout/Sources/Model/ConnectionService+Configurations.swift b/Passepartout/Sources/Model/ConnectionService+Configurations.swift index c45a04bb..5d559f7c 100644 --- a/Passepartout/Sources/Model/ConnectionService+Configurations.swift +++ b/Passepartout/Sources/Model/ConnectionService+Configurations.swift @@ -59,7 +59,7 @@ extension ConnectionService { func pendingConfigurationURLs() -> [URL] { do { - let list = try FileManager.default.contentsOfDirectory(at: directory, includingPropertiesForKeys: nil, options: []) + let list = try FileManager.default.contentsOfDirectory(at: rootURL, includingPropertiesForKeys: nil, options: []) return list.filter { $0.pathExtension == "ovpn" } } catch let e { log.error("Could not list imported configurations: \(e)") diff --git a/Passepartout/Sources/Model/ConnectionService.swift b/Passepartout/Sources/Model/ConnectionService.swift index 5bf30af4..345e0378 100644 --- a/Passepartout/Sources/Model/ConnectionService.swift +++ b/Passepartout/Sources/Model/ConnectionService.swift @@ -83,14 +83,18 @@ class ConnectionService: Codable { } } - lazy var directory = FileManager.default.userURL(for: .documentDirectory, appending: nil) + var directory: String? = nil + + var rootURL: URL { + return FileManager.default.userURL(for: .documentDirectory, appending: directory) + } private var providersURL: URL { - return directory.appendingPathComponent(AppConstants.Store.providersDirectory) + return rootURL.appendingPathComponent(AppConstants.Store.providersDirectory) } private var hostsURL: URL { - return directory.appendingPathComponent(AppConstants.Store.hostsDirectory) + return rootURL.appendingPathComponent(AppConstants.Store.hostsDirectory) } private var build: Int diff --git a/Passepartout/Sources/Model/TransientStore.swift b/Passepartout/Sources/Model/TransientStore.swift index 52c7be8a..9525b029 100644 --- a/Passepartout/Sources/Model/TransientStore.swift +++ b/Passepartout/Sources/Model/TransientStore.swift @@ -35,12 +35,10 @@ class TransientStore { static let shared = TransientStore() - private let rootURL: URL - - private let serviceURL: URL - let service: ConnectionService + private let serviceURL = FileManager.default.userURL(for: .documentDirectory, appending: AppConstants.Store.serviceFilename) + var didHandleSubreddit: Bool { get { return UserDefaults.standard.bool(forKey: Keys.didHandleSubreddit) @@ -51,9 +49,6 @@ class TransientStore { } private init() { - rootURL = FileManager.default.userURL(for: .documentDirectory, appending: nil) - serviceURL = rootURL.appendingPathComponent(AppConstants.Store.serviceFilename) - let cfg = AppConstants.VPN.baseConfiguration() do { ConnectionService.migrateJSON(at: serviceURL, to: serviceURL) @@ -64,7 +59,6 @@ class TransientStore { log.verbose(content) } service = try JSONDecoder().decode(ConnectionService.self, from: data) - service.directory = rootURL service.baseConfiguration = cfg service.loadProfiles() } catch let e { @@ -73,7 +67,6 @@ class TransientStore { withAppGroup: GroupConstants.App.appGroup, baseConfiguration: cfg ) - service.directory = rootURL // // hardcoded loading // _ = service.addProfile(ProviderConnectionProfile(name: .pia), credentials: nil) From 964a4d701b52fd9f126fd7d68afa908b9b672b9e Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 1 Nov 2018 13:31:08 +0100 Subject: [PATCH 3/4] Make serviceURL an external static var --- Passepartout/Sources/Model/TransientStore.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Passepartout/Sources/Model/TransientStore.swift b/Passepartout/Sources/Model/TransientStore.swift index 9525b029..0b65b692 100644 --- a/Passepartout/Sources/Model/TransientStore.swift +++ b/Passepartout/Sources/Model/TransientStore.swift @@ -35,10 +35,12 @@ class TransientStore { static let shared = TransientStore() + private static var serviceURL: URL { + return FileManager.default.userURL(for: .documentDirectory, appending: AppConstants.Store.serviceFilename) + } + let service: ConnectionService - private let serviceURL = FileManager.default.userURL(for: .documentDirectory, appending: AppConstants.Store.serviceFilename) - var didHandleSubreddit: Bool { get { return UserDefaults.standard.bool(forKey: Keys.didHandleSubreddit) @@ -51,9 +53,9 @@ class TransientStore { private init() { let cfg = AppConstants.VPN.baseConfiguration() do { - ConnectionService.migrateJSON(at: serviceURL, to: serviceURL) + ConnectionService.migrateJSON(at: TransientStore.serviceURL, to: TransientStore.serviceURL) - let data = try Data(contentsOf: serviceURL) + let data = try Data(contentsOf: TransientStore.serviceURL) if let content = String(data: data, encoding: .utf8) { log.verbose("Service JSON:") log.verbose(content) @@ -76,7 +78,7 @@ class TransientStore { } func serialize() { - try? JSONEncoder().encode(service).write(to: serviceURL) + try? JSONEncoder().encode(service).write(to: TransientStore.serviceURL) try? service.saveProfiles() } } From a4f46d0d7bb5d5ac83f3868dbc0e2901b5627522 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 1 Nov 2018 13:27:11 +0100 Subject: [PATCH 4/4] Reword imported hosts title --- Passepartout/Resources/en.lproj/Localizable.strings | 2 +- Passepartout/Sources/SwiftGen+Strings.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Passepartout/Resources/en.lproj/Localizable.strings b/Passepartout/Resources/en.lproj/Localizable.strings index 96259a89..43cd719f 100644 --- a/Passepartout/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Resources/en.lproj/Localizable.strings @@ -59,7 +59,7 @@ "parsed_file.alerts.parsing.message" = "Unable to parse the provided configuration file (%@)."; "parsed_file.alerts.buttons.report" = "Report an issue"; -"imported_hosts.title" = "Imported"; +"imported_hosts.title" = "Imported hosts"; "service.welcome.message" = "Welcome to Passepartout!\n\nUse the organizer to add a new profile."; "service.sections.general.header" = "General"; diff --git a/Passepartout/Sources/SwiftGen+Strings.swift b/Passepartout/Sources/SwiftGen+Strings.swift index 2344eb0f..2ef727f9 100644 --- a/Passepartout/Sources/SwiftGen+Strings.swift +++ b/Passepartout/Sources/SwiftGen+Strings.swift @@ -246,7 +246,7 @@ internal enum L10n { } internal enum ImportedHosts { - /// Imported + /// Imported hosts internal static let title = L10n.tr("Localizable", "imported_hosts.title") }