RouteOptions subclass type is erased when rerouting

[1ec5]

We allow and sometimes encourage developers to subclass NavigationRouteOptions to fine-tune the Directions API requests that come from the application. For example, mapbox/mapbox-navigation-ios-examples#91 illustrates how to use beta query parameters that the MapboxDirections library doesn’t formally support yet. When either RouteController or LegacyRouteController reroutes the user, the SDK stops using the developer’s subclass, so any customizations implemented in a urlQueryItems override get blown away.

Diagnosis

RouteOptions.without(waypoint:) and RouteProgress.reroutingOptions(with:) effectively replace the subclass instance with a RouteOptions instance (not even a NavigationRouteOptions instance), so that it can modify the set of waypoints without affecting the original options object.

mapbox-navigation-ios/Sources/MapboxCoreNavigation/Router.swift

Lines 246 to 248 in e308e0c

	func getDirections(from location: CLLocation, along progress: RouteProgress, completion: @escaping Directions.RouteCompletionHandler) {
	routeTask?.cancel()
	let options = progress.reroutingOptions(with: location)

mapbox-navigation-ios/Sources/MapboxCoreNavigation/RouteProgress.swift

Lines 249 to 251 in e308e0c

	let newWaypoints = [user] + remainingWaypointsForCalculatingRoute()
	let newOptions = oldOptions.copy() as! RouteOptions
	newOptions.waypoints = newWaypoints

mapbox-navigation-ios/Sources/MapboxCoreNavigation/RouteOptions.swift

Lines 32 to 40 in e308e0c

	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)
	} catch {
	preconditionFailure("Unable to copy RouteOptions by round-tripping it through JSON")
	}
	}

This is an awkward mix of Objective-C and Swift paradigms for dealing with value types: traditionally, in Objective-C, RouteOptions would be immutable and a separate MutableRouteOptions class would have mutable properties, but that complicates subclassing. In Swift, a struct would be the correct way to implement the pass-by-value semantics we’re going for: mapbox/mapbox-directions-swift#564.

In practice, RouteOptions itself has mutable properties, so as a workaround, 70bd68f for #2275 implemented NSCopying on RouteOptions, hard-coding the RouteOptions type in the implementation of copy(with:). This means any subclass of RouteOptions, including NavigationRouteOptions, would need to override copy(with:) to preserve any customizations after rerouting. NavigationRouteOptions happens to be unaffected, because all it does is set a few good default values in its initializer – values that RouteOptions’ Codable implementation would naturally preserve when round-tripping through JSON.

Next steps

While we await the bright future after mapbox/mapbox-directions-swift#564 is fixed, RouteOptions.copy(with:) should dynamically decode the current object’s type rather than hard-coding RouteOptions per se. Documentation about RouteOptions and NavigationRouteOptions should include a subclassing note that discusses overriding init(from:) and encode(to:). The modified RouteOptions.copy(with:) would only work to the extent that RouteOptions subclasses are resilient to being round-tripped through JSON.

/cc @mapbox/navigation-ios