Convert SignInState to sealed data class (#2506)

* Convert SignInState to sealed data class * Use the value from SignedIn state when saving the user instead of getting from auth manager * Use the same instance for refreshing remote profile as well * Fix import order
google · Jun 24, 2024 · ca51662 · ca51662
1 parent 2e5cff5
commit ca51662
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 82 deletions.
diff --git a/ground/src/main/java/com/google/android/ground/MainViewModel.kt b/ground/src/main/java/com/google/android/ground/MainViewModel.kt
@@ -21,6 +21,7 @@ import androidx.lifecycle.viewModelScope
 import androidx.navigation.NavDirections
 import com.google.android.ground.coroutines.IoDispatcher
 import com.google.android.ground.domain.usecases.survey.ReactivateLastSurveyUseCase
+import com.google.android.ground.model.User
 import com.google.android.ground.persistence.local.room.LocalDatabase
 import com.google.android.ground.repository.SurveyRepository
 import com.google.android.ground.repository.TermsOfServiceRepository
@@ -79,18 +80,14 @@ constructor(
 
   private suspend fun onSignInStateChange(signInState: SignInState): NavDirections? {
     // Display progress only when signing in.
-    signInProgressDialogVisibility.postValue(signInState.state == SignInState.State.SIGNING_IN)
+    signInProgressDialogVisibility.postValue(signInState == SignInState.SigningIn)
 
-    return signInState.result.fold(
-      {
-        when (signInState.state) {
-          SignInState.State.SIGNED_IN -> onUserSignedIn()
-          SignInState.State.SIGNED_OUT -> onUserSignedOut()
-          else -> null
-        }
-      },
-      { onUserSignInError(it) },
-    )
+    return when (signInState) {
+      is SignInState.Error -> onUserSignInError(signInState.error)
+      is SignInState.SignedIn -> onUserSignedIn(signInState.user)
+      is SignInState.SignedOut -> onUserSignedOut()
+      is SignInState.SigningIn -> null
+    }
   }
 
   private suspend fun onUserSignInError(error: Throwable): NavDirections? {
@@ -118,9 +115,9 @@ constructor(
     return SignInFragmentDirections.showSignInScreen()
   }
 
-  private suspend fun onUserSignedIn(): NavDirections? =
+  private suspend fun onUserSignedIn(user: User): NavDirections? =
     try {
-      userRepository.saveUserDetails()
+      userRepository.saveUserDetails(user)
       val tos = termsOfServiceRepository.getTermsOfService()
       if (tos == null || termsOfServiceRepository.isTermsOfServiceAccepted) {
         reactivateLastSurvey()

diff --git a/ground/src/main/java/com/google/android/ground/repository/UserRepository.kt b/ground/src/main/java/com/google/android/ground/repository/UserRepository.kt
@@ -52,19 +52,19 @@ constructor(
 
   suspend fun getAuthenticatedUser() = authenticationManager.getAuthenticatedUser()
 
-  /** Stores the current user's profile details into the local and remote dbs. */
-  suspend fun saveUserDetails() {
-    localUserStore.insertOrUpdateUser(getAuthenticatedUser())
-    updateRemoteUserInfo()
+  /** Stores the given user to the local and remote dbs. */
+  suspend fun saveUserDetails(user: User) {
+    localUserStore.insertOrUpdateUser(user)
+    updateRemoteUserInfo(user)
   }
 
   /** Attempts to refresh current user's profile in remote database if network is available. */
-  private suspend fun updateRemoteUserInfo() {
+  private suspend fun updateRemoteUserInfo(user: User) {
     if (!networkManager.isNetworkConnected()) {
       Timber.d("Skipped refreshing user profile as device is offline.")
       return
     }
-    if (!getAuthenticatedUser().isAnonymous) {
+    if (!user.isAnonymous) {
       runCatching { remoteDataStore.refreshUserProfile() }
         .fold(
           { Timber.i("Profile refreshed") },

diff --git a/ground/src/main/java/com/google/android/ground/system/auth/AnonymousAuthenticationManager.kt b/ground/src/main/java/com/google/android/ground/system/auth/AnonymousAuthenticationManager.kt
@@ -43,8 +43,8 @@ constructor(
 
   override fun initInternal() {
     setState(
-      if (firebaseAuth.currentUser == null) SignInState.signedOut()
-      else SignInState.signedIn(anonymousUser)
+      if (firebaseAuth.currentUser == null) SignInState.SignedOut
+      else SignInState.SignedIn(anonymousUser)
     )
   }
 
@@ -53,13 +53,13 @@ constructor(
   }
 
   override fun signIn() {
-    setState(SignInState.signingIn())
+    setState(SignInState.SigningIn)
     externalScope.launch { firebaseAuth.signInAnonymously().await() }
-    setState(SignInState.signedIn(anonymousUser))
+    setState(SignInState.SignedIn(anonymousUser))
   }
 
   override fun signOut() {
     firebaseAuth.signOut()
-    setState(SignInState.signedOut())
+    setState(SignInState.SignedOut)
   }
 }
diff --git a/ground/src/main/java/com/google/android/ground/system/auth/BaseAuthenticationManager.kt b/ground/src/main/java/com/google/android/ground/system/auth/BaseAuthenticationManager.kt
@@ -16,9 +16,8 @@
 package com.google.android.ground.system.auth
 
 import com.google.android.ground.model.User
-import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.filterIsInstance
 import kotlinx.coroutines.flow.first
-import kotlinx.coroutines.flow.mapNotNull
 
 abstract class BaseAuthenticationManager : AuthenticationManager {
 
@@ -36,9 +35,6 @@ abstract class BaseAuthenticationManager : AuthenticationManager {
       init()
     }
 
-    return signInState
-      .filter { it.state == SignInState.State.SIGNED_IN }
-      .mapNotNull { it.result.getOrNull() }
-      .first()
+    return signInState.filterIsInstance<SignInState.SignedIn>().first().user
   }
 }
diff --git a/ground/src/main/java/com/google/android/ground/system/auth/GoogleAuthenticationManager.kt b/ground/src/main/java/com/google/android/ground/system/auth/GoogleAuthenticationManager.kt
@@ -71,7 +71,7 @@ constructor(
   override fun initInternal() {
     firebaseAuth.addAuthStateListener { auth ->
       val user = auth.currentUser?.toUser()
-      setState(if (user == null) SignInState.signedOut() else SignInState.signedIn(user))
+      setState(if (user == null) SignInState.SignedOut else SignInState.SignedIn(user))
     }
   }
 
@@ -80,7 +80,7 @@ constructor(
   }
 
   override fun signIn() {
-    setState(SignInState.signingIn())
+    setState(SignInState.SigningIn)
     showSignInDialog()
   }
 
@@ -92,7 +92,7 @@ constructor(
 
   override fun signOut() {
     firebaseAuth.signOut()
-    setState(SignInState.signedOut())
+    setState(SignInState.SignedOut)
     activityStreams.withActivity { getGoogleSignInClient(it).signOut() }
   }
 
@@ -108,18 +108,18 @@ constructor(
       googleSignInTask.getResult(ApiException::class.java)?.let { onGoogleSignIn(it) }
     } catch (e: ApiException) {
       Timber.e(e, "Sign in failed")
-      setState(SignInState.error(e))
+      setState(SignInState.Error(e))
     }
   }
 
   private fun onGoogleSignIn(googleAccount: GoogleSignInAccount) =
     firebaseAuth
       .signInWithCredential(getFirebaseAuthCredential(googleAccount))
       .addOnSuccessListener { authResult: AuthResult -> onFirebaseAuthSuccess(authResult) }
-      .addOnFailureListener { setState(SignInState.error(it)) }
+      .addOnFailureListener { setState(SignInState.Error(it)) }
 
   private fun onFirebaseAuthSuccess(authResult: AuthResult) {
-    setState(SignInState.signedIn(authResult.user!!.toUser()))
+    setState(SignInState.SignedIn(authResult.user!!.toUser()))
   }
 
   private fun getFirebaseAuthCredential(googleAccount: GoogleSignInAccount): AuthCredential =

diff --git a/ground/src/main/java/com/google/android/ground/system/auth/SignInState.kt b/ground/src/main/java/com/google/android/ground/system/auth/SignInState.kt
@@ -17,23 +17,13 @@ package com.google.android.ground.system.auth
 
 import com.google.android.ground.model.User
 
-data class SignInState(val state: State, val result: Result<User?>) {
+sealed class SignInState {
 
-  enum class State {
-    SIGNED_OUT,
-    SIGNING_IN,
-    SIGNED_IN,
-    ERROR,
-  }
+  data object SignedOut : SignInState()
 
-  companion object {
+  data object SigningIn : SignInState()
 
-    fun signedOut() = SignInState(State.SIGNED_OUT, Result.success(null))
+  data class SignedIn(val user: User) : SignInState()
 
-    fun signingIn() = SignInState(State.SIGNING_IN, Result.success(null))
-
-    fun signedIn(user: User) = SignInState(State.SIGNED_IN, Result.success(user))
-
-    fun error(error: Throwable) = SignInState(State.ERROR, Result.failure(error))
-  }
+  data class Error(val error: Throwable) : SignInState()
 }
diff --git a/ground/src/main/java/com/google/android/ground/ui/signin/SignInViewModel.kt b/ground/src/main/java/com/google/android/ground/ui/signin/SignInViewModel.kt
@@ -45,11 +45,9 @@ internal constructor(
 
   fun onSignInButtonClick() {
     viewModelScope.launch {
-      val signInState = userRepository.getSignInState().first()
-      when (signInState.state) {
-        SignInState.State.SIGNED_OUT,
-        SignInState.State.ERROR -> userRepository.signIn()
-        else -> {}
+      val state = userRepository.getSignInState().first()
+      if (state is SignInState.SignedOut || state is SignInState.Error) {
+        userRepository.signIn()
       }
     }
   }

diff --git a/ground/src/test/java/com/google/android/ground/MainActivityTest.kt b/ground/src/test/java/com/google/android/ground/MainActivityTest.kt
@@ -47,7 +47,7 @@ class MainActivityTest : BaseHiltTest() {
       controller.setup() // Moves Activity to RESUMED state
       activity = controller.get()
 
-      fakeAuthenticationManager.setState(SignInState.signingIn())
+      fakeAuthenticationManager.setState(SignInState.SigningIn)
       advanceUntilIdle()
 
       assertThat(ShadowProgressDialog.getLatestDialog().isShowing).isTrue()
@@ -60,8 +60,8 @@ class MainActivityTest : BaseHiltTest() {
       controller.setup() // Moves Activity to RESUMED state
       activity = controller.get()
 
-      fakeAuthenticationManager.setState(SignInState.signingIn())
-      fakeAuthenticationManager.setState(SignInState.signedOut())
+      fakeAuthenticationManager.setState(SignInState.SigningIn)
+      fakeAuthenticationManager.setState(SignInState.SignedOut)
       advanceUntilIdle()
 
       assertThat(ShadowProgressDialog.getLatestDialog().isShowing).isFalse()

diff --git a/ground/src/test/java/com/google/android/ground/MainViewModelTest.kt b/ground/src/test/java/com/google/android/ground/MainViewModelTest.kt
@@ -20,8 +20,7 @@ import app.cash.turbine.test
 import com.google.android.ground.persistence.local.room.LocalDataStoreException
 import com.google.android.ground.repository.TermsOfServiceRepository
 import com.google.android.ground.repository.UserRepository
-import com.google.android.ground.system.auth.SignInState.Companion.error
-import com.google.android.ground.system.auth.SignInState.Companion.signingIn
+import com.google.android.ground.system.auth.SignInState
 import com.google.android.ground.ui.common.Navigator
 import com.google.android.ground.ui.signin.SignInFragmentDirections
 import com.google.common.truth.Truth.assertThat
@@ -95,7 +94,7 @@ class MainViewModelTest : BaseHiltTest() {
   @Test
   fun testSignInStateChanged_onSigningIn() = runWithTestDispatcher {
     testNoNavigation(navigator.getNavigateRequests()) {
-      fakeAuthenticationManager.setState(signingIn())
+      fakeAuthenticationManager.setState(SignInState.SigningIn)
     }
 
     verifyProgressDialogVisible(true)
@@ -180,7 +179,7 @@ class MainViewModelTest : BaseHiltTest() {
     setupUserPreferences()
 
     testNavigateTo(navigator.getNavigateRequests(), SignInFragmentDirections.showSignInScreen()) {
-      fakeAuthenticationManager.setState(error(Exception()))
+      fakeAuthenticationManager.setState(SignInState.Error(Exception()))
     }
 
     verifyProgressDialogVisible(false)

diff --git a/...nd/src/test/java/com/google/android/ground/repository/LocationOfInterestRepositoryTest.kt b/...nd/src/test/java/com/google/android/ground/repository/LocationOfInterestRepositoryTest.kt
@@ -66,7 +66,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() {
     runWithTestDispatcher {
       // Setup user
       fakeAuthenticationManager.setUser(TEST_USER)
-      userRepository.saveUserDetails()
+      userRepository.saveUserDetails(TEST_USER)
 
       // Setup survey and LOIs
       fakeRemoteDataStore.surveys = listOf(TEST_SURVEY)

diff --git a/ground/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt b/ground/src/test/java/com/google/android/ground/repository/UserRepositoryTest.kt
@@ -59,31 +59,28 @@ class UserRepositoryTest : BaseHiltTest() {
 
   @Test
   fun `saveUserDetails() updates local user profile`() = runWithTestDispatcher {
-    fakeAuthenticationManager.setUser(FakeData.USER)
     whenever(networkManager.isNetworkConnected()).thenReturn(true)
 
-    userRepository.saveUserDetails()
+    userRepository.saveUserDetails(FakeData.USER)
 
     assertThat(localUserStore.getUser(FakeData.USER.id)).isEqualTo(FakeData.USER)
   }
 
   @Test
   fun `saveUserDetails() updates remote user profile`() = runWithTestDispatcher {
-    fakeAuthenticationManager.setUser(FakeData.USER)
     whenever(networkManager.isNetworkConnected()).thenReturn(true)
 
-    userRepository.saveUserDetails()
+    userRepository.saveUserDetails(FakeData.USER)
 
     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()
+      userRepository.saveUserDetails(FakeData.USER)
 
       assertThat(fakeRemoteDataStore.userProfileRefreshCount).isEqualTo(0)
     }

diff --git a/ground/src/test/java/com/google/android/ground/ui/signin/SignInFragmentTest.kt b/ground/src/test/java/com/google/android/ground/ui/signin/SignInFragmentTest.kt
@@ -59,7 +59,7 @@ class SignInFragmentTest : BaseHiltTest() {
   @Before
   override fun setUp() {
     super.setUp()
-    fakeAuthenticationManager.setState(SignInState.signedOut())
+    fakeAuthenticationManager.setState(SignInState.SignedOut)
   }
 
   @Test
@@ -73,8 +73,7 @@ class SignInFragmentTest : BaseHiltTest() {
       advanceUntilIdle()
 
       fakeAuthenticationManager.signInState.test {
-        assertThat(expectMostRecentItem())
-          .isEqualTo(SignInState(SignInState.State.SIGNED_IN, Result.success(TEST_USER)))
+        assertThat(expectMostRecentItem()).isEqualTo(SignInState.SignedIn(TEST_USER))
       }
     }
 
@@ -88,8 +87,7 @@ class SignInFragmentTest : BaseHiltTest() {
 
     // Assert that the sign-in state is still signed out
     fakeAuthenticationManager.signInState.test {
-      assertThat(expectMostRecentItem())
-        .isEqualTo(SignInState(SignInState.State.SIGNED_OUT, Result.success(null)))
+      assertThat(expectMostRecentItem()).isEqualTo(SignInState.SignedOut)
     }
 
     onView(withId(com.google.android.material.R.id.snackbar_text))
@@ -107,8 +105,7 @@ class SignInFragmentTest : BaseHiltTest() {
     advanceUntilIdle()
 
     fakeAuthenticationManager.signInState.test {
-      assertThat(awaitItem())
-        .isEqualTo(SignInState(SignInState.State.SIGNED_IN, Result.success(TEST_USER)))
+      assertThat(awaitItem()).isEqualTo(SignInState.SignedIn(TEST_USER))
       // Fails if there are further emitted sign-in events.
     }
   }

diff --git a/sharedTest/src/main/kotlin/com/sharedtest/system/auth/FakeAuthenticationManager.kt b/sharedTest/src/main/kotlin/com/sharedtest/system/auth/FakeAuthenticationManager.kt
@@ -19,8 +19,6 @@ import com.google.android.ground.coroutines.ApplicationScope
 import com.google.android.ground.model.User
 import com.google.android.ground.system.auth.BaseAuthenticationManager
 import com.google.android.ground.system.auth.SignInState
-import com.google.android.ground.system.auth.SignInState.Companion.signedIn
-import com.google.android.ground.system.auth.SignInState.Companion.signedOut
 import com.sharedtest.FakeData
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -52,9 +50,9 @@ constructor(@ApplicationScope private val externalScope: CoroutineScope) :
     externalScope.launch { _signInStateFlow.emit(state) }
   }
 
-  override fun initInternal() = setState(signedIn(currentUser))
+  override fun initInternal() = setState(SignInState.SignedIn(currentUser))
 
-  override fun signIn() = setState(signedIn(currentUser))
+  override fun signIn() = setState(SignInState.SignedIn(currentUser))
 
-  override fun signOut() = setState(signedOut())
+  override fun signOut() = setState(SignInState.SignedOut)
 }