fix(sync): Cancel notifications on worker cancellation

Author: Giyutomioka-SS

AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt

@@ -83,20 +83,23 @@ class SyncMediaWorker(
                 monitorProgress(backend)
             }
         } catch (cancellationException: CancellationException) {
-            Timber.w(cancellationException)
+            Timber.w(cancellationException, "SyncMediaWorker cancelled (user tapped Cancel or WorkManager cancelled)")
+            notificationManager?.cancel(NotificationId.SYNC_MEDIA)
+            Timber.d("SyncMediaWorker: progress notification cancelled after worker cancellation")
             cancelMediaSync(CollectionManager.getBackend())
             throw cancellationException
         } catch (throwable: Throwable) {
-            Timber.w(throwable)
+            Timber.w(throwable, "SyncMediaWorker failed")
             notify {
                 setContentTitle(CollectionManager.TR.syncMediaFailed())
                 throwable.localizedMessage?.let { message ->
                     setContentText(message)
                 }
             }
+            Timber.d("SyncMediaWorker: showing failure notification")
             return Result.failure()
         }
-        Timber.d("SyncMediaWorker: cancelling notification")
+        Timber.d("SyncMediaWorker: cancelling progress notification (sync completed)")
         notificationManager?.cancel(NotificationId.SYNC_MEDIA)
 
         Timber.d("SyncMediaWorker: success")

AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt

@@ -99,19 +99,23 @@ class SyncWorker(
         try {
             syncCollection(auth, shouldSyncMedia)
         } catch (cancellationException: CancellationException) {
+            Timber.w(cancellationException, "SyncWorker cancelled (user tapped Cancel or WorkManager cancelled)")
+            notificationManager?.cancel(NotificationId.SYNC)
+            Timber.d("SyncWorker: progress notification cancelled after worker cancellation")
             cancelSync(CollectionManager.getBackend())
             throw cancellationException
         } catch (throwable: Throwable) {
-            Timber.w(throwable)
+            Timber.w(throwable, "SyncWorker failed")
             notify {
                 setContentTitle(applicationContext.getString(R.string.sync_error))
                 throwable.localizedMessage?.let { message ->
                     setContentText(message)
                 }
             }
+            Timber.d("SyncWorker: showing failure notification")
             return Result.failure()
         }
-        Timber.d("SyncWorker: cancelling notification")
+        Timber.d("SyncWorker: cancelling progress notification (sync completed)")
         notificationManager?.cancel(NotificationId.SYNC)
 
         Timber.d("SyncWorker: success")

AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt

@@ -15,13 +15,74 @@
  */
 package com.ichi2.anki.worker
 
+import androidx.core.app.NotificationManagerCompat
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.work.Data
+import androidx.work.testing.TestListenableWorkerBuilder
+import com.ichi2.anki.CollectionManager
 import com.ichi2.anki.NOTIFICATION_MIN_DELAY_MS
+import com.ichi2.anki.RobolectricTest
+import com.ichi2.anki.notifications.NotificationId
+import com.ichi2.utils.Permissions
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.mockkStatic
+import io.mockk.unmockkAll
+import io.mockk.verify
+import net.ankiweb.rsdroid.Backend
+import org.junit.After
 import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class SyncMediaWorkerTest : RobolectricTest() {
+    private lateinit var notificationManager: NotificationManagerCompat
+    private lateinit var backend: Backend
+
+    @After
+    override fun tearDown() {
+        super.tearDown()
+        unmockkAll()
+    }
 
-class SyncMediaWorkerTest {
     @Test
     @Suppress("SimplifyBooleanWithConstants")
     fun `notification update delay is not lower than min delay`() {
         assert(SyncMediaWorker.NOTIFICATION_UPDATE_RATE_MS >= NOTIFICATION_MIN_DELAY_MS)
     }
+
+    @Test
+    fun `doWork cancels notification when error happens after notification is shown`() =
+        runTest {
+            mockkObject(Permissions)
+            every { Permissions.canPostNotifications(any()) } returns true
+
+            mockkStatic(NotificationManagerCompat::class)
+            notificationManager = mockk(relaxed = true)
+            every { NotificationManagerCompat.from(any()) } returns notificationManager
+
+            mockkObject(CollectionManager)
+            backend = mockk(relaxed = true)
+            every { CollectionManager.getColUnsafe().backend } returns backend
+            every { CollectionManager.getBackend() } returns backend
+
+            // Simulate an error after the notification would be shown by throwing from mediaSyncStatus()
+            every { backend.mediaSyncStatus() } throws IllegalStateException("simulated failure after notification")
+
+            val inputData =
+                Data
+                    .Builder()
+                    .putString("hkey", "test-hkey")
+                    .build()
+
+            val worker =
+                TestListenableWorkerBuilder<SyncMediaWorker>(targetContext)
+                    .setInputData(inputData)
+                    .build()
+
+            worker.doWork()
+
+            verify { notificationManager.cancel(NotificationId.SYNC_MEDIA) }
+        }
 }