From 84d0338ed338a5ef206552329be04a2d790c6ee0 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Tue, 28 Oct 2025 21:06:57 +0100 Subject: [PATCH] Fix issues with WorkManager on Android 12 and below (#5606) * Add `getForegroundInfo` implementation to try to fix issues with WorkManager on Android 12 and below This may be a MIUI-only issue as I couldn't reproduce it with several emulators on Android 11, 12 and 13. * Use `setExpedited` only on Android 13 or higher, it's not needed on older versions * Use an actual string resource, fix tests * Fix review comments * Fix broken test with Element Pro: Instead of using Robolectric with API < 33 (since Pro uses minSdk 33) use a `BuildVersionSdkIntProvider` * Remove `getForegroundInfo` and the associated permission, as we expect it to be dead code * Fix lint issues * Cleanup NotificationIdProvider --------- Co-authored-by: Benoit Marty --- .../notifications/NotificationIdProvider.kt | 5 ---- .../push/impl/src/main/AndroidManifest.xml | 1 - .../NotificationResolverQueue.kt | 3 ++ .../workmanager/FetchNotificationsWorker.kt | 5 +++- .../SyncNotificationWorkManagerRequest.kt | 12 +++++++- .../push/impl/push/DefaultPushHandlerTest.kt | 2 ++ .../FetchNotificationWorkerTest.kt | 2 ++ .../SyncNotificationWorkManagerRequestTest.kt | 30 +++++++++++++++++-- 8 files changed, 49 insertions(+), 11 deletions(-) diff --git a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt index 83ab37682b..390932bb79 100644 --- a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt @@ -31,10 +31,6 @@ object NotificationIdProvider { return getOffset(sessionId) + FALLBACK_NOTIFICATION_ID } - fun getCallNotificationId(sessionId: SessionId): Int { - return getOffset(sessionId) + ROOM_CALL_NOTIFICATION_ID - } - fun getForegroundServiceNotificationId(type: ForegroundServiceType): Int { return type.id * 10 + FOREGROUND_SERVICE_NOTIFICATION_ID } @@ -49,7 +45,6 @@ object NotificationIdProvider { private const val ROOM_MESSAGES_NOTIFICATION_ID = 1 private const val ROOM_EVENT_NOTIFICATION_ID = 2 private const val ROOM_INVITATION_NOTIFICATION_ID = 3 - private const val ROOM_CALL_NOTIFICATION_ID = 3 private const val FOREGROUND_SERVICE_NOTIFICATION_ID = 4 } diff --git a/libraries/push/impl/src/main/AndroidManifest.xml b/libraries/push/impl/src/main/AndroidManifest.xml index c08c16ed5f..ecdb3faaa2 100644 --- a/libraries/push/impl/src/main/AndroidManifest.xml +++ b/libraries/push/impl/src/main/AndroidManifest.xml @@ -7,7 +7,6 @@ - diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt index bbdd241c24..156c925202 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -18,6 +18,7 @@ import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEv import io.element.android.libraries.push.impl.workmanager.SyncNotificationWorkManagerRequest import io.element.android.libraries.push.impl.workmanager.WorkerDataConverter import io.element.android.libraries.workmanager.api.WorkManagerScheduler +import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job @@ -49,6 +50,7 @@ class DefaultNotificationResolverQueue( private val workManagerScheduler: WorkManagerScheduler, private val featureFlagService: FeatureFlagService, private val workerDataConverter: WorkerDataConverter, + private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, ) : NotificationResolverQueue { companion object { private const val BATCH_WINDOW_MS = 250L @@ -100,6 +102,7 @@ class DefaultNotificationResolverQueue( sessionId = sessionId, notificationEventRequests = requests, workerDataConverter = workerDataConverter, + buildVersionSdkIntProvider = buildVersionSdkIntProvider, ) ) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationsWorker.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationsWorker.kt index 6904b2f833..b661560cf5 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationsWorker.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationsWorker.kt @@ -29,6 +29,7 @@ import io.element.android.libraries.push.impl.notifications.NotificationResolver import io.element.android.libraries.workmanager.api.WorkManagerScheduler import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory import io.element.android.libraries.workmanager.api.di.WorkerKey +import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext @@ -39,7 +40,7 @@ import kotlin.time.Duration.Companion.seconds @AssistedInject class FetchNotificationsWorker( @Assisted workerParams: WorkerParameters, - @ApplicationContext context: Context, + @ApplicationContext private val context: Context, private val networkMonitor: NetworkMonitor, private val eventResolver: NotifiableEventResolver, private val queue: NotificationResolverQueue, @@ -47,6 +48,7 @@ class FetchNotificationsWorker( private val syncOnNotifiableEvent: SyncOnNotifiableEvent, private val coroutineDispatchers: CoroutineDispatchers, private val workerDataConverter: WorkerDataConverter, + private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, ) : CoroutineWorker(context, workerParams) { override suspend fun doWork(): Result = withContext(coroutineDispatchers.io) { Timber.d("FetchNotificationsWorker started") @@ -88,6 +90,7 @@ class FetchNotificationsWorker( sessionId = failedSessionId, notificationEventRequests = requestsToRetry, workerDataConverter = workerDataConverter, + buildVersionSdkIntProvider = buildVersionSdkIntProvider, ) ) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequest.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequest.kt index b0aabe1cc7..f11aabe469 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequest.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequest.kt @@ -7,6 +7,7 @@ package io.element.android.libraries.push.impl.workmanager +import android.os.Build import androidx.work.OneTimeWorkRequestBuilder import androidx.work.OutOfQuotaPolicy import androidx.work.WorkRequest @@ -15,6 +16,7 @@ import io.element.android.libraries.push.api.push.NotificationEventRequest import io.element.android.libraries.workmanager.api.WorkManagerRequest import io.element.android.libraries.workmanager.api.WorkManagerRequestType import io.element.android.libraries.workmanager.api.workManagerTag +import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import timber.log.Timber @@ -24,6 +26,7 @@ class SyncNotificationWorkManagerRequest( private val sessionId: SessionId, private val notificationEventRequests: List, private val workerDataConverter: WorkerDataConverter, + private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, ) : WorkManagerRequest { override fun build(): Result { if (notificationEventRequests.isEmpty()) { @@ -36,7 +39,14 @@ class SyncNotificationWorkManagerRequest( return Result.success( OneTimeWorkRequestBuilder() .setInputData(data) - .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) + .apply { + // Expedited workers aren't needed on Android 12 or lower: + // They force displaying a foreground sync notification for no good reason, since they sync almost immediately anyway + // See https://developer.android.com/develop/background-work/background-tasks/persistent/getting-started/define-work#backwards-compat + if (buildVersionSdkIntProvider.isAtLeast(Build.VERSION_CODES.TIRAMISU)) { + setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) + } + } .setTraceTag(workManagerTag(sessionId, WorkManagerRequestType.NOTIFICATION_SYNC)) // TODO investigate using this instead of the resolver queue // .setInputMerger() diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt index 870b9e1e1a..4576051be6 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt @@ -55,6 +55,7 @@ import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushSto import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.FakePushClientSecret import io.element.android.libraries.workmanager.api.WorkManagerRequest import io.element.android.libraries.workmanager.test.FakeWorkManagerScheduler +import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider import io.element.android.services.toolbox.test.strings.FakeStringProvider import io.element.android.services.toolbox.test.systemclock.FakeSystemClock import io.element.android.tests.testutils.lambda.any @@ -717,6 +718,7 @@ class DefaultPushHandlerTest { workManagerScheduler = workManagerScheduler, featureFlagService = featureFlagService, workerDataConverter = WorkerDataConverter(DefaultJsonProvider()), + buildVersionSdkIntProvider = FakeBuildVersionSdkIntProvider(33), ), appCoroutineScope = backgroundScope, fallbackNotificationFactory = FallbackNotificationFactory( diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationWorkerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationWorkerTest.kt index 74241fecd8..c20cfbe61e 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationWorkerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationWorkerTest.kt @@ -28,6 +28,7 @@ import io.element.android.libraries.push.test.notifications.FakeNotificationReso import io.element.android.libraries.workmanager.api.WorkManagerRequest import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory import io.element.android.libraries.workmanager.test.FakeWorkManagerScheduler +import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -176,6 +177,7 @@ class FetchNotificationWorkerTest { syncOnNotifiableEvent = syncOnNotifiableEvent, coroutineDispatchers = testCoroutineDispatchers(), workerDataConverter = WorkerDataConverter(DefaultJsonProvider()), + buildVersionSdkIntProvider = FakeBuildVersionSdkIntProvider(33), ) private fun TestScope.createWorkerParams( diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequestTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequestTest.kt index ebbe4eb865..34738854ac 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequestTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequestTest.kt @@ -17,15 +17,17 @@ import io.element.android.libraries.push.api.push.NotificationEventRequest import io.element.android.libraries.push.impl.notifications.fixtures.aNotificationEventRequest import io.element.android.libraries.workmanager.api.WorkManagerRequestType import io.element.android.libraries.workmanager.api.workManagerTag +import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider import kotlinx.coroutines.test.runTest import org.junit.Test class SyncNotificationWorkManagerRequestTest { @Test - fun `build - success`() = runTest { + fun `build - success API 33`() = runTest { val request = createSyncNotificationWorkManagerRequest( sessionId = A_SESSION_ID, - notificationEventRequests = listOf(aNotificationEventRequest()) + notificationEventRequests = listOf(aNotificationEventRequest()), + sdkVersion = 33, ) val result = request.build() @@ -33,11 +35,31 @@ class SyncNotificationWorkManagerRequestTest { result.getOrNull()!!.run { assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java) assertThat(workSpec.input.hasKeyWithValueOfType("requests")).isTrue() + // True in API 33+ assertThat(workSpec.expedited).isTrue() assertThat(workSpec.traceTag).isEqualTo(workManagerTag(A_SESSION_ID, WorkManagerRequestType.NOTIFICATION_SYNC)) } } + @Test + fun `build - success API 32 and lower`() = runTest { + val request = createSyncNotificationWorkManagerRequest( + sessionId = A_SESSION_ID, + notificationEventRequests = listOf(aNotificationEventRequest()), + sdkVersion = 32, + ) + + val result = request.build() + assertThat(result.isSuccess).isTrue() + result.getOrNull()!!.run { + assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java) + assertThat(workSpec.input.hasKeyWithValueOfType("requests")).isTrue() + // False before API 33 + assertThat(workSpec.expedited).isFalse() + assertThat(workSpec.traceTag).isEqualTo(workManagerTag(A_SESSION_ID, WorkManagerRequestType.NOTIFICATION_SYNC)) + } + } + @Test fun `build - empty list of requests fails`() = runTest { val request = createSyncNotificationWorkManagerRequest( @@ -64,9 +86,11 @@ class SyncNotificationWorkManagerRequestTest { private fun createSyncNotificationWorkManagerRequest( sessionId: SessionId, notificationEventRequests: List, - workerDataConverter: WorkerDataConverter = WorkerDataConverter(DefaultJsonProvider()) + workerDataConverter: WorkerDataConverter = WorkerDataConverter(DefaultJsonProvider()), + sdkVersion: Int = 33, ) = SyncNotificationWorkManagerRequest( sessionId = sessionId, notificationEventRequests = notificationEventRequests, workerDataConverter = workerDataConverter, + buildVersionSdkIntProvider = FakeBuildVersionSdkIntProvider(sdkVersion), )