diff options
2 files changed, 135 insertions, 113 deletions
diff --git a/services/core/java/com/android/server/notification/ValidateNotificationPeople.java b/services/core/java/com/android/server/notification/ValidateNotificationPeople.java index 5e0a18039152..8417049bf297 100644 --- a/services/core/java/com/android/server/notification/ValidateNotificationPeople.java +++ b/services/core/java/com/android/server/notification/ValidateNotificationPeople.java @@ -62,7 +62,7 @@ import java.util.concurrent.TimeUnit; public class ValidateNotificationPeople implements NotificationSignalExtractor { // Using a shorter log tag since setprop has a limit of 32chars on variable name. private static final String TAG = "ValidateNoPeople"; - private static final boolean VERBOSE = Log.isLoggable(TAG, Log.VERBOSE);; + private static final boolean VERBOSE = Log.isLoggable(TAG, Log.VERBOSE); private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private static final boolean ENABLE_PEOPLE_VALIDATOR = true; @@ -105,12 +105,13 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { private int mEvictionCount; private NotificationUsageStats mUsageStats; + @Override public void initialize(Context context, NotificationUsageStats usageStats) { if (DEBUG) Slog.d(TAG, "Initializing " + getClass().getSimpleName() + "."); mUserToContextMap = new ArrayMap<>(); mBaseContext = context; mUsageStats = usageStats; - mPeopleCache = new LruCache<String, LookupResult>(PEOPLE_CACHE_SIZE); + mPeopleCache = new LruCache<>(PEOPLE_CACHE_SIZE); mEnabled = ENABLE_PEOPLE_VALIDATOR && 1 == Settings.Global.getInt( mBaseContext.getContentResolver(), SETTING_ENABLE_PEOPLE_VALIDATOR, 1); if (mEnabled) { @@ -134,7 +135,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { // For tests: just do the setting of various local variables without actually doing work @VisibleForTesting protected void initForTests(Context context, NotificationUsageStats usageStats, - LruCache peopleCache) { + LruCache<String, LookupResult> peopleCache) { mUserToContextMap = new ArrayMap<>(); mBaseContext = context; mUsageStats = usageStats; @@ -142,6 +143,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { mEnabled = true; } + @Override public RankingReconsideration process(NotificationRecord record) { if (!mEnabled) { if (VERBOSE) Slog.i(TAG, "disabled"); @@ -272,7 +274,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } if (VERBOSE) Slog.i(TAG, "Validating: " + key + " for " + context.getUserId()); - final LinkedList<String> pendingLookups = new LinkedList<String>(); + final LinkedList<String> pendingLookups = new LinkedList<>(); int personIdx = 0; for (String handle : people) { if (TextUtils.isEmpty(handle)) continue; @@ -320,7 +322,6 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { return Integer.toString(userId) + ":" + handle; } - // VisibleForTesting public static String[] getExtraPeople(Bundle extras) { String[] peopleList = getExtraPeopleForKey(extras, Notification.EXTRA_PEOPLE_LIST); String[] legacyPeople = getExtraPeopleForKey(extras, Notification.EXTRA_PEOPLE); @@ -417,101 +418,6 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { return null; } - private LookupResult resolvePhoneContact(Context context, final String number) { - Uri phoneUri = Uri.withAppendedPath(ContactsContract.PhoneLookup.CONTENT_FILTER_URI, - Uri.encode(number)); - return searchContacts(context, phoneUri); - } - - private LookupResult resolveEmailContact(Context context, final String email) { - Uri numberUri = Uri.withAppendedPath( - ContactsContract.CommonDataKinds.Email.CONTENT_LOOKUP_URI, - Uri.encode(email)); - return searchContacts(context, numberUri); - } - - @VisibleForTesting - LookupResult searchContacts(Context context, Uri lookupUri) { - LookupResult lookupResult = new LookupResult(); - final Uri corpLookupUri = - ContactsContract.Contacts.createCorpLookupUriFromEnterpriseLookupUri(lookupUri); - if (corpLookupUri == null) { - addContacts(lookupResult, context, lookupUri); - } else { - addWorkContacts(lookupResult, context, corpLookupUri); - } - return lookupResult; - } - - @VisibleForTesting - // Performs a contacts search using searchContacts, and then follows up by looking up - // any phone numbers associated with the resulting contact information and merge those - // into the lookup result as well. Will have no additional effect if the contact does - // not have any phone numbers. - LookupResult searchContactsAndLookupNumbers(Context context, Uri lookupUri) { - LookupResult lookupResult = searchContacts(context, lookupUri); - String phoneLookupKey = lookupResult.getPhoneLookupKey(); - if (phoneLookupKey != null) { - String selection = Contacts.LOOKUP_KEY + " = ?"; - String[] selectionArgs = new String[] { phoneLookupKey }; - try (Cursor cursor = context.getContentResolver().query( - ContactsContract.CommonDataKinds.Phone.CONTENT_URI, PHONE_LOOKUP_PROJECTION, - selection, selectionArgs, /* sortOrder= */ null)) { - if (cursor == null) { - Slog.w(TAG, "Cursor is null when querying contact phone number."); - return lookupResult; - } - - while (cursor.moveToNext()) { - lookupResult.mergePhoneNumber(cursor); - } - } catch (Throwable t) { - Slog.w(TAG, "Problem getting content resolver or querying phone numbers.", t); - } - } - return lookupResult; - } - - private void addWorkContacts(LookupResult lookupResult, Context context, Uri corpLookupUri) { - final int workUserId = findWorkUserId(context); - if (workUserId == -1) { - Slog.w(TAG, "Work profile user ID not found for work contact: " + corpLookupUri); - return; - } - final Uri corpLookupUriWithUserId = - ContentProvider.maybeAddUserId(corpLookupUri, workUserId); - addContacts(lookupResult, context, corpLookupUriWithUserId); - } - - /** Returns the user ID of the managed profile or -1 if none is found. */ - private int findWorkUserId(Context context) { - final UserManager userManager = context.getSystemService(UserManager.class); - final int[] profileIds = - userManager.getProfileIds(context.getUserId(), /* enabledOnly= */ true); - for (int profileId : profileIds) { - if (userManager.isManagedProfile(profileId)) { - return profileId; - } - } - return -1; - } - - /** Modifies the given lookup result to add contacts found at the given URI. */ - private void addContacts(LookupResult lookupResult, Context context, Uri uri) { - try (Cursor c = context.getContentResolver().query( - uri, LOOKUP_PROJECTION, null, null, null)) { - if (c == null) { - Slog.w(TAG, "Null cursor from contacts query."); - return; - } - while (c.moveToNext()) { - lookupResult.mergeContact(c); - } - } catch (Throwable t) { - Slog.w(TAG, "Problem getting content resolver or performing contacts query.", t); - } - } - @VisibleForTesting protected static class LookupResult { private static final long CONTACT_REFRESH_MILLIS = 60 * 60 * 1000; // 1hr @@ -619,19 +525,18 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } } - private class PeopleRankingReconsideration extends RankingReconsideration { + @VisibleForTesting + class PeopleRankingReconsideration extends RankingReconsideration { private final LinkedList<String> mPendingLookups; private final Context mContext; - // Amount of time to wait for a result from the contacts db before rechecking affinity. - private static final long LOOKUP_TIME = 1000; private float mContactAffinity = NONE; private ArraySet<String> mPhoneNumbers = null; private NotificationRecord mRecord; private PeopleRankingReconsideration(Context context, String key, LinkedList<String> pendingLookups) { - super(key, LOOKUP_TIME); + super(key); mContext = context; mPendingLookups = pendingLookups; } @@ -642,7 +547,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { long timeStartMs = System.currentTimeMillis(); for (final String handle: mPendingLookups) { final String cacheKey = getCacheKey(mContext.getUserId(), handle); - LookupResult lookupResult = null; + LookupResult lookupResult; boolean cacheHit = false; synchronized (mPeopleCache) { lookupResult = mPeopleCache.get(cacheKey); @@ -703,6 +608,102 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } } + private static LookupResult resolvePhoneContact(Context context, final String number) { + Uri phoneUri = Uri.withAppendedPath(ContactsContract.PhoneLookup.CONTENT_FILTER_URI, + Uri.encode(number)); + return searchContacts(context, phoneUri); + } + + private static LookupResult resolveEmailContact(Context context, final String email) { + Uri numberUri = Uri.withAppendedPath( + ContactsContract.CommonDataKinds.Email.CONTENT_LOOKUP_URI, + Uri.encode(email)); + return searchContacts(context, numberUri); + } + + @VisibleForTesting + static LookupResult searchContacts(Context context, Uri lookupUri) { + LookupResult lookupResult = new LookupResult(); + final Uri corpLookupUri = + ContactsContract.Contacts.createCorpLookupUriFromEnterpriseLookupUri(lookupUri); + if (corpLookupUri == null) { + addContacts(lookupResult, context, lookupUri); + } else { + addWorkContacts(lookupResult, context, corpLookupUri); + } + return lookupResult; + } + + @VisibleForTesting + // Performs a contacts search using searchContacts, and then follows up by looking up + // any phone numbers associated with the resulting contact information and merge those + // into the lookup result as well. Will have no additional effect if the contact does + // not have any phone numbers. + static LookupResult searchContactsAndLookupNumbers(Context context, Uri lookupUri) { + LookupResult lookupResult = searchContacts(context, lookupUri); + String phoneLookupKey = lookupResult.getPhoneLookupKey(); + if (phoneLookupKey != null) { + String selection = Contacts.LOOKUP_KEY + " = ?"; + String[] selectionArgs = new String[] { phoneLookupKey }; + try (Cursor cursor = context.getContentResolver().query( + ContactsContract.CommonDataKinds.Phone.CONTENT_URI, PHONE_LOOKUP_PROJECTION, + selection, selectionArgs, /* sortOrder= */ null)) { + if (cursor == null) { + Slog.w(TAG, "Cursor is null when querying contact phone number."); + return lookupResult; + } + + while (cursor.moveToNext()) { + lookupResult.mergePhoneNumber(cursor); + } + } catch (Throwable t) { + Slog.w(TAG, "Problem getting content resolver or querying phone numbers.", t); + } + } + return lookupResult; + } + + private static void addWorkContacts(LookupResult lookupResult, Context context, + Uri corpLookupUri) { + final int workUserId = findWorkUserId(context); + if (workUserId == -1) { + Slog.w(TAG, "Work profile user ID not found for work contact: " + corpLookupUri); + return; + } + final Uri corpLookupUriWithUserId = + ContentProvider.maybeAddUserId(corpLookupUri, workUserId); + addContacts(lookupResult, context, corpLookupUriWithUserId); + } + + /** Returns the user ID of the managed profile or -1 if none is found. */ + private static int findWorkUserId(Context context) { + final UserManager userManager = context.getSystemService(UserManager.class); + final int[] profileIds = + userManager.getProfileIds(context.getUserId(), /* enabledOnly= */ true); + for (int profileId : profileIds) { + if (userManager.isManagedProfile(profileId)) { + return profileId; + } + } + return -1; + } + + /** Modifies the given lookup result to add contacts found at the given URI. */ + private static void addContacts(LookupResult lookupResult, Context context, Uri uri) { + try (Cursor c = context.getContentResolver().query( + uri, LOOKUP_PROJECTION, null, null, null)) { + if (c == null) { + Slog.w(TAG, "Null cursor from contacts query."); + return; + } + while (c.moveToNext()) { + lookupResult.mergeContact(c); + } + } catch (Throwable t) { + Slog.w(TAG, "Problem getting content resolver or performing contacts query.", t); + } + } + @Override public void applyChangesLocked(NotificationRecord operand) { float affinityBound = operand.getContactAffinity(); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ValidateNotificationPeopleTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ValidateNotificationPeopleTest.java index d72cfc70fc02..0564a73bf3fc 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ValidateNotificationPeopleTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ValidateNotificationPeopleTest.java @@ -15,6 +15,8 @@ */ package com.android.server.notification; +import static com.google.common.truth.Truth.assertThat; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -51,6 +53,8 @@ import android.util.LruCache; import androidx.test.runner.AndroidJUnit4; import com.android.server.UiServiceTestCase; +import com.android.server.notification.ValidateNotificationPeople.LookupResult; +import com.android.server.notification.ValidateNotificationPeople.PeopleRankingReconsideration; import org.junit.Test; import org.junit.runner.RunWith; @@ -60,6 +64,7 @@ import org.mockito.stubbing.Answer; import java.util.ArrayList; import java.util.Arrays; +import java.util.concurrent.TimeUnit; @SmallTest @RunWith(AndroidJUnit4.class) @@ -215,7 +220,7 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { ContactsContract.Contacts.CONTENT_LOOKUP_URI, ContactsContract.Contacts.ENTERPRISE_CONTACT_LOOKUP_PREFIX + contactId); - new ValidateNotificationPeople().searchContacts(mockContext, lookupUri); + PeopleRankingReconsideration.searchContacts(mockContext, lookupUri); ArgumentCaptor<Uri> queryUri = ArgumentCaptor.forClass(Uri.class); verify(mockContentResolver).query( @@ -242,7 +247,7 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { final Uri lookupUri = Uri.withAppendedPath( ContactsContract.Contacts.CONTENT_LOOKUP_URI, String.valueOf(contactId)); - new ValidateNotificationPeople().searchContacts(mockContext, lookupUri); + PeopleRankingReconsideration.searchContacts(mockContext, lookupUri); ArgumentCaptor<Uri> queryUri = ArgumentCaptor.forClass(Uri.class); verify(mockContentResolver).query( @@ -277,7 +282,7 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { // call searchContacts and then mergePhoneNumbers, make sure we never actually // query the content resolver for a phone number - new ValidateNotificationPeople().searchContactsAndLookupNumbers(mockContext, lookupUri); + PeopleRankingReconsideration.searchContactsAndLookupNumbers(mockContext, lookupUri); verify(mockContentResolver, never()).query( eq(ContactsContract.CommonDataKinds.Phone.CONTENT_URI), eq(ValidateNotificationPeople.PHONE_LOOKUP_PROJECTION), @@ -320,7 +325,7 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { // call searchContacts and then mergePhoneNumbers, and check that we query // once for the - new ValidateNotificationPeople().searchContactsAndLookupNumbers(mockContext, lookupUri); + PeopleRankingReconsideration.searchContactsAndLookupNumbers(mockContext, lookupUri); verify(mockContentResolver, times(1)).query( eq(ContactsContract.CommonDataKinds.Phone.CONTENT_URI), eq(ValidateNotificationPeople.PHONE_LOOKUP_PROJECTION), @@ -339,7 +344,7 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { // Create validator with empty cache ValidateNotificationPeople vnp = new ValidateNotificationPeople(); - LruCache cache = new LruCache<String, ValidateNotificationPeople.LookupResult>(5); + LruCache<String, LookupResult> cache = new LruCache<>(5); vnp.initForTests(mockContext, mockNotificationUsageStats, cache); NotificationRecord record = getNotificationRecord(); @@ -366,9 +371,8 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { float affinity = 0.7f; // Create a fake LookupResult for the data we'll pass in - LruCache cache = new LruCache<String, ValidateNotificationPeople.LookupResult>(5); - ValidateNotificationPeople.LookupResult lr = - mock(ValidateNotificationPeople.LookupResult.class); + LruCache<String, LookupResult> cache = new LruCache<>(5); + LookupResult lr = mock(LookupResult.class); when(lr.getAffinity()).thenReturn(affinity); when(lr.getPhoneNumbers()).thenReturn(new ArraySet<>(new String[]{lookupTel})); when(lr.isExpired()).thenReturn(false); @@ -392,6 +396,23 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { assertTrue(record.getPhoneNumbers().contains(lookupTel)); } + @Test + public void validatePeople_reconsiderationWillNotBeDelayed() { + final Context mockContext = mock(Context.class); + final ContentResolver mockContentResolver = mock(ContentResolver.class); + when(mockContext.getContentResolver()).thenReturn(mockContentResolver); + ValidateNotificationPeople vnp = new ValidateNotificationPeople(); + vnp.initForTests(mockContext, mock(NotificationUsageStats.class), new LruCache<>(5)); + NotificationRecord record = getNotificationRecord(); + String[] callNumber = new String[]{"tel:12345678910"}; + setNotificationPeople(record, callNumber); + + RankingReconsideration rr = vnp.validatePeople(mockContext, record); + + assertThat(rr).isNotNull(); + assertThat(rr.getDelay(TimeUnit.MILLISECONDS)).isEqualTo(0); + } + // Creates a cursor that points to one item of Contacts data with the specified // columns. private Cursor makeMockCursor(int id, String lookupKey, int starred, int hasPhone) { |