diff --git a/feature/setup/src/main/java/com/simprints/feature/setup/SetupModule.kt b/feature/setup/src/main/java/com/simprints/feature/setup/SetupModule.kt index eda16a45d4..29d667c6fd 100644 --- a/feature/setup/src/main/java/com/simprints/feature/setup/SetupModule.kt +++ b/feature/setup/src/main/java/com/simprints/feature/setup/SetupModule.kt @@ -1,6 +1,6 @@ package com.simprints.feature.setup -import com.simprints.feature.setup.location.LocationStoreWorkerScheduler +import com.simprints.feature.setup.location.LocationStoreImpl import dagger.Binds import dagger.Module import dagger.hilt.InstallIn @@ -10,5 +10,5 @@ import dagger.hilt.components.SingletonComponent @InstallIn(SingletonComponent::class) abstract class SetupModule { @Binds - internal abstract fun provideLocationStore(authManager: LocationStoreWorkerScheduler): LocationStore + internal abstract fun provideLocationStore(locationStoreImpl: LocationStoreImpl): LocationStore } diff --git a/feature/setup/src/main/java/com/simprints/feature/setup/location/CollectLocationUseCase.kt b/feature/setup/src/main/java/com/simprints/feature/setup/location/CollectLocationUseCase.kt new file mode 100644 index 0000000000..edcf982226 --- /dev/null +++ b/feature/setup/src/main/java/com/simprints/feature/setup/location/CollectLocationUseCase.kt @@ -0,0 +1,48 @@ +package com.simprints.feature.setup.location + +import com.simprints.infra.events.event.domain.models.scope.Location +import com.simprints.infra.logging.Simber +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.flow.filterNotNull +import javax.inject.Inject + +internal class CollectLocationUseCase @Inject constructor( + private val locationManager: LocationManager, + private val updateSessionScopeLocationUseCase: UpdateSessionScopeLocationUseCase, +) { + /** + * Runs directly in the caller's coroutine context/scope so that cancelling the caller + * (e.g. cancelling its scope's children) cancels this collection too, without needing to + * track and cancel a [kotlinx.coroutines.Job] manually. + */ + suspend operator fun invoke() { + val requestStartTimeMs = System.currentTimeMillis() + Simber.i("Started collecting location", tag = TAG) + try { + locationManager + .requestLocation() + .filterNotNull() + .collect { location -> runCatching { saveUserLocation(location, requestStartTimeMs) } } + Simber.d("Finished collecting location (took ${elapsedMs(requestStartTimeMs)}ms)", tag = TAG) + } catch (c: CancellationException) { + Simber.d("Stopped collecting location (took ${elapsedMs(requestStartTimeMs)}ms)", tag = TAG) + throw c + } catch (t: Throwable) { + Simber.e("Failed to collect location", t, tag = TAG) + } + } + + private suspend fun saveUserLocation( + location: Location, + requestStartTimeMs: Long, + ) { + updateSessionScopeLocationUseCase(location) + Simber.d("Saved user's location into the current session (took ${elapsedMs(requestStartTimeMs)}ms)", tag = TAG) + } + + private fun elapsedMs(requestStartTimeMs: Long) = System.currentTimeMillis() - requestStartTimeMs + + private companion object { + private const val TAG = "CollectLocationUseCase" + } +} diff --git a/feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreImpl.kt b/feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreImpl.kt new file mode 100644 index 0000000000..a9d588f866 --- /dev/null +++ b/feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreImpl.kt @@ -0,0 +1,29 @@ +package com.simprints.feature.setup.location + +import com.simprints.core.AppScope +import com.simprints.feature.setup.LocationStore +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancelChildren +import kotlinx.coroutines.launch +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +internal class LocationStoreImpl @Inject constructor( + @AppScope appScope: CoroutineScope, + private val collectLocation: CollectLocationUseCase, +) : LocationStore { + // A child scope of the app scope so that collection coroutines can be cancelled + private val scope = CoroutineScope(appScope.coroutineContext + SupervisorJob(appScope.coroutineContext[Job])) + + override fun collectLocationInBackground() { + scope.coroutineContext.cancelChildren() + scope.launch { collectLocation() } + } + + override fun cancelLocationCollection() { + scope.coroutineContext.cancelChildren() + } +} diff --git a/feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreWorkerScheduler.kt b/feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreWorkerScheduler.kt deleted file mode 100644 index bf0216da5d..0000000000 --- a/feature/setup/src/main/java/com/simprints/feature/setup/location/LocationStoreWorkerScheduler.kt +++ /dev/null @@ -1,28 +0,0 @@ -package com.simprints.feature.setup.location - -import android.content.Context -import androidx.work.OneTimeWorkRequest -import androidx.work.WorkManager -import com.simprints.feature.setup.LocationStore -import dagger.hilt.android.qualifiers.ApplicationContext -import javax.inject.Inject - -internal class LocationStoreWorkerScheduler @Inject constructor( - @param:ApplicationContext private val appContext: Context, -) : LocationStore { - override fun collectLocationInBackground() { - val request = OneTimeWorkRequest - .Builder(StoreUserLocationIntoCurrentSessionWorker::class.java) - .addTag(STORE_USER_LOCATION_WORKER_TAG) - .build() - WorkManager.getInstance(appContext).enqueue(request) - } - - override fun cancelLocationCollection() { - WorkManager.getInstance(appContext).cancelAllWorkByTag(STORE_USER_LOCATION_WORKER_TAG) - } - - companion object { - private const val STORE_USER_LOCATION_WORKER_TAG = "StoreUserLocationWorkerTag" - } -} diff --git a/feature/setup/src/main/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorker.kt b/feature/setup/src/main/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorker.kt deleted file mode 100644 index f93065750e..0000000000 --- a/feature/setup/src/main/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorker.kt +++ /dev/null @@ -1,50 +0,0 @@ -package com.simprints.feature.setup.location - -import android.content.Context -import androidx.hilt.work.HiltWorker -import androidx.work.WorkerParameters -import com.simprints.core.DispatcherMain -import com.simprints.core.workers.SimCoroutineWorker -import com.simprints.infra.events.event.domain.models.scope.Location -import com.simprints.infra.logging.Simber -import dagger.assisted.Assisted -import dagger.assisted.AssistedInject -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.flow.filterNotNull -import kotlinx.coroutines.withContext - -/** - * Worker that collects user's last known location and save it into current session - */ -@HiltWorker -internal class StoreUserLocationIntoCurrentSessionWorker @AssistedInject constructor( - @Assisted context: Context, - @Assisted params: WorkerParameters, - private val updateSessionScopeLocationUseCase: UpdateSessionScopeLocationUseCase, - private val locationManager: LocationManager, - @param:DispatcherMain private val dispatcher: CoroutineDispatcher, -) : SimCoroutineWorker(context, params) { - override val tag: String = "StoreUserLocationWorker" - - override suspend fun doWork(): Result = withContext(dispatcher) { - showProgressNotification() - crashlyticsLog("Started") - try { - locationManager - .requestLocation() - .filterNotNull() - .collect { location -> runCatching { saveUserLocation(location) } } - } catch (t: Throwable) { - fail(t) - } - success() - } - - private suspend fun saveUserLocation(location: Location) { - if (!isStopped) { - // Only store location if SID didn't yet sent the response to the calling app - updateSessionScopeLocationUseCase(location) - Simber.d("Saving user's location into the current session", tag = tag) - } - } -} diff --git a/feature/setup/src/test/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorkerTest.kt b/feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt similarity index 51% rename from feature/setup/src/test/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorkerTest.kt rename to feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt index 95fdfced5f..f59af41c86 100644 --- a/feature/setup/src/test/java/com/simprints/feature/setup/location/StoreUserLocationIntoCurrentSessionWorkerTest.kt +++ b/feature/setup/src/test/java/com/simprints/feature/setup/location/CollectLocationUseCaseTest.kt @@ -1,9 +1,11 @@ package com.simprints.feature.setup.location -import android.os.PowerManager import com.simprints.infra.events.event.domain.models.scope.Location import com.simprints.testtools.common.coroutines.TestCoroutineRule -import io.mockk.* +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every import io.mockk.impl.annotations.MockK import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest @@ -11,7 +13,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test -internal class StoreUserLocationIntoCurrentSessionWorkerTest { +internal class CollectLocationUseCaseTest { @get:Rule val testCoroutineRule = TestCoroutineRule() @@ -21,53 +23,43 @@ internal class StoreUserLocationIntoCurrentSessionWorkerTest { @MockK private lateinit var updateSessionScopeLocationUseCase: UpdateSessionScopeLocationUseCase - private lateinit var worker: StoreUserLocationIntoCurrentSessionWorker + private lateinit var collectLocation: CollectLocationUseCase @Before fun setUp() { MockKAnnotations.init(this, relaxed = true) - worker = StoreUserLocationIntoCurrentSessionWorker( - mockk(relaxed = true) { - every { getSystemService(any()) } returns mockk { - every { isIgnoringBatteryOptimizations(any()) } returns true - } - }, - mockk(relaxed = true), - updateSessionScopeLocationUseCase, - locationManager, - testCoroutineRule.testCoroutineDispatcher, + collectLocation = CollectLocationUseCase( + locationManager = locationManager, + updateSessionScopeLocationUseCase = updateSessionScopeLocationUseCase, ) } @Test - fun storeUserLocationIntoCurrentSession() = runTest { + fun `invoke saves location into current session`() = runTest { every { locationManager.requestLocation() } returns flowOf(Location(latitude = 23.0, longitude = 54.0)) - worker.doWork() + + collectLocation() + coVerify(exactly = 1) { updateSessionScopeLocationUseCase.invoke(any()) } } @Test - fun `storeUserLocationIntoCurrentSession requestLocation throw exception`() = runTest { + fun `invoke requestLocation throws exception does not crash`() = runTest { every { locationManager.requestLocation() } throws Exception("Location collect exception") - worker.doWork() + + collectLocation() + coVerify(exactly = 0) { updateSessionScopeLocationUseCase.invoke(any()) } } @Test(expected = Test.None::class) - fun `storeUserLocationIntoCurrentSession can't save event should not crash the app`() = runTest { + fun `invoke can't save event should not crash the app`() = runTest { every { locationManager.requestLocation() } returns flowOf(Location(latitude = 23.0, longitude = 54.0)) coEvery { updateSessionScopeLocationUseCase.invoke(any()) } throws Exception("No session capture event found") - worker.doWork() - } - @Test - fun `storeUserLocationIntoCurrentSession can't save events if the worker is canceled`() = runTest { - every { locationManager.requestLocation() } returns flowOf(Location(latitude = 23.0, longitude = 54.0)) - worker.stop(0) - worker.doWork() - coVerify(exactly = 0) { updateSessionScopeLocationUseCase.invoke(any()) } + collectLocation() } } diff --git a/feature/setup/src/test/java/com/simprints/feature/setup/location/LocationStoreImplTest.kt b/feature/setup/src/test/java/com/simprints/feature/setup/location/LocationStoreImplTest.kt new file mode 100644 index 0000000000..355856914c --- /dev/null +++ b/feature/setup/src/test/java/com/simprints/feature/setup/location/LocationStoreImplTest.kt @@ -0,0 +1,79 @@ +package com.simprints.feature.setup.location + +import com.google.common.truth.Truth.assertThat +import com.simprints.testtools.common.coroutines.TestCoroutineRule +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.awaitCancellation +import org.junit.Before +import org.junit.Rule +import org.junit.Test + +internal class LocationStoreImplTest { + @get:Rule + val testCoroutineRule = TestCoroutineRule() + + @MockK + private lateinit var collectLocation: CollectLocationUseCase + + private lateinit var appScope: CoroutineScope + private lateinit var locationStore: LocationStoreImpl + + @Before + fun setUp() { + MockKAnnotations.init(this, relaxed = true) + appScope = CoroutineScope(testCoroutineRule.testCoroutineDispatcher + Job()) + locationStore = LocationStoreImpl(appScope, collectLocation) + } + + @Test + fun `collectLocationInBackground starts a new collection`() { + coEvery { collectLocation() } returns Unit + + locationStore.collectLocationInBackground() + + coVerify(exactly = 1) { collectLocation() } + } + + @Test + fun `collectLocationInBackground cancels a previous collection before starting a new one`() { + val firstCollectionCancelled = CompletableDeferred() + coEvery { collectLocation() } coAnswers { + try { + awaitCancellation() + } catch (c: CancellationException) { + firstCollectionCancelled.complete(Unit) + throw c + } + } + + locationStore.collectLocationInBackground() + locationStore.collectLocationInBackground() + + assertThat(firstCollectionCancelled.isCompleted).isTrue() + } + + @Test + fun `cancelLocationCollection cancels the current collection`() { + val collectionCancelled = CompletableDeferred() + coEvery { collectLocation() } coAnswers { + try { + awaitCancellation() + } catch (c: CancellationException) { + collectionCancelled.complete(Unit) + throw c + } + } + + locationStore.collectLocationInBackground() + locationStore.cancelLocationCollection() + + assertThat(collectionCancelled.isCompleted).isTrue() + } +}