Skip to content

Commit

Permalink
Prevent users from getting signed out when opening the app in offline…
Browse files Browse the repository at this point in the history
… mode (#1992)
  • Loading branch information
shobhitagarwal1612 authored Oct 20, 2023
1 parent 06a6e1f commit f252664
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ internal constructor(
Firebase.messaging.subscribeToTopic(surveyId).await()
}

/** Calls Cloud Function to refresh the current user's profile info in the remote database. */
/**
* Calls Cloud Function to refresh the current user's profile info in the remote database if
* network is available.
*/
override suspend fun refreshUserProfile() {
firebaseFunctions.getHttpsCallable(PROFILE_REFRESH_CLOUD_FUNCTION_NAME).call().await()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.google.android.ground.persistence.remote.firebase.base

import com.google.android.ground.system.NetworkManager.requireActiveNetwork
import com.google.android.ground.system.NetworkManager
import com.google.firebase.firestore.CollectionReference
import com.google.firebase.firestore.DocumentSnapshot
import com.google.firebase.firestore.Query
Expand Down Expand Up @@ -44,7 +44,7 @@ protected constructor(
query: Query,
mappingFunction: Function<DocumentSnapshot, T>
): List<T> {
requireActiveNetwork(context)
NetworkManager(context).requireNetworkConnection()
val querySnapshot = query.get().await()
return querySnapshot.documents
.filter { it.exists() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.google.android.ground.repository
import com.google.android.ground.model.TermsOfService
import com.google.android.ground.persistence.local.LocalValueStore
import com.google.android.ground.persistence.remote.RemoteDataStore
import com.google.android.ground.system.NetworkManager
import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.coroutines.withTimeoutOrNull
Expand All @@ -30,16 +31,26 @@ private const val LOAD_REMOTE_SURVEY_TERMS_OF_SERVICE_TIMEOUT_MILLIS: Long = 30
class TermsOfServiceRepository
@Inject
constructor(
private val networkManager: NetworkManager,
private val remoteDataStore: RemoteDataStore,
private val localValueStore: LocalValueStore
) {

var isTermsOfServiceAccepted: Boolean by localValueStore::isTermsOfServiceAccepted

suspend fun getTermsOfService(): TermsOfService? =
/**
* @return [TermsOfService] from remote data store. Otherwise null if the request times out or
* network is unavailable.
*/
suspend fun getTermsOfService(): TermsOfService? {
// TODO(#1691): Maybe parse the exception and display to the user.
withTimeoutOrNull(LOAD_REMOTE_SURVEY_TERMS_OF_SERVICE_TIMEOUT_MILLIS) {
if (!networkManager.isNetworkConnected()) {
return null
}

return withTimeoutOrNull(LOAD_REMOTE_SURVEY_TERMS_OF_SERVICE_TIMEOUT_MILLIS) {
Timber.d("Loading Terms of Service")
remoteDataStore.loadTermsOfService()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import com.google.android.ground.model.User
import com.google.android.ground.persistence.local.LocalValueStore
import com.google.android.ground.persistence.local.stores.LocalUserStore
import com.google.android.ground.persistence.remote.RemoteDataStore
import com.google.android.ground.system.NetworkManager
import com.google.android.ground.system.auth.AuthenticationManager
import javax.inject.Inject
import javax.inject.Singleton
import timber.log.Timber

/**
* Coordinates persistence of [User] instance in local data store. For more details on this pattern
Expand All @@ -35,13 +37,23 @@ constructor(
private val authenticationManager: AuthenticationManager,
private val localValueStore: LocalValueStore,
private val localUserStore: LocalUserStore,
private val networkManager: NetworkManager,
private val surveyRepository: SurveyRepository,
private val remoteDataStore: RemoteDataStore,
) : AuthenticationManager by authenticationManager {

/** Stores the current user's profile details into the local and remote dbs. */
suspend fun saveUserDetails() {
localUserStore.insertOrUpdateUser(authenticationManager.currentUser)
updateRemoteUserInfo()
}

/** Attempts to refresh current user's profile in remote database if network is available. */
private suspend fun updateRemoteUserInfo() {
if (!networkManager.isNetworkConnected()) {
Timber.d("Skipped refreshing user profile as device is offline.")
return
}
remoteDataStore.refreshUserProfile()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,26 @@ package com.google.android.ground.system
import android.content.Context
import android.net.ConnectivityManager
import androidx.annotation.RequiresPermission
import dagger.hilt.android.qualifiers.ApplicationContext
import java.net.ConnectException
import javax.inject.Inject
import javax.inject.Singleton

/** Abstracts access to network state. */
object NetworkManager {
@Singleton
class NetworkManager @Inject constructor(@ApplicationContext private val context: Context) {

/** Returns true iff the device has internet connectivity, false otherwise. */
@RequiresPermission("android.permission.ACCESS_NETWORK_STATE")
private fun isNetworkAvailable(context: Context): Boolean {
fun isNetworkConnected(): Boolean {
val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
val networkInfo = cm.activeNetworkInfo
return networkInfo?.isConnected ?: false
}

/** Throws an error if network isn't available. */
fun requireActiveNetwork(context: Context) {
if (!isNetworkAvailable(context)) {
fun requireNetworkConnection() {
if (!isNetworkConnected()) {
throw ConnectException()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ data class SignInState(val state: State, val result: Result<User?>) {

companion object {

@JvmStatic fun signedOut() = SignInState(State.SIGNED_OUT, Result.success(null))
fun signedOut() = SignInState(State.SIGNED_OUT, Result.success(null))

@JvmStatic fun signingIn() = SignInState(State.SIGNING_IN, Result.success(null))
fun signingIn() = SignInState(State.SIGNING_IN, Result.success(null))

@JvmStatic fun signedIn(user: User) = SignInState(State.SIGNED_IN, Result.success(user))
fun signedIn(user: User) = SignInState(State.SIGNED_IN, Result.success(user))

@JvmStatic fun error(error: Throwable) = SignInState(State.ERROR, Result.failure(error))
fun error(error: Throwable) = SignInState(State.ERROR, Result.failure(error))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,42 +16,77 @@
package com.google.android.ground.repository

import com.google.android.ground.BaseHiltTest
import com.google.android.ground.system.NetworkManager
import com.google.common.truth.Truth.assertThat
import com.google.firebase.firestore.FirebaseFirestoreException
import com.sharedtest.FakeData
import com.sharedtest.persistence.remote.FakeRemoteDataStore
import dagger.hilt.android.testing.BindValue
import dagger.hilt.android.testing.HiltAndroidTest
import javax.inject.Inject
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.kotlin.whenever
import org.robolectric.RobolectricTestRunner

@HiltAndroidTest
@RunWith(RobolectricTestRunner::class)
class TermsOfServiceRepositoryTest : BaseHiltTest() {
@Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore

@Inject lateinit var termsOfServiceRepository: TermsOfServiceRepository

@BindValue @Mock lateinit var mockNetworkManager: NetworkManager

@Test
fun testGetTermsOfService() = runBlocking {
whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
fakeRemoteDataStore.termsOfService = Result.success(FakeData.TERMS_OF_SERVICE)

assertThat(termsOfServiceRepository.getTermsOfService()).isEqualTo(FakeData.TERMS_OF_SERVICE)
}

@Test
fun testGetTermsOfService_whenMissing_returnsNull() = runBlocking {
whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)

assertThat(termsOfServiceRepository.getTermsOfService()).isNull()
}

@Test
fun testGetTermsOfService_whenRequestFails_throwsError() {
whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
fakeRemoteDataStore.termsOfService =
Result.failure(
FirebaseFirestoreException("user error", FirebaseFirestoreException.Code.ABORTED)
)

assertThrows(FirebaseFirestoreException::class.java) {
runBlocking { termsOfServiceRepository.getTermsOfService() }
}
}

@Test
fun testGetTermsOfService_whenOffline_returnsNull() = runBlocking {
whenever(mockNetworkManager.isNetworkConnected()).thenReturn(false)
fakeRemoteDataStore.termsOfService =
Result.failure(
FirebaseFirestoreException("user error", FirebaseFirestoreException.Code.ABORTED)
)

assertThat(termsOfServiceRepository.getTermsOfService()).isNull()
}

@Test
fun testGetTermsOfService_whenServiceUnavailable_throwsError() {
whenever(mockNetworkManager.isNetworkConnected()).thenReturn(true)
fakeRemoteDataStore.termsOfService =
Result.failure(
FirebaseFirestoreException("device offline", FirebaseFirestoreException.Code.UNAVAILABLE)
)

assertThrows(FirebaseFirestoreException::class.java) {
runBlocking { termsOfServiceRepository.getTermsOfService() }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,19 @@ package com.google.android.ground.repository
import com.google.android.ground.BaseHiltTest
import com.google.android.ground.persistence.local.LocalValueStore
import com.google.android.ground.persistence.local.stores.LocalUserStore
import com.google.android.ground.system.NetworkManager
import com.google.common.truth.Truth.assertThat
import com.sharedtest.FakeData
import com.sharedtest.persistence.remote.FakeRemoteDataStore
import com.sharedtest.system.auth.FakeAuthenticationManager
import dagger.hilt.android.testing.BindValue
import dagger.hilt.android.testing.HiltAndroidTest
import javax.inject.Inject
import kotlinx.coroutines.ExperimentalCoroutinesApi
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.kotlin.whenever
import org.robolectric.RobolectricTestRunner

@OptIn(ExperimentalCoroutinesApi::class)
Expand All @@ -39,6 +43,8 @@ class UserRepositoryTest : BaseHiltTest() {
@Inject lateinit var userRepository: UserRepository
@Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore

@BindValue @Mock lateinit var networkManager: NetworkManager

@Test
fun `currentUser returns current user`() {
fakeAuthenticationManager.setUser(FakeData.USER)
Expand All @@ -49,6 +55,7 @@ class UserRepositoryTest : BaseHiltTest() {
@Test
fun `saveUserDetails() updates local user profile`() = runWithTestDispatcher {
fakeAuthenticationManager.setUser(FakeData.USER)
whenever(networkManager.isNetworkConnected()).thenReturn(true)

userRepository.saveUserDetails()

Expand All @@ -58,12 +65,24 @@ class UserRepositoryTest : BaseHiltTest() {
@Test
fun `saveUserDetails() updates remote user profile`() = runWithTestDispatcher {
fakeAuthenticationManager.setUser(FakeData.USER)
whenever(networkManager.isNetworkConnected()).thenReturn(true)

userRepository.saveUserDetails()

assertThat(fakeRemoteDataStore.userProfileRefreshCount).isEqualTo(1)
}

@Test
fun `saveUserDetails() doesn't update remote user profile when offline `() =
runWithTestDispatcher {
fakeAuthenticationManager.setUser(FakeData.USER)
whenever(networkManager.isNetworkConnected()).thenReturn(false)

userRepository.saveUserDetails()

assertThat(fakeRemoteDataStore.userProfileRefreshCount).isEqualTo(0)
}

@Test
fun `clearUserPreferences() clears lastActiveSurveyId`() {
localValueStore.lastActiveSurveyId = "foo"
Expand Down

0 comments on commit f252664

Please sign in to comment.