Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Code health] Migrate remaining business logic from SurveyRepository to a domain use case #3023

Merged
merged 7 commits into from
Jan 30, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ constructor(
private fun onUserSignedOut(): MainUiState {
// Scope of subscription is until view model is cleared. Dispose it manually otherwise, firebase
// attempts to maintain a connection even after user has logged out and throws an error.
userRepository.clearUserPreferences()

// TODO: Once multi-user login is supported, avoid clearing local db data. This is
// currently being done to prevent one user's data to be submitted as another user after
Expand All @@ -108,6 +107,7 @@ constructor(
viewModelScope.launch {
withContext(ioDispatcher) {
surveyRepository.clearActiveSurvey()
userRepository.clearUserPreferences()
localDatabase.clearAllTables()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ constructor(
private val surveyRepository: SurveyRepository,
) {

suspend operator fun invoke(): Boolean =
if (getLastActiveSurveyId().isEmpty()) {
false
} else if (surveyRepository.hasActiveSurvey()) {
true
} else {
activateSurvey(getLastActiveSurveyId())
suspend operator fun invoke(): Boolean {
if (surveyRepository.activeSurvey != null) {
// Skip if there is an active survey.
return true
}

private fun getLastActiveSurveyId(): String = localValueStore.lastActiveSurveyId
val lastActiveSurveyId = localValueStore.lastActiveSurveyId
if (lastActiveSurveyId.isEmpty()) {
// Nothing to be re-activated.
return false
}
return activateSurvey(lastActiveSurveyId)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2025 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.android.ground.domain.usecases.survey

import com.google.android.ground.persistence.local.stores.LocalSurveyStore
import com.google.android.ground.repository.SurveyRepository
import javax.inject.Inject

class RemoveOfflineSurveyUseCase
@Inject
constructor(
private val localSurveyStore: LocalSurveyStore,
private val surveyRepository: SurveyRepository,
) {

/**
* Attempts to remove the locally synced survey. Also, deactivates it before removing, if it is
* active. Doesn't throw an error if it doesn't exist.
*/
suspend operator fun invoke(surveyId: String) {
if (surveyRepository.isSurveyActive(surveyId)) {
surveyRepository.clearActiveSurvey()
}

val survey = localSurveyStore.getSurveyById(surveyId) ?: return
localSurveyStore.deleteSurvey(survey)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,16 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.withTimeout

private const val ACTIVATE_SURVEY_TIMEOUT_MILLS: Long = 3 * 1000

/**
* Coordinates persistence and retrieval of [Survey] instances from remote, local, and in memory
* data stores. For more details on this pattern and overall architecture, see
* https://developer.android.com/jetpack/docs/guide.
*/
/** Maintains the state of currently active survey. */
@OptIn(ExperimentalCoroutinesApi::class)
@Singleton
class SurveyRepository
Expand All @@ -66,13 +59,6 @@ constructor(
val activeSurvey: Survey?
get() = activeSurveyFlow.value

/** The id of the last activated survey. */
private var lastActiveSurveyId: String by localValueStore::lastActiveSurveyId

init {
activeSurveyFlow.filterNotNull().onEach { lastActiveSurveyId = it.id }.launchIn(externalScope)
}

/**
* Returns the survey with the specified id from the local db, or `null` if not available offline.
*/
Expand All @@ -99,24 +85,16 @@ constructor(
}
}

firebaseCrashLogger.setSelectedSurveyId(surveyId)
if (isSurveyActive(surveyId) || surveyId.isBlank()) {
firebaseCrashLogger.setSelectedSurveyId(surveyId)
localValueStore.lastActiveSurveyId = surveyId
}
}

suspend fun clearActiveSurvey() {
activateSurvey("")
}

fun hasActiveSurvey(): Boolean = activeSurvey != null

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) ?: return
localSurveyStore.deleteSurvey(survey)
if (isSurveyActive(surveyId)) {
clearActiveSurvey()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import com.google.android.ground.coroutines.ApplicationScope
import com.google.android.ground.coroutines.IoDispatcher
import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase
import com.google.android.ground.domain.usecases.survey.ListAvailableSurveysUseCase
import com.google.android.ground.domain.usecases.survey.RemoveOfflineSurveyUseCase
import com.google.android.ground.model.SurveyListItem
import com.google.android.ground.repository.SurveyRepository
import com.google.android.ground.repository.UserRepository
import com.google.android.ground.ui.common.AbstractViewModel
import javax.inject.Inject
Expand All @@ -40,11 +40,11 @@ import timber.log.Timber
class SurveySelectorViewModel
@Inject
internal constructor(
private val surveyRepository: SurveyRepository,
private val activateSurveyUseCase: ActivateSurveyUseCase,
@ApplicationScope private val externalScope: CoroutineScope,
@IoDispatcher private val ioDispatcher: CoroutineDispatcher,
private val listAvailableSurveysUseCase: ListAvailableSurveysUseCase,
private val remoteOfflineSurveyUseCase: RemoveOfflineSurveyUseCase,
private val userRepository: UserRepository,
) : AbstractViewModel() {

Expand Down Expand Up @@ -108,7 +108,7 @@ internal constructor(
}

fun deleteSurvey(surveyId: String) {
externalScope.launch(ioDispatcher) { surveyRepository.removeOfflineSurvey(surveyId) }
externalScope.launch(ioDispatcher) { remoteOfflineSurveyUseCase(surveyId) }
}

fun signOut() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright 2025 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.android.ground.domain.usecases.survey

import app.cash.turbine.test
import com.google.android.ground.BaseHiltTest
import com.google.android.ground.FakeData.SURVEY
import com.google.android.ground.persistence.local.stores.LocalSurveyStore
import com.google.android.ground.repository.SurveyRepository
import com.google.common.truth.Truth.assertThat
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 RemoveOfflineSurveyUseCaseTest : BaseHiltTest() {
@Inject lateinit var activateSurvey: ActivateSurveyUseCase
@Inject lateinit var localSurveyStore: LocalSurveyStore
@Inject lateinit var removeOfflineSurveyUseCase: RemoveOfflineSurveyUseCase
@Inject lateinit var surveyRepository: SurveyRepository

@Test
fun `should delete local copy`() = runWithTestDispatcher {
localSurveyStore.insertOrUpdateSurvey(SURVEY)

removeOfflineSurveyUseCase(SURVEY.id)

localSurveyStore.surveys.test { assertThat(expectMostRecentItem()).isEmpty() }
}

@Test
fun `should not throw if local copy missing`() = runWithTestDispatcher {
removeOfflineSurveyUseCase(SURVEY.id)

localSurveyStore.surveys.test { assertThat(expectMostRecentItem()).isEmpty() }
}

@Test
fun `when active survey is same, should deactivate as well`() = runWithTestDispatcher {
localSurveyStore.insertOrUpdateSurvey(SURVEY)
activateSurvey(SURVEY.id)

removeOfflineSurveyUseCase(SURVEY.id)

assertThat(surveyRepository.activeSurvey).isNull()
}

@Test
fun `when active survey is different, should not deactivate`() = runWithTestDispatcher {
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()

removeOfflineSurveyUseCase(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)) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,44 +64,4 @@ class SurveyRepositoryTest : BaseHiltTest() {
surveyRepository.activeSurveyFlow.test { assertThat(expectMostRecentItem()).isNull() }
assertThat(surveyRepository.activeSurvey).isNull()
}

@Test
fun `removeOfflineSurvey - should delete local copy`() = runWithTestDispatcher {
localSurveyStore.insertOrUpdateSurvey(SURVEY)

surveyRepository.removeOfflineSurvey(SURVEY.id)

localSurveyStore.surveys.test { assertThat(expectMostRecentItem()).isEmpty() }
}

@Test
fun `removeOfflineSurvey - when active survey is same, should deactivate as well`() =
runWithTestDispatcher {
localSurveyStore.insertOrUpdateSurvey(SURVEY)
activateSurvey(SURVEY.id)

surveyRepository.removeOfflineSurvey(SURVEY.id)

assertThat(surveyRepository.activeSurvey).isNull()
}

@Test
fun `removeOfflineSurvey - when active survey is different, should not deactivate`() =
runWithTestDispatcher {
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))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import com.google.android.ground.FakeData
import com.google.android.ground.R
import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase
import com.google.android.ground.domain.usecases.survey.ListAvailableSurveysUseCase
import com.google.android.ground.domain.usecases.survey.RemoveOfflineSurveyUseCase
import com.google.android.ground.launchFragmentInHiltContainer
import com.google.android.ground.launchFragmentWithNavController
import com.google.android.ground.model.SurveyListItem
Expand Down Expand Up @@ -81,6 +82,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
@BindValue @Mock lateinit var userRepository: UserRepository
@BindValue @Mock lateinit var activateSurvey: ActivateSurveyUseCase
@BindValue @Mock lateinit var listAvailableSurveysUseCase: ListAvailableSurveysUseCase
@BindValue @Mock lateinit var removeOfflineSurveyUseCase: RemoveOfflineSurveyUseCase
@Inject lateinit var fakeAuthenticationManager: FakeAuthenticationManager

private lateinit var fragment: SurveySelectorFragment
Expand Down Expand Up @@ -257,7 +259,7 @@ class SurveySelectorFragmentTest : BaseHiltTest() {
advanceUntilIdle()

// Assert survey is deleted
verify(surveyRepository).removeOfflineSurvey(TEST_SURVEY_2.id)
verify(removeOfflineSurveyUseCase).invoke(TEST_SURVEY_2.id)
}

@Test
Expand Down