Make source ownership consistent and make MGLGeoJSONSource's content properties mutable

platform/darwin/src/MGLGeoJSONSource.h

@@ -109,7 +109,7 @@ extern NSString * const MGLGeoJSONToleranceOption;
  is set to `nil`. This property is unavailable until the receiver is passed into
  `-[MGLStyle addSource]`.
  */
-@property (nonatomic, readonly, nullable) NS_ARRAY_OF(id <MGLFeature>) *features;
+@property (nonatomic, nullable) NS_ARRAY_OF(id <MGLFeature>) *features;
 
 /**
  A GeoJSON representation of the contents of the source.
@@ -123,15 +123,15 @@ extern NSString * const MGLGeoJSONToleranceOption;
  This property is unavailable until the receiver is passed 
  into `-[MGLStyle addSource]`.
  */
-@property (nonatomic, readonly, nullable, copy) NSData *geoJSONData;
+@property (nonatomic, nullable, copy) NSData *geoJSONData;
 
 /**
  The URL to the GeoJSON document that specifies the contents of the source.
 
  If the receiver was initialized using `-initWithIdentifier:geoJSONData:options`, this
  property is set to `nil`.
  */
-@property (nonatomic, readonly, nullable) NSURL *URL;
+@property (nonatomic, nullable) NSURL *URL;
 
 
 @end

platform/darwin/src/MGLGeoJSONSource.mm

@@ -1,4 +1,4 @@
-#import "MGLGeoJSONSource.h"
+#import "MGLGeoJSONSource_Private.h"
 
 #import "MGLSource_Private.h"
 #import "MGLFeature_Private.h"
@@ -7,6 +7,7 @@
 
 #include <mbgl/style/sources/geojson_source.hpp>
 
+
 NSString * const MGLGeoJSONClusterOption = @"MGLGeoJSONCluster";
 NSString * const MGLGeoJSONClusterRadiusOption = @"MGLGeoJSONClusterRadius";
 NSString * const MGLGeoJSONClusterMaximumZoomLevelOption = @"MGLGeoJSONClusterMaximumZoomLevel";
@@ -17,6 +18,7 @@
 @interface MGLGeoJSONSource ()
 
 @property (nonatomic, readwrite) NSDictionary *options;
+@property (nonatomic) mbgl::style::GeoJSONSource *rawSource;
 
 @end
 
@@ -28,6 +30,7 @@ - (instancetype)initWithIdentifier:(NSString *)identifier geoJSONData:(NSData *)
     {
         _geoJSONData = data;
         _options = options;
+        [self commonInit];
     }
     return self;
 }
@@ -38,6 +41,7 @@ - (instancetype)initWithIdentifier:(NSString *)identifier URL:(NSURL *)url optio
     {
         _URL = url;
         _options = options;
+        [self commonInit];
     }
     return self;
 }
@@ -46,11 +50,39 @@ - (instancetype)initWithIdentifier:(NSString *)identifier features:(NSArray<id<M
     if (self = [super initWithIdentifier:identifier]) {
         _features = features;
         _options = options;
+        [self commonInit];
     }
 
     return self;
 }
 
+- (void)commonInit
+{
+    auto source = std::make_unique<mbgl::style::GeoJSONSource>(self.identifier.UTF8String, self.geoJSONOptions);
+
+    if (self.URL) {
+        NSURL *url = self.URL.mgl_URLByStandardizingScheme;
+        source->setURL(url.absoluteString.UTF8String);
+        _features = nil;
+    } else if (self.geoJSONData) {
+        NSString *string = [[NSString alloc] initWithData:self.geoJSONData encoding:NSUTF8StringEncoding];
+        const auto geojson = mapbox::geojson::parse(string.UTF8String).get<mapbox::geojson::feature_collection>();
+        source->setGeoJSON(geojson);
+        _features = MGLFeaturesFromMBGLFeatures(geojson);
+    } else {
+        mbgl::FeatureCollection featureCollection;
+        featureCollection.reserve(self.features.count);
+        for (id <MGLFeaturePrivate> feature in self.features) {
+            featureCollection.push_back([feature mbglFeature]);
+        }
+        const auto geojson = mbgl::GeoJSON{featureCollection};
+        source->setGeoJSON(geojson);
+        _features = MGLFeaturesFromMBGLFeatures(featureCollection);
+    }
+
+    self.pendingSource = std::move(source);
+}
+
 - (mbgl::style::GeoJSONOptions)geoJSONOptions
 {
     auto mbglOptions = mbgl::style::GeoJSONOptions();
@@ -102,30 +134,51 @@ - (void)validateValue:(id)value
     }
 }
 
