diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/DataForWorkManagerIsTooBig.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/DataForWorkManagerIsTooBig.kt new file mode 100644 index 0000000000..e700a58a7e --- /dev/null +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/DataForWorkManagerIsTooBig.kt @@ -0,0 +1,10 @@ +/* + * Copyright (c) 2025 Element Creations 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 + +class DataForWorkManagerIsTooBig : Exception() 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 9492977c3d..b11b83d6e4 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 @@ -29,30 +29,29 @@ class SyncNotificationWorkManagerRequest( private val workerDataConverter: WorkerDataConverter, private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, ) : WorkManagerRequest { - override fun build(): Result { + override fun build(): Result> { if (notificationEventRequests.isEmpty()) { return Result.failure(InvalidParameterException("notificationEventRequests cannot be empty")) } - 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() - .setInputData(data) - .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) + return workerDataConverter.serialize(notificationEventRequests).map { dataList -> + dataList.map { data -> + OneTimeWorkRequestBuilder() + .setInputData(data) + .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() - .build() - ) + .setTraceTag(workManagerTag(sessionId, WorkManagerRequestType.NOTIFICATION_SYNC)) + // TODO investigate using this instead of the resolver queue + // .setInputMerger() + .build() + } + } } @Serializable diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverter.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverter.kt index ac51c6c6a9..23e66396c6 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverter.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverter.kt @@ -12,6 +12,7 @@ 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.mapCatchingExceptions 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 @@ -23,13 +24,67 @@ import timber.log.Timber class WorkerDataConverter( private val json: JsonProvider, ) { - fun serialize(notificationEventRequests: List): Result { + fun serialize(notificationEventRequests: List): Result> { + // First try to serialize all requests at once. In the vast majority of cases this will work. + return serializeRequests(notificationEventRequests) + .map { listOf(it) } + .recoverCatching { t -> + if (t is DataForWorkManagerIsTooBig) { + // Perform serialization on sublists, workDataOf have failed because of size limit + Timber.w(t, "Failed to serialize ${notificationEventRequests.size} notification requests, split the requests per room.") + // Group the requests per rooms + val requestsSortedPerRoom = notificationEventRequests.groupBy { it.roomId }.values + // Build a list of sublist with size at most CHUNK_SIZE, and with all rooms kept together + buildList { + val currentChunk = mutableListOf() + for (requests in requestsSortedPerRoom) { + if (currentChunk.size + requests.size <= CHUNK_SIZE) { + // Can add the whole room requests to the current chunk + currentChunk.addAll(requests) + } else { + // Add the current chunk + add(currentChunk.toList()) + // Start a new chunk with the current room requests + currentChunk.clear() + // If a room has more requests than CHUNK_SIZE, we need to split them + requests.chunked(CHUNK_SIZE) { chunk -> + if (chunk.size == CHUNK_SIZE) { + add(chunk.toList()) + } else { + currentChunk.addAll(chunk) + } + } + } + } + // Add any remaining requests + add(currentChunk.toList()) + } + .filter { it.isNotEmpty() } + .also { + Timber.d("Split notification requests into ${it.size} chunks for WorkManager serialization") + it.forEach { requests -> + Timber.d(" - Chunk with ${requests.size} requests") + } + } + .mapNotNull { serializeRequests(it).getOrNull() } + } else { + throw t + } + } + } + + private fun serializeRequests(notificationEventRequests: List): Result { return runCatchingExceptions { json().encodeToString(notificationEventRequests.map { it.toData() }) } .onFailure { Timber.e(it, "Failed to serialize notification requests") } - .map { str -> - workDataOf(REQUESTS_KEY to str) + .mapCatchingExceptions { str -> + // Note: workDataOf can fail if the data is too large + try { + workDataOf(REQUESTS_KEY to str) + } catch (_: IllegalStateException) { + throw DataForWorkManagerIsTooBig() + } } } @@ -51,6 +106,7 @@ class WorkerDataConverter( companion object { private const val REQUESTS_KEY = "requests" + internal const val CHUNK_SIZE = 20 } } 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 62acaa16b1..9dae435a9a 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 @@ -21,6 +21,7 @@ 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 +import kotlin.collections.first class SyncNotificationWorkManagerRequestTest { @Test @@ -33,7 +34,7 @@ class SyncNotificationWorkManagerRequestTest { val result = request.build() assertThat(result.isSuccess).isTrue() - result.getOrNull()!!.run { + result.getOrNull()!!.first().run { assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java) assertThat(workSpec.input.hasKeyWithValueOfType("requests")).isTrue() // True in API 33+ @@ -52,7 +53,7 @@ class SyncNotificationWorkManagerRequestTest { val result = request.build() assertThat(result.isSuccess).isTrue() - result.getOrNull()!!.run { + result.getOrNull()!!.first().run { assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java) assertThat(workSpec.input.hasKeyWithValueOfType("requests")).isTrue() // False before API 33 diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverterTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverterTest.kt index ad6999b4ea..6c6998cb35 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverterTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/workmanager/WorkerDataConverterTest.kt @@ -10,10 +10,12 @@ 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.api.core.EventId 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_ROOM_ID_3 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 @@ -45,10 +47,95 @@ class WorkerDataConverterTest { ) } + @Test + fun `serializing lots of data leads to several work data generated - one room - 100 events should be split in 5 chunks`() { + val data = List(100) { + NotificationEventRequest( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + eventId = EventId(AN_EVENT_ID.value + it), + providerInfo = "info$it", + ) + } + val sut = WorkerDataConverter(DefaultJsonProvider()) + val serialized = sut.serialize(data) + assertThat(serialized.getOrNull()?.size).isGreaterThan(1) + assertThat(serialized.getOrNull()?.size).isEqualTo(100 / WorkerDataConverter.CHUNK_SIZE) + // All the items are present + val deserialized = serialized.getOrNull()?.flatMap { sut.deserialize(it)!! } + assertThat(deserialized).containsExactlyElementsIn(data) + } + + @Test + fun `serializing lots of data leads to several work data generated - one room - 101 events should be split in 6 chunks`() { + val data = List(101) { + NotificationEventRequest( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + eventId = EventId(AN_EVENT_ID.value + it), + providerInfo = "info$it", + ) + } + val sut = WorkerDataConverter(DefaultJsonProvider()) + val serialized = sut.serialize(data) + assertThat(serialized.getOrNull()?.size).isGreaterThan(1) + assertThat(serialized.getOrNull()?.size).isEqualTo(100 / WorkerDataConverter.CHUNK_SIZE + 1) + // All the items are present + val deserialized = serialized.getOrNull()?.flatMap { sut.deserialize(it)!! } + assertThat(deserialized).containsExactlyElementsIn(data) + } + + @Test + fun `serializing lots of data leads to several work data generated - 3 rooms - 25 events should be split in 2 chunks and room not mixed`() { + val data1 = List(15) { + NotificationEventRequest( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + eventId = EventId(AN_EVENT_ID.value + it), + providerInfo = "info".repeat(100) + it, + ) + } + val data2 = List(3) { + NotificationEventRequest( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID_2, + eventId = EventId(AN_EVENT_ID.value + it), + providerInfo = "info".repeat(100) + it, + ) + } + val data3 = List(7) { + NotificationEventRequest( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID_3, + eventId = EventId(AN_EVENT_ID.value + it), + providerInfo = "info".repeat(100) + it, + ) + } + val data = (data1 + data2 + data3).shuffled() + val sut = WorkerDataConverter(DefaultJsonProvider()) + val serialized = sut.serialize(data) + assertThat(serialized.getOrNull()?.size).isEqualTo(2) + // All the items are present + val deserialized = serialized.getOrNull()?.flatMap { sut.deserialize(it)!! } + assertThat(deserialized).containsExactlyElementsIn(data) + // Rooms are not mixed between the chunks + val setsOfRooms = serialized.getOrNull()!! + .map { workData -> sut.deserialize(workData)!! } + .map { + it.map { request -> request.roomId }.toSet() + } + // Ensure that all sets are distinct + assertThat(setsOfRooms.size).isEqualTo(2) + // 3 roomId are present + assertThat(setsOfRooms.flatten().toSet()).containsExactly(A_ROOM_ID, A_ROOM_ID_2, A_ROOM_ID_3) + // No intersection between sets + assertThat(setsOfRooms[0].intersect(setsOfRooms[1])).isEmpty() + } + private fun testIdentity(data: List) { val sut = WorkerDataConverter(DefaultJsonProvider()) val serialized = sut.serialize(data).getOrThrow() - val result = sut.deserialize(serialized) + val result = sut.deserialize(serialized.first()) assertThat(result).isEqualTo(data) } } diff --git a/libraries/workmanager/api/src/main/kotlin/io/element/android/libraries/workmanager/api/WorkManagerRequest.kt b/libraries/workmanager/api/src/main/kotlin/io/element/android/libraries/workmanager/api/WorkManagerRequest.kt index 7c3b165102..af49c3dc87 100644 --- a/libraries/workmanager/api/src/main/kotlin/io/element/android/libraries/workmanager/api/WorkManagerRequest.kt +++ b/libraries/workmanager/api/src/main/kotlin/io/element/android/libraries/workmanager/api/WorkManagerRequest.kt @@ -11,5 +11,5 @@ package io.element.android.libraries.workmanager.api import androidx.work.WorkRequest interface WorkManagerRequest { - fun build(): Result + fun build(): Result> } diff --git a/libraries/workmanager/impl/src/main/kotlin/io/element/android/libraries/workmanager/impl/DefaultWorkManagerScheduler.kt b/libraries/workmanager/impl/src/main/kotlin/io/element/android/libraries/workmanager/impl/DefaultWorkManagerScheduler.kt index 70c9b719b2..6943c1432e 100644 --- a/libraries/workmanager/impl/src/main/kotlin/io/element/android/libraries/workmanager/impl/DefaultWorkManagerScheduler.kt +++ b/libraries/workmanager/impl/src/main/kotlin/io/element/android/libraries/workmanager/impl/DefaultWorkManagerScheduler.kt @@ -28,8 +28,8 @@ class DefaultWorkManagerScheduler( override fun submit(workManagerRequest: WorkManagerRequest) { workManagerRequest.build().fold( - onSuccess = { - workManager.enqueue(it) + onSuccess = { workRequests -> + workManager.enqueue(workRequests) }, onFailure = { Timber.e(it, "Failed to build WorkManager request $workManagerRequest")