-
Notifications
You must be signed in to change notification settings - Fork 38
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
Fix: issue 629 #714
Open
aaryankotharii
wants to merge
1
commit into
OneBusAway:main
Choose a base branch
from
aaryankotharii:fix/issue-629
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+166
−54
Open
Fix: issue 629 #714
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
private let apiService: RegionsAPIService? | ||
private let locationService: LocationService | ||
private let userDefaults: UserDefaults | ||
private let fileManager: RegionsServiceFileManagerProtocol | ||
private let bundledRegionsFilePath: String | ||
private let apiPath: String? | ||
|
||
|
@@ -40,13 +41,15 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
/// - apiService: Retrieves new data from the region server and turns it into models. | ||
/// - locationService: A location service object. | ||
/// - userDefaults: The user defaults object. | ||
/// - fileManager: The file manager object. | ||
/// - bundledRegionsFilePath: The path to the bundled regions file. It is probably named "regions.json" or something similar. | ||
/// - apiPath: The path to the remote regions.json file on the server. e.g. /path/to/regions.json | ||
/// - delegate: A delegate object for callbacks. | ||
public init(apiService: RegionsAPIService?, locationService: LocationService, userDefaults: UserDefaults, bundledRegionsFilePath: String, apiPath: String?, delegate: RegionsServiceDelegate? = nil) { | ||
public init(apiService: RegionsAPIService?, locationService: LocationService, userDefaults: UserDefaults, fileManager: RegionsServiceFileManagerProtocol, bundledRegionsFilePath: String, apiPath: String?, delegate: RegionsServiceDelegate? = nil) { | ||
self.apiService = apiService | ||
self.locationService = locationService | ||
self.userDefaults = userDefaults | ||
self.fileManager = fileManager | ||
self.bundledRegionsFilePath = bundledRegionsFilePath | ||
self.apiPath = apiPath | ||
|
||
|
@@ -55,7 +58,7 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
RegionsService.alwaysRefreshRegionsOnLaunchUserDefaultsKey: false | ||
]) | ||
|
||
if let regions = RegionsService.loadStoredRegions(from: userDefaults), regions.count > 0 { | ||
if let regions = RegionsService.loadStoredRegions(from: fileManager), regions.count > 0 { | ||
self.regions = regions | ||
} | ||
else { | ||
|
@@ -204,17 +207,10 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
|
||
// MARK: - Custom Regions | ||
/// Adds the provided custom region to the RegionsService. | ||
/// If an existing custom region with the same `regionIdentifier` exists, the new region replaces the existing region. | ||
/// - throws: Persistence storage errors. | ||
/// - throws: File system errors. | ||
public func add(customRegion newRegion: Region) async throws { | ||
var regions = customRegions | ||
if let index = regions.firstIndex(where: { $0.regionIdentifier == newRegion.regionIdentifier }) { | ||
regions.remove(at: index) | ||
} | ||
|
||
regions.append(newRegion) | ||
|
||
try userDefaults.encodeUserDefaultsObjects(regions, key: RegionsService.storedCustomRegionsUserDefaultsKey) | ||
let customRegionPath = RegionsService.getCustomRegionPath(customRegionIdentifier: newRegion.regionIdentifier) | ||
try fileManager.save(newRegion, to: customRegionPath) | ||
} | ||
|
||
/// Deletes the custom region. If the region could not be found, this method exits normally. | ||
|
@@ -228,28 +224,27 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
/// - parameter identifier: The region identifier used to find the custom region to delete. | ||
/// - throws: If a custom region with the provided `identifier` could not be found, or is not a custom region, this method will throw. | ||
public func delete(customRegionIdentifier identifier: RegionIdentifier) async throws { | ||
var regions = customRegions | ||
|
||
|
||
guard self.currentRegion?.regionIdentifier != identifier else { | ||
throw UnstructuredError( | ||
"Cannot delete the current selected region", | ||
recoverySuggestion: "Choose a different region to be the currently selected region, before deleting this region.") | ||
} | ||
|
||
guard let index = regions.firstIndex(where: { $0.regionIdentifier == identifier }) else { | ||
return | ||
} | ||
|
||
regions.remove(at: index) | ||
try userDefaults.encodeUserDefaultsObjects(regions, key: RegionsService.storedCustomRegionsUserDefaultsKey) | ||
|
||
let customRegionPath = RegionsService.getCustomRegionPath(customRegionIdentifier: identifier) | ||
try fileManager.remove(at: customRegionPath) | ||
} | ||
|
||
/// Retrieves an array of custom regions loaded from files in the custom regions directory. | ||
/// - Returns: An array of `Region` objects representing custom regions. | ||
public var customRegions: [Region] { | ||
let regions: [Region] | ||
do { | ||
regions = try userDefaults.decodeUserDefaultsObjects(type: [Region].self, key: RegionsService.storedCustomRegionsUserDefaultsKey) ?? [] | ||
} catch { | ||
regions = [] | ||
var regions: [Region] = [] | ||
if let fileURLs = try? fileManager.urls(at: RegionsService.customRegionsPath) { | ||
for url in fileURLs { | ||
if let region = try? fileManager.load(Region.self, from: url) { | ||
regions.append(region) | ||
} | ||
} | ||
} | ||
return regions | ||
} | ||
|
@@ -262,16 +257,20 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
|
||
public static let alwaysRefreshRegionsOnLaunchUserDefaultsKey = "OBAAlwaysRefreshRegionsOnLaunchUserDefaultsKey" | ||
static let automaticallySelectRegionUserDefaultsKey = "OBAAutomaticallySelectRegionUserDefaultsKey" | ||
static let storedRegionsUserDefaultsKey = "OBAStoredRegionsUserDefaultsKey" | ||
static let storedCustomRegionsUserDefaultsKey = "OBAStoredCustomRegionsUserDefaultsKey" | ||
static let currentRegionUserDefaultsKey = "OBACurrentRegionUserDefaultsKey" | ||
static let regionsUpdatedAtUserDefaultsKey = "OBARegionsUpdatedAtUserDefaultsKey" | ||
static let defaultRegionsPath = URL.applicationSupportDirectory.appendingPathComponent("default-regions.json") | ||
static let customRegionsPath = URL.documentsDirectory.appendingPathComponent("Regions/custom-regions") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
private class func getCustomRegionPath(customRegionIdentifier identifier: RegionIdentifier) -> URL { | ||
return customRegionsPath.appendingPathComponent("region-\(identifier).json") | ||
} | ||
|
||
// MARK: - Save Regions | ||
|
||
private func storeRegions() { | ||
do { | ||
try userDefaults.encodeUserDefaultsObjects(regions, key: RegionsService.storedRegionsUserDefaultsKey) | ||
try fileManager.save(regions, to: RegionsService.defaultRegionsPath) | ||
userDefaults.set(Date(), forKey: RegionsService.regionsUpdatedAtUserDefaultsKey) | ||
} | ||
catch { | ||
|
@@ -281,11 +280,11 @@ public class RegionsService: NSObject, LocationServiceDelegate { | |
|
||
// MARK: - Load Stored Regions | ||
|
||
private class func loadStoredRegions(from userDefaults: UserDefaults) -> [Region]? { | ||
private class func loadStoredRegions(from fileManager: RegionsServiceFileManagerProtocol) -> [Region]? { | ||
let regions: [Region] | ||
|
||
do { | ||
regions = try userDefaults.decodeUserDefaultsObjects(type: [Region].self, key: RegionsService.storedRegionsUserDefaultsKey) ?? [] | ||
regions = try fileManager.load([Region].self, from: RegionsService.defaultRegionsPath) | ||
} catch { | ||
return nil | ||
} | ||
|
74 changes: 74 additions & 0 deletions
74
OBAKitCore/Location/Regions/RegionsServiceFileManagerProtocol.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
// | ||
// RegionsServiceFileManagerProtocol.swift | ||
// OBAKit | ||
// | ||
// Copyright © Open Transit Software Foundation | ||
// This source code is licensed under the Apache 2.0 license found in the | ||
// LICENSE file in the root directory of this source tree. | ||
// | ||
|
||
import Foundation | ||
|
||
/// A protocol defining file management operations. | ||
public protocol RegionsServiceFileManagerProtocol: AnyObject { | ||
|
||
/// Saves the Codable object to the specified directory with the given name. | ||
/// | ||
/// - Parameters: | ||
/// - object: The Codable object to save. | ||
/// - destination: The destination specifying where to save the object. | ||
func save<T: Codable>(_ object: T, to destination: URL) throws | ||
|
||
/// Loads a Codable object of the specified type from the file with the given URL. | ||
/// | ||
/// - Parameters: | ||
/// - type: The type of the Codable object to load. | ||
/// - fileURL: The URL of the file to load the object from. | ||
/// - Returns: The decoded Codable object. | ||
func load<T: Codable>(_ type: T.Type, from fileURL: URL) throws -> T | ||
|
||
/// Removes the file at the specified destination. | ||
/// | ||
/// - Parameters: | ||
/// - destination: The destination specifying which file to remove. | ||
func remove(at destination: URL) throws | ||
|
||
/// Returns an array of URLs for the items at the specified URL. | ||
/// | ||
/// - Parameters: | ||
/// - destination: The destination specifying where to load contents of. | ||
/// - Returns: An array of URLs for the items in the directory. | ||
func urls(at destination: URL) throws -> [URL] | ||
|
||
} | ||
|
||
extension FileManager: RegionsServiceFileManagerProtocol { | ||
|
||
/// Creates a directory at the specified URL if it doesn't already exist. | ||
private func createDirectoryIfNeeded(at url: URL) throws { | ||
guard !fileExists(atPath: url.path) else { return } | ||
try createDirectory(at: url, withIntermediateDirectories: true, attributes: nil) | ||
} | ||
|
||
public func urls(at destination: URL) throws -> [URL] { | ||
return try contentsOfDirectory(at: destination, includingPropertiesForKeys: nil) | ||
} | ||
|
||
public func load<T: Codable>(_ type: T.Type, from fileURL: URL) throws -> T { | ||
let data = try Data(contentsOf: fileURL) | ||
return try JSONDecoder().decode(T.self, from: data) | ||
} | ||
|
||
public func save<T: Codable>(_ object: T, to destination: URL) throws { | ||
let directoryURL = destination.deletingLastPathComponent() | ||
try createDirectoryIfNeeded(at: directoryURL) | ||
let encoded = try JSONEncoder().encode(object) | ||
try encoded.write(to: destination) | ||
} | ||
|
||
public func remove(at destination: URL) throws { | ||
guard fileExists(atPath: destination.path) else { return } | ||
try removeItem(at: destination) | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// | ||
// FileManagerMock.swift | ||
// OBAKitTests | ||
// | ||
// Copyright © Open Transit Software Foundation | ||
// This source code is licensed under the Apache 2.0 license found in the | ||
// LICENSE file in the root directory of this source tree. | ||
// | ||
|
||
import Foundation | ||
import OBAKitCore | ||
|
||
class RegionsServiceFileManagerMock: RegionsServiceFileManagerProtocol { | ||
|
||
var savedObjects: [String: Data] = [:] | ||
|
||
func save<T>(_ object: T, to destination: URL) throws where T : Decodable, T : Encodable { | ||
let encodedData = try JSONEncoder().encode(object) | ||
savedObjects[destination.path] = encodedData | ||
} | ||
|
||
func load<T>(_ type: T.Type, from fileURL: URL) throws -> T where T : Decodable { | ||
let data = savedObjects[fileURL.path, default: Data()] | ||
return try JSONDecoder().decode(T.self, from: data) | ||
} | ||
|
||
func remove(at destination: URL) throws { | ||
savedObjects.removeValue(forKey: destination.path) | ||
} | ||
|
||
func urls(at destination: URL) throws -> [URL] { | ||
return [] | ||
} | ||
|
||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this new file manager service, what happens if an existing custom region with the same
regionIdentifier
exists?