From ea76ea94928f32a7cc8ee00c59e0086c83449a15 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 Jan 2019 16:17:07 -0700 Subject: [PATCH 1/3] fix phone number parsing test --- SignalServiceKit/src/Contacts/PhoneNumber.m | 27 ++++++++++++++++--- .../tests/Contacts/PhoneNumberTest.m | 7 +---- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Contacts/PhoneNumber.m b/SignalServiceKit/src/Contacts/PhoneNumber.m index 12de6be57..db530e2de 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumber.m +++ b/SignalServiceKit/src/Contacts/PhoneNumber.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // #import "PhoneNumber.h" @@ -203,8 +203,10 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN static NSString *result = nil; static NSString *cachedClientPhoneNumber = nil; static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - // clientPhoneNumber is the local user's phone number and should never change. + + // clientPhoneNumber is the local user's phone number and should never change. + static void (^updateCachedClientPhoneNumber)(void); + updateCachedClientPhoneNumber = ^(void) { NSNumber *localCallingCode = [[PhoneNumber phoneNumberFromE164:clientPhoneNumber] getCountryCode]; if (localCallingCode != nil) { NSString *localCallingCodePrefix = [NSString stringWithFormat:@"+%@", localCallingCode]; @@ -214,11 +216,30 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN NBMetadataHelper *helper = [[NBMetadataHelper alloc] init]; NBPhoneMetaData *localNumberRegionMetadata = [helper getMetadataForRegion:localCountryCode]; result = localNumberRegionMetadata.nationalPrefixTransformRule; + } else { + result = nil; } } cachedClientPhoneNumber = [clientPhoneNumber copy]; + }; + +#ifdef DEBUG + // For performance, we want to cahce this result, but it breaks tests since local number + // can change. + if (CurrentAppContext().isRunningTests) { + updateCachedClientPhoneNumber(); + } else { + dispatch_once(&onceToken, ^{ + updateCachedClientPhoneNumber(); + }); + } +#else + dispatch_once(&onceToken, ^{ + updateCachedClientPhoneNumber(); }); OWSAssertDebug([cachedClientPhoneNumber isEqualToString:clientPhoneNumber]); +#endif + return result; } diff --git a/SignalServiceKit/tests/Contacts/PhoneNumberTest.m b/SignalServiceKit/tests/Contacts/PhoneNumberTest.m index 360432d72..35e0ae29f 100644 --- a/SignalServiceKit/tests/Contacts/PhoneNumberTest.m +++ b/SignalServiceKit/tests/Contacts/PhoneNumberTest.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // #import "PhoneNumber.h" @@ -13,13 +13,10 @@ @implementation PhoneNumberTest -#ifdef BROKEN_TESTS - -(void)testE164 { XCTAssertEqualObjects(@"+19025555555", [[PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:@"+1 (902) 555-5555"] toE164]); XCTAssertEqualObjects(@"+19025555555", [[PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:@"1 (902) 555-5555"] toE164]); XCTAssertEqualObjects(@"+19025555555", [[PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:@"1-902-555-5555"] toE164]); - XCTAssertEqualObjects(@"+19025555555", [[PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:@"1-902-５５５-5555"] toE164]); // Phone numbers missing a calling code. XCTAssertEqualObjects(@"+19025555555", [[PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:@"9025555555"] toE164]); @@ -132,6 +129,4 @@ XCTAssertTrue([parsed containsObject:@"+13235551234"]); } -#endif - @end From 5d9e03ba40317964f6daf35bcf5aa58f4f5a6848 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 Jan 2019 16:19:31 -0700 Subject: [PATCH 2/3] convert to guard statements for readability --- SignalServiceKit/src/Contacts/PhoneNumber.m | 59 +++++++++++---------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/SignalServiceKit/src/Contacts/PhoneNumber.m b/SignalServiceKit/src/Contacts/PhoneNumber.m index db530e2de..a0a561600 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumber.m +++ b/SignalServiceKit/src/Contacts/PhoneNumber.m @@ -324,35 +324,38 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN // Order matters; better results should appear first so prefer // matches with the same country code as this client's phone number. - OWSAssertDebug(clientPhoneNumber.length > 0); - if (clientPhoneNumber.length > 0) { - // Note that NBPhoneNumber uses "country code" to refer to what we call a - // "calling code" (i.e. 44 in +44123123). Within SSK we use "country code" - // (and sometimes "region code") to refer to a country's ISO 2-letter code - // (ISO 3166-1 alpha-2). - NSNumber *callingCodeForLocalNumber = [[PhoneNumber phoneNumberFromE164:clientPhoneNumber] getCountryCode]; - if (callingCodeForLocalNumber != nil) { - NSString *callingCodePrefix = [NSString stringWithFormat:@"+%@", callingCodeForLocalNumber]; - - tryParsingWithCountryCode( - [callingCodePrefix stringByAppendingString:sanitizedString], [self defaultCountryCode]); - - // Try to determine what the country code is for the local phone number - // and also try parsing the phone number using that country code if it - // differs from the device's region code. - // - // For example, a French person living in Italy might have an - // Italian phone number but use French region/language for their - // phone. They're likely to have both Italian and French contacts. - NSString *localCountryCode = - [PhoneNumberUtil.sharedThreadLocal probableCountryCodeForCallingCode:callingCodePrefix]; - if (localCountryCode && ![localCountryCode isEqualToString:[self defaultCountryCode]]) { - tryParsingWithCountryCode( - [callingCodePrefix stringByAppendingString:sanitizedString], localCountryCode); - } - } + if (clientPhoneNumber.length == 0) { + OWSFailDebug(@"clientPhoneNumber had unexpected length"); + return result; } - + + // Note that NBPhoneNumber uses "country code" to refer to what we call a + // "calling code" (i.e. 44 in +44123123). Within SSK we use "country code" + // (and sometimes "region code") to refer to a country's ISO 2-letter code + // (ISO 3166-1 alpha-2). + NSNumber *callingCodeForLocalNumber = [[PhoneNumber phoneNumberFromE164:clientPhoneNumber] getCountryCode]; + if (callingCodeForLocalNumber == nil) { + OWSFailDebug(@"callingCodeForLocalNumber was unexpectedly nil"); + return result; + } + + NSString *callingCodePrefix = [NSString stringWithFormat:@"+%@", callingCodeForLocalNumber]; + + tryParsingWithCountryCode([callingCodePrefix stringByAppendingString:sanitizedString], [self defaultCountryCode]); + + // Try to determine what the country code is for the local phone number + // and also try parsing the phone number using that country code if it + // differs from the device's region code. + // + // For example, a French person living in Italy might have an + // Italian phone number but use French region/language for their + // phone. They're likely to have both Italian and French contacts. + NSString *localCountryCode = + [PhoneNumberUtil.sharedThreadLocal probableCountryCodeForCallingCode:callingCodePrefix]; + if (localCountryCode && ![localCountryCode isEqualToString:[self defaultCountryCode]]) { + tryParsingWithCountryCode([callingCodePrefix stringByAppendingString:sanitizedString], localCountryCode); + } + return result; } From 60f816c7478b4fe509da3f1c6aa17d1868abac56 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 9 Jan 2019 17:17:02 -0700 Subject: [PATCH 3/3] Area code inference for US and Brazil --- SignalServiceKit/src/Contacts/PhoneNumber.m | 125 ++++++++++++++++++ .../tests/Contacts/PhoneNumberTest.m | 70 ++++++++++ 2 files changed, 195 insertions(+) diff --git a/SignalServiceKit/src/Contacts/PhoneNumber.m b/SignalServiceKit/src/Contacts/PhoneNumber.m index a0a561600..064c0f81c 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumber.m +++ b/SignalServiceKit/src/Contacts/PhoneNumber.m @@ -356,9 +356,134 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN tryParsingWithCountryCode([callingCodePrefix stringByAppendingString:sanitizedString], localCountryCode); } + NSString *_Nullable phoneNumberByApplyingMissingAreaCode = + [self applyMissingAreaCodeWithCallingCodeForReferenceNumber:callingCodeForLocalNumber + referenceNumber:clientPhoneNumber + sanitizedInputText:sanitizedString]; + if (phoneNumberByApplyingMissingAreaCode) { + tryParsingWithCountryCode(phoneNumberByApplyingMissingAreaCode, localCountryCode); + } + return result; } +#pragma mark - missing area code + ++ (nullable NSString *)applyMissingAreaCodeWithCallingCodeForReferenceNumber:(NSNumber *)callingCodeForReferenceNumber + referenceNumber:(NSString *)referenceNumber + sanitizedInputText:(NSString *)sanitizedInputText +{ + if ([callingCodeForReferenceNumber isEqual:@(55)]) { + return + [self applyMissingBrazilAreaCodeWithReferenceNumber:referenceNumber sanitizedInputText:sanitizedInputText]; + } else if ([callingCodeForReferenceNumber isEqual:@(1)]) { + return [self applyMissingUnitedStatesAreaCodeWithReferenceNumber:referenceNumber + sanitizedInputText:sanitizedInputText]; + } else { + return nil; + } +} + +#pragma mark - missing brazil area code + ++ (nullable NSString *)applyMissingBrazilAreaCodeWithReferenceNumber:(NSString *)referenceNumber + sanitizedInputText:(NSString *)sanitizedInputText +{ + NSError *error; + NSRegularExpression *missingAreaCodeRegex = + [[NSRegularExpression alloc] initWithPattern:@"^(9?\\d{8})$" options:0 error:&error]; + if (error) { + OWSFailDebug(@"failure: %@", error); + return nil; + } + + if ([missingAreaCodeRegex firstMatchInString:sanitizedInputText + options:0 + range:NSMakeRange(0, sanitizedInputText.length)] + == nil) { + } + + NSString *_Nullable referenceAreaCode = [self brazilAreaCodeFromReferenceNumberE164:referenceNumber]; + if (!referenceAreaCode) { + return nil; + } + return [NSString stringWithFormat:@"+55%@%@", referenceAreaCode, sanitizedInputText]; +} + ++ (nullable NSString *)brazilAreaCodeFromReferenceNumberE164:(NSString *)referenceNumberE164 +{ + NSError *error; + NSRegularExpression *areaCodeRegex = + [[NSRegularExpression alloc] initWithPattern:@"^\\+55(\\d{2})9?\\d{8}" options:0 error:&error]; + if (error) { + OWSFailDebug(@"failure: %@", error); + return nil; + } + + NSArray *matches = + [areaCodeRegex matchesInString:referenceNumberE164 options:0 range:NSMakeRange(0, referenceNumberE164.length)]; + if (matches.count == 0) { + OWSFailDebug(@"failure: unexpectedly unable to extract area code from US number"); + return nil; + } + NSTextCheckingResult *match = matches[0]; + + NSRange firstCaptureRange = [match rangeAtIndex:1]; + return [referenceNumberE164 substringWithRange:firstCaptureRange]; +} + +#pragma mark - missing US area code + ++ (nullable NSString *)applyMissingUnitedStatesAreaCodeWithReferenceNumber:(NSString *)referenceNumber + sanitizedInputText:(NSString *)sanitizedInputText +{ + NSError *error; + NSRegularExpression *missingAreaCodeRegex = + [[NSRegularExpression alloc] initWithPattern:@"^(\\d{7})$" options:0 error:&error]; + if (error) { + OWSFailDebug(@"failure: %@", error); + return nil; + } + + if ([missingAreaCodeRegex firstMatchInString:sanitizedInputText + options:0 + range:NSMakeRange(0, sanitizedInputText.length)] + == nil) { + // area code isn't missing + return nil; + } + + NSString *_Nullable referenceAreaCode = [self unitedStateAreaCodeFromReferenceNumberE164:referenceNumber]; + if (!referenceAreaCode) { + return nil; + } + return [NSString stringWithFormat:@"+1%@%@", referenceAreaCode, sanitizedInputText]; +} + ++ (nullable NSString *)unitedStateAreaCodeFromReferenceNumberE164:(NSString *)referenceNumberE164 +{ + NSError *error; + NSRegularExpression *areaCodeRegex = + [[NSRegularExpression alloc] initWithPattern:@"^\\+1(\\d{3})" options:0 error:&error]; + if (error) { + OWSFailDebug(@"failure: %@", error); + return nil; + } + + NSArray *matches = + [areaCodeRegex matchesInString:referenceNumberE164 options:0 range:NSMakeRange(0, referenceNumberE164.length)]; + if (matches.count == 0) { + OWSFailDebug(@"failure: unexpectedly unable to extract area code from US number"); + return nil; + } + NSTextCheckingResult *match = matches[0]; + + NSRange firstCaptureRange = [match rangeAtIndex:1]; + return [referenceNumberE164 substringWithRange:firstCaptureRange]; +} + +#pragma mark - + + (NSString *)removeFormattingCharacters:(NSString *)inputString { char outputString[inputString.length + 1]; diff --git a/SignalServiceKit/tests/Contacts/PhoneNumberTest.m b/SignalServiceKit/tests/Contacts/PhoneNumberTest.m index 35e0ae29f..81b64fe8d 100644 --- a/SignalServiceKit/tests/Contacts/PhoneNumberTest.m +++ b/SignalServiceKit/tests/Contacts/PhoneNumberTest.m @@ -129,4 +129,74 @@ XCTAssertTrue([parsed containsObject:@"+13235551234"]); } +- (void)testMissingAreaCode_USA +{ + // Add area code to numbers that look like "local" numbers + NSArray *parsed = + [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"555-1234" clientPhoneNumber:@"+13233214321"]; + XCTAssertTrue([parsed containsObject:@"+13235551234"]); + + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"5551234" clientPhoneNumber:@"+13233214321"]; + XCTAssertTrue([parsed containsObject:@"+13235551234"]); + + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"555 1234" clientPhoneNumber:@"+13233214321"]; + XCTAssertTrue([parsed containsObject:@"+13235551234"]); + + // Don't touch numbers that look like e164, even if they're the same length as a "local" us number + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"+5551234" clientPhoneNumber:@"+13213214321"]; + XCTAssertTrue([parsed containsObject:@"+5551234"]); + + // Don't touch numbers that already have an area code + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"570 555 1234" clientPhoneNumber:@"+13233214321"]; + XCTAssertTrue([parsed containsObject:@"+15705551234"]); + + // Don't touch numbers that are already in e164 + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"+33170393800" clientPhoneNumber:@"+13213214321"]; + XCTAssertTrue([parsed containsObject:@"+33170393800"]); +} + +- (void)testMissingAreaCode_Brazil +{ + // Add area code to land-line numbers that look like "local" numbers + NSArray *parsed = + [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"87654321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+552187654321"]); + + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"8765-4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+552187654321"]); + + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"8765 4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+552187654321"]); + + // Add area code to mobile numbers that look like "local" numbers + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"987654321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+5521987654321"]); + + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"9 8765-4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+5521987654321"]); + + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"9 8765 4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+5521987654321"]); + + // Don't touch numbers that look like e164, even if they're the same length as a "local" us number + parsed = [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"+3365-4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+33654321"]); + + // Don't touch land-line numbers that already have an area code + parsed = + [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"22 8765 4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+552287654321"]); + + // Don't touch mobile numbers that already have an area code + parsed = + [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"22 9 8765 4321" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+5522987654321"]); + + // Don't touch numbers that are already in e164 + parsed = + [self unpackTryParsePhoneNumbersFromsUserSpecifiedText:@"+33170393800" clientPhoneNumber:@"+5521912345678"]; + XCTAssertTrue([parsed containsObject:@"+33170393800"]); +} + + @end