Merge pull request #3154 from CruGlobal/accountManagerCleanup

Cleanup some AccountManager properties

app/src/main/kotlin/org/cru/godtools/ui/drawer/DrawerViewModel.kt

@@ -15,7 +15,7 @@ import org.cru.godtools.account.GodToolsAccountManager
 class DrawerViewModel @Inject constructor(
     private val accountManager: GodToolsAccountManager,
 ) : ViewModel() {
-    val isAuthenticatedFlow = accountManager.isAuthenticatedFlow()
+    val isAuthenticatedFlow = accountManager.isAuthenticatedFlow
         .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), false)
 
     // region Actions

app/src/main/kotlin/org/cru/godtools/ui/login/LoginActivity.kt

@@ -54,7 +54,7 @@ class LoginActivity : BaseActivity() {
     }
 
     private fun finishWhenAuthenticated() {
-        accountManager.isAuthenticatedFlow()
+        accountManager.isAuthenticatedFlow
             .flowWithLifecycle(lifecycle, Lifecycle.State.RESUMED)
             .onEach { if (it) finish() }
             .launchIn(lifecycleScope)

app/src/test/kotlin/org/cru/godtools/ExternalSingletonsModule.kt

@@ -52,7 +52,7 @@ class ExternalSingletonsModule {
     @get:Provides
     val accountManager by lazy {
         mockk<GodToolsAccountManager> {
-            every { isAuthenticatedFlow() } returns flowOf(false)
+            every { isAuthenticatedFlow } returns flowOf(false)
             every { prepareForLogin(any()) } returns mockk()
         }
     }

library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt

@@ -24,7 +24,7 @@ abstract class AccountModule {
             @ApplicationContext context: Context,
             accountManager: GodToolsAccountManager
         ) = object : MobileContentApiSessionInterceptor(context) {
-            override suspend fun userId() = accountManager.userId()
+            override fun userId() = accountManager.userId
             override suspend fun authenticate() = accountManager.authenticateWithMobileContentApi()
         }
     }

library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt

@@ -25,15 +25,15 @@ import org.cru.godtools.account.provider.AccountProvider
 class GodToolsAccountManager @VisibleForTesting internal constructor(
     @get:VisibleForTesting
     internal val providers: List<AccountProvider>,
-    private val coroutineScope: CoroutineScope = CoroutineScope(SupervisorJob()),
+    coroutineScope: CoroutineScope = CoroutineScope(SupervisorJob()),
 ) {
     @Inject
     internal constructor(providers: Set<@JvmSuppressWildcards AccountProvider>) :
         this(providers.sortedWith(Ordered.COMPARATOR))
 
     // region Active Provider
     @VisibleForTesting
-    internal suspend fun activeProvider() = providers.firstOrNull { it.isAuthenticated() }
+    internal val activeProvider get() = providers.firstOrNull { it.isAuthenticated }
 
     @VisibleForTesting
     internal val activeProviderFlow =
@@ -42,13 +42,13 @@ class GodToolsAccountManager @VisibleForTesting internal constructor(
         }.stateIn(coroutineScope, SharingStarted.Eagerly, null)
     // endregion Active Provider
 
-    suspend fun isAuthenticated() = activeProvider()?.isAuthenticated() ?: false
-    suspend fun userId() = activeProvider()?.userId()
-    fun isAuthenticatedFlow() = activeProviderFlow
+    val isAuthenticated get() = activeProvider?.isAuthenticated ?: false
+    val userId get() = activeProvider?.userId
+    val isAuthenticatedFlow = activeProviderFlow
         .flatMapLatest { it?.isAuthenticatedFlow() ?: flowOf(false) }
         .shareIn(coroutineScope, SharingStarted.WhileSubscribed(), replay = 1)
         .distinctUntilChanged()
-    fun userIdFlow() = activeProviderFlow
+    val userIdFlow = activeProviderFlow
         .flatMapLatest { it?.userIdFlow() ?: flowOf(null) }
         .shareIn(coroutineScope, SharingStarted.WhileSubscribed(), replay = 1)
         .distinctUntilChanged()
@@ -68,5 +68,5 @@ class GodToolsAccountManager @VisibleForTesting internal constructor(
     }
     // endregion Login/Logout
 
-    internal suspend fun authenticateWithMobileContentApi() = activeProvider()?.authenticateWithMobileContentApi()
+    internal suspend fun authenticateWithMobileContentApi() = activeProvider?.authenticateWithMobileContentApi()
 }

library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt

@@ -9,8 +9,8 @@ import org.cru.godtools.api.model.AuthToken
 internal interface AccountProvider : Ordered {
     val type: AccountType
 
-    suspend fun isAuthenticated(): Boolean
-    suspend fun userId(): String?
+    val isAuthenticated: Boolean
+    val userId: String?
     fun isAuthenticatedFlow(): Flow<Boolean>
     fun userIdFlow(): Flow<String?>
 

library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt

@@ -46,9 +46,8 @@ internal class FacebookAccountProvider @Inject constructor(
     @VisibleForTesting
     internal val prefs by lazy { context.getSharedPreferences(PREFS_FACEBOOK_ACCOUNT_PROVIDER, Context.MODE_PRIVATE) }
 
-    override suspend fun userId() =
-        accessTokenManager.currentAccessToken?.let { prefs.getString(PREF_USER_ID(it), null) }
-    override suspend fun isAuthenticated() = accessTokenManager.currentAccessToken?.isExpired == false
+    override val userId get() = accessTokenManager.currentAccessToken?.let { prefs.getString(PREF_USER_ID(it), null) }
+    override val isAuthenticated get() = accessTokenManager.currentAccessToken?.isExpired == false
     override fun userIdFlow() = accessTokenManager.currentAccessTokenFlow()
         .flatMapLatest { it?.let { prefs.getStringFlow(PREF_USER_ID(it), null) } ?: flowOf(null) }
     override fun isAuthenticatedFlow() = accessTokenManager.isAuthenticatedFlow()

library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt

@@ -44,8 +44,8 @@ internal class GoogleAccountProvider @Inject constructor(
     internal val prefs by lazy { context.getSharedPreferences(PREFS_GOOGLE_ACCOUNT_PROVIDER, Context.MODE_PRIVATE) }
     override val type = AccountType.GOOGLE
 
-    override suspend fun isAuthenticated() = GoogleSignIn.getLastSignedInAccount(context) != null
-    override suspend fun userId() = GoogleSignIn.getLastSignedInAccount(context)
+    override val isAuthenticated get() = GoogleSignIn.getLastSignedInAccount(context) != null
+    override val userId get() = GoogleSignIn.getLastSignedInAccount(context)
         ?.let { prefs.getString(PREF_USER_ID(it), null) }
     override fun isAuthenticatedFlow() = GoogleSignInKtx.getLastSignedInAccountFlow(context).map { it != null }
     override fun userIdFlow() = GoogleSignInKtx.getLastSignedInAccountFlow(context)

library/account/src/test/kotlin/org/cru/godtools/account/GodToolsAccountManagerTest.kt

@@ -25,12 +25,12 @@ class GodToolsAccountManagerTest {
 
     private val provider1 = mockk<AccountProvider>(relaxed = true) {
         every { order } returns 1
-        coEvery { isAuthenticated() } answers { provider1Authenticated.value }
+        coEvery { isAuthenticated } answers { provider1Authenticated.value }
         every { isAuthenticatedFlow() } returns provider1Authenticated
     }
     private val provider2 = mockk<AccountProvider>(relaxed = true) {
         every { order } returns 2
-        coEvery { isAuthenticated() } answers { provider2Authenticated.value }
+        coEvery { isAuthenticated } answers { provider2Authenticated.value }
         every { isAuthenticatedFlow() } returns provider2Authenticated
     }
     private val testScope = TestScope()
@@ -49,13 +49,13 @@ class GodToolsAccountManagerTest {
     @Test
     fun verifyActiveProvider() = testScope.runTest {
         provider1Authenticated.value = true
-        assertSame(provider1, manager.activeProvider())
+        assertSame(provider1, manager.activeProvider)
         provider2Authenticated.value = true
-        assertSame(provider1, manager.activeProvider())
+        assertSame(provider1, manager.activeProvider)
         provider1Authenticated.value = false
-        assertSame(provider2, manager.activeProvider())
+        assertSame(provider2, manager.activeProvider)
         provider2Authenticated.value = false
-        assertNull(manager.activeProvider())
+        assertNull(manager.activeProvider)
     }
 
     @Test
@@ -88,15 +88,15 @@ class GodToolsAccountManagerTest {
     @Test
     fun verifyIsAuthenticated() = testScope.runTest {
         provider1Authenticated.value = false
-        assertFalse(manager.isAuthenticated())
+        assertFalse(manager.isAuthenticated)
         provider1Authenticated.value = true
-        assertTrue(manager.isAuthenticated())
+        assertTrue(manager.isAuthenticated)
     }
 
     @Test
     fun verifyIsAuthenticatedFlow() = testScope.runTest {
         provider1Authenticated.value = false
-        manager.isAuthenticatedFlow().test {
+        manager.isAuthenticatedFlow.test {
             assertFalse(awaitItem())
 
             provider1Authenticated.value = true

library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt

@@ -69,13 +69,13 @@ class FacebookAccountProviderTest {
 
     @Test
     fun `userId()`() = runTest {
-        assertNull(provider.userId())
+        assertNull(provider.userId)
 
         val user = UUID.randomUUID().toString()
         val token = accessToken()
         currentAccessTokenFlow.value = token
         provider.prefs.edit { putString(FacebookAccountProvider.PREF_USER_ID(token), user) }
-        assertEquals(user, provider.userId())
+        assertEquals(user, provider.userId)
     }
 
     // region userIdFlow()
@@ -124,7 +124,7 @@ class FacebookAccountProviderTest {
         coEvery { api.authenticate(any()) } returns Response.success(JsonApiObject.of(token))
 
         assertEquals(token, provider.authenticateWithMobileContentApi())
-        assertEquals(token.userId, provider.userId())
+        assertEquals(token.userId, provider.userId)
         coVerifyAll {
             api.authenticate(AuthToken.Request(fbAccessToken = accessToken.token))
         }
@@ -152,7 +152,7 @@ class FacebookAccountProviderTest {
             .returns(Response.success(JsonApiObject.of(token)))
 
         assertEquals(token, provider.authenticateWithMobileContentApi())
-        assertEquals(token.userId, provider.userId())
+        assertEquals(token.userId, provider.userId)
         coVerifyAll {
             api.authenticate(AuthToken.Request(fbAccessToken = accessToken.token))
             accessTokenManager.refreshCurrentAccessToken()
@@ -220,7 +220,7 @@ class FacebookAccountProviderTest {
         coEvery { api.authenticate(any()) } returns Response.success(JsonApiObject.of(token))
 
         assertEquals(token, provider.authenticateWithMobileContentApi())
-        assertNull(provider.userId())
+        assertNull(provider.userId)
         coVerifyAll {
             api.authenticate(AuthToken.Request(fbAccessToken = accessToken.token))
         }

library/analytics/src/main/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsService.kt

@@ -108,7 +108,7 @@ class FirebaseAnalyticsService @VisibleForTesting internal constructor(
             }
             .launchIn(coroutineScope)
 
-        accountManager.isAuthenticatedFlow()
+        accountManager.isAuthenticatedFlow
             .onEach { firebase.setUserProperty(USER_PROP_LOGGED_IN_STATUS, "$it") }
             .launchIn(coroutineScope)
 

library/analytics/src/test/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsServiceTest.kt

@@ -27,7 +27,7 @@ class FirebaseAnalyticsServiceTest {
     private val userFlow = MutableSharedFlow<User?>()
 
     private val accountManager: GodToolsAccountManager = mockk {
-        every { isAuthenticatedFlow() } returns flowOf(false)
+        every { isAuthenticatedFlow } returns flowOf(false)
     }
     private val eventBus: EventBus = mockk(relaxUnitFun = true)
     private val firebase: FirebaseAnalytics = mockk(relaxUnitFun = true)

library/api/src/main/kotlin/org/cru/godtools/api/MobileContentApiSessionInterceptor.kt

@@ -13,7 +13,7 @@ import org.cru.godtools.api.model.AuthToken
 private const val HTTP_HEADER_AUTHORIZATION = "Authorization"
 
 abstract class MobileContentApiSessionInterceptor(context: Context) : SessionInterceptor<UserIdSession>(context) {
-    override fun loadSession(prefs: SharedPreferences) = runBlocking { userId()?.let { UserIdSession(prefs, it) } }
+    override fun loadSession(prefs: SharedPreferences) = userId()?.let { UserIdSession(prefs, it) }
 
     override fun attachSession(request: Request, session: UserIdSession): Request {
         if (!session.isValid) return request
@@ -31,6 +31,6 @@ abstract class MobileContentApiSessionInterceptor(context: Context) : SessionInt
 
     override fun isSessionInvalid(response: Response) = response.code == HttpURLConnection.HTTP_UNAUTHORIZED
 
-    protected abstract suspend fun userId(): String?
+    protected abstract fun userId(): String?
     protected abstract suspend fun authenticate(): AuthToken?
 }

library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasks.kt

@@ -30,8 +30,8 @@ internal class UserCounterSyncTasks @Inject internal constructor(
     private val countersUpdateMutex = Mutex()
 
     suspend fun syncCounters(force: Boolean): Boolean = countersMutex.withLock {
-        if (!accountManager.isAuthenticated()) return true
-        val userId = accountManager.userId().orEmpty()
+        if (!accountManager.isAuthenticated) return true
+        val userId = accountManager.userId.orEmpty()
 
         // short-circuit if we aren't forcing a sync and the data isn't stale
         if (!force &&
@@ -57,7 +57,7 @@ internal class UserCounterSyncTasks @Inject internal constructor(
     }
 
     suspend fun syncDirtyCounters(): Boolean = countersUpdateMutex.withLock {
-        if (!accountManager.isAuthenticated()) return true
+        if (!accountManager.isAuthenticated) return true
 
         coroutineScope {
             // process any counters that need to be updated

library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasks.kt

@@ -42,8 +42,8 @@ internal class UserFavoriteToolsSyncTasks @Inject constructor(
     private val favoritesUpdateMutex = Mutex()
 
     suspend fun syncFavoriteTools(force: Boolean) = favoriteToolsMutex.withLock {
-        if (!accountManager.isAuthenticated()) return true
-        val userId = accountManager.userId().orEmpty()
+        if (!accountManager.isAuthenticated) return true
+        val userId = accountManager.userId.orEmpty()
 
         // short-circuit if we aren't forcing a sync and the data isn't stale
         if (!force &&
@@ -73,8 +73,8 @@ internal class UserFavoriteToolsSyncTasks @Inject constructor(
 
     suspend fun syncDirtyFavoriteTools(): Boolean = favoritesUpdateMutex.withLock {
         coroutineScope {
-            if (!accountManager.isAuthenticated()) return@coroutineScope true
-            val userId = accountManager.userId().orEmpty()
+            if (!accountManager.isAuthenticated) return@coroutineScope true
+            val userId = accountManager.userId.orEmpty()
 
             val user = userRepository.findUser(userId)?.takeIf { it.isInitialFavoriteToolsSynced }
                 ?: userApi.getUser().takeIf { it.isSuccessful }

library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserSyncTasks.kt

@@ -32,8 +32,8 @@ internal class UserSyncTasks @Inject constructor(
     private val userMutex = Mutex()
 
     suspend fun syncUser(force: Boolean) = userMutex.withLock {
-        if (!accountManager.isAuthenticated()) return true
-        val userId = accountManager.userId().orEmpty()
+        if (!accountManager.isAuthenticated) return true
+        val userId = accountManager.userId.orEmpty()
 
         // short-circuit if we aren't forcing a sync and the data isn't stale
         if (!force &&

library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasksTest.kt

@@ -25,8 +25,8 @@ private const val USER_ID_OTHER = "user_id_other"
 
 class UserCounterSyncTasksTest {
     private val accountManager: GodToolsAccountManager = mockk {
-        coEvery { isAuthenticated() } returns true
-        coEvery { userId() } returns USER_ID
+        coEvery { isAuthenticated } returns true
+        coEvery { userId } returns USER_ID
     }
     private val countersApi: UserCountersApi = mockk {
         coEvery { getCounters() } returns Response.success(JsonApiObject.of())
@@ -44,11 +44,11 @@ class UserCounterSyncTasksTest {
     // region syncCounters()
     @Test
     fun `syncCounters() - not authenticated`() = runTest {
-        coEvery { accountManager.isAuthenticated() } returns false
+        coEvery { accountManager.isAuthenticated } returns false
 
         assertTrue(tasks.syncCounters(false))
         coVerifyAll {
-            accountManager.isAuthenticated()
+            accountManager.isAuthenticated
             countersApi wasNot Called
             lastSyncTimeRepository wasNot Called
         }
@@ -62,8 +62,8 @@ class UserCounterSyncTasksTest {
 
         assertTrue(tasks.syncCounters(force = false))
         coVerifyAll {
-            accountManager.isAuthenticated()
-            accountManager.userId()
+            accountManager.isAuthenticated
+            accountManager.userId
             lastSyncTimeRepository.isLastSyncStale(SYNC_TIME_COUNTERS, USER_ID, staleAfter = any())
             countersApi wasNot Called
         }
@@ -73,8 +73,8 @@ class UserCounterSyncTasksTest {
     fun `syncCounters(force = true)`() = runTest {
         assertTrue(tasks.syncCounters(force = true))
         coVerifyAll {
-            accountManager.isAuthenticated()
-            accountManager.userId()
+            accountManager.isAuthenticated
+            accountManager.userId
             countersApi.getCounters()
             lastSyncTimeRepository.resetLastSyncTime(any(), isPrefix = true)
             lastSyncTimeRepository.updateLastSyncTime(any(), any())