From ebccd14637c1d22a2467b14b318cf6e69a2390dd Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Fri, 25 Sep 2026 23:46:07 -0700 Subject: [PATCH 1/2] Post tripPlanEmpty when a plan yields no itinerary Hosts embedding the planner had no way to learn that a request came back with nothing usable: itinerariesUpdated fires only on success, and a server error such as PATH_NOT_FOUND only shows an alert. Post a new notification in both cases, with the reason and the planned origin and destination in userInfo, so a host can offer alternatives such as on-demand services between those points. --- .../Sources/OTPKit/Core/Notifications.swift | 12 ++++ .../TripPlannerViewModel+EmptyPlan.swift | 27 ++++++++ .../ViewModel/TripPlannerViewModel.swift | 6 +- OTPKit/Tests/TripPlannerViewModelTests.swift | 66 ++++++++++++++++++- 4 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift diff --git a/OTPKit/Sources/OTPKit/Core/Notifications.swift b/OTPKit/Sources/OTPKit/Core/Notifications.swift index cd21acb..a132778 100644 --- a/OTPKit/Sources/OTPKit/Core/Notifications.swift +++ b/OTPKit/Sources/OTPKit/Core/Notifications.swift @@ -12,6 +12,18 @@ import Foundation /// and/or `TripPlannerViewModel`. public struct Notifications { public static let itinerariesUpdated = NSNotification.Name("org.onebusaway.otpkit.itinerariesUpdated") + /// Posted when a plan request completes without a usable itinerary: the server answered with an + /// error (for example `PATH_NOT_FOUND`) or with an empty plan. `userInfo[tripPlanEmptyReasonKey]` + /// is `"error"` or `"empty"`, and the origin and destination the rider planned between travel + /// under the coordinate keys as `Double`s. Hosts use this to offer alternatives such as on-demand services. + public static let tripPlanEmpty = NSNotification.Name("org.onebusaway.otpkit.tripPlanEmpty") + /// The `userInfo` key under which `tripPlanEmpty` carries its reason. + public static let tripPlanEmptyReasonKey = "reason" + /// `userInfo` keys under which `tripPlanEmpty` carries the planned endpoints, as `Double`s. + public static let tripPlanEmptyOriginLatitudeKey = "originLatitude" + public static let tripPlanEmptyOriginLongitudeKey = "originLongitude" + public static let tripPlanEmptyDestinationLatitudeKey = "destinationLatitude" + public static let tripPlanEmptyDestinationLongitudeKey = "destinationLongitude" public static let itineraryPreviewStarted = NSNotification.Name("org.onebusaway.otpkit.itineraryPreviewStarted") public static let itineraryPreviewEnded = NSNotification.Name("org.onebusaway.otpkit.itineraryPreviewEnded") public static let tripStarted = NSNotification.Name("org.onebusaway.otpkit.tripStarted") diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift new file mode 100644 index 0000000..40b79db --- /dev/null +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift @@ -0,0 +1,27 @@ +// +// TripPlannerViewModel+EmptyPlan.swift +// OTPKit +// +// 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 +// MARK: - Empty plan notification + +extension TripPlannerViewModel { + /// Tells the host that the plan came back with nothing usable, with the endpoints it was planned between. + func postTripPlanEmpty(reason: String) { + var userInfo: [String: Any] = [Notifications.tripPlanEmptyReasonKey: reason] + if let origin = selectedOrigin { + userInfo[Notifications.tripPlanEmptyOriginLatitudeKey] = origin.latitude + userInfo[Notifications.tripPlanEmptyOriginLongitudeKey] = origin.longitude + } + if let destination = selectedDestination { + userInfo[Notifications.tripPlanEmptyDestinationLatitudeKey] = destination.latitude + userInfo[Notifications.tripPlanEmptyDestinationLongitudeKey] = destination.longitude + } + notificationCenter.post(name: Notifications.tripPlanEmpty, object: nil, userInfo: userInfo) + } +} diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift index c00614f..f04c954 100644 --- a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift @@ -98,7 +98,7 @@ public class TripPlannerViewModel: ObservableObject { private let mapCoordinator: MapCoordinator /// NotificationCenter object for sending notifications. - private let notificationCenter: NotificationCenter + let notificationCenter: NotificationCenter /// Flag to prevent saving during initialization private var isInitializationComplete = false @@ -290,10 +290,14 @@ public class TripPlannerViewModel: ObservableObject { tripPlanResponse = nil showError(error.messageCode.displayMessage) + postTripPlanEmpty(reason: "error") } else { tripPlanResponse = response HapticManager.shared.success() notificationCenter.post(name: Notifications.itinerariesUpdated, object: nil) + if itineraries.isEmpty { + postTripPlanEmpty(reason: "empty") + } } } diff --git a/OTPKit/Tests/TripPlannerViewModelTests.swift b/OTPKit/Tests/TripPlannerViewModelTests.swift index e154aa3..f0ab2ef 100644 --- a/OTPKit/Tests/TripPlannerViewModelTests.swift +++ b/OTPKit/Tests/TripPlannerViewModelTests.swift @@ -42,7 +42,8 @@ struct TripPlannerViewModelTests { func createViewModel( enabledModes: [TransportMode] = [.transit, .walk, .bike], - mockAPIService: TestFixtures.MockAPIService? = nil + mockAPIService: TestFixtures.MockAPIService? = nil, + notificationCenter: NotificationCenter = .default ) -> TripPlannerViewModel { // Clear UserDefaults before each test to ensure clean state let defaults = UserDefaults.standard @@ -59,10 +60,40 @@ struct TripPlannerViewModelTests { return TripPlannerViewModel( config: config, apiService: apiService, - mapCoordinator: mapCoordinator + mapCoordinator: mapCoordinator, + notificationCenter: notificationCenter ) } + static let emptyPlanOrigin = TestHelpers.location(title: "Origin", lat: 37.78, lon: -122.41) + static let emptyPlanDestination = TestHelpers.location(title: "Destination", lat: 37.80, lon: -122.40) + + /// Runs `planTrip()` against `response` on a private notification center and returns the + /// reasons of every `tripPlanEmpty` notification it posted. + func tripPlanEmptyReasons(for response: OTPResponse) async -> [String] { + await tripPlanEmptyNotifications(for: response).map { $0.userInfo?[Notifications.tripPlanEmptyReasonKey] as? String ?? "" } + } + + /// Runs `planTrip()` against `response` on a private notification center and returns every + /// `tripPlanEmpty` notification it posted. + func tripPlanEmptyNotifications(for response: OTPResponse) async -> [Notification] { + let mockAPIService = TestFixtures.MockAPIService() + mockAPIService.mockResponse = response + let center = NotificationCenter() + var notifications: [Notification] = [] + let token = center.addObserver(forName: Notifications.tripPlanEmpty, object: nil, queue: nil) { note in + notifications.append(note) + } + defer { center.removeObserver(token) } + + let viewModel = createViewModel(mockAPIService: mockAPIService, notificationCenter: center) + viewModel.selectedOrigin = Self.emptyPlanOrigin + viewModel.selectedDestination = Self.emptyPlanDestination + viewModel.planTrip() + await waitForLoadingComplete(viewModel) + return notifications + } + // MARK: - Initialization Tests @Test("ViewModel initializes with correct defaults") @@ -270,6 +301,37 @@ struct TripPlannerViewModelTests { #expect(mockAPIService.fetchPlanCallCount == 1) } + @Test("planTrip posts tripPlanEmpty when the plan has no itineraries") + func planTripPostsEmptyForEmptyPlan() async throws { + let reasons = await tripPlanEmptyReasons(for: TestHelpers.response(with: [])) + #expect(reasons == ["empty"]) + } + + @Test("planTrip posts tripPlanEmpty when the server reports an error") + func planTripPostsEmptyForServerError() async throws { + let errorResponse = try JSONDecoder().decode(OTPResponse.self, from: Fixtures.loadData(file: "otp_response_error.json")) + let reasons = await tripPlanEmptyReasons(for: errorResponse) + #expect(reasons == ["error"]) + } + + @Test("tripPlanEmpty carries the planned origin and destination") + func tripPlanEmptyCarriesEndpoints() async throws { + let notifications = await tripPlanEmptyNotifications(for: TestHelpers.response(with: [])) + let userInfo = try #require(notifications.first?.userInfo) + let origin = Self.emptyPlanOrigin + let destination = Self.emptyPlanDestination + #expect(userInfo[Notifications.tripPlanEmptyOriginLatitudeKey] as? Double == origin.latitude) + #expect(userInfo[Notifications.tripPlanEmptyOriginLongitudeKey] as? Double == origin.longitude) + #expect(userInfo[Notifications.tripPlanEmptyDestinationLatitudeKey] as? Double == destination.latitude) + #expect(userInfo[Notifications.tripPlanEmptyDestinationLongitudeKey] as? Double == destination.longitude) + } + + @Test("planTrip does not post tripPlanEmpty when itineraries are returned") + func planTripDoesNotPostEmptyForItineraries() async throws { + let reasons = await tripPlanEmptyReasons(for: TestHelpers.response(with: [TestHelpers.itinerary()])) + #expect(reasons.isEmpty) + } + @Test("planTrip sets loading state correctly") func planTripSetsLoadingState() async throws { let mockAPIService = TestFixtures.MockAPIService() From e4d7cf26bc279a534d498447670f0d78e55ed825 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Mon, 5 Oct 2026 22:41:50 -0700 Subject: [PATCH 2/2] Use request endpoints for tripPlanEmpty and drop superseded plans - tripPlanEmpty now carries the coordinates of the request that produced the result, not the current selection, which may have changed in flight. - Classify the response as empty before posting itinerariesUpdated so an observer cannot change the outcome. - Cancel an in-flight plan when a new one starts or the planner resets, so a stale response neither overwrites state nor notifies the host. - Expose the reason values as tripPlanEmptyReasonError / ...Empty. Co-Authored-By: Claude --- .../Sources/OTPKit/Core/Notifications.swift | 9 +++-- .../TripPlannerViewModel+EmptyPlan.swift | 23 +++++++------ .../ViewModel/TripPlannerViewModel.swift | 21 +++++++++--- OTPKit/Tests/TripPlannerViewModelTests.swift | 34 ++++++++++++++++--- 4 files changed, 64 insertions(+), 23 deletions(-) diff --git a/OTPKit/Sources/OTPKit/Core/Notifications.swift b/OTPKit/Sources/OTPKit/Core/Notifications.swift index a132778..db62d6c 100644 --- a/OTPKit/Sources/OTPKit/Core/Notifications.swift +++ b/OTPKit/Sources/OTPKit/Core/Notifications.swift @@ -14,11 +14,16 @@ public struct Notifications { public static let itinerariesUpdated = NSNotification.Name("org.onebusaway.otpkit.itinerariesUpdated") /// Posted when a plan request completes without a usable itinerary: the server answered with an /// error (for example `PATH_NOT_FOUND`) or with an empty plan. `userInfo[tripPlanEmptyReasonKey]` - /// is `"error"` or `"empty"`, and the origin and destination the rider planned between travel - /// under the coordinate keys as `Double`s. Hosts use this to offer alternatives such as on-demand services. + /// is `tripPlanEmptyReasonError` or `tripPlanEmptyReasonEmpty`, and the origin and destination of + /// that request travel under the coordinate keys as `Double`s. Hosts use this to offer alternatives + /// such as on-demand services. public static let tripPlanEmpty = NSNotification.Name("org.onebusaway.otpkit.tripPlanEmpty") /// The `userInfo` key under which `tripPlanEmpty` carries its reason. public static let tripPlanEmptyReasonKey = "reason" + /// `tripPlanEmpty` reason: the server answered with an error such as `PATH_NOT_FOUND`. + public static let tripPlanEmptyReasonError = "error" + /// `tripPlanEmpty` reason: the server answered with a plan that has no itineraries. + public static let tripPlanEmptyReasonEmpty = "empty" /// `userInfo` keys under which `tripPlanEmpty` carries the planned endpoints, as `Double`s. public static let tripPlanEmptyOriginLatitudeKey = "originLatitude" public static let tripPlanEmptyOriginLongitudeKey = "originLongitude" diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift index 40b79db..abe4ba8 100644 --- a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel+EmptyPlan.swift @@ -7,21 +7,22 @@ // LICENSE file in the root directory of this source tree. // +import CoreLocation import Foundation + // MARK: - Empty plan notification extension TripPlannerViewModel { - /// Tells the host that the plan came back with nothing usable, with the endpoints it was planned between. - func postTripPlanEmpty(reason: String) { - var userInfo: [String: Any] = [Notifications.tripPlanEmptyReasonKey: reason] - if let origin = selectedOrigin { - userInfo[Notifications.tripPlanEmptyOriginLatitudeKey] = origin.latitude - userInfo[Notifications.tripPlanEmptyOriginLongitudeKey] = origin.longitude - } - if let destination = selectedDestination { - userInfo[Notifications.tripPlanEmptyDestinationLatitudeKey] = destination.latitude - userInfo[Notifications.tripPlanEmptyDestinationLongitudeKey] = destination.longitude - } + /// Tells the host that the plan came back with nothing usable, with the endpoints of the request + /// that produced it (not the current selection, which the rider may have changed since). + func postTripPlanEmpty(reason: String, for request: TripPlanRequest) { + let userInfo: [String: Any] = [ + Notifications.tripPlanEmptyReasonKey: reason, + Notifications.tripPlanEmptyOriginLatitudeKey: request.origin.latitude, + Notifications.tripPlanEmptyOriginLongitudeKey: request.origin.longitude, + Notifications.tripPlanEmptyDestinationLatitudeKey: request.destination.latitude, + Notifications.tripPlanEmptyDestinationLongitudeKey: request.destination.longitude + ] notificationCenter.post(name: Notifications.tripPlanEmpty, object: nil, userInfo: userInfo) } } diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift index f04c954..9019b99 100644 --- a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift @@ -236,14 +236,19 @@ public class TripPlannerViewModel: ObservableObject { errorMessage = nil showingError = false + // A newer plan supersedes any request still in flight; its late response must not + // overwrite this one's results or notify the host about stale endpoints. + activePlanTask?.cancel() activePlanTask = Task { do { let response = try await apiService.fetchPlan(request) await MainActor.run { - self.handlePlanResponse(response) + guard !Task.isCancelled else { return } + self.handlePlanResponse(response, for: request) } } catch { await MainActor.run { + guard !Task.isCancelled else { return } self.handleError(error) } } @@ -281,7 +286,7 @@ public class TripPlannerViewModel: ObservableObject { } } - private func handlePlanResponse(_ response: OTPResponse) { + private func handlePlanResponse(_ response: OTPResponse, for request: TripPlanRequest) { isLoading = false if let error = response.error { @@ -290,13 +295,15 @@ public class TripPlannerViewModel: ObservableObject { tripPlanResponse = nil showError(error.messageCode.displayMessage) - postTripPlanEmpty(reason: "error") + postTripPlanEmpty(reason: Notifications.tripPlanEmptyReasonError, for: request) } else { + // Classify from the response itself: an itinerariesUpdated observer may change state. + let isEmpty = response.plan?.itineraries.isEmpty ?? true tripPlanResponse = response HapticManager.shared.success() notificationCenter.post(name: Notifications.itinerariesUpdated, object: nil) - if itineraries.isEmpty { - postTripPlanEmpty(reason: "empty") + if isEmpty { + postTripPlanEmpty(reason: Notifications.tripPlanEmptyReasonEmpty, for: request) } } } @@ -400,6 +407,10 @@ public class TripPlannerViewModel: ObservableObject { /// Reset all trip planner state to initial values /// Clears locations, itineraries, and returns to clean state func resetTripPlanner() { + // Drop any plan still in flight so its response can't repopulate the cleared state. + activePlanTask?.cancel() + activePlanTask = nil + // Clear location selections selectedOrigin = nil selectedDestination = nil diff --git a/OTPKit/Tests/TripPlannerViewModelTests.swift b/OTPKit/Tests/TripPlannerViewModelTests.swift index f0ab2ef..969eceb 100644 --- a/OTPKit/Tests/TripPlannerViewModelTests.swift +++ b/OTPKit/Tests/TripPlannerViewModelTests.swift @@ -75,8 +75,12 @@ struct TripPlannerViewModelTests { } /// Runs `planTrip()` against `response` on a private notification center and returns every - /// `tripPlanEmpty` notification it posted. - func tripPlanEmptyNotifications(for response: OTPResponse) async -> [Notification] { + /// `tripPlanEmpty` notification it posted. `whileInFlight` runs after the request starts and + /// before its response is handled. + func tripPlanEmptyNotifications( + for response: OTPResponse, + whileInFlight: (TripPlannerViewModel) -> Void = { _ in } + ) async -> [Notification] { let mockAPIService = TestFixtures.MockAPIService() mockAPIService.mockResponse = response let center = NotificationCenter() @@ -90,7 +94,9 @@ struct TripPlannerViewModelTests { viewModel.selectedOrigin = Self.emptyPlanOrigin viewModel.selectedDestination = Self.emptyPlanDestination viewModel.planTrip() - await waitForLoadingComplete(viewModel) + let inFlight = viewModel.activePlanTask + whileInFlight(viewModel) + await inFlight?.value return notifications } @@ -304,14 +310,14 @@ struct TripPlannerViewModelTests { @Test("planTrip posts tripPlanEmpty when the plan has no itineraries") func planTripPostsEmptyForEmptyPlan() async throws { let reasons = await tripPlanEmptyReasons(for: TestHelpers.response(with: [])) - #expect(reasons == ["empty"]) + #expect(reasons == [Notifications.tripPlanEmptyReasonEmpty]) } @Test("planTrip posts tripPlanEmpty when the server reports an error") func planTripPostsEmptyForServerError() async throws { let errorResponse = try JSONDecoder().decode(OTPResponse.self, from: Fixtures.loadData(file: "otp_response_error.json")) let reasons = await tripPlanEmptyReasons(for: errorResponse) - #expect(reasons == ["error"]) + #expect(reasons == [Notifications.tripPlanEmptyReasonError]) } @Test("tripPlanEmpty carries the planned origin and destination") @@ -326,6 +332,24 @@ struct TripPlannerViewModelTests { #expect(userInfo[Notifications.tripPlanEmptyDestinationLongitudeKey] as? Double == destination.longitude) } + @Test("tripPlanEmpty carries the requested endpoints even if the selection changed in flight") + func tripPlanEmptyUsesRequestEndpoints() async throws { + let notifications = await tripPlanEmptyNotifications(for: TestHelpers.response(with: [])) { viewModel in + viewModel.selectedOrigin = TestHelpers.location(title: "Elsewhere", lat: 47.6, lon: -122.3) + } + let userInfo = try #require(notifications.first?.userInfo) + #expect(userInfo[Notifications.tripPlanEmptyOriginLatitudeKey] as? Double == Self.emptyPlanOrigin.latitude) + #expect(userInfo[Notifications.tripPlanEmptyOriginLongitudeKey] as? Double == Self.emptyPlanOrigin.longitude) + } + + @Test("a plan cancelled by reset does not post tripPlanEmpty") + func resetCancelsInFlightPlan() async throws { + let notifications = await tripPlanEmptyNotifications(for: TestHelpers.response(with: [])) { viewModel in + viewModel.resetTripPlanner() + } + #expect(notifications.isEmpty) + } + @Test("planTrip does not post tripPlanEmpty when itineraries are returned") func planTripDoesNotPostEmptyForItineraries() async throws { let reasons = await tripPlanEmptyReasons(for: TestHelpers.response(with: [TestHelpers.itinerary()]))