Add tests, update changelog and formatting for final destination

kried · kried · commit 9e29e97724c6 · 2023-03-08T17:42:13.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,27 +16,14 @@
 ### Map
 
 * Fixed an issue when `NavigationMapView` would not display a map, when using CarPlay and custom tile storage location. ([#4399](https://github.com/mapbox/mapbox-navigation-ios/pull/4399))
+* Added `NavigationMapView.addDestinationAnnotation(_:identifier:styleLoaded:)`, `NavigationMapView.removeDestinationAnnotation(_:)` to present and remove the final destination annotation on a NavigationMapView. ([#4253](https://github.com/mapbox/mapbox-navigation-ios/pull/4253))
 
 ### Camera
 
 * Fixes an issue where the camera's position was not calculated correctly when banners fully overlapped the map. ([#4400](https://github.com/mapbox/mapbox-navigation-ios/pull/4400))
 
 ## v2.11.0
 
-### Packaging
-
-* MapboxCoreNavigation now requires [MapboxNavigationNative v126._x_](https://github.com/mapbox/mapbox-navigation-native-ios/releases/tag/126.0.0). ([#4367](https://github.com/mapbox/mapbox-navigation-ios/pull/4367))
-* MapboxCoreNavigation now requires [MapboxDirections v2.10.0-rc.1](https://github.com/mapbox/mapbox-directions-swift/releases/tag/v2.10.0-rc.1). ([#4367](https://github.com/mapbox/mapbox-navigation-ios/pull/4367))
-* MapboxNavigation now requires [MapboxMaps v10.11.0](https://github.com/mapbox/mapbox-maps-ios/releases/tag/v10.11.0). ([#4367](https://github.com/mapbox/mapbox-navigation-ios/pull/4367))
-
-### Visual instructions
-* `MapboxRoadNameView` now renders localized road names. ([#4375](https://github.com/mapbox/mapbox-navigation-ios/pull/4375))
-* Added `localizedRoadNameKey` to the user info dictionary of `Notification.Name.passiveLocationManagerDidUpdate` posted by `PassiveLocationManager`, and the `Notification.Name.currentRoadNameDidChange` posted by `RouteController`. The corresponding string value contains localized road name. ([#4375](https://github.com/mapbox/mapbox-navigation-ios/pull/4375))
-
-### Camera
-
-* Fixes an issue where the camera padding was not calculated correctly when the map view didn't take the whole screen. ([#4350](https://github.com/mapbox/mapbox-navigation-ios/pull/4350))
-
 ### CarPlay
 
 * Added `CarPlayManagerDelegate.carPlayManagerDidCancelPreview(_:)` to notify developers after CarPlay canceled routes preview, and `CarPlayManager.cancelRoutesPreview()` method to cancel routes preview on CarPlay. ([#4311](https://github.com/mapbox/mapbox-navigation-ios/pull/4311))
diff --git a/MapboxNavigation-SPM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/MapboxNavigation-SPM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -24,17 +24,17 @@
         "repositoryURL": "https://github.com/mapbox/mapbox-common-ios.git",
         "state": {
           "branch": null,
-          "revision": "185d791e7478cc5da7cc2b4756ecd0d0bd387eac",
-          "version": "23.4.0-beta.1"
+          "revision": "d38abed1cfca3843308d478450c3d8b07a797c5a",
+          "version": "23.4.0-rc.1"
         }
       },
       {
         "package": "MapboxCoreMaps",
         "repositoryURL": "https://github.com/mapbox/mapbox-core-maps-ios.git",
         "state": {
           "branch": null,
-          "revision": "81b929488b5bbc467df4cf294ceffa0ae98ef0e0",
-          "version": "10.12.0-beta.1"
+          "revision": "82706a65bf7a51818d943c91f014d17588b0f65e",
+          "version": "10.12.0-rc.1"
         }
       },
       {
@@ -60,8 +60,8 @@
         "repositoryURL": "https://github.com/mapbox/mapbox-maps-ios.git",
         "state": {
           "branch": null,
-          "revision": "246fa104e4eec0fb913c036cb0505723e26a9a2e",
-          "version": "10.12.0-beta.1"
+          "revision": "e47d5d6f2b55b0decd9d60b47b3c9c9b6c726fa3",
+          "version": "10.12.0-rc.1"
         }
       },
       {
@@ -123,8 +123,8 @@
         "repositoryURL": "https://github.com/apple/swift-argument-parser",
         "state": {
           "branch": null,
-          "revision": "fee6933f37fde9a5e12a1e4aeaa93fe60116ff2a",
-          "version": "1.2.2"
+          "revision": "fddd1c00396eed152c45a46bea9f47b98e59301d",
+          "version": "1.2.0"
         }
       },
       {
diff --git a/Sources/MapboxNavigation/NavigationMapView.swift b/Sources/MapboxNavigation/NavigationMapView.swift
@@ -1273,7 +1273,7 @@ open class NavigationMapView: UIView {
      final destination `PointAnnotation` will be stored in this property and added to the `MapView`
      later on.
      */
-    var finalDestinationAnnotations: [PointAnnotation] = []
+    private(set) var finalDestinationAnnotations: [PointAnnotation] = []
     
     /**
      Adds the route waypoints to the map given the current leg index. Previous waypoints for completed legs will be omitted.
@@ -1335,31 +1335,25 @@ open class NavigationMapView: UIView {
         
         if let lastLeg = route.legs.last,
            let destinationCoordinate = lastLeg.destination?.coordinate {
-            addDestinationAnnotation(destinationCoordinate) { [weak self] in
-                guard self != nil else { return }
-            }
+            addDestinationAnnotation(destinationCoordinate, identifier: AnnotationIdentifier.finalDestinationAnnotation)
         }
     }
-    
+
     /**
-     Adds a final destination annotation to the map.
-     
+     Adds a final destination annotation to the map.  The annotation will be added only after fully loading `MapView` style.  In such case
+     final destination will be stored and added to the `MapView` later on. `delegate` will be notified about the change via
+     `NavigationMapViewDelegate.navigationMapView(_:didAdd:pointAnnotationManager:)` method.
+
      - parameter coordinate: Coordinate which represents the annotation location.
      - parameter identifier: String to uniquely identify the destination annotation. Defaults to `nil` and a default identifier will be provided.
-     - parameter styleLoaded: An escaping closure to be executed when the `MapView` style has finished loading.
      */
     public func addDestinationAnnotation(_ coordinate: CLLocationCoordinate2D,
-                                         identifier: String? = nil,
-                                         styleLoaded: @escaping () -> Void) {
-        let identifier = identifier ?? String("finalDestinationAnnotation_\(finalDestinationAnnotations.count)")
+                                         identifier: String? = nil) {
+        let count = pointAnnotationManager?.annotations.count ?? finalDestinationAnnotations.count
+        let identifier = identifier ?? "\(AnnotationIdentifier.finalDestinationAnnotation)_\(count)"
         var destinationAnnotation = PointAnnotation(id: identifier, coordinate: coordinate)
         destinationAnnotation.image = .init(image: .defaultMarkerImage, name: ImageIdentifier.markerImage)
-                
-        mapView.mapboxMap.onNext(event: .styleLoaded) { [weak self] _ in
-            guard self != nil else { return }
-            styleLoaded()
-        }
-        
+
         // If `PointAnnotationManager` is available - add `PointAnnotation`, if not - remember it
         // and add it only after fully loading `MapView` style.
         if let pointAnnotationManager = pointAnnotationManager {
@@ -1373,18 +1367,20 @@ open class NavigationMapView: UIView {
     }
     
     /**
-     Removes a final destination annotation to the map.
-     
-     - parameter identifier: String to uniquely identify the destination annotation to be removed. Defaults to `nil` and removes all destination annotations.
+     Removes a final destination annotation from the map.
+
+     - parameter identifier: String to uniquely identify the destination annotation to be removed. Defaults to `nil` and removes all destination annotations with non-custom identifiers.
      */
-    public func removeDestinationAnnotation(_ identifier: String? = nil) {
-        if let identifier {
-            finalDestinationAnnotations.removeAll(where: { $0.id == identifier })
-            pointAnnotationManager?.annotations.removeAll(where: { $0.id == identifier })
+    public func removeDestinationAnnotation(identifier: String? = nil) {
+        let filter: (PointAnnotation) -> Bool
+        if let identifier = identifier {
+            filter = { $0.id == identifier }
         } else {
-            finalDestinationAnnotations.removeAll(where: { $0.id.contains("finalDestinationAnnotation") })
-            pointAnnotationManager?.annotations.removeAll(where: { $0.id.contains("finalDestinationAnnotation") })
+            filter = { $0.id.contains(AnnotationIdentifier.finalDestinationAnnotation) }
         }
+
+        finalDestinationAnnotations.removeAll(where: filter)
+        pointAnnotationManager?.annotations.removeAll(where: filter)
     }
     
     /**
@@ -1456,11 +1452,6 @@ open class NavigationMapView: UIView {
      */
     public var pointAnnotationManager: PointAnnotationManager?
     
-    func annotationsToRemove() -> [Annotation] {
-        let identifier = NavigationMapView.AnnotationIdentifier.finalDestinationAnnotation
-        return pointAnnotationManager?.annotations.filter({ $0.id == identifier }) ?? []
-    }
-    
     // MARK: Map Rendering and Observing
     
     var routes: [Route]?
diff --git a/Tests/MapboxNavigationTests/NavigationMapViewTests.swift b/Tests/MapboxNavigationTests/NavigationMapViewTests.swift
@@ -2,7 +2,7 @@ import XCTest
 import MapboxDirections
 import TestHelper
 import Turf
-import MapboxMaps
+@testable import MapboxMaps
 @testable import MapboxNavigation
 @testable import MapboxCoreNavigation
 
@@ -13,6 +13,7 @@ class NavigationMapViewTests: TestCase {
     ]))
     var navigationMapView: NavigationMapView!
     var navigator: CoreNavigator!
+    var pointAnnotationManager: PointAnnotationManager!
     
     let options: NavigationRouteOptions = .init(coordinates: [
         CLLocationCoordinate2D(latitude: 40.311012, longitude: -112.47926),
@@ -22,11 +23,34 @@ class NavigationMapViewTests: TestCase {
         let route = response.routes!.first!
         return route
     }()
+
+    private let coordinate1 = CLLocationCoordinate2D(latitude: 40.311012, longitude: -112.47926)
+    private let coordinate2 = CLLocationCoordinate2D(latitude: 30.176322, longitude: -102.806108)
+    private let finalDestinationAnnotationTestPrefix = "MapboxNavigation-MapboxNavigation-resources_finalDestinationAnnotation"
+
+    private final class DisplayLinkCoordinatorSpy: DisplayLinkCoordinator {
+        func add(_ participant: DisplayLinkParticipant) {}
+        func remove(_ participant: DisplayLinkParticipant) {}
+    }
+
+    private final class AnnotationImagesManagerSpy: AnnotationImagesManagerProtocol {
+        func addImage(_ image: UIImage, id: String, sdf: Bool, contentInsets: UIEdgeInsets) {}
+        func removeImage(_ imageName: String) {}
+        func register(imagesConsumer: MapboxMaps.AnnotationImagesConsumer) {}
+        func unregister(imagesConsumer: MapboxMaps.AnnotationImagesConsumer) {}
+    }
     
     override func setUp() {
         navigator = Navigator.shared
         super.setUp()
         navigationMapView = NavigationMapView(frame: CGRect(origin: .zero, size: .iPhone6Plus))
+        let mapboxMap = navigationMapView.mapView.mapboxMap!
+        pointAnnotationManager = PointAnnotationManager(id: "",
+                                                        style: mapboxMap.style,
+                                                        layerPosition: nil,
+                                                        displayLinkCoordinator: DisplayLinkCoordinatorSpy(),
+                                                        imagesManager: AnnotationImagesManagerSpy(),
+                                                        offsetPointCalculator: OffsetPointCalculator(mapboxMap: mapboxMap))
     }
     
     override func tearDown() {
@@ -587,6 +611,82 @@ class NavigationMapViewTests: TestCase {
         XCTAssertFalse(style.layerExists(withId: NavigationMapView.LayerIdentifier.intersectionAnnotationsLayer))
     }
 
+    func testAddFinalDestinationWithDefaultIdentifierIfStyleNotLoaded() {
+        XCTAssertTrue(navigationMapView.finalDestinationAnnotations.isEmpty)
+        XCTAssertTrue(pointAnnotationManager.annotations.isEmpty)
+
+        navigationMapView.addDestinationAnnotation(coordinate1)
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 1)
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations[0].id, "\(finalDestinationAnnotationTestPrefix)_0")
+        XCTAssertTrue(pointAnnotationManager.annotations.isEmpty)
+
+        navigationMapView.addDestinationAnnotation(coordinate2)
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 2)
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations[1].id, "\(finalDestinationAnnotationTestPrefix)_1")
+        XCTAssertTrue(pointAnnotationManager.annotations.isEmpty)
+    }
+
+    func testAddFinalDestinationWithCustomIdentifierIfStyleNotLoaded() {
+        navigationMapView.addDestinationAnnotation(coordinate1, identifier: "custom")
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 1)
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations[0].id, "custom")
+    }
+
+    func testRemovesFinalDestinationIfStyleNotLoaded() {
+        navigationMapView.addDestinationAnnotation(coordinate1)
+        navigationMapView.addDestinationAnnotation(coordinate2)
+
+        navigationMapView.removeDestinationAnnotation(identifier: "custom")
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 2)
+
+        navigationMapView.addDestinationAnnotation(coordinate2, identifier: "custom")
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 3)
+        navigationMapView.removeDestinationAnnotation(identifier: "custom")
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 2)
+
+        navigationMapView.removeDestinationAnnotation()
+        XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 0)
+    }
+
+    func testAddFinalDestinationWithDefaultIdentifierIfStyleLoaded() {
+        navigationMapView.pointAnnotationManager = pointAnnotationManager
+
+        navigationMapView.addDestinationAnnotation(coordinate1)
+        XCTAssertTrue(navigationMapView.finalDestinationAnnotations.isEmpty)
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 1)
+        XCTAssertEqual(pointAnnotationManager.annotations[0].id, "\(finalDestinationAnnotationTestPrefix)_0")
+
+        navigationMapView.addDestinationAnnotation(coordinate2)
+        XCTAssertTrue(navigationMapView.finalDestinationAnnotations.isEmpty)
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 2)
+        XCTAssertEqual(pointAnnotationManager.annotations[1].id, "\(finalDestinationAnnotationTestPrefix)_1")
+    }
+
+    func testAddFinalDestinationWithCustomIdentifierIfStyleLoaded() {
+        navigationMapView.pointAnnotationManager = pointAnnotationManager
+
+        navigationMapView.addDestinationAnnotation(coordinate1, identifier: "custom")
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 1)
+        XCTAssertEqual(pointAnnotationManager.annotations[0].id, "custom")
+    }
+
+    func testRemovesFinalDestinationIfStyleLoaded() {
+        navigationMapView.pointAnnotationManager = pointAnnotationManager
+        navigationMapView.addDestinationAnnotation(coordinate1)
+        navigationMapView.addDestinationAnnotation(coordinate2)
+
+        navigationMapView.removeDestinationAnnotation(identifier: "custom")
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 2)
+
+        navigationMapView.addDestinationAnnotation(coordinate2, identifier: "custom")
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 3)
+        navigationMapView.removeDestinationAnnotation(identifier: "custom")
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 2)
+
+        navigationMapView.removeDestinationAnnotation()
+        XCTAssertEqual(pointAnnotationManager.annotations.count, 0)
+    }
+
     private func configureIntersections() {
         let style = navigationMapView.mapView.mapboxMap.style
         var source = GeoJSONSource()

Original file line number	Diff line number	Diff line change
`@@ -24,17 +24,17 @@`
`24`	`24`	`"repositoryURL": "https://github.com/mapbox/mapbox-common-ios.git",`
`25`	`25`	`"state": {`
`26`	`26`	`"branch": null,`
`27`		`- "revision": "185d791e7478cc5da7cc2b4756ecd0d0bd387eac",`
`28`		`- "version": "23.4.0-beta.1"`
	`27`	`+ "revision": "d38abed1cfca3843308d478450c3d8b07a797c5a",`
	`28`	`+ "version": "23.4.0-rc.1"`
`29`	`29`	`}`
`30`	`30`	`},`
`31`	`31`	`{`
`32`	`32`	`"package": "MapboxCoreMaps",`
`33`	`33`	`"repositoryURL": "https://github.com/mapbox/mapbox-core-maps-ios.git",`
`34`	`34`	`"state": {`
`35`	`35`	`"branch": null,`
`36`		`- "revision": "81b929488b5bbc467df4cf294ceffa0ae98ef0e0",`
`37`		`- "version": "10.12.0-beta.1"`
	`36`	`+ "revision": "82706a65bf7a51818d943c91f014d17588b0f65e",`
	`37`	`+ "version": "10.12.0-rc.1"`
`38`	`38`	`}`
`39`	`39`	`},`
`40`	`40`	`{`
`@@ -60,8 +60,8 @@`
`60`	`60`	`"repositoryURL": "https://github.com/mapbox/mapbox-maps-ios.git",`
`61`	`61`	`"state": {`
`62`	`62`	`"branch": null,`
`63`		`- "revision": "246fa104e4eec0fb913c036cb0505723e26a9a2e",`
`64`		`- "version": "10.12.0-beta.1"`
	`63`	`+ "revision": "e47d5d6f2b55b0decd9d60b47b3c9c9b6c726fa3",`
	`64`	`+ "version": "10.12.0-rc.1"`
`65`	`65`	`}`
`66`	`66`	`},`
`67`	`67`	`{`
`@@ -123,8 +123,8 @@`
`123`	`123`	`"repositoryURL": "https://github.com/apple/swift-argument-parser",`
`124`	`124`	`"state": {`
`125`	`125`	`"branch": null,`
`126`		`- "revision": "fee6933f37fde9a5e12a1e4aeaa93fe60116ff2a",`
`127`		`- "version": "1.2.2"`
	`126`	`+ "revision": "fddd1c00396eed152c45a46bea9f47b98e59301d",`
	`127`	`+ "version": "1.2.0"`
`128`	`128`	`}`
`129`	`129`	`},`
`130`	`130`	`{`