From 94da5af0da31b8940676cef3786ba841b92ec428 Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Wed, 25 Feb 2026 19:54:15 +0530 Subject: [PATCH 1/3] fix(sync): always cancel notifications --- .../com/ichi2/anki/worker/SyncMediaWorker.kt | 14 +++++++------- .../java/com/ichi2/anki/worker/SyncWorker.kt | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt index 4624c60c64d6..8ca31b375e91 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt @@ -62,7 +62,7 @@ class SyncMediaWorker( override suspend fun doWork(): Result { Timber.v("SyncMediaWorker::doWork") - try { + return try { val auth = syncAuth { hkey = inputData.getString(HKEY_KEY)!! @@ -82,6 +82,8 @@ class SyncMediaWorker( trySetForeground(getForegroundInfo()) monitorProgress(backend) } + Timber.d("SyncMediaWorker: success") + Result.success() } catch (cancellationException: CancellationException) { Timber.w(cancellationException) cancelMediaSync(CollectionManager.getBackend()) @@ -94,13 +96,11 @@ class SyncMediaWorker( setContentText(message) } } - return Result.failure() + Result.failure() + } finally { + Timber.d("SyncMediaWorker: cancelling notification") + notificationManager?.cancel(NotificationId.SYNC_MEDIA) } - Timber.d("SyncMediaWorker: cancelling notification") - notificationManager?.cancel(NotificationId.SYNC_MEDIA) - - Timber.d("SyncMediaWorker: success") - return Result.success() } private suspend fun monitorProgress(backend: Backend) { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt index b832f346c8ad..bc6d796daa9f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt @@ -96,8 +96,11 @@ class SyncWorker( } val shouldSyncMedia = inputData.getBoolean(SYNC_MEDIA_KEY, false) - try { + return try { syncCollection(auth, shouldSyncMedia) + Timber.d("SyncWorker: success") + setLastSyncTimeToNow() + Result.success() } catch (cancellationException: CancellationException) { cancelSync(CollectionManager.getBackend()) throw cancellationException @@ -109,14 +112,11 @@ class SyncWorker( setContentText(message) } } - return Result.failure() + Result.failure() + } finally { + Timber.d("SyncWorker: cancelling notification") + notificationManager?.cancel(NotificationId.SYNC) } - Timber.d("SyncWorker: cancelling notification") - notificationManager?.cancel(NotificationId.SYNC) - - Timber.d("SyncWorker: success") - setLastSyncTimeToNow() - return Result.success() } private suspend fun syncCollection( From 3adb4960b2adefb2e8c50bb0069db8c529101285 Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Sat, 28 Feb 2026 01:08:14 +0530 Subject: [PATCH 2/3] test: ensure media worker cancels notification on error --- .../ichi2/anki/worker/SyncMediaWorkerTest.kt | 63 ++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt index 09c06b254bf3..6a94d0e1f53a 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt +++ b/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(targetContext) + .setInputData(inputData) + .build() + + worker.doWork() + + verify { notificationManager.cancel(NotificationId.SYNC_MEDIA) } + } } From 01f02c0d23309ab064123b4182bc092a2557c287 Mon Sep 17 00:00:00 2001 From: Sumit Singh Date: Mon, 2 Mar 2026 22:16:40 +0530 Subject: [PATCH 3/3] Cancel sync notifications on worker cancellation. --- .../com/ichi2/anki/worker/SyncMediaWorker.kt | 15 ++--- .../java/com/ichi2/anki/worker/SyncWorker.kt | 18 +++--- .../ichi2/anki/worker/SyncMediaWorkerTest.kt | 63 +------------------ 3 files changed, 19 insertions(+), 77 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt index 8ca31b375e91..b3c121ec2bc4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncMediaWorker.kt @@ -62,7 +62,7 @@ class SyncMediaWorker( override suspend fun doWork(): Result { Timber.v("SyncMediaWorker::doWork") - return try { + try { val auth = syncAuth { hkey = inputData.getString(HKEY_KEY)!! @@ -82,10 +82,9 @@ class SyncMediaWorker( trySetForeground(getForegroundInfo()) monitorProgress(backend) } - Timber.d("SyncMediaWorker: success") - Result.success() } catch (cancellationException: CancellationException) { Timber.w(cancellationException) + notificationManager?.cancel(NotificationId.SYNC_MEDIA) cancelMediaSync(CollectionManager.getBackend()) throw cancellationException } catch (throwable: Throwable) { @@ -96,11 +95,13 @@ class SyncMediaWorker( setContentText(message) } } - Result.failure() - } finally { - Timber.d("SyncMediaWorker: cancelling notification") - notificationManager?.cancel(NotificationId.SYNC_MEDIA) + return Result.failure() } + Timber.d("SyncMediaWorker: cancelling notification") + notificationManager?.cancel(NotificationId.SYNC_MEDIA) + + Timber.d("SyncMediaWorker: success") + return Result.success() } private suspend fun monitorProgress(backend: Backend) { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt index bc6d796daa9f..f9518c6b059e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/worker/SyncWorker.kt @@ -96,12 +96,11 @@ class SyncWorker( } val shouldSyncMedia = inputData.getBoolean(SYNC_MEDIA_KEY, false) - return try { + try { syncCollection(auth, shouldSyncMedia) - Timber.d("SyncWorker: success") - setLastSyncTimeToNow() - Result.success() } catch (cancellationException: CancellationException) { + Timber.w(cancellationException) + notificationManager?.cancel(NotificationId.SYNC) cancelSync(CollectionManager.getBackend()) throw cancellationException } catch (throwable: Throwable) { @@ -112,11 +111,14 @@ class SyncWorker( setContentText(message) } } - Result.failure() - } finally { - Timber.d("SyncWorker: cancelling notification") - notificationManager?.cancel(NotificationId.SYNC) + return Result.failure() } + Timber.d("SyncWorker: cancelling notification") + notificationManager?.cancel(NotificationId.SYNC) + + Timber.d("SyncWorker: success") + setLastSyncTimeToNow() + return Result.success() } private suspend fun syncCollection( diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt index 6a94d0e1f53a..09c06b254bf3 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/worker/SyncMediaWorkerTest.kt @@ -15,74 +15,13 @@ */ 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(targetContext) - .setInputData(inputData) - .build() - - worker.doWork() - - verify { notificationManager.cancel(NotificationId.SYNC_MEDIA) } - } }