From 4aea88877a5136cf1d33cd3c8f2e23219d69c650 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Fri, 12 Dec 2025 13:02:17 -0600 Subject: [PATCH] refactor(concurrent): Introduce SequentialJob to manage service setup (#3983) Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com> --- app/build.gradle.kts | 1 + .../com/geeksville/mesh/MeshServiceClient.kt | 19 ++--- .../mesh/concurrent/SequentialJob.kt | 51 ++++++++++++ .../mesh/concurrent/SequentialJobTest.kt | 81 +++++++++++++++++++ gradle/libs.versions.toml | 1 + 5 files changed, 141 insertions(+), 12 deletions(-) create mode 100644 app/src/main/java/com/geeksville/mesh/concurrent/SequentialJob.kt create mode 100644 app/src/test/java/com/geeksville/mesh/concurrent/SequentialJobTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index d376b4e18..a32cb45b9 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -254,6 +254,7 @@ dependencies { androidTestImplementation(libs.hilt.android.testing) testImplementation(libs.junit) + testImplementation(libs.kotlinx.coroutines.test) dokkaPlugin(libs.dokka.android.documentation.plugin) } diff --git a/app/src/main/java/com/geeksville/mesh/MeshServiceClient.kt b/app/src/main/java/com/geeksville/mesh/MeshServiceClient.kt index 8bced3f24..f57e487ce 100644 --- a/app/src/main/java/com/geeksville/mesh/MeshServiceClient.kt +++ b/app/src/main/java/com/geeksville/mesh/MeshServiceClient.kt @@ -25,12 +25,11 @@ import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.lifecycleScope import com.geeksville.mesh.android.BindFailedException import com.geeksville.mesh.android.ServiceClient -import com.geeksville.mesh.concurrent.handledLaunch +import com.geeksville.mesh.concurrent.SequentialJob import com.geeksville.mesh.service.MeshService import com.geeksville.mesh.service.startService import dagger.hilt.android.qualifiers.ActivityContext import dagger.hilt.android.scopes.ActivityScoped -import kotlinx.coroutines.Job import org.meshtastic.core.service.IMeshService import org.meshtastic.core.service.ServiceRepository import timber.log.Timber @@ -43,14 +42,12 @@ class MeshServiceClient constructor( @ActivityContext private val context: Context, private val serviceRepository: ServiceRepository, + private val serviceSetupJob: SequentialJob, ) : ServiceClient(IMeshService.Stub::asInterface), DefaultLifecycleObserver { private val lifecycleOwner: LifecycleOwner = context as LifecycleOwner - // TODO Inject this for ease of testing - private var serviceSetupJob: Job? = null - init { Timber.d("Adding self as LifecycleObserver for $lifecycleOwner") lifecycleOwner.lifecycle.addObserver(this) @@ -59,16 +56,14 @@ constructor( // region ServiceClient overrides override fun onConnected(service: IMeshService) { - serviceSetupJob?.cancel() - serviceSetupJob = - lifecycleOwner.lifecycleScope.handledLaunch { - serviceRepository.setMeshService(service) - Timber.d("connected to mesh service, connectionState=${serviceRepository.connectionState.value}") - } + serviceSetupJob.launch(lifecycleOwner.lifecycleScope) { + serviceRepository.setMeshService(service) + Timber.d("connected to mesh service, connectionState=${serviceRepository.connectionState.value}") + } } override fun onDisconnected() { - serviceSetupJob?.cancel() + serviceSetupJob.cancel() serviceRepository.setMeshService(null) } diff --git a/app/src/main/java/com/geeksville/mesh/concurrent/SequentialJob.kt b/app/src/main/java/com/geeksville/mesh/concurrent/SequentialJob.kt new file mode 100644 index 000000000..8678c9ea3 --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/concurrent/SequentialJob.kt @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2025 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.geeksville.mesh.concurrent + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import java.util.concurrent.atomic.AtomicReference +import javax.inject.Inject + +/** + * A helper class that manages a single [Job]. + * + * When a new job is launched, the previous one is cancelled. This is useful for ensuring that only one operation of a + * certain type is running at a time. + */ +class SequentialJob @Inject constructor() { + private val job = AtomicReference(null) + + /** + * Cancels the previous job (if any) and launches a new one in the given [scope]. + * + * The new job uses [handledLaunch] to ensure exceptions are reported. + */ + fun launch(scope: CoroutineScope, block: suspend CoroutineScope.() -> Unit) { + cancel() + val newJob = scope.handledLaunch(block = block) + job.set(newJob) + + newJob.invokeOnCompletion { job.compareAndSet(newJob, null) } + } + + /** Cancels the current job. */ + fun cancel() { + job.getAndSet(null)?.cancel() + } +} diff --git a/app/src/test/java/com/geeksville/mesh/concurrent/SequentialJobTest.kt b/app/src/test/java/com/geeksville/mesh/concurrent/SequentialJobTest.kt new file mode 100644 index 000000000..fc4a4dec2 --- /dev/null +++ b/app/src/test/java/com/geeksville/mesh/concurrent/SequentialJobTest.kt @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2025 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.geeksville.mesh.concurrent + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertTrue +import org.junit.Test + +@ExperimentalCoroutinesApi +class SequentialJobTest { + + private val sequentialJob = SequentialJob() + + @Test + fun `launch cancels previous job`() = runTest { + var job1Active = false + var job1Cancelled = false + + // Launch first job + sequentialJob.launch(this) { + try { + job1Active = true + delay(1000) + } finally { + job1Cancelled = true + } + } + + advanceTimeBy(100) + assertTrue("Job 1 should be active", job1Active) + + // Launch second job + sequentialJob.launch(this) { + // Do nothing + } + + advanceTimeBy(100) + assertTrue("Job 1 should be cancelled", job1Cancelled) + } + + @Test + fun `cancel stops the job`() = runTest { + var jobActive = false + var jobCancelled = false + + sequentialJob.launch(this) { + try { + jobActive = true + delay(1000) + } finally { + jobCancelled = true + } + } + + advanceTimeBy(100) + assertTrue("Job should be active", jobActive) + + sequentialJob.cancel() + + advanceTimeBy(100) + assertTrue("Job should be cancelled", jobCancelled) + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index f1dfbb50b..132a729e0 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -111,6 +111,7 @@ truth = { module = "com.google.truth:truth", version = "1.4.5" } kotlin-gradlePlugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" } kotlinx-collections-immutable = { module = "org.jetbrains.kotlinx:kotlinx-collections-immutable", version = "0.4.0" } kotlinx-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines-android" } +kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinx-coroutines-android" } kotlinx-serialization-core = { module = "org.jetbrains.kotlinx:kotlinx-serialization-core", version.ref = "kotlinx-serialization" } kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinx-serialization" }