From 7342567ce86368495a09285431215a3492b08da0 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Tue, 31 May 2022 10:35:56 +0300 Subject: [PATCH] For #12258 - Retry deleting Pocket profile if initially failed It's important to ensure the profile is deleted when the sponsored stories feature is disabled but since this involves a network call which may fail we need to support retrying a previous failed request until profile deletion is successful or the sponsored stories functionality is started again. --- .../service/pocket/PocketStoriesService.kt | 16 +++-- .../pocket/update/DeleteSpocsProfileWorker.kt | 36 ++++++++++ .../pocket/update/SpocsRefreshScheduler.kt | 34 ++++++++- .../pocket/PocketStoriesServiceTest.kt | 61 +++++++++++----- .../update/DeleteSpocsProfileWorkerTest.kt | 65 +++++++++++++++++ .../update/SpocsRefreshSchedulerTest.kt | 71 +++++++++++++++++-- docs/changelog.md | 3 + 7 files changed, 255 insertions(+), 31 deletions(-) create mode 100644 components/service/pocket/src/main/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorker.kt create mode 100644 components/service/pocket/src/test/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorkerTest.kt diff --git a/components/service/pocket/src/main/java/mozilla/components/service/pocket/PocketStoriesService.kt b/components/service/pocket/src/main/java/mozilla/components/service/pocket/PocketStoriesService.kt index 603483346b6..4edd1726603 100644 --- a/components/service/pocket/src/main/java/mozilla/components/service/pocket/PocketStoriesService.kt +++ b/components/service/pocket/src/main/java/mozilla/components/service/pocket/PocketStoriesService.kt @@ -101,6 +101,7 @@ class PocketStoriesService( } GlobalDependencyProvider.SponsoredStories.initialize(useCases) + spocsRefreshscheduler.stopProfileDeletion(context) spocsRefreshscheduler.schedulePeriodicRefreshes(context) } @@ -114,7 +115,6 @@ class PocketStoriesService( */ fun stopPeriodicSponsoredStoriesRefresh() { spocsRefreshscheduler.stopPeriodicRefreshes(context) - GlobalDependencyProvider.SponsoredStories.reset() } /** @@ -126,10 +126,18 @@ class PocketStoriesService( /** * Delete all stored user data used for downloading personalized sponsored stories. + * This returns immediately but will handle the profile deletion in background. */ - suspend fun deleteProfile(): Boolean { - stopPeriodicSponsoredStoriesRefresh() - return spocsUseCases?.deleteProfile?.invoke() ?: false + fun deleteProfile() { + val useCases = spocsUseCases + if (useCases == null) { + logger.warn("Cannot delete sponsored stories profile. Service has incomplete setup") + return + } + + GlobalDependencyProvider.SponsoredStories.initialize(useCases) + spocsRefreshscheduler.stopPeriodicRefreshes(context) + spocsRefreshscheduler.scheduleProfileDeletion(context) } /** diff --git a/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorker.kt b/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorker.kt new file mode 100644 index 00000000000..afe1d40bf5e --- /dev/null +++ b/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorker.kt @@ -0,0 +1,36 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.service.pocket.update + +import android.content.Context +import androidx.work.CoroutineWorker +import androidx.work.WorkerParameters +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import mozilla.components.service.pocket.GlobalDependencyProvider + +/** + * WorkManager Worker used for deleting the profile used for downloading Pocket sponsored stories. + */ +internal class DeleteSpocsProfileWorker( + context: Context, + params: WorkerParameters +) : CoroutineWorker(context, params) { + + override suspend fun doWork(): Result { + return withContext(Dispatchers.IO) { + if (GlobalDependencyProvider.SponsoredStories.useCases?.deleteProfile?.invoke() == true) { + Result.success() + } else { + Result.retry() + } + } + } + + internal companion object { + const val DELETE_SPOCS_PROFILE_WORK_TAG = + "mozilla.components.feature.pocket.spocs.profile.delete.work.tag" + } +} diff --git a/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/SpocsRefreshScheduler.kt b/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/SpocsRefreshScheduler.kt index 24ca6efd48b..311a26474d5 100644 --- a/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/SpocsRefreshScheduler.kt +++ b/components/service/pocket/src/main/java/mozilla/components/service/pocket/update/SpocsRefreshScheduler.kt @@ -8,12 +8,16 @@ import android.content.Context import androidx.annotation.VisibleForTesting import androidx.work.Constraints import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.ExistingWorkPolicy import androidx.work.NetworkType +import androidx.work.OneTimeWorkRequest +import androidx.work.OneTimeWorkRequestBuilder import androidx.work.PeriodicWorkRequest import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager import mozilla.components.service.pocket.PocketStoriesConfig import mozilla.components.service.pocket.logger +import mozilla.components.service.pocket.update.DeleteSpocsProfileWorker.Companion.DELETE_SPOCS_PROFILE_WORK_TAG import mozilla.components.service.pocket.update.RefreshSpocsWorker.Companion.REFRESH_SPOCS_WORK_TAG import mozilla.components.support.base.worker.Frequency @@ -26,7 +30,7 @@ internal class SpocsRefreshScheduler( internal fun schedulePeriodicRefreshes(context: Context) { logger.info("Scheduling sponsored stories background refresh") - val refreshWork = createPeriodicWorkerRequest( + val refreshWork = createPeriodicRefreshWorkerRequest( frequency = pocketStoriesConfig.sponsoredStoriesRefreshFrequency ) @@ -39,8 +43,34 @@ internal class SpocsRefreshScheduler( .cancelAllWorkByTag(REFRESH_SPOCS_WORK_TAG) } + internal fun scheduleProfileDeletion(context: Context) { + logger.info("Scheduling sponsored stories profile deletion") + + val deleteProfileWork = createOneTimeProfileDeletionWorkerRequest() + + getWorkManager(context) + .enqueueUniqueWork(DELETE_SPOCS_PROFILE_WORK_TAG, ExistingWorkPolicy.KEEP, deleteProfileWork) + } + + internal fun stopProfileDeletion(context: Context) { + getWorkManager(context) + .cancelAllWorkByTag(DELETE_SPOCS_PROFILE_WORK_TAG) + } + + @VisibleForTesting + internal fun createOneTimeProfileDeletionWorkerRequest(): OneTimeWorkRequest { + val constraints = getWorkerConstrains() + + return OneTimeWorkRequestBuilder() + .apply { + setConstraints(constraints) + addTag(DELETE_SPOCS_PROFILE_WORK_TAG) + } + .build() + } + @VisibleForTesting - internal fun createPeriodicWorkerRequest( + internal fun createPeriodicRefreshWorkerRequest( frequency: Frequency ): PeriodicWorkRequest { val constraints = getWorkerConstrains() diff --git a/components/service/pocket/src/test/java/mozilla/components/service/pocket/PocketStoriesServiceTest.kt b/components/service/pocket/src/test/java/mozilla/components/service/pocket/PocketStoriesServiceTest.kt index c0bccafa652..bfbe4b3c946 100644 --- a/components/service/pocket/src/test/java/mozilla/components/service/pocket/PocketStoriesServiceTest.kt +++ b/components/service/pocket/src/test/java/mozilla/components/service/pocket/PocketStoriesServiceTest.kt @@ -12,7 +12,6 @@ import mozilla.components.service.pocket.PocketStory.PocketRecommendedStory import mozilla.components.service.pocket.PocketStory.PocketSponsoredStory import mozilla.components.service.pocket.helpers.assertConstructorsVisibility import mozilla.components.service.pocket.spocs.SpocsUseCases -import mozilla.components.service.pocket.spocs.SpocsUseCases.DeleteProfile import mozilla.components.service.pocket.spocs.SpocsUseCases.GetSponsoredStories import mozilla.components.service.pocket.spocs.SpocsUseCases.RecordImpression import mozilla.components.service.pocket.stories.PocketStoriesUseCases @@ -23,7 +22,6 @@ import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import org.junit.After import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -31,8 +29,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.doReturn import org.mockito.Mockito.never -import org.mockito.Mockito.spy -import org.mockito.Mockito.times import org.mockito.Mockito.verify import java.util.UUID import kotlin.reflect.KVisibility @@ -77,7 +73,7 @@ class PocketStoriesServiceTest { } @Test - fun `GIVEN PocketStoriesService is initialized with a valid profile WHEN called to start periodic refreshes THEN persist dependencies and schedule stories refresh`() { + fun `GIVEN PocketStoriesService is initialized with a valid profile WHEN called to start periodic refreshes THEN persist dependencies, cancel profile deletion and schedule stories refresh`() { val client: Client = mock() val profileId = UUID.randomUUID() val appId = "test" @@ -97,6 +93,7 @@ class PocketStoriesServiceTest { service.startPeriodicSponsoredStoriesRefresh() assertNotNull(GlobalDependencyProvider.SponsoredStories.useCases) + verify(service.spocsRefreshscheduler).stopProfileDeletion(any()) verify(service.spocsRefreshscheduler).schedulePeriodicRefreshes(any()) } @@ -119,7 +116,7 @@ class PocketStoriesServiceTest { } @Test - fun `GIVEN PocketStoriesService WHEN called to stop periodic refreshes THEN stop refreshing stories and clear dependencies`() { + fun `GIVEN PocketStoriesService WHEN called to stop periodic refreshes THEN stop refreshing stories`() { // Mock periodic refreshes were started previously and profile details were set. // Now they will have to be cleaned. GlobalDependencyProvider.SponsoredStories.initialize(mock()) @@ -128,7 +125,6 @@ class PocketStoriesServiceTest { service.stopPeriodicSponsoredStoriesRefresh() verify(service.spocsRefreshscheduler).stopPeriodicRefreshes(any()) - assertNull(GlobalDependencyProvider.SponsoredStories.useCases) } @Test @@ -168,17 +164,46 @@ class PocketStoriesServiceTest { } @Test - fun `GIVEN PocketStoriesService WHEN deleteProfile THEN delegate to spocs useCases`() = runTest { - val mockedService = spy(service) - val noProfileResponse = mockedService.deleteProfile() - assertFalse(noProfileResponse) - - val deleteProfileUseCase: DeleteProfile = mock() - doReturn(deleteProfileUseCase).`when`(spocsUseCases).deleteProfile - doReturn(true).`when`(deleteProfileUseCase).invoke() - val existingProfileResponse = mockedService.deleteProfile() - assertTrue(existingProfileResponse) - verify(mockedService, times(2)).stopPeriodicSponsoredStoriesRefresh() + fun `GIVEN PocketStoriesService is initialized with a valid profile WHEN called to delete profile THEN persist dependencies, cancel stories refresh and schedule profile deletion`() { + val client: Client = mock() + val profileId = UUID.randomUUID() + val appId = "test" + val service = PocketStoriesService( + context = testContext, + pocketStoriesConfig = PocketStoriesConfig( + client = client, + profile = Profile( + profileId = profileId, + appId = appId + ) + ) + ).apply { + spocsRefreshscheduler = mock() + } + + service.deleteProfile() + + assertNotNull(GlobalDependencyProvider.SponsoredStories.useCases) + verify(service.spocsRefreshscheduler).stopPeriodicRefreshes(any()) + verify(service.spocsRefreshscheduler).scheduleProfileDeletion(any()) + } + + @Test + fun `GIVEN PocketStoriesService is initialized with an invalid profile WHEN called to delete profile THEN don't schedule profile deletion and don't persist dependencies`() { + val service = PocketStoriesService( + context = testContext, + pocketStoriesConfig = PocketStoriesConfig( + client = mock(), + profile = null + ) + ).apply { + spocsRefreshscheduler = mock() + } + + service.deleteProfile() + + verify(service.spocsRefreshscheduler, never()).scheduleProfileDeletion(any()) + assertNull(GlobalDependencyProvider.SponsoredStories.useCases) } @Test diff --git a/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorkerTest.kt b/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorkerTest.kt new file mode 100644 index 00000000000..ab663655777 --- /dev/null +++ b/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/DeleteSpocsProfileWorkerTest.kt @@ -0,0 +1,65 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.service.pocket.update + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.work.ListenableWorker.Result +import androidx.work.await +import androidx.work.testing.TestListenableWorkerBuilder +import kotlinx.coroutines.ExperimentalCoroutinesApi +import mozilla.components.service.pocket.GlobalDependencyProvider +import mozilla.components.service.pocket.helpers.assertClassVisibility +import mozilla.components.service.pocket.spocs.SpocsUseCases +import mozilla.components.service.pocket.spocs.SpocsUseCases.DeleteProfile +import mozilla.components.support.test.mock +import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.rule.runTestOnMain +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.doReturn +import kotlin.reflect.KVisibility.INTERNAL + +@ExperimentalCoroutinesApi // for runTestOnMain +@RunWith(AndroidJUnit4::class) +class DeleteSpocsProfileWorkerTest { + @get:Rule + val mainCoroutineRule = MainCoroutineRule() + + @Test + fun `GIVEN a DeleteSpocsProfileWorker THEN its visibility is internal`() { + assertClassVisibility(RefreshSpocsWorker::class, INTERNAL) + } + + @Test + fun `GIVEN a DeleteSpocsProfileWorker WHEN profile deletion is successful THEN return success`() = runTestOnMain { + val useCases: SpocsUseCases = mock() + val deleteProfileUseCase: DeleteProfile = mock() + doReturn(true).`when`(deleteProfileUseCase).invoke() + doReturn(deleteProfileUseCase).`when`(useCases).deleteProfile + GlobalDependencyProvider.SponsoredStories.initialize(useCases) + val worker = TestListenableWorkerBuilder(testContext).build() + + val result = worker.startWork().await() + + assertEquals(Result.success(), result) + } + + @Test + fun `GIVEN a DeleteSpocsProfileWorker WHEN profile deletion fails THEN work should be retried`() = runTestOnMain { + val useCases: SpocsUseCases = mock() + val deleteProfileUseCase: DeleteProfile = mock() + doReturn(false).`when`(deleteProfileUseCase).invoke() + doReturn(deleteProfileUseCase).`when`(useCases).deleteProfile + GlobalDependencyProvider.SponsoredStories.initialize(useCases) + val worker = TestListenableWorkerBuilder(testContext).build() + + val result = worker.startWork().await() + + assertEquals(Result.retry(), result) + } +} diff --git a/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/SpocsRefreshSchedulerTest.kt b/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/SpocsRefreshSchedulerTest.kt index e1d673d0bcd..f828c1a2bef 100644 --- a/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/SpocsRefreshSchedulerTest.kt +++ b/components/service/pocket/src/test/java/mozilla/components/service/pocket/update/SpocsRefreshSchedulerTest.kt @@ -5,24 +5,30 @@ package mozilla.components.service.pocket.update import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.work.BackoffPolicy import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.ExistingWorkPolicy import androidx.work.NetworkType +import androidx.work.OneTimeWorkRequest import androidx.work.PeriodicWorkRequest import androidx.work.WorkManager import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient import mozilla.components.service.pocket.PocketStoriesConfig import mozilla.components.service.pocket.helpers.assertClassVisibility +import mozilla.components.service.pocket.update.DeleteSpocsProfileWorker.Companion.DELETE_SPOCS_PROFILE_WORK_TAG import mozilla.components.service.pocket.update.RefreshSpocsWorker.Companion.REFRESH_SPOCS_WORK_TAG import mozilla.components.support.base.worker.Frequency import mozilla.components.support.test.any import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito import org.mockito.Mockito.doReturn +import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.verify import java.util.concurrent.TimeUnit @@ -31,12 +37,12 @@ import kotlin.reflect.KVisibility @RunWith(AndroidJUnit4::class) class SpocsRefreshSchedulerTest { @Test - fun `GIVEN a SpocsRefreshScheduler THEN its visibility is internal`() { + fun `GIVEN a spocs refresh scheduler THEN its visibility is internal`() { assertClassVisibility(SpocsRefreshScheduler::class, KVisibility.INTERNAL) } @Test - fun `GIVEN a SpocsRefreshScheduler WHEN schedulePeriodicRefreshes THEN a RefreshPocketWorker is created and enqueued`() { + fun `GIVEN a spocs refresh scheduler WHEN scheduling stories refresh THEN a RefreshPocketWorker is created and enqueued`() { val client: HttpURLConnectionClient = mock() val scheduler = spy( SpocsRefreshScheduler( @@ -48,7 +54,7 @@ class SpocsRefreshSchedulerTest { val workManager = mock() val worker = mock() doReturn(workManager).`when`(scheduler).getWorkManager(any()) - doReturn(worker).`when`(scheduler).createPeriodicWorkerRequest(any()) + doReturn(worker).`when`(scheduler).createPeriodicRefreshWorkerRequest(any()) scheduler.schedulePeriodicRefreshes(testContext) @@ -56,7 +62,7 @@ class SpocsRefreshSchedulerTest { } @Test - fun `GIVEN a SpocsRefreshScheduler WHEN stopPeriodicRefreshes THEN it should cancel all unfinished work`() { + fun `GIVEN a spocs refresh scheduler WHEN stopping stories refresh THEN it should cancel all unfinished work`() { val scheduler = spy(SpocsRefreshScheduler(mock())) val workManager = mock() doReturn(workManager).`when`(scheduler).getWorkManager(any()) @@ -68,10 +74,42 @@ class SpocsRefreshSchedulerTest { } @Test - fun `GIVEN a SpocsRefreshScheduler WHEN createPeriodicWorkerRequest THEN a properly configured PeriodicWorkRequest is returned`() { + fun `GIVEN a spocs refresh scheduler WHEN scheduling profile deletion THEN a RefreshPocketWorker is created and enqueued`() { + val client: HttpURLConnectionClient = mock() + val scheduler = spy( + SpocsRefreshScheduler( + PocketStoriesConfig( + client, Frequency(1, TimeUnit.HOURS) + ) + ) + ) + val workManager = mock() + val worker = mock() + doReturn(workManager).`when`(scheduler).getWorkManager(any()) + doReturn(worker).`when`(scheduler).createOneTimeProfileDeletionWorkerRequest() + + scheduler.scheduleProfileDeletion(testContext) + + verify(workManager).enqueueUniqueWork(DELETE_SPOCS_PROFILE_WORK_TAG, ExistingWorkPolicy.KEEP, worker) + } + + @Test + fun `GIVEN a spocs refresh scheduler WHEN cancelling profile deletion THEN it should cancel all unfinished work`() { + val scheduler = spy(SpocsRefreshScheduler(mock())) + val workManager = mock() + doReturn(workManager).`when`(scheduler).getWorkManager(any()) + + scheduler.stopProfileDeletion(testContext) + + verify(workManager).cancelAllWorkByTag(DELETE_SPOCS_PROFILE_WORK_TAG) + verify(workManager, never()).cancelAllWork() + } + + @Test + fun `GIVEN a spocs refresh scheduler WHEN creating a periodic worker THEN a properly configured PeriodicWorkRequest is returned`() { val scheduler = spy(SpocsRefreshScheduler(mock())) - val result = scheduler.createPeriodicWorkerRequest( + val result = scheduler.createPeriodicRefreshWorkerRequest( Frequency(1, TimeUnit.HOURS) ) @@ -87,7 +125,26 @@ class SpocsRefreshSchedulerTest { } @Test - fun `GIVEN SpocsRefreshScheduler THEN Worker constraints should be to have Internet`() { + fun `GIVEN a spocs refresh scheduler WHEN creating a one time worker THEN a properly configured OneTimeWorkRequest is returned`() { + val scheduler = spy(SpocsRefreshScheduler(mock())) + + val result = scheduler.createOneTimeProfileDeletionWorkerRequest() + + verify(scheduler).getWorkerConstrains() + assertEquals(0, result.workSpec.intervalDuration) + assertEquals(0, result.workSpec.initialDelay) + assertEquals(BackoffPolicy.EXPONENTIAL, result.workSpec.backoffPolicy) + assertFalse(result.workSpec.constraints.requiresBatteryNotLow()) + assertFalse(result.workSpec.constraints.requiresCharging()) + assertFalse(result.workSpec.constraints.hasContentUriTriggers()) + assertFalse(result.workSpec.constraints.requiresStorageNotLow()) + assertFalse(result.workSpec.constraints.requiresDeviceIdle()) + assertTrue(result.workSpec.constraints.requiredNetworkType == NetworkType.CONNECTED) + assertTrue(result.tags.contains(DELETE_SPOCS_PROFILE_WORK_TAG)) + } + + @Test + fun `GIVEN a spocs refresh scheduler THEN Worker constraints should be to have Internet`() { val scheduler = SpocsRefreshScheduler(mock()) val result = scheduler.getWorkerConstrains() diff --git a/docs/changelog.md b/docs/changelog.md index e53933fbb20..6e71b0edc32 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,9 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml) +* **service-pocket** + * Ensure sponsored stories profile deletion is retried in background until successful or the feature is re-enabled. [#12258](https://github.com/mozilla-mobile/android-components/issues/12258) + # 102.0.0 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v101.0.0...v102.0.1) * [Milestone](https://github.com/mozilla-mobile/android-components/milestone/149?closed=1)