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 <benoit@matrix.org>

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
 }

libraries/push/impl/src/main/AndroidManifest.xml

@@ -7,7 +7,6 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android">
 
     <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
-
     <uses-permission android:name="android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS" />
 
     <application>

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,
                         )
                     )
                 }

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,
                     )
                 )
             }

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<NotificationEventRequest>,
     private val workerDataConverter: WorkerDataConverter,
+    private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider,
 ) : WorkManagerRequest {
     override fun build(): Result<WorkRequest> {
         if (notificationEventRequests.isEmpty()) {
@@ -36,7 +39,14 @@ class SyncNotificationWorkManagerRequest(
         return Result.success(
             OneTimeWorkRequestBuilder<FetchNotificationsWorker>()
                 .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()

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(

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(

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<String>("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<String>("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<NotificationEventRequest>,
-    workerDataConverter: WorkerDataConverter = WorkerDataConverter(DefaultJsonProvider())
+    workerDataConverter: WorkerDataConverter = WorkerDataConverter(DefaultJsonProvider()),
+    sdkVersion: Int = 33,
 ) = SyncNotificationWorkManagerRequest(
     sessionId = sessionId,
     notificationEventRequests = notificationEventRequests,
     workerDataConverter = workerDataConverter,
+    buildVersionSdkIntProvider = FakeBuildVersionSdkIntProvider(sdkVersion),
 )