From e55003bbef45fe634c3a7a41fe395155eb2ea9c7 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Sat, 19 Oct 2019 13:40:55 -0400 Subject: [PATCH 1/2] Unit tests for PeopleHubViewController.kt Specifically, this tests the glue code between the data source (currently implemented as an in-flux prototype) and the UI (abstracted behind a boundary interface, which simply forward to View methods). Test: atest Change-Id: Iee308af4680bfc43427c66c88926f64ee9f812c6 --- .../notification/people/PeopleHubModule.kt | 2 +- .../people/PeopleHubNotificationListener.kt | 3 +- .../people/PeopleHubViewControllerTest.kt | 165 ++++++++++++++++++ 3 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/people/PeopleHubViewControllerTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubModule.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubModule.kt index 45709893a7b5b..4f03003402db2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubModule.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubModule.kt @@ -23,7 +23,7 @@ import dagger.Module abstract class PeopleHubModule { @Binds - abstract fun peopleHubSectionFooterViewController( + abstract fun peopleHubSectionFooterViewAdapter( impl: PeopleHubSectionFooterViewAdapterImpl ): PeopleHubSectionFooterViewAdapter diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt index fe257d9fd2521..9091d94e45f05 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt @@ -180,8 +180,7 @@ private fun NotificationEntry.extractPerson(): PersonModel? { if (!isMessagingNotification()) { return null } - val clickIntent = sbn.notification.contentIntent - ?: return null + val clickIntent = sbn.notification.contentIntent ?: return null val extras = sbn.notification.extras val name = extras.getString(Notification.EXTRA_CONVERSATION_TITLE) ?: extras.getString(Notification.EXTRA_TITLE) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/people/PeopleHubViewControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/people/PeopleHubViewControllerTest.kt new file mode 100644 index 0000000000000..a1822c72fe18f --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/people/PeopleHubViewControllerTest.kt @@ -0,0 +1,165 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification.people + +import android.app.PendingIntent +import android.content.Intent +import android.graphics.drawable.Drawable +import android.testing.AndroidTestingRunner +import android.view.View +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.plugins.ActivityStarter +import com.google.common.truth.Truth.assertThat +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito +import org.mockito.Mockito.`when` +import org.mockito.Mockito.mock +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit +import org.mockito.junit.MockitoRule +import kotlin.reflect.KClass + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class PeopleHubViewControllerTest : SysuiTestCase() { + + @JvmField @Rule val mockito: MockitoRule = MockitoJUnit.rule() + + @Mock private lateinit var mockViewBoundary: PeopleHubSectionFooterViewBoundary + @Mock private lateinit var mockActivityStarter: ActivityStarter + + @Test + fun testBindViewModelToViewBoundary() { + val fakePerson1 = fakePersonViewModel("name") + val fakeViewModel = PeopleHubViewModel(sequenceOf(fakePerson1), true) + + val fakePersonViewAdapter1 = FakeDataListener() + val fakePersonViewAdapter2 = FakeDataListener() + + val mockClickView = mock(View::class.java) + + `when`(mockViewBoundary.associatedViewForClickAnimation).thenReturn(mockClickView) + `when`(mockViewBoundary.personViewAdapters) + .thenReturn(sequenceOf(fakePersonViewAdapter1, fakePersonViewAdapter2)) + + val mockFactory = mock(PeopleHubViewModelFactory::class.java) + `when`(mockFactory.createWithAssociatedClickView(any())).thenReturn(fakeViewModel) + + val mockSubscription = mock(Subscription::class.java) + val fakeFactoryDataSource = object : DataSource { + override fun registerListener( + listener: DataListener + ): Subscription { + listener.onDataChanged(mockFactory) + return mockSubscription + } + } + val adapter = PeopleHubSectionFooterViewAdapterImpl(fakeFactoryDataSource) + + adapter.bindView(mockViewBoundary) + + assertThat(fakePersonViewAdapter1.lastSeen).isEqualTo(Maybe.Just(fakePerson1)) + assertThat(fakePersonViewAdapter2.lastSeen).isEqualTo(Maybe.Just(null)) + verify(mockViewBoundary).setVisible(true) + verify(mockFactory).createWithAssociatedClickView(mockClickView) + } + + fun testViewModelDataSourceTransformsModel() { + val fakeClickIntent = PendingIntent.getActivity(context, 0, Intent("action"), 0) + val fakePerson = fakePersonModel("id", "name", fakeClickIntent) + val fakeModel = PeopleHubModel(listOf(fakePerson)) + val mockSubscription = mock(Subscription::class.java) + val fakeModelDataSource = object : DataSource { + override fun registerListener(listener: DataListener): Subscription { + listener.onDataChanged(fakeModel) + return mockSubscription + } + } + val factoryDataSource = PeopleHubViewModelFactoryDataSourceImpl( + mockActivityStarter, fakeModelDataSource) + val fakeListener = FakeDataListener() + val mockClickView = mock(View::class.java) + + factoryDataSource.registerListener(fakeListener) + val viewModel = (fakeListener.lastSeen as Maybe.Just).value + .createWithAssociatedClickView(mockClickView) + assertThat(viewModel.isVisible).isTrue() + + val people = viewModel.people.toList() + assertThat(people.size).isEqualTo(1) + assertThat(people[0].name).isEqualTo("name") + assertThat(people[0].icon).isSameAs(fakePerson.avatar) + + people[0].onClick() + verify(mockActivityStarter).startPendingIntentDismissingKeyguard( + same(fakeClickIntent), + any(), + same(mockClickView) + ) + } +} + +private inline fun any(): T { + return Mockito.any() ?: createInstance(T::class) +} + +private inline fun same(value: T): T { + return Mockito.same(value) ?: createInstance(T::class) +} + +private fun createInstance(clazz: KClass): T = castNull() + +@Suppress("UNCHECKED_CAST") +private fun castNull(): T = null as T + +private fun fakePersonModel( + id: String, + name: CharSequence, + clickIntent: PendingIntent +): PersonModel = + PersonModel(id, name, mock(Drawable::class.java), clickIntent) + +private fun fakePersonViewModel(name: CharSequence): PersonViewModel = + PersonViewModel(name, mock(Drawable::class.java), mock({}.javaClass)) + +sealed class Maybe { + data class Just(val value: T) : Maybe() + class Nothing : Maybe() { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + return true + } + + override fun hashCode(): Int { + return javaClass.hashCode() + } + } +} + +class FakeDataListener : DataListener { + + var lastSeen: Maybe = Maybe.Nothing() + + override fun onDataChanged(data: T) { + lastSeen = Maybe.Just(data) + } +} \ No newline at end of file From bfc57f36d8e4d7184d2a63e4d95280934e838332 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Thu, 24 Oct 2019 14:35:06 -0400 Subject: [PATCH 2/2] Finer-grain control of people sorting via plugin This change allows for NotificationPersonExtractorPlugin implementors to mark notifications as "people" (to be sorted into the People bucket) separately from extracting data from them for PeopleHub. Test: manual Change-Id: Ieb8a9aeea06bbfe70976ebf46274567347e695d7 --- .../plugins/NotificationPersonExtractorPlugin.java | 13 +++++++++++-- .../people/PeopleHubNotificationListener.kt | 4 ++++ .../people/PeopleNotificationIdentifier.kt | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/plugin/src/com/android/systemui/plugins/NotificationPersonExtractorPlugin.java b/packages/SystemUI/plugin/src/com/android/systemui/plugins/NotificationPersonExtractorPlugin.java index 802a8dab92d94..6650c15d6742f 100644 --- a/packages/SystemUI/plugin/src/com/android/systemui/plugins/NotificationPersonExtractorPlugin.java +++ b/packages/SystemUI/plugin/src/com/android/systemui/plugins/NotificationPersonExtractorPlugin.java @@ -32,12 +32,13 @@ import com.android.systemui.plugins.annotations.ProvidesInterface; public interface NotificationPersonExtractorPlugin extends Plugin { String ACTION = "com.android.systemui.action.PEOPLE_HUB_PERSON_EXTRACTOR"; - int VERSION = 0; + int VERSION = 1; /** * Attempts to extract a person from a notification. Returns {@code null} if one is not found. */ - @Nullable PersonData extractPerson(StatusBarNotification sbn); + @Nullable + PersonData extractPerson(StatusBarNotification sbn); /** * Attempts to extract a person id from a notification. Returns {@code null} if one is not @@ -50,6 +51,14 @@ public interface NotificationPersonExtractorPlugin extends Plugin { return extractPerson(sbn).key; } + /** + * Determines whether or not a notification should be treated as having a person. Used for + * appropriate positioning in the notification shade. + */ + default boolean isPersonNotification(StatusBarNotification sbn) { + return extractPersonKey(sbn) != null; + } + /** A person to be surfaced in PeopleHub. */ @ProvidesInterface(version = PersonData.VERSION) final class PersonData { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt index 9091d94e45f05..987b52dbc2e8d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleHubNotificationListener.kt @@ -47,6 +47,7 @@ private const val MAX_STORED_INACTIVE_PEOPLE = 10 interface NotificationPersonExtractor { fun extractPerson(sbn: StatusBarNotification): PersonModel? fun extractPersonKey(sbn: StatusBarNotification): String? + fun isPersonNotification(sbn: StatusBarNotification): Boolean } @Singleton @@ -75,6 +76,9 @@ class NotificationPersonExtractorPluginBoundary @Inject constructor( } override fun extractPersonKey(sbn: StatusBarNotification) = plugin?.extractPersonKey(sbn) + + override fun isPersonNotification(sbn: StatusBarNotification): Boolean = + plugin?.isPersonNotification(sbn) ?: false } @Singleton diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleNotificationIdentifier.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleNotificationIdentifier.kt index bfd4070846f32..78eaf3ee10a4a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleNotificationIdentifier.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/people/PeopleNotificationIdentifier.kt @@ -32,5 +32,5 @@ class PeopleNotificationIdentifierImpl @Inject constructor( override fun isPeopleNotification(sbn: StatusBarNotification) = sbn.notification.notificationStyle == Notification.MessagingStyle::class.java || - personExtractor.extractPersonKey(sbn) != null + personExtractor.isPersonNotification(sbn) } \ No newline at end of file