From 6b4c2c6f30dc9696e7d7f642925e3dbc09164614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Fri, 20 Mar 2020 08:52:25 +0100 Subject: [PATCH 1/3] ensure there's always a themeResource returning --- app/src/main/java/com/duckduckgo/app/global/Theming.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/global/Theming.kt b/app/src/main/java/com/duckduckgo/app/global/Theming.kt index 9ce90e12f409..68c7b3ec1003 100644 --- a/app/src/main/java/com/duckduckgo/app/global/Theming.kt +++ b/app/src/main/java/com/duckduckgo/app/global/Theming.kt @@ -25,7 +25,6 @@ import com.duckduckgo.app.browser.R import com.duckduckgo.app.global.Theming.Constants.BROADCAST_THEME_CHANGED import com.duckduckgo.app.global.Theming.Constants.THEME_MAP import com.duckduckgo.app.settings.db.SettingsDataStore -import com.duckduckgo.app.statistics.VariantManager enum class DuckDuckGoTheme { DARK, @@ -74,5 +73,9 @@ fun DuckDuckGoActivity.sendThemeChangedBroadcast() { } private fun DuckDuckGoActivity.manifestThemeId(): Int { - return packageManager.getActivityInfo(componentName, 0).themeResource + return try { + packageManager.getActivityInfo(componentName, 0).themeResource + } catch (exception: Exception) { + R.style.AppTheme + } } From c351ddcd207f53ad7493b4c424084daf67428ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Fri, 20 Mar 2020 11:36:48 +0100 Subject: [PATCH 2/3] fixing issue where DaggerWorkerFactory was not able to find the Worker class --- ...kt => AndroidNotificationSchedulerTest.kt} | 13 ++++--- .../app/settings/SettingsViewModelTest.kt | 4 +- .../duckduckgo/app/di/DaggerWorkerFactory.kt | 38 ++++++++++++------- .../duckduckgo/app/di/NotificationModule.kt | 8 ++-- .../app/global/DuckDuckGoApplication.kt | 4 +- .../duckduckgo/app/global/ViewModelFactory.kt | 5 +-- ...ler.kt => AndroidNotificationScheduler.kt} | 17 ++++----- .../NotificationHandlerService.kt | 2 +- .../app/settings/SettingsViewModel.kt | 4 +- 9 files changed, 51 insertions(+), 44 deletions(-) rename app/src/androidTest/java/com/duckduckgo/app/notification/{NotificationSchedulerTest.kt => AndroidNotificationSchedulerTest.kt} (94%) rename app/src/main/java/com/duckduckgo/app/notification/{NotificationScheduler.kt => AndroidNotificationScheduler.kt} (93%) diff --git a/app/src/androidTest/java/com/duckduckgo/app/notification/NotificationSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt similarity index 94% rename from app/src/androidTest/java/com/duckduckgo/app/notification/NotificationSchedulerTest.kt rename to app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt index 40c32cc0d77a..54f8a3f383ea 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/NotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -22,7 +22,7 @@ import androidx.test.platform.app.InstrumentationRegistry import androidx.work.WorkInfo import androidx.work.WorkManager import com.duckduckgo.app.CoroutineTestRule -import com.duckduckgo.app.notification.AndroidNotificationScheduler.* +import com.duckduckgo.app.notification.NotificationScheduler.* import com.duckduckgo.app.notification.model.SchedulableNotification import com.duckduckgo.app.notification.model.SearchNotification import com.duckduckgo.app.statistics.VariantManager @@ -39,7 +39,7 @@ import org.junit.Rule import org.junit.Test import kotlin.reflect.jvm.jvmName -class NotificationSchedulerTest { +class AndroidNotificationSchedulerTest { @ExperimentalCoroutinesApi @get:Rule @@ -57,7 +57,7 @@ class NotificationSchedulerTest { @Before fun before() { whenever(variantManager.getVariant(any())).thenReturn(DEFAULT_VARIANT) - testee = AndroidNotificationScheduler( + testee = NotificationScheduler( workManager, clearNotification, privacyNotification, @@ -67,7 +67,7 @@ class NotificationSchedulerTest { @After fun resetWorkers() { - workManager.cancelAllWorkByTag(AndroidNotificationScheduler.CONTINUOUS_APP_USE_REQUEST_TAG) + workManager.cancelAllWorkByTag(NotificationScheduler.CONTINUOUS_APP_USE_REQUEST_TAG) } @Test @@ -141,6 +141,7 @@ class NotificationSchedulerTest { whenever(privacyNotification.canShow()).thenReturn(false) whenever(clearNotification.canShow()).thenReturn(false) whenever(searchPromptNotification.canShow()).thenReturn(true) + testee.scheduleNextNotification() assertContinuousAppUseNotificationScheduled(SearchPromptNotificationWorker::class.jvmName) @@ -180,14 +181,14 @@ class NotificationSchedulerTest { private fun getUnusedAppScheduledWorkers(): List { return workManager - .getWorkInfosByTag(AndroidNotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG) + .getWorkInfosByTag(NotificationScheduler.UNUSED_APP_WORK_REQUEST_TAG) .get() .filter { it.state == WorkInfo.State.ENQUEUED } } private fun getContinuousAppUseScheduledWorkers(): List { return workManager - .getWorkInfosByTag(AndroidNotificationScheduler.CONTINUOUS_APP_USE_REQUEST_TAG) + .getWorkInfosByTag(NotificationScheduler.CONTINUOUS_APP_USE_REQUEST_TAG) .get() .filter { it.state == WorkInfo.State.ENQUEUED } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt index b9f7a2e7af9d..48a624d41884 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt @@ -25,7 +25,7 @@ import com.duckduckgo.app.browser.BuildConfig import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector import com.duckduckgo.app.global.DuckDuckGoTheme import com.duckduckgo.app.icon.api.AppIcon -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.duckduckgo.app.settings.SettingsViewModel.Command import com.duckduckgo.app.settings.clear.ClearWhatOption.CLEAR_NONE import com.duckduckgo.app.settings.clear.ClearWhenOption.APP_EXIT_ONLY @@ -68,7 +68,7 @@ class SettingsViewModelTest { private lateinit var mockVariantManager: VariantManager @Mock - private lateinit var notificationScheduler: NotificationScheduler + private lateinit var notificationScheduler: AndroidNotificationScheduler @Mock private lateinit var mockPixel: Pixel diff --git a/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt b/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt index c97f9590a577..13ff4f646365 100644 --- a/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt +++ b/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt @@ -23,7 +23,11 @@ import androidx.work.WorkerFactory import androidx.work.WorkerParameters import com.duckduckgo.app.fire.DataClearingWorker import com.duckduckgo.app.global.view.ClearDataAction -import com.duckduckgo.app.notification.AndroidNotificationScheduler.* +import com.duckduckgo.app.notification.NotificationScheduler.ClearDataNotificationWorker +import com.duckduckgo.app.notification.NotificationScheduler.PrivacyNotificationWorker +import com.duckduckgo.app.notification.NotificationScheduler.SearchPromptNotificationWorker +import com.duckduckgo.app.notification.NotificationScheduler.StickySearchNotificationWorker +import com.duckduckgo.app.notification.NotificationScheduler.DismissSearchNotificationWorker import com.duckduckgo.app.notification.NotificationFactory import com.duckduckgo.app.notification.db.NotificationDao import com.duckduckgo.app.notification.model.ClearDataNotification @@ -52,22 +56,28 @@ class DaggerWorkerFactory( override fun createWorker(appContext: Context, workerClassName: String, workerParameters: WorkerParameters): ListenableWorker? { - val workerClass = Class.forName(workerClassName).asSubclass(ListenableWorker::class.java) - val constructor = workerClass.getDeclaredConstructor(Context::class.java, WorkerParameters::class.java) - val instance = constructor.newInstance(appContext, workerParameters) + try { + val workerClass = Class.forName(workerClassName).asSubclass(ListenableWorker::class.java) + val constructor = workerClass.getDeclaredConstructor(Context::class.java, WorkerParameters::class.java) + val instance = constructor.newInstance(appContext, workerParameters) - when (instance) { - is OfflinePixelScheduler.OfflinePixelWorker -> injectOfflinePixelWorker(instance) - is DataClearingWorker -> injectDataClearWorker(instance) - is ClearDataNotificationWorker -> injectClearDataNotificationWorker(instance) - is PrivacyNotificationWorker -> injectPrivacyNotificationWorker(instance) - is SearchPromptNotificationWorker -> injectSearchPromptNotificationWorker(instance) - is StickySearchNotificationWorker -> injectStickySearchNotificationWorker(instance) - is DismissSearchNotificationWorker -> injectDismissSearchNotificationWorker(instance) - else -> Timber.i("No injection required for worker $workerClassName") + when (instance) { + is OfflinePixelScheduler.OfflinePixelWorker -> injectOfflinePixelWorker(instance) + is DataClearingWorker -> injectDataClearWorker(instance) + is ClearDataNotificationWorker -> injectClearDataNotificationWorker(instance) + is PrivacyNotificationWorker -> injectPrivacyNotificationWorker(instance) + is SearchPromptNotificationWorker -> injectSearchPromptNotificationWorker(instance) + is StickySearchNotificationWorker -> injectStickySearchNotificationWorker(instance) + is DismissSearchNotificationWorker -> injectDismissSearchNotificationWorker(instance) + else -> Timber.i("No injection required for worker $workerClassName") + } + + return instance + } catch (exception: Exception){ + Timber.i("Worker $workerClassName could not be created") + return null } - return instance } private fun injectOfflinePixelWorker(worker: OfflinePixelScheduler.OfflinePixelWorker) { diff --git a/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt b/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt index 2e4ee9fdab14..2040ece068d7 100644 --- a/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt @@ -21,9 +21,9 @@ import android.content.Context import androidx.core.app.NotificationManagerCompat import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.work.WorkManager -import com.duckduckgo.app.notification.AndroidNotificationScheduler -import com.duckduckgo.app.notification.NotificationFactory import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.NotificationFactory +import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.duckduckgo.app.notification.db.NotificationDao import com.duckduckgo.app.notification.model.ClearDataNotification import com.duckduckgo.app.notification.model.PrivacyProtectionNotification @@ -100,8 +100,8 @@ class NotificationModule { clearDataNotification: ClearDataNotification, privacyProtectionNotification: PrivacyProtectionNotification, stickySearchNotification: StickySearchNotification - ): NotificationScheduler { - return AndroidNotificationScheduler( + ): AndroidNotificationScheduler { + return NotificationScheduler( workManager, clearDataNotification, privacyProtectionNotification, 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 08631e0c8f91..fd4d9fa7d451 100644 --- a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt +++ b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt @@ -41,7 +41,7 @@ import com.duckduckgo.app.global.shortcut.AppShortcutCreator import com.duckduckgo.app.httpsupgrade.HttpsUpgrader import com.duckduckgo.app.job.AppConfigurationSyncer import com.duckduckgo.app.notification.NotificationRegistrar -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.duckduckgo.app.referral.AppInstallationReferrerStateListener import com.duckduckgo.app.settings.db.SettingsDataStore import com.duckduckgo.app.statistics.AtbInitializer @@ -129,7 +129,7 @@ open class DuckDuckGoApplication : HasActivityInjector, HasServiceInjector, HasS lateinit var dataClearer: DataClearer @Inject - lateinit var notificationScheduler: NotificationScheduler + lateinit var notificationScheduler: AndroidNotificationScheduler @Inject lateinit var workerFactory: WorkerFactory diff --git a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt index 203db987d56b..0c209f9db8a5 100644 --- a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt +++ b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt @@ -41,11 +41,10 @@ import com.duckduckgo.app.global.install.AppInstallStore import com.duckduckgo.app.global.model.SiteFactory import com.duckduckgo.app.global.rating.AppEnjoymentPromptEmitter import com.duckduckgo.app.global.rating.AppEnjoymentUserEventRecorder -import com.duckduckgo.app.icon.api.AppIconModifier import com.duckduckgo.app.icon.api.IconModifier import com.duckduckgo.app.icon.ui.ChangeIconViewModel import com.duckduckgo.app.launch.LaunchViewModel -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.duckduckgo.app.onboarding.store.OnboardingStore import com.duckduckgo.app.onboarding.ui.OnboardingPageManager import com.duckduckgo.app.onboarding.ui.OnboardingViewModel @@ -112,7 +111,7 @@ class ViewModelFactory @Inject constructor( private val onboardingPageManager: OnboardingPageManager, private val appInstallationReferrerStateListener: AppInstallationReferrerStateListener, private val appIconModifier: IconModifier, - private val notificationScheduler: NotificationScheduler + private val notificationScheduler: AndroidNotificationScheduler ) : ViewModelProvider.NewInstanceFactory() { override fun create(modelClass: Class) = diff --git a/app/src/main/java/com/duckduckgo/app/notification/NotificationScheduler.kt b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt similarity index 93% rename from app/src/main/java/com/duckduckgo/app/notification/NotificationScheduler.kt rename to app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt index 6d2db085b3e5..7fc334ad6e3d 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/NotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -17,7 +17,6 @@ package com.duckduckgo.app.notification import android.content.Context -import android.content.Intent import androidx.annotation.WorkerThread import androidx.core.app.NotificationManagerCompat import androidx.work.CoroutineWorker @@ -25,34 +24,32 @@ import androidx.work.OneTimeWorkRequest import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkManager import androidx.work.WorkerParameters -import com.duckduckgo.app.notification.NotificationHandlerService.Companion.NOTIFICATION_AUTO_CANCEL -import com.duckduckgo.app.notification.NotificationHandlerService.Companion.NOTIFICATION_SYSTEM_ID_EXTRA -import com.duckduckgo.app.notification.NotificationHandlerService.Companion.PIXEL_SUFFIX_EXTRA import com.duckduckgo.app.notification.db.NotificationDao import com.duckduckgo.app.notification.model.Notification import com.duckduckgo.app.notification.model.SchedulableNotification import com.duckduckgo.app.notification.model.SearchNotification -import com.duckduckgo.app.notification.model.SearchPromptNotification -import com.duckduckgo.app.settings.db.SettingsDataStore import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.app.statistics.pixels.Pixel.PixelName.NOTIFICATION_SHOWN import timber.log.Timber import java.util.concurrent.TimeUnit + +// Please don't rename any Worker class name or class path. +// More information: https://craigrussell.io/2019/04/a-workmanager-pitfall-modifying-a-scheduled-worker/ @WorkerThread -interface NotificationScheduler { +interface AndroidNotificationScheduler { suspend fun scheduleNextNotification() fun launchStickySearchNotification() fun dismissStickySearchNotification() fun launchSearchPromptNotification() } -class AndroidNotificationScheduler( +class NotificationScheduler( private val workManager: WorkManager, private val clearDataNotification: SchedulableNotification, private val privacyNotification: SchedulableNotification, private val searchPromptNotification: SearchNotification -) : NotificationScheduler { +) : AndroidNotificationScheduler { override suspend fun scheduleNextNotification() { scheduleInactiveUserNotifications() @@ -66,7 +63,7 @@ class AndroidNotificationScheduler( } private suspend fun scheduleInactiveUserNotifications() { - workManager.cancelAllWorkByTag(UNUSED_APP_WORK_REQUEST_TAG) + workManager.cancelUniqueWork(UNUSED_APP_WORK_REQUEST_TAG) when { privacyNotification.canShow() -> { diff --git a/app/src/main/java/com/duckduckgo/app/notification/NotificationHandlerService.kt b/app/src/main/java/com/duckduckgo/app/notification/NotificationHandlerService.kt index 5f3438ab2bb0..704f392acd0c 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/NotificationHandlerService.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/NotificationHandlerService.kt @@ -54,7 +54,7 @@ class NotificationHandlerService : IntentService("NotificationHandlerService") { lateinit var notificationManager: NotificationManagerCompat @Inject - lateinit var notificationScheduler: NotificationScheduler + lateinit var notificationScheduler: AndroidNotificationScheduler @Inject lateinit var settingsDataStore: SettingsDataStore diff --git a/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt b/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt index 86a2b39ac0dd..5b94bb3fea72 100644 --- a/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt @@ -23,7 +23,7 @@ import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector import com.duckduckgo.app.global.DuckDuckGoTheme import com.duckduckgo.app.global.SingleLiveEvent import com.duckduckgo.app.icon.api.AppIcon -import com.duckduckgo.app.notification.NotificationScheduler +import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.duckduckgo.app.settings.clear.ClearWhatOption import com.duckduckgo.app.settings.clear.ClearWhenOption import com.duckduckgo.app.settings.db.SettingsDataStore @@ -40,7 +40,7 @@ class SettingsViewModel @Inject constructor( private val defaultWebBrowserCapability: DefaultBrowserDetector, private val variantManager: VariantManager, private val pixel: Pixel, - private val notificationScheduler: NotificationScheduler + private val notificationScheduler: AndroidNotificationScheduler ) : ViewModel() { data class ViewState( From 632df2635697681dbba2d3042f3838c56fbf64d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Fri, 20 Mar 2020 13:49:23 +0100 Subject: [PATCH 3/3] addressed PR comments --- .../main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt | 2 +- .../app/notification/AndroidNotificationScheduler.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt b/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt index 13ff4f646365..ba28774a8498 100644 --- a/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt +++ b/app/src/main/java/com/duckduckgo/app/di/DaggerWorkerFactory.kt @@ -74,7 +74,7 @@ class DaggerWorkerFactory( return instance } catch (exception: Exception){ - Timber.i("Worker $workerClassName could not be created") + Timber.e(exception, "Worker $workerClassName could not be created") return null } 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 7fc334ad6e3d..889b35e90b44 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -34,7 +34,7 @@ import timber.log.Timber import java.util.concurrent.TimeUnit -// Please don't rename any Worker class name or class path. +// Please don't rename any Worker class name or class path // More information: https://craigrussell.io/2019/04/a-workmanager-pitfall-modifying-a-scheduled-worker/ @WorkerThread interface AndroidNotificationScheduler { @@ -63,7 +63,7 @@ class NotificationScheduler( } private suspend fun scheduleInactiveUserNotifications() { - workManager.cancelUniqueWork(UNUSED_APP_WORK_REQUEST_TAG) + workManager.cancelAllWorkByTag(UNUSED_APP_WORK_REQUEST_TAG) when { privacyNotification.canShow() -> {