[ios, darwin] Make MGLFeature.attributes non-nullable and add integration test

Enforce non-nullable semantics for MGLFeature.attributes, to avoid construction of invalid mbgl::Feature properties from nil NSDictionary object and align with public SDK property definition. Integration test "testShapeSourceWithLineDistanceMetrics" is added to verify that MGLFeature is correctly converted. Fixes issue #13378
2018-12-11 17:07:47 +02:00
parent 1d410c94fc
commit b6b1067caf
6 changed files with 70 additions and 15 deletions
--- a/platform/darwin/src/MGLFeature.mm
+++ b/platform/darwin/src/MGLFeature.mm
@@ -25,11 +25,12 @@
@implementation MGLEmptyFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -60,11 +61,12 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLPointFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -96,11 +98,12 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLPolylineFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -133,11 +136,12 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLPolygonFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -170,11 +174,12 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLPointCollectionFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -197,11 +202,12 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLMultiPolylineFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -234,11 +240,12 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLMultiPolygonFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    MGLLogDebug(@"Retrieving attributeForKey: %@", key);
@@ -271,7 +278,7 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
@implementation MGLShapeCollectionFeature

@synthesize identifier;
-@synthesize attributes;
+@synthesize attributes = _attributes;

@dynamic shapes;

@@ -282,6 +289,7 @@ MGL_DEFINE_FEATURE_IS_EQUAL();
 MGL_DEFINE_FEATURE_INIT_WITH_CODER();
 MGL_DEFINE_FEATURE_ENCODE();
 MGL_DEFINE_FEATURE_IS_EQUAL();
+MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER();

 - (id)attributeForKey:(NSString *)key {
    return self.attributes[key];
@@ -463,7 +471,7 @@ mbgl::Feature mbglFeature(mbgl::Feature feature, id identifier, NSDictionary *at

 NSDictionary<NSString *, id> *NSDictionaryFeatureForGeometry(NSDictionary *geometry, NSDictionary *attributes, id identifier) {
    NSMutableDictionary *feature = [@{@"type": @"Feature",
-                                      @"properties": (attributes) ?: [NSNull null],
+                                      @"properties": attributes,
                                      @"geometry": geometry} mutableCopy];
    feature[@"id"] = identifier;
    return [feature copy];
--- a/platform/darwin/src/MGLFeature_Private.h
+++ b/platform/darwin/src/MGLFeature_Private.h
@@ -31,7 +31,7 @@ MGLShape* MGLShapeFromGeoJSON(const mapbox::geojson::geojson &geojson);
 returns the feature object with converted `mbgl::FeatureIdentifier` and
 `mbgl::PropertyMap` properties.
 */
-mbgl::Feature mbglFeature(mbgl::Feature feature, id identifier, NSDictionary *attributes);
+mbgl::Feature mbglFeature(mbgl::Feature feature, id identifier, NSDictionary * attributes);

 /**
 Returns an `NSDictionary` representation of an `MGLFeature`.
@@ -45,7 +45,7 @@ NS_ASSUME_NONNULL_END
        if (self = [super initWithCoder:decoder]) { \
            NSSet<Class> *identifierClasses = [NSSet setWithArray:@[[NSString class], [NSNumber class]]]; \
            identifier = [decoder decodeObjectOfClasses:identifierClasses forKey:@"identifier"]; \
-            attributes = [decoder decodeObjectOfClass:[NSDictionary class] forKey:@"attributes"]; \
+            _attributes = [decoder decodeObjectOfClass:[NSDictionary class] forKey:@"attributes"]; \
        } \
        return self; \
    }
@@ -54,7 +54,7 @@ NS_ASSUME_NONNULL_END
    - (void)encodeWithCoder:(NSCoder *)coder { \
        [super encodeWithCoder:coder]; \
        [coder encodeObject:identifier forKey:@"identifier"]; \
-        [coder encodeObject:attributes forKey:@"attributes"]; \
+        [coder encodeObject:_attributes forKey:@"attributes"]; \
    }

 #define MGL_DEFINE_FEATURE_IS_EQUAL() \
@@ -67,3 +67,11 @@ NS_ASSUME_NONNULL_END
    - (NSUInteger)hash { \
        return [super hash] + [[self geoJSONDictionary] hash]; \
    }
+
+#define MGL_DEFINE_FEATURE_ATTRIBUTES_GETTER() \
+    - (NSDictionary *) attributes { \
+        if (!_attributes) { \
+            return @{}; \
+        } \
+        return _attributes; \
+    }
--- a/platform/darwin/test/MGLFeatureTests.mm
+++ b/platform/darwin/test/MGLFeatureTests.mm
@@ -176,7 +176,7 @@
    // it has no "id" key (or value)
    XCTAssertNil(geoJSONFeature[@"id"]);
    // it has a null representation of the properties object
-    XCTAssertEqualObjects(geoJSONFeature[@"properties"], [NSNull null]);
+    XCTAssertEqualObjects(geoJSONFeature[@"properties"], @{});

    // when there is a string identifier
    pointFeature.identifier = @"string-id";
@@ -317,13 +317,13 @@
                                       @"geometries": @[
                                           @{ @"geometry": @{@"type": @"Point",
                                                             @"coordinates": @[@(pointCoordinate.longitude), @(pointCoordinate.latitude)]},
-                                              @"properties": [NSNull null],
+                                              @"properties": @{},
                                              @"type": @"Feature",
                                             },
                                            @{ @"geometry": @{@"type": @"LineString",
                                                              @"coordinates": @[@[@(coord1.longitude), @(coord1.latitude)],
                                                                                @[@(coord2.longitude), @(coord2.latitude)]]},
-                                               @"properties": [NSNull null],
+                                               @"properties": @{},
                                               @"type": @"Feature",
                                            }
                                        ]
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -29,6 +29,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Renamed `-[MGLOfflineStorage putResourceWithUrl:data:modified:expires:etag:mustRevalidate:]` to `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:]`. ([#13318](https://github.com/mapbox/mapbox-gl-native/pull/13318))
 * Added `MGLLoggingConfiguration` and `MGLLoggingBlockHandler` that handle error and fault events produced by the SDK. ([#13235](https://github.com/mapbox/mapbox-gl-native/pull/13235))
 * Fixed random crashes during app termination. ([#13367](https://github.com/mapbox/mapbox-gl-native/pull/13367))
+* Fixed a crash when specifying MGLShapeSourceOptionLineDistanceMetrics when creating an MGLShapeSource. ([#13543](https://github.com/mapbox/mapbox-gl-native/pull/13543))

 ## 4.6.0 - November 7, 2018

--- a/Tests/MGLShapeSourceTests.m
+++ b/Tests/MGLShapeSourceTests.m
@@ -122,5 +122,42 @@
    [self waitForExpectations:@[expectation] timeout:1.0];
 }

+- (void)testShapeSourceWithLineDistanceMetrics {
+    CLLocationCoordinate2D coordinates[] = {
+        CLLocationCoordinate2DMake(9.6315313, 52.4133574),
+        CLLocationCoordinate2DMake(24.9410248, 60.1733244)};
+
+    MGLPolylineFeature *polylineFeature = [MGLPolylineFeature polylineWithCoordinates:coordinates count:sizeof(coordinates)/sizeof(coordinates[0])];
+    NSDictionary *options = @{MGLShapeSourceOptionLineDistanceMetrics: @YES};
+    MGLShapeSource *source = [[MGLShapeSource alloc] initWithIdentifier:@"route" shape:polylineFeature options:options];
+    MGLLineStyleLayer *lineLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:@"lineLayer" source:source];
+
+    [self.style addSource:source];
+    [self.style addLayer:lineLayer];
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(9.6315313, 52.4133574) animated:YES];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 1;
+    expectation.assertForOverFulfill = YES;
+
+    __weak id weakself = self;
+    self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
+
+        id strongSelf = weakself;
+        if (!strongSelf)
+            return;
+
+        NSArray *features = [source featuresMatchingPredicate:nil];
+        MGLTestAssert(strongSelf, features.count == 1UL, @"Should contain one Feature");
+
+        MGLPolylineFeature *feature = [features objectAtIndex:0];
+        MGLTestAssertNotNil(strongSelf, [feature.attributes objectForKey:@"mapbox_clip_start"], @"Attributes should contain mapbox_clip_start property");
+        MGLTestAssertNotNil(strongSelf, [feature.attributes objectForKey:@"mapbox_clip_end"], @"Attributes should contain mapbox_clip_end property");
+
+        [expectation fulfill];
+    };
+
+    [self waitForExpectations:@[expectation] timeout:1.0];
+}

@end
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -17,6 +17,7 @@
 * Renamed `-[MGLOfflineStorage putResourceWithUrl:data:modified:expires:etag:mustRevalidate:]` to `-[MGLOfflineStorage preloadData:forURL:modificationDate:expirationDate:eTag:mustRevalidate:]`. ([#13318](https://github.com/mapbox/mapbox-gl-native/pull/13318))
 * `MGLMapSnapshotter` now respects the `MGLIdeographicFontFamilyName` key in Info.plist, which reduces bandwidth consumption when snapshotting regions that contain Chinese or Japanese characters. ([#13427](https://github.com/mapbox/mapbox-gl-native/pull/13427))
 * Added `MGLLoggingConfiguration` and `MGLLoggingBlockHandler` that handle error and fault events produced by the SDK. ([#13235](https://github.com/mapbox/mapbox-gl-native/pull/13235))
+* Fixed a crash when specifying MGLShapeSourceOptionLineDistanceMetrics when creating an MGLShapeSource. ([#13543](https://github.com/mapbox/mapbox-gl-native/pull/13543))

 ## 0.12.0 - November 8, 2018