diff --git a/services/core/java/com/android/server/notification/ValidateNotificationPeople.java b/services/core/java/com/android/server/notification/ValidateNotificationPeople.java index bdc571103ffd3..5e0a180391524 100644 --- a/services/core/java/com/android/server/notification/ValidateNotificationPeople.java +++ b/services/core/java/com/android/server/notification/ValidateNotificationPeople.java @@ -131,6 +131,17 @@ 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) { + mUserToContextMap = new ArrayMap<>(); + mBaseContext = context; + mUsageStats = usageStats; + mPeopleCache = peopleCache; + mEnabled = true; + } + public RankingReconsideration process(NotificationRecord record) { if (!mEnabled) { if (VERBOSE) Slog.i(TAG, "disabled"); @@ -179,7 +190,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { return NONE; } final PeopleRankingReconsideration prr = - validatePeople(context, key, extras, null, affinityOut); + validatePeople(context, key, extras, null, affinityOut, null); float affinity = affinityOut[0]; if (prr != null) { @@ -224,15 +235,21 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { return context; } - private RankingReconsideration validatePeople(Context context, + @VisibleForTesting + protected RankingReconsideration validatePeople(Context context, final NotificationRecord record) { final String key = record.getKey(); final Bundle extras = record.getNotification().extras; final float[] affinityOut = new float[1]; + ArraySet phoneNumbersOut = new ArraySet<>(); final PeopleRankingReconsideration rr = - validatePeople(context, key, extras, record.getPeopleOverride(), affinityOut); + validatePeople(context, key, extras, record.getPeopleOverride(), affinityOut, + phoneNumbersOut); final float affinity = affinityOut[0]; record.setContactAffinity(affinity); + if (phoneNumbersOut.size() > 0) { + record.mergePhoneNumbers(phoneNumbersOut); + } if (rr == null) { mUsageStats.registerPeopleAffinity(record, affinity > NONE, affinity == STARRED_CONTACT, true /* cached */); @@ -243,7 +260,7 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } private PeopleRankingReconsideration validatePeople(Context context, String key, Bundle extras, - List peopleOverride, float[] affinityOut) { + List peopleOverride, float[] affinityOut, ArraySet phoneNumbersOut) { float affinity = NONE; if (extras == null) { return null; @@ -270,6 +287,15 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } if (lookupResult != null) { affinity = Math.max(affinity, lookupResult.getAffinity()); + + // add all phone numbers associated with this lookup result, if they exist + // and if requested + if (phoneNumbersOut != null) { + ArraySet phoneNumbers = lookupResult.getPhoneNumbers(); + if (phoneNumbers != null && phoneNumbers.size() > 0) { + phoneNumbersOut.addAll(phoneNumbers); + } + } } } if (++personIdx == MAX_PEOPLE) { @@ -289,7 +315,8 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { return new PeopleRankingReconsideration(context, key, pendingLookups); } - private String getCacheKey(int userId, String handle) { + @VisibleForTesting + protected static String getCacheKey(int userId, String handle) { return Integer.toString(userId) + ":" + handle; } @@ -485,7 +512,8 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { } } - private static class LookupResult { + @VisibleForTesting + protected static class LookupResult { private static final long CONTACT_REFRESH_MILLIS = 60 * 60 * 1000; // 1hr private final long mExpireMillis; @@ -574,7 +602,8 @@ public class ValidateNotificationPeople implements NotificationSignalExtractor { return mPhoneNumbers; } - private boolean isExpired() { + @VisibleForTesting + protected boolean isExpired() { return mExpireMillis < System.currentTimeMillis(); } 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 c12f0a9651465..d72cfc70fc023 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ValidateNotificationPeopleTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ValidateNotificationPeopleTest.java @@ -17,7 +17,9 @@ package com.android.server.notification; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; @@ -30,6 +32,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.Notification; +import android.app.NotificationChannel; import android.app.Person; import android.content.ContentProvider; import android.content.ContentResolver; @@ -39,8 +42,11 @@ import android.net.Uri; import android.os.Bundle; import android.os.UserManager; import android.provider.ContactsContract; +import android.service.notification.StatusBarNotification; import android.test.suitebuilder.annotation.SmallTest; import android.text.SpannableString; +import android.util.ArraySet; +import android.util.LruCache; import androidx.test.runner.AndroidJUnit4; @@ -323,6 +329,69 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { isNull()); // sort order } + @Test + public void testValidatePeople_needsLookupWhenNoCache() { + final Context mockContext = mock(Context.class); + final ContentResolver mockContentResolver = mock(ContentResolver.class); + when(mockContext.getContentResolver()).thenReturn(mockContentResolver); + final NotificationUsageStats mockNotificationUsageStats = + mock(NotificationUsageStats.class); + + // Create validator with empty cache + ValidateNotificationPeople vnp = new ValidateNotificationPeople(); + LruCache cache = new LruCache(5); + vnp.initForTests(mockContext, mockNotificationUsageStats, cache); + + NotificationRecord record = getNotificationRecord(); + String[] callNumber = new String[]{"tel:12345678910"}; + setNotificationPeople(record, callNumber); + + // Returned ranking reconsideration not null indicates that there is a lookup to be done + RankingReconsideration rr = vnp.validatePeople(mockContext, record); + assertNotNull(rr); + } + + @Test + public void testValidatePeople_noLookupWhenCached_andPopulatesContactInfo() { + final Context mockContext = mock(Context.class); + final ContentResolver mockContentResolver = mock(ContentResolver.class); + when(mockContext.getContentResolver()).thenReturn(mockContentResolver); + when(mockContext.getUserId()).thenReturn(1); + final NotificationUsageStats mockNotificationUsageStats = + mock(NotificationUsageStats.class); + + // Information to be passed in & returned from the lookup result + String lookup = "lookup:contactinfohere"; + String lookupTel = "16175551234"; + float affinity = 0.7f; + + // Create a fake LookupResult for the data we'll pass in + LruCache cache = new LruCache(5); + ValidateNotificationPeople.LookupResult lr = + mock(ValidateNotificationPeople.LookupResult.class); + when(lr.getAffinity()).thenReturn(affinity); + when(lr.getPhoneNumbers()).thenReturn(new ArraySet<>(new String[]{lookupTel})); + when(lr.isExpired()).thenReturn(false); + cache.put(ValidateNotificationPeople.getCacheKey(1, lookup), lr); + + // Create validator with the established cache + ValidateNotificationPeople vnp = new ValidateNotificationPeople(); + vnp.initForTests(mockContext, mockNotificationUsageStats, cache); + + NotificationRecord record = getNotificationRecord(); + String[] peopleInfo = new String[]{lookup}; + setNotificationPeople(record, peopleInfo); + + // Returned ranking reconsideration null indicates that there is no pending work to be done + RankingReconsideration rr = vnp.validatePeople(mockContext, record); + assertNull(rr); + + // Confirm that the affinity & phone number made it into our record + assertEquals(affinity, record.getContactAffinity(), 1e-8); + assertNotNull(record.getPhoneNumbers()); + assertTrue(record.getPhoneNumbers().contains(lookupTel)); + } + // 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) { @@ -365,4 +434,17 @@ public class ValidateNotificationPeopleTest extends UiServiceTestCase { String resultString = Arrays.toString(result); assertEquals(message + ": arrays differ", expectedString, resultString); } + + private NotificationRecord getNotificationRecord() { + StatusBarNotification sbn = mock(StatusBarNotification.class); + Notification notification = mock(Notification.class); + when(sbn.getNotification()).thenReturn(notification); + return new NotificationRecord(mContext, sbn, mock(NotificationChannel.class)); + } + + private void setNotificationPeople(NotificationRecord r, String[] people) { + Bundle extras = new Bundle(); + extras.putObject(Notification.EXTRA_PEOPLE_LIST, people); + r.getSbn().getNotification().extras = extras; + } }