diff options
author | Justin R. Miller <incanus@users.noreply.github.com> | 2016-09-16 11:46:22 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-09-16 11:46:22 -0700 |
commit | 0bd66d40ddf9e75f860fe18e7c80de9c840f48ac (patch) | |
tree | 43e4509dca2ad447cab14454b1da8a7bdfe445be | |
parent | 767aedc6766d1dd277f674ae54894cfcf21a437a (diff) | |
download | qtlocation-mapboxgl-0bd66d40ddf9e75f860fe18e7c80de9c840f48ac.tar.gz |
[ios] fixes #6248: properly handle all NSNumber types in filters (#6290)
-rw-r--r-- | platform/darwin/src/NSExpression+MGLAdditions.mm | 33 | ||||
-rw-r--r-- | platform/darwin/test/MGLExpressionTests.mm | 200 | ||||
-rw-r--r-- | platform/ios/ios.xcodeproj/project.pbxproj | 6 | ||||
-rw-r--r-- | platform/macos/macos.xcodeproj/project.pbxproj | 4 |
4 files changed, 236 insertions, 7 deletions
diff --git a/platform/darwin/src/NSExpression+MGLAdditions.mm b/platform/darwin/src/NSExpression+MGLAdditions.mm index 129c9a740c..52a0b9bd88 100644 --- a/platform/darwin/src/NSExpression+MGLAdditions.mm +++ b/platform/darwin/src/NSExpression+MGLAdditions.mm @@ -27,12 +27,33 @@ return { std::string([(NSString *)value UTF8String]) }; } else if ([value isKindOfClass:NSNumber.class]) { NSNumber *number = (NSNumber *)value; - if((strcmp([number objCType], @encode(int))) == 0) { - return { number.intValue }; - } else if ((strcmp([number objCType], @encode(double))) == 0) { - return { number.doubleValue }; - } else if ((strcmp([number objCType], @encode(char))) == 0) { - return { number.boolValue }; + if ((strcmp([number objCType], @encode(char)) == 0) || + (strcmp([number objCType], @encode(BOOL)) == 0)) { + // char: 32-bit boolean + // BOOL: 64-bit boolean + return { (bool)number.boolValue }; + } else if (strcmp([number objCType], @encode(double)) == 0) { + // Double values on all platforms are interpreted precisely. + return { (double)number.doubleValue }; + } else if (strcmp([number objCType], @encode(float)) == 0) { + // Float values when taken as double introduce precision problems, + // so warn the user to avoid them. This would require them to + // explicitly use -[NSNumber numberWithFloat:] arguments anyway. + // We still do this conversion in order to provide a valid value. + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + NSLog(@"One-time warning: Float values are converted to double and can introduce imprecision; please use double values explicitly in predicate arguments."); + }); + return { (double)number.doubleValue }; + } else if ([number compare:@(0)] == NSOrderedDescending || + [number compare:@(0)] == NSOrderedSame) { + // Positive integer or zero; use uint64_t per mbgl::Value definition. + // We use unsigned long long here to avoid any truncation. + return { (uint64_t)number.unsignedLongLongValue }; + } else if ([number compare:@(0)] == NSOrderedAscending) { + // Negative integer; use int64_t per mbgl::Value definition. + // We use long long here to avoid any truncation. + return { (int64_t)number.longLongValue }; } } [NSException raise:@"Value not handled" diff --git a/platform/darwin/test/MGLExpressionTests.mm b/platform/darwin/test/MGLExpressionTests.mm new file mode 100644 index 0000000000..971f7d0e54 --- /dev/null +++ b/platform/darwin/test/MGLExpressionTests.mm @@ -0,0 +1,200 @@ +#import <XCTest/XCTest.h> + +#import <string> + +#import "NSExpression+MGLAdditions.h" + +@interface MGLExpressionTests : XCTestCase + +@end + +@implementation MGLExpressionTests + +#pragma mark - Utility + +- (NSComparisonPredicate *)equalityComparisonPredicateWithRightConstantValue:(id)rightConstantValue +{ + NSComparisonPredicate *predicate = [NSComparisonPredicate + predicateWithLeftExpression:[NSExpression expressionForKeyPath:@"foo"] + rightExpression:[NSExpression expressionForConstantValue:rightConstantValue] + modifier:NSDirectPredicateModifier + type:NSEqualToPredicateOperatorType + options:0]; + return predicate; +} + +#pragma mark - String Tests + +- (void)testExpressionConversionString +{ + NSComparisonPredicate *predicate = [self equalityComparisonPredicateWithRightConstantValue:@"bar"]; + mbgl::Value convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssert(convertedValue.is<std::string>() == true); + XCTAssertEqualObjects(@(convertedValue.get<std::string>().c_str()), @"bar"); +} + +#pragma mark - Boolean Tests + +- (void)testExpressionConversionBooleanTrue +{ + NSComparisonPredicate *predicate = [self equalityComparisonPredicateWithRightConstantValue:@YES]; + mbgl::Value convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssert(convertedValue.is<bool>() == true); + XCTAssert(convertedValue.get<bool>() == true); +} + +- (void)testExpressionConversionBooleanFalse +{ + NSComparisonPredicate *predicate = [self equalityComparisonPredicateWithRightConstantValue:@NO]; + mbgl::Value convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssert(convertedValue.is<bool>() == true); + XCTAssert(convertedValue.get<bool>() == false); +} + +#pragma mark - Floating Point Tests + +- (void)testExpressionConversionDouble +{ + NSComparisonPredicate *predicate; + mbgl::Value convertedValue; + + predicate = [self equalityComparisonPredicateWithRightConstantValue:[NSNumber numberWithDouble:DBL_MIN]]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<double>()); + XCTAssertEqual(convertedValue.get<double>(), DBL_MIN); + + predicate = [self equalityComparisonPredicateWithRightConstantValue:[NSNumber numberWithDouble:DBL_MAX]]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<double>()); + XCTAssertEqual(convertedValue.get<double>(), DBL_MAX); +} + +- (void)testExpressionConversionFloat +{ + // Because we can't guarantee precision when using float, and because + // we warn the user to this effect in mgl_convertedValueWithValue:, + // we just check that things are in the ballpark here with integer values + // and some lower-precision checks. + + NSComparisonPredicate *predicate; + mbgl::Value convertedValue; + + predicate = [self equalityComparisonPredicateWithRightConstantValue:[NSNumber numberWithFloat:-1]]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<double>()); + XCTAssertEqual(convertedValue.get<double>(), -1); + + predicate = [self equalityComparisonPredicateWithRightConstantValue:[NSNumber numberWithFloat:1]]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<double>()); + XCTAssertEqual(convertedValue.get<double>(), 1); + + predicate = [self equalityComparisonPredicateWithRightConstantValue:[NSNumber numberWithFloat:-23.232342]]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<double>()); + XCTAssertLessThan(-23.24, convertedValue.get<double>()); + XCTAssertGreaterThan(-23.23, convertedValue.get<double>()); + + predicate = [self equalityComparisonPredicateWithRightConstantValue:[NSNumber numberWithFloat:23.232342]]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<double>()); + XCTAssertLessThan(23.23, convertedValue.get<double>()); + XCTAssertGreaterThan(23.24, convertedValue.get<double>()); +} + +#pragma mark - Integer Tests + +- (void)testExpressionNegativeIntegers +{ + NSComparisonPredicate *predicate; + mbgl::Value convertedValue; + + NSArray<NSNumber *> *minValues = @[ + [NSNumber numberWithShort: SHRT_MIN], + [NSNumber numberWithInt: INT_MIN], + [NSNumber numberWithLong: LONG_MIN], + [NSNumber numberWithLongLong: LLONG_MIN], + [NSNumber numberWithInteger: NSIntegerMin] + ]; + + NSArray<NSNumber *> *maxValues = @[ + [NSNumber numberWithShort: SHRT_MAX], + [NSNumber numberWithInt: INT_MAX], + [NSNumber numberWithLong: LONG_MAX], + [NSNumber numberWithLongLong: LLONG_MAX], + [NSNumber numberWithInteger: NSIntegerMax] + ]; + + // Negative integers should always come back as int64_t per mbgl::Value definition. + // We use the long long value because it can store the highest number on both 32- + // and 64-bit and won't overflow. + + for (NSNumber *min in minValues) + { + predicate = [self equalityComparisonPredicateWithRightConstantValue:min]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<int64_t>()); + XCTAssertEqual(convertedValue.get<int64_t>(), min.longLongValue); + } + + // Positive integers should always come back as uint64_t per mbgl::Value definition. + // We use the unsigned long long value because it can store the highest number on + // both 32- and 64-bit and won't overflow. + + for (NSNumber *max in maxValues) + { + predicate = [self equalityComparisonPredicateWithRightConstantValue:max]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<uint64_t>()); + XCTAssertEqual(convertedValue.get<uint64_t>(), max.unsignedLongLongValue); + } + +} + +- (void)testExpressionPositiveAndZeroIntegers +{ + NSComparisonPredicate *predicate; + mbgl::Value convertedValue; + + NSArray<NSNumber *> *minValues = @[ + [NSNumber numberWithUnsignedShort: 0], + [NSNumber numberWithUnsignedInt: 0], + [NSNumber numberWithUnsignedLong: 0], + [NSNumber numberWithUnsignedLongLong: 0], + [NSNumber numberWithUnsignedInteger: 0] + ]; + + NSArray<NSNumber *> *maxValues = @[ + [NSNumber numberWithUnsignedShort: USHRT_MAX], + [NSNumber numberWithUnsignedInt: UINT_MAX], + [NSNumber numberWithUnsignedLong: ULONG_MAX], + [NSNumber numberWithUnsignedLongLong: ULLONG_MAX], + [NSNumber numberWithUnsignedInteger: NSUIntegerMax] + ]; + + // Zero-value integers should always come back as uint64_t per mbgl::Value definition + // (using the interpretation that zero is not negative). We use the unsigned long long + // value just for parity with the positive integer test. + + for (NSNumber *min in minValues) + { + predicate = [self equalityComparisonPredicateWithRightConstantValue:min]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<uint64_t>()); + XCTAssertEqual(convertedValue.get<uint64_t>(), min.unsignedLongLongValue); + } + + // Positive integers should always come back as uint64_t per mbgl::Value definition. + // We use the unsigned long long value because it can store the highest number on + // both 32- and 64-bit and won't overflow. + + for (NSNumber *max in maxValues) + { + predicate = [self equalityComparisonPredicateWithRightConstantValue:max]; + convertedValue = predicate.rightExpression.mgl_filterValue; + XCTAssertTrue(convertedValue.is<uint64_t>()); + XCTAssertEqual(convertedValue.get<uint64_t>(), max.unsignedLongLongValue); + } +} + +@end diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj index 90e0f1cfbc..35a1075301 100644 --- a/platform/ios/ios.xcodeproj/project.pbxproj +++ b/platform/ios/ios.xcodeproj/project.pbxproj @@ -397,6 +397,7 @@ DAED38641D62D0FC00D7640F /* NSURL+MGLAdditions.h in Headers */ = {isa = PBXBuildFile; fileRef = DAED38611D62D0FC00D7640F /* NSURL+MGLAdditions.h */; }; DAED38651D62D0FC00D7640F /* NSURL+MGLAdditions.m in Sources */ = {isa = PBXBuildFile; fileRef = DAED38621D62D0FC00D7640F /* NSURL+MGLAdditions.m */; }; DAED38661D62D0FC00D7640F /* NSURL+MGLAdditions.m in Sources */ = {isa = PBXBuildFile; fileRef = DAED38621D62D0FC00D7640F /* NSURL+MGLAdditions.m */; }; + DD58A4C61D822BD000E1F038 /* MGLExpressionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = DD58A4C51D822BD000E1F038 /* MGLExpressionTests.mm */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -746,6 +747,7 @@ DAD165771CF4CDFF001FF4B9 /* MGLShapeCollection.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MGLShapeCollection.m; sourceTree = "<group>"; }; DAED38611D62D0FC00D7640F /* NSURL+MGLAdditions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "NSURL+MGLAdditions.h"; sourceTree = "<group>"; }; DAED38621D62D0FC00D7640F /* NSURL+MGLAdditions.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "NSURL+MGLAdditions.m"; sourceTree = "<group>"; }; + DD58A4C51D822BD000E1F038 /* MGLExpressionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLExpressionTests.mm; path = ../../darwin/test/MGLExpressionTests.mm; sourceTree = "<group>"; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -882,7 +884,7 @@ children = ( 3575798F1D513EF1000B822E /* Layers */, 35B8E08B1D6C8B5100E768D2 /* MGLFilterTests.mm */, - 40CFA64E1D78754A008103BD /* Sources */, + 40CFA64E1D78754A008103BD /* Sources */, ); name = Styling; sourceTree = "<group>"; @@ -1037,6 +1039,7 @@ DA2E885E1CC0382C00F24E7B /* MGLOfflineRegionTests.m */, DA2E885F1CC0382C00F24E7B /* MGLOfflineStorageTests.m */, DA2E88601CC0382C00F24E7B /* MGLStyleTests.mm */, + DD58A4C51D822BD000E1F038 /* MGLExpressionTests.mm */, DA2E88551CC036F400F24E7B /* Info.plist */, ); name = "SDK Tests"; @@ -1860,6 +1863,7 @@ 40CFA6511D7875BB008103BD /* MGLGeoJSONSourceTests.mm in Sources */, DA35A2C51CCA9F8300E826B2 /* MGLClockDirectionFormatterTests.m in Sources */, 35B8E08C1D6C8B5100E768D2 /* MGLFilterTests.mm in Sources */, + DD58A4C61D822BD000E1F038 /* MGLExpressionTests.mm in Sources */, 3575798B1D502B0C000B822E /* MGLBackgroundStyleLayerTests.m in Sources */, DA2E88621CC0382C00F24E7B /* MGLOfflinePackTests.m in Sources */, DA35A2AA1CCA058D00E826B2 /* MGLCoordinateFormatterTests.m in Sources */, diff --git a/platform/macos/macos.xcodeproj/project.pbxproj b/platform/macos/macos.xcodeproj/project.pbxproj index 728e15ea95..54d141e739 100644 --- a/platform/macos/macos.xcodeproj/project.pbxproj +++ b/platform/macos/macos.xcodeproj/project.pbxproj @@ -180,6 +180,7 @@ DAE6C3D61CC34C9900DB3429 /* MGLStyleTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = DAE6C3CC1CC34BD800DB3429 /* MGLStyleTests.mm */; }; DAED385F1D62CED700D7640F /* NSURL+MGLAdditions.h in Headers */ = {isa = PBXBuildFile; fileRef = DAED385D1D62CED700D7640F /* NSURL+MGLAdditions.h */; }; DAED38601D62CED700D7640F /* NSURL+MGLAdditions.m in Sources */ = {isa = PBXBuildFile; fileRef = DAED385E1D62CED700D7640F /* NSURL+MGLAdditions.m */; }; + DD58A4C91D822C6700E1F038 /* MGLExpressionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = DD58A4C71D822C6200E1F038 /* MGLExpressionTests.mm */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -412,6 +413,7 @@ DAE6C3CC1CC34BD800DB3429 /* MGLStyleTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLStyleTests.mm; path = ../../darwin/test/MGLStyleTests.mm; sourceTree = "<group>"; }; DAED385D1D62CED700D7640F /* NSURL+MGLAdditions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "NSURL+MGLAdditions.h"; sourceTree = "<group>"; }; DAED385E1D62CED700D7640F /* NSURL+MGLAdditions.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "NSURL+MGLAdditions.m"; sourceTree = "<group>"; }; + DD58A4C71D822C6200E1F038 /* MGLExpressionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLExpressionTests.mm; path = ../../darwin/test/MGLExpressionTests.mm; sourceTree = "<group>"; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -773,6 +775,7 @@ DAE6C3CA1CC34BD800DB3429 /* MGLOfflineRegionTests.m */, DAE6C3CB1CC34BD800DB3429 /* MGLOfflineStorageTests.m */, DAE6C3CC1CC34BD800DB3429 /* MGLStyleTests.mm */, + DD58A4C71D822C6200E1F038 /* MGLExpressionTests.mm */, DAE6C33A1CC30DB200DB3429 /* Info.plist */, ); name = "SDK Tests"; @@ -1169,6 +1172,7 @@ DA35A2B61CCA14D700E826B2 /* MGLCompassDirectionFormatterTests.m in Sources */, DAE6C3D21CC34C9900DB3429 /* MGLGeometryTests.mm in Sources */, DAE6C3D51CC34C9900DB3429 /* MGLOfflineStorageTests.m in Sources */, + DD58A4C91D822C6700E1F038 /* MGLExpressionTests.mm in Sources */, DAE6C3D31CC34C9900DB3429 /* MGLOfflinePackTests.m in Sources */, DA35A2A81CC9F41600E826B2 /* MGLCoordinateFormatterTests.m in Sources */, DA0CD58E1CF56F5800A5F5A5 /* MGLFeatureTests.mm in Sources */, |