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

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

Merged
merged 1 commit into from
Dec 13, 2018

Conversation

alexshalamov
Copy link
Contributor

@alexshalamov alexshalamov commented Dec 11, 2018

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

Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the question about whether the test should be part of this PR, but otherwise 👍

@alexshalamov alexshalamov changed the title [ios, darwin] Check if MGLFeature.attributes is nil [ios, darwin] Check if MGLFeature.attributes is nil and add integration test Dec 11, 2018
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_for_13378 branch from d3a1d2b to dccc06f Compare December 11, 2018 16:28
@alexshalamov alexshalamov requested a review from a team December 11, 2018 16:28
platform/darwin/src/MGLFeature.h Outdated Show resolved Hide resolved
platform/darwin/src/MGLFeature.mm Outdated Show resolved Hide resolved
platform/ios/Integration Tests/MGLShapeSourceTests.m Outdated Show resolved Hide resolved
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_for_13378 branch 2 times, most recently from 1f34949 to 4863d0e Compare December 12, 2018 20:54
@alexshalamov alexshalamov changed the title [ios, darwin] Check if MGLFeature.attributes is nil and add integration test [ios, darwin] Make MGLFeature.attributes non-nullable and add integration test Dec 12, 2018
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I feel better about the integration test now that its timeout is only a second long, though it would be nice to have a unit test at some point. Would you do the honors and add iOS and macOS changelog entries along these lines?

Fixed a crash when specifying MGLShapeSourceOptionLineDistanceMetrics when creating an MGLShapeSource.

@alexshalamov alexshalamov force-pushed the alexshalamov_fix_for_13378 branch from 4863d0e to dbfe111 Compare December 13, 2018 12:17
…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
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_for_13378 branch from dbfe111 to b232873 Compare December 13, 2018 14:35
@alexshalamov alexshalamov merged commit b6b1067 into master Dec 13, 2018
@alexshalamov alexshalamov deleted the alexshalamov_fix_for_13378 branch December 13, 2018 17:39
@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS refactor macOS Mapbox Maps SDK for macOS labels Jan 2, 2019
@friedbunny friedbunny added this to the release-iowaska milestone Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants