Converting phone number from hint selector to e164 optimistically

ashwinraghav · ashwinraghav · commit bc672c4bb2cc · 2017-05-19T10:14:47.000-07:00
Change-Id: Icd1e115e51c37c1b599166422be6072fc0fd4f4c
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/CountryListSpinner.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/CountryListSpinner.java
@@ -67,11 +67,8 @@ private void init() {
         dialogPopup = new DialogPopup(countryListAdapter);
         textFormat = "%1$s  +%2$d";
         selectedCountryName = "";
-        final Locale defaultLocale = Locale.getDefault();
-        final int defaultCountryCode =
-                PhoneNumberUtils.getCountryCode(defaultLocale.getCountry());
-
-        setSpinnerText(defaultCountryCode, defaultLocale);
+        final CountryInfo countryInfo = PhoneNumberUtils.getCurrentCountryInfo(getContext());
+        setSpinnerText(countryInfo.countryCode, countryInfo.locale);
     }
 
     private void setSpinnerText(int countryCode, Locale locale) {
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneNumberUtils.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneNumberUtils.java
@@ -17,8 +17,12 @@
  */
 package com.firebase.ui.auth.ui.phone;
 
+import android.content.Context;
 import android.os.Build;
+import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.telephony.TelephonyManager;
+import android.text.TextUtils;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -29,9 +33,11 @@
 import java.util.concurrent.ConcurrentHashMap;
 
 final class PhoneNumberUtils {
-    private final static String DEFAULT_COUNTRY_CODE = "1";
     private final static int DEFAULT_COUNTRY_CODE_INT = 1;
-    private final static String DEFAULT_COUNTRY_ISO = "US";
+    private final static String DEFAULT_COUNTRY_CODE = String.valueOf(DEFAULT_COUNTRY_CODE_INT);
+    private final static Locale DEFAULT_LOCALE = Locale.US;
+    private final static CountryInfo DEFAULT_COUNTRY =
+            new CountryInfo(DEFAULT_LOCALE, DEFAULT_COUNTRY_CODE_INT);
 
     private final static int MAX_COUNTRIES = 291;
     private final static int MAX_COUNTRY_CODES = 286;
@@ -54,27 +60,75 @@ final class PhoneNumberUtils {
     static void load() {
     }
 
-
+    /**
+     * This method works as follow:
+     * <ol><li>When the android version is LOLLIPOP or greater, the reliable
+     * {{@link android.telephony.PhoneNumberUtils#formatNumberToE164}} is used to format.</li>
+     * <li>For lower versions, we construct a value with the input phone number stripped of
+     * non numeric characters and prefix it with a "+" and country code</li>
+     * </ol>
+
+     * @param phoneNumber that may or may not itself have country code
+     * @param countryInfo must have locale with ISO 3166 2-letter code for country
+     * @return
+     */
     @Nullable
-    static String normalize(String phone) {
+    static String formatPhoneNumber(@NonNull String phoneNumber, @NonNull CountryInfo countryInfo) {
+
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
-            return android.telephony.PhoneNumberUtils.isGlobalPhoneNumber(phone) ? android
-                    .telephony.PhoneNumberUtils.normalizeNumber(phone) : null;
+            return android.telephony.PhoneNumberUtils
+                    .formatNumberToE164(phoneNumber,countryInfo.locale.getCountry());
         }
-        return android.telephony.PhoneNumberUtils.isGlobalPhoneNumber(phone) ? phone : null;
+        return phoneNumber.startsWith("+")
+                ? phoneNumber
+                : ("+" + String.valueOf(countryInfo.countryCode)
+                        + phoneNumber.replaceAll("[^\\d.]", ""));
+    }
+
+
+    /**
+     * This method uses the country returned by  {@link #getCurrentCountryInfo(Context)} to
+     * format the phone number. Internall invokes {@link #formatPhoneNumber(String, CountryInfo)}
+     * @param phoneNumber that may or may not itself have country code
+     * @return
+     */
+
+    @Nullable
+    static String formatPhoneNumberUsingCurrentCountry(
+            @NonNull String phoneNumber, Context context) {
+        final CountryInfo currentCountry = PhoneNumberUtils.getCurrentCountryInfo(context);
+        return formatPhoneNumber(phoneNumber, currentCountry);
+    }
+
+    @NonNull
+    static CountryInfo getCurrentCountryInfo(@NonNull Context context) {
+        Locale locale = getSimBasedLocale(context);
+
+        if (locale == null) {
+            locale = getOSLocale();
+        }
+
+        if (locale == null) {
+            return DEFAULT_COUNTRY;
+        }
+
+        Integer countryCode = PhoneNumberUtils.getCountryCode(locale.getCountry());
+
+        return countryCode == null ? DEFAULT_COUNTRY : new CountryInfo(locale, countryCode);
     }
 
     /**
      * This method should not be called on UI thread. Potentially creates a country code by iso
-     * * map which can take long in some devices
-     * TODO(ashwinraghav)
+     * map which can take long in some devices
+     * @param providedPhoneNumber works best when formatted as e164
      *
      * @return an instance of the PhoneNumber using the SIM information
      */
 
-    protected static PhoneNumber getPhoneNumber(String providedPhoneNumber) {
+    protected static PhoneNumber getPhoneNumber(@NonNull String providedPhoneNumber) {
         String countryCode = DEFAULT_COUNTRY_CODE;
-        String countryIso = DEFAULT_COUNTRY_ISO;
+        String countryIso = DEFAULT_LOCALE.getCountry();
+
         String phoneNumber = providedPhoneNumber;
         if (providedPhoneNumber.startsWith("+")) {
             countryCode = countryCodeForPhoneNumber(providedPhoneNumber);
@@ -89,7 +143,7 @@ private static String countryIsoForCountryCode(String countryCode) {
         if (countries != null) {
             return countries.get(0);
         }
-        return DEFAULT_COUNTRY_ISO;
+        return DEFAULT_LOCALE.getCountry();
     }
 
     /**
@@ -1272,16 +1326,25 @@ private synchronized static Map<String, Integer> createCountryCodeByIsoMap() {
         return countryCodeByIso;
     }
 
-    public static int getCountryCode(String countryIso) {
-        if(countryIso == null){
-            countryIso = DEFAULT_COUNTRY_ISO;
-        }
-        Integer countryCode = CountryCodeByIsoMap.get(countryIso.toUpperCase(Locale.getDefault()));
-
-        return countryCode == null ? DEFAULT_COUNTRY_CODE_INT : countryCode;
+    @Nullable
+    public static Integer getCountryCode(String countryIso) {
+        return countryIso == null
+                ? null
+                : CountryCodeByIsoMap.get(countryIso.toUpperCase(Locale.getDefault()));
     }
 
     private static String stripCountryCode(String phoneNumber, String countryCode) {
         return phoneNumber.replaceFirst("^\\+?" + countryCode, "");
     }
+
+    private static Locale getSimBasedLocale(@NonNull Context context) {
+        final TelephonyManager tm =
+                (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE);
+        final String countryIso = tm != null ? tm.getSimCountryIso() : null;
+        return TextUtils.isEmpty(countryIso) ? null : new Locale("", countryIso);
+    }
+
+    private static Locale getOSLocale() {
+        return Locale.getDefault();
+    }
 }
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java
@@ -106,6 +106,8 @@ public void onActivityCreated(@Nullable Bundle savedInstanceState) {
         }
 
         // Check for phone
+        // It is assumed that the phone number that are being wired in via Credential Selector
+        // are e164 since we store it.
         String phone = getArguments().getString(ExtraConstants.EXTRA_PHONE);
         if (!TextUtils.isEmpty(phone)) {
             // Use phone passed in
@@ -125,11 +127,22 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
             if (data != null) {
                 Credential cred = data.getParcelableExtra(Credential.EXTRA_KEY);
                 if (cred != null) {
-                    // Hint provided by telephony only sometimes contains country codes
-                    // This is gracefully handled by the PhoneNumberUtils
-                    PhoneNumber phoneNumber = PhoneNumberUtils.getPhoneNumber(cred.getId());
-                    setPhoneNumber(phoneNumber);
-                    setCountryCode(phoneNumber);
+                    // Hint selector does not always return phone numbers in e164 format.
+                    // To accommodate either case, we normalize to e164 with best effort
+                    final String unformattedPhone = cred.getId();
+                    final String formattedPhone =
+                            PhoneNumberUtils
+                                    .formatPhoneNumberUsingCurrentCountry(unformattedPhone,
+                                                                          getContext());
+                    if(formattedPhone == null) {
+                        Log.e(TAG, "Unable to normalize phone number from hint selector:"
+                                + unformattedPhone);
+                        return;
+                    }
+                    final PhoneNumber phoneNumberObj =
+                            PhoneNumberUtils.getPhoneNumber(formattedPhone);
+                    setPhoneNumber(phoneNumberObj);
+                    setCountryCode(phoneNumberObj);
                 }
             }
         }
@@ -147,16 +160,14 @@ public void onClick(View v) {
 
     @Nullable
     String getPseudoValidPhoneNumber() {
-        final int countryCode = ((CountryInfo) countryListSpinner.getTag()).countryCode;
+        final CountryInfo countryInfo = (CountryInfo) countryListSpinner.getTag();
         final String everythingElse = mPhoneEditText.getText().toString();
 
         if (TextUtils.isEmpty(everythingElse)) {
             return null;
         }
 
-        String ret = "+" + String.valueOf(countryCode) + everythingElse.replaceAll("[^\\d.]", "");
-
-        return PhoneNumberUtils.normalize(ret);
+        return PhoneNumberUtils.formatPhoneNumber(everythingElse, countryInfo);
     }
 
     private void setUpCountrySpinner() {
diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/phone/PhoneNumberUtilsTest.java b/auth/src/test/java/com/firebase/ui/auth/ui/phone/PhoneNumberUtilsTest.java
@@ -18,34 +18,42 @@
 
 package com.firebase.ui.auth.ui.phone;
 
+import android.content.Context;
+import android.telephony.TelephonyManager;
+
 import com.firebase.ui.auth.BuildConfig;
 import com.firebase.ui.auth.testhelpers.CustomRobolectricGradleTestRunner;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 
 import java.util.Locale;
 
+import static com.firebase.ui.auth.ui.phone.PhoneNumberUtils.*;
 import static com.firebase.ui.auth.ui.phone.PhoneTestConstants.RAW_PHONE;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.robolectric.Shadows.shadowOf;
 
 @RunWith(CustomRobolectricGradleTestRunner.class)
 @Config(constants = BuildConfig.class, sdk = 21)
 public class PhoneNumberUtilsTest {
-    private static final String INVENTED_ISO = "random";
-
     @Test
     public void testGetPhoneNumber() throws Exception {
-        final PhoneNumber number = PhoneNumberUtils.getPhoneNumber(RAW_PHONE);
+        final PhoneNumber number = getPhoneNumber(RAW_PHONE);
         assertEquals(PhoneTestConstants.PHONE_NO_COUNTRY_CODE, number.getPhoneNumber());
         assertEquals(PhoneTestConstants.US_COUNTRY_CODE, number.getCountryCode());
         assertEquals(PhoneTestConstants.US_ISO2, number.getCountryIso());
     }
 
     @Test
     public void testGetPhoneNumber_withLongestCountryCode() throws Exception {
-        final PhoneNumber phoneNumber = PhoneNumberUtils.getPhoneNumber(PhoneTestConstants
+        final PhoneNumber phoneNumber = getPhoneNumber(PhoneTestConstants
                 .YE_RAW_PHONE);
         assertEquals(PhoneTestConstants.PHONE_NO_COUNTRY_CODE, phoneNumber.getPhoneNumber());
         assertEquals(PhoneTestConstants.YE_COUNTRY_CODE, phoneNumber.getCountryCode());
@@ -54,15 +62,15 @@ public void testGetPhoneNumber_withLongestCountryCode() throws Exception {
 
     @Test
     public void testGetPhoneNumber_withPhoneWithoutPlusSign() throws Exception {
-        final PhoneNumber phoneNumber = PhoneNumberUtils.getPhoneNumber(PhoneTestConstants.PHONE);
+        final PhoneNumber phoneNumber = getPhoneNumber(PhoneTestConstants.PHONE);
         assertEquals(PhoneTestConstants.PHONE, phoneNumber.getPhoneNumber());
         assertEquals(PhoneTestConstants.US_COUNTRY_CODE, phoneNumber.getCountryCode());
         assertEquals(PhoneTestConstants.US_ISO2, phoneNumber.getCountryIso());
     }
 
     @Test
     public void testGetPhoneNumber_noCountryCode() throws Exception {
-        final PhoneNumber number = PhoneNumberUtils.getPhoneNumber("0" + PhoneTestConstants
+        final PhoneNumber number = getPhoneNumber("0" + PhoneTestConstants
                 .PHONE_NO_COUNTRY_CODE);
         assertEquals("0" + PhoneTestConstants.PHONE_NO_COUNTRY_CODE, number.getPhoneNumber());
         assertEquals(PhoneTestConstants.US_COUNTRY_CODE, number.getCountryCode());
@@ -71,8 +79,94 @@ public void testGetPhoneNumber_noCountryCode() throws Exception {
 
     @Test
     public void testGetCountryCode() throws Exception {
-        assertEquals(86, PhoneNumberUtils.getCountryCode(Locale.CHINA.getCountry()));
-        assertEquals(1, PhoneNumberUtils.getCountryCode(null));
-        assertEquals(1, PhoneNumberUtils.getCountryCode(new Locale("", "DJJZ").getCountry()));
+        assertEquals(new Integer(86), getCountryCode(Locale.CHINA.getCountry()));
+        assertEquals(null, getCountryCode(null));
+        assertEquals(null, getCountryCode(new Locale("", "DJJZ").getCountry()));
+    }
+
+    @Test
+    @Config(constants = BuildConfig.class, sdk = 21)
+    public void testFormatNumberToE164_aboveApi21() {
+        String validPhoneNumber = "+919994947354";
+        CountryInfo indiaCountryInfo = new CountryInfo(new Locale("", "IN"), 91);
+        //no leading plus
+        assertEquals(validPhoneNumber, formatPhoneNumber("9994947354", indiaCountryInfo));
+        //no country code
+        assertEquals(validPhoneNumber, formatPhoneNumber("919994947354", indiaCountryInfo));
+        //fully formatted
+        assertEquals(validPhoneNumber, formatPhoneNumber("+919994947354", indiaCountryInfo));
+        //with hyphens
+        assertEquals(validPhoneNumber, formatPhoneNumber("+91-(999)-(49)-(47354)", indiaCountryInfo));
+        //with spaces leading plus
+        assertEquals(validPhoneNumber, formatPhoneNumber("+91 99949 47354", indiaCountryInfo));
+        // space formatting
+        assertEquals(validPhoneNumber, formatPhoneNumber("91 99949 47354", indiaCountryInfo));
+        // parantheses and hyphens
+        assertEquals(validPhoneNumber, formatPhoneNumber("(99949) 47-354", indiaCountryInfo));
+        // mismatched country
+        assertEquals(validPhoneNumber, formatPhoneNumber("+919994947354",
+                                                         new CountryInfo(
+                                                                 new Locale("", "US"), 1)));
+        // incorrect country with well formatted number
+        assertNull(formatPhoneNumber("999474735", indiaCountryInfo));
+
+        // incorrect country with unformattednumber
+        assertNull(validPhoneNumber, formatPhoneNumber("919994947354",
+                                                         new CountryInfo(
+                                                                 new Locale("", "US"), 1)));
+        //incorrect country, incorrect phone number
+        assertNull(formatPhoneNumber("+914349873457", new CountryInfo(
+                new Locale("", "US"), 1)));
+    }
+
+    @Test
+    @Config(constants = BuildConfig.class, sdk = 16)
+    public void testFormatNumberToE164_belowApi21() {
+        String validPhoneNumber = "+919994947354";
+        CountryInfo indiaCountryInfo = new CountryInfo(new Locale("", "IN"), 91);
+        // no leading plus
+        assertEquals(validPhoneNumber, formatPhoneNumber("9994947354", indiaCountryInfo));
+        // fully formatted
+        assertEquals(validPhoneNumber, formatPhoneNumber("+919994947354", indiaCountryInfo));
+        // parantheses and hyphens
+        assertEquals(validPhoneNumber, formatPhoneNumber("(99949) 47-354", indiaCountryInfo));
+
+        // The following cases would fail for lower api versions.
+        // Leaving tests in place to formally identify cases
+
+        // no leading +
+        // assertEquals(validPhoneNumber, formatPhoneNumber("919994947354", indiaCountryInfo));
+
+        // with hyphens
+        // assertEquals(validPhoneNumber, formatPhoneNumber("+91-(999)-(49)-(47354)",
+        // indiaCountryInfo));
+
+        // with spaces leading plus
+        // assertEquals(validPhoneNumber, formatPhoneNumber("+91 99949 47354", indiaCountryInfo));
+
+        // space formatting
+        // assertEquals(validPhoneNumber, formatPhoneNumber("91 99949 47354", indiaCountryInfo));
+
+        // invalid phone number
+        // assertNull(formatPhoneNumber("999474735", indiaCountryInfo));
+    }
+
+    @Test
+    public void testGetCurrentCountryInfo_fromSim() {
+        Context context = mock(Context.class);
+        TelephonyManager telephonyManager = mock(TelephonyManager.class);
+
+        when(context.getSystemService(Context.TELEPHONY_SERVICE)).thenReturn(telephonyManager);
+        when(telephonyManager.getSimCountryIso()).thenReturn("IN");
+        assertEquals(new CountryInfo(new Locale("", "IN"), 91), getCurrentCountryInfo(context));
+    }
+
+    @Test
+    public void testGetCurrentCountryInfo_noTelephonyReturnsDefaultLocale() {
+        Context context = mock(Context.class);
+        assertEquals(new CountryInfo(
+                Locale.getDefault(),
+                PhoneNumberUtils.getCountryCode(Locale.getDefault().getCountry())),
+                getCurrentCountryInfo(context));
     }
 }