-- (std::unique_ptr<mbgl::style::Source>)mbglSource
+- (void)setGeoJSONData:(NSData *)geoJSONData
 {
-    auto source = std::make_unique<mbgl::style::GeoJSONSource>(self.identifier.UTF8String, self.geoJSONOptions);
+    _geoJSONData = geoJSONData;
 
-    if (self.URL) {
-        NSURL *url = self.URL.mgl_URLByStandardizingScheme;
-        source->setURL(url.absoluteString.UTF8String);
-    } else if (self.geoJSONData) {
-        NSString *string = [[NSString alloc] initWithData:self.geoJSONData encoding:NSUTF8StringEncoding];
-        const auto geojson = mapbox::geojson::parse(string.UTF8String).get<mapbox::geojson::feature_collection>();
-        source->setGeoJSON(geojson);
-        _features = MGLFeaturesFromMBGLFeatures(geojson);
-    } else {
-        mbgl::FeatureCollection featureCollection;
-        featureCollection.reserve(self.features.count);
-        for (id <MGLFeaturePrivate> feature in self.features) {
-            featureCollection.push_back([feature mbglFeature]);
-        }
-        const auto geojson = mbgl::GeoJSON{featureCollection};
-        source->setGeoJSON(geojson);        
-        _features = MGLFeaturesFromMBGLFeatures(featureCollection);
+    if (self.rawSource == NULL)
+    {
+        [self commonInit];
+    }
+
+    NSString *string = [[NSString alloc] initWithData:_geoJSONData encoding:NSUTF8StringEncoding];
+    const auto geojson = mapbox::geojson::parse(string.UTF8String).get<mapbox::geojson::feature_collection>();
+    self.rawSource->setGeoJSON(geojson);
+
+    _features = MGLFeaturesFromMBGLFeatures(geojson);
+}
+
+- (void)setURL:(NSURL *)URL
+{
+    _URL = URL;
+
+    if (self.rawSource == NULL)
+    {
+        [self commonInit];
+    }
+
+    NSURL *url = self.URL.mgl_URLByStandardizingScheme;
+    self.rawSource->setURL(url.absoluteString.UTF8String);
+}
+
+- (void)setFeatures:(NSArray *)features
+{
+    if (self.rawSource == NULL)
+    {
+        [self commonInit];
+    }
+
+    mbgl::FeatureCollection featureCollection;
+    featureCollection.reserve(features.count);
+    for (id <MGLFeaturePrivate> feature in features) {
+        featureCollection.push_back([feature mbglFeature]);
     }
+    const auto geojson = mbgl::GeoJSON{featureCollection};
+    self.rawSource->setGeoJSON(geojson);
 
-    return std::move(source);
+    _features = MGLFeaturesFromMBGLFeatures(featureCollection);
 }
 
 @end

platform/darwin/src/MGLGeoJSONSource_Private.h

@@ -1,3 +1,4 @@
+#import "MGLGeoJSONSource.h"
 #import "MGLGeoJSONSource_Private.h"
 
 #include <mbgl/style/sources/geojson_source.hpp>

platform/darwin/src/MGLRasterSource.mm

@@ -6,13 +6,20 @@
 
 #include <mbgl/style/sources/raster_source.hpp>
 
+@interface MGLRasterSource ()
+
+@property (nonatomic) mbgl::style::RasterSource *rawSource;
+
+@end
+
 @implementation MGLRasterSource
 
 - (instancetype)initWithIdentifier:(NSString *)identifier URL:(NSURL *)url tileSize:(CGFloat)tileSize
 {
     if (self = [super initWithIdentifier:identifier]) {
         _URL = url;
         _tileSize = tileSize;
+        [self commonInit];
     }
     return self;
 }
@@ -23,11 +30,12 @@ - (instancetype)initWithIdentifier:(NSString *)identifier tileSet:(MGLTileSet *)
     {
         _tileSet = tileSet;
         _tileSize = tileSize;
+        [self commonInit];
     }
     return self;
 }
 
-- (std::unique_ptr<mbgl::style::Source>)mbglSource
+- (void)commonInit
 {
     std::unique_ptr<mbgl::style::RasterSource> source;
 
@@ -42,10 +50,9 @@ - (instancetype)initWithIdentifier:(NSString *)identifier tileSet:(MGLTileSet *)
         source = std::make_unique<mbgl::style::RasterSource>(self.identifier.UTF8String,
                                                              self.tileSet.mbglTileset,
                                                              uint16_t(self.tileSize));
-
     }
 
-    return std::move(source);
+    self.pendingSource = std::move(source);
 }
 
 @end

platform/darwin/src/MGLSource.mm

@@ -2,27 +2,30 @@
 
 #include <mbgl/style/source.hpp>
 
-@interface MGLSource ()
-
-@property (nonatomic) mbgl::style::Source *source;
-
-@end
-
 @implementation MGLSource
+{
+    std::unique_ptr<mbgl::style::Source> _pendingSource;
+}
 
