From 3b55a5affa716e0dacc4d6f90cdc5edbb6be744e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Tue, 20 Jul 2021 12:48:08 -0700 Subject: [PATCH 1/3] Copy RouteOptions as its most specific type Fixed an issue where copying a RouteOptions subclass resulted in an instance of RouteOptions itself. --- CHANGELOG.md | 1 + MapboxNavigation.xcodeproj/project.pbxproj | 4 +++ .../NavigationRouteOptions.swift | 5 ++- .../MapboxCoreNavigation/RouteOptions.swift | 4 +-- .../Extensions/RouteOptionsTests.swift | 35 +++++++++++++++++++ 5 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 Tests/MapboxCoreNavigationTests/Extensions/RouteOptionsTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 2282892ca0d..5a2f2ac57c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,7 @@ * Fixed an issue where traffic congestion segments along the route line blurred into each other when the map was zoomed in far enough. ([#3153](https://github.com/mapbox/mapbox-navigation-ios/pull/3153)) * Supported feedbacks during passive navigation. Set `PassiveLocationManager` as a data source for `NavigationEventsManager` and then use this class to create and send user feedbacks. `FeedbackViewController` provides drop-in feedback screen and can be used during both passive and active navigation. ([#3122](https://github.com/mapbox/mapbox-navigation-ios/pull/3122)) * `NavigationViewController.indexedRoute`, `NavigationService.indexedRoute` and `Router.indexedRoute` properties are readonly now. Use dedicated `Router.updateRoute(with:routeOptions:)` method to update the route. ([#3159](https://github.com/mapbox/mapbox-navigation-ios/pull/#3159)) +* Fixed an issue where a subclass of `NavigationRouteOptions` would turn into an ordinary `RouteOptions` when rerouting the user. ([#3192](https://github.com/mapbox/mapbox-navigation-ios/pull/3192)) ## v1.4.1 diff --git a/MapboxNavigation.xcodeproj/project.pbxproj b/MapboxNavigation.xcodeproj/project.pbxproj index bfdb660a3da..209f9c09b0c 100644 --- a/MapboxNavigation.xcodeproj/project.pbxproj +++ b/MapboxNavigation.xcodeproj/project.pbxproj @@ -329,6 +329,7 @@ DA5F450025F07DE200F573EC /* ElectronicHorizonOptions.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA5F44FF25F07DE200F573EC /* ElectronicHorizonOptions.swift */; }; DA66063023B32F99007832E5 /* Array.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA66062F23B32F99007832E5 /* Array.swift */; }; DA754E1923AC56E5007E16B5 /* MBXAccountsLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = DA754E1723AC56E5007E16B5 /* MBXAccountsLoader.m */; }; + DA7A97CF26A7613D001B6A9A /* RouteOptionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA7A97CE26A7613C001B6A9A /* RouteOptionsTests.swift */; }; DA85D5EF25DB4AA4008A2AD4 /* LaneViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA85D5EE25DB4AA3008A2AD4 /* LaneViewTests.swift */; }; DA8F3A7623B5D84900B56786 /* SpeedLimitView.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA8F3A7523B5D84900B56786 /* SpeedLimitView.swift */; }; DA8F3A7823B5DB7900B56786 /* SpeedLimitStyleKit.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA8F3A7723B5DB7900B56786 /* SpeedLimitStyleKit.swift */; }; @@ -864,6 +865,7 @@ DA6C925A24C60C92003A0AD6 /* tr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = tr; path = tr.lproj/Navigation.strings; sourceTree = ""; }; DA73F87820BF851B0067649B /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = de; path = Resources/de.lproj/Localizable.stringsdict; sourceTree = ""; }; DA754E1723AC56E5007E16B5 /* MBXAccountsLoader.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = MBXAccountsLoader.m; path = include/MBXAccountsLoader.m; sourceTree = ""; }; + DA7A97CE26A7613C001B6A9A /* RouteOptionsTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RouteOptionsTests.swift; sourceTree = ""; }; DA8264871F2AADC200454B24 /* zh-Hant */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.strings; name = "zh-Hant"; path = "zh-Hant.lproj/Navigation.strings"; sourceTree = ""; }; DA85D5EE25DB4AA3008A2AD4 /* LaneViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LaneViewTests.swift; sourceTree = ""; }; DA8F3A7523B5D84900B56786 /* SpeedLimitView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpeedLimitView.swift; sourceTree = ""; }; @@ -1175,6 +1177,7 @@ children = ( 3A163ADF249901C300D66A0D /* RouteStateTests.swift */, 11D1F8A12696EBD40053A93F /* Dictionary+Equality.swift */, + DA7A97CE26A7613C001B6A9A /* RouteOptionsTests.swift */, ); path = Extensions; sourceTree = ""; @@ -2502,6 +2505,7 @@ C582BA2C2073E77E00647DAA /* StringTests.swift in Sources */, B426FEF425FFD5DC001884C8 /* RouteControllerTests.swift in Sources */, 352762A4225B751A0015B632 /* OptionsTests.swift in Sources */, + DA7A97CF26A7613D001B6A9A /* RouteOptionsTests.swift in Sources */, 8DB7EF6A2176674800DA83A3 /* MapboxNavigationServiceSpec.swift in Sources */, B426FF0525FFD679001884C8 /* SimulatedLocationManagerTests.swift in Sources */, 35EF782A212C324E001B4BB5 /* TunnelAuthorityTests.swift in Sources */, diff --git a/Sources/MapboxCoreNavigation/NavigationRouteOptions.swift b/Sources/MapboxCoreNavigation/NavigationRouteOptions.swift index d0ec0639bb4..d657d6fdec2 100644 --- a/Sources/MapboxCoreNavigation/NavigationRouteOptions.swift +++ b/Sources/MapboxCoreNavigation/NavigationRouteOptions.swift @@ -6,7 +6,10 @@ import MapboxDirections A `NavigationRouteOptions` object specifies turn-by-turn-optimized criteria for results returned by the Mapbox Directions API. `NavigationRouteOptions` is a subclass of `RouteOptions` that has been optimized for navigation. Pass an instance of this class into the `Directions.calculate(_:completionHandler:)` method. - - note: `NavigationRouteOptions` is designed to be used with the `Directions` and `NavigationDirections` classes for specifying routing criteria. To customize the user experience in a `NavigationViewController`, use the `NavigationOptions` class. + + This class implements the `NSCopying` protocol by round-tripping the object through `JSONEncoder` and `JSONDecoder`. If you subclass `NavigationRouteOptions`, make sure any properties you add are accounted for in `Decodable(from:)` and `Encodable.encode(to:)`. If your subclass contains any customizations that cannot be represented in JSON, make sure the subclass overrides `NSCopying.copy(with:)` to persist those customizations. + + `NavigationRouteOptions` is designed to be used with the `Directions` and `NavigationDirections` classes for specifying routing criteria. To customize the user experience in a `NavigationViewController`, use the `NavigationOptions` class. */ open class NavigationRouteOptions: RouteOptions, OptimizedForNavigation { /** diff --git a/Sources/MapboxCoreNavigation/RouteOptions.swift b/Sources/MapboxCoreNavigation/RouteOptions.swift index f9a73140023..e19f232233a 100644 --- a/Sources/MapboxCoreNavigation/RouteOptions.swift +++ b/Sources/MapboxCoreNavigation/RouteOptions.swift @@ -33,9 +33,9 @@ extension RouteOptions: NSCopying { public func copy(with zone: NSZone? = nil) -> Any { do { let encodedOptions = try JSONEncoder().encode(self) - return try JSONDecoder().decode(RouteOptions.self, from: encodedOptions) + return try JSONDecoder().decode(type(of: self), from: encodedOptions) } catch { - preconditionFailure("Unable to copy RouteOptions by round-tripping it through JSON") + preconditionFailure("Unable to copy \(type(of: self)) by round-tripping it through JSON: \(error)") } } diff --git a/Tests/MapboxCoreNavigationTests/Extensions/RouteOptionsTests.swift b/Tests/MapboxCoreNavigationTests/Extensions/RouteOptionsTests.swift new file mode 100644 index 00000000000..57d662ec502 --- /dev/null +++ b/Tests/MapboxCoreNavigationTests/Extensions/RouteOptionsTests.swift @@ -0,0 +1,35 @@ +import XCTest +import CoreLocation +import MapboxDirections +@testable import MapboxCoreNavigation + +/** + A hypothetical set of options optimized for golf carts. + + This class uses options that may or may not be supported by the actual Mapbox Directions API. + */ +class GolfCartRouteOptions: NavigationRouteOptions { + override var urlQueryItems: [URLQueryItem] { + let maximumSpeed = Measurement(value: 20, unit: UnitSpeed.milesPerHour) // maximum legal speed in Ohio + let hourFromNow = Date().addingTimeInterval(60 * 60) // an hour from now + let hourFromNowString = ISO8601DateFormatter.string(from: hourFromNow, timeZone: .current, formatOptions: .withInternetDateTime) + return super.urlQueryItems + [ + URLQueryItem(name: "maxspeed", value: String(maximumSpeed.converted(to: .kilometersPerHour).value)), + URLQueryItem(name: "depart_at", value: hourFromNowString), + URLQueryItem(name: "passengers", value: "3"), + ] + } +} + +class RouteOptionsTests: XCTestCase { + func testCopying() { + let coordinates: [CLLocationCoordinate2D] = [ + .init(latitude: 0, longitude: 0), + .init(latitude: 1, longitude: 1), + ] + let options = GolfCartRouteOptions(coordinates: coordinates, profileIdentifier: .automobile) + let copy = options.copy() as? GolfCartRouteOptions + XCTAssertNotNil(copy) + XCTAssertTrue(copy?.urlQueryItems.contains(URLQueryItem(name: "passengers", value: "3")) ?? false) + } +} From e47a51c2dc51ab9751af6f1e8662878638b8585d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Tue, 20 Jul 2021 12:49:18 -0700 Subject: [PATCH 2/3] Renamed RouteOptions.without(waypoint:) to RouteOptions.without(_:) --- CHANGELOG.md | 1 + Example/ViewController.swift | 2 +- Sources/MapboxCoreNavigation/RouteOptions.swift | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a2f2ac57c7..b06dc583df1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,6 +127,7 @@ * Supported feedbacks during passive navigation. Set `PassiveLocationManager` as a data source for `NavigationEventsManager` and then use this class to create and send user feedbacks. `FeedbackViewController` provides drop-in feedback screen and can be used during both passive and active navigation. ([#3122](https://github.com/mapbox/mapbox-navigation-ios/pull/3122)) * `NavigationViewController.indexedRoute`, `NavigationService.indexedRoute` and `Router.indexedRoute` properties are readonly now. Use dedicated `Router.updateRoute(with:routeOptions:)` method to update the route. ([#3159](https://github.com/mapbox/mapbox-navigation-ios/pull/#3159)) * Fixed an issue where a subclass of `NavigationRouteOptions` would turn into an ordinary `RouteOptions` when rerouting the user. ([#3192](https://github.com/mapbox/mapbox-navigation-ios/pull/3192)) +* Renamed `RouteOptions.without(waypoint:)` to `RouteOptions.without(_:)`. ([#3192](https://github.com/mapbox/mapbox-navigation-ios/pull/3192)) ## v1.4.1 diff --git a/Example/ViewController.swift b/Example/ViewController.swift index 7fce4c7bf04..7f3c2a6949b 100755 --- a/Example/ViewController.swift +++ b/Example/ViewController.swift @@ -519,7 +519,7 @@ extension ViewController: NavigationMapViewDelegate { func navigationMapView(_ mapView: NavigationMapView, didSelect waypoint: Waypoint) { guard let responseOptions = response?.options, case let .route(routeOptions) = responseOptions else { return } - let modifiedOptions = routeOptions.without(waypoint: waypoint) + let modifiedOptions = routeOptions.without(waypoint) presentWaypointRemovalAlert { _ in self.requestRoute(with:modifiedOptions, success: self.defaultSuccess, failure: self.defaultFailure) diff --git a/Sources/MapboxCoreNavigation/RouteOptions.swift b/Sources/MapboxCoreNavigation/RouteOptions.swift index e19f232233a..52dd97c2f01 100644 --- a/Sources/MapboxCoreNavigation/RouteOptions.swift +++ b/Sources/MapboxCoreNavigation/RouteOptions.swift @@ -45,7 +45,7 @@ extension RouteOptions: NSCopying { - parameter waypoint: the Waypoint to exclude. - returns: a copy of self excluding the specified waypoint. */ - public func without(waypoint: Waypoint) -> RouteOptions { + public func without(_ waypoint: Waypoint) -> RouteOptions { let waypointsWithoutSpecified = waypoints.filter { $0 != waypoint } let copy = self.copy() as! RouteOptions copy.waypoints = waypointsWithoutSpecified From 4a8b3ff2b56f1740c8f04ff9eef76b7aaea3b2ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguye=CC=82=CC=83n?= Date: Tue, 20 Jul 2021 18:25:22 -0700 Subject: [PATCH 3/3] Open up RouteOptions.copy(with:) --- Sources/MapboxCoreNavigation/RouteOptions.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/MapboxCoreNavigation/RouteOptions.swift b/Sources/MapboxCoreNavigation/RouteOptions.swift index 52dd97c2f01..08678d9052a 100644 --- a/Sources/MapboxCoreNavigation/RouteOptions.swift +++ b/Sources/MapboxCoreNavigation/RouteOptions.swift @@ -30,7 +30,7 @@ extension RouteOptions { } extension RouteOptions: NSCopying { - public func copy(with zone: NSZone? = nil) -> Any { + open func copy(with zone: NSZone? = nil) -> Any { do { let encodedOptions = try JSONEncoder().encode(self) return try JSONDecoder().decode(type(of: self), from: encodedOptions)