From dbfe111c3c7441cde8bd824d872f0d546c0b1add Mon Sep 17 00:00:00 2001 From: Alexander Shalamov Date: Tue, 11 Dec 2018 17:07:47 +0200 Subject: [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 --- platform/darwin/src/MGLFeature.mm | 26 +++++++++------ platform/darwin/src/MGLFeature_Private.h | 14 ++++++-- platform/darwin/test/MGLFeatureTests.mm | 6 ++-- platform/ios/CHANGELOG.md | 1 + .../ios/Integration Tests/MGLShapeSourceTests.m | 37 ++++++++++++++++++++++ 5 files changed, 69 insertions(+), 15 deletions(-) diff --git a/platform/darwin/src/MGLFeature.mm b/platform/darwin/src/MGLFeature.mm index 3b6a2ee488..d24c807625 100644 --- 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 *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]; diff --git a/platform/darwin/src/MGLFeature_Private.h b/platform/darwin/src/MGLFeature_Private.h index d4074b53ed..9fb1f91820 100644 --- 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 *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; \ + } diff --git a/platform/darwin/test/MGLFeatureTests.mm b/platform/darwin/test/MGLFeatureTests.mm index 3e8e47474b..67f2a9a45e 100644 --- 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", } ] diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 0244248b78..e1449b663e 100644 --- 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 diff --git a/platform/ios/Integration Tests/MGLShapeSourceTests.m b/platform/ios/Integration Tests/MGLShapeSourceTests.m index 088a9b011e..836118450e 100644 --- a/platform/ios/Integration Tests/MGLShapeSourceTests.m +++ b/platform/ios/Integration 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 -- cgit v1.2.1