From b5a3e79242a132b15f329f2d0b201e31f4ac3e85 Mon Sep 17 00:00:00 2001 From: Theo Spears Date: Wed, 28 Feb 2024 19:33:13 -0800 Subject: [PATCH] Do not hard-crash on missing usernames (#448) Previously we were using `username!` to look up usernames. This would throw a fatal exception if the username was not available, crashing the app. In particular attempts to update HealthKit metrics could hit this after logging out from the app. Avoid this by centralizing the username lookup logic in the request manager making URLs in the app slightly more readable, and have it throw a catchable exception. This could theoretically cause issues in the case that a URL contains an additional `{username}` string, but this seems unlikely. Testing: Launched the app on my phone and checked it could load goals. Signed out and did not immediately experience a crash. --- BeeKit/Goal.swift | 10 +++++----- BeeKit/Managers/CurrentUserManager.swift | 8 ++++++-- BeeKit/Managers/RequestManager.swift | 19 ++++++++++++++----- BeeSwift/EditDatapointViewController.swift | 4 ++-- .../ConfigureHKMetricViewController.swift | 2 +- ...itDefaultNotificationsViewController.swift | 6 +++--- .../EditGoalNotificationsViewController.swift | 10 +++++----- .../RemoveHKMetricViewController.swift | 2 +- BeeSwift/TimerViewController.swift | 2 +- 9 files changed, 38 insertions(+), 25 deletions(-) diff --git a/BeeKit/Goal.swift b/BeeKit/Goal.swift index ff2a38a52..579956876 100644 --- a/BeeKit/Goal.swift +++ b/BeeKit/Goal.swift @@ -350,7 +350,7 @@ public class Goal { Task { @MainActor in let params = ["sort" : "daystamp", "count" : 7] as [String : Any] do { - let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints.json", parameters: params) + let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints.json", parameters: params) let responseJSON = JSON(response!) success(try ExistingDataPoint.fromJSONArray(array: responseJSON.arrayValue)) } catch { @@ -377,7 +377,7 @@ public class Goal { "comment": "Auto-updated via Apple Health", ] do { - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints/\(datapoint.id).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints/\(datapoint.id).json", parameters: params) success?() } catch { errorCompletion?() @@ -388,7 +388,7 @@ public class Goal { func deleteDatapoint(datapoint : ExistingDataPoint) { Task { @MainActor in do { - let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints/\(datapoint.id)", parameters: nil) + let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints/\(datapoint.id)", parameters: nil) } catch { logger.error("Error deleting datapoint: \(error)") } @@ -398,7 +398,7 @@ public class Goal { func postDatapoint(params : [String : String], success : ((Any?) -> Void)?, failure : ((Error?, String?) -> Void)?) { Task { @MainActor in do { - let response = try await ServiceLocator.requestManager.post(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints.json", parameters: params) + let response = try await ServiceLocator.requestManager.post(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints.json", parameters: params) success?(response) } catch { failure?(error, error.localizedDescription) @@ -408,7 +408,7 @@ public class Goal { func fetchDatapoints(sort: String, per: Int, page: Int) async throws -> [ExistingDataPoint] { let params = ["sort" : sort, "per" : per, "page": page] as [String : Any] - let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug)/datapoints.json", parameters: params) + let response = try await ServiceLocator.requestManager.get(url: "api/v1/users/{username}/goals/\(self.slug)/datapoints.json", parameters: params) let responseJSON = JSON(response!) return try ExistingDataPoint.fromJSONArray(array: responseJSON.arrayValue) } diff --git a/BeeKit/Managers/CurrentUserManager.swift b/BeeKit/Managers/CurrentUserManager.swift index 9dcb1901e..aef079a78 100644 --- a/BeeKit/Managers/CurrentUserManager.swift +++ b/BeeKit/Managers/CurrentUserManager.swift @@ -110,7 +110,7 @@ public class CurrentUserManager { // Fetch a user from the persistent store let request = NSFetchRequest(entityName: "User") // TODO: Handle (or at least log) an error here - let users = try? (context ?? container.viewContext).fetch(request) + let users = try? (context ?? container.newBackgroundContext()).fetch(request) return users?.first } @@ -153,7 +153,7 @@ public class CurrentUserManager { public var username :String? { return user()?.username } - + public func defaultLeadTime() -> NSNumber { return (user()?.defaultLeadTime ?? 0) as NSNumber } @@ -281,3 +281,7 @@ public class CurrentUserManager { }.value } } + +public enum CurrentUserManagerError : Error { + case loggedOut +} diff --git a/BeeKit/Managers/RequestManager.swift b/BeeKit/Managers/RequestManager.swift index 48bf39f4f..a8885c5ff 100644 --- a/BeeKit/Managers/RequestManager.swift +++ b/BeeKit/Managers/RequestManager.swift @@ -13,9 +13,9 @@ import OSLog struct ServerError: Error { let message: String - let requestError: Error + let requestError: Error? - init(_ message: String, requestError: Error) { + init(_ message: String, requestError: Error?) { self.message = message self.requestError = requestError } @@ -30,9 +30,18 @@ public class RequestManager { private let logger = Logger(subsystem: "com.beeminder.beeminder", category: "RequestManager") func rawRequest(url: String, method: HTTPMethod, parameters: [String: Any]?, headers: HTTPHeaders) async throws -> Any? { - let encoding: ParameterEncoding = if method == .get { URLEncoding.default } else { JSONEncoding.default } + + var urlWithSubstitutions = url + if url.contains("{username}") { + guard let username = ServiceLocator.currentUserManager.username else { + throw ServerError("Attempted to make request to username-based URL \(url) while logged out", requestError: nil) + } + urlWithSubstitutions = urlWithSubstitutions.replacingOccurrences(of: "{username}", with: username) + } + + let encoding: ParameterEncoding = if method == .get { URLEncoding.default } else { JSONEncoding.default }// TODO let response = await AF.request( - "\(baseURLString)/\(url)", + "\(baseURLString)/\(urlWithSubstitutions)", method: method, parameters: parameters, encoding: encoding, @@ -99,7 +108,7 @@ public class RequestManager { public func addDatapoint(urtext: String, slug: String) async throws -> Any? { let params = ["urtext": urtext, "requestid": UUID().uuidString] - return try await post(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(slug)/datapoints.json", parameters: params) + return try await post(url: "api/v1/users/{username}/goals/\(slug)/datapoints.json", parameters: params) } } diff --git a/BeeSwift/EditDatapointViewController.swift b/BeeSwift/EditDatapointViewController.swift index 186652250..8b22a2f04 100644 --- a/BeeSwift/EditDatapointViewController.swift +++ b/BeeSwift/EditDatapointViewController.swift @@ -212,7 +212,7 @@ class EditDatapointViewController: UIViewController, UITextFieldDelegate { let params = [ "urtext": self.urtext() ] - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: params) try await ServiceLocator.goalManager.refreshGoal(self.goal) hud.mode = .customView @@ -234,7 +234,7 @@ class EditDatapointViewController: UIViewController, UITextFieldDelegate { hud.mode = .indeterminate do { - let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: nil) + let _ = try await ServiceLocator.requestManager.delete(url: "api/v1/users/{username}/goals/\(self.goal.slug)/datapoints/\(self.datapoint.id).json", parameters: nil) try await ServiceLocator.goalManager.refreshGoal(self.goal) hud.mode = .customView diff --git a/BeeSwift/Settings/ConfigureHKMetricViewController.swift b/BeeSwift/Settings/ConfigureHKMetricViewController.swift index 496521b11..9161a82c2 100644 --- a/BeeSwift/Settings/ConfigureHKMetricViewController.swift +++ b/BeeSwift/Settings/ConfigureHKMetricViewController.swift @@ -157,7 +157,7 @@ class ConfigureHKMetricViewController : UIViewController { params = ["ii_params" : ["name" : "apple", "metric" : self.goal.healthKitMetric!]] do { - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal.slug).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal.slug).json", parameters: params) hud.mode = .customView hud.customView = UIImageView(image: UIImage(named: "BasicCheckmark")) hud.hide(animated: true, afterDelay: 2) diff --git a/BeeSwift/Settings/EditDefaultNotificationsViewController.swift b/BeeSwift/Settings/EditDefaultNotificationsViewController.swift index 05a933245..700f97a17 100644 --- a/BeeSwift/Settings/EditDefaultNotificationsViewController.swift +++ b/BeeSwift/Settings/EditDefaultNotificationsViewController.swift @@ -32,7 +32,7 @@ class EditDefaultNotificationsViewController: EditNotificationsViewController { guard let leadtime = userInfo["leadtime"] else { return } let params = [ "default_leadtime" : leadtime ] do { - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}.json", parameters: params) ServiceLocator.currentUserManager.setDefaultLeadTime(leadtime) } catch { logger.error("Error setting default leadtime: \(error)") @@ -52,7 +52,7 @@ class EditDefaultNotificationsViewController: EditNotificationsViewController { self.updateAlertstartLabel(self.midnightOffsetFromTimePickerView()) let params = ["default_alertstart" : self.midnightOffsetFromTimePickerView()] do { - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}.json", parameters: params) ServiceLocator.currentUserManager.setDefaultAlertstart(self.midnightOffsetFromTimePickerView()) } catch { logger.error("Error setting default alert start: \(error)") @@ -63,7 +63,7 @@ class EditDefaultNotificationsViewController: EditNotificationsViewController { self.updateDeadlineLabel(self.midnightOffsetFromTimePickerView()) let params = ["default_deadline" : self.midnightOffsetFromTimePickerView()] do { - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}.json", parameters: params) ServiceLocator.currentUserManager.setDefaultDeadline(self.midnightOffsetFromTimePickerView()) } catch { logger.error("Error setting default deadline: \(error)") diff --git a/BeeSwift/Settings/EditGoalNotificationsViewController.swift b/BeeSwift/Settings/EditGoalNotificationsViewController.swift index fa3d36e27..98456e4e5 100644 --- a/BeeSwift/Settings/EditGoalNotificationsViewController.swift +++ b/BeeSwift/Settings/EditGoalNotificationsViewController.swift @@ -68,7 +68,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController { let leadtime = userInfo["leadtime"] let params = [ "leadtime" : leadtime, "use_defaults" : false ] do { - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params as [String : Any]) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params as [String : Any]) self.goal!.leadtime = leadtime! self.goal!.use_defaults = NSNumber(value: false as Bool) self.useDefaultsSwitch.isOn = false @@ -85,7 +85,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController { self.updateAlertstartLabel(self.midnightOffsetFromTimePickerView()) do { let params = ["alertstart" : self.midnightOffsetFromTimePickerView(), "use_defaults" : false] - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params) self.goal!.alertstart = self.midnightOffsetFromTimePickerView() self.goal!.use_defaults = NSNumber(value: false as Bool) self.useDefaultsSwitch.isOn = false @@ -98,7 +98,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController { self.updateDeadlineLabel(self.midnightOffsetFromTimePickerView()) do { let params = ["deadline" : self.midnightOffsetFromTimePickerView(), "use_defaults" : false] - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params) self.goal?.deadline = self.midnightOffsetFromTimePickerView() self.goal!.use_defaults = NSNumber(value: false as Bool) self.useDefaultsSwitch.isOn = false @@ -120,7 +120,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController { Task { @MainActor in do { let params = ["use_defaults" : true] - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params) self.goal?.use_defaults = NSNumber(value: true as Bool) } catch { self.logger.error("Error setting goal to use defaults: \(error)") @@ -155,7 +155,7 @@ class EditGoalNotificationsViewController : EditNotificationsViewController { Task { @MainActor in do { let params = ["use_defaults" : false] - let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params) + let _ = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params) self.goal?.use_defaults = NSNumber(value: false as Bool) } catch { logger.error("Error setting goal to NOT use defaults: \(error)") diff --git a/BeeSwift/Settings/RemoveHKMetricViewController.swift b/BeeSwift/Settings/RemoveHKMetricViewController.swift index 339f87751..83f9feb28 100644 --- a/BeeSwift/Settings/RemoveHKMetricViewController.swift +++ b/BeeSwift/Settings/RemoveHKMetricViewController.swift @@ -75,7 +75,7 @@ class RemoveHKMetricViewController: UIViewController { Task { @MainActor in do { - let responseObject = try await ServiceLocator.requestManager.put(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.goal!.slug).json", parameters: params) + let responseObject = try await ServiceLocator.requestManager.put(url: "api/v1/users/{username}/goals/\(self.goal!.slug).json", parameters: params) self.goal = Goal(json: JSON(responseObject!)) diff --git a/BeeSwift/TimerViewController.swift b/BeeSwift/TimerViewController.swift index 99e6f3783..94f36eaac 100644 --- a/BeeSwift/TimerViewController.swift +++ b/BeeSwift/TimerViewController.swift @@ -174,7 +174,7 @@ class TimerViewController: UIViewController { Task { @MainActor in do { let params = ["urtext": self.urtext(), "requestid": UUID().uuidString] - let _ = try await ServiceLocator.requestManager.post(url: "api/v1/users/\(ServiceLocator.currentUserManager.username!)/goals/\(self.slug!)/datapoints.json", parameters: params) + let _ = try await ServiceLocator.requestManager.post(url: "api/v1/users/{username}/goals/\(self.slug!)/datapoints.json", parameters: params) hud.mode = .text hud.label.text = "Added!" DispatchQueue.main.asyncAfter(deadline: .now() + 2, execute: {