Merge pull request #5592 from element-hq/feature/bma/constValKey

Improve how data is handled for the WorkManager.

libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/json/JsonProvider.kt

@@ -16,7 +16,7 @@ import kotlinx.serialization.json.Json
 /**
  * Provides a Json instance configured to ignore unknown keys.
  */
-interface JsonProvider : Provider<Json>
+fun interface JsonProvider : Provider<Json>
 
 @ContributesBinding(AppScope::class)
 @SingleIn(AppScope::class)

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt

@@ -10,13 +10,13 @@ package io.element.android.libraries.push.impl.notifications
 import dev.zacsweers.metro.AppScope
 import dev.zacsweers.metro.ContributesBinding
 import dev.zacsweers.metro.SingleIn
-import io.element.android.libraries.androidutils.json.JsonProvider
 import io.element.android.libraries.di.annotations.AppCoroutineScope
 import io.element.android.libraries.featureflag.api.FeatureFlagService
 import io.element.android.libraries.featureflag.api.FeatureFlags
 import io.element.android.libraries.push.api.push.NotificationEventRequest
 import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEvent
 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 kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -48,7 +48,7 @@ class DefaultNotificationResolverQueue(
     private val appCoroutineScope: CoroutineScope,
     private val workManagerScheduler: WorkManagerScheduler,
     private val featureFlagService: FeatureFlagService,
-    private val json: JsonProvider,
+    private val workerDataConverter: WorkerDataConverter,
 ) : NotificationResolverQueue {
     companion object {
         private const val BATCH_WINDOW_MS = 250L
@@ -99,7 +99,7 @@ class DefaultNotificationResolverQueue(
                         SyncNotificationWorkManagerRequest(
                             sessionId = sessionId,
                             notificationEventRequests = requests,
-                            json = json,
+                            workerDataConverter = workerDataConverter,
                         )
                     )
                 }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationsWorker.kt

@@ -18,7 +18,6 @@ import dev.zacsweers.metro.ContributesIntoMap
 import dev.zacsweers.metro.binding
 import io.element.android.features.networkmonitor.api.NetworkMonitor
 import io.element.android.features.networkmonitor.api.NetworkStatus
-import io.element.android.libraries.androidutils.json.JsonProvider
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.core.extensions.runCatchingExceptions
 import io.element.android.libraries.di.annotations.ApplicationContext
@@ -47,20 +46,11 @@ class FetchNotificationsWorker(
     private val workManagerScheduler: WorkManagerScheduler,
     private val syncOnNotifiableEvent: SyncOnNotifiableEvent,
     private val coroutineDispatchers: CoroutineDispatchers,
-    private val json: JsonProvider,
+    private val workerDataConverter: WorkerDataConverter,
 ) : CoroutineWorker(context, workerParams) {
     override suspend fun doWork(): Result = withContext(coroutineDispatchers.io) {
         Timber.d("FetchNotificationsWorker started")
-        val rawRequestsJson = inputData.getString("requests") ?: return@withContext Result.failure()
-        val requests = runCatchingExceptions {
-            json().decodeFromString<List<SyncNotificationWorkManagerRequest.Data>>(rawRequestsJson).map { it.toRequest() }
-        }.getOrElse {
-            Timber.e(it, "Failed to deserialize notification requests")
-            return@withContext Result.failure()
-        }
-
-        Timber.d("Deserialized ${requests.size} requests")
-
+        val requests = workerDataConverter.deserialize(inputData) ?: return@withContext Result.failure()
         // Wait for network to be available, but not more than 10 seconds
         val hasNetwork = withTimeoutOrNull(10.seconds) {
             networkMonitor.connectivity.first { it == NetworkStatus.Connected }
@@ -97,7 +87,7 @@ class FetchNotificationsWorker(
                     SyncNotificationWorkManagerRequest(
                         sessionId = failedSessionId,
                         notificationEventRequests = requestsToRetry,
-                        json = json,
+                        workerDataConverter = workerDataConverter,
                     )
                 )
             }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequest.kt

@@ -10,11 +10,6 @@ package io.element.android.libraries.push.impl.workmanager
 import androidx.work.OneTimeWorkRequestBuilder
 import androidx.work.OutOfQuotaPolicy
 import androidx.work.WorkRequest
-import androidx.work.workDataOf
-import io.element.android.libraries.androidutils.json.JsonProvider
-import io.element.android.libraries.core.extensions.runCatchingExceptions
-import io.element.android.libraries.matrix.api.core.EventId
-import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.push.api.push.NotificationEventRequest
 import io.element.android.libraries.workmanager.api.WorkManagerRequest
@@ -28,24 +23,19 @@ import java.security.InvalidParameterException
 class SyncNotificationWorkManagerRequest(
     private val sessionId: SessionId,
     private val notificationEventRequests: List<NotificationEventRequest>,
-    private val json: JsonProvider,
+    private val workerDataConverter: WorkerDataConverter,
 ) : WorkManagerRequest {
     override fun build(): Result<WorkRequest> {
         if (notificationEventRequests.isEmpty()) {
             return Result.failure(InvalidParameterException("notificationEventRequests cannot be empty"))
         }
-
-        val json = runCatchingExceptions { json().encodeToString(notificationEventRequests.map { it.toData() }) }
-            .getOrElse {
-                Timber.e(it, "Failed to serialize notification requests")
-                return Result.failure(it)
-            }
-
+        val data = workerDataConverter.serialize(notificationEventRequests).getOrElse {
+            return Result.failure(it)
+        }
         Timber.d("Scheduling ${notificationEventRequests.size} notification requests with WorkManager for $sessionId")
-
         return Result.success(
             OneTimeWorkRequestBuilder<FetchNotificationsWorker>()
-                .setInputData(workDataOf("requests" to json))
+                .setInputData(data)
                 .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
                 .setTraceTag(workManagerTag(sessionId, WorkManagerRequestType.NOTIFICATION_SYNC))
                 // TODO investigate using this instead of the resolver queue
@@ -64,23 +54,5 @@ class SyncNotificationWorkManagerRequest(
         val eventId: String,
         @SerialName("provider_info")
         val providerInfo: String,
-    ) {
-        fun toRequest(): NotificationEventRequest {
-            return NotificationEventRequest(
-                sessionId = SessionId(sessionId),
-                roomId = RoomId(roomId),
-                eventId = EventId(eventId),
-                providerInfo = providerInfo,
-            )
-        }
-    }
-}
-
-private fun NotificationEventRequest.toData(): SyncNotificationWorkManagerRequest.Data {
-    return SyncNotificationWorkManagerRequest.Data(
-        sessionId = sessionId.value,
-        roomId = roomId.value,
-        eventId = eventId.value,
-        providerInfo = providerInfo,
     )
 }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverter.kt

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.libraries.push.impl.workmanager
+
+import androidx.work.Data
+import androidx.work.workDataOf
+import dev.zacsweers.metro.Inject
+import io.element.android.libraries.androidutils.json.JsonProvider
+import io.element.android.libraries.core.extensions.runCatchingExceptions
+import io.element.android.libraries.matrix.api.core.EventId
+import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.SessionId
+import io.element.android.libraries.push.api.push.NotificationEventRequest
+import timber.log.Timber
+
+@Inject
+class WorkerDataConverter(
+    private val json: JsonProvider,
+) {
+    fun serialize(notificationEventRequests: List<NotificationEventRequest>): Result<Data> {
+        return runCatchingExceptions { json().encodeToString(notificationEventRequests.map { it.toData() }) }
+            .onFailure {
+                Timber.e(it, "Failed to serialize notification requests")
+            }
+            .map { str ->
+                workDataOf(REQUESTS_KEY to str)
+            }
+    }
+
+    fun deserialize(data: Data): List<NotificationEventRequest>? {
+        val rawRequestsJson = data.getString(REQUESTS_KEY) ?: return null
+        return runCatchingExceptions {
+            json().decodeFromString<List<SyncNotificationWorkManagerRequest.Data>>(rawRequestsJson).map { it.toRequest() }
+        }.fold(
+            onSuccess = {
+                Timber.d("Deserialized ${it.size} requests")
+                it
+            },
+            onFailure = {
+                Timber.e(it, "Failed to deserialize notification requests")
+                null
+            }
+        )
+    }
+
+    companion object {
+        private const val REQUESTS_KEY = "requests"
+    }
+}
+
+private fun NotificationEventRequest.toData(): SyncNotificationWorkManagerRequest.Data {
+    return SyncNotificationWorkManagerRequest.Data(
+        sessionId = sessionId.value,
+        roomId = roomId.value,
+        eventId = eventId.value,
+        providerInfo = providerInfo,
+    )
+}
+
+private fun SyncNotificationWorkManagerRequest.Data.toRequest(): NotificationEventRequest {
+    return NotificationEventRequest(
+        sessionId = SessionId(sessionId),
+        roomId = RoomId(roomId),
+        eventId = EventId(eventId),
+        providerInfo = providerInfo,
+    )
+}

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt

@@ -46,6 +46,7 @@ import io.element.android.libraries.push.impl.notifications.model.NotifiableEven
 import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEvent
 import io.element.android.libraries.push.impl.test.DefaultTestPush
 import io.element.android.libraries.push.impl.troubleshoot.DiagnosticPushHandler
+import io.element.android.libraries.push.impl.workmanager.WorkerDataConverter
 import io.element.android.libraries.pushproviders.api.PushData
 import io.element.android.libraries.pushstore.api.UserPushStore
 import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret
@@ -715,7 +716,7 @@ class DefaultPushHandlerTest {
                 appCoroutineScope = backgroundScope,
                 workManagerScheduler = workManagerScheduler,
                 featureFlagService = featureFlagService,
-                json = DefaultJsonProvider(),
+                workerDataConverter = WorkerDataConverter(DefaultJsonProvider()),
             ),
             appCoroutineScope = backgroundScope,
             fallbackNotificationFactory = FallbackNotificationFactory(

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/FetchNotificationWorkerTest.kt

@@ -175,7 +175,7 @@ class FetchNotificationWorkerTest {
         workManagerScheduler = workManagerScheduler,
         syncOnNotifiableEvent = syncOnNotifiableEvent,
         coroutineDispatchers = testCoroutineDispatchers(),
-        json = DefaultJsonProvider(),
+        workerDataConverter = WorkerDataConverter(DefaultJsonProvider()),
     )
 
     private fun TestScope.createWorkerParams(

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/SyncNotificationWorkManagerRequestTest.kt

@@ -49,14 +49,24 @@ class SyncNotificationWorkManagerRequestTest {
         assertThat(result.isFailure).isTrue()
     }
 
-    // TODO add test for invalid serialization (how?)
+    @Test
+    fun `build - invalid serialization`() = runTest {
+        val request = createSyncNotificationWorkManagerRequest(
+            sessionId = A_SESSION_ID,
+            notificationEventRequests = listOf(aNotificationEventRequest()),
+            workerDataConverter = WorkerDataConverter({ error("error during serialization") })
+        )
+        val result = request.build()
+        assertThat(result.isFailure).isTrue()
+    }
 }
 
 private fun createSyncNotificationWorkManagerRequest(
     sessionId: SessionId,
     notificationEventRequests: List<NotificationEventRequest>,
+    workerDataConverter: WorkerDataConverter = WorkerDataConverter(DefaultJsonProvider())
 ) = SyncNotificationWorkManagerRequest(
     sessionId = sessionId,
     notificationEventRequests = notificationEventRequests,
-    json = DefaultJsonProvider(),
+    workerDataConverter = workerDataConverter,
 )

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverterTest.kt

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.libraries.push.impl.workmanager
+
+import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.androidutils.json.DefaultJsonProvider
+import io.element.android.libraries.matrix.test.AN_EVENT_ID
+import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
+import io.element.android.libraries.matrix.test.A_ROOM_ID
+import io.element.android.libraries.matrix.test.A_ROOM_ID_2
+import io.element.android.libraries.matrix.test.A_SESSION_ID
+import io.element.android.libraries.matrix.test.A_SESSION_ID_2
+import io.element.android.libraries.push.api.push.NotificationEventRequest
+import org.junit.Test
+
+class WorkerDataConverterTest {
+    @Test
+    fun `ensure identity when serializing - deserializing an empty list`() {
+        testIdentity(emptyList())
+    }
+
+    @Test
+    fun `ensure identity when serializing - deserializing a list`() {
+        testIdentity(
+            listOf(
+                NotificationEventRequest(
+                    sessionId = A_SESSION_ID,
+                    roomId = A_ROOM_ID,
+                    eventId = AN_EVENT_ID,
+                    providerInfo = "info1",
+                ),
+                NotificationEventRequest(
+                    sessionId = A_SESSION_ID_2,
+                    roomId = A_ROOM_ID_2,
+                    eventId = AN_EVENT_ID_2,
+                    providerInfo = "info2",
+                ),
+            )
+        )
+    }
+
+    private fun testIdentity(data: List<NotificationEventRequest>) {
+        val sut = WorkerDataConverter(DefaultJsonProvider())
+        val serialized = sut.serialize(data).getOrThrow()
+        val result = sut.deserialize(serialized)
+        assertThat(result).isEqualTo(data)
+    }
+}