Merge pull request #752 from mapbox/pad-refresh-incidents

mapbox · Oct 10, 2022 · 83c906d · 83c906d
2 parents d1bce29 + 9b512cd
commit 83c906d
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Other Changes
 
 * Added the `Waypoint.layer` property, which can ensure that the route begins on the correct road if it is above or below another road. ([#745](https://github.com/mapbox/mapbox-directions-swift/pull/745))
+* Fixed incorrect shape indicies in `RouteLeg.incidents` after route refresh. ([#752](https://github.com/mapbox/mapbox-directions-swift/pull/752))
 
 ## 2.7.0
 

diff --git a/Sources/MapboxDirections/RouteLeg.swift b/Sources/MapboxDirections/RouteLeg.swift
@@ -289,23 +289,15 @@ open class RouteLeg: Codable, ForeignMemberContainerClass {
     }
 
     func refreshIncidents(newIncidents: [Incident]?, startLegShapeIndex: Int = 0) {
-        guard let newIncidents = newIncidents else {
-            incidents = nil
-            return
+        incidents = newIncidents?.map { incident in
+            var adjustedIncident = incident
+            let startIndex = startLegShapeIndex + incident.shapeIndexRange.lowerBound
+            let endIndex = startLegShapeIndex + incident.shapeIndexRange.upperBound
+            adjustedIncident.shapeIndexRange = startIndex..<endIndex
+            return adjustedIncident
         }
-
-        incidents?.removeAll(where: {
-            $0.shapeIndexRange.upperBound > startLegShapeIndex
-        })
-
-        if incidents == nil {
-            incidents = []
-        }
-
-        incidents?.append(contentsOf: newIncidents)
     }
-
-
+
     /**
      Returns the ISO 3166-1 alpha-2 region code for the administrative region through which the given intersection passes. The intersection is identified by its step index and intersection index.
      

diff --git a/Sources/MapboxDirections/RouteRefreshResponse.swift b/Sources/MapboxDirections/RouteRefreshResponse.swift
@@ -140,15 +140,21 @@ extension Route {
 
     /**
      Merges the incidents of the given route’s legs into the receiver’s legs.
-     
+
      - parameter refreshedRoute: The route containing leg incidents to merge into the receiver. If this route contains fewer legs than the receiver, this method skips legs from the beginning of the route to make up the difference, so that merging the incidents from a one-leg route affects only the last leg of the receiver.
      - parameter legIndex: The index of a leg, from which to start applying the refreshed incidents.
      - parameter legShapeIndex: Index of a geometry of the `legIndex` leg, where to start refreshing from.
      */
     public func refreshLegIncidents(from refreshedRoute: RouteRefreshSource, legIndex: Int, legShapeIndex: Int) {
-        for (leg, refreshedLeg) in zip(legs[legIndex..<legIndex + refreshedRoute.refreshedLegs.count].enumerated(), refreshedRoute.refreshedLegs) {
-            let startIndex = leg.offset == 0 ? legShapeIndex : 0
-            leg.element.refreshIncidents(newIncidents: refreshedLeg.refreshedIncidents, startLegShapeIndex: startIndex)
+        let endRefreshIndex = legIndex + refreshedRoute.refreshedLegs.count
+        for (index, leg) in legs.enumerated() {
+            if (legIndex..<endRefreshIndex).contains(index) {
+                let refreshedLeg = refreshedRoute.refreshedLegs[index-legIndex]
+                let startIndex = index == legIndex ? legShapeIndex : 0
+                leg.refreshIncidents(newIncidents: refreshedLeg.refreshedIncidents, startLegShapeIndex: startIndex)
+            } else {
+                leg.incidents = nil
+            }
         }
     }
 }
diff --git a/Tests/MapboxDirectionsTests/RouteRefreshTests.swift b/Tests/MapboxDirectionsTests/RouteRefreshTests.swift
@@ -170,7 +170,7 @@ class RouteRefreshTests: XCTestCase {
 
                 let route = routeResponse.routes?[routeIndex]
                 let originalCongestions = route!.legs[0].attributes.segmentCongestionLevels!
-                let originalIncidents = route!.legs[0].incidents
+                let originalIncidents = route!.legs.map(\.incidents)
 
                 route?.refreshLegAttributes(from: refresh.route,
                                             legIndex: legIndex,
@@ -181,8 +181,8 @@ class RouteRefreshTests: XCTestCase {
 
                 let refreshCongestions = refresh.route.legs[0].attributes.segmentCongestionLevels!
                 let refreshedCongestions = route!.legs[0].attributes.segmentCongestionLevels!
-                let refreshIncidents = refresh.route.legs[0].incidents
-                let refreshedIncidents = route!.legs[0].incidents
+                let refreshIncidents = refresh.route.legs.map(\.incidents)
+                let refreshedIncidents = route!.legs.map(\.incidents)
 
                 XCTAssertEqual(originalCongestions[PartialRangeUpTo(geometryIndex)],
                                refreshedCongestions[PartialRangeUpTo(geometryIndex)],
@@ -197,9 +197,25 @@ class RouteRefreshTests: XCTestCase {
                 XCTAssertNotEqual(originalIncidents,
                                   refreshedIncidents,
                                   "Incidents should be refreshed")
-                XCTAssertEqual(refreshIncidents,
-                               refreshedIncidents,
-                               "Incidents are not refreshed")
+                for leg in zip(refreshIncidents, refreshedIncidents).enumerated() {
+                    let (new, updated) = leg.element
+                    if leg.offset == legIndex {
+                        XCTAssertEqual(new != nil, updated != nil)
+
+                        if let new = new, let updated = updated {
+                            for incident in zip(new, updated) {
+                                var offsetNewIncident = incident.0
+                                // refreshed ranges should be offset by leg shape index
+                                let startIndex = offsetNewIncident.shapeIndexRange.lowerBound + geometryIndex
+                                let endIndex = offsetNewIncident.shapeIndexRange.upperBound + geometryIndex
+                                offsetNewIncident.shapeIndexRange = startIndex..<endIndex
+                                XCTAssertEqual(offsetNewIncident, incident.1, "Incidents are not refreshed")
+                            }
+                        }
+                    } else {
+                        XCTAssertEqual(new, updated, "Incidents are not refreshed")
+                    }
+                }
 
                 routeUpdatedExpectation.fulfill()
             }