From a0975c33c78b898bb5470e0b677e597b27c3b28f Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Mon, 20 Jan 2025 12:24:19 +0530 Subject: [PATCH] Refactor out single use logic from SurveyRepository to existing use cases (#3017) * Simplify tests for MakeSurveyAvailableOfflineUseCase * Cleanup tests for removeOfflineSurvey() This is in preparation for moving loadAndSyncSurveyWithRemote() out of SurveyRepository * Move logic for fetching and saving survey to SyncSurveyUseCase * Inline subscribeToSurveyUpdates in MakeSurveyAvailableOfflineUseCase * Fix detekt warnings * Merge survey active check with ReactivateLastSurveyUseCase * Create methods for activating survey instead of directly accessing internal variable * Automatically added GitHub issue links to TODOs --- .../google/android/ground/MainViewModel.kt | 8 +- .../usecases/survey/ActivateSurveyUseCase.kt | 28 ++++--- .../MakeSurveyAvailableOfflineUseCase.kt | 21 +++-- .../survey/ReactivateLastSurveyUseCase.kt | 26 ++++--- .../usecases/survey/SyncSurveyUseCase.kt | 39 +++++++--- .../ground/repository/SurveyRepository.kt | 56 +++++--------- .../MakeSurveyAvailableOfflineUseCaseTest.kt | 46 +++++------ .../ReactivateLastSurveyUseCaseTest.kt | 70 +++++++++++------ .../domain/usecase/SyncSurveyUseCaseTest.kt | 34 ++++---- .../ground/repository/SurveyRepositoryTest.kt | 77 +++++++------------ .../ground/repository/UserRepositoryTest.kt | 6 +- .../ground/ui/home/HomeScreenFragmentTest.kt | 4 +- .../SurveySelectorFragmentTest.kt | 2 - .../ui/syncstatus/SyncStatusFragmentTest.kt | 4 +- 14 files changed, 207 insertions(+), 214 deletions(-) diff --git a/app/src/main/java/com/google/android/ground/MainViewModel.kt b/app/src/main/java/com/google/android/ground/MainViewModel.kt index 7513830d18..fa8120e46d 100644 --- a/app/src/main/java/com/google/android/ground/MainViewModel.kt +++ b/app/src/main/java/com/google/android/ground/MainViewModel.kt @@ -115,7 +115,7 @@ constructor( userRepository.saveUserDetails(user) if (!isTosAccepted()) { MainUiState.TosNotAccepted - } else if (!attemptToReactiveLastActiveSurvey()) { + } else if (!reactivateLastSurvey()) { MainUiState.NoActiveSurvey } else { // Everything is fine, show the home screen @@ -127,10 +127,4 @@ constructor( /** Returns true if the user has already accepted the Terms of Service. */ private fun isTosAccepted(): Boolean = termsOfServiceRepository.isTermsOfServiceAccepted - - /** Returns true if the last survey was successfully reactivated, if any. */ - private suspend fun attemptToReactiveLastActiveSurvey(): Boolean { - reactivateLastSurvey() - return surveyRepository.selectedSurveyId != null - } } diff --git a/app/src/main/java/com/google/android/ground/domain/usecases/survey/ActivateSurveyUseCase.kt b/app/src/main/java/com/google/android/ground/domain/usecases/survey/ActivateSurveyUseCase.kt index 45fb301128..4d55f8d282 100644 --- a/app/src/main/java/com/google/android/ground/domain/usecases/survey/ActivateSurveyUseCase.kt +++ b/app/src/main/java/com/google/android/ground/domain/usecases/survey/ActivateSurveyUseCase.kt @@ -16,32 +16,36 @@ package com.google.android.ground.domain.usecases.survey +import com.google.android.ground.persistence.local.stores.LocalSurveyStore +import com.google.android.ground.persistence.sync.SurveySyncWorker import com.google.android.ground.repository.SurveyRepository import javax.inject.Inject +/** + * Sets the survey with the specified ID as the currently active. + * + * First attempts to load the survey from the local db. If not present, fetches from remote and + * activates offline sync. Throws an error if the survey isn't found or cannot be made available + * offline. Activating a survey which is already available offline doesn't force a re-sync, since + * this is handled by [SurveySyncWorker]. + */ class ActivateSurveyUseCase @Inject constructor( - private val surveyRepository: SurveyRepository, + private val localSurveyStore: LocalSurveyStore, private val makeSurveyAvailableOffline: MakeSurveyAvailableOfflineUseCase, + private val surveyRepository: SurveyRepository, ) { - /** - * Sets the survey with the specified ID as the currently active. First attempts to load the - * survey from the local db, and if not present, fetches from remote and activates offline sync. - * Throws an error if the survey isn't found or cannot be made available offlien. Activating a - * survey which is already available offline doesn't force a resync, since this is handled by - * [com.google.android.ground.persistence.sync.SurveySyncWorker]. - */ suspend operator fun invoke(surveyId: String) { - // Do nothing if specified survey is already active. - if (surveyId == surveyRepository.activeSurvey?.id) { + if (surveyRepository.isSurveyActive(surveyId)) { + // Do nothing if survey is already active. return } - surveyRepository.getOfflineSurvey(surveyId) + localSurveyStore.getSurveyById(surveyId) ?: makeSurveyAvailableOffline(surveyId) ?: error("Survey $surveyId not found in remote db") - surveyRepository.selectedSurveyId = surveyId + surveyRepository.activateSurvey(surveyId) } } diff --git a/app/src/main/java/com/google/android/ground/domain/usecases/survey/MakeSurveyAvailableOfflineUseCase.kt b/app/src/main/java/com/google/android/ground/domain/usecases/survey/MakeSurveyAvailableOfflineUseCase.kt index f4e4cc5cfe..c0c0cf60da 100644 --- a/app/src/main/java/com/google/android/ground/domain/usecases/survey/MakeSurveyAvailableOfflineUseCase.kt +++ b/app/src/main/java/com/google/android/ground/domain/usecases/survey/MakeSurveyAvailableOfflineUseCase.kt @@ -17,23 +17,20 @@ package com.google.android.ground.domain.usecases.survey import com.google.android.ground.model.Survey -import com.google.android.ground.repository.SurveyRepository +import com.google.android.ground.persistence.remote.RemoteDataStore import javax.inject.Inject +/** + * Makes the survey with the specified ID and related LOIs available offline. Subscribes to updates + * from the remote server so that they may be re-fetched on change. Throws an error if the survey + * cannot be retrieved, or `null` if not found in the remote db. + */ class MakeSurveyAvailableOfflineUseCase @Inject constructor( - private val surveyRepository: SurveyRepository, + private val remoteDataStore: RemoteDataStore, private val syncSurvey: SyncSurveyUseCase, ) { - /** - * Makes the survey with the specified ID and related LOIs available offline. Subscribes to - * updates from the remote server so that they may be refetched on change. Throws an error if the - * survey cannot be retrieved, or `null` if not found in the remote db. - */ - suspend operator fun invoke(surveyId: String): Survey? { - val survey = syncSurvey(surveyId) ?: return null - surveyRepository.subscribeToSurveyUpdates(surveyId) - return survey - } + suspend operator fun invoke(surveyId: String): Survey? = + syncSurvey(surveyId)?.also { remoteDataStore.subscribeToSurveyUpdates(surveyId) } } diff --git a/app/src/main/java/com/google/android/ground/domain/usecases/survey/ReactivateLastSurveyUseCase.kt b/app/src/main/java/com/google/android/ground/domain/usecases/survey/ReactivateLastSurveyUseCase.kt index e945845c05..6d368e281a 100644 --- a/app/src/main/java/com/google/android/ground/domain/usecases/survey/ReactivateLastSurveyUseCase.kt +++ b/app/src/main/java/com/google/android/ground/domain/usecases/survey/ReactivateLastSurveyUseCase.kt @@ -16,24 +16,28 @@ package com.google.android.ground.domain.usecases.survey +import com.google.android.ground.persistence.local.LocalValueStore import com.google.android.ground.repository.SurveyRepository import javax.inject.Inject +/** Attempts to reactivate the last survey. If survey is already active, does nothing. */ class ReactivateLastSurveyUseCase @Inject constructor( - private val surveyRepository: SurveyRepository, private val activateSurvey: ActivateSurveyUseCase, + private val localValueStore: LocalValueStore, + private val surveyRepository: SurveyRepository, ) { - suspend operator fun invoke() { - // Do nothing if never activated. - if (surveyRepository.lastActiveSurveyId.isEmpty()) { - return - } - // Do nothing if survey is already active. - if (surveyRepository.activeSurvey != null) { - return + + suspend operator fun invoke(): Boolean = + if (getLastActiveSurveyId().isEmpty()) { + false + } else if (surveyRepository.hasActiveSurvey()) { + true + } else { + activateSurvey(getLastActiveSurveyId()) + surveyRepository.hasActiveSurvey() } - activateSurvey(surveyRepository.lastActiveSurveyId) - } + + private fun getLastActiveSurveyId(): String = localValueStore.lastActiveSurveyId } diff --git a/app/src/main/java/com/google/android/ground/domain/usecases/survey/SyncSurveyUseCase.kt b/app/src/main/java/com/google/android/ground/domain/usecases/survey/SyncSurveyUseCase.kt index 6dd71345cf..29a87b018c 100644 --- a/app/src/main/java/com/google/android/ground/domain/usecases/survey/SyncSurveyUseCase.kt +++ b/app/src/main/java/com/google/android/ground/domain/usecases/survey/SyncSurveyUseCase.kt @@ -17,26 +17,43 @@ package com.google.android.ground.domain.usecases.survey import com.google.android.ground.model.Survey +import com.google.android.ground.persistence.local.stores.LocalSurveyStore +import com.google.android.ground.persistence.remote.RemoteDataStore import com.google.android.ground.repository.LocationOfInterestRepository -import com.google.android.ground.repository.SurveyRepository import javax.inject.Inject +import kotlinx.coroutines.withTimeoutOrNull +import timber.log.Timber +private const val LOAD_REMOTE_SURVEY_TIMEOUT_MILLS: Long = 15 * 1000 + +/** + * Loads the survey with the specified id and related LOIs from remote and writes to local db. + * + * If the survey isn't found or operation times out, then we return null. Otherwise returns the + * updated [Survey]. + * + * @throws error if the remote query fails. + */ class SyncSurveyUseCase @Inject constructor( - private val surveyRepository: SurveyRepository, + private val localSurveyStore: LocalSurveyStore, private val loiRepository: LocationOfInterestRepository, + private val remoteDataStore: RemoteDataStore, ) { - /** - * Downloads the survey with the specified ID and related LOIs from remote and inserts and/or - * updates them on the local device. Returns the updated [Survey], or `null` if the survey could - * not be found. - */ - suspend operator fun invoke(surveyId: String): Survey? { - val survey = surveyRepository.loadAndSyncSurveyWithRemote(surveyId) ?: return null - loiRepository.syncLocationsOfInterest(survey) + suspend operator fun invoke(surveyId: String): Survey? = + fetchSurvey(surveyId)?.also { syncSurvey(it) } + + private suspend fun fetchSurvey(surveyId: String): Survey? = + withTimeoutOrNull(LOAD_REMOTE_SURVEY_TIMEOUT_MILLS) { + Timber.d("Loading survey $surveyId") + remoteDataStore.loadSurvey(surveyId) + } - return survey + private suspend fun syncSurvey(survey: Survey) { + localSurveyStore.insertOrUpdateSurvey(survey) + loiRepository.syncLocationsOfInterest(survey) + Timber.d("Synced survey ${survey.id}") } } diff --git a/app/src/main/java/com/google/android/ground/repository/SurveyRepository.kt b/app/src/main/java/com/google/android/ground/repository/SurveyRepository.kt index c23bdcd50c..5e8d2c4d55 100644 --- a/app/src/main/java/com/google/android/ground/repository/SurveyRepository.kt +++ b/app/src/main/java/com/google/android/ground/repository/SurveyRepository.kt @@ -20,7 +20,6 @@ import com.google.android.ground.coroutines.ApplicationScope import com.google.android.ground.model.Survey import com.google.android.ground.persistence.local.LocalValueStore import com.google.android.ground.persistence.local.stores.LocalSurveyStore -import com.google.android.ground.persistence.remote.RemoteDataStore import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.CoroutineScope @@ -36,10 +35,6 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update -import kotlinx.coroutines.withTimeoutOrNull -import timber.log.Timber - -private const val LOAD_REMOTE_SURVEY_TIMEOUT_MILLS: Long = 15 * 1000 /** * Coordinates persistence and retrieval of [Survey] instances from remote, local, and in memory @@ -53,21 +48,14 @@ class SurveyRepository constructor( private val firebaseCrashLogger: FirebaseCrashLogger, private val localSurveyStore: LocalSurveyStore, - private val remoteDataStore: RemoteDataStore, private val localValueStore: LocalValueStore, @ApplicationScope private val externalScope: CoroutineScope, ) { - private val _selectedSurveyIdFlow = MutableStateFlow(null) - var selectedSurveyId: String? - get() = _selectedSurveyIdFlow.value - set(value) { - _selectedSurveyIdFlow.update { value } - firebaseCrashLogger.setSelectedSurveyId(value) - } + private val _selectedSurveyId = MutableStateFlow(null) val activeSurveyFlow: StateFlow = - _selectedSurveyIdFlow - .flatMapLatest { id -> offlineSurvey(id) } + _selectedSurveyId + .flatMapLatest { id -> getOfflineSurveyFlow(id) } .stateIn(externalScope, SharingStarted.Lazily, null) /** The currently active survey, or `null` if no survey is active. */ @@ -75,47 +63,41 @@ constructor( get() = activeSurveyFlow.value /** The id of the last activated survey. */ - var lastActiveSurveyId: String by localValueStore::lastActiveSurveyId - internal set + private var lastActiveSurveyId: String by localValueStore::lastActiveSurveyId init { activeSurveyFlow.filterNotNull().onEach { lastActiveSurveyId = it.id }.launchIn(externalScope) } - /** Listens for remote changes to the survey with the specified id. */ - suspend fun subscribeToSurveyUpdates(surveyId: String) = - remoteDataStore.subscribeToSurveyUpdates(surveyId) - /** * Returns the survey with the specified id from the local db, or `null` if not available offline. */ suspend fun getOfflineSurvey(surveyId: String): Survey? = localSurveyStore.getSurveyById(surveyId) - private fun offlineSurvey(id: String?): Flow = - if (id == null) flowOf(null) else localSurveyStore.survey(id) + private fun getOfflineSurveyFlow(id: String?): Flow = + if (id.isNullOrBlank()) flowOf(null) else localSurveyStore.survey(id) - /** - * Loads the survey with the specified id from remote and writes to local db. If the survey isn't - * found or operation times out, then we return null and don't fetch the survey from local db. - * - * @throws error if the remote query fails. - */ - suspend fun loadAndSyncSurveyWithRemote(id: String): Survey? = - withTimeoutOrNull(LOAD_REMOTE_SURVEY_TIMEOUT_MILLS) { - Timber.d("Loading survey $id") - remoteDataStore.loadSurvey(id) - } - ?.apply { localSurveyStore.insertOrUpdateSurvey(this) } + fun activateSurvey(surveyId: String) { + _selectedSurveyId.update { surveyId } + firebaseCrashLogger.setSelectedSurveyId(surveyId) + } fun clearActiveSurvey() { - selectedSurveyId = null + activateSurvey("") } + // TODO: Use activeSurvey instead of selectedSurveyId as it is possible to have no active survey. + // Issue URL: https://github.com/google/ground-android/issues/3020 + fun hasActiveSurvey(): Boolean = _selectedSurveyId.value?.isNotBlank() ?: false + + fun isSurveyActive(surveyId: String): Boolean = + surveyId.isNotBlank() && activeSurvey?.id == surveyId + /** Attempts to remove the locally synced survey. Doesn't throw an error if it doesn't exist. */ suspend fun removeOfflineSurvey(surveyId: String) { val survey = localSurveyStore.getSurveyById(surveyId) survey?.let { localSurveyStore.deleteSurvey(survey) } - if (activeSurvey?.id == surveyId) { + if (isSurveyActive(surveyId)) { clearActiveSurvey() } } diff --git a/app/src/test/java/com/google/android/ground/domain/usecase/MakeSurveyAvailableOfflineUseCaseTest.kt b/app/src/test/java/com/google/android/ground/domain/usecase/MakeSurveyAvailableOfflineUseCaseTest.kt index aeac2ab293..b1879af6f3 100644 --- a/app/src/test/java/com/google/android/ground/domain/usecase/MakeSurveyAvailableOfflineUseCaseTest.kt +++ b/app/src/test/java/com/google/android/ground/domain/usecase/MakeSurveyAvailableOfflineUseCaseTest.kt @@ -18,56 +18,52 @@ package com.google.android.ground.domain.usecase import com.google.android.ground.BaseHiltTest import com.google.android.ground.domain.usecases.survey.MakeSurveyAvailableOfflineUseCase -import com.google.android.ground.repository.SurveyRepository +import com.google.android.ground.domain.usecases.survey.SyncSurveyUseCase +import com.google.common.truth.Truth.assertThat import com.sharedtest.FakeData.SURVEY +import com.sharedtest.persistence.remote.FakeRemoteDataStore import dagger.hilt.android.testing.BindValue import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject -import kotlin.test.assertEquals -import kotlin.test.assertFails -import kotlin.test.assertNull import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertThrows import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock -import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` +import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @HiltAndroidTest @RunWith(RobolectricTestRunner::class) class MakeSurveyAvailableOfflineUseCaseTest : BaseHiltTest() { + @BindValue @Mock lateinit var syncSurveyUseCase: SyncSurveyUseCase + @Inject lateinit var makeSurveyAvailableOffline: MakeSurveyAvailableOfflineUseCase - @BindValue @Mock lateinit var surveyRepository: SurveyRepository + @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore @Test - fun `Returns null when survey doesn't exist`() = runWithTestDispatcher { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenReturn(null) + fun `when survey sync returns null, should return null`() = runWithTestDispatcher { + whenever(syncSurveyUseCase(SURVEY.id)).thenReturn(null) - assertNull(makeSurveyAvailableOffline(SURVEY.id)) - } + val result = makeSurveyAvailableOffline(SURVEY.id) - @Test - fun `Throws error when survey can't be loaded`() { - runBlocking { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenThrow(Error::class.java) - - assertFails { makeSurveyAvailableOffline(SURVEY.id) } - } + assertThat(result).isNull() } @Test - fun `Returns survey on success`() = runWithTestDispatcher { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenReturn(SURVEY) + fun `when survey sync throws error, should throw error`() = runWithTestDispatcher { + whenever(syncSurveyUseCase(SURVEY.id)).thenThrow(Error::class.java) - assertEquals(SURVEY, makeSurveyAvailableOffline(SURVEY.id)) + assertThrows(Error::class.java) { runBlocking { makeSurveyAvailableOffline(SURVEY.id) } } } @Test - fun `Subscribes to updates on success`() = runWithTestDispatcher { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenReturn(SURVEY) + fun `when survey sync succeeds, should subscribe to updates`() = runWithTestDispatcher { + whenever(syncSurveyUseCase(SURVEY.id)).thenReturn(SURVEY) + + val result = makeSurveyAvailableOffline(SURVEY.id) - makeSurveyAvailableOffline(SURVEY.id) - verify(surveyRepository).subscribeToSurveyUpdates(SURVEY.id) + assertThat(result).isEqualTo(SURVEY) + assertThat(fakeRemoteDataStore.isSubscribedToSurveyUpdates(SURVEY.id)).isTrue() } } diff --git a/app/src/test/java/com/google/android/ground/domain/usecase/ReactivateLastSurveyUseCaseTest.kt b/app/src/test/java/com/google/android/ground/domain/usecase/ReactivateLastSurveyUseCaseTest.kt index 2bc9283152..d112056664 100644 --- a/app/src/test/java/com/google/android/ground/domain/usecase/ReactivateLastSurveyUseCaseTest.kt +++ b/app/src/test/java/com/google/android/ground/domain/usecase/ReactivateLastSurveyUseCaseTest.kt @@ -19,56 +19,76 @@ package com.google.android.ground.domain.usecase import com.google.android.ground.BaseHiltTest import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase import com.google.android.ground.domain.usecases.survey.ReactivateLastSurveyUseCase -import com.google.android.ground.repository.SurveyRepository -import com.sharedtest.FakeData.SURVEY -import dagger.hilt.android.testing.BindValue +import com.google.android.ground.model.Survey +import com.google.android.ground.persistence.local.LocalValueStore +import com.google.android.ground.persistence.local.stores.LocalSurveyStore +import com.google.common.truth.Truth.assertThat import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject +import kotlin.test.assertFails import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Test import org.junit.runner.RunWith -import org.mockito.Mock -import org.mockito.kotlin.never -import org.mockito.kotlin.times -import org.mockito.kotlin.verify import org.robolectric.RobolectricTestRunner @OptIn(ExperimentalCoroutinesApi::class) @HiltAndroidTest @RunWith(RobolectricTestRunner::class) class ReactivateLastSurveyUseCaseTest : BaseHiltTest() { - @Inject lateinit var reactivateLastSurvey: ReactivateLastSurveyUseCase - @Inject lateinit var surveyRepository: SurveyRepository - @BindValue @Mock lateinit var activateSurvey: ActivateSurveyUseCase + @Inject lateinit var activateSurvey: ActivateSurveyUseCase + @Inject lateinit var localSurveyStore: LocalSurveyStore + @Inject lateinit var localValueStore: LocalValueStore + @Inject lateinit var reactivateLastSurvey: ReactivateLastSurveyUseCase @Test - fun `Reactivate last survey`() = runWithTestDispatcher { - surveyRepository.lastActiveSurveyId = SURVEY.id - reactivateLastSurvey() + fun `when last survey id is present, should activate it`() = runWithTestDispatcher { + localValueStore.lastActiveSurveyId = SURVEY_ID + localSurveyStore.insertOrUpdateSurvey(SURVEY) + + val result = reactivateLastSurvey() advanceUntilIdle() - verify(activateSurvey).invoke(SURVEY.id) + assertThat(result).isTrue() } @Test - fun `Does nothing when a survey is already active`() = runWithTestDispatcher { - activateSurvey(SURVEY.id) - reactivateLastSurvey() + fun `when survey is already active, should do nothing`() = runWithTestDispatcher { + localValueStore.lastActiveSurveyId = SURVEY_ID + localSurveyStore.insertOrUpdateSurvey(SURVEY) + activateSurvey(SURVEY_ID) + + val result = reactivateLastSurvey() advanceUntilIdle() - // Tries to activate survey. - verify(activateSurvey, times(1)).invoke(SURVEY.id) + assertThat(result).isTrue() } @Test - fun `Does nothing when survey never activated`() = runWithTestDispatcher { - surveyRepository.lastActiveSurveyId = "" - reactivateLastSurvey() - advanceUntilIdle() + fun `when last survey id is not present, should do nothing`() = runWithTestDispatcher { + localValueStore.lastActiveSurveyId = "" + localSurveyStore.insertOrUpdateSurvey(SURVEY) + + assertThat(reactivateLastSurvey()).isFalse() + } + + @Test + fun `when last survey id is present but survey is not present, should do nothing`() { + localValueStore.lastActiveSurveyId = SURVEY_ID + + assertFails { runWithTestDispatcher { reactivateLastSurvey() } } + } + + companion object { + private const val SURVEY_ID = "survey_id" - // Should never try to activate survey. - verify(activateSurvey, never()).invoke(SURVEY.id) + private val SURVEY = + Survey( + id = SURVEY_ID, + title = "survey title", + description = "survey description", + jobMap = emptyMap(), + ) } } diff --git a/app/src/test/java/com/google/android/ground/domain/usecase/SyncSurveyUseCaseTest.kt b/app/src/test/java/com/google/android/ground/domain/usecase/SyncSurveyUseCaseTest.kt index 94f9078fc4..22ac4ee564 100644 --- a/app/src/test/java/com/google/android/ground/domain/usecase/SyncSurveyUseCaseTest.kt +++ b/app/src/test/java/com/google/android/ground/domain/usecase/SyncSurveyUseCaseTest.kt @@ -16,54 +16,54 @@ package com.google.android.ground.domain.usecase +import app.cash.turbine.test import com.google.android.ground.BaseHiltTest import com.google.android.ground.domain.usecases.survey.SyncSurveyUseCase +import com.google.android.ground.persistence.local.stores.LocalSurveyStore import com.google.android.ground.repository.LocationOfInterestRepository -import com.google.android.ground.repository.SurveyRepository +import com.google.common.truth.Truth.assertThat import com.sharedtest.FakeData.SURVEY +import com.sharedtest.persistence.remote.FakeRemoteDataStore import dagger.hilt.android.testing.BindValue import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject -import kotlin.test.assertFails -import kotlin.test.assertNull import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertThrows import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` import org.robolectric.RobolectricTestRunner @HiltAndroidTest @RunWith(RobolectricTestRunner::class) class SyncSurveyUseCaseTest : BaseHiltTest() { - @Inject lateinit var syncSurvey: SyncSurveyUseCase - @BindValue @Mock lateinit var surveyRepository: SurveyRepository @BindValue @Mock lateinit var loiRepository: LocationOfInterestRepository + @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore + @Inject lateinit var localSurveyStore: LocalSurveyStore + @Inject lateinit var syncSurvey: SyncSurveyUseCase + @Test fun `Syncs survey and LOIs with remote`() = runBlocking { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenReturn(SURVEY) + fakeRemoteDataStore.surveys = listOf(SURVEY) syncSurvey(SURVEY.id) - verify(surveyRepository).loadAndSyncSurveyWithRemote(SURVEY.id) + assertThat(localSurveyStore.getSurveyById(SURVEY.id)).isEqualTo(SURVEY) verify(loiRepository).syncLocationsOfInterest(SURVEY) } @Test - fun `Returns null when survey not found`() = runBlocking { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenReturn(null) - - assertNull(syncSurvey(SURVEY.id)) + fun `when survey is not found in remote storage, should return null`() = runWithTestDispatcher { + assertThat(syncSurvey("someUnknownSurveyId")).isNull() + localSurveyStore.surveys.test { assertThat(expectMostRecentItem()).isEmpty() } } @Test - fun `Throws error when load fails`() { - runBlocking { - `when`(surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id)).thenThrow(Error::class.java) + fun `when remote survey load fails, should throw error`() { + fakeRemoteDataStore.onLoadSurvey = { error("Something went wrong") } - assertFails { syncSurvey(SURVEY.id) } - } + assertThrows(IllegalStateException::class.java) { runBlocking { syncSurvey(SURVEY.id) } } } } diff --git a/app/src/test/java/com/google/android/ground/repository/SurveyRepositoryTest.kt b/app/src/test/java/com/google/android/ground/repository/SurveyRepositoryTest.kt index 484087cc5d..d97f74b7ce 100644 --- a/app/src/test/java/com/google/android/ground/repository/SurveyRepositoryTest.kt +++ b/app/src/test/java/com/google/android/ground/repository/SurveyRepositoryTest.kt @@ -20,13 +20,10 @@ import com.google.android.ground.BaseHiltTest import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase import com.google.android.ground.persistence.local.stores.LocalSurveyStore import com.google.common.truth.Truth.assertThat -import com.sharedtest.FakeData.JOB import com.sharedtest.FakeData.SURVEY import com.sharedtest.persistence.remote.FakeRemoteDataStore import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject -import kotlin.test.assertFails -import kotlin.test.assertNull import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Before @@ -38,10 +35,10 @@ import org.robolectric.RobolectricTestRunner @HiltAndroidTest @RunWith(RobolectricTestRunner::class) class SurveyRepositoryTest : BaseHiltTest() { - @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore - @Inject lateinit var surveyRepository: SurveyRepository @Inject lateinit var activateSurvey: ActivateSurveyUseCase + @Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore @Inject lateinit var localSurveyStore: LocalSurveyStore + @Inject lateinit var surveyRepository: SurveyRepository @Before override fun setUp() { @@ -52,7 +49,7 @@ class SurveyRepositoryTest : BaseHiltTest() { @Test fun `setting selectedSurveyId updates the active survey`() = runWithTestDispatcher { localSurveyStore.insertOrUpdateSurvey(SURVEY) - surveyRepository.selectedSurveyId = SURVEY.id + surveyRepository.activateSurvey(SURVEY.id) advanceUntilIdle() surveyRepository.activeSurveyFlow.test { assertThat(expectMostRecentItem()).isEqualTo(SURVEY) } @@ -69,60 +66,42 @@ class SurveyRepositoryTest : BaseHiltTest() { } @Test - fun `removeOfflineSurvey() deletes local copy`() = runWithTestDispatcher { - fakeRemoteDataStore.surveys = listOf(SURVEY) - surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id) - activateSurvey(SURVEY.id) + fun `removeOfflineSurvey - should delete local copy`() = runWithTestDispatcher { + localSurveyStore.insertOrUpdateSurvey(SURVEY) surveyRepository.removeOfflineSurvey(SURVEY.id) - advanceUntilIdle() localSurveyStore.surveys.test { assertThat(expectMostRecentItem()).isEmpty() } - assertThat(surveyRepository.activeSurvey).isNull() } @Test - fun `removeOfflineSurvey() deactivates active survey`() = runWithTestDispatcher { - fakeRemoteDataStore.surveys = listOf(SURVEY) - surveyRepository.loadAndSyncSurveyWithRemote(SURVEY.id) - activateSurvey(SURVEY.id) - - surveyRepository.removeOfflineSurvey(SURVEY.id) - advanceUntilIdle() - - assertThat(surveyRepository.activeSurvey).isNull() - } - - @Test - fun `removeOfflineSurvey() removes inactive survey`() = runWithTestDispatcher { - // Job ID must be globally unique. - val job1 = JOB.copy(id = "job1") - val job2 = JOB.copy(id = "job2") - val survey1 = SURVEY.copy(id = "active survey id", jobMap = mapOf(job1.id to job1)) - val survey2 = SURVEY.copy(id = "inactive survey id", jobMap = mapOf(job2.id to job2)) - fakeRemoteDataStore.surveys = listOf(survey1, survey2) - surveyRepository.loadAndSyncSurveyWithRemote(survey1.id) - surveyRepository.loadAndSyncSurveyWithRemote(survey2.id) - activateSurvey(survey1.id) - advanceUntilIdle() - - surveyRepository.removeOfflineSurvey(survey2.id) - advanceUntilIdle() + fun `removeOfflineSurvey - when active survey is same, should deactivate as well`() = + runWithTestDispatcher { + localSurveyStore.insertOrUpdateSurvey(SURVEY) + activateSurvey(SURVEY.id) - // Verify active survey isn't cleared - assertThat(surveyRepository.activeSurvey).isEqualTo(survey1) - localSurveyStore.surveys.test { assertThat(expectMostRecentItem()).isEqualTo(listOf(survey1)) } - } + surveyRepository.removeOfflineSurvey(SURVEY.id) - @Test - fun `loadAndSyncSurveyWithRemote() returns null when survey not found`() = runWithTestDispatcher { - assertNull(surveyRepository.loadAndSyncSurveyWithRemote("someUnknownSurveyId")) - } + assertThat(surveyRepository.activeSurvey).isNull() + } @Test - fun `loadAndSyncSurveyWithRemote() throws error when remote fetch fails`() = + fun `removeOfflineSurvey - when active survey is different, should not deactivate`() = runWithTestDispatcher { - fakeRemoteDataStore.onLoadSurvey = { error("Something went wrong") } - assertFails { surveyRepository.loadAndSyncSurveyWithRemote("anySurveyId") } + val survey1 = SURVEY.copy(id = "active survey id", jobMap = emptyMap()) + val survey2 = SURVEY.copy(id = "inactive survey id", jobMap = emptyMap()) + localSurveyStore.insertOrUpdateSurvey(survey1) + localSurveyStore.insertOrUpdateSurvey(survey2) + activateSurvey(survey1.id) + advanceUntilIdle() + + surveyRepository.removeOfflineSurvey(survey2.id) + advanceUntilIdle() + + // Verify that active survey isn't cleared or de-activated + assertThat(surveyRepository.activeSurvey).isEqualTo(survey1) + localSurveyStore.surveys.test { + assertThat(expectMostRecentItem()).isEqualTo(listOf(survey1)) + } } } diff --git a/app/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt b/app/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt index 2d0e759460..ac8538cfaa 100644 --- a/app/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt +++ b/app/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt @@ -101,7 +101,7 @@ class UserRepositoryTest : BaseHiltTest() { val user = FakeData.USER val survey = FakeData.SURVEY.copy(acl = mapOf(Pair(user.email, Role.OWNER.toString()))) fakeAuthenticationManager.setUser(user) - surveyRepository.selectedSurveyId = survey.id + surveyRepository.activateSurvey(survey.id) assertThat(userRepository.canUserSubmitData()).isTrue() } @@ -113,7 +113,7 @@ class UserRepositoryTest : BaseHiltTest() { val survey = FakeData.SURVEY.copy(acl = mapOf()) fakeAuthenticationManager.setUser(user) localSurveyStore.insertOrUpdateSurvey(survey) - surveyRepository.selectedSurveyId = survey.id + surveyRepository.activateSurvey(survey.id) advanceUntilIdle() assertThat(userRepository.canUserSubmitData()).isFalse() @@ -136,7 +136,7 @@ class UserRepositoryTest : BaseHiltTest() { val survey = FakeData.SURVEY.copy(acl = mapOf(Pair("user@gmail.com", Role.OWNER.toString()))) fakeAuthenticationManager.setUser(user) localSurveyStore.insertOrUpdateSurvey(survey) - surveyRepository.selectedSurveyId = survey.id + surveyRepository.activateSurvey(survey.id) advanceUntilIdle() assertThat(userRepository.canUserSubmitData()).isFalse() diff --git a/app/src/test/java/com/google/android/ground/ui/home/HomeScreenFragmentTest.kt b/app/src/test/java/com/google/android/ground/ui/home/HomeScreenFragmentTest.kt index 88b5fcbf8c..ff491c3260 100644 --- a/app/src/test/java/com/google/android/ground/ui/home/HomeScreenFragmentTest.kt +++ b/app/src/test/java/com/google/android/ground/ui/home/HomeScreenFragmentTest.kt @@ -153,7 +153,7 @@ class HomeScreenFragmentTest : AbstractHomeScreenFragmentTest() { @Test fun `all menu item is always enabled`() = runWithTestDispatcher { - surveyRepository.selectedSurveyId = surveyWithoutBasemap.id + surveyRepository.activateSurvey(surveyWithoutBasemap.id) advanceUntilIdle() openDrawer() @@ -252,7 +252,7 @@ class NavigationDrawerItemClickTest( @Test fun clickDrawerMenuItem() = runWithTestDispatcher { localSurveyStore.insertOrUpdateSurvey(survey) - surveyRepository.selectedSurveyId = survey.id + surveyRepository.activateSurvey(survey.id) advanceUntilIdle() openDrawer() diff --git a/app/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt b/app/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt index 61e0c19a6d..6da0501b28 100644 --- a/app/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt +++ b/app/src/test/java/com/google/android/ground/ui/surveyselector/SurveySelectorFragmentTest.kt @@ -45,7 +45,6 @@ import com.google.android.ground.domain.usecases.survey.ListAvailableSurveysUseC import com.google.android.ground.launchFragmentInHiltContainer import com.google.android.ground.launchFragmentWithNavController import com.google.android.ground.model.SurveyListItem -import com.google.android.ground.persistence.local.stores.LocalSurveyStore import com.google.android.ground.recyclerChildAction import com.google.android.ground.repository.SurveyRepository import com.google.android.ground.repository.UserRepository @@ -83,7 +82,6 @@ class SurveySelectorFragmentTest : BaseHiltTest() { @BindValue @Mock lateinit var activateSurvey: ActivateSurveyUseCase @BindValue @Mock lateinit var listAvailableSurveysUseCase: ListAvailableSurveysUseCase @Inject lateinit var fakeAuthenticationManager: FakeAuthenticationManager - @Inject lateinit var localSurveyStore: LocalSurveyStore private lateinit var fragment: SurveySelectorFragment private lateinit var navController: NavController diff --git a/app/src/test/java/com/google/android/ground/ui/syncstatus/SyncStatusFragmentTest.kt b/app/src/test/java/com/google/android/ground/ui/syncstatus/SyncStatusFragmentTest.kt index 65110eb14f..e6c71df793 100644 --- a/app/src/test/java/com/google/android/ground/ui/syncstatus/SyncStatusFragmentTest.kt +++ b/app/src/test/java/com/google/android/ground/ui/syncstatus/SyncStatusFragmentTest.kt @@ -31,11 +31,13 @@ import com.sharedtest.FakeData.SURVEY import com.sharedtest.persistence.remote.FakeRemoteDataStore import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +@OptIn(ExperimentalCoroutinesApi::class) @HiltAndroidTest @RunWith(RobolectricTestRunner::class) class SyncStatusFragmentTest : BaseHiltTest() { @@ -58,7 +60,7 @@ class SyncStatusFragmentTest : BaseHiltTest() { fun `Sync items should be displayed`() = runWithTestDispatcher { fakeRemoteDataStore.surveys = listOf(SURVEY) localSurveyStore.insertOrUpdateSurvey(SURVEY) - surveyRepository.selectedSurveyId = SURVEY.id + surveyRepository.activateSurvey(SURVEY.id) advanceUntilIdle() composeTestRule.onNodeWithTag("sync list").assertIsDisplayed()