Introduce JsonProvider.

It will ensure that classes are using the correct Json instances in the unit tests.
This commit is contained in:
Benoit Marty 2025-10-17 17:31:15 +02:00 committed by Benoit Marty
parent fa8ddba1f5
commit df48ed5a2d
21 changed files with 103 additions and 52 deletions

View file

@ -11,6 +11,7 @@ import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.Inject
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
@ -49,10 +50,12 @@ class DefaultNotificationResolverQueue(
private val appCoroutineScope: CoroutineScope,
private val workManagerScheduler: WorkManagerScheduler,
private val featureFlagService: FeatureFlagService,
private val json: JsonProvider,
) : NotificationResolverQueue {
companion object {
private const val BATCH_WINDOW_MS = 250L
}
private val requestQueue = Channel<NotificationEventRequest>(capacity = 100)
private var currentProcessingJob: Job? = null
@ -94,7 +97,13 @@ class DefaultNotificationResolverQueue(
if (featureFlagService.isFeatureEnabled(FeatureFlags.SyncNotificationsWithWorkManager)) {
for ((sessionId, requests) in groupedRequestsById) {
workManagerScheduler.submit(SyncNotificationWorkManagerRequest(sessionId, requests))
workManagerScheduler.submit(
SyncNotificationWorkManagerRequest(
sessionId = sessionId,
notificationEventRequests = requests,
json = json,
)
)
}
} else {
val sessionIds = groupedRequestsById.keys

View file

@ -18,6 +18,7 @@ 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
@ -33,7 +34,6 @@ import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import kotlinx.serialization.json.Json
import timber.log.Timber
import kotlin.time.Duration.Companion.seconds
@ -47,13 +47,13 @@ class FetchNotificationsWorker(
private val workManagerScheduler: WorkManagerScheduler,
private val syncOnNotifiableEvent: SyncOnNotifiableEvent,
private val coroutineDispatchers: CoroutineDispatchers,
private val json: Json,
private val json: JsonProvider,
) : 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() }
json().decodeFromString<List<SyncNotificationWorkManagerRequest.Data>>(rawRequestsJson).map { it.toRequest() }
}.getOrElse {
Timber.e(it, "Failed to deserialize notification requests")
return@withContext Result.failure()
@ -93,7 +93,13 @@ class FetchNotificationsWorker(
for (failedSessionId in failedSyncForSessions) {
val requestsToRetry = groupedRequests[failedSessionId] ?: continue
Timber.d("Re-scheduling ${requestsToRetry.size} failed notification requests for session $failedSessionId")
workManagerScheduler.submit(SyncNotificationWorkManagerRequest(failedSessionId, requestsToRetry))
workManagerScheduler.submit(
SyncNotificationWorkManagerRequest(
sessionId = failedSessionId,
notificationEventRequests = requestsToRetry,
json = json,
)
)
}
}

View file

@ -11,6 +11,7 @@ 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
@ -21,22 +22,20 @@ import io.element.android.libraries.workmanager.api.WorkManagerRequestType
import io.element.android.libraries.workmanager.api.workManagerTag
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import timber.log.Timber
import java.security.InvalidParameterException
class SyncNotificationWorkManagerRequest(
private val sessionId: SessionId,
private val notificationEventRequests: List<NotificationEventRequest>,
private val json: JsonProvider,
) : WorkManagerRequest {
private val json = Json { ignoreUnknownKeys = true }
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() }) }
val json = runCatchingExceptions { json().encodeToString(notificationEventRequests.map { it.toData() }) }
.getOrElse {
Timber.e(it, "Failed to serialize notification requests")
return Result.failure(it)
@ -50,7 +49,7 @@ class SyncNotificationWorkManagerRequest(
.setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
.setTraceTag(workManagerTag(sessionId, WorkManagerRequestType.NOTIFICATION_SYNC))
// TODO investigate using this instead of the resolver queue
// .setInputMerger()
// .setInputMerger()
.build()
)
}

View file

@ -13,6 +13,7 @@ import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.call.api.CallType
import io.element.android.features.call.test.FakeElementCallEntryPoint
import io.element.android.libraries.androidutils.json.DefaultJsonProvider
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
@ -714,6 +715,7 @@ class DefaultPushHandlerTest {
appCoroutineScope = backgroundScope,
workManagerScheduler = workManagerScheduler,
featureFlagService = featureFlagService,
json = DefaultJsonProvider(),
),
appCoroutineScope = backgroundScope,
fallbackNotificationFactory = FallbackNotificationFactory(

View file

@ -18,6 +18,7 @@ import com.google.common.truth.Truth.assertThat
import com.google.common.util.concurrent.ListenableFuture
import io.element.android.features.networkmonitor.api.NetworkStatus
import io.element.android.features.networkmonitor.test.FakeNetworkMonitor
import io.element.android.libraries.androidutils.json.DefaultJsonProvider
import io.element.android.libraries.push.api.push.SyncOnNotifiableEvent
import io.element.android.libraries.push.impl.notifications.FakeNotifiableEventResolver
import io.element.android.libraries.push.impl.notifications.NotificationResolverQueue
@ -33,7 +34,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.Json
import org.junit.Test
import org.junit.runner.RunWith
import java.util.UUID
@ -175,7 +175,7 @@ class FetchNotificationWorkerTest {
workManagerScheduler = workManagerScheduler,
syncOnNotifiableEvent = syncOnNotifiableEvent,
coroutineDispatchers = testCoroutineDispatchers(),
json = Json {},
json = DefaultJsonProvider(),
)
private fun TestScope.createWorkerParams(

View file

@ -10,7 +10,10 @@ package io.element.android.libraries.push.impl.workmanager
import androidx.work.OneTimeWorkRequest
import androidx.work.hasKeyWithValueOfType
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.androidutils.json.DefaultJsonProvider
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.test.A_SESSION_ID
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
@ -20,7 +23,7 @@ import org.junit.Test
class SyncNotificationWorkManagerRequestTest {
@Test
fun `build - success`() = runTest {
val request = SyncNotificationWorkManagerRequest(
val request = createSyncNotificationWorkManagerRequest(
sessionId = A_SESSION_ID,
notificationEventRequests = listOf(aNotificationEventRequest())
)
@ -37,7 +40,7 @@ class SyncNotificationWorkManagerRequestTest {
@Test
fun `build - empty list of requests fails`() = runTest {
val request = SyncNotificationWorkManagerRequest(
val request = createSyncNotificationWorkManagerRequest(
sessionId = A_SESSION_ID,
notificationEventRequests = emptyList()
)
@ -48,3 +51,12 @@ class SyncNotificationWorkManagerRequestTest {
// TODO add test for invalid serialization (how?)
}
private fun createSyncNotificationWorkManagerRequest(
sessionId: SessionId,
notificationEventRequests: List<NotificationEventRequest>,
) = SyncNotificationWorkManagerRequest(
sessionId = sessionId,
notificationEventRequests = notificationEventRequests,
json = DefaultJsonProvider(),
)