mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 08:15:02 +00:00
Prevent foreground backfill from running when the app is active (#7932)
* Prevent foreground backfill from running when the app is active This should be a background task, since it can severely degrade performance if the app is in use while it runs. The worker won't run any more if the app is in foreground and will stop running if it was in background and now it's in foreground. * Fix return inside `withTimeout` Co-authored-by: Benoit Marty <benoitm@element.io> * Remove initial delay: it shouldn't be necessary if the foreground state of the app is checked when the worker starts Instead, if the app was in foreground, cancel the work and retry in 5 minutes. --------- Co-authored-by: Benoit Marty <benoitm@element.io>
This commit is contained in:
committed by
GitHub
parent
8689961f0d
commit
287001f9aa
@@ -82,6 +82,7 @@ dependencies {
|
|||||||
implementation(variantOf(libs.jna) { artifactType("aar") })
|
implementation(variantOf(libs.jna) { artifactType("aar") })
|
||||||
implementation(libs.androidx.datastore.preferences)
|
implementation(libs.androidx.datastore.preferences)
|
||||||
implementation(libs.kotlinx.collections.immutable)
|
implementation(libs.kotlinx.collections.immutable)
|
||||||
|
implementation(libs.androidx.lifecycle.process)
|
||||||
|
|
||||||
testCommonDependencies(libs)
|
testCommonDependencies(libs)
|
||||||
testImplementation(projects.features.enterprise.test)
|
testImplementation(projects.features.enterprise.test)
|
||||||
|
|||||||
+22
@@ -0,0 +1,22 @@
|
|||||||
|
/*
|
||||||
|
* Copyright (c) 2026 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.matrix.impl.di
|
||||||
|
|
||||||
|
import androidx.lifecycle.LifecycleOwner
|
||||||
|
import androidx.lifecycle.ProcessLifecycleOwner
|
||||||
|
import dev.zacsweers.metro.AppScope
|
||||||
|
import dev.zacsweers.metro.BindingContainer
|
||||||
|
import dev.zacsweers.metro.ContributesTo
|
||||||
|
import dev.zacsweers.metro.Provides
|
||||||
|
|
||||||
|
@BindingContainer
|
||||||
|
@ContributesTo(AppScope::class)
|
||||||
|
object AppMatrixBindingContainer {
|
||||||
|
@Provides
|
||||||
|
fun provideLifecycleOwner(): LifecycleOwner = ProcessLifecycleOwner.get()
|
||||||
|
}
|
||||||
+7
-3
@@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.impl.search.workmanager
|
|||||||
|
|
||||||
import android.net.NetworkCapabilities
|
import android.net.NetworkCapabilities
|
||||||
import android.net.NetworkRequest
|
import android.net.NetworkRequest
|
||||||
|
import androidx.work.BackoffPolicy
|
||||||
import androidx.work.Constraints
|
import androidx.work.Constraints
|
||||||
import androidx.work.NetworkType
|
import androidx.work.NetworkType
|
||||||
import androidx.work.PeriodicWorkRequestBuilder
|
import androidx.work.PeriodicWorkRequestBuilder
|
||||||
@@ -28,8 +29,8 @@ import io.element.android.libraries.workmanager.api.WorkManagerWorkerType
|
|||||||
import io.element.android.libraries.workmanager.api.workManagerTag
|
import io.element.android.libraries.workmanager.api.workManagerTag
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import timber.log.Timber
|
import timber.log.Timber
|
||||||
|
import java.util.concurrent.TimeUnit
|
||||||
import kotlin.time.Duration.Companion.hours
|
import kotlin.time.Duration.Companion.hours
|
||||||
import kotlin.time.Duration.Companion.minutes
|
|
||||||
import kotlin.time.toJavaDuration
|
import kotlin.time.toJavaDuration
|
||||||
|
|
||||||
interface SearchBackfillRequestBuilder : WorkManagerRequestBuilder {
|
interface SearchBackfillRequestBuilder : WorkManagerRequestBuilder {
|
||||||
@@ -76,11 +77,14 @@ class DefaultSearchBackfillRequestBuilder(
|
|||||||
.build()
|
.build()
|
||||||
|
|
||||||
val request = PeriodicWorkRequestBuilder<SearchBackfillWorker>(1.hours.toJavaDuration())
|
val request = PeriodicWorkRequestBuilder<SearchBackfillWorker>(1.hours.toJavaDuration())
|
||||||
// Add a delay before the first execution to avoid running the backfill immediately after the app starts
|
|
||||||
.setInitialDelay(5.minutes.toJavaDuration())
|
|
||||||
.setInputData(workDataOf(SESSION_ID_PARAM to sessionId.value))
|
.setInputData(workDataOf(SESSION_ID_PARAM to sessionId.value))
|
||||||
.addTag(tag)
|
.addTag(tag)
|
||||||
.setConstraints(constraints)
|
.setConstraints(constraints)
|
||||||
|
.setBackoffCriteria(
|
||||||
|
backoffPolicy = BackoffPolicy.LINEAR,
|
||||||
|
backoffDelay = 5,
|
||||||
|
timeUnit = TimeUnit.MINUTES,
|
||||||
|
)
|
||||||
.build()
|
.build()
|
||||||
|
|
||||||
return Result.success(listOf(WorkManagerRequestWrapper(request, type)))
|
return Result.success(listOf(WorkManagerRequestWrapper(request, type)))
|
||||||
|
|||||||
+17
-1
@@ -8,6 +8,9 @@
|
|||||||
package io.element.android.libraries.matrix.impl.search.workmanager
|
package io.element.android.libraries.matrix.impl.search.workmanager
|
||||||
|
|
||||||
import android.content.Context
|
import android.content.Context
|
||||||
|
import androidx.annotation.VisibleForTesting
|
||||||
|
import androidx.lifecycle.Lifecycle
|
||||||
|
import androidx.lifecycle.LifecycleOwner
|
||||||
import androidx.work.CoroutineWorker
|
import androidx.work.CoroutineWorker
|
||||||
import androidx.work.WorkerParameters
|
import androidx.work.WorkerParameters
|
||||||
import dev.zacsweers.metro.AppScope
|
import dev.zacsweers.metro.AppScope
|
||||||
@@ -34,14 +37,22 @@ class SearchBackfillWorker(
|
|||||||
@Assisted params: WorkerParameters,
|
@Assisted params: WorkerParameters,
|
||||||
@ApplicationContext context: Context,
|
@ApplicationContext context: Context,
|
||||||
private val matrixClientProvider: MatrixClientProvider,
|
private val matrixClientProvider: MatrixClientProvider,
|
||||||
|
private val processLifecycleOwner: LifecycleOwner,
|
||||||
) : CoroutineWorker(context, params) {
|
) : CoroutineWorker(context, params) {
|
||||||
companion object {
|
companion object {
|
||||||
const val SESSION_ID_PARAM = "session_id"
|
const val SESSION_ID_PARAM = "session_id"
|
||||||
private val TIMEOUT = 10.minutes
|
private val TIMEOUT = 10.minutes
|
||||||
private val POLL_INTERVAL = 5.seconds
|
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
|
||||||
|
internal val POLL_INTERVAL = 5.seconds
|
||||||
}
|
}
|
||||||
|
|
||||||
override suspend fun doWork(): Result {
|
override suspend fun doWork(): Result {
|
||||||
|
val appLifecycle = processLifecycleOwner.lifecycle
|
||||||
|
if (appLifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) {
|
||||||
|
Timber.w("App is in foreground, skipping search backfill as it would degrade performance")
|
||||||
|
return Result.retry()
|
||||||
|
}
|
||||||
|
|
||||||
Timber.d("Starting search backfill. This will run up to $TIMEOUT")
|
Timber.d("Starting search backfill. This will run up to $TIMEOUT")
|
||||||
val sessionId = inputData.getString(SESSION_ID_PARAM)?.let(::SessionId) ?: return Result.failure()
|
val sessionId = inputData.getString(SESSION_ID_PARAM)?.let(::SessionId) ?: return Result.failure()
|
||||||
val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return Result.failure()
|
val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return Result.failure()
|
||||||
@@ -62,6 +73,11 @@ class SearchBackfillWorker(
|
|||||||
withTimeout(TIMEOUT) {
|
withTimeout(TIMEOUT) {
|
||||||
while (task.isRunning()) {
|
while (task.isRunning()) {
|
||||||
delay(POLL_INTERVAL)
|
delay(POLL_INTERVAL)
|
||||||
|
|
||||||
|
if (task.isRunning() && appLifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) {
|
||||||
|
Timber.w("App is now in foreground, cancelling search backfill as it would degrade performance")
|
||||||
|
return@withTimeout
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Timber.d("Search backfill finished")
|
Timber.d("Search backfill finished")
|
||||||
|
|||||||
+3
-1
@@ -8,6 +8,7 @@
|
|||||||
package io.element.android.libraries.matrix.impl.search.workmanager
|
package io.element.android.libraries.matrix.impl.search.workmanager
|
||||||
|
|
||||||
import android.net.NetworkCapabilities
|
import android.net.NetworkCapabilities
|
||||||
|
import androidx.work.BackoffPolicy
|
||||||
import androidx.work.PeriodicWorkRequest
|
import androidx.work.PeriodicWorkRequest
|
||||||
import androidx.work.hasKeyWithValueOfType
|
import androidx.work.hasKeyWithValueOfType
|
||||||
import com.google.common.truth.Truth.assertThat
|
import com.google.common.truth.Truth.assertThat
|
||||||
@@ -34,9 +35,10 @@ class DefaultSearchBackfillRequestBuilderTest : RobolectricTest() {
|
|||||||
assertThat(this).isInstanceOf(PeriodicWorkRequest::class.java)
|
assertThat(this).isInstanceOf(PeriodicWorkRequest::class.java)
|
||||||
assertThat(workSpec.isPeriodic).isTrue()
|
assertThat(workSpec.isPeriodic).isTrue()
|
||||||
assertThat(workSpec.intervalDuration).isEqualTo(1.hours.inWholeMilliseconds)
|
assertThat(workSpec.intervalDuration).isEqualTo(1.hours.inWholeMilliseconds)
|
||||||
assertThat(workSpec.initialDelay).isEqualTo(5.minutes.inWholeMilliseconds)
|
|
||||||
assertThat(workSpec.input.hasKeyWithValueOfType<String>(SearchBackfillWorker.SESSION_ID_PARAM)).isTrue()
|
assertThat(workSpec.input.hasKeyWithValueOfType<String>(SearchBackfillWorker.SESSION_ID_PARAM)).isTrue()
|
||||||
assertThat(workSpec.input.getString(SearchBackfillWorker.SESSION_ID_PARAM)).isEqualTo(A_SESSION_ID.value)
|
assertThat(workSpec.input.getString(SearchBackfillWorker.SESSION_ID_PARAM)).isEqualTo(A_SESSION_ID.value)
|
||||||
|
assertThat(workSpec.backoffPolicy).isEqualTo(BackoffPolicy.LINEAR)
|
||||||
|
assertThat(workSpec.backoffDelayDuration).isEqualTo(5.minutes.inWholeMilliseconds)
|
||||||
assertThat(tags).contains(expectedTag)
|
assertThat(tags).contains(expectedTag)
|
||||||
assertThat(workSpec.workerClassName).isEqualTo(SearchBackfillWorker::class.java.name)
|
assertThat(workSpec.workerClassName).isEqualTo(SearchBackfillWorker::class.java.name)
|
||||||
}
|
}
|
||||||
|
|||||||
+59
@@ -7,6 +7,7 @@
|
|||||||
|
|
||||||
package io.element.android.libraries.matrix.impl.search.workmanager
|
package io.element.android.libraries.matrix.impl.search.workmanager
|
||||||
|
|
||||||
|
import androidx.lifecycle.Lifecycle
|
||||||
import androidx.test.platform.app.InstrumentationRegistry
|
import androidx.test.platform.app.InstrumentationRegistry
|
||||||
import androidx.work.Data
|
import androidx.work.Data
|
||||||
import androidx.work.ListenableWorker
|
import androidx.work.ListenableWorker
|
||||||
@@ -24,6 +25,7 @@ import io.element.android.libraries.matrix.test.FakeMatrixClientProvider
|
|||||||
import io.element.android.libraries.matrix.test.FakeSdkPendingTask
|
import io.element.android.libraries.matrix.test.FakeSdkPendingTask
|
||||||
import io.element.android.libraries.matrix.test.search.FakeSearchBackfillService
|
import io.element.android.libraries.matrix.test.search.FakeSearchBackfillService
|
||||||
import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory
|
import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory
|
||||||
|
import io.element.android.tests.testutils.FakeLifecycleOwner
|
||||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||||
import io.element.android.tests.testutils.lambda.value
|
import io.element.android.tests.testutils.lambda.value
|
||||||
import io.element.android.tests.testutils.robolectric.RobolectricTest
|
import io.element.android.tests.testutils.robolectric.RobolectricTest
|
||||||
@@ -37,6 +39,7 @@ import java.util.UUID
|
|||||||
import java.util.concurrent.Executor
|
import java.util.concurrent.Executor
|
||||||
import java.util.concurrent.Executors
|
import java.util.concurrent.Executors
|
||||||
import java.util.concurrent.TimeUnit
|
import java.util.concurrent.TimeUnit
|
||||||
|
import kotlin.time.Duration.Companion.milliseconds
|
||||||
import kotlin.time.Duration.Companion.minutes
|
import kotlin.time.Duration.Companion.minutes
|
||||||
import kotlin.time.Duration.Companion.seconds
|
import kotlin.time.Duration.Companion.seconds
|
||||||
|
|
||||||
@@ -120,9 +123,64 @@ class SearchBackfillWorkerTest : RobolectricTest() {
|
|||||||
closeLambda.assertions().isCalledOnce()
|
closeLambda.assertions().isCalledOnce()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `backfill will not run if the app is in foreground`() = runTest {
|
||||||
|
val startSearchBackfillLambda = lambdaRecorder<SearchBackfillStrategy, Result<SdkPendingTask>> {
|
||||||
|
Result.success(FakeSdkPendingTask(isRunningLambda = { true }))
|
||||||
|
}
|
||||||
|
val client = FakeMatrixClient(
|
||||||
|
searchBackfillService = FakeSearchBackfillService(
|
||||||
|
startSearchBackfillLambda = startSearchBackfillLambda,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
val lifecycleOwner = FakeLifecycleOwner(initialState = Lifecycle.State.RESUMED)
|
||||||
|
val deferred = async {
|
||||||
|
createWorker(client = client, lifecycleOwner = lifecycleOwner).doWork()
|
||||||
|
}
|
||||||
|
|
||||||
|
// Advance time to allow the worker to check the lifecycle state.
|
||||||
|
advanceTimeBy(1.milliseconds)
|
||||||
|
assertThat(deferred.isCompleted).isTrue()
|
||||||
|
|
||||||
|
// The worker should return 'retry' immediately without starting the backfill.
|
||||||
|
assertThat(deferred.await()).isEqualTo(ListenableWorker.Result.retry())
|
||||||
|
startSearchBackfillLambda.assertions().isNeverCalled()
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `backfill will stop if the app comes to foreground while it's running`() = runTest {
|
||||||
|
val closeLambda = lambdaRecorder<Unit> { }
|
||||||
|
val startSearchBackfillLambda = lambdaRecorder<SearchBackfillStrategy, Result<SdkPendingTask>> {
|
||||||
|
Result.success(FakeSdkPendingTask(isRunningLambda = { true }, closeLambda = closeLambda))
|
||||||
|
}
|
||||||
|
val client = FakeMatrixClient(
|
||||||
|
searchBackfillService = FakeSearchBackfillService(
|
||||||
|
startSearchBackfillLambda = startSearchBackfillLambda,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
val lifecycleOwner = FakeLifecycleOwner(initialState = Lifecycle.State.CREATED)
|
||||||
|
val deferred = async {
|
||||||
|
createWorker(client = client, lifecycleOwner = lifecycleOwner).doWork()
|
||||||
|
}
|
||||||
|
advanceTimeBy(1.seconds)
|
||||||
|
assertThat(deferred.isCompleted).isFalse()
|
||||||
|
|
||||||
|
lifecycleOwner.givenState(Lifecycle.State.RESUMED)
|
||||||
|
|
||||||
|
advanceTimeBy(SearchBackfillWorker.POLL_INTERVAL)
|
||||||
|
|
||||||
|
assertThat(deferred.isCompleted).isTrue()
|
||||||
|
assertThat(deferred.await()).isEqualTo(ListenableWorker.Result.success())
|
||||||
|
startSearchBackfillLambda.assertions().isCalledOnce()
|
||||||
|
closeLambda.assertions().isCalledOnce()
|
||||||
|
}
|
||||||
|
|
||||||
private fun TestScope.createWorker(
|
private fun TestScope.createWorker(
|
||||||
input: String? = A_SESSION_ID.value,
|
input: String? = A_SESSION_ID.value,
|
||||||
client: MatrixClient?,
|
client: MatrixClient?,
|
||||||
|
lifecycleOwner: FakeLifecycleOwner = FakeLifecycleOwner(),
|
||||||
) = SearchBackfillWorker(
|
) = SearchBackfillWorker(
|
||||||
params = createWorkerParams(
|
params = createWorkerParams(
|
||||||
inputData = input?.let { workDataOf(SearchBackfillWorker.SESSION_ID_PARAM to it) } ?: Data.EMPTY,
|
inputData = input?.let { workDataOf(SearchBackfillWorker.SESSION_ID_PARAM to it) } ?: Data.EMPTY,
|
||||||
@@ -131,6 +189,7 @@ class SearchBackfillWorkerTest : RobolectricTest() {
|
|||||||
matrixClientProvider = FakeMatrixClientProvider(
|
matrixClientProvider = FakeMatrixClientProvider(
|
||||||
getClient = { client?.let { Result.success(it) } ?: Result.failure(IllegalStateException("No client")) }
|
getClient = { client?.let { Result.success(it) } ?: Result.failure(IllegalStateException("No client")) }
|
||||||
),
|
),
|
||||||
|
processLifecycleOwner = lifecycleOwner,
|
||||||
)
|
)
|
||||||
|
|
||||||
private fun TestScope.createWorkerParams(
|
private fun TestScope.createWorkerParams(
|
||||||
|
|||||||
Reference in New Issue
Block a user