Consistently filter waypoints by leg separation

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changes to MapboxDirections.swift
 
+## master
+
+* Fixed an issue where `Waypoint.separatesLegs` caused the resulting `RouteLeg.source` and `RouteLeg.destination` to have mismatched coordinates and names. ([#358](https://github.com/mapbox/MapboxDirections.swift/pull/358))
+* Fixed an issue where a Directions API or Map Matching API request would fail if a `Waypoint` has `Waypoint.name` set and `Waypoint.separatesLegs` set to `false`. ([#358](https://github.com/mapbox/MapboxDirections.swift/pull/358))
+
 ## v0.27.1
 
 ### Offline routing

MapboxDirections.xcodeproj/project.pbxproj

@@ -232,6 +232,9 @@
 		DAC05F181CFC075300FA0071 /* MBRoute.swift in Sources */ = {isa = PBXBuildFile; fileRef = DAC05F171CFC075300FA0071 /* MBRoute.swift */; };
 		DAC05F1A1CFC077C00FA0071 /* MBRouteLeg.swift in Sources */ = {isa = PBXBuildFile; fileRef = DAC05F191CFC077C00FA0071 /* MBRouteLeg.swift */; };
 		DAC05F1C1CFC1E5300FA0071 /* v5_driving_dc_polyline.json in Resources */ = {isa = PBXBuildFile; fileRef = DAC05F1B1CFC1E5300FA0071 /* v5_driving_dc_polyline.json */; };
+		DACCFCA92225359600110FC9 /* v5_driving_oldenburg_polyline.json in Resources */ = {isa = PBXBuildFile; fileRef = DACCFCA82225359500110FC9 /* v5_driving_oldenburg_polyline.json */; };
+		DACCFCAA2225359600110FC9 /* v5_driving_oldenburg_polyline.json in Resources */ = {isa = PBXBuildFile; fileRef = DACCFCA82225359500110FC9 /* v5_driving_oldenburg_polyline.json */; };
+		DACCFCAB2225359600110FC9 /* v5_driving_oldenburg_polyline.json in Resources */ = {isa = PBXBuildFile; fileRef = DACCFCA82225359500110FC9 /* v5_driving_oldenburg_polyline.json */; };
 		DADD27B81E5AAAD800D31FAD /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = DADD27B71E5AAAD800D31FAD /* AppDelegate.swift */; };
 		DADD27BA1E5AAAD800D31FAD /* ViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = DADD27B91E5AAAD800D31FAD /* ViewController.swift */; };
 		DADD27BF1E5AAAD800D31FAD /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = DADD27BE1E5AAAD800D31FAD /* Assets.xcassets */; };
@@ -387,6 +390,7 @@
 		DAC05F171CFC075300FA0071 /* MBRoute.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MBRoute.swift; sourceTree = "<group>"; };
 		DAC05F191CFC077C00FA0071 /* MBRouteLeg.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MBRouteLeg.swift; sourceTree = "<group>"; };
 		DAC05F1B1CFC1E5300FA0071 /* v5_driving_dc_polyline.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = v5_driving_dc_polyline.json; sourceTree = "<group>"; };
+		DACCFCA82225359500110FC9 /* v5_driving_oldenburg_polyline.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = v5_driving_oldenburg_polyline.json; sourceTree = "<group>"; };
 		DADD27B51E5AAAD800D31FAD /* Example (Swift).app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = "Example (Swift).app"; sourceTree = BUILT_PRODUCTS_DIR; };
 		DADD27B71E5AAAD800D31FAD /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = "<group>"; };
 		DADD27B91E5AAAD800D31FAD /* ViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewController.swift; sourceTree = "<group>"; };
@@ -665,6 +669,7 @@
 			children = (
 				DA737EE31D05F91E005BDA16 /* v5_driving_dc_geojson.json */,
 				DAC05F1B1CFC1E5300FA0071 /* v5_driving_dc_polyline.json */,
+				DACCFCA82225359500110FC9 /* v5_driving_oldenburg_polyline.json */,
 			);
 			name = V5;
 			path = v5;
