letro/letro-android
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

Merge pull request #5768 from element-hq/feature/bma/fixCrashInWorkManager

Browse Source 
Fix crash in work manager
This commit is contained in:
Benoit Marty
2025-11-20 15:26:49 +01:00
committed by GitHub
parent 8718bd5239 3c53c4d96b
commit e9b50f604e
7 changed files with 181 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/workmanager/DataForWorkManagerIsTooBig.kt Normal file
View File

@@ -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()

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

@@ -29,30 +29,29 @@ class SyncNotificationWorkManagerRequest(
private val workerDataConverter: WorkerDataConverter,
private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider,
) : WorkManagerRequest {
override fun build(): Result<WorkRequest> {
override fun build(): Result<List<WorkRequest>> {
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<FetchNotificationsWorker>()
.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<FetchNotificationsWorker>()
.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

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

@@ -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<NotificationEventRequest>): Result<Data> {
fun serialize(notificationEventRequests: List<NotificationEventRequest>): Result<List<Data>> {
// 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<NotificationEventRequest>()
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<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)
.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
}
}

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

@@ -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<String>("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<String>("requests")).isTrue()
// False before API 33

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

@@ -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<NotificationEventRequest>) {
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)
}
}

2
libraries/workmanager/api/src/main/kotlin/io/element/android/libraries/workmanager/api/WorkManagerRequest.kt
View File

@@ -11,5 +11,5 @@ package io.element.android.libraries.workmanager.api
import androidx.work.WorkRequest
interface WorkManagerRequest {
fun build(): Result<WorkRequest>
fun build(): Result<List<WorkRequest>>
}

4
libraries/workmanager/impl/src/main/kotlin/io/element/android/libraries/workmanager/impl/DefaultWorkManagerScheduler.kt
View File

@@ -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")