From 6a802e58054c7c9da8245ba291c4d773b090543a Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 12 Jan 2021 15:34:45 +0100 Subject: Unbundle the Car Calendar App. Set the sdk_version to "system_current". Remove use of hidden APIs for phone number validation and formatting. Test: Manual testing on Hawk and updated and ran all tests. Bug: 176965230 Change-Id: Id33da19d10f0780c9f569dfd98957eb33fa6b00a --- Android.bp | 6 +- .../android/car/calendar/CarCalendarActivity.java | 25 ++++++-- .../android/car/calendar/CarCalendarViewModel.java | 13 +++-- .../android/car/calendar/EventCalendarItem.java | 2 +- .../car/calendar/common/EventDescriptions.java | 68 ++++++++++------------ .../android/car/calendar/CarCalendarUiTest.java | 4 +- .../car/calendar/common/EventDescriptionsTest.java | 23 +++++--- 7 files changed, 84 insertions(+), 57 deletions(-) diff --git a/Android.bp b/Android.bp index e79e37b..63f7ceb 100644 --- a/Android.bp +++ b/Android.bp @@ -17,14 +17,15 @@ android_app { name: "CarCalendarApp", srcs: ["src/**/*.java"], resource_dirs: ["res"], - platform_apis: true, + sdk_version: "system_current", + min_sdk_version: "29", optimize: { enabled: false, }, dex_preopt: { enabled: false, }, - aaptflags: ["--auto-add-overlay"], + libs: ["android.car-system-stubs"], static_libs: [ "car-ui-lib", "androidx.lifecycle_lifecycle-extensions", @@ -33,5 +34,4 @@ android_app { "androidx.lifecycle_lifecycle-viewmodel", "guava", ], - libs: ["android.car"], } diff --git a/src/com/android/car/calendar/CarCalendarActivity.java b/src/com/android/car/calendar/CarCalendarActivity.java index 97e2031..945482a 100644 --- a/src/com/android/car/calendar/CarCalendarActivity.java +++ b/src/com/android/car/calendar/CarCalendarActivity.java @@ -20,6 +20,7 @@ import android.content.ContentResolver; import android.content.pm.PackageManager; import android.os.Bundle; import android.os.StrictMode; +import android.telephony.TelephonyManager; import android.util.Log; import androidx.annotation.NonNull; @@ -58,7 +59,10 @@ public class CarCalendarActivity extends FragmentActivity { // Tests can set fake dependencies before onCreate. if (mDependencies == null) { mDependencies = new Dependencies( - Locale.getDefault(), Clock.systemDefaultZone(), getContentResolver()); + Locale.getDefault(), + Clock.systemDefaultZone(), + getContentResolver(), + getSystemService(TelephonyManager.class)); } CarCalendarViewModel carCalendarViewModel = @@ -67,6 +71,7 @@ public class CarCalendarActivity extends FragmentActivity { new CarCalendarViewModelFactory( mDependencies.mResolver, mDependencies.mLocale, + mDependencies.mTelephonyManager, mDependencies.mClock)) .get(CarCalendarViewModel.class); @@ -142,12 +147,18 @@ public class CarCalendarActivity extends FragmentActivity { private static class CarCalendarViewModelFactory implements ViewModelProvider.Factory { private final ContentResolver mResolver; + private final TelephonyManager mTelephonyManager; private final Locale mLocale; private final Clock mClock; - CarCalendarViewModelFactory(ContentResolver resolver, Locale locale, Clock clock) { + CarCalendarViewModelFactory( + ContentResolver resolver, + Locale locale, + TelephonyManager telephonyManager, + Clock clock) { mResolver = resolver; mLocale = locale; + mTelephonyManager = telephonyManager; mClock = clock; } @@ -155,7 +166,7 @@ public class CarCalendarActivity extends FragmentActivity { @NonNull @Override public T create(@NonNull Class aClass) { - return (T) new CarCalendarViewModel(mResolver, mLocale, mClock); + return (T) new CarCalendarViewModel(mResolver, mLocale, mTelephonyManager, mClock); } } @@ -163,11 +174,17 @@ public class CarCalendarActivity extends FragmentActivity { private final Locale mLocale; private final Clock mClock; private final ContentResolver mResolver; + private final TelephonyManager mTelephonyManager; - Dependencies(Locale locale, Clock clock, ContentResolver resolver) { + Dependencies( + Locale locale, + Clock clock, + ContentResolver resolver, + TelephonyManager telephonyManager) { mLocale = locale; mClock = clock; mResolver = resolver; + mTelephonyManager = telephonyManager; } } } diff --git a/src/com/android/car/calendar/CarCalendarViewModel.java b/src/com/android/car/calendar/CarCalendarViewModel.java index c8e80ee..337f794 100644 --- a/src/com/android/car/calendar/CarCalendarViewModel.java +++ b/src/com/android/car/calendar/CarCalendarViewModel.java @@ -17,7 +17,9 @@ package com.android.car.calendar; import android.content.ContentResolver; +import android.os.Handler; import android.os.HandlerThread; +import android.telephony.TelephonyManager; import android.util.Log; import androidx.annotation.Nullable; @@ -38,14 +40,17 @@ class CarCalendarViewModel extends ViewModel { private final Clock mClock; private final ContentResolver mResolver; private final Locale mLocale; + private final TelephonyManager mTelephonyManager; @Nullable private EventsLiveData mEventsLiveData; - CarCalendarViewModel(ContentResolver resolver, Locale locale, Clock clock) { + CarCalendarViewModel(ContentResolver resolver, Locale locale, + TelephonyManager telephonyManager, Clock clock) { + mLocale = locale; if (DEBUG) Log.d(TAG, "Creating view model"); mResolver = resolver; + mTelephonyManager = telephonyManager; mHandlerThread.start(); - mLocale = locale; mClock = clock; } @@ -55,9 +60,9 @@ class CarCalendarViewModel extends ViewModel { mEventsLiveData = new EventsLiveData( mClock, - mHandlerThread.getThreadHandler(), + new Handler(mHandlerThread.getLooper()), mResolver, - new EventDescriptions(mLocale), + new EventDescriptions(mLocale, mTelephonyManager), new EventLocations()); } return mEventsLiveData; diff --git a/src/com/android/car/calendar/EventCalendarItem.java b/src/com/android/car/calendar/EventCalendarItem.java index a7c023d..e83c21c 100644 --- a/src/com/android/car/calendar/EventCalendarItem.java +++ b/src/com/android/car/calendar/EventCalendarItem.java @@ -17,7 +17,6 @@ package com.android.car.calendar; import android.Manifest; -import android.annotation.ColorInt; import android.graphics.Paint; import android.graphics.Typeface; import android.graphics.drawable.InsetDrawable; @@ -34,6 +33,7 @@ import android.widget.ImageButton; import android.widget.TextView; import android.widget.Toast; +import androidx.annotation.ColorInt; import androidx.annotation.DrawableRes; import androidx.core.content.ContextCompat; import androidx.recyclerview.widget.RecyclerView; diff --git a/src/com/android/car/calendar/common/EventDescriptions.java b/src/com/android/car/calendar/common/EventDescriptions.java index e6aad5b..8d2030f 100644 --- a/src/com/android/car/calendar/common/EventDescriptions.java +++ b/src/com/android/car/calendar/common/EventDescriptions.java @@ -16,20 +16,15 @@ package com.android.car.calendar.common; -import static com.android.i18n.phonenumbers.PhoneNumberUtil.PhoneNumberFormat.INTERNATIONAL; -import static com.android.i18n.phonenumbers.PhoneNumberUtil.ValidationResult.IS_POSSIBLE; -import static com.android.i18n.phonenumbers.PhoneNumberUtil.ValidationResult.IS_POSSIBLE_LOCAL_ONLY; -import static com.android.i18n.phonenumbers.PhoneNumberUtil.ValidationResult.TOO_LONG; - import static com.google.common.base.Verify.verifyNotNull; import android.net.Uri; +import android.telephony.PhoneNumberUtils; +import android.telephony.TelephonyManager; import com.android.car.calendar.common.Dialer.NumberAndAccess; -import com.android.i18n.phonenumbers.NumberParseException; -import com.android.i18n.phonenumbers.PhoneNumberUtil; -import com.android.i18n.phonenumbers.Phonenumber; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import java.util.LinkedHashMap; @@ -45,37 +40,45 @@ import javax.annotation.Nullable; public class EventDescriptions { // Requires a phone number to include only numbers, spaces and dash, optionally a leading "+". - // The number must be at least 6 characters. + // The number must be at least 6 characters and can contain " " or "-" but end with a digit. // The access code must be at least 3 characters. // The number and the access to include "pin" or "code" between the numbers. private static final Pattern PHONE_PIN_PATTERN = Pattern.compile( - "(\\+?[\\d -]{6,})(?:.*\\b(?:PIN|code)\\b.*?([\\d,;#*]{3,}))?", + "(\\+?[\\d -]{6,}\\d)(?:.*\\b(?:PIN|code)\\b.*?([\\d,;#*]{3,}))?", Pattern.CASE_INSENSITIVE); // Matches numbers in the encoded format "". private static final Pattern TEL_PIN_PATTERN = Pattern.compile(""); - private static final PhoneNumberUtil PHONE_NUMBER_UTIL = PhoneNumberUtil.getInstance(); - // Ensure numbers are over 5 digits to reduce false positives. - private static final int MIN_NATIONAL_NUMBER = 10_000; + private static final int MIN_DIGITS = 5; - private final Locale mLocale; + private final String mCountryIso; - public EventDescriptions(Locale locale) { - mLocale = locale; + public EventDescriptions(Locale locale, TelephonyManager telephonyManager) { + String networkCountryIso = telephonyManager.getNetworkCountryIso().toUpperCase(); + if (!Strings.isNullOrEmpty(networkCountryIso)) { + mCountryIso = networkCountryIso; + } else { + mCountryIso = locale.getCountry(); + } } /** Find conference call data embedded in the description. */ public List extractNumberAndPins(String descriptionText) { String decoded = Uri.decode(descriptionText); + // Use a map keyed by number to act like a set and only add a single number. Map results = new LinkedHashMap<>(); addMatchedNumbers(decoded, results, PHONE_PIN_PATTERN); + + // Add the most restrictive precise format last to replace others with the same number. addMatchedNumbers(decoded, results, TEL_PIN_PATTERN); - return ImmutableList.copyOf(results.values()); + + // Reverse order so the most precise format is first. + return ImmutableList.copyOf(results.values()).reverse(); } private void addMatchedNumbers( @@ -93,27 +96,18 @@ public class EventDescriptions { private NumberAndAccess validNumberAndAccess(Matcher phoneFormatMatcher) { String number = verifyNotNull(phoneFormatMatcher.group(1)); String access = phoneFormatMatcher.group(2); - try { - Phonenumber.PhoneNumber phoneNumber = - PHONE_NUMBER_UTIL.parse(number, mLocale.getCountry()); - PhoneNumberUtil.ValidationResult result = - PHONE_NUMBER_UTIL.isPossibleNumberWithReason(phoneNumber); - if (isAcceptableResult(result)) { - if (phoneNumber.getNationalNumber() < MIN_NATIONAL_NUMBER) { - return null; - } - String formatted = PHONE_NUMBER_UTIL.format(phoneNumber, INTERNATIONAL); - return new NumberAndAccess(formatted, access); - } - } catch (NumberParseException e) { - // Ignore invalid numbers. + + // Ensure that there are a minimum number of digits to reduce false positives. + String onlyDigits = number.replaceAll("\\D", ""); + if (onlyDigits.length() < MIN_DIGITS) { + return null; } - return null; - } - private boolean isAcceptableResult(PhoneNumberUtil.ValidationResult result) { - // The result can be too long and still valid because the US locale is used by default - // which does not accept valid long numbers from other regions. - return result == IS_POSSIBLE || result == IS_POSSIBLE_LOCAL_ONLY || result == TOO_LONG; + // Keep local numbers in local format which the dialer can make more sense of. + String formatted = PhoneNumberUtils.formatNumber(number, mCountryIso); + if (formatted == null) { + return null; + } + return new NumberAndAccess(formatted, access); } } diff --git a/tests/ui/src/com/android/car/calendar/CarCalendarUiTest.java b/tests/ui/src/com/android/car/calendar/CarCalendarUiTest.java index 5c7883c..d342e3d 100644 --- a/tests/ui/src/com/android/car/calendar/CarCalendarUiTest.java +++ b/tests/ui/src/com/android/car/calendar/CarCalendarUiTest.java @@ -35,6 +35,7 @@ import android.net.Uri; import android.os.Bundle; import android.os.CancellationSignal; import android.provider.CalendarContract; +import android.telephony.TelephonyManager; import android.test.mock.MockContentProvider; import android.test.mock.MockContentResolver; @@ -122,7 +123,8 @@ public class CarCalendarUiTest { new TestCalendarContentProvider(context); mockContentResolver.addProvider(CalendarContract.AUTHORITY, testCalendarContentProvider); activity.mDependencies = - new CarCalendarActivity.Dependencies(LOCALE, fixedTimeClock, mockContentResolver); + new CarCalendarActivity.Dependencies(LOCALE, fixedTimeClock, mockContentResolver, + activity.getSystemService(TelephonyManager.class)); } private void observeEventsLiveData(CarCalendarActivity activity) { diff --git a/tests/unit/src/com/android/car/calendar/common/EventDescriptionsTest.java b/tests/unit/src/com/android/car/calendar/common/EventDescriptionsTest.java index 358e9cf..bea029f 100644 --- a/tests/unit/src/com/android/car/calendar/common/EventDescriptionsTest.java +++ b/tests/unit/src/com/android/car/calendar/common/EventDescriptionsTest.java @@ -18,28 +18,36 @@ package com.android.car.calendar.common; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.when; + import android.net.Uri; +import android.telephony.TelephonyManager; import com.google.common.collect.Iterables; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import java.io.UnsupportedEncodingException; import java.util.List; import java.util.Locale; public class EventDescriptionsTest { - private static final String BASE_NUMBER = "30 303986300"; private static final String LOCAL_NUMBER = "0" + BASE_NUMBER; private static final String INTERNATIONAL_NUMBER = "+49 " + BASE_NUMBER; private static final String ACCESS = ",,12;3*45#"; + private static final String COUNTRY_ISO_CODE = "DE"; + private EventDescriptions mEventDescriptions; + private TelephonyManager mMockTelephonyManager; @Before public void setUp() { - mEventDescriptions = new EventDescriptions(Locale.GERMANY); + mMockTelephonyManager = Mockito.mock(TelephonyManager.class); + when(mMockTelephonyManager.getNetworkCountryIso()).thenReturn(COUNTRY_ISO_CODE); + mEventDescriptions = new EventDescriptions(Locale.GERMANY, mMockTelephonyManager); } @Test @@ -48,7 +56,7 @@ public class EventDescriptionsTest { mEventDescriptions.extractNumberAndPins(LOCAL_NUMBER); assertThat(numberAndAccesses).isNotEmpty(); Dialer.NumberAndAccess numberAndAccess = Iterables.getFirst(numberAndAccesses, null); - assertThat(numberAndAccess.getNumber()).isEqualTo(INTERNATIONAL_NUMBER); + assertThat(numberAndAccess.getNumber()).isEqualTo(LOCAL_NUMBER); } @Test @@ -62,9 +70,10 @@ public class EventDescriptionsTest { @Test public void extractNumberAndPin_internationalNumberWithDifferentLocale_resultIsInternational() { - mEventDescriptions = new EventDescriptions(Locale.FRANCE); + EventDescriptions eventDescriptions = + new EventDescriptions(Locale.FRANCE, mMockTelephonyManager); List numberAndAccesses = - mEventDescriptions.extractNumberAndPins(INTERNATIONAL_NUMBER); + eventDescriptions.extractNumberAndPins(INTERNATIONAL_NUMBER); assertThat(numberAndAccesses).isNotEmpty(); Dialer.NumberAndAccess numberAndAccess = Iterables.getFirst(numberAndAccesses, null); assertThat(numberAndAccess.getNumber()).isEqualTo(INTERNATIONAL_NUMBER); @@ -105,8 +114,8 @@ public class EventDescriptionsTest { List numberAndAccesses = mEventDescriptions.extractNumberAndPins(input); - // The local number is valid but repeated so only included once. - assertThat(numberAndAccesses).hasSize(1); + // Keep all variations of a base number. + assertThat(numberAndAccesses).hasSize(3); } @Test -- cgit v1.2.3