From b52da000b16844b6817486f58121aefe63f767d2 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Mon, 27 Apr 2020 15:55:14 +0200 Subject: [PATCH 1/4] we want to make sure that old work tags are cancelled --- .../AndroidNotificationSchedulerTest.kt | 20 +++++++++++++++---- .../AndroidNotificationScheduler.kt | 14 +++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt index fb94257e2a13..791d84829a26 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -98,17 +98,29 @@ class AndroidNotificationSchedulerTest { assertNoUnusedAppNotificationScheduled() } + @Test + fun allDeprecatedWorkIsCancelledUponStart() = runBlocking { + whenever(privacyNotification.canShow()).thenReturn(false) + whenever(clearNotification.canShow()).thenReturn(false) + + testee.scheduleNextNotification() + + NotificationScheduler.allDeprecatedWorkTags().forEach { + assertTrue(getScheduledWorkers(it).isEmpty()) + } + } + private fun assertUnusedAppNotificationScheduled(workerName: String) { - assertTrue(getUnusedAppScheduledWorkers().any { it.tags.contains(workerName) }) + assertTrue(getScheduledWorkers(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG).any { it.tags.contains(workerName) }) } private fun assertNoUnusedAppNotificationScheduled() { - assertTrue(getUnusedAppScheduledWorkers().isEmpty()) + assertTrue(getScheduledWorkers(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG).isEmpty()) } - private fun getUnusedAppScheduledWorkers(): List { + private fun getScheduledWorkers(tag: String): List { return workManager - .getWorkInfosByTag(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG) + .getWorkInfosByTag(tag) .get() .filter { it.state == WorkInfo.State.ENQUEUED } } diff --git a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt index d4e4053d526c..6e8b82459767 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -46,9 +46,17 @@ class NotificationScheduler( ) : AndroidNotificationScheduler { override suspend fun scheduleNextNotification() { + cancelAllUnnecessaryWork() scheduleInactiveUserNotifications() } + private fun cancelAllUnnecessaryWork(){ + allDeprecatedWorkTags().forEach { tag -> + workManager.cancelAllWorkByTag(tag) + } + + } + private suspend fun scheduleInactiveUserNotifications() { workManager.cancelAllWorkByTag(UNUSED_APP_WORK_REQUEST_TAG) @@ -109,5 +117,11 @@ class NotificationScheduler( companion object { const val UNUSED_APP_WORK_REQUEST_TAG = "com.duckduckgo.notification.schedule" + + // below there is a list of TAGs that were used at some point but that are no longer active + // we want to make sure that this TAGs are cancelled to avoid inconsistencies + const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment + + fun allDeprecatedWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) } } \ No newline at end of file From adbedb08d49f12b9c8885f643407a3e9095d890f Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Mon, 27 Apr 2020 15:59:11 +0200 Subject: [PATCH 2/4] this could be private --- .../duckduckgo/app/notification/AndroidNotificationScheduler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt index 6e8b82459767..efd1cb607729 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -120,7 +120,7 @@ class NotificationScheduler( // below there is a list of TAGs that were used at some point but that are no longer active // we want to make sure that this TAGs are cancelled to avoid inconsistencies - const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment + private const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment fun allDeprecatedWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) } From 3f26bcad1ee79ed5f77bd13d3c44f20a5c39081a Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Mon, 27 Apr 2020 16:23:18 +0200 Subject: [PATCH 3/4] better naming for this method --- .../app/notification/AndroidNotificationSchedulerTest.kt | 3 +-- .../app/notification/AndroidNotificationScheduler.kt | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt index 791d84829a26..5cf3de8740f6 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -31,7 +31,6 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking -import org.junit.After import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule @@ -105,7 +104,7 @@ class AndroidNotificationSchedulerTest { testee.scheduleNextNotification() - NotificationScheduler.allDeprecatedWorkTags().forEach { + NotificationScheduler.allDeprecatedNotificationWorkTags().forEach { assertTrue(getScheduledWorkers(it).isEmpty()) } } diff --git a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt index efd1cb607729..099bc0f8660e 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -51,7 +51,7 @@ class NotificationScheduler( } private fun cancelAllUnnecessaryWork(){ - allDeprecatedWorkTags().forEach { tag -> + allDeprecatedNotificationWorkTags().forEach { tag -> workManager.cancelAllWorkByTag(tag) } @@ -122,6 +122,6 @@ class NotificationScheduler( // we want to make sure that this TAGs are cancelled to avoid inconsistencies private const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment - fun allDeprecatedWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) + fun allDeprecatedNotificationWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) } } \ No newline at end of file From b85d348a1e28b9f5fa2804e773f4f698cbb5d271 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Tue, 28 Apr 2020 14:31:12 +0200 Subject: [PATCH 4/4] adding deprecated jobs before launching the test so it actually tests what's supposed to do --- .../AndroidNotificationSchedulerTest.kt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt index 5cf3de8740f6..544a48b13e4f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -19,10 +19,12 @@ package com.duckduckgo.app.notification import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager import com.duckduckgo.app.CoroutineTestRule -import com.duckduckgo.app.notification.NotificationScheduler.* +import com.duckduckgo.app.notification.NotificationScheduler.ClearDataNotificationWorker +import com.duckduckgo.app.notification.NotificationScheduler.PrivacyNotificationWorker import com.duckduckgo.app.notification.model.SchedulableNotification import com.duckduckgo.app.statistics.VariantManager import com.duckduckgo.app.statistics.VariantManager.Companion.DEFAULT_VARIANT @@ -98,10 +100,12 @@ class AndroidNotificationSchedulerTest { } @Test - fun allDeprecatedWorkIsCancelledUponStart() = runBlocking { + fun whenNotificationIsScheduledOldJobsAreCancelled() = runBlocking { whenever(privacyNotification.canShow()).thenReturn(false) whenever(clearNotification.canShow()).thenReturn(false) + enqueueDeprecatedJobs() + testee.scheduleNextNotification() NotificationScheduler.allDeprecatedNotificationWorkTags().forEach { @@ -109,6 +113,16 @@ class AndroidNotificationSchedulerTest { } } + private fun enqueueDeprecatedJobs() { + NotificationScheduler.allDeprecatedNotificationWorkTags().forEach { + val request = OneTimeWorkRequestBuilder() + .addTag(it) + .build() + + workManager.enqueue(request) + } + } + private fun assertUnusedAppNotificationScheduled(workerName: String) { assertTrue(getScheduledWorkers(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG).any { it.tags.contains(workerName) }) } @@ -123,5 +137,4 @@ class AndroidNotificationSchedulerTest { .get() .filter { it.state == WorkInfo.State.ENQUEUED } } - } \ No newline at end of file