-- (instancetype)initWithIdentifier:(NSString *)identifier {
+- (instancetype)initWithIdentifier:(NSString *)identifier
+{
     if (self = [super init]) {
         _identifier = identifier;
     }
     return self;
 }
 
-- (std::unique_ptr<mbgl::style::Source>)mbglSource {
-    [NSException raise:@"MGLAbstractClassException" format:
-     @"The source %@ cannot be added to the style. "
-     @"Make sure the source was created as a member of a concrete subclass of MGLSource.",
-     NSStringFromClass(self)];
-    return nil;
+- (std::unique_ptr<mbgl::style::Source>)pendingSource
+{
+    return std::move(_pendingSource);
+}
+
+- (void)setPendingSource:(std::unique_ptr<mbgl::style::Source>)pendingSource
+{
+    if (pendingSource != nullptr) {
+        self.rawSource = pendingSource.get();
+    }
+    _pendingSource = std::move(pendingSource);
 }
 
 @end

platform/darwin/src/MGLSource_Private.h

@@ -5,8 +5,24 @@
 
 @interface MGLSource (Private)
 
-- (std::unique_ptr<mbgl::style::Source>)mbglSource;
+/**
+ A raw pointer to the mbgl object, which is always initialized, either to the 
+ value returned by `mbgl::Map getSource`, or for independently created objects,
+ to the pointer value held in `pendingSource`. In the latter case, this raw
+ pointer value stays even after ownership of the object is transferred via 
+ `mbgl::Map addSource`.
+ */
+@property (nonatomic) mbgl::style::Source *rawSource;
 
-@property (nonatomic) mbgl::style::Source *source;
+/**
+ A std::unique_ptr<>, which is present only for objects that were created 
+ independently (not obtained by `mbgl::Map getSource`) and have not yet
+ been added via `mbgl::Map addSource`. Once a source is added, ownership of the 
+ object is transferred to the `mbgl::Map` and `pendingSource` is NULL. If the
+ `MGLSource` object's mbgl source is in that state, the mbgl source can still be
+ changed but the changes will not be visible until the `MGLSource` is added back
+ via `-[MGLStyle addSource:]` and styled with an `MGLLayer`.
+ */
+@property (nonatomic) std::unique_ptr<mbgl::style::Source> pendingSource;
 
 @end

platform/darwin/src/MGLStyle.mm

@@ -142,6 +142,7 @@ - (MGLStyleLayer *)layerWithIdentifier:(NSString *)identifier
 - (MGLSource *)sourceWithIdentifier:(NSString *)identifier
 {
     auto mbglSource = self.mapView.mbglMap->getSource(identifier.UTF8String);
+
     if (!mbglSource) {
         return nil;
     }
@@ -159,8 +160,8 @@ - (MGLSource *)sourceWithIdentifier:(NSString *)identifier
         NSAssert(NO, @"Unrecognized source type");
         return nil;
     }
-
-    source.source = mbglSource;
+    
+    source.rawSource = mbglSource;
 
     return source;
 }
@@ -205,12 +206,21 @@ - (void)insertLayer:(MGLStyleLayer *)layer belowLayer:(MGLStyleLayer *)otherLaye
 
 - (void)addSource:(MGLSource *)source
 {
-    self.mapView.mbglMap->addSource(source.mbglSource);
+    // After it is added, the pending source is moved to the mbgl map and is
+    // no longer "pending". The move operation sets it to NULL. However, the
+    // MGLSource's rawSource will still point to the mbgl source and clients
+    // can mutate it via that reference.
+    self.mapView.mbglMap->addSource(source.pendingSource);
 }
 
 - (void)removeSource:(MGLSource *)source
 {
     self.mapView.mbglMap->removeSource(source.identifier.UTF8String);
+
+    // Once a mbgl source is removed from the map, ownership does not return
+    // to the MGL source. Therefore, the rawSource pointer is set to NULL
+    // so that the implementation of MGL source can avoid using it again.
+    source.rawSource = NULL;
 }
 
 - (NS_ARRAY_OF(NSString *) *)styleClasses

platform/darwin/src/MGLVectorSource.mm

@@ -6,6 +6,12 @@
 
 #include <mbgl/style/sources/vector_source.hpp>
 
+@interface MGLVectorSource ()
+
+@property (nonatomic) mbgl::style::VectorSource *rawSource;
+
+@end
+
 @implementation MGLVectorSource
 
 static NSString *MGLVectorSourceType   = @"vector";
@@ -15,6 +21,7 @@ - (instancetype)initWithIdentifier:(NSString *)identifier URL:(NSURL *)url
     if (self = [super initWithIdentifier:identifier])
     {
         _URL = url;
+        [self commonInit];
     }
     return self;
 }
@@ -24,11 +31,12 @@ - (instancetype)initWithIdentifier:(NSString *)identifier tileSet:(MGLTileSet *)
     if (self = [super initWithIdentifier:identifier])
     {
         _tileSet = tileSet;
+        [self commonInit];
     }
     return self;
 }
 
-- (std::unique_ptr<mbgl::style::Source>)mbglSource
+- (void)commonInit
 {
     std::unique_ptr<mbgl::style::VectorSource> source;
 
@@ -43,7 +51,7 @@ - (instancetype)initWithIdentifier:(NSString *)identifier tileSet:(MGLTileSet *)
                                                              self.tileSet.mbglTileset);
     }
 
-    return std::move(source);
+    self.pendingSource = std::move(source);
 }
 
 @end