Improve code again

This commit is contained in:
Benoit Marty 2025-11-19 17:53:12 +01:00
parent b976d9deac
commit 0355c0bda9
7 changed files with 48 additions and 35 deletions

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

View file

@ -29,9 +29,9 @@ class SyncNotificationWorkManagerRequest(
private val workerDataConverter: WorkerDataConverter, private val workerDataConverter: WorkerDataConverter,
private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider,
) : WorkManagerRequest { ) : WorkManagerRequest {
override fun build(): List<Result<WorkRequest>> { override fun build(): Result<List<WorkRequest>> {
if (notificationEventRequests.isEmpty()) { if (notificationEventRequests.isEmpty()) {
return listOf(Result.failure(InvalidParameterException("notificationEventRequests cannot be empty"))) return Result.failure(InvalidParameterException("notificationEventRequests cannot be empty"))
} }
Timber.d("Scheduling ${notificationEventRequests.size} notification requests with WorkManager for $sessionId") Timber.d("Scheduling ${notificationEventRequests.size} notification requests with WorkManager for $sessionId")
return workerDataConverter.serialize(notificationEventRequests).map { return workerDataConverter.serialize(notificationEventRequests).map {

View file

@ -24,21 +24,22 @@ import timber.log.Timber
class WorkerDataConverter( class WorkerDataConverter(
private val json: JsonProvider, private val json: JsonProvider,
) { ) {
fun serialize(notificationEventRequests: List<NotificationEventRequest>): List<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. // First try to serialize all requests at once. In the vast majority of cases this will work.
return serializeRequests(notificationEventRequests) return serializeRequests(notificationEventRequests)
.fold( .map { listOf(it) }
onSuccess = { .recoverCatching {
listOf(Result.success(it)) if (it is DataForWorkManagerIsTooBig) {
}, // Perform serialization on sublists, workDataOf have failed because of size limit
onFailure = {
// Perform serialization on sublists, workDataOf may have failed because of size limit
Timber.w(it, "Failed to serialize ${notificationEventRequests.size} notification requests, trying with chunks of $CHUNK_SIZE.") Timber.w(it, "Failed to serialize ${notificationEventRequests.size} notification requests, trying with chunks of $CHUNK_SIZE.")
notificationEventRequests.chunked(CHUNK_SIZE).map { chunk -> // TODO Do not split rooms
serializeRequests(chunk) notificationEventRequests.chunked(CHUNK_SIZE).mapNotNull { chunk ->
serializeRequests(chunk).getOrNull()
} }
}, } else {
) throw it
}
}
} }
private fun serializeRequests(notificationEventRequests: List<NotificationEventRequest>): Result<Data> { private fun serializeRequests(notificationEventRequests: List<NotificationEventRequest>): Result<Data> {
@ -48,7 +49,11 @@ class WorkerDataConverter(
} }
.mapCatchingExceptions { str -> .mapCatchingExceptions { str ->
// Note: workDataOf can fail if the data is too large // Note: workDataOf can fail if the data is too large
workDataOf(REQUESTS_KEY to str) try {
workDataOf(REQUESTS_KEY to str)
} catch (_: IllegalStateException) {
throw DataForWorkManagerIsTooBig()
}
} }
} }

View file

@ -33,8 +33,8 @@ class SyncNotificationWorkManagerRequestTest {
) )
val result = request.build() val result = request.build()
assertThat(result.first().isSuccess).isTrue() assertThat(result.isSuccess).isTrue()
result.first().getOrNull()!!.run { result.getOrNull()!!.first().run {
assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java) assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java)
assertThat(workSpec.input.hasKeyWithValueOfType<String>("requests")).isTrue() assertThat(workSpec.input.hasKeyWithValueOfType<String>("requests")).isTrue()
// True in API 33+ // True in API 33+
@ -52,8 +52,8 @@ class SyncNotificationWorkManagerRequestTest {
) )
val result = request.build() val result = request.build()
assertThat(result.first().isSuccess).isTrue() assertThat(result.isSuccess).isTrue()
result.first().getOrNull()!!.run { result.getOrNull()!!.first().run {
assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java) assertThat(this).isInstanceOf(OneTimeWorkRequest::class.java)
assertThat(workSpec.input.hasKeyWithValueOfType<String>("requests")).isTrue() assertThat(workSpec.input.hasKeyWithValueOfType<String>("requests")).isTrue()
// False before API 33 // False before API 33
@ -70,7 +70,7 @@ class SyncNotificationWorkManagerRequestTest {
) )
val result = request.build() val result = request.build()
assertThat(result.first().isFailure).isTrue() assertThat(result.isFailure).isTrue()
} }
@Test @Test
@ -81,7 +81,7 @@ class SyncNotificationWorkManagerRequestTest {
workerDataConverter = WorkerDataConverter({ error("error during serialization") }) workerDataConverter = WorkerDataConverter({ error("error during serialization") })
) )
val result = request.build() val result = request.build()
assertThat(result.first().isFailure).isTrue() assertThat(result.isFailure).isTrue()
} }
} }

View file

@ -58,14 +58,14 @@ class WorkerDataConverterTest {
} }
val sut = WorkerDataConverter(DefaultJsonProvider()) val sut = WorkerDataConverter(DefaultJsonProvider())
val serialized = sut.serialize(data) val serialized = sut.serialize(data)
assertThat(serialized.size).isGreaterThan(1) assertThat(serialized.getOrNull()?.size).isGreaterThan(1)
assertThat(serialized.size).isEqualTo(100 / WorkerDataConverter.CHUNK_SIZE) assertThat(serialized.getOrNull()?.size).isEqualTo(100 / WorkerDataConverter.CHUNK_SIZE)
} }
private fun testIdentity(data: List<NotificationEventRequest>) { private fun testIdentity(data: List<NotificationEventRequest>) {
val sut = WorkerDataConverter(DefaultJsonProvider()) val sut = WorkerDataConverter(DefaultJsonProvider())
val serialized = sut.serialize(data).first().getOrThrow() val serialized = sut.serialize(data).getOrThrow()
val result = sut.deserialize(serialized) val result = sut.deserialize(serialized.first())
assertThat(result).isEqualTo(data) assertThat(result).isEqualTo(data)
} }
} }

View file

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

View file

@ -27,16 +27,14 @@ class DefaultWorkManagerScheduler(
private val workManager by lazy { WorkManager.getInstance(context) } private val workManager by lazy { WorkManager.getInstance(context) }
override fun submit(workManagerRequest: WorkManagerRequest) { override fun submit(workManagerRequest: WorkManagerRequest) {
workManagerRequest.build().forEach { workManagerRequest.build().fold(
it.fold( onSuccess = { workRequests ->
onSuccess = { workRequest -> workManager.enqueue(workRequests)
workManager.enqueue(workRequest) },
}, onFailure = {
onFailure = { Timber.e(it, "Failed to build WorkManager request $workManagerRequest")
Timber.e(it, "Failed to build WorkManager request $workManagerRequest") }
} )
)
}
} }
override fun cancel(sessionId: SessionId) { override fun cancel(sessionId: SessionId) {