From 8a33f94a22db32536099368c13f199fb60e140ef Mon Sep 17 00:00:00 2001 From: Lara Scheidegger Date: Thu, 31 Mar 2016 12:36:08 +0200 Subject: [PATCH 1/2] * Updating parse documentation to be more clear about what it accepts * New useful getExampleNumber methods (for testing) - one for invalid numbers and one for getting a number of a particular type without specifying a country. --- cpp/src/phonenumbers/phonenumberutil.cc | 94 ++++++++++++++- cpp/src/phonenumbers/phonenumberutil.h | 41 ++++++- cpp/test/phonenumbers/phonenumberutil_test.cc | 46 ++++++- .../i18n/phonenumbers/PhoneNumberUtil.java | 112 ++++++++++++++++-- .../phonenumbers/PhoneNumberUtilTest.java | 19 ++- java/pending_code_changes.txt | 3 + .../i18n/phonenumbers/phonenumberutil.js | 24 ++-- 7 files changed, 317 insertions(+), 22 deletions(-) diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index d8f72b5c2..358a9963d 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -1645,6 +1645,59 @@ bool PhoneNumberUtil::GetExampleNumber(const string& region_code, return GetExampleNumberForType(region_code, FIXED_LINE, number); } +bool PhoneNumberUtil::GetInvalidExampleNumber(const string& region_code, + PhoneNumber* number) const { + DCHECK(number); + if (!IsValidRegionCode(region_code)) { + LOG(WARNING) << "Invalid or unknown region code (" << region_code + << ") provided."; + return false; + } + // We start off with a valid fixed-line number since every country supports + // this. Alternatively we could start with a different number type, since + // fixed-line numbers typically have a wide breadth of valid number lengths + // and we may have to make it very short before we get an invalid number. + const PhoneMetadata* region_metadata = GetMetadataForRegion(region_code); + const PhoneNumberDesc* desc = + GetNumberDescByType(*region_metadata, FIXED_LINE); + if (!desc->has_example_number()) { + // This shouldn't happen - we have a test for this. + return false; + } + const string& example_number = desc->example_number(); + // Try and make the number invalid. We do this by changing the length. We try + // reducing the length of the number, since currently no region has a number + // that is the same length as kMinLengthForNsn. This is probably quicker than + // making the number longer, which is another alternative. We could also use + // the possible number pattern to extract the possible lengths of the number + // to make this faster, but this method is only for unit-testing so simplicity + // is preferred to performance. + // We don't want to return a number that can't be parsed, so we check the + // number is long enough. We try all possible lengths because phone number + // plans often have overlapping prefixes so the number 123456 might be valid + // as a fixed-line number, and 12345 as a mobile number. It would be faster to + // loop in a different order, but we prefer numbers that look closer to real + // numbers (and it gives us a variety of different lengths for the resulting + // phone numbers - otherwise they would all be kMinLengthForNsn digits long.) + for (size_t phone_number_length = example_number.length() - 1; + phone_number_length >= kMinLengthForNsn; + phone_number_length--) { + string number_to_try = example_number.substr(0, phone_number_length); + PhoneNumber possibly_valid_number; + Parse(number_to_try, region_code, &possibly_valid_number); + // We don't check the return value since we have already checked the + // length, we know example numbers have only valid digits, and we know the + // region code is fine. + if (!IsValidNumber(possibly_valid_number)) { + number->MergeFrom(possibly_valid_number); + return true; + } + } + // We have a test to check that this doesn't happen for any of our supported + // regions. + return false; +} + // Gets a valid number for the specified region_code and type. Returns false if // the country was unknown or 001 (representing non-geographical regions), or if // no number exists. @@ -1672,6 +1725,45 @@ bool PhoneNumberUtil::GetExampleNumberForType( return false; } +bool PhoneNumberUtil::GetExampleNumberForType( + PhoneNumberUtil::PhoneNumberType type, + PhoneNumber* number) const { + DCHECK(number); + set regions; + GetSupportedRegions(®ions); + for (set::const_iterator it = regions.begin(); + it != regions.end(); ++it) { + if (GetExampleNumberForType(*it, type, number)) { + return true; + } + } + // If there wasn't an example number for a region, try the non-geographical + // entities. + set global_network_calling_codes; + GetSupportedGlobalNetworkCallingCodes(&global_network_calling_codes); + for (set::const_iterator it = global_network_calling_codes.begin(); + it != global_network_calling_codes.end(); ++it) { + int country_calling_code = *it; + const PhoneMetadata* metadata = + GetMetadataForNonGeographicalRegion(country_calling_code); + const PhoneNumberDesc& desc = metadata->general_desc(); + if (desc.has_example_number()) { + ErrorType success = Parse(StrCat(kPlusSign, + country_calling_code, + desc.example_number()), + RegionCode::GetUnknown(), number); + if (success == NO_PARSING_ERROR) { + return true; + } else { + LOG(ERROR) << "Error parsing example number (" + << static_cast(success) << ")"; + } + } + } + // There are no example numbers of this type for any country in the library. + return false; +} + bool PhoneNumberUtil::GetExampleNumberForNonGeoEntity( int country_calling_code, PhoneNumber* number) const { DCHECK(number); @@ -1683,7 +1775,7 @@ bool PhoneNumberUtil::GetExampleNumberForNonGeoEntity( ErrorType success = Parse(StrCat(kPlusSign, SimpleItoa(country_calling_code), desc.example_number()), - RegionCode::ZZ(), number); + RegionCode::GetUnknown(), number); if (success == NO_PARSING_ERROR) { return true; } else { diff --git a/cpp/src/phonenumbers/phonenumberutil.h b/cpp/src/phonenumbers/phonenumberutil.h index d16318587..108d91c72 100644 --- a/cpp/src/phonenumbers/phonenumberutil.h +++ b/cpp/src/phonenumbers/phonenumberutil.h @@ -496,6 +496,18 @@ class PhoneNumberUtil : public Singleton { bool GetExampleNumber(const string& region_code, PhoneNumber* number) const; + // Gets an invalid number for the specified region. This is useful for + // unit-testing purposes, where you want to test that will happen with an + // invalid number. Note that the number that is returned will always be able + // to be parsed and will have the correct country code. It may also be a valid + // *short* number/code for this region. Validity checking such + // numbers is handled with ShortNumberInfo. + // + // Returns false when an unsupported region or the region 001 (Earth) is + // passed in. + bool GetInvalidExampleNumber(const string& region_code, + PhoneNumber* number) const; + // Gets a valid number of the specified type for the specified region. // Returns false if the region was unknown or 001, or if no example number of // that type could be found. For 001 (representing non-geographical numbers), @@ -504,6 +516,13 @@ class PhoneNumberUtil : public Singleton { PhoneNumberType type, PhoneNumber* number) const; + // Gets a valid number for the specified type (it may belong to any country). + // Returns false when the metadata does not contain such information. This + // should only happen when no numbers of this type are allocated anywhere in + // the world anymore. + bool GetExampleNumberForType(PhoneNumberType type, + PhoneNumber* number) const; + // Gets a valid number for the specified country calling code for a // non-geographical entity. Returns false if the metadata does not contain // such information, or the country calling code passed in does not belong to @@ -511,10 +530,19 @@ class PhoneNumberUtil : public Singleton { bool GetExampleNumberForNonGeoEntity( int country_calling_code, PhoneNumber* number) const; - // Parses a string and returns it in proto buffer format. This method will - // return an error like INVALID_COUNTRY_CODE if the number is not considered - // to be a possible number, and NO_PARSING_ERROR if it parsed correctly. Note - // that validation of whether the number is actually a valid number for a + // Parses a string and returns it as a phone number in proto buffer format. + // The method is quite lenient and looks for a number in the input text + // (raw input) and does not check whether the string is definitely only a + // phone number. To do this, it ignores punctuation and white-space, as well + // as any text before the number (e.g. a leading “Tel: ”) and trims the + // non-number bits. It will accept a number in any format (E164, national, + // international etc), assuming it can be interpreted with the defaultRegion + // supplied. It also attempts to convert any alpha characters into digits + // if it thinks this is a vanity number of the type "1800 MICROSOFT". + // + // This method will return an error if the number is not considered to be a + // possible number, and NO_PARSING_ERROR if it parsed correctly. + // Note that validation of whether the number is actually a valid number for a // particular region is not performed. This can be done separately with // IsValidNumber(). // @@ -526,6 +554,11 @@ class PhoneNumberUtil : public Singleton { // stored as that of the default country supplied. If the number is guaranteed // to start with a '+' followed by the country calling code, then // "ZZ" can be supplied. + // + // Returns an error if the string is not considered to be a viable phone + // number (e.g.too few or too many digits) or if no default region was + // supplied and the number is not in international format (does not + // start with +). ErrorType Parse(const string& number_to_parse, const string& default_region, PhoneNumber* number) const; diff --git a/cpp/test/phonenumbers/phonenumberutil_test.cc b/cpp/test/phonenumbers/phonenumberutil_test.cc index 31fd893c9..4f5be02bb 100644 --- a/cpp/test/phonenumbers/phonenumberutil_test.cc +++ b/cpp/test/phonenumbers/phonenumberutil_test.cc @@ -372,6 +372,25 @@ TEST_F(PhoneNumberUtilTest, GetExampleNumber) { EXPECT_FALSE(phone_util_.GetExampleNumber(RegionCode::UN001(), &test_number)); } +TEST_F(PhoneNumberUtilTest, GetInvalidExampleNumber) { + // RegionCode 001 is reserved for supporting non-geographical country calling + // codes. We don't support getting an invalid example number for it with + // GetInvalidExampleNumber. + PhoneNumber test_number; + EXPECT_FALSE(phone_util_.GetInvalidExampleNumber(RegionCode::UN001(), + &test_number)); + EXPECT_EQ(PhoneNumber::default_instance(), test_number); + EXPECT_FALSE(phone_util_.GetInvalidExampleNumber(RegionCode::CS(), + &test_number)); + EXPECT_EQ(PhoneNumber::default_instance(), test_number); + + EXPECT_TRUE(phone_util_.GetInvalidExampleNumber(RegionCode::US(), + &test_number)); + // At least the country calling code should be set correctly. + EXPECT_EQ(1, test_number.country_code()); + EXPECT_NE(0, test_number.national_number()); +} + TEST_F(PhoneNumberUtilTest, GetExampleNumberForNonGeoEntity) { PhoneNumber toll_free_number; toll_free_number.set_country_code(800); @@ -390,6 +409,30 @@ TEST_F(PhoneNumberUtilTest, GetExampleNumberForNonGeoEntity) { EXPECT_EQ(universal_premium_rate, test_number); } +TEST_F(PhoneNumberUtilTest, GetExampleNumberWithoutRegion) { + // In our test metadata we don't cover all types: in our real metadata, we do. + PhoneNumber test_number; + bool success = phone_util_.GetExampleNumberForType( + PhoneNumberUtil::FIXED_LINE, + &test_number); + // We test that the call to get an example number succeeded, and that the + // number passed in was modified. + EXPECT_TRUE(success); + EXPECT_NE(PhoneNumber::default_instance(), test_number); + test_number.Clear(); + + success = phone_util_.GetExampleNumberForType(PhoneNumberUtil::MOBILE, + &test_number); + EXPECT_TRUE(success); + EXPECT_NE(PhoneNumber::default_instance(), test_number); + test_number.Clear(); + + success = phone_util_.GetExampleNumberForType(PhoneNumberUtil::PREMIUM_RATE, + &test_number); + EXPECT_TRUE(success); + EXPECT_NE(PhoneNumber::default_instance(), test_number); +} + TEST_F(PhoneNumberUtilTest, FormatUSNumber) { PhoneNumber test_number; string formatted_number; @@ -2363,7 +2406,7 @@ TEST_F(PhoneNumberUtilTest, ConvertAlphaCharactersInNumber) { // Try with some non-ASCII characters. input.assign("1\xE3\x80\x80\xEF\xBC\x88" "800) ABC-DEF" - /* "1　（800) ABCD-DEF" */); + /* "1　（800) ABC-DEF" */); static const string kExpectedFullwidthOutput = "1\xE3\x80\x80\xEF\xBC\x88" "800) 222-333" /* "1　（800) 222-333" */; phone_util_.ConvertAlphaCharactersInNumber(&input); @@ -3184,6 +3227,7 @@ TEST_F(PhoneNumberUtilTest, ParseWithInternationalPrefixes) { phone_util_.Parse("\xEF\xBC\x8B" "1 (650) 333-6000", /* "＋1 (650) 333-6000" */ RegionCode::SG(), &test_number)); + EXPECT_EQ(us_number, test_number); // Using a soft hyphen U+00AD. EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR, phone_util_.Parse("1 (650) 333" "\xC2\xAD" "-6000", diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index 0a0e455f6..ae034bd2e 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -1818,6 +1818,62 @@ public class PhoneNumberUtil { return getExampleNumberForType(regionCode, PhoneNumberType.FIXED_LINE); } + /** + * Gets an invalid number for the specified region. This is useful for unit-testing purposes, + * where you want to test what will happen with an invalid number. Note that the number that is + * returned will always be able to be parsed and will have the correct country code. It may also + * be a valid *short* number/code for this region. Validity checking such numbers is handled with + * {@link com.google.i18n.phonenumbers.ShortNumberInfo}. + * + * @param regionCode the region for which an example number is needed + * @return an invalid number for the specified region. Returns null when an unsupported region or + * the region 001 (Earth) is passed in. + */ + public PhoneNumber getInvalidExampleNumber(String regionCode) { + if (!isValidRegionCode(regionCode)) { + logger.log(Level.WARNING, "Invalid or unknown region code provided: " + regionCode); + return null; + } + // We start off with a valid fixed-line number since every country supports this. Alternatively + // we could start with a different number type, since fixed-line numbers typically have a wide + // breadth of valid number lengths and we may have to make it very short before we get an + // invalid number. + PhoneNumberDesc desc = getNumberDescByType(getMetadataForRegion(regionCode), + PhoneNumberType.FIXED_LINE); + if (desc.exampleNumber.equals("")) { + // This shouldn't happen; we have a test for this. + return null; + } + String exampleNumber = desc.exampleNumber; + // Try and make the number invalid. We do this by changing the length. We try reducing the + // length of the number, since currently no region has a number that is the same length as + // MIN_LENGTH_FOR_NSN. This is probably quicker than making the number longer, which is another + // alternative. We could also use the possible number pattern to extract the possible lengths of + // the number to make this faster, but this method is only for unit-testing so simplicity is + // preferred to performance. We don't want to return a number that can't be parsed, so we check + // the number is long enough. We try all possible lengths because phone number plans often have + // overlapping prefixes so the number 123456 might be valid as a fixed-line number, and 12345 as + // a mobile number. It would be faster to loop in a different order, but we prefer numbers that + // look closer to real numbers (and it gives us a variety of different lengths for the resulting + // phone numbers - otherwise they would all be MIN_LENGTH_FOR_NSN digits long.) + for (int phoneNumberLength = exampleNumber.length() - 1; + phoneNumberLength >= MIN_LENGTH_FOR_NSN; + phoneNumberLength--) { + String numberToTry = exampleNumber.substring(0, phoneNumberLength); + try { + PhoneNumber possiblyValidNumber = parse(numberToTry, regionCode); + if (!isValidNumber(possiblyValidNumber)) { + return possiblyValidNumber; + } + } catch (NumberParseException e) { + // Shouldn't happen: we have already checked the length, we know example numbers have + // only valid digits, and we know the region code is fine. + } + } + // We have a test to check that this doesn't happen for any of our supported regions. + return null; + } + /** * Gets a valid number for the specified region and number type. * @@ -1845,6 +1901,37 @@ public class PhoneNumberUtil { return null; } + /** + * Gets a valid number for the specified number type (it may belong to any country). + * + * @param type the type of number that is needed + * @return a valid number for the specified type. Returns null when the metadata + * does not contain such information. This should only happen when no numbers of this type are + * allocated anywhere in the world anymore. + */ + public PhoneNumber getExampleNumberForType(PhoneNumberType type) { + for (String regionCode : getSupportedRegions()) { + PhoneNumber exampleNumber = getExampleNumberForType(regionCode, type); + if (exampleNumber != null) { + return exampleNumber; + } + } + // If there wasn't an example number for a region, try the non-geographical entities. + for (int countryCallingCode : getSupportedGlobalNetworkCallingCodes()) { + PhoneNumberDesc desc = getNumberDescByType( + getMetadataForNonGeographicalRegion(countryCallingCode), type); + try { + if (!desc.exampleNumber.equals("")) { + return parse("+" + countryCallingCode + desc.exampleNumber, UNKNOWN_REGION); + } + } catch (NumberParseException e) { + logger.log(Level.SEVERE, e.toString()); + } + } + // There are no example numbers of this type for any country in the library. + return null; + } + /** * Gets a valid number for the specified country calling code for a non-geographical entity. * @@ -1859,7 +1946,7 @@ public class PhoneNumberUtil { PhoneNumberDesc desc = metadata.generalDesc; try { if (!desc.exampleNumber.equals("")) { - return parse("+" + countryCallingCode + desc.exampleNumber, "ZZ"); + return parse("+" + countryCallingCode + desc.exampleNumber, UNKNOWN_REGION); } } catch (NumberParseException e) { logger.log(Level.SEVERE, e.toString()); @@ -2670,10 +2757,18 @@ public class PhoneNumberUtil { } /** - * Parses a string and returns it in proto buffer format. This method will throw a - * {@link com.google.i18n.phonenumbers.NumberParseException} if the number is not considered to be - * a possible number. Note that validation of whether the number is actually a valid number for a - * particular region is not performed. This can be done separately with {@link #isValidNumber}. + * Parses a string and returns it as a phone number in proto buffer format. The method is quite + * lenient and looks for a number in the input text (raw input) and does not check whether the + * string is definitely only a phone number. To do this, it ignores punctuation and white-space, + * as well as any text before the number (e.g. a leading “Tel: ”) and trims the non-number bits. + * It will accept a number in any format (E164, national, international etc), assuming it can be + * interpreted with the defaultRegion supplied. It also attempts to convert any alpha characters + * into digits if it thinks this is a vanity number of the type "1800 MICROSOFT". + * + *

This method will throw a {@link com.google.i18n.phonenumbers.NumberParseException} if the + * number is not considered to be a possible number. Note that validation of whether the number + * is actually a valid number for a particular region is not performed. This can be done + * separately with {@link #isValidNumber}. * * @param numberToParse number that we are attempting to parse. This can contain formatting * such as +, ( and -, as well as a phone number extension. It can also @@ -2685,9 +2780,10 @@ public class PhoneNumberUtil { * start with a '+' followed by the country calling code, then * "ZZ" or null can be supplied. * @return a phone number proto buffer filled with the parsed number - * @throws NumberParseException if the string is not considered to be a viable phone number or if - * no default region was supplied and the number is not in - * international format (does not start with +) + * @throws NumberParseException if the string is not considered to be a viable phone number (e.g. + * too few or too many digits) or if no default region was supplied + * and the number is not in international format (does not start + * with +) */ public PhoneNumber parse(String numberToParse, String defaultRegion) throws NumberParseException { diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java index 4f6bcaf56..a85eb60a4 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java @@ -330,7 +330,17 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { PhoneNumberUtil.PhoneNumberType.MOBILE)); // RegionCode 001 is reserved for supporting non-geographical country calling code. We don't // support getting an example number for it with this method. - assertEquals(null, phoneUtil.getExampleNumber(RegionCode.UN001)); + assertNull(phoneUtil.getExampleNumber(RegionCode.UN001)); + } + + public void testGetInvalidExampleNumber() { + // RegionCode 001 is reserved for supporting non-geographical country calling codes. We don't + // support getting an invalid example number for it with getInvalidExampleNumber. + assertNull(phoneUtil.getInvalidExampleNumber(RegionCode.UN001)); + assertNull(phoneUtil.getInvalidExampleNumber(RegionCode.CS)); + PhoneNumber usInvalidNumber = phoneUtil.getInvalidExampleNumber(RegionCode.US); + assertEquals(1, usInvalidNumber.getCountryCode()); + assertFalse(usInvalidNumber.getNationalNumber() == 0); } public void testGetExampleNumberForNonGeoEntity() { @@ -338,6 +348,13 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { assertEquals(UNIVERSAL_PREMIUM_RATE, phoneUtil.getExampleNumberForNonGeoEntity(979)); } + public void testGetExampleNumberWithoutRegion() { + // In our test metadata we don't cover all types: in our real metadata, we do. + assertNotNull(phoneUtil.getExampleNumberForType(PhoneNumberUtil.PhoneNumberType.FIXED_LINE)); + assertNotNull(phoneUtil.getExampleNumberForType(PhoneNumberUtil.PhoneNumberType.MOBILE)); + assertNotNull(phoneUtil.getExampleNumberForType(PhoneNumberUtil.PhoneNumberType.PREMIUM_RATE)); + } + public void testConvertAlphaCharactersInNumber() { String input = "1800-ABC-DEF"; // Alpha chars are converted to digits; everything else is left untouched. diff --git a/java/pending_code_changes.txt b/java/pending_code_changes.txt index ad5b28484..6c3e64d35 100644 --- a/java/pending_code_changes.txt +++ b/java/pending_code_changes.txt @@ -1,3 +1,6 @@ Code changes: - Added java/pending_code_changes.txt for contributors to track code changes between releases. + - Added getExampleNumberForType that doesn't take in a region, and + getInvalidExampleNumber + - Improvements to javadoc for parse method diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index 264aecf3d..abceab43e 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -3663,11 +3663,20 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.checkRegionForParsing_ = function( /** - * Parses a string and returns it in proto buffer format. This method will throw - * a {@link i18n.phonenumbers.Error} if the number is not considered to be a - * possible number. Note that validation of whether the number is actually a - * valid number for a particular region is not performed. This can be done - * separately with {@link #isValidNumber}. + * Parses a string and returns it as a phone number in proto buffer format. The + * method is quite lenient and looks for a number in the input text (raw input) + * and does not check whether the string is definitely only a phone number. To + * do this, it ignores punctuation and white-space, as well as any text before + * the number (e.g. a leading “Tel: ”) and trims the non-number bits. It will + * accept a number in any format (E164, national, international etc), assuming + * it can be interpreted with the defaultRegion supplied. It also attempts to + * convert any alpha characters into digits if it thinks this is a vanity number + * of the type "1800 MICROSOFT". + * + * This method will throw a {@link i18n.phonenumbers.Error} if the number is not + * considered to be a possible number. Note that validation of whether the + * number is actually a valid number for a particular region is not performed. + * This can be done separately with {@link #isValidNumber}. * * @param {?string} numberToParse number that we are attempting to parse. This * can contain formatting such as +, ( and -, as well as a phone number @@ -3681,8 +3690,9 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.checkRegionForParsing_ = function( * @return {i18n.phonenumbers.PhoneNumber} a phone number proto buffer filled * with the parsed number. * @throws {i18n.phonenumbers.Error} if the string is not considered to be a - * viable phone number or if no default region was supplied and the number - * is not in international format (does not start with +). + * viable phone number (e.g. too few or too many digits) or if no default + * region was supplied and the number is not in international format (does + * not start with +). */ i18n.phonenumbers.PhoneNumberUtil.prototype.parse = function(numberToParse, defaultRegion) { From c5b796281660f79118737efd9379c50e67c17246 Mon Sep 17 00:00:00 2001 From: Lara Scheidegger Date: Thu, 31 Mar 2016 21:56:22 +0200 Subject: [PATCH 2/2] Adding tests for our metadata to make sure that we can create an invalid example number for every region, and that we have an example number for every type. Also adding better logging for example short number cost mismatches. --- .../i18n/phonenumbers/ExampleNumbersTest.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/ExampleNumbersTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/ExampleNumbersTest.java index 1365ac3f7..a69b85be9 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/ExampleNumbersTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/ExampleNumbersTest.java @@ -175,7 +175,21 @@ public class ExampleNumbersTest extends TestCase { public void testEveryRegionHasAnExampleNumber() throws Exception { for (String regionCode : phoneNumberUtil.getSupportedRegions()) { PhoneNumber exampleNumber = phoneNumberUtil.getExampleNumber(regionCode); - assertNotNull("None found for region " + regionCode, exampleNumber); + assertNotNull("No example number found for region " + regionCode, exampleNumber); + } + } + + public void testEveryRegionHasAnInvalidExampleNumber() throws Exception { + for (String regionCode : phoneNumberUtil.getSupportedRegions()) { + PhoneNumber exampleNumber = phoneNumberUtil.getInvalidExampleNumber(regionCode); + assertNotNull("No invalid example number found for region " + regionCode, exampleNumber); + } + } + + public void testEveryTypeHasAnExampleNumber() throws Exception { + for (PhoneNumberUtil.PhoneNumberType type : PhoneNumberUtil.PhoneNumberType.values()) { + PhoneNumber exampleNumber = phoneNumberUtil.getExampleNumberForType(type); + assertNotNull("No example number found for type " + type, exampleNumber); } } @@ -199,10 +213,14 @@ public class ExampleNumbersTest extends TestCase { for (ShortNumberInfo.ShortNumberCost cost : ShortNumberInfo.ShortNumberCost.values()) { exampleShortNumber = shortNumberInfo.getExampleShortNumberForCost(regionCode, cost); if (!exampleShortNumber.equals("")) { - if (cost != shortNumberInfo.getExpectedCostForRegion( - phoneNumberUtil.parse(exampleShortNumber, regionCode), regionCode)) { + phoneNumber = phoneNumberUtil.parse(exampleShortNumber, regionCode); + ShortNumberInfo.ShortNumberCost exampleShortNumberCost = + shortNumberInfo.getExpectedCostForRegion(phoneNumber, regionCode); + if (cost != exampleShortNumberCost) { wrongTypeCases.add(phoneNumber); - LOGGER.log(Level.SEVERE, "Wrong cost for " + phoneNumber.toString()); + LOGGER.log(Level.SEVERE, "Wrong cost for " + phoneNumber.toString() + + ": got " + exampleShortNumberCost + + ", expected: " + cost); } } }