Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[ios, darwin] Make MGLFeature.attributes non-nullable and add integra…
Browse files Browse the repository at this point in the history
…tion 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
  • Loading branch information
alexshalamov committed Dec 13, 2018
1 parent 2528601 commit b232873
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 15 deletions.
26 changes: 17 additions & 9 deletions platform/darwin/src/MGLFeature.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ @interface MGLEmptyFeature ()
@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);
Expand Down Expand Up @@ -60,11 +61,12 @@ @interface MGLPointFeature ()
@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);
Expand Down Expand Up @@ -96,11 +98,12 @@ @interface MGLPolylineFeature ()
@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);
Expand Down Expand Up @@ -133,11 +136,12 @@ @interface MGLPolygonFeature ()
@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);
Expand Down Expand Up @@ -170,11 +174,12 @@ @interface MGLPointCollectionFeature ()
@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);
Expand All @@ -197,11 +202,12 @@ @interface MGLMultiPolylineFeature ()
@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);
Expand Down Expand Up @@ -234,11 +240,12 @@ @interface MGLMultiPolygonFeature ()
@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);
Expand Down Expand Up @@ -271,7 +278,7 @@ @interface MGLShapeCollectionFeature ()
@implementation MGLShapeCollectionFeature

@synthesize identifier;
@synthesize attributes;
@synthesize attributes = _attributes;

@dynamic shapes;

Expand All @@ -282,6 +289,7 @@ + (instancetype)shapeCollectionWithShapes:(NSArray<MGLShape<MGLFeature> *> *)sha
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];
Expand Down Expand Up @@ -463,7 +471,7 @@ static CLLocationCoordinate2D toLocationCoordinate2D(const mbgl::Point<T> &point

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];
Expand Down
14 changes: 11 additions & 3 deletions platform/darwin/src/MGLFeature_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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; \
}
Expand All @@ -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() \
Expand All @@ -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; \
}
6 changes: 3 additions & 3 deletions platform/darwin/test/MGLFeatureTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ - (void)testPointFeatureGeoJSONDictionary {
// 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";
Expand Down Expand Up @@ -317,13 +317,13 @@ - (void)testShapeCollectionFeatureGeoJSONDictionary {
@"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",
}
]
Expand Down
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
37 changes: 37 additions & 0 deletions platform/ios/Integration Tests/MGLShapeSourceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,42 @@ - (void)testSettingShapeSourceToNilInRegionIsChanging {
[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
1 change: 1 addition & 0 deletions platform/macos/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit b232873

Please sign in to comment.