From b52da000b16844b6817486f58121aefe63f767d2 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Mon, 27 Apr 2020 15:55:14 +0200 Subject: [PATCH 01/14] 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 02/14] 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 03/14] 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 bed3a0ae8100380602ef027248e8291e12f58b84 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Mon, 27 Apr 2020 21:44:31 +0200 Subject: [PATCH 04/14] we want to separate the responsibilities of clearing old jobs and scheduling new ones --- .../com/duckduckgo/app/job/JobCleanerTest.kt | 53 +++++++++++++++++++ .../duckduckgo/app/job/WorkSchedulerTest.kt | 48 +++++++++++++++++ .../java/com/duckduckgo/app/di/JobsModule.kt | 18 ++++++- .../java/com/duckduckgo/app/job/JobCleaner.kt | 41 ++++++++++++++ .../com/duckduckgo/app/job/WorkScheduler.kt | 33 ++++++++++++ 5 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt create mode 100644 app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt create mode 100644 app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt create mode 100644 app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt new file mode 100644 index 000000000000..5c9a6a4cb06b --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * 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.duckduckgo.app.job + +import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.WorkInfo +import androidx.work.WorkManager +import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +class JobCleanerTest { + + private val context = InstrumentationRegistry.getInstrumentation().targetContext + private var workManager = WorkManager.getInstance(context) + private lateinit var testee: JobCleaner + + @Before + fun before() { + testee = AndroidJobCleaner(workManager) + } + + @Test + fun allDeprecatedWorkIsCancelledUponStart() { + testee.cleanDeprecatedJobs() + + allDeprecatedNotificationWorkTags().forEach { + assertTrue(getScheduledWorkers(it).isEmpty()) + } + } + + private fun getScheduledWorkers(tag: String): List { + return workManager + .getWorkInfosByTag(tag) + .get() + .filter { it.state == WorkInfo.State.ENQUEUED } + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt new file mode 100644 index 000000000000..2a9fde8c1376 --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * 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.duckduckgo.app.job + +import com.duckduckgo.app.notification.NotificationScheduler +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify +import kotlinx.coroutines.runBlocking +import org.junit.Before +import org.junit.Test + +class WorkSchedulerTest { + + private val notificationScheduler: NotificationScheduler = mock() + private val jobCleaner: JobCleaner = mock() + + private lateinit var testee: WorkScheduler + + @Before + fun before() { + testee = AndroidWorkScheduler( + notificationScheduler, + jobCleaner + ) + } + + @Test + fun allDeprecatedWorkIsCancelledUponStart() = runBlocking { + testee.scheduleWork() + + verify(notificationScheduler).scheduleNextNotification() + verify(jobCleaner).cleanDeprecatedJobs() + } +} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt b/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt index dec7f870b8f6..7b261e732dfa 100644 --- a/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt @@ -18,11 +18,16 @@ package com.duckduckgo.app.di import android.app.job.JobScheduler import android.content.Context +import androidx.work.WorkManager +import com.duckduckgo.app.job.AndroidJobCleaner +import com.duckduckgo.app.job.AndroidWorkScheduler +import com.duckduckgo.app.job.JobCleaner +import com.duckduckgo.app.job.WorkScheduler +import com.duckduckgo.app.notification.NotificationScheduler import dagger.Module import dagger.Provides import javax.inject.Singleton - @Module class JobsModule { @@ -32,4 +37,15 @@ class JobsModule { return context.getSystemService(Context.JOB_SCHEDULER_SERVICE) as JobScheduler } + @Singleton + @Provides + fun providesJobCleaner(workManager: WorkManager): JobCleaner { + return AndroidJobCleaner(workManager) + } + + @Singleton + @Provides + fun providesWorkScheduler(notificationScheduler: NotificationScheduler, jobCleaner: JobCleaner): WorkScheduler { + return AndroidWorkScheduler(notificationScheduler, jobCleaner) + } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt b/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt new file mode 100644 index 000000000000..5a25799b153d --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * 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.duckduckgo.app.job + +import androidx.work.WorkManager +import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags + +interface JobCleaner { + fun cleanDeprecatedJobs() + + companion object { + // 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 + private const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment + + fun allDeprecatedNotificationWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) + } +} + +class AndroidJobCleaner(private val workManager: WorkManager) : JobCleaner { + + override fun cleanDeprecatedJobs() { + allDeprecatedNotificationWorkTags().forEach { tag -> + workManager.cancelAllWorkByTag(tag) + } + } +} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt b/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt new file mode 100644 index 000000000000..4f74908dc565 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * 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.duckduckgo.app.job + +import com.duckduckgo.app.notification.NotificationScheduler + +interface WorkScheduler { + suspend fun scheduleWork() +} + +class AndroidWorkScheduler( + private val notificationScheduler: NotificationScheduler, + private val jobCleaner: JobCleaner +) : WorkScheduler { + override suspend fun scheduleWork() { + jobCleaner.cleanDeprecatedJobs() + notificationScheduler.scheduleNextNotification() + } +} \ No newline at end of file From 67f48ffc4f2556f6cfea11454879248cd9c87010 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Mon, 27 Apr 2020 22:30:22 +0200 Subject: [PATCH 05/14] we want to separate the responsibilities of clearing old jobs and scheduling new ones --- .../app/di/StubJobSchedulerModule.kt | 18 ++++++++++++++++++ .../com/duckduckgo/app/di/TestAppComponent.kt | 1 - .../duckduckgo/app/job/WorkSchedulerTest.kt | 6 +++--- .../AndroidNotificationSchedulerTest.kt | 15 ++------------- .../java/com/duckduckgo/app/di/JobsModule.kt | 4 ++-- .../app/global/DuckDuckGoApplication.kt | 6 +++--- .../com/duckduckgo/app/job/WorkScheduler.kt | 4 ++-- .../AndroidNotificationScheduler.kt | 14 -------------- 8 files changed, 30 insertions(+), 38 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/di/StubJobSchedulerModule.kt b/app/src/androidTest/java/com/duckduckgo/app/di/StubJobSchedulerModule.kt index 0e54b9d3da37..b3f1b2654367 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/di/StubJobSchedulerModule.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/di/StubJobSchedulerModule.kt @@ -19,6 +19,12 @@ package com.duckduckgo.app.di import android.app.job.JobInfo import android.app.job.JobScheduler import android.app.job.JobWorkItem +import androidx.work.WorkManager +import com.duckduckgo.app.job.AndroidJobCleaner +import com.duckduckgo.app.job.AndroidWorkScheduler +import com.duckduckgo.app.job.JobCleaner +import com.duckduckgo.app.job.WorkScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import dagger.Module import dagger.Provides import javax.inject.Singleton @@ -44,4 +50,16 @@ class StubJobSchedulerModule { } } + + @Singleton + @Provides + fun providesJobCleaner(workManager: WorkManager): JobCleaner { + return AndroidJobCleaner(workManager) + } + + @Singleton + @Provides + fun providesWorkScheduler(notificationScheduler: AndroidNotificationScheduler, jobCleaner: JobCleaner): WorkScheduler { + return AndroidWorkScheduler(notificationScheduler, jobCleaner) + } } \ No newline at end of file diff --git a/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt b/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt index 3c3151c54476..f9f9d7f57fca 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt @@ -34,7 +34,6 @@ import retrofit2.Retrofit import javax.inject.Named import javax.inject.Singleton - @Singleton @Component( modules = [ diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt index 2a9fde8c1376..77ac299c56fe 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/WorkSchedulerTest.kt @@ -16,7 +16,7 @@ package com.duckduckgo.app.job -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import kotlinx.coroutines.runBlocking @@ -25,7 +25,7 @@ import org.junit.Test class WorkSchedulerTest { - private val notificationScheduler: NotificationScheduler = mock() + private val notificationScheduler: AndroidNotificationScheduler = mock() private val jobCleaner: JobCleaner = mock() private lateinit var testee: WorkScheduler @@ -39,7 +39,7 @@ class WorkSchedulerTest { } @Test - fun allDeprecatedWorkIsCancelledUponStart() = runBlocking { + fun schedulesNextNotificationAndCleansDeprecatedJobs() = runBlocking { testee.scheduleWork() verify(notificationScheduler).scheduleNextNotification() 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..a72b0dc668d2 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -22,7 +22,8 @@ import androidx.test.platform.app.InstrumentationRegistry 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 @@ -97,18 +98,6 @@ class AndroidNotificationSchedulerTest { assertNoUnusedAppNotificationScheduled() } - @Test - fun allDeprecatedWorkIsCancelledUponStart() = runBlocking { - whenever(privacyNotification.canShow()).thenReturn(false) - whenever(clearNotification.canShow()).thenReturn(false) - - testee.scheduleNextNotification() - - NotificationScheduler.allDeprecatedNotificationWorkTags().forEach { - assertTrue(getScheduledWorkers(it).isEmpty()) - } - } - private fun assertUnusedAppNotificationScheduled(workerName: String) { assertTrue(getScheduledWorkers(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG).any { it.tags.contains(workerName) }) } diff --git a/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt b/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt index 7b261e732dfa..867a8628e823 100644 --- a/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/JobsModule.kt @@ -23,7 +23,7 @@ import com.duckduckgo.app.job.AndroidJobCleaner import com.duckduckgo.app.job.AndroidWorkScheduler import com.duckduckgo.app.job.JobCleaner import com.duckduckgo.app.job.WorkScheduler -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import dagger.Module import dagger.Provides import javax.inject.Singleton @@ -45,7 +45,7 @@ class JobsModule { @Singleton @Provides - fun providesWorkScheduler(notificationScheduler: NotificationScheduler, jobCleaner: JobCleaner): WorkScheduler { + fun providesWorkScheduler(notificationScheduler: AndroidNotificationScheduler, jobCleaner: JobCleaner): WorkScheduler { return AndroidWorkScheduler(notificationScheduler, jobCleaner) } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt index 83097f16a618..b02aa7ee20f1 100644 --- a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt +++ b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt @@ -37,7 +37,7 @@ import com.duckduckgo.app.global.rating.AppEnjoymentLifecycleObserver import com.duckduckgo.app.global.shortcut.AppShortcutCreator import com.duckduckgo.app.httpsupgrade.HttpsUpgrader import com.duckduckgo.app.job.AppConfigurationSyncer -import com.duckduckgo.app.notification.AndroidNotificationScheduler +import com.duckduckgo.app.job.WorkScheduler import com.duckduckgo.app.notification.NotificationRegistrar import com.duckduckgo.app.referral.AppInstallationReferrerStateListener import com.duckduckgo.app.settings.db.SettingsDataStore @@ -120,7 +120,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO lateinit var dataClearer: DataClearer @Inject - lateinit var notificationScheduler: AndroidNotificationScheduler + lateinit var workScheduler: WorkScheduler @Inject lateinit var workerFactory: WorkerFactory @@ -293,7 +293,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO fun onAppResumed() { notificationRegistrar.updateStatus() GlobalScope.launch { - notificationScheduler.scheduleNextNotification() + workScheduler.scheduleWork() atbInitializer.initializeAfterReferrerAvailable() } } diff --git a/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt b/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt index 4f74908dc565..354cc7f9168c 100644 --- a/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/job/WorkScheduler.kt @@ -16,14 +16,14 @@ package com.duckduckgo.app.job -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler interface WorkScheduler { suspend fun scheduleWork() } class AndroidWorkScheduler( - private val notificationScheduler: NotificationScheduler, + private val notificationScheduler: AndroidNotificationScheduler, private val jobCleaner: JobCleaner ) : WorkScheduler { override suspend fun scheduleWork() { 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 099bc0f8660e..d4e4053d526c 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -46,17 +46,9 @@ class NotificationScheduler( ) : AndroidNotificationScheduler { override suspend fun scheduleNextNotification() { - cancelAllUnnecessaryWork() scheduleInactiveUserNotifications() } - private fun cancelAllUnnecessaryWork(){ - allDeprecatedNotificationWorkTags().forEach { tag -> - workManager.cancelAllWorkByTag(tag) - } - - } - private suspend fun scheduleInactiveUserNotifications() { workManager.cancelAllWorkByTag(UNUSED_APP_WORK_REQUEST_TAG) @@ -117,11 +109,5 @@ 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 - private const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment - - fun allDeprecatedNotificationWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) } } \ No newline at end of file From 76e156830a53878683c5e39fd2e5b3379fc3e992 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Tue, 12 May 2020 13:21:04 +0200 Subject: [PATCH 06/14] ensure we are scheduling some jobs before removing them --- .../com/duckduckgo/app/job/JobCleanerTest.kt | 15 ++++++++++-- .../AndroidNotificationSchedulerTest.kt | 24 ------------------- .../java/com/duckduckgo/app/job/JobCleaner.kt | 6 ++--- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index 5c9a6a4cb06b..680a6fcf919f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -17,9 +17,12 @@ package com.duckduckgo.app.job import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager +import androidx.work.WorkRequest import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags +import com.duckduckgo.app.notification.NotificationScheduler import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -36,7 +39,15 @@ class JobCleanerTest { } @Test - fun allDeprecatedWorkIsCancelledUponStart() { + fun whenStartedThenAllDeprecatedWorkIsCancelled() { + allDeprecatedNotificationWorkTags().forEach { + val requestBuilder = OneTimeWorkRequestBuilder() + val request = requestBuilder + .addTag(it) + .build() + workManager.enqueue(request) + } + testee.cleanDeprecatedJobs() allDeprecatedNotificationWorkTags().forEach { @@ -48,6 +59,6 @@ class JobCleanerTest { return workManager .getWorkInfosByTag(tag) .get() - .filter { it.state == WorkInfo.State.ENQUEUED } + // .filter { it.state == WorkInfo.State.ENQUEUED } } } \ No newline at end of file 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 544a48b13e4f..ca89247c18e1 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -99,30 +99,6 @@ class AndroidNotificationSchedulerTest { assertNoUnusedAppNotificationScheduled() } - @Test - fun whenNotificationIsScheduledOldJobsAreCancelled() = runBlocking { - whenever(privacyNotification.canShow()).thenReturn(false) - whenever(clearNotification.canShow()).thenReturn(false) - - enqueueDeprecatedJobs() - - testee.scheduleNextNotification() - - NotificationScheduler.allDeprecatedNotificationWorkTags().forEach { - assertTrue(getScheduledWorkers(it).isEmpty()) - } - } - - 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) }) } diff --git a/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt b/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt index 5a25799b153d..0e4da20e3e44 100644 --- a/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt +++ b/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt @@ -23,11 +23,9 @@ interface JobCleaner { fun cleanDeprecatedJobs() companion object { - // 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 - private const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment + private const val STICKY_SEARCH_CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment - fun allDeprecatedNotificationWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) + fun allDeprecatedNotificationWorkTags() = listOf(STICKY_SEARCH_CONTINUOUS_APP_USE_REQUEST_TAG) } } From e4eb51c4dc3b5fd62170b7c63c684cf4fa241f6a Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Tue, 12 May 2020 13:30:16 +0200 Subject: [PATCH 07/14] adding work managet test library --- app/build.gradle | 1 + .../com/duckduckgo/app/job/JobCleanerTest.kt | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index df5a19712903..de6aec87f9e7 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -187,6 +187,7 @@ dependencies { // WorkManager implementation "androidx.work:work-runtime-ktx:$workManager" + androidTestImplementation "androidx.work:work-testing:$workManager" // Dagger kapt "com.google.dagger:dagger-android-processor:$dagger" diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index 680a6fcf919f..24497199afc7 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -16,11 +16,16 @@ package com.duckduckgo.app.job +import android.util.Log +import androidx.test.InstrumentationRegistry.getTargetContext import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.Configuration import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkRequest +import androidx.work.impl.utils.SynchronousExecutor +import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags import com.duckduckgo.app.notification.NotificationScheduler import org.junit.Assert.assertTrue @@ -30,11 +35,13 @@ import org.junit.Test class JobCleanerTest { private val context = InstrumentationRegistry.getInstrumentation().targetContext - private var workManager = WorkManager.getInstance(context) + private lateinit var workManager: WorkManager private lateinit var testee: JobCleaner @Before fun before() { + initializeWorkManager() + workManager = WorkManager.getInstance(context) testee = AndroidJobCleaner(workManager) } @@ -43,7 +50,7 @@ class JobCleanerTest { allDeprecatedNotificationWorkTags().forEach { val requestBuilder = OneTimeWorkRequestBuilder() val request = requestBuilder - .addTag(it) + .addTag("tag") .build() workManager.enqueue(request) } @@ -55,10 +62,20 @@ class JobCleanerTest { } } + // https://developer.android.com/topic/libraries/architecture/workmanager/how-to/integration-testing + private fun initializeWorkManager() { + val config = Configuration.Builder() + .setMinimumLoggingLevel(Log.DEBUG) + .setExecutor(SynchronousExecutor()) + .build() + + WorkManagerTestInitHelper.initializeTestWorkManager(context, config) + } + private fun getScheduledWorkers(tag: String): List { return workManager .getWorkInfosByTag(tag) .get() - // .filter { it.state == WorkInfo.State.ENQUEUED } + .filter { it.state == WorkInfo.State.ENQUEUED } } } \ No newline at end of file From 89cddf99cd83bd48105635ecca5b47be400a3f21 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Wed, 13 May 2020 08:42:24 +0200 Subject: [PATCH 08/14] properly testing that the jobs are scheduled or not --- .../com/duckduckgo/app/job/JobCleanerTest.kt | 13 ++++++--- .../java/com/duckduckgo/app/job/TestWorker.kt | 27 +++++++++++++++++++ .../AndroidNotificationScheduler.kt | 1 - 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 app/src/androidTest/java/com/duckduckgo/app/job/TestWorker.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index 24497199afc7..8adad4deb5d5 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -16,6 +16,7 @@ package com.duckduckgo.app.job +import android.content.Context import android.util.Log import androidx.test.InstrumentationRegistry.getTargetContext import androidx.test.platform.app.InstrumentationRegistry @@ -24,6 +25,8 @@ import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkRequest +import androidx.work.Worker +import androidx.work.WorkerParameters import androidx.work.impl.utils.SynchronousExecutor import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags @@ -41,16 +44,15 @@ class JobCleanerTest { @Before fun before() { initializeWorkManager() - workManager = WorkManager.getInstance(context) testee = AndroidJobCleaner(workManager) } @Test fun whenStartedThenAllDeprecatedWorkIsCancelled() { allDeprecatedNotificationWorkTags().forEach { - val requestBuilder = OneTimeWorkRequestBuilder() + val requestBuilder = OneTimeWorkRequestBuilder() val request = requestBuilder - .addTag("tag") + .addTag(it) .build() workManager.enqueue(request) } @@ -58,7 +60,8 @@ class JobCleanerTest { testee.cleanDeprecatedJobs() allDeprecatedNotificationWorkTags().forEach { - assertTrue(getScheduledWorkers(it).isEmpty()) + val scheduledWorkers = getScheduledWorkers(it) + assertTrue(scheduledWorkers.isEmpty()) } } @@ -70,6 +73,7 @@ class JobCleanerTest { .build() WorkManagerTestInitHelper.initializeTestWorkManager(context, config) + workManager = WorkManager.getInstance(context) } private fun getScheduledWorkers(tag: String): List { @@ -78,4 +82,5 @@ class JobCleanerTest { .get() .filter { it.state == WorkInfo.State.ENQUEUED } } + } \ No newline at end of file diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/TestWorker.kt b/app/src/androidTest/java/com/duckduckgo/app/job/TestWorker.kt new file mode 100644 index 000000000000..6787656cc76a --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/job/TestWorker.kt @@ -0,0 +1,27 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * 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.duckduckgo.app.job + +import android.content.Context +import androidx.work.Worker +import androidx.work.WorkerParameters + +class TestWorker(context: Context, parameters: WorkerParameters) : Worker(context, parameters) { + override fun doWork(): Result { + return Result.success() + } +} \ No newline at end of file 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 099bc0f8660e..16b02c6b3398 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -54,7 +54,6 @@ class NotificationScheduler( allDeprecatedNotificationWorkTags().forEach { tag -> workManager.cancelAllWorkByTag(tag) } - } private suspend fun scheduleInactiveUserNotifications() { From a8eda12b5574063ae42cdea79fd6cf246ab66ab0 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Wed, 13 May 2020 08:44:08 +0200 Subject: [PATCH 09/14] initialising workmanager as documented --- .../AndroidNotificationSchedulerTest.kt | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) 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 ca89247c18e1..e3489ee94a66 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -18,10 +18,14 @@ package com.duckduckgo.app.notification +import android.util.Log import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.Configuration import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager +import androidx.work.impl.utils.SynchronousExecutor +import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.CoroutineTestRule import com.duckduckgo.app.notification.NotificationScheduler.ClearDataNotificationWorker import com.duckduckgo.app.notification.NotificationScheduler.PrivacyNotificationWorker @@ -50,11 +54,12 @@ class AndroidNotificationSchedulerTest { private val privacyNotification: SchedulableNotification = mock() private val context = InstrumentationRegistry.getInstrumentation().targetContext - private var workManager = WorkManager.getInstance(context) + private lateinit var workManager: WorkManager private lateinit var testee: NotificationScheduler @Before fun before() { + initializeWorkManager() whenever(variantManager.getVariant(any())).thenReturn(DEFAULT_VARIANT) testee = NotificationScheduler( workManager, @@ -63,6 +68,17 @@ class AndroidNotificationSchedulerTest { ) } + // https://developer.android.com/topic/libraries/architecture/workmanager/how-to/integration-testing + private fun initializeWorkManager() { + val config = Configuration.Builder() + .setMinimumLoggingLevel(Log.DEBUG) + .setExecutor(SynchronousExecutor()) + .build() + + WorkManagerTestInitHelper.initializeTestWorkManager(context, config) + workManager = WorkManager.getInstance(context) + } + @Test fun whenPrivacyNotificationClearDataCanShowThenPrivacyNotificationIsScheduled() = runBlocking { whenever(privacyNotification.canShow()).thenReturn(true) From d69cf7d037589563acc162da5c4a1e38e790e36a Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Wed, 13 May 2020 09:00:11 +0200 Subject: [PATCH 10/14] better testing strategy --- .../com/duckduckgo/app/job/JobCleanerTest.kt | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index 8adad4deb5d5..c0e58e74966a 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -31,9 +31,11 @@ import androidx.work.impl.utils.SynchronousExecutor import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags import com.duckduckgo.app.notification.NotificationScheduler +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import java.util.concurrent.TimeUnit class JobCleanerTest { @@ -47,40 +49,54 @@ class JobCleanerTest { testee = AndroidJobCleaner(workManager) } + // https://developer.android.com/topic/libraries/architecture/workmanager/how-to/integration-testing + private fun initializeWorkManager() { + val config = Configuration.Builder() + .setMinimumLoggingLevel(Log.DEBUG) + .setExecutor(SynchronousExecutor()) + .build() + + WorkManagerTestInitHelper.initializeTestWorkManager(context, config) + workManager = WorkManager.getInstance(context) + } + @Test fun whenStartedThenAllDeprecatedWorkIsCancelled() { + enqueueDeprecatedWorkers() + assertWorkersAreEnqueued() + testee.cleanDeprecatedJobs() + assertWorkersAreNotEnqueued() + } + + private fun enqueueDeprecatedWorkers(){ allDeprecatedNotificationWorkTags().forEach { val requestBuilder = OneTimeWorkRequestBuilder() val request = requestBuilder .addTag(it) + .setInitialDelay(10, TimeUnit.SECONDS) .build() workManager.enqueue(request) } + } - testee.cleanDeprecatedJobs() - + private fun assertWorkersAreEnqueued(){ allDeprecatedNotificationWorkTags().forEach { val scheduledWorkers = getScheduledWorkers(it) - assertTrue(scheduledWorkers.isEmpty()) + assertFalse(scheduledWorkers.isEmpty()) } } - // https://developer.android.com/topic/libraries/architecture/workmanager/how-to/integration-testing - private fun initializeWorkManager() { - val config = Configuration.Builder() - .setMinimumLoggingLevel(Log.DEBUG) - .setExecutor(SynchronousExecutor()) - .build() - - WorkManagerTestInitHelper.initializeTestWorkManager(context, config) - workManager = WorkManager.getInstance(context) + private fun assertWorkersAreNotEnqueued(){ + allDeprecatedNotificationWorkTags().forEach { + val scheduledWorkers = getScheduledWorkers(it) + assertTrue(scheduledWorkers.isEmpty()) + } } private fun getScheduledWorkers(tag: String): List { return workManager .getWorkInfosByTag(tag) .get() - .filter { it.state == WorkInfo.State.ENQUEUED } } } \ No newline at end of file From b75848c7429d533736644c840ca27c1cb508c938 Mon Sep 17 00:00:00 2001 From: David Gonzalez Date: Wed, 13 May 2020 09:22:29 +0200 Subject: [PATCH 11/14] add filter back --- .../androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index c0e58e74966a..8ef13fdf826d 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -97,6 +97,7 @@ class JobCleanerTest { return workManager .getWorkInfosByTag(tag) .get() + .filter { it.state == WorkInfo.State.ENQUEUED } } } \ No newline at end of file From f2f8db0d7c68352f9aef336684152dc5b2141361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Wed, 13 May 2020 11:48:59 +0200 Subject: [PATCH 12/14] formatting and comment cleanup --- .../java/com/duckduckgo/app/job/JobCleanerTest.kt | 13 +++---------- .../main/java/com/duckduckgo/app/job/JobCleaner.kt | 2 +- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index 8ef13fdf826d..3254c81f6fb4 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -16,21 +16,15 @@ package com.duckduckgo.app.job -import android.content.Context import android.util.Log -import androidx.test.InstrumentationRegistry.getTargetContext import androidx.test.platform.app.InstrumentationRegistry import androidx.work.Configuration import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager -import androidx.work.WorkRequest -import androidx.work.Worker -import androidx.work.WorkerParameters import androidx.work.impl.utils.SynchronousExecutor import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags -import com.duckduckgo.app.notification.NotificationScheduler import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -68,7 +62,7 @@ class JobCleanerTest { assertWorkersAreNotEnqueued() } - private fun enqueueDeprecatedWorkers(){ + private fun enqueueDeprecatedWorkers() { allDeprecatedNotificationWorkTags().forEach { val requestBuilder = OneTimeWorkRequestBuilder() val request = requestBuilder @@ -79,14 +73,14 @@ class JobCleanerTest { } } - private fun assertWorkersAreEnqueued(){ + private fun assertWorkersAreEnqueued() { allDeprecatedNotificationWorkTags().forEach { val scheduledWorkers = getScheduledWorkers(it) assertFalse(scheduledWorkers.isEmpty()) } } - private fun assertWorkersAreNotEnqueued(){ + private fun assertWorkersAreNotEnqueued() { allDeprecatedNotificationWorkTags().forEach { val scheduledWorkers = getScheduledWorkers(it) assertTrue(scheduledWorkers.isEmpty()) @@ -99,5 +93,4 @@ class JobCleanerTest { .get() .filter { it.state == WorkInfo.State.ENQUEUED } } - } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt b/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt index 0e4da20e3e44..8d6b9cfd7f58 100644 --- a/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt +++ b/app/src/main/java/com/duckduckgo/app/job/JobCleaner.kt @@ -23,7 +23,7 @@ interface JobCleaner { fun cleanDeprecatedJobs() companion object { - private const val STICKY_SEARCH_CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment + private const val STICKY_SEARCH_CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" fun allDeprecatedNotificationWorkTags() = listOf(STICKY_SEARCH_CONTINUOUS_APP_USE_REQUEST_TAG) } From 0e6d77c2734575be18392aec45b0a87f9c19d6f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Wed, 13 May 2020 12:15:41 +0200 Subject: [PATCH 13/14] clean up duplicate scheduler code --- .../notification/AndroidNotificationScheduler.kt | 13 ------------- 1 file changed, 13 deletions(-) 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 16b02c6b3398..d4e4053d526c 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -46,16 +46,9 @@ class NotificationScheduler( ) : AndroidNotificationScheduler { override suspend fun scheduleNextNotification() { - cancelAllUnnecessaryWork() scheduleInactiveUserNotifications() } - private fun cancelAllUnnecessaryWork(){ - allDeprecatedNotificationWorkTags().forEach { tag -> - workManager.cancelAllWorkByTag(tag) - } - } - private suspend fun scheduleInactiveUserNotifications() { workManager.cancelAllWorkByTag(UNUSED_APP_WORK_REQUEST_TAG) @@ -116,11 +109,5 @@ 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 - private const val CONTINUOUS_APP_USE_REQUEST_TAG = "com.duckduckgo.notification.schedule.continuous" // Sticky Search Experiment - - fun allDeprecatedNotificationWorkTags() = listOf(CONTINUOUS_APP_USE_REQUEST_TAG) } } \ No newline at end of file From bbde961acb7725de394ae97c69f4da4e95a768de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Wed, 13 May 2020 12:25:39 +0200 Subject: [PATCH 14/14] added new test to ensure non deprecated work is still enqueued --- .../com/duckduckgo/app/job/JobCleanerTest.kt | 39 +++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt index 3254c81f6fb4..62b81800a00c 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/job/JobCleanerTest.kt @@ -25,6 +25,7 @@ import androidx.work.WorkManager import androidx.work.impl.utils.SynchronousExecutor import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.job.JobCleaner.Companion.allDeprecatedNotificationWorkTags +import com.duckduckgo.app.notification.NotificationScheduler import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -57,9 +58,17 @@ class JobCleanerTest { @Test fun whenStartedThenAllDeprecatedWorkIsCancelled() { enqueueDeprecatedWorkers() - assertWorkersAreEnqueued() + assertDeprecatedWorkersAreEnqueued() testee.cleanDeprecatedJobs() - assertWorkersAreNotEnqueued() + assertDeprecatedWorkersAreNotEnqueued() + } + + @Test + fun whenStartedAndNoDeprecatedJobsAreScheduledThenNothingIsRemoved() { + enqueueNonDeprecatedWorkers() + assertNonDeprecatedWorkersAreEnqueued() + testee.cleanDeprecatedJobs() + assertNonDeprecatedWorkersAreEnqueued() } private fun enqueueDeprecatedWorkers() { @@ -73,20 +82,42 @@ class JobCleanerTest { } } - private fun assertWorkersAreEnqueued() { + private fun enqueueNonDeprecatedWorkers() { + allDeprecatedNotificationWorkTags().forEach { + val requestBuilder = OneTimeWorkRequestBuilder() + val request = requestBuilder + .addTag(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG) + .setInitialDelay(10, TimeUnit.SECONDS) + .build() + workManager.enqueue(request) + } + } + + private fun assertDeprecatedWorkersAreEnqueued() { allDeprecatedNotificationWorkTags().forEach { val scheduledWorkers = getScheduledWorkers(it) assertFalse(scheduledWorkers.isEmpty()) } } - private fun assertWorkersAreNotEnqueued() { + private fun assertDeprecatedWorkersAreNotEnqueued() { allDeprecatedNotificationWorkTags().forEach { val scheduledWorkers = getScheduledWorkers(it) assertTrue(scheduledWorkers.isEmpty()) } } + private fun assertNonDeprecatedWorkersAreEnqueued() { + val scheduledWorkers = getScheduledWorkers(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG) + assertFalse(scheduledWorkers.isEmpty()) + } + + private fun assertNonDeprecatedWorkersAreNotEnqueued() { + val scheduledWorkers = getScheduledWorkers(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG) + assertTrue(scheduledWorkers.isEmpty()) + } + + private fun getScheduledWorkers(tag: String): List { return workManager .getWorkInfosByTag(tag)