From fb6c7bd9ed6edf172eade1019c6c1f58c93960d2 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Wed, 29 Jul 2026 00:32:20 -0700 Subject: [PATCH 1/6] Add VehicleRentalSource, via-point planning, and rental map rendering - VehicleRentalSource: stateful viewport-to-snapshot pipeline (coalescing, cancellation, bbox padding, id-keyed diffing, partial-error forwarding) with a separate fetchFailures stream for host availability states - TripPlanRequest.viaPoint wired through both services: GraphQL plan query gains $via (PlanViaLocationInput, verified live against Sound Transit), REST gains intermediatePlaces - TransportMode.transitBikeRental (Transit + Bikeshare) with localized names in all 13 locales; capability-gated like .bikeRental - TripPlanner.createTripPlannerView accepts prefilled viaPoint + mode, the entry point for 'Plan a trip using this bike' - MapCoordinator: BICYCLE legs get bike color/icon; rental pickup/dropoff annotations via new OTPAnnotationType.rentalVehicle in rental purple --- .../OTPKit/Core/Map/MapCoordinator.swift | 44 ++- .../OTPKit/Core/Map/OTPMapProvider.swift | 9 + .../Core/Models/OTP/TransportMode.swift | 23 +- .../Core/Models/OTP/TripPlanRequest.swift | 15 +- .../Models/OTP/VehicleRentalSnapshot.swift | 58 +++ .../OTPKit/Network/GraphQLAPIService.swift | 18 +- .../OTPKit/Network/RestAPIService.swift | 11 +- .../OTPKit/Network/VehicleRentalSource.swift | 229 +++++++++++ .../OTPKit/Presentation/TripPlanner.swift | 25 +- .../ViewModel/TripPlannerViewModel.swift | 9 +- .../Resources/ar.lproj/Localizable.strings | 3 +- .../Resources/en.lproj/Localizable.strings | 3 +- .../Resources/es.lproj/Localizable.strings | 3 +- .../Resources/fil.lproj/Localizable.strings | 3 +- .../Resources/fr.lproj/Localizable.strings | 3 +- .../Resources/it.lproj/Localizable.strings | 3 +- .../Resources/ko.lproj/Localizable.strings | 3 +- .../Resources/pl.lproj/Localizable.strings | 3 +- .../Resources/pt-BR.lproj/Localizable.strings | 3 +- .../Resources/ru.lproj/Localizable.strings | 3 +- .../Resources/vi.lproj/Localizable.strings | 3 +- .../zh-Hans.lproj/Localizable.strings | 3 +- .../zh-Hant.lproj/Localizable.strings | 3 +- OTPKit/Tests/GraphQLAPIServiceTests.swift | 43 +++ OTPKit/Tests/TripPlanRequestTests.swift | 34 ++ OTPKit/Tests/TripPlannerViewModelTests.swift | 17 + OTPKit/Tests/VehicleRentalSourceTests.swift | 355 ++++++++++++++++++ 27 files changed, 903 insertions(+), 26 deletions(-) create mode 100644 OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRentalSnapshot.swift create mode 100644 OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift create mode 100644 OTPKit/Tests/VehicleRentalSourceTests.swift diff --git a/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift b/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift index 95bf50c..8375451 100644 --- a/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift +++ b/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift @@ -212,7 +212,9 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b return .gray case "BUS", "TRAM", "TRAIN", "SUBWAY", "FERRY": return .blue - case "BIKE", "CAR": + // OTP 2.x spells bicycle legs "BICYCLE" (rental rides included); "BIKE" is + // the OTP 1.x REST spelling. + case "BIKE", "BICYCLE", "CAR": return .orange default: return .gray @@ -238,7 +240,7 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b return "bus" case "TRAM": return "tram" - case "BIKE": + case "BIKE", "BICYCLE": return "bicycle" case "CAR": return "car" @@ -297,6 +299,12 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b } private func addStationAnnotations(for leg: Leg, index: Int, totalLegs: Int) { + // Rental legs get pickup/dropoff markers; transit legs get embark/debark markers. + if leg.rentedBike == true { + addRentalAnnotations(for: leg, index: index) + return + } + // Only add embark/debark markers for transit legs guard leg.transitLeg == true else { return } // Add annotation for "from" location (embark point) @@ -329,6 +337,38 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b // Add intermediate stop markers addIntermediateStopAnnotations(for: leg, index: index) } + /// Marks the rental pickup and dropoff points of a rental ride leg. Rental places + /// carry a `bikeShareId` instead of a `vertexType`/`stopId`, so the transit-station + /// checks never match them. Only vehicles on the planned route are annotated — the + /// browse layer is a separate surface owned by the host. + private func addRentalAnnotations(for leg: Leg, index: Int) { + if leg.from.bikeShareId != nil { + mapProvider.addAnnotation( + coordinate: CLLocationCoordinate2D(latitude: leg.from.lat, longitude: leg.from.lon), + title: leg.from.name, + subtitle: nil, + identifier: "rental_pickup_\(index)", + type: .rentalVehicle, + routeName: nil, + routeBackgroundColor: nil, + routeTextColor: nil + ) + } + + if leg.to.bikeShareId != nil { + mapProvider.addAnnotation( + coordinate: CLLocationCoordinate2D(latitude: leg.to.lat, longitude: leg.to.lon), + title: leg.to.name, + subtitle: nil, + identifier: "rental_dropoff_\(index)", + type: .rentalVehicle, + routeName: nil, + routeBackgroundColor: nil, + routeTextColor: nil + ) + } + } + private func addIntermediateStopAnnotations(for leg: Leg, index: Int) { guard leg.transitLeg == true, let stops = leg.intermediateStops, !stops.isEmpty else { return } let routeColor = leg.routeColor.flatMap { UIColor(hex: $0) } diff --git a/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift b/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift index 1eacc01..597fe28 100644 --- a/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift +++ b/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift @@ -140,6 +140,8 @@ public enum OTPAnnotationType { case embark case debark case intermediateStop + /// A rental vehicle or station that is part of the planned route (pickup/dropoff). + case rentalVehicle /// Returns the appropriate color for this annotation type public var color: Color { @@ -162,6 +164,11 @@ public enum OTPAnnotationType { return .orange case .intermediateStop: return .gray + case .rentalVehicle: + // Rental purple (#7B4FD1): every rental surface — browse layer pins and + // trip-planner pickup/dropoff markers — shares this color so rentals + // read as one system. + return Color(red: 0x7B / 255.0, green: 0x4F / 255.0, blue: 0xD1 / 255.0) case .routeLegend: return .clear // Custom view will handle coloring } @@ -190,6 +197,8 @@ public enum OTPAnnotationType { return "magnifyingglass" case .intermediateStop: return "circle.fill" + case .rentalVehicle: + return "bicycle.circle.fill" case .routeLegend: return "" // Custom view will handle display } diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift index 014830c..5b10c66 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift @@ -17,9 +17,14 @@ public enum TransportMode: String, CaseIterable, Codable { case bike = "BIKE" /// Driving case car = "CAR" - /// Rented bicycle/micromobility (bikeshare). The raw value is the OTP 1.x REST wire - /// token; the GraphQL service translates it to `{mode: BICYCLE, qualifier: RENT}`. + /// Rented bicycle/micromobility (bikeshare) without transit — "Bikeshare Only". + /// The raw value is the OTP 1.x REST wire token; the GraphQL service translates + /// it to `{mode: BICYCLE, qualifier: RENT}`. case bikeRental = "BICYCLE_RENT" + /// Transit combined with rented micromobility — "Transit + Bikeshare". Unlike the + /// other cases, the raw value is *not* a wire token: this mode only ever reaches a + /// request expanded through `apiModes`, never as itself. + case transitBikeRental = "TRANSIT_BICYCLE_RENT" /// Localized, human-readable description of the transport mode public var displayName: String { @@ -33,7 +38,9 @@ public enum TransportMode: String, CaseIterable, Codable { case .car: return OTPLoc("transport_mode.car", comment: "Transport mode: Car") case .bikeRental: - return OTPLoc("transport_mode.bike_rental", comment: "Transport mode: Bike Rental") + return OTPLoc("transport_mode.bike_rental", comment: "Transport mode: Bikeshare Only") + case .transitBikeRental: + return OTPLoc("transport_mode.transit_bike_rental", comment: "Transport mode: Transit + Bikeshare") } } @@ -50,6 +57,8 @@ public enum TransportMode: String, CaseIterable, Codable { return "car" case .bikeRental: return "bicycle.circle" + case .transitBikeRental: + return "bicycle.circle.fill" } } @@ -67,6 +76,14 @@ public enum TransportMode: String, CaseIterable, Codable { return [.car] case .bikeRental: return [.bikeRental, .walk] + case .transitBikeRental: + return [.transit, .walk, .bikeRental] } } + + /// True for modes that only work against a rental-capable backend + /// (`apiService is VehicleRentalService`). The UI hides these otherwise. + public var requiresVehicleRentalSupport: Bool { + self == .bikeRental || self == .transitBikeRental + } } diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift index 124b701..24db4c8 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift @@ -35,6 +35,10 @@ public struct TripPlanRequest: Codable, Hashable { public let wheelchairAccessible: Bool /// Whether the time parameter refers to arrival time (true) or departure time (false) public let arriveBy: Bool + /// An intermediate coordinate the trip must pass through, e.g. a specific rental + /// vehicle's location for "plan a trip using this bike". Servers may require a + /// transit mode in `transportModes` to route through a via point. + public let viaPoint: CLLocationCoordinate2D? /// Creates a new trip plan request /// - Parameters: @@ -46,6 +50,7 @@ public struct TripPlanRequest: Codable, Hashable { /// - maxWalkDistance: The maximum walking distance in meters (defaults to 1000) /// - wheelchairAccessible: Whether the route should be wheelchair accessible (defaults to false) /// - arriveBy: Whether the time parameter refers to arrival time (defaults to false) + /// - viaPoint: An intermediate coordinate the trip must pass through (defaults to nil) public init( origin: CLLocationCoordinate2D, destination: CLLocationCoordinate2D, @@ -54,7 +59,8 @@ public struct TripPlanRequest: Codable, Hashable { transportModes: [TransportMode] = [.transit, .walk], maxWalkDistance: Int = 1000, wheelchairAccessible: Bool = false, - arriveBy: Bool = false + arriveBy: Bool = false, + viaPoint: CLLocationCoordinate2D? = nil ) { self.origin = origin self.destination = destination @@ -64,6 +70,7 @@ public struct TripPlanRequest: Codable, Hashable { self.maxWalkDistance = maxWalkDistance self.wheelchairAccessible = wheelchairAccessible self.arriveBy = arriveBy + self.viaPoint = viaPoint } /// Converts the transport modes to the API string format @@ -108,6 +115,8 @@ public struct TripPlanRequest: Codable, Hashable { hasher.combine(maxWalkDistance) hasher.combine(wheelchairAccessible) hasher.combine(arriveBy) + hasher.combine(viaPoint?.latitude) + hasher.combine(viaPoint?.longitude) } public static func == (lhs: TripPlanRequest, rhs: TripPlanRequest) -> Bool { @@ -120,7 +129,9 @@ public struct TripPlanRequest: Codable, Hashable { lhs.transportModes == rhs.transportModes && lhs.maxWalkDistance == rhs.maxWalkDistance && lhs.wheelchairAccessible == rhs.wheelchairAccessible && - lhs.arriveBy == rhs.arriveBy + lhs.arriveBy == rhs.arriveBy && + lhs.viaPoint?.latitude == rhs.viaPoint?.latitude && + lhs.viaPoint?.longitude == rhs.viaPoint?.longitude } } diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRentalSnapshot.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRentalSnapshot.swift new file mode 100644 index 0000000..13df5d2 --- /dev/null +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRentalSnapshot.swift @@ -0,0 +1,58 @@ +/* + * Copyright (C) Open Transit Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for specific language governing permissions and + * limitations under the License. + */ + +import Foundation + +/// A diffed set of rental entities delivered by `VehicleRentalSource` as the +/// viewport changes. Consumers apply the diff directly — no reconciliation +/// against previously delivered state is needed. +public struct VehicleRentalSnapshot: Sendable { + /// Entities that were not present in the previous snapshot. + public let added: [VehicleRental] + + /// IDs of entities present in the previous snapshot but gone now. + public let removed: [VehicleRental.ID] + + /// Entities whose data changed since the previous snapshot (position, + /// availability, fuel, operative state). Updating these in place — rather + /// than remove/re-add — preserves a selected callout across refreshes. + public let updated: [VehicleRental] + + public let fetchedAt: Date + + /// Non-fatal GraphQL error messages that accompanied partial data. + /// Empty on full success. + public let partialErrors: [String] + + public init( + added: [VehicleRental], + removed: [VehicleRental.ID], + updated: [VehicleRental], + fetchedAt: Date, + partialErrors: [String] = [] + ) { + self.added = added + self.removed = removed + self.updated = updated + self.fetchedAt = fetchedAt + self.partialErrors = partialErrors + } + + /// True when the snapshot changes nothing (no adds, removals, or updates). + public var isEmpty: Bool { + added.isEmpty && removed.isEmpty && updated.isEmpty + } +} diff --git a/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift b/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift index 9b1b761..27ec112 100644 --- a/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift +++ b/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift @@ -140,7 +140,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { /// Builds the GraphQL `variables` payload for a trip plan request. private static func planVariables(for request: TripPlanRequest) -> [String: Any] { - [ + var variables: [String: Any] = [ "from": ["lat": request.origin.latitude, "lon": request.origin.longitude], "to": ["lat": request.destination.latitude, "lon": request.destination.longitude], "date": request.date.formattedTripDate, @@ -150,6 +150,16 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { "wheelchair": request.wheelchairAccessible, "maxWalkDistance": Double(request.maxWalkDistance) ] + + // Omitted entirely when absent: a null `via` and a missing `via` are not + // guaranteed to be treated identically by every OTP build. + if let viaPoint = request.viaPoint { + variables["via"] = [ + ["visit": ["coordinate": ["latitude": viaPoint.latitude, "longitude": viaPoint.longitude]]] + ] + } + + return variables } /// The GraphQL `TransportMode` input value for a transport mode. `TransportMode.rawValue` @@ -161,6 +171,10 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { return ["mode": "BICYCLE"] case .bikeRental: return ["mode": "BICYCLE", "qualifier": "RENT"] + case .transitBikeRental: + // Unreachable in practice: composite UI modes reach requests expanded + // through `apiModes`, never as themselves. + return ["mode": "TRANSIT"] case .transit, .walk, .car: return ["mode": mode.rawValue] } @@ -213,6 +227,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { $arriveBy: Boolean $wheelchair: Boolean $maxWalkDistance: Float + $via: [PlanViaLocationInput!] ) { plan( from: $from @@ -223,6 +238,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { arriveBy: $arriveBy wheelchair: $wheelchair maxWalkDistance: $maxWalkDistance + via: $via ) { date from { name lon lat vertexType } diff --git a/OTPKit/Sources/OTPKit/Network/RestAPIService.swift b/OTPKit/Sources/OTPKit/Network/RestAPIService.swift index 5b9af41..43472f2 100644 --- a/OTPKit/Sources/OTPKit/Network/RestAPIService.swift +++ b/OTPKit/Sources/OTPKit/Network/RestAPIService.swift @@ -46,7 +46,8 @@ public actor RestAPIService: APIService { mode: request.transportModesString, arriveBy: request.arriveBy, maxWalkDistance: request.maxWalkDistance, - wheelchair: request.wheelchairAccessible + wheelchair: request.wheelchairAccessible, + intermediatePlaces: request.viaPoint.map { [$0.formattedForAPI] } ?? [] ) } @@ -59,7 +60,8 @@ public actor RestAPIService: APIService { mode: String, arriveBy: Bool, maxWalkDistance: Int, - wheelchair: Bool + wheelchair: Bool, + intermediatePlaces: [String] = [] ) async throws -> OTPResponse { var components = URLComponents( url: buildURL(endpoint: "plan"), @@ -78,6 +80,11 @@ public actor RestAPIService: APIService { .init(name: "showIntermediateStops", value: "true") ] + // OTP 1.x routes through each intermediatePlaces value ("lat,lon") in order. + components.queryItems?.append(contentsOf: intermediatePlaces.map { + .init(name: "intermediatePlaces", value: $0) + }) + let request = URLRequest(url: components.url!) Logger.main.info("Fetching trip plan: \(request.url!.absoluteString)") diff --git a/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift b/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift new file mode 100644 index 0000000..627257e --- /dev/null +++ b/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift @@ -0,0 +1,229 @@ +/* + * Copyright (C) Open Transit Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for specific language governing permissions and + * limitations under the License. + */ + +import Foundation + +/// A stateful, cancellable pipeline from viewport updates to diffed rental snapshots. +/// +/// The work between "user pans the map" and "annotations are correct" is the same for +/// every host and involves no MapKit: coalescing viewport changes, cancelling superseded +/// fetches, filtering by form factor, and reconciling against what was previously +/// delivered. This actor owns all of it; hosts apply the emitted diffs directly. +/// +/// `snapshots` and `fetchFailures` are single-consumer streams: attach exactly one +/// iterator to each for the lifetime of the source. +public actor VehicleRentalSource { + + /// A failed fetch. Emitted on `fetchFailures` instead of the snapshot stream so a + /// transient error never disturbs already-delivered map state. Hosts use the *first* + /// failure to dim a layer row ("Not available right now") and any later success to + /// un-dim it. + public struct FetchFailure: Sendable { + public let underlyingError: any Error + public let occurredAt: Date + + /// Convenience for logging and rider-facing reasons. + public var message: String { underlyingError.localizedDescription } + } + + /// Snapshots delivered as the viewport or form-factor selection changes. + public nonisolated let snapshots: AsyncStream + + /// Fetch failures (transport, HTTP, decode). Superseded and cancelled fetches + /// are never reported. + public nonisolated let fetchFailures: AsyncStream + + private let snapshotContinuation: AsyncStream.Continuation + private let failureContinuation: AsyncStream.Continuation + + private let service: VehicleRentalService + private let coalescingInterval: Duration + private let boundingBoxPadding: Double + + private var formFactors: Set? + private var viewport: VehicleRentalBoundingBox? + private var delivered: [VehicleRental.ID: VehicleRental] = [:] + private var pendingFetch: Task? + + /// Incremented whenever pending work becomes stale (new viewport, new filter, + /// clear). A fetch only applies its result if its generation is still current — + /// this covers the window where a task has passed its cancellation checks but + /// not yet delivered. + private var generation = 0 + + /// - Parameters: + /// - service: The rental-capable backend, typically a `GraphQLAPIService`. + /// - formFactors: Initial form-factor filter; `nil` fetches everything. + /// - coalescingInterval: Trailing debounce applied to viewport/filter changes. + /// The 250 ms default matches OBA's established map debounce. + /// - boundingBoxPadding: Multiplier applied to the viewport before fetching, so + /// small pans are already covered. 1.1 mirrors OBA's region fudge factor. + public init( + service: VehicleRentalService, + formFactors: Set? = nil, + coalescingInterval: Duration = .milliseconds(250), + boundingBoxPadding: Double = 1.1 + ) { + self.service = service + self.formFactors = formFactors + self.coalescingInterval = coalescingInterval + self.boundingBoxPadding = boundingBoxPadding + + (snapshots, snapshotContinuation) = AsyncStream.makeStream(of: VehicleRentalSnapshot.self) + (fetchFailures, failureContinuation) = AsyncStream.makeStream(of: FetchFailure.self) + } + + deinit { + pendingFetch?.cancel() + snapshotContinuation.finish() + failureContinuation.finish() + } + + // MARK: - Inputs + + /// Called on every map region change. Coalesces; cancels superseded work. + /// Passing `nil` (e.g. the zoom gate closed) immediately emits a snapshot + /// removing everything. + public func setViewport(_ boundingBox: VehicleRentalBoundingBox?) { + guard let boundingBox else { + clear() + return + } + viewport = boundingBox + scheduleFetch() + } + + /// Changes what is being fetched without tearing down the stream. Triggers a + /// refetch when a viewport is set. + public func setFormFactors(_ formFactors: Set?) { + guard formFactors != self.formFactors else { return } + self.formFactors = formFactors + if viewport != nil { + scheduleFetch() + } + } + + /// Clears all state (e.g. the layer was switched off) and emits a snapshot + /// removing everything previously delivered. + public func reset() { + clear() + } + + // MARK: - Pipeline + + private func clear() { + pendingFetch?.cancel() + pendingFetch = nil + generation += 1 + viewport = nil + + let removed = delivered.keys.sorted() + delivered = [:] + snapshotContinuation.yield(VehicleRentalSnapshot( + added: [], + removed: removed, + updated: [], + fetchedAt: Date() + )) + } + + private func scheduleFetch() { + pendingFetch?.cancel() + generation += 1 + let scheduledGeneration = generation + let interval = coalescingInterval + + // The task inherits the actor's isolation: the sleep and the fetch suspend + // without blocking other actor work (fetchPlan on the same service is + // unaffected — decode already runs off-actor in GraphQLAPIService). + pendingFetch = Task { + try? await Task.sleep(for: interval) + guard !Task.isCancelled else { return } + await self.performFetch(generation: scheduledGeneration) + } + } + + private func performFetch(generation scheduledGeneration: Int) async { + guard scheduledGeneration == generation, let viewport else { return } + let paddedBox = viewport.padded(by: boundingBoxPadding) + + do { + let result = try await service.fetchVehicleRentals(in: paddedBox, formFactors: formFactors) + guard scheduledGeneration == generation else { return } + apply(result) + } catch is CancellationError { + // Superseded by a newer viewport; the newer fetch reports instead. + } catch { + guard scheduledGeneration == generation else { return } + failureContinuation.yield(FetchFailure(underlyingError: error, occurredAt: Date())) + } + } + + /// Diffs the fetch result against what was previously delivered, keyed on + /// `VehicleRental.id`. `added`/`updated` preserve response order; `removed` is + /// sorted for determinism. + private func apply(_ result: VehicleRentalFetchResult) { + var added: [VehicleRental] = [] + var updated: [VehicleRental] = [] + var incoming: [VehicleRental.ID: VehicleRental] = [:] + incoming.reserveCapacity(result.rentals.count) + + for rental in result.rentals { + // Duplicate IDs in one payload: last write wins in state, first wins in the + // delivered arrays (guarded by the incoming-dict check). + guard incoming.updateValue(rental, forKey: rental.id) == nil else { continue } + + if let previous = delivered[rental.id] { + if previous != rental { + updated.append(rental) + } + } else { + added.append(rental) + } + } + + let removed = delivered.keys.filter { incoming[$0] == nil }.sorted() + delivered = incoming + + snapshotContinuation.yield(VehicleRentalSnapshot( + added: added, + removed: removed, + updated: updated, + fetchedAt: Date(), + partialErrors: result.partialErrors + )) + } +} + +extension VehicleRentalBoundingBox { + /// Expands the box around its center by the given factor, clamped to valid + /// coordinate ranges. Does not handle antimeridian-spanning boxes. + func padded(by factor: Double) -> VehicleRentalBoundingBox { + guard factor != 1 else { return self } + + let latitudeCenter = (minimumLatitude + maximumLatitude) / 2 + let longitudeCenter = (minimumLongitude + maximumLongitude) / 2 + let latitudeHalfSpan = (maximumLatitude - minimumLatitude) / 2 * factor + let longitudeHalfSpan = (maximumLongitude - minimumLongitude) / 2 * factor + + return VehicleRentalBoundingBox( + minimumLatitude: max(-90, latitudeCenter - latitudeHalfSpan), + maximumLatitude: min(90, latitudeCenter + latitudeHalfSpan), + minimumLongitude: max(-180, longitudeCenter - longitudeHalfSpan), + maximumLongitude: min(180, longitudeCenter + longitudeHalfSpan) + ) + } +} diff --git a/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift b/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift index 6570497..5afc224 100644 --- a/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift +++ b/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift @@ -63,7 +63,30 @@ public class TripPlanner { // MARK: - Presentation & Dismissal - public func createTripPlannerView(origin: Location? = nil, destination: Location? = nil, onClose: @escaping VoidBlock) -> some View { + /// Creates the trip planner UI, optionally prefilled. + /// + /// - Parameters: + /// - origin: Prefilled origin location. + /// - destination: Prefilled destination location. + /// - viaPoint: An intermediate coordinate every planned trip must pass through — + /// the "plan a trip using this bike" entry point passes the vehicle's location. + /// Note: OTP servers may require a transit mode in the request to route through + /// a via point, so pair this with `.transitBikeRental` rather than `.bikeRental`. + /// - transportMode: Preselected transport mode. Ignored when the injected API + /// service cannot support it (e.g. rental modes on an OTP 1.x REST backend). + /// - onClose: Called when the rider dismisses the planner. + public func createTripPlannerView( + origin: Location? = nil, + destination: Location? = nil, + viaPoint: CLLocationCoordinate2D? = nil, + transportMode: TransportMode? = nil, + onClose: @escaping VoidBlock + ) -> some View { + viewModel.viaPoint = viaPoint + if let transportMode, viewModel.availableTransportModes.contains(transportMode) { + viewModel.selectTransportMode(transportMode) + } + let view = TripPlannerView( viewModel: viewModel, mapCoordinator: mapCoordinator, diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift index cedf53f..3565d43 100644 --- a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift @@ -30,6 +30,9 @@ public class TripPlannerViewModel: ObservableObject { @Published var departureTime: Date? /// User-selected departure date @Published var departureDate: Date? + /// An intermediate coordinate every planned trip must pass through — e.g. a rental + /// vehicle's location for "plan a trip using this bike". Cleared on reset. + @Published var viaPoint: CLLocationCoordinate2D? // MARK: - Advanced Options @@ -164,7 +167,7 @@ public class TripPlannerViewModel: ObservableObject { /// Whether the API service can actually plan trips for the given mode. private static func isModeAvailable(_ mode: TransportMode, apiService: APIService) -> Bool { - mode != .bikeRental || apiService is VehicleRentalService + !mode.requiresVehicleRentalSupport || apiService is VehicleRentalService } /// The default selected mode: the first capability-available configured mode. @@ -216,7 +219,8 @@ public class TripPlannerViewModel: ObservableObject { transportModes: selectedTransportMode.apiModes, maxWalkDistance: maxWalkingDistance.meters, wheelchairAccessible: isWheelchairAccessible, - arriveBy: timePreference == .arriveBy + arriveBy: timePreference == .arriveBy, + viaPoint: viaPoint ) // Start loading state @@ -387,6 +391,7 @@ public class TripPlannerViewModel: ObservableObject { // Clear location selections selectedOrigin = nil selectedDestination = nil + viaPoint = nil // Clear trip planning results tripPlanResponse = nil diff --git a/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings index 580d765..df8c731 100644 --- a/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "المشي"; "transport_mode.bike" = "الدراجة"; "transport_mode.car" = "السيارة"; -"transport_mode.bike_rental" = "دراجات مشتركة"; +"transport_mode.bike_rental" = "دراجات مشتركة فقط"; +"transport_mode.transit_bike_rental" = "المواصلات + دراجات مشتركة"; "rental.vehicle_type.bike" = "دراجة"; "rental.vehicle_type.ebike" = "دراجة كهربائية"; diff --git a/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings index 30e8934..4fbddae 100644 --- a/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings @@ -217,7 +217,8 @@ "transport_mode.walk" = "Walk"; "transport_mode.bike" = "Bike"; "transport_mode.car" = "Car"; -"transport_mode.bike_rental" = "Bike Rental"; +"transport_mode.bike_rental" = "Bikeshare Only"; +"transport_mode.transit_bike_rental" = "Transit + Bikeshare"; "rental.vehicle_type.bike" = "bike"; "rental.vehicle_type.ebike" = "e-bike"; diff --git a/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings index e3363e0..afe5199 100644 --- a/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "A pie"; "transport_mode.bike" = "Bicicleta"; "transport_mode.car" = "Coche"; -"transport_mode.bike_rental" = "Bicicleta compartida"; +"transport_mode.bike_rental" = "Solo bicicleta compartida"; +"transport_mode.transit_bike_rental" = "Transporte público + bicicleta compartida"; "rental.vehicle_type.bike" = "bici"; "rental.vehicle_type.ebike" = "bici eléctrica"; diff --git a/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings index e53e84a..02e9a9d 100644 --- a/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "Lakad"; "transport_mode.bike" = "Bisikleta"; "transport_mode.car" = "Kotse"; -"transport_mode.bike_rental" = "Rentahan ng Bisikleta"; +"transport_mode.bike_rental" = "Rentahan ng Bisikleta Lamang"; +"transport_mode.transit_bike_rental" = "Transit + Rentahan ng Bisikleta"; "rental.vehicle_type.bike" = "bisikleta"; "rental.vehicle_type.ebike" = "e-bike"; diff --git a/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings index 58d6f07..5476bd8 100644 --- a/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "À pied"; "transport_mode.bike" = "Vélo"; "transport_mode.car" = "Voiture"; -"transport_mode.bike_rental" = "Vélo en libre-service"; +"transport_mode.bike_rental" = "Vélo en libre-service uniquement"; +"transport_mode.transit_bike_rental" = "Transports en commun + vélo en libre-service"; "rental.vehicle_type.bike" = "vélo"; "rental.vehicle_type.ebike" = "vélo électrique"; diff --git a/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings index d9a1a26..7a59bdc 100644 --- a/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "A piedi"; "transport_mode.bike" = "Bicicletta"; "transport_mode.car" = "Auto"; -"transport_mode.bike_rental" = "Bike sharing"; +"transport_mode.bike_rental" = "Solo bike sharing"; +"transport_mode.transit_bike_rental" = "Trasporto pubblico + bike sharing"; "rental.vehicle_type.bike" = "bici"; "rental.vehicle_type.ebike" = "bici elettrica"; diff --git a/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings index dc9b9fd..4b8c922 100644 --- a/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "도보"; "transport_mode.bike" = "자전거"; "transport_mode.car" = "자동차"; -"transport_mode.bike_rental" = "공유 자전거"; +"transport_mode.bike_rental" = "공유 자전거만"; +"transport_mode.transit_bike_rental" = "대중교통 + 공유 자전거"; "rental.vehicle_type.bike" = "자전거"; "rental.vehicle_type.ebike" = "전기 자전거"; diff --git a/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings index c431058..c136e97 100644 --- a/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "Pieszo"; "transport_mode.bike" = "Rower"; "transport_mode.car" = "Samochód"; -"transport_mode.bike_rental" = "Rower miejski"; +"transport_mode.bike_rental" = "Tylko rower miejski"; +"transport_mode.transit_bike_rental" = "Komunikacja miejska + rower miejski"; "rental.vehicle_type.bike" = "rower"; "rental.vehicle_type.ebike" = "rower elektryczny"; diff --git a/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings index d7904bd..f63ce07 100644 --- a/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "A pé"; "transport_mode.bike" = "Bicicleta"; "transport_mode.car" = "Carro"; -"transport_mode.bike_rental" = "Bicicleta compartilhada"; +"transport_mode.bike_rental" = "Apenas bicicleta compartilhada"; +"transport_mode.transit_bike_rental" = "Transporte público + bicicleta compartilhada"; "rental.vehicle_type.bike" = "bicicleta"; "rental.vehicle_type.ebike" = "bicicleta elétrica"; diff --git a/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings index 79f3dd1..0e3b77b 100644 --- a/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "Пешком"; "transport_mode.bike" = "Велосипед"; "transport_mode.car" = "Автомобиль"; -"transport_mode.bike_rental" = "Велопрокат"; +"transport_mode.bike_rental" = "Только велопрокат"; +"transport_mode.transit_bike_rental" = "Транспорт + велопрокат"; "rental.vehicle_type.bike" = "велосипед"; "rental.vehicle_type.ebike" = "электровелосипед"; diff --git a/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings index b2b463b..806087f 100644 --- a/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "Đi bộ"; "transport_mode.bike" = "Xe đạp"; "transport_mode.car" = "Ô tô"; -"transport_mode.bike_rental" = "Xe đạp công cộng"; +"transport_mode.bike_rental" = "Chỉ xe đạp công cộng"; +"transport_mode.transit_bike_rental" = "Phương tiện công cộng + xe đạp công cộng"; "rental.vehicle_type.bike" = "xe đạp"; "rental.vehicle_type.ebike" = "xe đạp điện"; diff --git a/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings index ad6ba17..c4f4bfa 100644 --- a/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings @@ -215,7 +215,8 @@ "transport_mode.walk" = "步行"; "transport_mode.bike" = "骑行"; "transport_mode.car" = "驾车"; -"transport_mode.bike_rental" = "共享单车"; +"transport_mode.bike_rental" = "仅共享单车"; +"transport_mode.transit_bike_rental" = "公交 + 共享单车"; "rental.vehicle_type.bike" = "单车"; "rental.vehicle_type.ebike" = "电动单车"; diff --git a/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings index 82fff66..36e68f6 100644 --- a/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings @@ -216,7 +216,8 @@ "transport_mode.walk" = "步行"; "transport_mode.bike" = "自行車"; "transport_mode.car" = "開車"; -"transport_mode.bike_rental" = "共享單車"; +"transport_mode.bike_rental" = "僅共享單車"; +"transport_mode.transit_bike_rental" = "大眾運輸 + 共享單車"; "rental.vehicle_type.bike" = "單車"; "rental.vehicle_type.ebike" = "電動單車"; diff --git a/OTPKit/Tests/GraphQLAPIServiceTests.swift b/OTPKit/Tests/GraphQLAPIServiceTests.swift index 22b4390..066b907 100644 --- a/OTPKit/Tests/GraphQLAPIServiceTests.swift +++ b/OTPKit/Tests/GraphQLAPIServiceTests.swift @@ -110,6 +110,49 @@ class GraphQLAPIServiceTests: OTPTestCase { XCTAssertNil(modes[1]["qualifier"]) } + func testFetchPlanSendsViaPoint() async throws { + mockDataLoader.mockResponse(data: Fixtures.loadData(file: "graphql_plan_success.json")) + + let request = TripPlanRequest( + origin: CLLocationCoordinate2D(latitude: 47.6097, longitude: -122.3331), + destination: CLLocationCoordinate2D(latitude: 47.6205, longitude: -122.3493), + date: Self.testDate, + time: Self.testTime, + transportModes: TransportMode.transitBikeRental.apiModes, + maxWalkDistance: 800, + viaPoint: CLLocationCoordinate2D(latitude: 47.6095, longitude: -122.337) + ) + _ = try await service.fetchPlan(request) + + let urlRequest = try XCTUnwrap(mockDataLoader.lastRequest) + let body = try XCTUnwrap(urlRequest.httpBody) + let payload = try XCTUnwrap(try JSONSerialization.jsonObject(with: body) as? [String: Any]) + + let query = try XCTUnwrap(payload["query"] as? String) + XCTAssertTrue(query.contains("via: $via")) + + let variables = try XCTUnwrap(payload["variables"] as? [String: Any]) + let via = try XCTUnwrap(variables["via"] as? [[String: Any]]) + XCTAssertEqual(via.count, 1) + let visit = try XCTUnwrap(via[0]["visit"] as? [String: Any]) + let coordinate = try XCTUnwrap(visit["coordinate"] as? [String: Any]) + XCTAssertEqual(coordinate["latitude"] as? Double, 47.6095) + XCTAssertEqual(coordinate["longitude"] as? Double, -122.337) + } + + func testFetchPlanOmitsViaWhenAbsent() async throws { + mockDataLoader.mockResponse(data: Fixtures.loadData(file: "graphql_plan_success.json")) + + _ = try await service.fetchPlan(createTripPlanRequest()) + + let request = try XCTUnwrap(mockDataLoader.lastRequest) + let body = try XCTUnwrap(request.httpBody) + let payload = try XCTUnwrap(try JSONSerialization.jsonObject(with: body) as? [String: Any]) + let variables = try XCTUnwrap(payload["variables"] as? [String: Any]) + // Absent, not null: not every OTP build treats a null via as "no via". + XCTAssertNil(variables["via"]) + } + // MARK: - Response Mapping func testFetchPlanMapsItineraries() async throws { diff --git a/OTPKit/Tests/TripPlanRequestTests.swift b/OTPKit/Tests/TripPlanRequestTests.swift index df07b92..1c8ca83 100644 --- a/OTPKit/Tests/TripPlanRequestTests.swift +++ b/OTPKit/Tests/TripPlanRequestTests.swift @@ -93,6 +93,40 @@ struct TripPlanRequestTests { #expect(request.transportModesString == "BICYCLE_RENT,WALK") } + @Test("Transit + Bikeshare expands to the REST mode list from the spec") + func transportModesStringTransitBikeRental() { + let request = TripPlanRequest( + origin: CLLocationCoordinate2D(latitude: 0, longitude: 0), + destination: CLLocationCoordinate2D(latitude: 1, longitude: 1), + date: Date(), + time: Date(), + transportModes: TransportMode.transitBikeRental.apiModes + ) + + // The composite mode itself never reaches the wire; its apiModes do. + #expect(request.transportModesString == "TRANSIT,WALK,BICYCLE_RENT") + } + + @Test("viaPoint participates in equality") + func viaPointEquality() { + let base = TripPlanRequest( + origin: CLLocationCoordinate2D(latitude: 0, longitude: 0), + destination: CLLocationCoordinate2D(latitude: 1, longitude: 1), + date: Date(timeIntervalSince1970: 0), + time: Date(timeIntervalSince1970: 0) + ) + let withVia = TripPlanRequest( + origin: CLLocationCoordinate2D(latitude: 0, longitude: 0), + destination: CLLocationCoordinate2D(latitude: 1, longitude: 1), + date: Date(timeIntervalSince1970: 0), + time: Date(timeIntervalSince1970: 0), + viaPoint: CLLocationCoordinate2D(latitude: 47.6, longitude: -122.3) + ) + + #expect(base != withVia) + #expect(base == base) + } + @Test("transportModesString with single mode") func transportModesStringSingleMode() { let request = TripPlanRequest( diff --git a/OTPKit/Tests/TripPlannerViewModelTests.swift b/OTPKit/Tests/TripPlannerViewModelTests.swift index 7163f01..e154aa3 100644 --- a/OTPKit/Tests/TripPlannerViewModelTests.swift +++ b/OTPKit/Tests/TripPlannerViewModelTests.swift @@ -151,6 +151,23 @@ struct TripPlannerViewModelTests { #expect(viewModel.availableTransportModes == [.bike, .car]) } + @Test("availableTransportModes filters transit + bikeshare when the service lacks rental support") + func availableTransportModesFiltersTransitBikeRental() { + let viewModel = createViewModel(enabledModes: [.transit, .walk, .transitBikeRental, .bikeRental]) + + #expect(viewModel.availableTransportModes == [.transit, .walk]) + } + + @Test("availableTransportModes keeps transit + bikeshare when the service supports rentals") + func availableTransportModesKeepsTransitBikeRentalWithCapableService() { + let viewModel = createViewModel( + enabledModes: [.transit, .walk, .transitBikeRental], + mockAPIService: TestFixtures.MockRentalAPIService() + ) + + #expect(viewModel.availableTransportModes == [.transit, .walk, .transitBikeRental]) + } + @Test("Default selected mode skips an unavailable rental mode listed first") func defaultSelectedModeSkipsUnavailableRentalMode() { // .bikeRental configured first, but the service has no rental support: diff --git a/OTPKit/Tests/VehicleRentalSourceTests.swift b/OTPKit/Tests/VehicleRentalSourceTests.swift new file mode 100644 index 0000000..bfe8120 --- /dev/null +++ b/OTPKit/Tests/VehicleRentalSourceTests.swift @@ -0,0 +1,355 @@ +// +// VehicleRentalSourceTests.swift +// OTPKitTests +// +// Tests for the stateful viewport→snapshot rental pipeline: coalescing, +// cancellation, diffing, clearing, and failure reporting. +// + +import Foundation +import Testing +@testable import OTPKit + +@Suite("VehicleRentalSource") +struct VehicleRentalSourceTests { + + // MARK: - Scripted service + + private actor ScriptedRentalService: VehicleRentalService { + struct Call: Sendable { + let boundingBox: VehicleRentalBoundingBox + let formFactors: Set? + } + + private(set) var calls: [Call] = [] + private var results: [Result] + private var delay: Duration = .zero + + init(results: [Result]) { + self.results = results + } + + func setDelay(_ delay: Duration) { + self.delay = delay + } + + func fetchVehicleRentals( + in boundingBox: VehicleRentalBoundingBox, + formFactors: Set? + ) async throws -> VehicleRentalFetchResult { + calls.append(Call(boundingBox: boundingBox, formFactors: formFactors)) + + // Claim the scripted result at call time, before any delay: a cancelled + // call must still consume its result so later calls stay aligned with + // the script. The last result is sticky so repeated fetches keep working. + let result: Result + if results.isEmpty { + result = .success(VehicleRentalFetchResult(rentals: [])) + } else if results.count == 1 { + result = results[0] + } else { + result = results.removeFirst() + } + + if delay > .zero { + try await Task.sleep(for: delay) + } + return try result.get() + } + } + + private struct ScriptedError: Error {} + + // MARK: - Fixtures + + private static let seattleBox = VehicleRentalBoundingBox( + minimumLatitude: 47.5, + maximumLatitude: 47.7, + minimumLongitude: -122.4, + maximumLongitude: -122.2 + ) + + private static func makeRental(id: String, lat: Double = 47.61) -> VehicleRental { + .vehicle(RentalVehicle( + vehicleId: id, + name: "Default vehicle type", + lat: lat, + lon: -122.33, + allowPickupNow: true, + operative: true, + rentalNetwork: RentalNetwork(networkId: "lime_seattle", url: nil), + rentalUris: nil, + vehicleType: VehicleType(formFactor: .bicycle, propulsionType: "ELECTRIC_ASSIST"), + fuel: nil + )) + } + + private static func makeSource( + service: ScriptedRentalService, + formFactors: Set? = nil, + coalescingInterval: Duration = .milliseconds(1), + boundingBoxPadding: Double = 1.0 + ) -> VehicleRentalSource { + VehicleRentalSource( + service: service, + formFactors: formFactors, + coalescingInterval: coalescingInterval, + boundingBoxPadding: boundingBoxPadding + ) + } + + // MARK: - Tests + + @Test("First fetch delivers everything as added") + func firstFetchAddsEverything() async throws { + let rentals = [Self.makeRental(id: "a"), Self.makeRental(id: "b")] + let service = ScriptedRentalService(results: [.success(VehicleRentalFetchResult(rentals: rentals))]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.added.map(\.id) == ["a", "b"]) + #expect(snapshot.removed.isEmpty) + #expect(snapshot.updated.isEmpty) + #expect(snapshot.partialErrors.isEmpty) + } + + @Test("Successive fetches deliver an id-keyed diff") + func diffAcrossViewports() async throws { + let first = [Self.makeRental(id: "a"), Self.makeRental(id: "b")] + let second = [Self.makeRental(id: "b", lat: 47.62), Self.makeRental(id: "c")] + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: first)), + .success(VehicleRentalFetchResult(rentals: second)) + ]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setViewport(Self.seattleBox) + let snapshot = try #require(await snapshots.next()) + + #expect(snapshot.added.map(\.id) == ["c"]) + #expect(snapshot.removed == ["a"]) + #expect(snapshot.updated.map(\.id) == ["b"]) + } + + @Test("An unchanged entity is neither added nor updated") + func unchangedEntityNotRedelivered() async throws { + let rentals = [Self.makeRental(id: "a")] + let service = ScriptedRentalService(results: [.success(VehicleRentalFetchResult(rentals: rentals))]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setViewport(Self.seattleBox) + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.isEmpty) + } + + @Test("Rapid viewport changes coalesce into one fetch") + func coalescesViewportChanges() async throws { + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])) + ]) + let source = Self.makeSource(service: service, coalescingInterval: .milliseconds(100)) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(VehicleRentalBoundingBox( + minimumLatitude: 40, maximumLatitude: 41, minimumLongitude: -100, maximumLongitude: -99 + )) + await source.setViewport(VehicleRentalBoundingBox( + minimumLatitude: 41, maximumLatitude: 42, minimumLongitude: -101, maximumLongitude: -100 + )) + await source.setViewport(Self.seattleBox) + + _ = try #require(await snapshots.next()) + + let calls = await service.calls + #expect(calls.count == 1) + #expect(calls.first?.boundingBox == Self.seattleBox) + } + + @Test("A superseded in-flight fetch is cancelled, not reported as a failure") + func supersededFetchIsCancelled() async throws { + let first = [Self.makeRental(id: "stale")] + let second = [Self.makeRental(id: "fresh")] + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: first)), + .success(VehicleRentalFetchResult(rentals: second)) + ]) + await service.setDelay(.milliseconds(200)) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + let failures = Box() + let failureWatcher = Task { + for await failure in source.fetchFailures { + await failures.append(failure.message) + } + } + + await source.setViewport(Self.seattleBox) + try await Task.sleep(for: .milliseconds(50)) // let the first fetch get in flight + await source.setViewport(Self.seattleBox) + + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.added.map(\.id) == ["fresh"]) + #expect(await service.calls.count == 2) + + try await Task.sleep(for: .milliseconds(50)) + #expect(await failures.values.isEmpty) + failureWatcher.cancel() + } + + @Test("A nil viewport clears everything immediately") + func nilViewportClears() async throws { + let rentals = [Self.makeRental(id: "a"), Self.makeRental(id: "b")] + let service = ScriptedRentalService(results: [.success(VehicleRentalFetchResult(rentals: rentals))]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setViewport(nil) + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.added.isEmpty) + #expect(snapshot.removed == ["a", "b"]) + #expect(await service.calls.count == 1) + } + + @Test("reset() clears state and emits a removal snapshot") + func resetClears() async throws { + let rentals = [Self.makeRental(id: "a")] + let service = ScriptedRentalService(results: [.success(VehicleRentalFetchResult(rentals: rentals))]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.reset() + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.removed == ["a"]) + } + + @Test("Partial GraphQL errors ride along on the snapshot") + func partialErrorsForwarded() async throws { + let result = VehicleRentalFetchResult( + rentals: [Self.makeRental(id: "a")], + partialErrors: ["feed lime_tacoma unavailable"] + ) + let service = ScriptedRentalService(results: [.success(result)]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.partialErrors == ["feed lime_tacoma unavailable"]) + } + + @Test("A failed fetch reports on fetchFailures and preserves delivered state") + func failureReported() async throws { + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])), + .failure(ScriptedError()) + ]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + var failures = source.fetchFailures.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setViewport(Self.seattleBox) + let failure = try #require(await failures.next()) + #expect(failure.underlyingError is ScriptedError) + + // The next successful fetch diffs against state that survived the failure. + await source.setViewport(Self.seattleBox) + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.isEmpty) + } + + @Test("Changing form factors refetches with the new filter") + func formFactorChangeRefetches() async throws { + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])) + ]) + let source = Self.makeSource(service: service, formFactors: [.bicycle]) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setFormFactors([.bicycle, .scooter]) + _ = await snapshots.next() + + let calls = await service.calls + #expect(calls.count == 2) + #expect(calls[0].formFactors == [.bicycle]) + #expect(calls[1].formFactors == [.bicycle, .scooter]) + } + + @Test("Setting the same form factors does not refetch") + func sameFormFactorsNoRefetch() async throws { + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: [])) + ]) + let source = Self.makeSource(service: service, formFactors: [.bicycle]) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setFormFactors([.bicycle]) + try await Task.sleep(for: .milliseconds(30)) + #expect(await service.calls.count == 1) + } + + @Test("The viewport is padded before fetching") + func paddingApplied() async throws { + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: [])) + ]) + let source = Self.makeSource(service: service, boundingBoxPadding: 2.0) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(VehicleRentalBoundingBox( + minimumLatitude: 47.0, maximumLatitude: 48.0, minimumLongitude: -122.0, maximumLongitude: -121.0 + )) + _ = await snapshots.next() + + let box = try #require(await service.calls.first?.boundingBox) + #expect(abs(box.minimumLatitude - 46.5) < 0.0001) + #expect(abs(box.maximumLatitude - 48.5) < 0.0001) + #expect(abs(box.minimumLongitude - (-122.5)) < 0.0001) + #expect(abs(box.maximumLongitude - (-120.5)) < 0.0001) + } + + @Test("Duplicate ids in one payload are delivered once") + func duplicateIdsDeliveredOnce() async throws { + let rentals = [Self.makeRental(id: "a"), Self.makeRental(id: "a", lat: 47.62)] + let service = ScriptedRentalService(results: [.success(VehicleRentalFetchResult(rentals: rentals))]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.added.map(\.id) == ["a"]) + } + + // MARK: - Helpers + + private actor Box { + private(set) var values: [String] = [] + func append(_ value: String) { values.append(value) } + } +} From aa633fdc539848ab1b927a3721b2986a6d113793 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Wed, 29 Jul 2026 00:47:17 -0700 Subject: [PATCH 2/6] Render rental legs across the Progress Rail and legacy leg views - TripProgress: rental rides count as .riding; rental legs produce pickup -> ride -> dropoff rows mirroring the transit board/ride/getOff shape; stopsRemaining stays transit-only; segments gain isRental - Rail views: rental row content, purple marks and ride bars, rental variants in the tip detent and progress bar - Placeholder hygiene: 'Default vehicle type' never reaches the UI (Leg.riderFacingToName falls back to a localized generic) - Legacy views: BICYCLE/BIKE arms in DirectionLegView and the itinerary preview flow, purple-tinted when the ride is a rental - shouldMergeLegs explicitly refuses to merge across a rental boundary - New rail strings localized in all 13 locales; TripProgress rental tests --- .../Core/Extensions/ColorExtension.swift | 24 ++++ .../OTPKit/Core/Map/OTPMapProvider.swift | 5 +- .../Sources/OTPKit/Core/Models/OTP/Leg.swift | 28 +++++ .../Core/TripProgress/TripProgress.swift | 66 +++++++++- .../DirectionLegs/DirectionLegBikeView.swift | 48 +++++++ .../DirectionLegs/DirectionLegView.swift | 4 + .../DirectionLegs/DirectionLegWalkView.swift | 2 +- .../Directions/Rail/InTripRailView.swift | 19 ++- .../Directions/Rail/RailRowContentViews.swift | 108 +++++++++++++++- .../Directions/Rail/TipContentView.swift | 45 +++++-- .../Directions/Rail/TripProgressBarView.swift | 3 + .../ItineraryLegs/ItineraryLegBikeView.swift | 31 +++++ .../Components/ItineraryPreviewView.swift | 2 + .../Resources/ar.lproj/Localizable.strings | 11 ++ .../Resources/en.lproj/Localizable.strings | 11 ++ .../Resources/es.lproj/Localizable.strings | 11 ++ .../Resources/fil.lproj/Localizable.strings | 11 ++ .../Resources/fr.lproj/Localizable.strings | 11 ++ .../Resources/it.lproj/Localizable.strings | 11 ++ .../Resources/ko.lproj/Localizable.strings | 11 ++ .../Resources/pl.lproj/Localizable.strings | 11 ++ .../Resources/pt-BR.lproj/Localizable.strings | 11 ++ .../Resources/ru.lproj/Localizable.strings | 11 ++ .../Resources/vi.lproj/Localizable.strings | 11 ++ .../zh-Hans.lproj/Localizable.strings | 11 ++ .../zh-Hant.lproj/Localizable.strings | 11 ++ OTPKit/Tests/TripProgressTests.swift | 117 ++++++++++++++++++ 27 files changed, 625 insertions(+), 20 deletions(-) create mode 100644 OTPKit/Sources/OTPKit/Core/Extensions/ColorExtension.swift create mode 100644 OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegBikeView.swift create mode 100644 OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryLegs/ItineraryLegBikeView.swift diff --git a/OTPKit/Sources/OTPKit/Core/Extensions/ColorExtension.swift b/OTPKit/Sources/OTPKit/Core/Extensions/ColorExtension.swift new file mode 100644 index 0000000..e9ed001 --- /dev/null +++ b/OTPKit/Sources/OTPKit/Core/Extensions/ColorExtension.swift @@ -0,0 +1,24 @@ +/* + * Copyright (C) Open Transit Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for specific language governing permissions and + * limitations under the License. + */ + +import SwiftUI + +extension Color { + /// Rental purple (#7B4FD1). Every rental surface — browse-layer pins, + /// trip-planner pickup/dropoff markers, and rail rows — shares this color + /// so rentals read as one system. + static let otpRentalPurple = Color(red: 0x7B / 255.0, green: 0x4F / 255.0, blue: 0xD1 / 255.0) +} diff --git a/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift b/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift index 597fe28..f1269e1 100644 --- a/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift +++ b/OTPKit/Sources/OTPKit/Core/Map/OTPMapProvider.swift @@ -165,10 +165,7 @@ public enum OTPAnnotationType { case .intermediateStop: return .gray case .rentalVehicle: - // Rental purple (#7B4FD1): every rental surface — browse layer pins and - // trip-planner pickup/dropoff markers — shares this color so rentals - // read as one system. - return Color(red: 0x7B / 255.0, green: 0x4F / 255.0, blue: 0xD1 / 255.0) + return .otpRentalPurple case .routeLegend: return .clear // Custom view will handle coloring } diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift index ad9f57a..349572e 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift @@ -49,6 +49,12 @@ public struct Leg: Codable, Hashable { mode.lowercased() == "walk" } + /// True when this leg is ridden on a rented vehicle. The ride leg's `mode` is + /// plain "BICYCLE", so `rentedBike` is the only reliable rental discriminator. + public var isRentalRide: Bool { + rentedBike == true + } + public let routeType: RouteType? public let routeColor: String? @@ -196,6 +202,12 @@ public struct Leg: Codable, Hashable { /// - leg2: The later leg /// - Returns: Whether or not to merge the legs. public static func shouldMergeLegs(leg1: Leg, leg2: Leg) -> Bool { + // A merge must never cross a rental boundary: a merged leg keeps leg1's + // rentedBike flag, which would silently absorb a pickup or dropoff. In + // practice rental legs are non-transit and can't match the conditions + // below anyway — this guard keeps that invariant explicit. + guard leg1.isRentalRide == leg2.isRentalRide else { return false } + return leg1.route != nil && leg2.route != nil && leg1.route == leg2.route && @@ -264,6 +276,22 @@ public struct Leg: Codable, Hashable { route ?? modeDisplayName } + /// The rider-facing name of this leg's ending place. Rental feeds ship the + /// literal placeholder "Default vehicle type" as a free-floating vehicle's + /// name; it is replaced with a localized generic so it never reaches the UI. + public var riderFacingToName: String { + Self.riderFacingName(of: to) + } + + static func riderFacingName(of place: Place) -> String { + let trimmed = place.name.trimmingCharacters(in: .whitespacesAndNewlines) + let isPlaceholder = trimmed.lowercased() == "default vehicle type" + if isPlaceholder || (trimmed.isEmpty && place.bikeShareId != nil) { + return OTPLoc("place.rental_bike", comment: "Generic name for a rental bike location") + } + return trimmed + } + // MARK: - Real-Time Status /// Real-time status of this leg's departure, from `realTime` and `departureDelay`. diff --git a/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift b/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift index 0730a09..2f016a5 100644 --- a/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift +++ b/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift @@ -30,7 +30,7 @@ public enum TripPhase: Equatable { /// The rider has finished the previous leg and is waiting to board transit. case waiting(boardingLegIndex: Int) - /// The rider is aboard a transit leg. + /// The rider is aboard a transit leg or riding a rental vehicle. case riding(legIndex: Int) /// The itinerary's end time has passed. @@ -64,6 +64,12 @@ public struct RailRow: Identifiable, Equatable { case ride(legIndex: Int) /// Alight from this leg. case getOff(legIndex: Int) + /// Pick up the rental vehicle that starts this rental ride leg. + case pickUpVehicle(legIndex: Int) + /// Currently riding this rental leg. Present only while `TripPhase.riding` it. + case rideRental(legIndex: Int) + /// Drop off the rental vehicle at the end of this rental ride leg. + case dropOffVehicle(legIndex: Int) /// The final destination. case arrive } @@ -91,7 +97,8 @@ public struct RailRow: Identifiable, Equatable { /// The index of the leg this row belongs to, if any. public var legIndex: Int? { switch kind { - case .walk(let index), .board(let index), .ride(let index), .getOff(let index): + case .walk(let index), .board(let index), .ride(let index), .getOff(let index), + .pickUpVehicle(let index), .rideRental(let index), .dropOffVehicle(let index): return index case .arrive: return nil @@ -127,9 +134,11 @@ public struct TripProgress { } for (index, leg) in legs.enumerated() { - // Inside the leg's own time window. + // Inside the leg's own time window. Rental rides count as riding even + // though they are not transit legs. if now >= leg.startTime && now < leg.endTime { - return leg.transitLeg == true ? .riding(legIndex: index) : .walking(legIndex: index) + let isAboard = leg.transitLeg == true || leg.isRentalRide + return isAboard ? .riding(legIndex: index) : .walking(legIndex: index) } // In the gap between this leg and the next: waiting if the next leg @@ -167,6 +176,24 @@ public struct TripProgress { } rows.append(getOffRow(for: leg, at: index)) + } else if leg.isRentalRide { + // Rental legs get pickup → ride → dropoff semantics, mirroring the + // transit board → ride → get off shape so the rail reads uniformly. + rows.append(pickUpVehicleRow(for: leg, at: index)) + + if case .riding(let ridingIndex) = phase, ridingIndex == index { + rows.append( + RailRow( + id: "riderental-\(index)", + kind: .rideRental(legIndex: index), + state: .current, + time: now, + status: nil + ) + ) + } + + rows.append(dropOffVehicleRow(for: leg, at: index)) } else { rows.append(walkRow(for: leg, at: index, phase: phase)) } @@ -224,6 +251,29 @@ public struct TripProgress { ) } + /// Unlike a transit boarding, a pickup is never "current": there is no waiting + /// phase for a parked vehicle — the rider walks up and rides. The current row + /// during the ride is the synthetic `rideRental` row. + private func pickUpVehicleRow(for leg: Leg, at index: Int) -> RailRow { + RailRow( + id: "pickup-\(index)", + kind: .pickUpVehicle(legIndex: index), + state: now >= leg.startTime ? .done : .upcoming, + time: leg.startTime, + status: nil + ) + } + + private func dropOffVehicleRow(for leg: Leg, at index: Int) -> RailRow { + RailRow( + id: "dropoff-\(index)", + kind: .dropOffVehicle(legIndex: index), + state: now >= leg.endTime ? .done : .upcoming, + time: leg.endTime, + status: nil + ) + } + /// The rider's current activity, localized ("Walking", "Waiting for the C Line", /// "Riding C Line"). Nil before the trip starts and after it ends — the one /// vocabulary shared by the Back-to-now pill, the tip footer, and the @@ -237,6 +287,9 @@ public struct TripProgress { comment: "The rider is waiting for this route", legs[index].riderFacingRouteName) case .riding(let index): + if legs[index].isRentalRide { + return OTPLoc("rail.now_riding_rental", comment: "The rider is riding a rental bike") + } return OTPLoc("rail.now_riding_fmt", comment: "The rider is aboard this route", legs[index].riderFacingRouteName) @@ -366,6 +419,8 @@ public struct TripProgress { public let fillFraction: Double /// True when this leg is a transit leg (drawn in the route color). public let isTransit: Bool + /// True when this leg is a rental ride (drawn in rental purple). + public let isRental: Bool } /// Proportional segments for the tip-detent progress bar. @@ -389,7 +444,8 @@ public struct TripProgress { legIndex: index, widthFraction: duration / totalDuration, fillFraction: fill, - isTransit: leg.transitLeg == true + isTransit: leg.transitLeg == true, + isRental: leg.isRentalRide ) } } diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegBikeView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegBikeView.swift new file mode 100644 index 0000000..26c76b1 --- /dev/null +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegBikeView.swift @@ -0,0 +1,48 @@ +// +// DirectionLegBikeView.swift +// OTPKit +// + +import SwiftUI + +/// A bicycle leg in the legacy directions list — personal bike or rental ride. +struct DirectionLegBikeView: View { + let leg: Leg + + var body: some View { + DirectionLegContainerView { + Image(systemName: "bicycle") + .font(.system(size: 24)) + .foregroundStyle(leg.isRentalRide ? Color.otpRentalPurple : Color.primary) + } rightContent: { + VStack(alignment: .leading, spacing: 4) { + Text(instruction) + .font(.title3) + .fontWeight(.bold) + .fixedSize(horizontal: false, vertical: true) + + Text(OTPLoc( + "leg.walk_distance_duration", + comment: "Walking distance followed by approximate duration", + Formatters.formatDistance(Int(leg.distance)), + Formatters.formatTimeDuration(leg.duration) + )) + .foregroundStyle(.gray) + .fixedSize(horizontal: false, vertical: true) + } + } + } + + private var instruction: String { + if leg.isRentalRide { + return OTPLoc("leg.ride_rental_bike_to", + comment: "Instruction to ride a rental bike to a place", + leg.riderFacingToName) + } + return OTPLoc("leg.bike_to", comment: "Instruction to bike to a place", leg.riderFacingToName) + } +} + +#Preview { + DirectionLegBikeView(leg: PreviewHelpers.buildLeg()) +} diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegView.swift index af8466a..501dac9 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegView.swift @@ -17,6 +17,10 @@ struct DirectionLegView: View { DirectionLegVehicleView(leg: leg) case "WALK": DirectionLegWalkView(leg: leg) + // OTP 2.x spells bicycle legs "BICYCLE" (rental rides included); + // "BIKE" is the OTP 1.x REST spelling. + case "BICYCLE", "BIKE": + DirectionLegBikeView(leg: leg) default: DirectionLegUnknownView(leg: leg) } diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegWalkView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegWalkView.swift index 73b2226..9bcca0e 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegWalkView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Components/DirectionLegs/DirectionLegWalkView.swift @@ -19,7 +19,7 @@ struct DirectionLegWalkView: View { } rightContent: { HStack { VStack(alignment: .leading, spacing: 4) { - Text(OTPLoc("leg.walk_to", comment: "Instruction to walk to a place", leg.to.name)) + Text(OTPLoc("leg.walk_to", comment: "Instruction to walk to a place", leg.riderFacingToName)) .font(.title3) .fontWeight(.bold) .fixedSize(horizontal: false, vertical: true) diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift index e1cae56..f6f8ed3 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift @@ -111,6 +111,16 @@ struct InTripRailView: View { RideRowContent(progress: progress, legIndex: index) case .getOff(let index): GetOffRowContent(progress: progress, legIndex: index, state: row.state) + case .pickUpVehicle(let index): + PickUpVehicleRowContent( + leg: progress.legs[index], + state: row.state, + isExpanded: isFocused + ) + case .rideRental(let index): + RideRentalRowContent(progress: progress, legIndex: index) + case .dropOffVehicle(let index): + DropOffVehicleRowContent(progress: progress, legIndex: index, state: row.state) case .arrive: ArriveRowContent() } @@ -137,6 +147,8 @@ struct InTripRailView: View { return .destination case (.walk, _): return .ring(Color(.systemGray3)) + case (.pickUpVehicle, _), (.rideRental, _), (.dropOffVehicle, _): + return .ring(.otpRentalPurple) case (.board(let index), _), (.getOff(let index), _), (.ride(let index), _): return .ring(progress.legs[index].routeUIColor ?? Color(.systemGray2)) } @@ -153,7 +165,12 @@ struct InTripRailView: View { return .thin } return .bar(progress.legs[index].routeUIColor ?? Color(.systemGray2)) - case .walk, .getOff: + case .pickUpVehicle(let index), .rideRental(let index): + if row.state == .done, case .pickUpVehicle = row.kind, !isRiding(index) { + return .thin + } + return .bar(.otpRentalPurple) + case .walk, .getOff, .dropOffVehicle: return .thin } } diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift index 94bfa52..d37b810 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift @@ -49,6 +49,14 @@ enum RailText { Formatters.formatDateToTime(leg.startTime)) } + /// The rider-facing name of a rental place, or nil when the feed sent a known + /// placeholder ("Default vehicle type") that must never reach the UI. + static func rentalPlaceName(_ name: String) -> String? { + let trimmed = name.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty, trimmed.lowercased() != "default vehicle type" else { return nil } + return trimmed + } + /// "12 stops" / "1 stop". static func stops(_ count: Int) -> String { count == 1 @@ -104,7 +112,7 @@ struct WalkRowContent: View { VStack(alignment: .leading, spacing: 2) { Text(OTPLoc("rail.walk_fmt", comment: "Walking instruction: distance, then destination", - Formatters.formatDistance(Int(leg.distance)), leg.to.name)) + Formatters.formatDistance(Int(leg.distance)), leg.riderFacingToName)) .font(.body.weight(.semibold)) Text(OTPLoc("rail.about_duration_fmt", @@ -357,6 +365,104 @@ struct GetOffRowContent: View { } } +// MARK: - Rental rows + +/// Pickup point of a rental ride: "Pick up rental bike", with the station or +/// vehicle name when the feed provides a real one. +struct PickUpVehicleRowContent: View { + let leg: Leg + let state: RailRow.State + let isExpanded: Bool + + var body: some View { + if state == .done { + Text(OTPLoc("rail.picked_up_bike", comment: "Collapsed row for a rental vehicle already picked up")) + .font(.subheadline) + .foregroundStyle(.secondary) + } else { + VStack(alignment: .leading, spacing: 2) { + Text(OTPLoc("rail.pick_up_bike", comment: "Instruction to pick up the rental bike")) + .font(.body.weight(.semibold)) + + if let name = RailText.rentalPlaceName(leg.from.name) { + Text(name) + .font(.subheadline) + .foregroundStyle(.secondary) + } + + if isExpanded { + Text(OTPLoc("rail.ride_distance_duration_fmt", + comment: "Distance, then approximate duration of the rental ride", + Formatters.formatDistance(Int(leg.distance)), + Formatters.formatTimeDuration(leg.duration))) + .font(.subheadline) + .foregroundStyle(.secondary) + } + } + } + } +} + +/// The synthetic row present only while riding a rental: destination and time +/// remaining inside the now-card. +struct RideRentalRowContent: View { + let progress: TripProgress + let legIndex: Int + + private var leg: Leg { progress.legs[legIndex] } + + var body: some View { + NowCard { + VStack(alignment: .leading, spacing: 6) { + Text(OTPLoc("rail.ride_bike_to_fmt", + comment: "Riding instruction: where the rental ride ends", leg.riderFacingToName)) + .font(.title3.weight(.semibold)) + + Text(OTPLoc("rail.ride_distance_duration_fmt", + comment: "Distance, then approximate duration of the rental ride", + Formatters.formatDistance(Int(leg.distance)), + Formatters.formatTimeDuration(leg.duration))) + .font(.subheadline) + .foregroundStyle(.secondary) + } + } + } +} + +/// "Drop off bike", with the dropoff station named when the ride ends docked. +struct DropOffVehicleRowContent: View { + let progress: TripProgress + let legIndex: Int + let state: RailRow.State + + private var leg: Leg { progress.legs[legIndex] } + + var body: some View { + if state == .done { + Text(OTPLoc("rail.drop_off_bike", comment: "Instruction to drop off the rental bike")) + .font(.subheadline) + .foregroundStyle(.secondary) + } else { + VStack(alignment: .leading, spacing: 2) { + Text(OTPLoc("rail.drop_off_bike", comment: "Instruction to drop off the rental bike")) + .font(.body.weight(.semibold)) + + if let name = RailText.rentalPlaceName(leg.to.name) { + Text(name) + .font(.subheadline) + .foregroundStyle(.secondary) + } + + if let wait = RailText.waitDescription(progress: progress, afterLegAt: legIndex) { + Text(wait) + .font(.subheadline) + .foregroundStyle(.secondary) + } + } + } + } +} + // MARK: - Arrive row /// The final destination row: a solid dark pip and one word. diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift index 840637c..4555a9c 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift @@ -99,7 +99,7 @@ struct TipContentView: View { walkIcon title(OTPLoc("rail.walk_fmt", comment: "Walking instruction: distance, then destination", - Formatters.formatDistance(Int(leg.distance)), leg.to.name), + Formatters.formatDistance(Int(leg.distance)), leg.riderFacingToName), subtitle: progress.currentStep(onLegAt: index)?.localizedInstruction) trailingCountdown( Formatters.formatCountdown(leg.endTime.timeIntervalSince(progress.now)), @@ -119,12 +119,24 @@ struct TipContentView: View { case .riding(let index): let leg = progress.legs[index] - HStack(alignment: .top, spacing: 12) { - RouteBadge(leg: leg) - title(progress.stopsRemaining(onLegAt: index).map(RailText.stopsToGo) ?? RailText.routeName(leg), - subtitle: OTPLoc("rail.get_off_at_fmt", - comment: "The stop where the rider gets off", leg.to.name)) - trailingTime(leg.endTime, status: leg.arrivalStatus) + if leg.isRentalRide { + HStack(alignment: .top, spacing: 12) { + bikeIcon + title(OTPLoc("rail.ride_bike_to_fmt", + comment: "Riding instruction: where the rental ride ends", + leg.riderFacingToName), + subtitle: OTPLoc("rail.drop_off_bike", + comment: "Instruction to drop off the rental bike")) + trailingTime(leg.endTime, status: nil) + } + } else { + HStack(alignment: .top, spacing: 12) { + RouteBadge(leg: leg) + title(progress.stopsRemaining(onLegAt: index).map(RailText.stopsToGo) ?? RailText.routeName(leg), + subtitle: OTPLoc("rail.get_off_at_fmt", + comment: "The stop where the rider gets off", leg.to.name)) + trailingTime(leg.endTime, status: leg.arrivalStatus) + } } case .arrived: @@ -146,11 +158,19 @@ struct TipContentView: View { subtitle: leg.headsign.map { OTPLoc("rail.toward_fmt", comment: "Vehicle headsign", $0) } ?? leg.to.name) + } else if leg.isRentalRide { + bikeIcon + title(OTPLoc("rail.ride_bike_to_fmt", + comment: "Riding instruction: where the rental ride ends", + leg.riderFacingToName), + subtitle: OTPLoc("rail.about_duration_fmt", + comment: "Approximate duration of a walking leg", + Formatters.formatTimeDuration(leg.duration))) } else { walkIcon title(OTPLoc("rail.walk_fmt", comment: "Walking instruction: distance, then destination", - Formatters.formatDistance(Int(leg.distance)), leg.to.name), + Formatters.formatDistance(Int(leg.distance)), leg.riderFacingToName), subtitle: OTPLoc("rail.about_duration_fmt", comment: "Approximate duration of a walking leg", Formatters.formatTimeDuration(leg.duration))) @@ -170,6 +190,15 @@ struct TipContentView: View { .accessibilityHidden(true) } + private var bikeIcon: some View { + Image(systemName: "bicycle") + .font(.subheadline.weight(.semibold)) + .foregroundStyle(.white) + .frame(width: 32, height: 32) + .background(Color.otpRentalPurple, in: RoundedRectangle(cornerRadius: 9)) + .accessibilityHidden(true) + } + private func title(_ text: String, subtitle: String?) -> some View { VStack(alignment: .leading, spacing: 3) { Text(text) diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TripProgressBarView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TripProgressBarView.swift index 9a531b8..97b7ca0 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TripProgressBarView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TripProgressBarView.swift @@ -37,6 +37,9 @@ struct TripProgressBarView: View { private func segmentView(_ segment: TripProgress.Segment) -> some View { let fillColor: Color = { + if segment.isRental { + return .otpRentalPurple + } if segment.isTransit { return progress.legs[segment.legIndex].routeUIColor ?? theme.primaryColor } diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryLegs/ItineraryLegBikeView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryLegs/ItineraryLegBikeView.swift new file mode 100644 index 0000000..957f01a --- /dev/null +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryLegs/ItineraryLegBikeView.swift @@ -0,0 +1,31 @@ +// +// ItineraryLegBikeView.swift +// OTPKit +// + +import SwiftUI + +/// Represents an itinerary leg ridden on a bicycle — personal or rental. +/// Rental rides tint in rental purple so they read as part of the rental system. +struct ItineraryLegBikeView: View { + let leg: Leg + + var body: some View { + HStack(spacing: 4) { + Image(systemName: "bicycle") + .font(.caption) + Text(Formatters.formatTimeDuration(leg.duration)) + .font(.caption) + } + .padding(.horizontal, 8) + .padding(.vertical, 4) + .background(leg.isRentalRide ? Color.otpRentalPurple.opacity(0.15) : Color.gray.opacity(0.2)) + .foregroundStyle(leg.isRentalRide ? Color.otpRentalPurple : Color.primary) + .clipShape(RoundedRectangle(cornerRadius: 8)) + .frame(height: 40) + } +} + +#Preview { + ItineraryLegBikeView(leg: PreviewHelpers.buildLeg()) +} diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryPreviewView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryPreviewView.swift index ad0827d..d6d0d72 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryPreviewView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/TripPlanner/Components/ItineraryPreviewView.swift @@ -71,6 +71,8 @@ struct ItineraryPreviewView: View { private func legView(for leg: Leg) -> some View { if leg.walkMode { ItineraryLegWalkView(leg: leg) + } else if leg.isRentalRide || LegMode(otpMode: leg.mode) == .bicycle { + ItineraryLegBikeView(leg: leg) } else if let routeType = leg.routeType, routeType != .nonTransit { ItineraryLegVehicleView(leg: leg) } else { diff --git a/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings index df8c731..989db7b 100644 --- a/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/ar.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "الخطوة الأخيرة"; "rail.arrive_time_fmt" = "الوصول %@"; "rail.now" = "الآن"; + +/* Vehicle rental legs */ +"leg.bike_to" = "اذهب بالدراجة إلى %@"; +"leg.ride_rental_bike_to" = "اركب الدراجة المستأجرة إلى %@"; +"place.rental_bike" = "دراجة مستأجرة"; +"rail.now_riding_rental" = "تركب دراجة مستأجرة"; +"rail.pick_up_bike" = "استلم الدراجة المستأجرة"; +"rail.picked_up_bike" = "تم استلام الدراجة المستأجرة"; +"rail.ride_bike_to_fmt" = "اركب إلى %@"; +"rail.drop_off_bike" = "أعد الدراجة"; +"rail.ride_distance_duration_fmt" = "%1$@، حوالي %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings index 4fbddae..6701fb9 100644 --- a/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings @@ -314,3 +314,14 @@ "rail.last_step" = "Last step"; "rail.arrive_time_fmt" = "Arrive %@"; "rail.now" = "now"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Bike to %@"; +"leg.ride_rental_bike_to" = "Ride rental bike to %@"; +"place.rental_bike" = "rental bike"; +"rail.now_riding_rental" = "Riding a rental bike"; +"rail.pick_up_bike" = "Pick up rental bike"; +"rail.picked_up_bike" = "Picked up rental bike"; +"rail.ride_bike_to_fmt" = "Ride to %@"; +"rail.drop_off_bike" = "Drop off bike"; +"rail.ride_distance_duration_fmt" = "%1$@, about %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings index afe5199..c9f623b 100644 --- a/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/es.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Último paso"; "rail.arrive_time_fmt" = "Llegada %@"; "rail.now" = "ahora"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Ve en bici a %@"; +"leg.ride_rental_bike_to" = "Monta la bici compartida hasta %@"; +"place.rental_bike" = "bici compartida"; +"rail.now_riding_rental" = "Montando una bici compartida"; +"rail.pick_up_bike" = "Recoge la bici compartida"; +"rail.picked_up_bike" = "Bici compartida recogida"; +"rail.ride_bike_to_fmt" = "Monta hasta %@"; +"rail.drop_off_bike" = "Deja la bici"; +"rail.ride_distance_duration_fmt" = "%1$@, unos %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings index 02e9a9d..f26adb9 100644 --- a/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/fil.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Huling hakbang"; "rail.arrive_time_fmt" = "Dating %@"; "rail.now" = "ngayon"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Magbisikleta papuntang %@"; +"leg.ride_rental_bike_to" = "Sakyan ang rentahang bisikleta papuntang %@"; +"place.rental_bike" = "rentahang bisikleta"; +"rail.now_riding_rental" = "Nakasakay sa rentahang bisikleta"; +"rail.pick_up_bike" = "Kunin ang rentahang bisikleta"; +"rail.picked_up_bike" = "Nakuha na ang rentahang bisikleta"; +"rail.ride_bike_to_fmt" = "Sumakay papuntang %@"; +"rail.drop_off_bike" = "Ibalik ang bisikleta"; +"rail.ride_distance_duration_fmt" = "%1$@, mga %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings index 5476bd8..47a46ac 100644 --- a/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/fr.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Dernière étape"; "rail.arrive_time_fmt" = "Arrivée %@"; "rail.now" = "maintenant"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Allez à vélo jusqu'à %@"; +"leg.ride_rental_bike_to" = "Roulez en vélo en libre-service jusqu'à %@"; +"place.rental_bike" = "vélo en libre-service"; +"rail.now_riding_rental" = "En vélo en libre-service"; +"rail.pick_up_bike" = "Prenez le vélo en libre-service"; +"rail.picked_up_bike" = "Vélo en libre-service pris"; +"rail.ride_bike_to_fmt" = "Roulez jusqu'à %@"; +"rail.drop_off_bike" = "Déposez le vélo"; +"rail.ride_distance_duration_fmt" = "%1$@, environ %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings index 7a59bdc..4e62978 100644 --- a/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/it.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Ultimo passaggio"; "rail.arrive_time_fmt" = "Arrivo alle %@"; "rail.now" = "ora"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Vai in bici fino a %@"; +"leg.ride_rental_bike_to" = "Pedala con la bici in sharing fino a %@"; +"place.rental_bike" = "bici in sharing"; +"rail.now_riding_rental" = "In sella a una bici in sharing"; +"rail.pick_up_bike" = "Prendi la bici in sharing"; +"rail.picked_up_bike" = "Bici in sharing presa"; +"rail.ride_bike_to_fmt" = "Pedala fino a %@"; +"rail.drop_off_bike" = "Riconsegna la bici"; +"rail.ride_distance_duration_fmt" = "%1$@, circa %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings index 4b8c922..d384ee8 100644 --- a/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/ko.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "마지막 단계"; "rail.arrive_time_fmt" = "도착 %@"; "rail.now" = "지금"; + +/* Vehicle rental legs */ +"leg.bike_to" = "%@까지 자전거 타기"; +"leg.ride_rental_bike_to" = "공유 자전거로 %@까지 이동"; +"place.rental_bike" = "공유 자전거"; +"rail.now_riding_rental" = "공유 자전거 탑승 중"; +"rail.pick_up_bike" = "공유 자전거 픽업"; +"rail.picked_up_bike" = "공유 자전거 픽업 완료"; +"rail.ride_bike_to_fmt" = "%@까지 타고 가기"; +"rail.drop_off_bike" = "자전거 반납"; +"rail.ride_distance_duration_fmt" = "%1$@, 약 %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings index c136e97..b83b602 100644 --- a/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/pl.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Ostatni krok"; "rail.arrive_time_fmt" = "Przyjazd %@"; "rail.now" = "teraz"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Jedź rowerem do %@"; +"leg.ride_rental_bike_to" = "Jedź rowerem miejskim do %@"; +"place.rental_bike" = "rower miejski"; +"rail.now_riding_rental" = "Jedziesz rowerem miejskim"; +"rail.pick_up_bike" = "Odbierz rower miejski"; +"rail.picked_up_bike" = "Rower miejski odebrany"; +"rail.ride_bike_to_fmt" = "Jedź do %@"; +"rail.drop_off_bike" = "Zostaw rower"; +"rail.ride_distance_duration_fmt" = "%1$@, około %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings index f63ce07..8e9acf6 100644 --- a/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/pt-BR.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Última etapa"; "rail.arrive_time_fmt" = "Chegada %@"; "rail.now" = "agora"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Vá de bicicleta até %@"; +"leg.ride_rental_bike_to" = "Pedale a bicicleta compartilhada até %@"; +"place.rental_bike" = "bicicleta compartilhada"; +"rail.now_riding_rental" = "Pedalando uma bicicleta compartilhada"; +"rail.pick_up_bike" = "Pegue a bicicleta compartilhada"; +"rail.picked_up_bike" = "Bicicleta compartilhada retirada"; +"rail.ride_bike_to_fmt" = "Pedale até %@"; +"rail.drop_off_bike" = "Devolva a bicicleta"; +"rail.ride_distance_duration_fmt" = "%1$@, cerca de %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings index 0e3b77b..afc1fb2 100644 --- a/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/ru.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Последний шаг"; "rail.arrive_time_fmt" = "Прибытие в %@"; "rail.now" = "сейчас"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Поезжайте на велосипеде до %@"; +"leg.ride_rental_bike_to" = "Поезжайте на прокатном велосипеде до %@"; +"place.rental_bike" = "прокатный велосипед"; +"rail.now_riding_rental" = "Едете на прокатном велосипеде"; +"rail.pick_up_bike" = "Возьмите прокатный велосипед"; +"rail.picked_up_bike" = "Прокатный велосипед взят"; +"rail.ride_bike_to_fmt" = "Поезжайте до %@"; +"rail.drop_off_bike" = "Верните велосипед"; +"rail.ride_distance_duration_fmt" = "%1$@, около %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings index 806087f..afa2038 100644 --- a/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/vi.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "Bước cuối"; "rail.arrive_time_fmt" = "Đến lúc %@"; "rail.now" = "bây giờ"; + +/* Vehicle rental legs */ +"leg.bike_to" = "Đạp xe đến %@"; +"leg.ride_rental_bike_to" = "Đi xe đạp thuê đến %@"; +"place.rental_bike" = "xe đạp thuê"; +"rail.now_riding_rental" = "Đang đi xe đạp thuê"; +"rail.pick_up_bike" = "Nhận xe đạp thuê"; +"rail.picked_up_bike" = "Đã nhận xe đạp thuê"; +"rail.ride_bike_to_fmt" = "Đi đến %@"; +"rail.drop_off_bike" = "Trả xe đạp"; +"rail.ride_distance_duration_fmt" = "%1$@, khoảng %2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings index c4f4bfa..fca2109 100644 --- a/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/zh-Hans.lproj/Localizable.strings @@ -312,3 +312,14 @@ "rail.last_step" = "最后一步"; "rail.arrive_time_fmt" = "%@ 到达"; "rail.now" = "现在"; + +/* Vehicle rental legs */ +"leg.bike_to" = "骑车前往%@"; +"leg.ride_rental_bike_to" = "骑共享单车前往%@"; +"place.rental_bike" = "共享单车"; +"rail.now_riding_rental" = "正在骑共享单车"; +"rail.pick_up_bike" = "取用共享单车"; +"rail.picked_up_bike" = "已取用共享单车"; +"rail.ride_bike_to_fmt" = "骑行前往%@"; +"rail.drop_off_bike" = "归还单车"; +"rail.ride_distance_duration_fmt" = "%1$@，约%2$@"; diff --git a/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings index 36e68f6..a9a290c 100644 --- a/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/zh-Hant.lproj/Localizable.strings @@ -313,3 +313,14 @@ "rail.last_step" = "最後一步"; "rail.arrive_time_fmt" = "抵達 %@"; "rail.now" = "現在"; + +/* Vehicle rental legs */ +"leg.bike_to" = "騎車前往%@"; +"leg.ride_rental_bike_to" = "騎共享單車前往%@"; +"place.rental_bike" = "共享單車"; +"rail.now_riding_rental" = "正在騎共享單車"; +"rail.pick_up_bike" = "取用共享單車"; +"rail.picked_up_bike" = "已取用共享單車"; +"rail.ride_bike_to_fmt" = "騎行前往%@"; +"rail.drop_off_bike" = "歸還單車"; +"rail.ride_distance_duration_fmt" = "%1$@，約%2$@"; diff --git a/OTPKit/Tests/TripProgressTests.swift b/OTPKit/Tests/TripProgressTests.swift index 750437b..592f9b1 100644 --- a/OTPKit/Tests/TripProgressTests.swift +++ b/OTPKit/Tests/TripProgressTests.swift @@ -289,4 +289,121 @@ struct TripProgressTests { #expect(leg.departureStatus == .late(minutes: 7)) #expect(leg.arrivalStatus == .late(minutes: 6)) } + + // MARK: - Rental legs + + /// Walk 3m to the vehicle → ride the rental 10m → walk 2m to the destination. + func makeRentalItinerary() -> Itinerary { + let walkEnd = tripStart.addingTimeInterval(180) + let rideEnd = walkEnd.addingTimeInterval(600) + let arrivalTime = rideEnd.addingTimeInterval(120) + + let vehiclePlace = Place(name: "Default vehicle type", lon: -122.34, lat: 47.615, + vertexType: "BIKESHARE", bikeShareId: "lime_seattle:abc") + let dropoffPlace = Place(name: "3rd Ave & Pine St", lon: -122.33, lat: 47.61, + vertexType: "BIKESHARE", bikeShareId: "pronto:BT-01") + + let walkLeg = Leg( + startTime: tripStart, endTime: walkEnd, mode: "WALK", + routeType: nil, routeColor: nil, routeTextColor: nil, route: nil, agencyName: nil, + from: Place(name: "Home", lon: -122.35, lat: 47.62, vertexType: "NORMAL"), + to: vehiclePlace, + legGeometry: LegGeometry(points: "AA@@", length: 4), + distance: 250, transitLeg: false, duration: 180, realTime: nil, + streetNames: nil, pathway: nil, steps: nil, headsign: nil, intermediateStops: nil, + rentedBike: false + ) + + let rideLeg = Leg( + startTime: walkEnd, endTime: rideEnd, mode: "BICYCLE", + routeType: nil, routeColor: nil, routeTextColor: nil, route: nil, agencyName: nil, + from: vehiclePlace, to: dropoffPlace, + legGeometry: LegGeometry(points: "AA@@", length: 4), + distance: 2100, transitLeg: false, duration: 600, realTime: nil, + streetNames: nil, pathway: nil, steps: nil, headsign: nil, intermediateStops: nil, + rentedBike: true + ) + + let finalWalkLeg = Leg( + startTime: rideEnd, endTime: arrivalTime, mode: "WALK", + routeType: nil, routeColor: nil, routeTextColor: nil, route: nil, agencyName: nil, + from: dropoffPlace, + to: Place(name: "Pike Place Market", lon: -122.34, lat: 47.609, vertexType: "NORMAL"), + legGeometry: LegGeometry(points: "AA@@", length: 4), + distance: 140, transitLeg: false, duration: 120, realTime: nil, + streetNames: nil, pathway: nil, steps: nil, headsign: nil, intermediateStops: nil, + rentedBike: false + ) + + return Itinerary( + duration: Int(arrivalTime.timeIntervalSince(tripStart)), + startTime: tripStart, + endTime: arrivalTime, + walkTime: 300, transitTime: 0, waitingTime: 0, + walkDistance: 390, walkLimitExceeded: false, + elevationLost: 0, elevationGained: 0, + transfers: 0, + legs: [walkLeg, rideLeg, finalWalkLeg] + ) + } + + func rentalProgress(at offset: TimeInterval) -> TripProgress { + TripProgress(itinerary: makeRentalItinerary(), now: tripStart.addingTimeInterval(offset)) + } + + @Test func rentalRideCountsAsRiding() { + // 180s..780s is the ride window. + #expect(rentalProgress(at: 400).phase == .riding(legIndex: 1)) + } + + @Test func rentalLegProducesPickupRideAndDropoffRows() { + let rows = rentalProgress(at: 400).rows + let kinds = rows.map(\.kind) + + #expect(kinds.contains(.pickUpVehicle(legIndex: 1))) + #expect(kinds.contains(.rideRental(legIndex: 1))) + #expect(kinds.contains(.dropOffVehicle(legIndex: 1))) + + // Exactly one current row: the synthetic riding row. + let currentRows = rows.filter { $0.state == .current } + #expect(currentRows.map(\.kind) == [.rideRental(legIndex: 1)]) + } + + @Test func rentalRideRowAbsentWhenNotRiding() { + let kinds = rentalProgress(at: 60).rows.map(\.kind) + #expect(!kinds.contains(.rideRental(legIndex: 1))) + #expect(kinds.contains(.pickUpVehicle(legIndex: 1))) + } + + @Test func rentalActivityNameIsRentalSpecific() { + let name = rentalProgress(at: 400).localizedActivityName + #expect(name == OTPLoc("rail.now_riding_rental", comment: "")) + } + + @Test func rentalSegmentIsMarkedRental() { + let segments = rentalProgress(at: 400).segments + #expect(segments[1].isRental) + #expect(!segments[1].isTransit) + #expect(!segments[0].isRental) + } + + @Test func stopsRemainingIsNilForRentalLegs() { + #expect(rentalProgress(at: 400).stopsRemaining(onLegAt: 1) == nil) + } + + @Test func placeholderVehicleNameNeverSurfaces() { + let itinerary = makeRentalItinerary() + // The walk leg ends at the vehicle, whose feed name is the placeholder. + #expect(itinerary.legs[0].riderFacingToName == OTPLoc("place.rental_bike", comment: "")) + // Real names pass through untouched. + #expect(itinerary.legs[1].riderFacingToName == "3rd Ave & Pine St") + } + + @Test func legsNeverMergeAcrossARentalBoundary() { + let itinerary = makeRentalItinerary() + // relevantLegs must keep the rental ride distinct from its neighbors. + #expect(itinerary.relevantLegs.filter(\.isRentalRide).count == 1) + #expect(!Leg.shouldMergeLegs(leg1: itinerary.legs[0], leg2: itinerary.legs[1])) + #expect(!Leg.shouldMergeLegs(leg1: itinerary.legs[1], leg2: itinerary.legs[2])) + } } From e4d12ec42a38ecd5c5dd6920a522dc3198fc903c Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Wed, 29 Jul 2026 01:05:37 -0700 Subject: [PATCH 3/6] Make VehicleRentalService Sendable Services are shared across isolation domains by design: VehicleRentalSource (an actor) fetches through one while hosts hold it on the main actor. The app's Swift 6 region-isolation checking rejects handing a non-Sendable existential into the actor. Conformers are typically actors already. --- OTPKit/Sources/OTPKit/Network/VehicleRentalService.swift | 6 +++++- OTPKit/Tests/Helpers/TestFixtures.swift | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/OTPKit/Sources/OTPKit/Network/VehicleRentalService.swift b/OTPKit/Sources/OTPKit/Network/VehicleRentalService.swift index 755766c..7473512 100644 --- a/OTPKit/Sources/OTPKit/Network/VehicleRentalService.swift +++ b/OTPKit/Sources/OTPKit/Network/VehicleRentalService.swift @@ -22,7 +22,11 @@ import Foundation /// `RestAPIService` (OTP 1.x) does not. Conformance doubles as the capability /// flag: hosts check `apiService is VehicleRentalService` to decide whether /// rental features can work at all. -public protocol VehicleRentalService { +/// +/// `Sendable` because services are shared across isolation domains by design: +/// `VehicleRentalSource` (an actor) fetches through one while hosts hold it on +/// the main actor. Conformers are typically actors already. +public protocol VehicleRentalService: Sendable { /// Fetches rental stations and free-floating vehicles in a bounding box. /// /// - Parameters: diff --git a/OTPKit/Tests/Helpers/TestFixtures.swift b/OTPKit/Tests/Helpers/TestFixtures.swift index 766cb64..af093a3 100644 --- a/OTPKit/Tests/Helpers/TestFixtures.swift +++ b/OTPKit/Tests/Helpers/TestFixtures.swift @@ -65,7 +65,8 @@ enum TestFixtures { /// A mock APIService that also advertises vehicle rental capability, for testing /// capability-gated behavior like `availableTransportModes`. - class MockRentalAPIService: MockAPIService, VehicleRentalService { + // @unchecked: single-threaded test usage; VehicleRentalService requires Sendable. + final class MockRentalAPIService: MockAPIService, VehicleRentalService, @unchecked Sendable { var mockRentals: [VehicleRental] = [] func fetchVehicleRentals( From 0d0d7785875b004bb9f707e562a88c257e5a33b3 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Wed, 29 Jul 2026 02:15:34 -0700 Subject: [PATCH 4/6] Harden the rental stack after simplify and review passes Simplify: - One placeholder-name predicate (String.isRentalPlaceholderName) behind the three call sites; MapCoordinator uses isRentalRide and riderFacingName; rail row builders and mode icons deduplicated - VehicleRentalSource: identical viewports no longer refetch; the debounce task holds self weakly so an abandoned source deallocates immediately Review fixes: - planQuery only declares/passes via when the request has a via point: GraphQL validates documents statically and the via argument only exists on OTP 2.7+, so declaring it unconditionally broke every plan request against older 2.x servers - wireTransportModes expands composite modes at the serialization boundary in both services, so .transitBikeRental's fabricated raw value can never leak into a request - A failed fetch forgets its viewport so a stationary map retries on the next (identical) region emission instead of staying dimmed forever - fetchFailures buffers only the newest failure (advisory stream; never grows unbounded when unconsumed) - Changing transport mode clears a stale via point; the prefilled planner entry is all-or-nothing and no longer resets state when re-invoked - A gap before a rental leg is now .waiting at the pickup (current pickup row, correct tip copy) instead of a bogus walking phase with no current row New regression tests for each fix; full suite green. --- .../Core/Extensions/StringExtension.swift | 8 ++ .../OTPKit/Core/Map/MapCoordinator.swift | 24 ++--- .../Sources/OTPKit/Core/Models/OTP/Leg.swift | 3 +- .../Core/Models/OTP/TransportMode.swift | 8 ++ .../Core/Models/OTP/TripPlanRequest.swift | 12 ++- .../Core/Models/OTP/VehicleRental.swift | 6 +- .../Core/TripProgress/TripProgress.swift | 95 ++++++++----------- .../OTPKit/Network/GraphQLAPIService.swift | 26 +++-- .../OTPKit/Network/VehicleRentalSource.swift | 42 +++++--- .../Directions/Rail/InTripRailView.swift | 8 +- .../Directions/Rail/RailRowContentViews.swift | 6 +- .../Directions/Rail/TipContentView.swift | 36 ++++--- .../OTPKit/Presentation/TripPlanner.swift | 16 +++- .../ViewModel/TripPlannerViewModel.swift | 6 ++ OTPKit/Tests/GraphQLAPIServiceTests.swift | 23 +++++ OTPKit/Tests/TripPlanRequestTests.swift | 15 +++ OTPKit/Tests/TripProgressTests.swift | 27 +++++- OTPKit/Tests/VehicleRentalSourceTests.swift | 58 ++++++++++- 18 files changed, 283 insertions(+), 136 deletions(-) diff --git a/OTPKit/Sources/OTPKit/Core/Extensions/StringExtension.swift b/OTPKit/Sources/OTPKit/Core/Extensions/StringExtension.swift index fee39f4..53d8f5f 100644 --- a/OTPKit/Sources/OTPKit/Core/Extensions/StringExtension.swift +++ b/OTPKit/Sources/OTPKit/Core/Extensions/StringExtension.swift @@ -20,6 +20,14 @@ extension String { isEmpty ? self : prefix(1).uppercased() + dropFirst() } + /// True when this is a known rental-feed placeholder name ("Default vehicle type") + /// that must never reach the UI. The single source of truth for the check — + /// `VehicleRental.displayLabel`, `Leg.riderFacingName(of:)`, and the rail views + /// all decide their own substitution policy on top of this one predicate. + var isRentalPlaceholderName: Bool { + trimmingCharacters(in: .whitespacesAndNewlines).lowercased() == "default vehicle type" + } + /// Renders an unrecognized OTP token as readable text: `SPIN_AROUND` becomes `Spin Around`. /// /// Last-resort display fallback for a mode or direction this client doesn't know about. diff --git a/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift b/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift index 8375451..ce4dcbc 100644 --- a/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift +++ b/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift @@ -300,7 +300,7 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b private func addStationAnnotations(for leg: Leg, index: Int, totalLegs: Int) { // Rental legs get pickup/dropoff markers; transit legs get embark/debark markers. - if leg.rentedBike == true { + if leg.isRentalRide { addRentalAnnotations(for: leg, index: index) return } @@ -342,25 +342,13 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b /// checks never match them. Only vehicles on the planned route are annotated — the /// browse layer is a separate surface owned by the host. private func addRentalAnnotations(for leg: Leg, index: Int) { - if leg.from.bikeShareId != nil { + let endpoints = [(leg.from, "rental_pickup_\(index)"), (leg.to, "rental_dropoff_\(index)")] + for (place, identifier) in endpoints where place.bikeShareId != nil { mapProvider.addAnnotation( - coordinate: CLLocationCoordinate2D(latitude: leg.from.lat, longitude: leg.from.lon), - title: leg.from.name, - subtitle: nil, - identifier: "rental_pickup_\(index)", - type: .rentalVehicle, - routeName: nil, - routeBackgroundColor: nil, - routeTextColor: nil - ) - } - - if leg.to.bikeShareId != nil { - mapProvider.addAnnotation( - coordinate: CLLocationCoordinate2D(latitude: leg.to.lat, longitude: leg.to.lon), - title: leg.to.name, + coordinate: CLLocationCoordinate2D(latitude: place.lat, longitude: place.lon), + title: Leg.riderFacingName(of: place), subtitle: nil, - identifier: "rental_dropoff_\(index)", + identifier: identifier, type: .rentalVehicle, routeName: nil, routeBackgroundColor: nil, diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift index 349572e..7fd7230 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/Leg.swift @@ -285,8 +285,7 @@ public struct Leg: Codable, Hashable { static func riderFacingName(of place: Place) -> String { let trimmed = place.name.trimmingCharacters(in: .whitespacesAndNewlines) - let isPlaceholder = trimmed.lowercased() == "default vehicle type" - if isPlaceholder || (trimmed.isEmpty && place.bikeShareId != nil) { + if trimmed.isRentalPlaceholderName || (trimmed.isEmpty && place.bikeShareId != nil) { return OTPLoc("place.rental_bike", comment: "Generic name for a rental bike location") } return trimmed diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift index 5b10c66..56c1a57 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/TransportMode.swift @@ -86,4 +86,12 @@ public enum TransportMode: String, CaseIterable, Codable { public var requiresVehicleRentalSupport: Bool { self == .bikeRental || self == .transitBikeRental } + + /// The primitive modes this mode puts on the wire. Composite UI modes + /// (`.transitBikeRental`) expand to their `apiModes`; primitives are themselves. + /// Both services serialize through this, so a composite's fabricated raw value + /// can never leak into a request — no matter how the host built it. + public var wireModes: [TransportMode] { + self == .transitBikeRental ? apiModes : [self] + } } diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift index 24db4c8..984e6a6 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/TripPlanRequest.swift @@ -73,9 +73,17 @@ public struct TripPlanRequest: Codable, Hashable { self.viaPoint = viaPoint } - /// Converts the transport modes to the API string format + /// Converts the transport modes to the OTP 1.x REST `mode` parameter: wire tokens, + /// comma-joined. Composite UI modes expand to their primitives, deduplicated in + /// first-appearance order. public var transportModesString: String { - transportModes.map { $0.rawValue }.joined(separator: ",") + wireTransportModes.map { $0.rawValue }.joined(separator: ",") + } + + /// The primitive, deduplicated modes requests actually serialize. + public var wireTransportModes: [TransportMode] { + var seen = Set() + return transportModes.flatMap(\.wireModes).filter { seen.insert($0).inserted } } /// Validates the request parameters diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRental.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRental.swift index f9c06b5..544e8cd 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRental.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/VehicleRental.swift @@ -105,7 +105,7 @@ public enum VehicleRental: Identifiable, Hashable, Sendable { } let trimmedName = vehicle.name.trimmingCharacters(in: .whitespacesAndNewlines) - if !trimmedName.isEmpty, !Self.isPlaceholderName(trimmedName) { + if !trimmedName.isEmpty, !trimmedName.isRentalPlaceholderName { return trimmedName } @@ -113,10 +113,6 @@ public enum VehicleRental: Identifiable, Hashable, Sendable { } } - private static func isPlaceholderName(_ name: String) -> Bool { - name.lowercased() == "default vehicle type" - } - private static func localizedTypeName(for vehicleType: VehicleType?) -> String { guard let vehicleType, let formFactor = vehicleType.formFactor else { return OTPLoc("rental.vehicle_type.vehicle", comment: "Generic rental vehicle type name") diff --git a/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift b/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift index 2f016a5..83d1077 100644 --- a/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift +++ b/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift @@ -141,11 +141,13 @@ public struct TripProgress { return isAboard ? .riding(legIndex: index) : .walking(legIndex: index) } - // In the gap between this leg and the next: waiting if the next leg - // is transit (the rider is at the stop), otherwise treat the gap as - // part of the upcoming walk. + // In the gap between this leg and the next: waiting if the next leg is + // transit (the rider is at the stop) or a rental pickup (the rider is + // walking up to the vehicle — sub-minute walks get merged away, leaving + // a real gap). Otherwise the gap is part of the upcoming walk. if now < leg.startTime { - return leg.transitLeg == true ? .waiting(boardingLegIndex: index) : .walking(legIndex: index) + let isBoardable = leg.transitLeg == true || leg.isRentalRide + return isBoardable ? .waiting(boardingLegIndex: index) : .walking(legIndex: index) } } @@ -162,38 +164,18 @@ public struct TripProgress { for (index, leg) in legs.enumerated() { if leg.transitLeg == true { rows.append(boardRow(for: leg, at: index, phase: phase)) - - if case .riding(let ridingIndex) = phase, ridingIndex == index { - rows.append( - RailRow( - id: "ride-\(index)", - kind: .ride(legIndex: index), - state: .current, - time: now, - status: nil - ) - ) + if isRiding(index, phase: phase) { + rows.append(currentRideRow(id: "ride-\(index)", kind: .ride(legIndex: index))) } - - rows.append(getOffRow(for: leg, at: index)) + rows.append(legEndRow(id: "getoff-\(index)", kind: .getOff(legIndex: index), for: leg)) } else if leg.isRentalRide { // Rental legs get pickup → ride → dropoff semantics, mirroring the // transit board → ride → get off shape so the rail reads uniformly. - rows.append(pickUpVehicleRow(for: leg, at: index)) - - if case .riding(let ridingIndex) = phase, ridingIndex == index { - rows.append( - RailRow( - id: "riderental-\(index)", - kind: .rideRental(legIndex: index), - state: .current, - time: now, - status: nil - ) - ) + rows.append(pickUpVehicleRow(for: leg, at: index, phase: phase)) + if isRiding(index, phase: phase) { + rows.append(currentRideRow(id: "riderental-\(index)", kind: .rideRental(legIndex: index))) } - - rows.append(dropOffVehicleRow(for: leg, at: index)) + rows.append(legEndRow(id: "dropoff-\(index)", kind: .dropOffVehicle(legIndex: index), for: leg)) } else { rows.append(walkRow(for: leg, at: index, phase: phase)) } @@ -241,37 +223,39 @@ public struct TripProgress { ) } - private func getOffRow(for leg: Leg, at index: Int) -> RailRow { - RailRow( - id: "getoff-\(index)", - kind: .getOff(legIndex: index), - state: now >= leg.endTime ? .done : .upcoming, - time: leg.endTime, - status: nil - ) - } + /// The pickup row goes current during a `.waiting` gap before the ride — the + /// stretch where the rider is walking up to the parked vehicle. During the ride + /// itself the current row is the synthetic `rideRental` row. + private func pickUpVehicleRow(for leg: Leg, at index: Int, phase: TripPhase) -> RailRow { + let state: RailRow.State + if case .waiting(let boardingIndex) = phase, boardingIndex == index { + state = .current + } else { + state = now >= leg.startTime ? .done : .upcoming + } - /// Unlike a transit boarding, a pickup is never "current": there is no waiting - /// phase for a parked vehicle — the rider walks up and rides. The current row - /// during the ride is the synthetic `rideRental` row. - private func pickUpVehicleRow(for leg: Leg, at index: Int) -> RailRow { - RailRow( + return RailRow( id: "pickup-\(index)", kind: .pickUpVehicle(legIndex: index), - state: now >= leg.startTime ? .done : .upcoming, + state: state, time: leg.startTime, status: nil ) } - private func dropOffVehicleRow(for leg: Leg, at index: Int) -> RailRow { - RailRow( - id: "dropoff-\(index)", - kind: .dropOffVehicle(legIndex: index), - state: now >= leg.endTime ? .done : .upcoming, - time: leg.endTime, - status: nil - ) + /// The synthetic row present only while the rider is aboard this leg. + private func currentRideRow(id: String, kind: RailRow.Kind) -> RailRow { + RailRow(id: id, kind: kind, state: .current, time: now, status: nil) + } + + /// A leg's final moment: get off transit, or drop off the rental vehicle. + private func legEndRow(id: String, kind: RailRow.Kind, for leg: Leg) -> RailRow { + RailRow(id: id, kind: kind, state: now >= leg.endTime ? .done : .upcoming, time: leg.endTime, status: nil) + } + + private func isRiding(_ index: Int, phase: TripPhase) -> Bool { + if case .riding(let ridingIndex) = phase { return ridingIndex == index } + return false } /// The rider's current activity, localized ("Walking", "Waiting for the C Line", @@ -283,6 +267,9 @@ public struct TripProgress { case .walking: return OTPLoc("rail.now_walking", comment: "The rider is currently walking") case .waiting(let index): + if legs[index].isRentalRide { + return OTPLoc("rail.pick_up_bike", comment: "Instruction to pick up the rental bike") + } return OTPLoc("rail.now_waiting_fmt", comment: "The rider is waiting for this route", legs[index].riderFacingRouteName) diff --git a/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift b/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift index 27ec112..78e2b70 100644 --- a/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift +++ b/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift @@ -42,7 +42,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { /// Fetches a trip plan using a `TripPlanRequest` public func fetchPlan(_ request: TripPlanRequest) async throws -> OTPResponse { let urlRequest = try makeGraphQLRequest( - query: Self.planQuery, + query: Self.planQuery(includingVia: request.viaPoint != nil), variables: Self.planVariables(for: request) ) @@ -145,7 +145,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { "to": ["lat": request.destination.latitude, "lon": request.destination.longitude], "date": request.date.formattedTripDate, "time": request.time.formattedTripTime, - "transportModes": request.transportModes.map { graphQLTransportMode(for: $0) }, + "transportModes": request.wireTransportModes.map { graphQLTransportMode(for: $0) }, "arriveBy": request.arriveBy, "wheelchair": request.wheelchairAccessible, "maxWalkDistance": Double(request.maxWalkDistance) @@ -172,8 +172,8 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { case .bikeRental: return ["mode": "BICYCLE", "qualifier": "RENT"] case .transitBikeRental: - // Unreachable in practice: composite UI modes reach requests expanded - // through `apiModes`, never as themselves. + // Unreachable: `wireTransportModes` expands composites before this + // mapping ever runs; the arm exists only for switch exhaustiveness. return ["mode": "TRANSIT"] case .transit, .walk, .car: return ["mode": mode.rawValue] @@ -217,7 +217,16 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { /// The GTFS GraphQL API `plan` query. Requests only the fields OTPKit's models consume, /// mirroring what the OTP 1.x REST API returns. - static let planQuery = """ + /// + /// The `via` argument only exists in the query document when the request actually + /// carries a via point: GraphQL validates documents statically, and `plan`'s `via` + /// argument (with its `PlanViaLocationInput` type) only exists on OTP 2.7+ — a + /// document that always declared it would break every plan request, transit + /// included, against older 2.x servers. + static func planQuery(includingVia: Bool) -> String { + let viaDeclaration = includingVia ? "\n $via: [PlanViaLocationInput!]" : "" + let viaArgument = includingVia ? "\n via: $via" : "" + return """ query TripPlan( $from: InputCoordinates! $to: InputCoordinates! @@ -226,8 +235,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { $transportModes: [TransportMode!] $arriveBy: Boolean $wheelchair: Boolean - $maxWalkDistance: Float - $via: [PlanViaLocationInput!] + $maxWalkDistance: Float\(viaDeclaration) ) { plan( from: $from @@ -237,8 +245,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { transportModes: $transportModes arriveBy: $arriveBy wheelchair: $wheelchair - maxWalkDistance: $maxWalkDistance - via: $via + maxWalkDistance: $maxWalkDistance\(viaArgument) ) { date from { name lon lat vertexType } @@ -291,6 +298,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { } } """ + } /// The GTFS GraphQL API `vehicleRentalsByBbox` query. Returns the `RentalPlace` /// union; `__typename` discriminates stations from free-floating vehicles. diff --git a/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift b/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift index 627257e..c830946 100644 --- a/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift +++ b/OTPKit/Sources/OTPKit/Network/VehicleRentalSource.swift @@ -83,7 +83,13 @@ public actor VehicleRentalSource { self.boundingBoxPadding = boundingBoxPadding (snapshots, snapshotContinuation) = AsyncStream.makeStream(of: VehicleRentalSnapshot.self) - (fetchFailures, failureContinuation) = AsyncStream.makeStream(of: FetchFailure.self) + // Failures are advisory and hosts only need the latest; bounding the buffer + // means a host that never consumes this stream can't accumulate errors + // without bound against a persistently failing endpoint. + (fetchFailures, failureContinuation) = AsyncStream.makeStream( + of: FetchFailure.self, + bufferingPolicy: .bufferingNewest(1) + ) } deinit { @@ -99,9 +105,12 @@ public actor VehicleRentalSource { /// removing everything. public func setViewport(_ boundingBox: VehicleRentalBoundingBox?) { guard let boundingBox else { - clear() + reset() return } + // Map frameworks re-emit identical regions on layout passes; an unchanged + // viewport must not cancel and refetch a multi-thousand-entity payload. + guard boundingBox != viewport else { return } viewport = boundingBox scheduleFetch() } @@ -116,15 +125,9 @@ public actor VehicleRentalSource { } } - /// Clears all state (e.g. the layer was switched off) and emits a snapshot - /// removing everything previously delivered. + /// Clears all state (e.g. the layer was switched off, or the zoom gate + /// closed) and emits a snapshot removing everything previously delivered. public func reset() { - clear() - } - - // MARK: - Pipeline - - private func clear() { pendingFetch?.cancel() pendingFetch = nil generation += 1 @@ -140,18 +143,23 @@ public actor VehicleRentalSource { )) } + // MARK: - Pipeline + private func scheduleFetch() { pendingFetch?.cancel() generation += 1 let scheduledGeneration = generation let interval = coalescingInterval - // The task inherits the actor's isolation: the sleep and the fetch suspend - // without blocking other actor work (fetchPlan on the same service is - // unaffected — decode already runs off-actor in GraphQLAPIService). - pendingFetch = Task { + // Weak so an abandoned source deallocates immediately instead of being + // kept alive through the debounce plus a multi-thousand-entity fetch + // whose snapshot nobody would consume. The body otherwise inherits the + // actor's isolation: the sleep and fetch suspend without blocking other + // actor work (fetchPlan on the same service is unaffected — decode + // already runs off-actor in GraphQLAPIService). + pendingFetch = Task { [weak self] in try? await Task.sleep(for: interval) - guard !Task.isCancelled else { return } + guard !Task.isCancelled, let self else { return } await self.performFetch(generation: scheduledGeneration) } } @@ -168,6 +176,10 @@ public actor VehicleRentalSource { // Superseded by a newer viewport; the newer fetch reports instead. } catch { guard scheduledGeneration == generation else { return } + // Forget the failed viewport so the next region emission — identical or + // not — retries instead of being swallowed by the same-viewport guard. + // A stationary map must be able to heal from a transient failure. + self.viewport = nil failureContinuation.yield(FetchFailure(underlyingError: error, occurredAt: Date())) } } diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift index f6f8ed3..6fe8d28 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/InTripRailView.swift @@ -115,7 +115,7 @@ struct InTripRailView: View { PickUpVehicleRowContent( leg: progress.legs[index], state: row.state, - isExpanded: isFocused + isExpanded: row.state == .current || isFocused ) case .rideRental(let index): RideRentalRowContent(progress: progress, legIndex: index) @@ -165,8 +165,10 @@ struct InTripRailView: View { return .thin } return .bar(progress.legs[index].routeUIColor ?? Color(.systemGray2)) - case .pickUpVehicle(let index), .rideRental(let index): - if row.state == .done, case .pickUpVehicle = row.kind, !isRiding(index) { + case .rideRental: + return .bar(.otpRentalPurple) + case .pickUpVehicle(let index): + if row.state == .done, !isRiding(index) { return .thin } return .bar(.otpRentalPurple) diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift index d37b810..4de3fb5 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift @@ -50,10 +50,12 @@ enum RailText { } /// The rider-facing name of a rental place, or nil when the feed sent a known - /// placeholder ("Default vehicle type") that must never reach the UI. + /// placeholder that must never reach the UI. Same predicate as + /// `Leg.riderFacingName(of:)`, different policy: these rows hide the line + /// instead of substituting a generic. static func rentalPlaceName(_ name: String) -> String? { let trimmed = name.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty, trimmed.lowercased() != "default vehicle type" else { return nil } + guard !trimmed.isEmpty, !trimmed.isRentalPlaceholderName else { return nil } return trimmed } diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift index 4555a9c..ad87e22 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/TipContentView.swift @@ -109,12 +109,21 @@ struct TipContentView: View { case .waiting(let index): let leg = progress.legs[index] - HStack(alignment: .top, spacing: 12) { - RouteBadge(leg: leg) - title(RailText.boardingCountdown(for: leg, now: progress.now), - subtitle: OTPLoc("rail.at_stop_fmt", - comment: "The stop the rider boards at", leg.from.name)) - trailingTime(leg.startTime, status: leg.departureStatus) + if leg.isRentalRide { + HStack(alignment: .top, spacing: 12) { + bikeIcon + title(OTPLoc("rail.pick_up_bike", comment: "Instruction to pick up the rental bike"), + subtitle: RailText.rentalPlaceName(leg.from.name)) + trailingTime(leg.startTime, status: nil) + } + } else { + HStack(alignment: .top, spacing: 12) { + RouteBadge(leg: leg) + title(RailText.boardingCountdown(for: leg, now: progress.now), + subtitle: OTPLoc("rail.at_stop_fmt", + comment: "The stop the rider boards at", leg.from.name)) + trailingTime(leg.startTime, status: leg.departureStatus) + } } case .riding(let index): @@ -182,20 +191,19 @@ struct TipContentView: View { // MARK: - Pieces private var walkIcon: some View { - Image(systemName: "figure.walk") - .font(.subheadline.weight(.semibold)) - .foregroundStyle(.white) - .frame(width: 32, height: 32) - .background(Color(.label), in: RoundedRectangle(cornerRadius: 9)) - .accessibilityHidden(true) + modeIcon("figure.walk", background: Color(.label)) } private var bikeIcon: some View { - Image(systemName: "bicycle") + modeIcon("bicycle", background: .otpRentalPurple) + } + + private func modeIcon(_ systemName: String, background: Color) -> some View { + Image(systemName: systemName) .font(.subheadline.weight(.semibold)) .foregroundStyle(.white) .frame(width: 32, height: 32) - .background(Color.otpRentalPurple, in: RoundedRectangle(cornerRadius: 9)) + .background(background, in: RoundedRectangle(cornerRadius: 9)) .accessibilityHidden(true) } diff --git a/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift b/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift index 5afc224..92dd32b 100644 --- a/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift +++ b/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift @@ -82,9 +82,19 @@ public class TripPlanner { transportMode: TransportMode? = nil, onClose: @escaping VoidBlock ) -> some View { - viewModel.viaPoint = viaPoint - if let transportMode, viewModel.availableTransportModes.contains(transportMode) { - viewModel.selectTransportMode(transportMode) + // The prefill is all-or-nothing: a via point paired with an unsupported mode + // must not be applied alone, or the planner would route the rider through a + // rental vehicle's location in a mode that can't use it. Nil parameters + // leave existing state untouched, so re-invoking the factory is harmless. + let modeIsAvailable = transportMode.map { viewModel.availableTransportModes.contains($0) } ?? true + if modeIsAvailable { + if let transportMode { + viewModel.selectTransportMode(transportMode) + } + if let viaPoint { + // After the mode change: selecting a mode clears any stale via point. + viewModel.viaPoint = viaPoint + } } let view = TripPlannerView( diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift index 3565d43..164f9c4 100644 --- a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift @@ -186,6 +186,12 @@ public class TripPlannerViewModel: ObservableObject { /// Update the selected transport mode /// - Parameter mode: The transport mode to select func selectTransportMode(_ mode: TransportMode) { + // A via point exists to route through a specific rental vehicle. A rider who + // switches modes has abandoned that plan — keeping the via would silently + // force every later trip through a detour with no visible cause. + if mode != selectedTransportMode { + viaPoint = nil + } selectedTransportMode = mode } diff --git a/OTPKit/Tests/GraphQLAPIServiceTests.swift b/OTPKit/Tests/GraphQLAPIServiceTests.swift index 066b907..c5fb482 100644 --- a/OTPKit/Tests/GraphQLAPIServiceTests.swift +++ b/OTPKit/Tests/GraphQLAPIServiceTests.swift @@ -151,6 +151,29 @@ class GraphQLAPIServiceTests: OTPTestCase { let variables = try XCTUnwrap(payload["variables"] as? [String: Any]) // Absent, not null: not every OTP build treats a null via as "no via". XCTAssertNil(variables["via"]) + + // The query document itself must not mention via either: GraphQL validates + // documents statically, and `plan`'s via argument only exists on OTP 2.7+ — + // declaring it unconditionally would break every request against older servers. + let query = try XCTUnwrap(payload["query"] as? String) + XCTAssertFalse(query.contains("$via")) + XCTAssertFalse(query.contains("via:")) + } + + func testFetchPlanExpandsCompositeModeToWireModes() async throws { + mockDataLoader.mockResponse(data: Fixtures.loadData(file: "graphql_plan_success.json")) + + // A host passing the composite mode directly must get the expanded wire + // modes, never the fabricated TRANSIT_BICYCLE_RENT raw value. + _ = try await service.fetchPlan(createTripPlanRequest(transportModes: [.transitBikeRental])) + + let request = try XCTUnwrap(mockDataLoader.lastRequest) + let body = try XCTUnwrap(request.httpBody) + let payload = try XCTUnwrap(try JSONSerialization.jsonObject(with: body) as? [String: Any]) + let variables = try XCTUnwrap(payload["variables"] as? [String: Any]) + let modes = try XCTUnwrap(variables["transportModes"] as? [[String: Any]]) + XCTAssertEqual(modes.map { $0["mode"] as? String }, ["TRANSIT", "WALK", "BICYCLE"]) + XCTAssertEqual(modes[2]["qualifier"] as? String, "RENT") } // MARK: - Response Mapping diff --git a/OTPKit/Tests/TripPlanRequestTests.swift b/OTPKit/Tests/TripPlanRequestTests.swift index 1c8ca83..aed7b2e 100644 --- a/OTPKit/Tests/TripPlanRequestTests.swift +++ b/OTPKit/Tests/TripPlanRequestTests.swift @@ -107,6 +107,21 @@ struct TripPlanRequestTests { #expect(request.transportModesString == "TRANSIT,WALK,BICYCLE_RENT") } + @Test("A composite mode passed directly still serializes to primitive wire tokens") + func compositeModeExpandsAndDeduplicates() { + let request = TripPlanRequest( + origin: CLLocationCoordinate2D(latitude: 0, longitude: 0), + destination: CLLocationCoordinate2D(latitude: 1, longitude: 1), + date: Date(), + time: Date(), + transportModes: [.transitBikeRental, .walk] + ) + + // The fabricated TRANSIT_BICYCLE_RENT raw value must never reach the wire, + // and the duplicate walk collapses in first-appearance order. + #expect(request.transportModesString == "TRANSIT,WALK,BICYCLE_RENT") + } + @Test("viaPoint participates in equality") func viaPointEquality() { let base = TripPlanRequest( diff --git a/OTPKit/Tests/TripProgressTests.swift b/OTPKit/Tests/TripProgressTests.swift index 592f9b1..146305a 100644 --- a/OTPKit/Tests/TripProgressTests.swift +++ b/OTPKit/Tests/TripProgressTests.swift @@ -292,10 +292,13 @@ struct TripProgressTests { // MARK: - Rental legs - /// Walk 3m to the vehicle → ride the rental 10m → walk 2m to the destination. - func makeRentalItinerary() -> Itinerary { + /// Walk 3m to the vehicle → (optional gap) → ride the rental 10m → walk 2m to + /// the destination. A gap models the sub-minute approach walk that + /// `relevantLegs` merges away, leaving a hole before the ride starts. + func makeRentalItinerary(pickupGap: TimeInterval = 0) -> Itinerary { let walkEnd = tripStart.addingTimeInterval(180) - let rideEnd = walkEnd.addingTimeInterval(600) + let rideStart = walkEnd.addingTimeInterval(pickupGap) + let rideEnd = rideStart.addingTimeInterval(600) let arrivalTime = rideEnd.addingTimeInterval(120) let vehiclePlace = Place(name: "Default vehicle type", lon: -122.34, lat: 47.615, @@ -315,7 +318,7 @@ struct TripProgressTests { ) let rideLeg = Leg( - startTime: walkEnd, endTime: rideEnd, mode: "BICYCLE", + startTime: rideStart, endTime: rideEnd, mode: "BICYCLE", routeType: nil, routeColor: nil, routeTextColor: nil, route: nil, agencyName: nil, from: vehiclePlace, to: dropoffPlace, legGeometry: LegGeometry(points: "AA@@", length: 4), @@ -356,6 +359,22 @@ struct TripProgressTests { #expect(rentalProgress(at: 400).phase == .riding(legIndex: 1)) } + @Test func gapBeforeRentalRideIsWaitingAtThePickup() throws { + // Walk ends at 180s, ride starts at 300s: the rider is approaching the + // parked vehicle. Classifying this as .walking would point the tip at the + // rental leg's full ride distance and leave the rail with no current row. + let progress = TripProgress( + itinerary: makeRentalItinerary(pickupGap: 120), + now: tripStart.addingTimeInterval(240) + ) + + #expect(progress.phase == .waiting(boardingLegIndex: 1)) + + let currentRows = progress.rows.filter { $0.state == .current } + #expect(currentRows.map(\.kind) == [.pickUpVehicle(legIndex: 1)]) + #expect(progress.localizedActivityName == OTPLoc("rail.pick_up_bike", comment: "")) + } + @Test func rentalLegProducesPickupRideAndDropoffRows() { let rows = rentalProgress(at: 400).rows let kinds = rows.map(\.kind) diff --git a/OTPKit/Tests/VehicleRentalSourceTests.swift b/OTPKit/Tests/VehicleRentalSourceTests.swift index bfe8120..707cbff 100644 --- a/OTPKit/Tests/VehicleRentalSourceTests.swift +++ b/OTPKit/Tests/VehicleRentalSourceTests.swift @@ -69,6 +69,17 @@ struct VehicleRentalSourceTests { maximumLongitude: -122.2 ) + /// A slightly panned viewport: identical boxes are deliberately deduplicated + /// by the source, so successive fetches in tests must actually move. + private static func pannedBox(_ offset: Double) -> VehicleRentalBoundingBox { + VehicleRentalBoundingBox( + minimumLatitude: 47.5 + offset, + maximumLatitude: 47.7 + offset, + minimumLongitude: -122.4, + maximumLongitude: -122.2 + ) + } + private static func makeRental(id: String, lat: Double = 47.61) -> VehicleRental { .vehicle(RentalVehicle( vehicleId: id, @@ -130,7 +141,7 @@ struct VehicleRentalSourceTests { await source.setViewport(Self.seattleBox) _ = await snapshots.next() - await source.setViewport(Self.seattleBox) + await source.setViewport(Self.pannedBox(0.01)) let snapshot = try #require(await snapshots.next()) #expect(snapshot.added.map(\.id) == ["c"]) @@ -138,6 +149,22 @@ struct VehicleRentalSourceTests { #expect(snapshot.updated.map(\.id) == ["b"]) } + @Test("An identical viewport does not refetch") + func identicalViewportDeduplicated() async throws { + let service = ScriptedRentalService(results: [ + .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])) + ]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = await snapshots.next() + + await source.setViewport(Self.seattleBox) + try await Task.sleep(for: .milliseconds(30)) + #expect(await service.calls.count == 1) + } + @Test("An unchanged entity is neither added nor updated") func unchangedEntityNotRedelivered() async throws { let rentals = [Self.makeRental(id: "a")] @@ -148,7 +175,7 @@ struct VehicleRentalSourceTests { await source.setViewport(Self.seattleBox) _ = await snapshots.next() - await source.setViewport(Self.seattleBox) + await source.setViewport(Self.pannedBox(0.01)) let snapshot = try #require(await snapshots.next()) #expect(snapshot.isEmpty) } @@ -197,7 +224,7 @@ struct VehicleRentalSourceTests { await source.setViewport(Self.seattleBox) try await Task.sleep(for: .milliseconds(50)) // let the first fetch get in flight - await source.setViewport(Self.seattleBox) + await source.setViewport(Self.pannedBox(0.01)) let snapshot = try #require(await snapshots.next()) #expect(snapshot.added.map(\.id) == ["fresh"]) @@ -268,16 +295,37 @@ struct VehicleRentalSourceTests { await source.setViewport(Self.seattleBox) _ = await snapshots.next() - await source.setViewport(Self.seattleBox) + await source.setViewport(Self.pannedBox(0.01)) let failure = try #require(await failures.next()) #expect(failure.underlyingError is ScriptedError) // The next successful fetch diffs against state that survived the failure. - await source.setViewport(Self.seattleBox) + await source.setViewport(Self.pannedBox(0.02)) let snapshot = try #require(await snapshots.next()) #expect(snapshot.isEmpty) } + @Test("A failed viewport retries on the next identical region emission") + func failedViewportRetries() async throws { + let service = ScriptedRentalService(results: [ + .failure(ScriptedError()), + .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])) + ]) + let source = Self.makeSource(service: service) + var snapshots = source.snapshots.makeAsyncIterator() + var failures = source.fetchFailures.makeAsyncIterator() + + await source.setViewport(Self.seattleBox) + _ = try #require(await failures.next()) + + // A stationary map re-emits the same region; the failure must not be + // swallowed by the same-viewport deduplication. + await source.setViewport(Self.seattleBox) + let snapshot = try #require(await snapshots.next()) + #expect(snapshot.added.map(\.id) == ["a"]) + #expect(await service.calls.count == 2) + } + @Test("Changing form factors refetches with the new filter") func formFactorChangeRefetches() async throws { let service = ScriptedRentalService(results: [ From 01cf7952881bb3264f98f97bb3b035aacadd5d86 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Wed, 29 Jul 2026 02:59:51 -0700 Subject: [PATCH 5/6] Satisfy SwiftLint strict mode File-length/type-length pragmas for the state machine and GraphQL documents; un-nest the scripted service's call record; reattach a doc comment the lint pragma had orphaned. --- .../Sources/OTPKit/Core/Map/MapCoordinator.swift | 4 ++++ .../OTPKit/Core/TripProgress/TripProgress.swift | 7 +++++++ .../OTPKit/Network/GraphQLAPIService.swift | 7 +++++-- .../Directions/Rail/RailRowContentViews.swift | 4 ++++ OTPKit/Tests/Helpers/TestFixtures.swift | 4 ++-- OTPKit/Tests/VehicleRentalSourceTests.swift | 15 ++++++++------- 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift b/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift index ce4dcbc..c1b29cc 100644 --- a/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift +++ b/OTPKit/Sources/OTPKit/Core/Map/MapCoordinator.swift @@ -11,6 +11,8 @@ import MapKit import SwiftUI import OSLog +// swiftlint:disable file_length + /// Coordinates all map operations between OTPKit and the external map provider /// This class manages routes, annotations, and user interactions with the map @MainActor @@ -405,3 +407,5 @@ public class MapCoordinator: ObservableObject { // swiftlint:disable:this type_b Logger.main.info("Annotation selected: \(identifier)") } } + +// swiftlint:enable file_length diff --git a/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift b/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift index 83d1077..7fcb190 100644 --- a/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift +++ b/OTPKit/Sources/OTPKit/Core/TripProgress/TripProgress.swift @@ -16,6 +16,11 @@ import Foundation +// swiftlint:disable file_length +// The pure in-trip state machine: phases, rows, and the rider questions they +// answer all live together deliberately — splitting them would scatter one +// state machine across files. + /// Where the rider is in an itinerary right now. /// /// This is the `currentLeg` cursor of the in-trip panel's two-cursor model: it @@ -437,3 +442,5 @@ public struct TripProgress { } } } + +// swiftlint:enable file_length diff --git a/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift b/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift index 78e2b70..0dc2c0c 100644 --- a/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift +++ b/OTPKit/Sources/OTPKit/Network/GraphQLAPIService.swift @@ -19,7 +19,10 @@ import OSLog /// Actor-based GraphQL API client for OTP 2.x trip planning and vehicle rentals /// via the GTFS GraphQL API. -public actor GraphQLAPIService: APIService, VehicleRentalService { +/// +/// Most of the type body is the two static GraphQL documents; the lint pragmas +/// below account for them, not for logic. +public actor GraphQLAPIService: APIService, VehicleRentalService { // swiftlint:disable:this type_body_length public nonisolated let baseURL: URL public nonisolated let dataLoader: URLDataLoader @@ -223,7 +226,7 @@ public actor GraphQLAPIService: APIService, VehicleRentalService { /// argument (with its `PlanViaLocationInput` type) only exists on OTP 2.7+ — a /// document that always declared it would break every plan request, transit /// included, against older 2.x servers. - static func planQuery(includingVia: Bool) -> String { + static func planQuery(includingVia: Bool) -> String { // swiftlint:disable:this function_body_length let viaDeclaration = includingVia ? "\n $via: [PlanViaLocationInput!]" : "" let viaArgument = includingVia ? "\n via: $via" : "" return """ diff --git a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift index 4de3fb5..5d7ea5e 100644 --- a/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift +++ b/OTPKit/Sources/OTPKit/Presentation/Sheets/Directions/Rail/RailRowContentViews.swift @@ -7,6 +7,8 @@ import SwiftUI +// swiftlint:disable file_length + // The content column of each rail row kind. The views are state-aware: done // rows collapse to one line, current boarding/riding rows expand into the // tinted NowCard, and a focused boarding row shows its full detail inside the @@ -474,3 +476,5 @@ struct ArriveRowContent: View { .font(.body.weight(.semibold)) } } + +// swiftlint:enable file_length diff --git a/OTPKit/Tests/Helpers/TestFixtures.swift b/OTPKit/Tests/Helpers/TestFixtures.swift index af093a3..6dd03ce 100644 --- a/OTPKit/Tests/Helpers/TestFixtures.swift +++ b/OTPKit/Tests/Helpers/TestFixtures.swift @@ -64,8 +64,8 @@ enum TestFixtures { } /// A mock APIService that also advertises vehicle rental capability, for testing - /// capability-gated behavior like `availableTransportModes`. - // @unchecked: single-threaded test usage; VehicleRentalService requires Sendable. + /// capability-gated behavior like `availableTransportModes`. `@unchecked Sendable`: + /// single-threaded test usage; `VehicleRentalService` requires `Sendable`. final class MockRentalAPIService: MockAPIService, VehicleRentalService, @unchecked Sendable { var mockRentals: [VehicleRental] = [] diff --git a/OTPKit/Tests/VehicleRentalSourceTests.swift b/OTPKit/Tests/VehicleRentalSourceTests.swift index 707cbff..21c50fe 100644 --- a/OTPKit/Tests/VehicleRentalSourceTests.swift +++ b/OTPKit/Tests/VehicleRentalSourceTests.swift @@ -10,18 +10,19 @@ import Foundation import Testing @testable import OTPKit +/// One recorded `fetchVehicleRentals` invocation on the scripted service. +private struct RentalServiceCall: Sendable { + let boundingBox: VehicleRentalBoundingBox + let formFactors: Set? +} + @Suite("VehicleRentalSource") struct VehicleRentalSourceTests { // MARK: - Scripted service private actor ScriptedRentalService: VehicleRentalService { - struct Call: Sendable { - let boundingBox: VehicleRentalBoundingBox - let formFactors: Set? - } - - private(set) var calls: [Call] = [] + private(set) var calls: [RentalServiceCall] = [] private var results: [Result] private var delay: Duration = .zero @@ -37,7 +38,7 @@ struct VehicleRentalSourceTests { in boundingBox: VehicleRentalBoundingBox, formFactors: Set? ) async throws -> VehicleRentalFetchResult { - calls.append(Call(boundingBox: boundingBox, formFactors: formFactors)) + calls.append(RentalServiceCall(boundingBox: boundingBox, formFactors: formFactors)) // Claim the scripted result at call time, before any delay: a cancelled // call must still consume its result so later calls stay aligned with From c00b5b23ed95f4d6386fd7a94d1cc20ab022e334 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Wed, 29 Jul 2026 21:57:01 -0700 Subject: [PATCH 6/6] Fix CI build break and a hanging rental test Two problems that CI's Xcode 26.2 exposes but the local Xcode 27 beta hides. TripPlanner.swift used CLLocationCoordinate2D for the new viaPoint parameter without importing CoreLocation. The iOS 27 SDK makes CoreLocation visible transitively through UIKit/SwiftUI, so this built locally; the iOS 26.2 SDK does not, so CI failed with "cannot find type 'CLLocationCoordinate2D' in scope". The two existing files using CL types both import MapKit, which genuinely re-exports CoreLocation on both SDKs, which is why they never broke. VehicleRentalSourceTests.failureReported() hung forever. It triggers three fetches but scripted only two results, and ScriptedRentalService makes its last result sticky -- so the recovery fetch failed again, emitting on fetchFailures instead of snapshots while the test awaited snapshots.next(). Added the third success entry the test's own comment assumes. CI never reached this test because the build failed first, so fixing only the import would have left CI hanging instead of failing. Full suite: 229 tests in 20 suites pass, SwiftLint --strict clean. --- OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift | 1 + OTPKit/Tests/VehicleRentalSourceTests.swift | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift b/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift index 92dd32b..5ebf4b9 100644 --- a/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift +++ b/OTPKit/Sources/OTPKit/Presentation/TripPlanner.swift @@ -5,6 +5,7 @@ // Created by Manu on 2025-09-18. // +import CoreLocation import UIKit import SwiftUI diff --git a/OTPKit/Tests/VehicleRentalSourceTests.swift b/OTPKit/Tests/VehicleRentalSourceTests.swift index 21c50fe..3348338 100644 --- a/OTPKit/Tests/VehicleRentalSourceTests.swift +++ b/OTPKit/Tests/VehicleRentalSourceTests.swift @@ -287,7 +287,11 @@ struct VehicleRentalSourceTests { func failureReported() async throws { let service = ScriptedRentalService(results: [ .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])), - .failure(ScriptedError()) + .failure(ScriptedError()), + // The script's last result is sticky, so the recovery fetch below needs + // its own success entry — otherwise the failure repeats forever and the + // snapshot this test waits on is never emitted. + .success(VehicleRentalFetchResult(rentals: [Self.makeRental(id: "a")])) ]) let source = Self.makeSource(service: service) var snapshots = source.snapshots.makeAsyncIterator()