@@ -1095,6 +1100,7 @@
 				35DBF01A217F38A30009D2AE /* versions.json in Resources */,
 				35D92FF1218203AB000C78CB /* 2018-10-16-Liechtenstein.tar in Resources */,
 				AEAB391220D9469A008F4E54 /* subVisualInstructions.json in Resources */,
+				DACCFCAA2225359600110FC9 /* v5_driving_oldenburg_polyline.json in Resources */,
 				C5DAACB5201AA9A7001F9261 /* match.json in Resources */,
 				DA737EEF1D06175B005BDA16 /* v4_driving_dc_geojson.json in Resources */,
 				C53A022B1E92C281009837BD /* annotation.json in Resources */,
@@ -1123,6 +1129,7 @@
 				35DBF01B217F38A30009D2AE /* versions.json in Resources */,
 				35D92FF2218203AB000C78CB /* 2018-10-16-Liechtenstein.tar in Resources */,
 				AEAB391320D9469A008F4E54 /* subVisualInstructions.json in Resources */,
+				DACCFCAB2225359600110FC9 /* v5_driving_oldenburg_polyline.json in Resources */,
 				C5DAACB6201AA9A7001F9261 /* match.json in Resources */,
 				DA737EF01D06175B005BDA16 /* v4_driving_dc_geojson.json in Resources */,
 				C53A022C1E92C281009837BD /* annotation.json in Resources */,
@@ -1158,6 +1165,7 @@
 				35DBF019217F38A30009D2AE /* versions.json in Resources */,
 				35D92FF0218203AB000C78CB /* 2018-10-16-Liechtenstein.tar in Resources */,
 				AEAB391120D9469A008F4E54 /* subVisualInstructions.json in Resources */,
+				DACCFCA92225359600110FC9 /* v5_driving_oldenburg_polyline.json in Resources */,
 				C5DAACB4201AA9A7001F9261 /* match.json in Resources */,
 				C5A3D3981E8188FE00D494A0 /* annotation.json in Resources */,
 				8D381B631FDB01D1008D5A58 /* apiDestinationName.json in Resources */,

MapboxDirections/MBDirectionsOptions.swift

