From b760ffdc1a843429c567043ace763f934de54000 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Mon, 26 Sep 2022 20:09:36 +0800 Subject: [PATCH] Refactor AppNotificationRepository Also add unit tests. Bug: 236346018 Test: Manual with Settings App Change-Id: Idf3fbcb3dff2cbee9039ff67b5c0f09133f9ff86 --- .../notification/AppNotificationRepository.kt | 70 +++--- .../notification/AppNotificationsListModel.kt | 15 +- .../AppNotificationRepositoryTest.kt | 236 ++++++++++++++++++ 3 files changed, 279 insertions(+), 42 deletions(-) create mode 100644 tests/spa_unit/src/com/android/settings/spa/notification/AppNotificationRepositoryTest.kt diff --git a/src/com/android/settings/spa/notification/AppNotificationRepository.kt b/src/com/android/settings/spa/notification/AppNotificationRepository.kt index 0f88ac93a91..7ec5d3edc71 100644 --- a/src/com/android/settings/spa/notification/AppNotificationRepository.kt +++ b/src/com/android/settings/spa/notification/AppNotificationRepository.kt @@ -30,7 +30,9 @@ import android.os.Build import android.os.RemoteException import android.os.ServiceManager import android.util.Log -import com.android.settingslib.spaprivileged.model.app.PackageManagers.hasRequestPermission +import com.android.settings.R +import com.android.settingslib.spaprivileged.model.app.IPackageManagers +import com.android.settingslib.spaprivileged.model.app.PackageManagers import java.util.concurrent.TimeUnit import kotlin.math.max import kotlin.math.roundToInt @@ -48,21 +50,23 @@ data class NotificationSentState( var sentCount: Int = 0, ) -class AppNotificationRepository(private val context: Context) { +class AppNotificationRepository( + private val context: Context, + private val packageManagers: IPackageManagers = PackageManagers, + private val usageStatsManager: IUsageStatsManager = IUsageStatsManager.Stub.asInterface( + ServiceManager.getService(Context.USAGE_STATS_SERVICE) + ), + private val notificationManager: INotificationManager = INotificationManager.Stub.asInterface( + ServiceManager.getService(Context.NOTIFICATION_SERVICE) + ), +) { fun getAggregatedUsageEvents(userIdFlow: Flow): Flow> = userIdFlow.map { userId -> val aggregatedStats = mutableMapOf() - queryEventsForUser(userId)?.let { events -> - val event = UsageEvents.Event() - while (events.hasNextEvent()) { - events.getNextEvent(event) - if (event.eventType == UsageEvents.Event.NOTIFICATION_INTERRUPTION) { - aggregatedStats.getOrPut(event.packageName, ::NotificationSentState) - .apply { - lastSent = max(lastSent, event.timeStamp) - sentCount++ - } - } + queryEventsForUser(userId).forEachNotificationEvent { event -> + aggregatedStats.getOrPut(event.packageName, ::NotificationSentState).apply { + lastSent = max(lastSent, event.timeStamp) + sentCount++ } } aggregatedStats @@ -90,7 +94,9 @@ class AppNotificationRepository(private val context: Context) { // If the app targets T but has not requested the permission, we cannot change the // permission state. return app.targetSdkVersion < Build.VERSION_CODES.TIRAMISU || - app.hasRequestPermission(Manifest.permission.POST_NOTIFICATIONS) + with(packageManagers) { + app.hasRequestPermission(Manifest.permission.POST_NOTIFICATIONS) + } } fun setEnabled(app: ApplicationInfo, enabled: Boolean): Boolean { @@ -109,6 +115,19 @@ class AppNotificationRepository(private val context: Context) { } } + fun calculateFrequencySummary(sentCount: Int): String { + val dailyFrequency = (sentCount.toFloat() / DAYS_TO_CHECK).roundToInt() + return if (dailyFrequency > 0) { + context.resources.getQuantityString( + R.plurals.notifications_sent_daily, dailyFrequency, dailyFrequency + ) + } else { + context.resources.getQuantityString( + R.plurals.notifications_sent_weekly, sentCount, sentCount + ) + } + } + private fun updateChannel(app: ApplicationInfo, channel: NotificationChannel) { notificationManager.updateNotificationChannelForPackage(app.packageName, app.uid, channel) } @@ -124,21 +143,16 @@ class AppNotificationRepository(private val context: Context) { companion object { private const val TAG = "AppNotificationsRepo" - const val DAYS_TO_CHECK = 7L + private const val DAYS_TO_CHECK = 7L - private val usageStatsManager by lazy { - IUsageStatsManager.Stub.asInterface( - ServiceManager.getService(Context.USAGE_STATS_SERVICE) - ) + private fun UsageEvents?.forEachNotificationEvent(action: (UsageEvents.Event) -> Unit) { + this ?: return + val event = UsageEvents.Event() + while (getNextEvent(event)) { + if (event.eventType == UsageEvents.Event.NOTIFICATION_INTERRUPTION) { + action(event) + } + } } - - private val notificationManager by lazy { - INotificationManager.Stub.asInterface( - ServiceManager.getService(Context.NOTIFICATION_SERVICE) - ) - } - - fun calculateDailyFrequent(sentCount: Int): Int = - (sentCount.toFloat() / DAYS_TO_CHECK).roundToInt() } } diff --git a/src/com/android/settings/spa/notification/AppNotificationsListModel.kt b/src/com/android/settings/spa/notification/AppNotificationsListModel.kt index c7baa033604..29c8a2bc849 100644 --- a/src/com/android/settings/spa/notification/AppNotificationsListModel.kt +++ b/src/com/android/settings/spa/notification/AppNotificationsListModel.kt @@ -92,7 +92,7 @@ class AppNotificationsListModel( override fun getSummary(option: Int, record: AppNotificationsRecord) = record.sentState?.let { when (option.toSpinnerItem()) { SpinnerItem.MostRecent -> stateOf(formatLastSent(it.lastSent)) - SpinnerItem.MostFrequent -> stateOf(calculateFrequent(it.sentCount)) + SpinnerItem.MostFrequent -> stateOf(repository.calculateFrequencySummary(it.sentCount)) else -> null } } @@ -109,19 +109,6 @@ class AppNotificationsListModel( RelativeDateTimeFormatter.Style.LONG, ).toString() - private fun calculateFrequent(sentCount: Int): String { - val dailyFrequent = AppNotificationRepository.calculateDailyFrequent(sentCount) - return if (dailyFrequent > 0) { - context.resources.getQuantityString( - R.plurals.notifications_sent_daily, dailyFrequent, dailyFrequent - ) - } else { - context.resources.getQuantityString( - R.plurals.notifications_sent_weekly, sentCount, sentCount - ) - } - } - @Composable override fun AppListItemModel.AppItem() { AppListSwitchItem( diff --git a/tests/spa_unit/src/com/android/settings/spa/notification/AppNotificationRepositoryTest.kt b/tests/spa_unit/src/com/android/settings/spa/notification/AppNotificationRepositoryTest.kt new file mode 100644 index 00000000000..7a5bc9f719a --- /dev/null +++ b/tests/spa_unit/src/com/android/settings/spa/notification/AppNotificationRepositoryTest.kt @@ -0,0 +1,236 @@ +/* + * Copyright (C) 2022 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.settings.spa.notification + +import android.Manifest +import android.app.INotificationManager +import android.app.NotificationChannel +import android.app.NotificationManager.IMPORTANCE_DEFAULT +import android.app.NotificationManager.IMPORTANCE_NONE +import android.app.NotificationManager.IMPORTANCE_UNSPECIFIED +import android.app.usage.IUsageStatsManager +import android.app.usage.UsageEvents +import android.content.Context +import android.content.pm.ApplicationInfo +import android.os.Build +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.settingslib.spa.testutils.any +import com.android.settingslib.spaprivileged.model.app.IPackageManagers +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.eq +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit +import org.mockito.junit.MockitoRule +import org.mockito.Mockito.`when` as whenever + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(AndroidJUnit4::class) +class AppNotificationRepositoryTest { + @get:Rule + val mockito: MockitoRule = MockitoJUnit.rule() + + private val context: Context = ApplicationProvider.getApplicationContext() + + @Mock + private lateinit var packageManagers: IPackageManagers + + @Mock + private lateinit var usageStatsManager: IUsageStatsManager + + @Mock + private lateinit var notificationManager: INotificationManager + + private lateinit var repository: AppNotificationRepository + + @Before + fun setUp() { + repository = AppNotificationRepository( + context, + packageManagers, + usageStatsManager, + notificationManager, + ) + } + + private fun mockOnlyHasDefaultChannel(): NotificationChannel { + whenever(notificationManager.onlyHasDefaultChannel(APP.packageName, APP.uid)) + .thenReturn(true) + val channel = + NotificationChannel(NotificationChannel.DEFAULT_CHANNEL_ID, null, IMPORTANCE_DEFAULT) + whenever( + notificationManager.getNotificationChannelForPackage( + APP.packageName, APP.uid, NotificationChannel.DEFAULT_CHANNEL_ID, null, true + ) + ).thenReturn(channel) + return channel + } + + @Test + fun getAggregatedUsageEvents() = runTest { + val events = listOf( + UsageEvents.Event().apply { + mEventType = UsageEvents.Event.NOTIFICATION_INTERRUPTION + mPackage = PACKAGE_NAME + mTimeStamp = 2 + }, + UsageEvents.Event().apply { + mEventType = UsageEvents.Event.NOTIFICATION_INTERRUPTION + mPackage = PACKAGE_NAME + mTimeStamp = 3 + }, + UsageEvents.Event().apply { + mEventType = UsageEvents.Event.NOTIFICATION_INTERRUPTION + mPackage = PACKAGE_NAME + mTimeStamp = 6 + }, + ) + whenever(usageStatsManager.queryEventsForUser(any(), any(), eq(USER_ID), any())) + .thenReturn(UsageEvents(events, arrayOf())) + + val usageEvents = repository.getAggregatedUsageEvents(flowOf(USER_ID)).first() + + assertThat(usageEvents).containsExactly( + PACKAGE_NAME, NotificationSentState(lastSent = 6, sentCount = 3), + ) + } + + @Test + fun isEnabled() { + whenever(notificationManager.areNotificationsEnabledForPackage(APP.packageName, APP.uid)) + .thenReturn(true) + + val isEnabled = repository.isEnabled(APP) + + assertThat(isEnabled).isTrue() + } + + @Test + fun isChangeable_importanceLocked() { + whenever(notificationManager.isImportanceLocked(APP.packageName, APP.uid)).thenReturn(true) + + val isChangeable = repository.isChangeable(APP) + + assertThat(isChangeable).isFalse() + } + + @Test + fun isChangeable_appTargetS() { + val targetSApp = ApplicationInfo().apply { + targetSdkVersion = Build.VERSION_CODES.S + } + + val isChangeable = repository.isChangeable(targetSApp) + + assertThat(isChangeable).isTrue() + } + + @Test + fun isChangeable_appTargetTiramisuWithoutNotificationPermission() { + val targetTiramisuApp = ApplicationInfo().apply { + targetSdkVersion = Build.VERSION_CODES.TIRAMISU + } + with(packageManagers) { + whenever(targetTiramisuApp.hasRequestPermission(Manifest.permission.POST_NOTIFICATIONS)) + .thenReturn(false) + } + + val isChangeable = repository.isChangeable(targetTiramisuApp) + + assertThat(isChangeable).isFalse() + } + + @Test + fun isChangeable_appTargetTiramisuWithNotificationPermission() { + val targetTiramisuApp = ApplicationInfo().apply { + targetSdkVersion = Build.VERSION_CODES.TIRAMISU + } + with(packageManagers) { + whenever(targetTiramisuApp.hasRequestPermission(Manifest.permission.POST_NOTIFICATIONS)) + .thenReturn(true) + } + + val isChangeable = repository.isChangeable(targetTiramisuApp) + + assertThat(isChangeable).isTrue() + } + + @Test + fun setEnabled_toTrueWhenOnlyHasDefaultChannel() { + val channel = mockOnlyHasDefaultChannel() + + repository.setEnabled(app = APP, enabled = true) + + verify(notificationManager) + .updateNotificationChannelForPackage(APP.packageName, APP.uid, channel) + assertThat(channel.importance).isEqualTo(IMPORTANCE_UNSPECIFIED) + } + + @Test + fun setEnabled_toFalseWhenOnlyHasDefaultChannel() { + val channel = mockOnlyHasDefaultChannel() + + repository.setEnabled(app = APP, enabled = false) + + verify(notificationManager) + .updateNotificationChannelForPackage(APP.packageName, APP.uid, channel) + assertThat(channel.importance).isEqualTo(IMPORTANCE_NONE) + } + + @Test + fun setEnabled_toTrueWhenNotOnlyHasDefaultChannel() { + whenever(notificationManager.onlyHasDefaultChannel(APP.packageName, APP.uid)) + .thenReturn(false) + + repository.setEnabled(app = APP, enabled = true) + + verify(notificationManager) + .setNotificationsEnabledForPackage(APP.packageName, APP.uid, true) + } + + @Test + fun calculateFrequencySummary_daily() { + val summary = repository.calculateFrequencySummary(4) + + assertThat(summary).isEqualTo("About 1 notification per day") + } + + @Test + fun calculateFrequencySummary_weekly() { + val summary = repository.calculateFrequencySummary(3) + + assertThat(summary).isEqualTo("About 3 notifications per week") + } + + private companion object { + const val USER_ID = 0 + const val PACKAGE_NAME = "package.name" + val APP = ApplicationInfo().apply { + packageName = PACKAGE_NAME + uid = 123 + } + } +} \ No newline at end of file