Using possible length info in ShortNumberInfo (#1407)

Using new possible length information to calculate whether a short
number is the right length or not. This made us realise that the data
was missing for MY; this was added.

cpp/src/phonenumbers/shortnumberinfo.cc

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Author: David Yonge-Mallo
-
 #include "phonenumbers/shortnumberinfo.h"
 
 #include <algorithm>
@@ -33,6 +31,7 @@
 namespace i18n {
 namespace phonenumbers {
 
+using google::protobuf::RepeatedField;
 using std::map;
 using std::string;
 
@@ -81,16 +80,19 @@ const PhoneMetadata* ShortNumberInfo::GetMetadataForRegion(
 }
 
 namespace {
-// Same as the matchesPossibleNumberAndNationalNumber method in
-// java/libphonenumber/src/com/google/i18n/phonenumbers/ShortNumberInfo.java
 // TODO: Once we have benchmarked ShortNumberInfo, consider if it is
-// worth keeping this performance optimization, and if so move this into the
-// matcher implementation.
+// worth keeping this performance optimization.
 bool MatchesPossibleNumberAndNationalNumber(
-    const MatcherApi& matcher_api, const string& number,
-    const PhoneNumberDesc& number_desc) {
-  return matcher_api.MatchesPossibleNumber(number, number_desc) &&
-         matcher_api.MatchesNationalNumber(number, number_desc, false);
+    const MatcherApi& matcher_api,
+    const string& number,
+    const PhoneNumberDesc& desc) {
+  const RepeatedField<int>& lengths = desc.possible_length();
+  if (desc.possible_length_size() > 0 &&
+      std::find(lengths.begin(), lengths.end(), number.length()) ==
+          lengths.end()) {
+    return false;
+  }
+  return matcher_api.MatchesNationalNumber(number, desc, false);
 }
 }  // namespace
 
@@ -113,8 +115,10 @@ bool ShortNumberInfo::IsPossibleShortNumberForRegion(
   if (!phone_metadata) {
     return false;
   }
-  const PhoneNumberDesc& general_desc = phone_metadata->general_desc();
-  return matcher_api_->MatchesPossibleNumber(short_number, general_desc);
+  const RepeatedField<int>& lengths =
+      phone_metadata->general_desc().possible_length();
+  return std::find(lengths.begin(), lengths.end(), short_number.length()) !=
+      lengths.end();
 }
 
 bool ShortNumberInfo::IsPossibleShortNumberForRegion(const PhoneNumber& number,
@@ -129,8 +133,10 @@ bool ShortNumberInfo::IsPossibleShortNumberForRegion(const PhoneNumber& number,
   }
   string short_number;
   phone_util_.GetNationalSignificantNumber(number, &short_number);
-  const PhoneNumberDesc& general_desc = phone_metadata->general_desc();
-  return matcher_api_->MatchesPossibleNumber(short_number, general_desc);
+  const RepeatedField<int>& lengths =
+      phone_metadata->general_desc().possible_length();
+  return (std::find(lengths.begin(), lengths.end(), short_number.length()) !=
+      lengths.end());
 }
 
 bool ShortNumberInfo::IsPossibleShortNumber(const PhoneNumber& number) const {
@@ -145,8 +151,10 @@ bool ShortNumberInfo::IsPossibleShortNumber(const PhoneNumber& number) const {
     if (!phone_metadata) {
       continue;
     }
-    if (matcher_api_->MatchesPossibleNumber(short_number,
-                                            phone_metadata->general_desc())) {
+    const RepeatedField<int>& lengths =
+        phone_metadata->general_desc().possible_length();
+    if (std::find(lengths.begin(), lengths.end(), short_number.length()) !=
+        lengths.end()) {
       return true;
     }
   }
@@ -212,9 +220,17 @@ ShortNumberInfo::ShortNumberCost ShortNumberInfo::GetExpectedCostForRegion(
     return ShortNumberInfo::UNKNOWN_COST;
   }
 
-  // The cost categories are tested in order of decreasing expense, since if
-  // for some reason the patterns overlap the most expensive matching cost
-  // category should be returned.
+  // The possible lengths are not present for a particular sub-type if they
+  // match the general description; for this reason, we check the possible
+  // lengths against the general description first to allow an early exit if
+  // possible.
+  const RepeatedField<int>& lengths =
+      phone_metadata->general_desc().possible_length();
+  if (std::find(lengths.begin(), lengths.end(), short_number.length()) ==
+      lengths.end()) {
+    return ShortNumberInfo::UNKNOWN_COST;
+  }
+
   if (MatchesPossibleNumberAndNationalNumber(*matcher_api_, short_number,
                                              phone_metadata->premium_rate())) {
     return ShortNumberInfo::PREMIUM_RATE;
@@ -247,6 +263,17 @@ ShortNumberInfo::ShortNumberCost ShortNumberInfo::GetExpectedCostForRegion(
   string short_number;
   phone_util_.GetNationalSignificantNumber(number, &short_number);
 
+  // The possible lengths are not present for a particular sub-type if they
+  // match the general description; for this reason, we check the possible
+  // lengths against the general description first to allow an early exit if
+  // possible.
+  const RepeatedField<int>& lengths =
+      phone_metadata->general_desc().possible_length();
+  if (std::find(lengths.begin(), lengths.end(), short_number.length()) ==
+      lengths.end()) {
+    return ShortNumberInfo::UNKNOWN_COST;
+  }
+
   // The cost categories are tested in order of decreasing expense, since if
   // for some reason the patterns overlap the most expensive matching cost
   // category should be returned.

java/libphonenumber/src/com/google/i18n/phonenumbers/ShortNumberInfo.java

@@ -70,8 +70,7 @@ public class ShortNumberInfo {
   }
 
   // MatcherApi supports the basic matching method for checking if a given national number matches
-  // a national number patten or a possible number patten defined in the given
-  // {@code PhoneNumberDesc}.
+  // a national number pattern defined in the given {@code PhoneNumberDesc}.
   private final MatcherApi matcherApi;
 
   // A mapping from a country calling code to the region codes which denote the region represented
@@ -130,7 +129,7 @@ public class ShortNumberInfo {
     if (phoneMetadata == null) {
       return false;
     }
-    return matcherApi.matchesPossibleNumber(shortNumber, phoneMetadata.getGeneralDesc());
+    return phoneMetadata.getGeneralDesc().getPossibleLengthList().contains(shortNumber.length());
   }
 
   /**
@@ -150,8 +149,8 @@ public class ShortNumberInfo {
     if (phoneMetadata == null) {
       return false;
     }
-    return matcherApi.matchesPossibleNumber(getNationalSignificantNumber(number),
-        phoneMetadata.getGeneralDesc());
+    int numberLength = getNationalSignificantNumber(number).length();
+    return phoneMetadata.getGeneralDesc().getPossibleLengthList().contains(numberLength);
   }
 
   /**
@@ -165,13 +164,13 @@ public class ShortNumberInfo {
    */
   public boolean isPossibleShortNumber(PhoneNumber number) {
     List<String> regionCodes = getRegionCodesForCountryCode(number.getCountryCode());
-    String shortNumber = getNationalSignificantNumber(number);
+    int shortNumberLength = getNationalSignificantNumber(number).length();
     for (String region : regionCodes) {
       PhoneMetadata phoneMetadata = MetadataManager.getShortNumberMetadataForRegion(region);
       if (phoneMetadata == null) {
         continue;
       }
-      if (matcherApi.matchesPossibleNumber(shortNumber, phoneMetadata.getGeneralDesc())) {
+      if (phoneMetadata.getGeneralDesc().getPossibleLengthList().contains(shortNumberLength)) {
         return true;
       }
     }
@@ -290,6 +289,13 @@ public class ShortNumberInfo {
       return ShortNumberCost.UNKNOWN_COST;
     }
 
+    // The possible lengths are not present for a particular sub-type if they match the general
+    // description; for this reason, we check the possible lengths against the general description
+    // first to allow an early exit if possible.
+    if (!phoneMetadata.getGeneralDesc().getPossibleLengthList().contains(shortNumber.length())) {
+      return ShortNumberCost.UNKNOWN_COST;
+    }
+
     // The cost categories are tested in order of decreasing expense, since if for some reason the
     // patterns overlap the most expensive matching cost category should be returned.
     if (matchesPossibleNumberAndNationalNumber(shortNumber, phoneMetadata.getPremiumRate())) {
@@ -344,6 +350,13 @@ public class ShortNumberInfo {
 
     String shortNumber = getNationalSignificantNumber(number);
 
+    // The possible lengths are not present for a particular sub-type if they match the general
+    // description; for this reason, we check the possible lengths against the general description
+    // first to allow an early exit if possible.
+    if (!phoneMetadata.getGeneralDesc().getPossibleLengthList().contains(shortNumber.length())) {
+      return ShortNumberCost.UNKNOWN_COST;
+    }
+
     // The cost categories are tested in order of decreasing expense, since if for some reason the
     // patterns overlap the most expensive matching cost category should be returned.
     if (matchesPossibleNumberAndNationalNumber(shortNumber, phoneMetadata.getPremiumRate())) {
@@ -599,10 +612,13 @@ public class ShortNumberInfo {
   }
 
   // TODO: Once we have benchmarked ShortNumberInfo, consider if it is worth keeping
-  // this performance optimization, and if so move this into the matcher implementation.
+  // this performance optimization.
   private boolean matchesPossibleNumberAndNationalNumber(String number,
       PhoneNumberDesc numberDesc) {
-    return matcherApi.matchesPossibleNumber(number, numberDesc)
-        && matcherApi.matchesNationalNumber(number, numberDesc, false);
+    if (numberDesc.getPossibleLengthCount() > 0
+        && !numberDesc.getPossibleLengthList().contains(number.length())) {
+      return false;
+    }
+    return matcherApi.matchesNationalNumber(number, numberDesc, false);
   }
 }

java/pending_code_changes.txt

@@ -1 +1,5 @@
-
+Code changes:
+ - Using new possibleLengthInfo to decide whether a short number is the right
+   length or not. This could result in more specific results; whereas before, a
+   number from length 3 to length 6 may have been deemed possible, now we may
+   exclude a number of length 5.

resources/ShortNumberMetadata.xml

@@ -7591,6 +7591,7 @@
           112|
           999
         </nationalNumberPattern>
+        <possibleLengths national="3"/>
         <exampleNumber>999</exampleNumber>
       </shortCode>
       <emergency>
@@ -7598,6 +7599,7 @@
           112|
           999
         </nationalNumberPattern>
+        <possibleLengths national="3"/>
         <exampleNumber>999</exampleNumber>
       </emergency>
     </territory>