@@ -366,6 +366,16 @@ open class DirectionsOptions: NSObject, NSSecureCoding, NSCopying {
      The array should contain at least two waypoints (the source and destination) and at most 25 waypoints.
      */
     @objc open var waypoints: [Waypoint]
+
+    /**
+     The waypoints that separate legs.
+     */
+    var legSeparators: [Waypoint] {
+        var waypoints = self.waypoints
+        let source = waypoints.removeFirst()
+        let destination = waypoints.removeLast()
+        return [source] + waypoints.filter { $0.separatesLegs } + [destination]
+    }
 
     /**
      A string specifying the primary mode of transportation for the routes.
@@ -507,7 +517,7 @@ open class DirectionsOptions: NSObject, NSSecureCoding, NSCopying {
         }
 
         if !waypoints.compactMap({ $0.name }).isEmpty {
-            let names = waypoints.map { $0.name ?? "" }.joined(separator: ";")
+            let names = legSeparators.map { $0.name ?? "" }.joined(separator: ";")
             queryItems.append(URLQueryItem(name: "waypoint_names", value: names))
         }
 

MapboxDirections/MBRouteOptions.swift

@@ -164,9 +164,12 @@ open class RouteOptions: DirectionsOptions {
         }
 
         let waypoints = namedWaypoints ?? self.waypoints
+        waypoints.first?.separatesLegs = true
+        waypoints.last?.separatesLegs = true
+        let legSeparators = waypoints.filter { $0.separatesLegs }
 
         let routes = (json["routes"] as? [JSONDictionary])?.map {
-            Route(json: $0, waypoints: waypoints, options: self)
+            Route(json: $0, waypoints: legSeparators, options: self)
         }
         return (waypoints, routes)
     }

MapboxDirections/Match/MBMatchOptions.swift

@@ -56,6 +56,14 @@ open class MatchOptions: DirectionsOptions {
     @available(*, deprecated: 0.1, message: "Use Waypoint.separatesLegs instead.")
     @objc open var waypointIndices: IndexSet?
 
+    override var legSeparators: [Waypoint] {
+        if let indices = (self as MatchOptionsDeprecations).waypointIndices {
+            return indices.map { super.waypoints[$0] }
+        } else {
+            return super.legSeparators
+        }
+    }
+
     @objc public required init?(coder decoder: NSCoder) {
         resamplesTraces = decoder.decodeBool(forKey: "resampleTraces")
         super.init(coder: decoder)
@@ -141,22 +149,17 @@ open class MatchOptions: DirectionsOptions {
         let waypoints = namedWaypoints ?? self.waypoints
         let opts = RouteOptions(matchOptions: self)
 
-        var filteredWaypoints: [Waypoint]?
+        let legSeparators: [Waypoint]
         if let indices = (self as MatchOptionsDeprecations).waypointIndices {
-            filteredWaypoints = []
-            for (i, waypoint) in waypoints.enumerated() {
-                if indices.contains(i) {
-                    filteredWaypoints?.append(waypoint)
-                }
-            }
+            legSeparators = indices.map { waypoints[$0] }
         } else {
             waypoints.first?.separatesLegs = true
             waypoints.last?.separatesLegs = true
-            filteredWaypoints = waypoints.filter { $0.separatesLegs }
+            legSeparators = waypoints.filter { $0.separatesLegs }
         }
 
         let routes = (json["matchings"] as? [JSONDictionary])?.map {
-            Route(json: $0, waypoints: filteredWaypoints ?? waypoints, options: opts)
+            Route(json: $0, waypoints: legSeparators, options: opts)
         }
 
         return (waypoints, routes)

MapboxDirectionsTests/Fixtures/v5/v5_driving_oldenburg_polyline.json

@@ -0,0 +1 @@
+{"routes":[{"geometry":"ahboF~y`gOeA?k@?_@?O@I?S?c@?S?sC@K@S?wABaC?k@@QA?IA{D?iAAkDAQbB?fAAt@?h@AJ??O?oAAmB?K?kAAmDCaKA[AMCG?EIMMEo@@kBBsA@K??F@|@","legs":[{"summary":"Perlen Strasse, Haupt Strasse","weight":238.9,"duration":122.1,"steps":[{"intersections":[{"out":0,"entry":[true],"bearings":[0],"location":[-85.206241,39.33841]},{"out":0,"in":1,"entry":[true,false,true],"bearings":[0,180,240],"location":[-85.206241,39.338979]},{"out":0,"in":2,"entry":[true,true,false,true],"bearings":[0,90,180,270],"location":[-85.206244,39.339143]},{"out":0,"in":2,"entry":[true,true,false],"bearings":[0,90,180],"location":[-85.206273,39.340549]}],"driving_side":"right","geometry":"ahboF~y`gOeA?k@?_@?O@I?S?c@?S?sC@K@S?wABaC?k@@QA","mode":"driving","maneuver":{"bearing_after":0,"bearing_before":0,"location":[-85.206241,39.33841],"type":"depart","instruction":"Fahren Sie Richtung Norden auf Maulbeerfeigen Strasse (SR 229)"},"ref":"SR 229","weight":38.1,"duration":25.6,"name":"Maulbeerfeigen Strasse (SR 229)","distance":393.3},{"intersections":[{"out":1,"in":2,"entry":[true,true,false,true],"bearings":[0,90,180,270],"location":[-85.206293,39.341945]},{"out":0,"in":2,"entry":[true,true,false],"bearings":[90,180,270],"location":[-85.204929,39.341962]}],"driving_side":"right","geometry":"e~boFhz`gO?IA{D?iAAkDAQ","mode":"driving","maneuver":{"bearing_after":88,"bearing_before":358,"location":[-85.206293,39.341945],"modifier":"right","type":"turn","instruction":"Rechts abbiegen auf Weinstock Strasse"},"weight":32.9,"duration":19.9,"name":"Weinstock Strasse","distance":198.8},{"intersections":[{"out":2,"in":3,"entry":[true,true,true,false],"bearings":[0,90,180,270],"location":[-85.203982,39.341975]}],"driving_side":"right","geometry":"k~boFzk`gObB?fAAt@?h@AJ?","mode":"driving","maneuver":{"bearing_after":178,"bearing_before":88,"location":[-85.203982,39.341975],"modifier":"right","type":"turn","instruction":"Rechts abbiegen auf Perlen Strasse"},"weight":94.7,"duration":35.8,"name":"Perlen Strasse","distance":155.5},{"intersections":[{"out":1,"in":0,"entry":[false,true,true,true],"bearings":[0,90,180,270],"location":[-85.203962,39.340577]},{"out":1,"in":3,"entry":[true,true,true,false],"bearings":[0,90,180,270],"location":[-85.201616,39.3406]},{"out":0,"in":1,"entry":[true,false],"bearings":[75,255],"location":[-85.199436,39.340656]}],"driving_side":"right","geometry":"suboFvk`gO?O?oAAmB?K?kAAmDCaKA[AMCG?EIMMEo@@kBBsA@K?","mode":"driving","maneuver":{"bearing_after":88,"bearing_before":178,"location":[-85.203962,39.340577],"modifier":"left","type":"turn","instruction":"Links abbiegen auf Haupt Strasse (SR 229)"},"ref":"SR 229","weight":71.2,"duration":38.8,"name":"Haupt Strasse (SR 229)","distance":548.2},{"intersections":[{"out":2,"in":1,"entry":[true,false,true],"bearings":[0,180,270],"location":[-85.199353,39.342038]}],"driving_side":"right","geometry":"w~boF|n_gO?F@|@","mode":"driving","maneuver":{"bearing_after":268,"bearing_before":357,"location":[-85.199353,39.342038],"modifier":"left","type":"turn","instruction":"Links abbiegen auf Weinstock Strasse"},"weight":2,"duration":2,"name":"Weinstock Strasse","distance":29.6},{"intersections":[{"in":0,"entry":[true],"bearings":[89],"location":[-85.199697,39.342033]}],"driving_side":"right","geometry":"u~boFbq_gO","mode":"driving","maneuver":{"bearing_after":0,"bearing_before":269,"location":[-85.199697,39.342033],"type":"arrive","instruction":"Sie haben Ihr To"},"weight":0,"duration":0,"name":"Weinstock Strasse","distance":0}],"distance":1325.4}],"weight_name":"routability","weight":238.9,"duration":122.1,"distance":1325.4}],"waypoints":[{"distance":0.7760785081050579,"name":"Maulbeerfeigen Strasse","location":[-85.206241,39.33841]},{"distance":0.9485403982504417,"name":"Perlen Strasse","location":[-85.20398,39.34181]},{"distance":1.665293749024342,"name":"Weinstock Strasse","location":[-85.199697,39.342033]}],"code":"Ok","uuid":"cjslj6r7403r34jo289thx5ax"}

MapboxDirectionsTests/V5Tests.swift

@@ -50,6 +50,8 @@ class V5Tests: XCTestCase {
         let task = Directions(accessToken: BogusToken).calculate(options) { (waypoints, routes, error) in
             XCTAssertNil(error, "Error: \(error!)")
 
+            XCTAssertEqual(waypoints?.count, 2)
+
             XCTAssertNotNil(routes)
             XCTAssertEqual(routes!.count, 2)
             route = routes!.first!
@@ -89,6 +91,7 @@ class V5Tests: XCTestCase {
         let opts = route.routeOptions
         XCTAssertEqual(opts, options)
 
+        XCTAssertEqual(route.legs.count, 1)
         let leg = route.legs.first
         XCTAssertEqual(leg?.name, "Dwight D. Eisenhower Highway, I-80")
         XCTAssertEqual(leg?.steps.count, 59)
@@ -203,6 +206,78 @@ class V5Tests: XCTestCase {
         test(shapeFormat: .polyline6, transformer: transformer, filePath: "v5_driving_dc_polyline")
     }
 
+    func testViaPoints() {
+        let expectation = self.expectation(description: "calculating directions should return results")
+
+        let queryParams: [String: String?] = [
+            "geometries": "polyline",
+            "overview": "full",
+            "steps": "true",
+            "language": "de_US",
+            "waypoints": "0;2",
+            "waypoint_names": "From;To",
+            "alternatives": "false",
+            "continue_straight": "true",
+            "roundabout_exits": "true",
+            "access_token": BogusToken,
+        ]
+        stub(condition: isHost("api.mapbox.com")
+            && isPath("/directions/v5/mapbox/driving/-85.206232,39.33841;-85.203991,39.34181;-85.199697,39.342048.json")
+            && containsQueryParams(queryParams)) { _ in
+                let path = Bundle(for: type(of: self)).path(forResource: "v5_driving_oldenburg_polyline", ofType: "json")
+                let filePath = URL(fileURLWithPath: path!)
+                let data = try! Data(contentsOf: filePath, options: [])
+                let jsonObject = try! JSONSerialization.jsonObject(with: data, options: [])
+                return OHHTTPStubsResponse(jsonObject: jsonObject, statusCode: 200, headers: ["Content-Type": "application/json"])
+        }
+
+        let waypoints = [
+            Waypoint(coordinate: CLLocationCoordinate2D(latitude: 39.33841036211459, longitude: -85.20623174166413), coordinateAccuracy: -1, name: "From"),
+            Waypoint(coordinate: CLLocationCoordinate2D(latitude: 39.34181048315713, longitude: -85.20399062653789), coordinateAccuracy: -1, name: "Via"),
+            Waypoint(coordinate: CLLocationCoordinate2D(latitude: 39.34204769474999, longitude: -85.19969651878529), coordinateAccuracy: -1, name: "To"),
+        ]
+        for waypoint in waypoints {
+            waypoint.separatesLegs = false
+        }
+
+        let options = RouteOptions(waypoints: waypoints)
+        XCTAssertEqual(options.shapeFormat, .polyline, "Route shape format should be Polyline by default.")
+        options.shapeFormat = .polyline
+        options.includesSteps = true
+        options.routeShapeResolution = .full
+        options.locale = Locale(identifier: "de_US")
+        options.includesExitRoundaboutManeuver = true
+
+        var route: Route?
+        let task = Directions(accessToken: BogusToken).calculate(options) { (waypoints, routes, error) in
+            XCTAssertNil(error, "Error: \(error!)")
+
+            XCTAssertEqual(waypoints?.count, 3)
+
+            XCTAssertNotNil(routes)
+            XCTAssertEqual(routes!.count, 1)
+            route = routes!.first!
+
+            expectation.fulfill()
+        }
+        XCTAssertNotNil(task)
+
+        waitForExpectations(timeout: 2) { (error) in
+            XCTAssertNil(error, "Error: \(error!)")
+            XCTAssertEqual(task.state, .completed)
+        }
+
+        XCTAssertEqual(route?.legs.count, 1)
+        let leg = route?.legs.first
+        XCTAssertEqual(leg?.source.name, waypoints[0].name)
+        XCTAssertEqual(leg?.source.coordinate.latitude ?? 0, waypoints[0].coordinate.latitude, accuracy: 1e-4)
+        XCTAssertEqual(leg?.source.coordinate.longitude ?? 0, waypoints[0].coordinate.longitude, accuracy: 1e-4)
+        XCTAssertEqual(leg?.destination.name, waypoints[2].name)
+        XCTAssertEqual(leg?.destination.coordinate.latitude ?? 0, waypoints[2].coordinate.latitude, accuracy: 1e-4)
+        XCTAssertEqual(leg?.destination.coordinate.longitude ?? 0, waypoints[2].coordinate.longitude, accuracy: 1e-4)
+        XCTAssertEqual(leg?.name, "Perlen Strasse, Haupt Strasse")
+    }
+
     func testCoding() {
         let path = Bundle(for: type(of: self)).path(forResource: "v5_driving_dc_polyline", ofType: "json")
         let filePath = URL(fileURLWithPath: path!)