[ios, macos] Make source removal consistent with layer removal

This refactors the source removal methods to make them consistent
with the way layers are removed. This makes removal of nonexistent
sources and removal of sources of a different type but same
identifier as a previously added source a no-ops.

As with layers, the check at the top of the method to ensure that the
raw pointer is the same as the one in mbgl for the same
identifier string should make it impossible to attempt to remove
a source of a different type than the one in mbgl for the same
identifier. However, for consistency with the layer implementation,
the reinterpret_cast has been replaced with a dynamic_cast and check
for nullptr.

4 files changed, 74 insertions, 13 deletions

diff --git a/platform/darwin/src/MGLRasterSource.mm b/platform/darwin/src/MGLRasterSource.mm
index 9f1d035994..c73a824ea8 100644
--- a/platform/darwin/src/MGLRasterSource.mm
+++ b/platform/darwin/src/MGLRasterSource.mm
@@ -82,12 +82,21 @@ static const CGFloat MGLRasterSourceRetinaTileSize = 512;
 }
 
 - (void)removeFromMapView:(MGLMapView *)mapView {
+    if (self.rawSource != mapView.mbglMap->getSource(self.identifier.UTF8String)) {
+        return;
+    }
+
     auto removedSource = mapView.mbglMap->removeSource(self.identifier.UTF8String);
 
-    if (removedSource) {
-        _pendingSource = std::move(reinterpret_cast<std::unique_ptr<mbgl::style::RasterSource> &>(removedSource));
-        self.rawSource = _pendingSource.get();
+    mbgl::style::RasterSource *source = dynamic_cast<mbgl::style::RasterSource *>(removedSource.get());
+    if (!source) {
+        return;
     }
+
+    removedSource.release();
+
+    _pendingSource = std::unique_ptr<mbgl::style::RasterSource>(source);
+    self.rawSource = _pendingSource.get();
 }
 
 - (mbgl::style::RasterSource *)rawSource {
diff --git a/platform/darwin/src/MGLShapeSource.mm b/platform/darwin/src/MGLShapeSource.mm
index d7d26ba36d..b37b01663f 100644
--- a/platform/darwin/src/MGLShapeSource.mm
+++ b/platform/darwin/src/MGLShapeSource.mm
@@ -86,12 +86,21 @@ const MGLShapeSourceOption MGLShapeSourceOptionSimplificationTolerance = @"MGLSh
 }
 
 - (void)removeFromMapView:(MGLMapView *)mapView {
+    if (self.rawSource != mapView.mbglMap->getSource(self.identifier.UTF8String)) {
+        return;
+    }
+
     auto removedSource = mapView.mbglMap->removeSource(self.identifier.UTF8String);
 
-    if (removedSource) {
-        _pendingSource = std::move(reinterpret_cast<std::unique_ptr<mbgl::style::GeoJSONSource> &>(removedSource));
-        self.rawSource = _pendingSource.get();
+    mbgl::style::GeoJSONSource *source = dynamic_cast<mbgl::style::GeoJSONSource *>(removedSource.get());
+    if (!source) {
+        return;
     }
+
+    removedSource.release();
+
+    _pendingSource = std::unique_ptr<mbgl::style::GeoJSONSource>(source);
+    self.rawSource = _pendingSource.get();
 }
 
 - (mbgl::style::GeoJSONSource *)rawSource {
diff --git a/platform/darwin/src/MGLVectorSource.mm b/platform/darwin/src/MGLVectorSource.mm
index 840382913d..94511900c1 100644
--- a/platform/darwin/src/MGLVectorSource.mm
+++ b/platform/darwin/src/MGLVectorSource.mm
@@ -56,12 +56,21 @@
 }
 
 - (void)removeFromMapView:(MGLMapView *)mapView {
+    if (self.rawSource != mapView.mbglMap->getSource(self.identifier.UTF8String)) {
+        return;
+    }
+
     auto removedSource = mapView.mbglMap->removeSource(self.identifier.UTF8String);
 
-    if (removedSource) {
-        _pendingSource = std::move(reinterpret_cast<std::unique_ptr<mbgl::style::VectorSource> &>(removedSource));
-        self.rawSource = _pendingSource.get();
+    mbgl::style::VectorSource *source = dynamic_cast<mbgl::style::VectorSource *>(removedSource.get());
+    if (!source) {
+        return;
     }
+
+    removedSource.release();
+
+    _pendingSource = std::unique_ptr<mbgl::style::VectorSource>(source);
+    self.rawSource = _pendingSource.get();
 }
 
 - (mbgl::style::VectorSource *)rawSource {
diff --git a/platform/darwin/test/MGLStyleTests.mm b/platform/darwin/test/MGLStyleTests.mm
index 4f6f5f0dba..36772e556d 100644
--- a/platform/darwin/test/MGLStyleTests.mm
+++ b/platform/darwin/test/MGLStyleTests.mm
@@ -191,6 +191,41 @@
     XCTAssertNotNil([self.style sourceWithIdentifier:vectorSource.identifier]);
 }
 
+- (void)testAddingSourceOfTypeABeforeSourceOfTypeBWithSameIdentifier {
+    // Add a raster source
+    MGLRasterSource *rasterSource = [[MGLRasterSource alloc] initWithIdentifier:@"some-identifier" tileURLTemplates:@[] options:nil];
+    [self.style addSource:rasterSource];
+
+    // Attempt to remove a shape source with the same identifier as the raster source
+    MGLShapeSource *shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"some-identifier" shape:nil options:nil];
+    [self.style removeSource:shapeSource];
+    // The raster source should still be added
+    XCTAssertTrue([[self.style sourceWithIdentifier:rasterSource.identifier] isMemberOfClass:[MGLRasterSource class]]);
+
+    // Remove the raster source
+    [self.style removeSource:rasterSource];
+
+    // Add the shape source
+    [self.style addSource:shapeSource];
+
+    // Attempt to remove a vector source with the same identifer as the shape source
+    MGLVectorSource *vectorSource = [[MGLVectorSource alloc] initWithIdentifier:@"some-identifier" tileURLTemplates:@[] options:nil];
+    [self.style removeSource:vectorSource];
+    // The shape source should still be added
+    XCTAssertTrue([[self.style sourceWithIdentifier:shapeSource.identifier] isMemberOfClass:[MGLShapeSource class]]);
+
+    // Remove the shape source
+    [self.style removeSource:shapeSource];
+
+    // Add the vector source
+    [self.style addSource:vectorSource];
+
+    // Attempt to remove the previously created raster source that has the same identifer as the shape source
+    [self.style removeSource:rasterSource];
+    // The vector source should still be added
+    XCTAssertTrue([[self.style sourceWithIdentifier:shapeSource.identifier] isMemberOfClass:[MGLVectorSource class]]);
+}
+
 - (void)testLayers {
     NSArray<MGLStyleLayer *> *initialLayers = self.style.layers;
     if ([initialLayers.firstObject.identifier isEqualToString:@"com.mapbox.annotations.points"]) {
@@ -252,7 +287,6 @@
     XCTAssertThrowsSpecificNamed([self.style insertLayer:[[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"my-layer"] atIndex:0], NSException, @"MGLRedundantLayerIdentifierException");
 }
 
-
 - (void)testRemovingLayerBeforeAddingSameLayer {
     MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-removing-before-adding" shape:nil options:nil];
 
@@ -291,14 +325,14 @@
 }
 
 - (void)testAddingLayerOfTypeABeforeRemovingLayerOfTypeBWithSameIdentifier {
-    MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-removing-before-adding" shape:nil options:nil];
+    MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"shape-source-identifier" shape:nil options:nil];
     [self.style addSource:source];
 
-    // Add a fill layer with an identifer
+    // Add a fill layer
     MGLFillStyleLayer *fillLayer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"some-identifier" source:source];
     [self.style addLayer:fillLayer];
 
-    // Remove a line layer (before adding it) with the same identifier as the fill layer previously added
+    // Attempt to remove a line layer with the same identifier as the fill layer
     MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:fillLayer.identifier source:source];
     [self.style removeLayer:lineLayer];