From fe3607ed2ed58f31d2ee999396af093687c5e931 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Thu, 12 Oct 2023 12:02:50 -0600 Subject: [PATCH 1/3] nothing requires isAuthenticated or userId to be suspendable --- .../org/cru/godtools/account/GodToolsAccountManager.kt | 6 +++--- .../org/cru/godtools/account/provider/AccountProvider.kt | 4 ++-- .../account/provider/facebook/FacebookAccountProvider.kt | 5 ++--- .../account/provider/google/GoogleAccountProvider.kt | 4 ++-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt b/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt index b2dc2939a7..896dc3a97b 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt +++ b/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt @@ -33,7 +33,7 @@ class GodToolsAccountManager @VisibleForTesting internal constructor( // region Active Provider @VisibleForTesting - internal suspend fun activeProvider() = providers.firstOrNull { it.isAuthenticated() } + internal fun activeProvider() = providers.firstOrNull { it.isAuthenticated() } @VisibleForTesting internal val activeProviderFlow = @@ -42,8 +42,8 @@ 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 isAuthenticated() = activeProvider()?.isAuthenticated() ?: false + fun userId() = activeProvider()?.userId() fun isAuthenticatedFlow() = activeProviderFlow .flatMapLatest { it?.isAuthenticatedFlow() ?: flowOf(false) } .shareIn(coroutineScope, SharingStarted.WhileSubscribed(), replay = 1) diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt index d4e7d51a0c..0f1a405eff 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt +++ b/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? + fun isAuthenticated(): Boolean + fun userId(): String? fun isAuthenticatedFlow(): Flow fun userIdFlow(): Flow diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt index cfb774e0e8..f1a9f9a498 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt +++ b/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 fun userId() = accessTokenManager.currentAccessToken?.let { prefs.getString(PREF_USER_ID(it), null) } + override fun isAuthenticated() = 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() diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt index fe35f9b18a..55788992da 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt +++ b/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 fun isAuthenticated() = GoogleSignIn.getLastSignedInAccount(context) != null + override fun userId() = 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) From d3ec3f1e3800e6aa45af590cd283db8a4c158987 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Thu, 12 Oct 2023 12:52:53 -0600 Subject: [PATCH 2/3] make isAuthenticated and userId properties instead of methods --- .../cru/godtools/ui/drawer/DrawerViewModel.kt | 2 +- .../cru/godtools/ui/login/LoginActivity.kt | 2 +- .../cru/godtools/ExternalSingletonsModule.kt | 2 +- .../org/cru/godtools/account/AccountModule.kt | 2 +- .../account/GodToolsAccountManager.kt | 14 ++++++------- .../account/provider/AccountProvider.kt | 4 ++-- .../facebook/FacebookAccountProvider.kt | 4 ++-- .../provider/google/GoogleAccountProvider.kt | 4 ++-- .../account/GodToolsAccountManagerTest.kt | 18 ++++++++--------- .../facebook/FacebookAccountProviderTest.kt | 10 +++++----- .../firebase/FirebaseAnalyticsService.kt | 2 +- .../firebase/FirebaseAnalyticsServiceTest.kt | 2 +- .../sync/task/UserCounterSyncTasks.kt | 6 +++--- .../sync/task/UserFavoriteToolsSyncTasks.kt | 8 ++++---- .../cru/godtools/sync/task/UserSyncTasks.kt | 4 ++-- .../sync/task/UserCounterSyncTasksTest.kt | 16 +++++++-------- .../task/UserFavoriteToolsSyncTasksTest.kt | 20 +++++++++---------- .../godtools/sync/task/UserSyncTasksTest.kt | 20 +++++++++---------- .../org/cru/godtools/user/data/UserManager.kt | 2 +- .../cru/godtools/user/data/UserManagerTest.kt | 2 +- 20 files changed, 72 insertions(+), 72 deletions(-) diff --git a/app/src/main/kotlin/org/cru/godtools/ui/drawer/DrawerViewModel.kt b/app/src/main/kotlin/org/cru/godtools/ui/drawer/DrawerViewModel.kt index c6d7aaa4ac..321865c00a 100644 --- a/app/src/main/kotlin/org/cru/godtools/ui/drawer/DrawerViewModel.kt +++ b/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 diff --git a/app/src/main/kotlin/org/cru/godtools/ui/login/LoginActivity.kt b/app/src/main/kotlin/org/cru/godtools/ui/login/LoginActivity.kt index afb7293446..dac91e7f16 100644 --- a/app/src/main/kotlin/org/cru/godtools/ui/login/LoginActivity.kt +++ b/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) diff --git a/app/src/test/kotlin/org/cru/godtools/ExternalSingletonsModule.kt b/app/src/test/kotlin/org/cru/godtools/ExternalSingletonsModule.kt index 12d61745ae..157bb89b48 100644 --- a/app/src/test/kotlin/org/cru/godtools/ExternalSingletonsModule.kt +++ b/app/src/test/kotlin/org/cru/godtools/ExternalSingletonsModule.kt @@ -52,7 +52,7 @@ class ExternalSingletonsModule { @get:Provides val accountManager by lazy { mockk { - every { isAuthenticatedFlow() } returns flowOf(false) + every { isAuthenticatedFlow } returns flowOf(false) every { prepareForLogin(any()) } returns mockk() } } diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt b/library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt index 13eb5e731c..91e6a86850 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt +++ b/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 suspend fun userId() = accountManager.userId override suspend fun authenticate() = accountManager.authenticateWithMobileContentApi() } } diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt b/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt index 896dc3a97b..a70699a016 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt +++ b/library/account/src/main/kotlin/org/cru/godtools/account/GodToolsAccountManager.kt @@ -25,7 +25,7 @@ import org.cru.godtools.account.provider.AccountProvider class GodToolsAccountManager @VisibleForTesting internal constructor( @get:VisibleForTesting internal val providers: List, - private val coroutineScope: CoroutineScope = CoroutineScope(SupervisorJob()), + coroutineScope: CoroutineScope = CoroutineScope(SupervisorJob()), ) { @Inject internal constructor(providers: Set<@JvmSuppressWildcards AccountProvider>) : @@ -33,7 +33,7 @@ class GodToolsAccountManager @VisibleForTesting internal constructor( // region Active Provider @VisibleForTesting - internal 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 - fun isAuthenticated() = activeProvider()?.isAuthenticated() ?: false - 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() } diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt index 0f1a405eff..7038298c9b 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/AccountProvider.kt +++ b/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 - fun isAuthenticated(): Boolean - fun userId(): String? + val isAuthenticated: Boolean + val userId: String? fun isAuthenticatedFlow(): Flow fun userIdFlow(): Flow diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt index f1a9f9a498..6e43788ac8 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt +++ b/library/account/src/main/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProvider.kt @@ -46,8 +46,8 @@ internal class FacebookAccountProvider @Inject constructor( @VisibleForTesting internal val prefs by lazy { context.getSharedPreferences(PREFS_FACEBOOK_ACCOUNT_PROVIDER, Context.MODE_PRIVATE) } - override fun userId() = accessTokenManager.currentAccessToken?.let { prefs.getString(PREF_USER_ID(it), null) } - override 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() diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt b/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt index 55788992da..250cf6608c 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/provider/google/GoogleAccountProvider.kt +++ b/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 fun isAuthenticated() = GoogleSignIn.getLastSignedInAccount(context) != null - override 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) diff --git a/library/account/src/test/kotlin/org/cru/godtools/account/GodToolsAccountManagerTest.kt b/library/account/src/test/kotlin/org/cru/godtools/account/GodToolsAccountManagerTest.kt index 61ba2af15b..2b0721bbdf 100644 --- a/library/account/src/test/kotlin/org/cru/godtools/account/GodToolsAccountManagerTest.kt +++ b/library/account/src/test/kotlin/org/cru/godtools/account/GodToolsAccountManagerTest.kt @@ -25,12 +25,12 @@ class GodToolsAccountManagerTest { private val provider1 = mockk(relaxed = true) { every { order } returns 1 - coEvery { isAuthenticated() } answers { provider1Authenticated.value } + coEvery { isAuthenticated } answers { provider1Authenticated.value } every { isAuthenticatedFlow() } returns provider1Authenticated } private val provider2 = mockk(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 diff --git a/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt b/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt index 392980ab6e..a1beb2cdc7 100644 --- a/library/account/src/test/kotlin/org/cru/godtools/account/provider/facebook/FacebookAccountProviderTest.kt +++ b/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)) } diff --git a/library/analytics/src/main/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsService.kt b/library/analytics/src/main/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsService.kt index 32855f840b..905675e1de 100644 --- a/library/analytics/src/main/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsService.kt +++ b/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) diff --git a/library/analytics/src/test/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsServiceTest.kt b/library/analytics/src/test/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsServiceTest.kt index f7d9254db5..0115b312c0 100644 --- a/library/analytics/src/test/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsServiceTest.kt +++ b/library/analytics/src/test/kotlin/org/cru/godtools/analytics/firebase/FirebaseAnalyticsServiceTest.kt @@ -27,7 +27,7 @@ class FirebaseAnalyticsServiceTest { private val userFlow = MutableSharedFlow() 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) diff --git a/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasks.kt b/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasks.kt index 5a17bff6cf..bff86ba431 100644 --- a/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasks.kt +++ b/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 diff --git a/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasks.kt b/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasks.kt index 1b37f1b2ca..357f2c3041 100644 --- a/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasks.kt +++ b/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 } diff --git a/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserSyncTasks.kt b/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserSyncTasks.kt index c50f6615db..f9b0edbfb0 100644 --- a/library/sync/src/main/kotlin/org/cru/godtools/sync/task/UserSyncTasks.kt +++ b/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 && diff --git a/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasksTest.kt b/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasksTest.kt index 376d1e3c63..8a8f7a5fbe 100644 --- a/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserCounterSyncTasksTest.kt +++ b/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()) diff --git a/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasksTest.kt b/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasksTest.kt index 3129f98d83..90f3767095 100644 --- a/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasksTest.kt +++ b/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserFavoriteToolsSyncTasksTest.kt @@ -32,8 +32,8 @@ class UserFavoriteToolsSyncTasksTest { private val userId = UUID.randomUUID().toString() private val accountManager: GodToolsAccountManager = mockk { - coEvery { isAuthenticated() } returns true - coEvery { userId() } returns userId + coEvery { isAuthenticated } returns true + coEvery { userId } returns this@UserFavoriteToolsSyncTasksTest.userId } private val favoritesApi: UserFavoriteToolsApi = mockk { coEvery { addFavoriteTools(any(), any()) } returns Response.success(JsonApiObject.of()) @@ -71,11 +71,11 @@ class UserFavoriteToolsSyncTasksTest { // region syncFavoriteTools() @Test fun `syncFavoriteTools() - not authenticated`() = runTest { - coEvery { accountManager.isAuthenticated() } returns false + coEvery { accountManager.isAuthenticated } returns false assertTrue(tasks.syncFavoriteTools(Random.nextBoolean())) coVerifyAll { - accountManager.isAuthenticated() + accountManager.isAuthenticated lastSyncTimeRepository wasNot Called userApi wasNot Called syncRepository wasNot Called @@ -90,8 +90,8 @@ class UserFavoriteToolsSyncTasksTest { assertTrue(tasks.syncFavoriteTools(force = false)) coVerifyAll { - accountManager.isAuthenticated() - accountManager.userId() + accountManager.isAuthenticated + accountManager.userId lastSyncTimeRepository.isLastSyncStale(SYNC_TIME_FAVORITE_TOOLS, userId, staleAfter = any()) userApi wasNot Called } @@ -105,8 +105,8 @@ class UserFavoriteToolsSyncTasksTest { assertTrue(tasks.syncFavoriteTools(force = true)) coVerifySequence { - accountManager.isAuthenticated() - accountManager.userId() + accountManager.isAuthenticated + accountManager.userId userApi.getUser(any()) syncRepository.storeUser(user, any()) lastSyncTimeRepository.resetLastSyncTime(SYNC_TIME_FAVORITE_TOOLS, isPrefix = true) @@ -216,11 +216,11 @@ class UserFavoriteToolsSyncTasksTest { @Test fun `syncDirtyFavoriteTools() - not authenticated`() = runTest { - coEvery { accountManager.isAuthenticated() } returns false + coEvery { accountManager.isAuthenticated } returns false assertTrue(tasks.syncDirtyFavoriteTools()) coVerifySequence { - accountManager.isAuthenticated() + accountManager.isAuthenticated userRepository wasNot Called userApi wasNot Called diff --git a/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserSyncTasksTest.kt b/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserSyncTasksTest.kt index cd90bb3fdb..8d44d2f5df 100644 --- a/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserSyncTasksTest.kt +++ b/library/sync/src/test/kotlin/org/cru/godtools/sync/task/UserSyncTasksTest.kt @@ -25,8 +25,8 @@ private const val USER_ID = "user_id" class UserSyncTasksTest { 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 userApi: UserApi = mockk() private val lastSyncTimeRepository = spyk(InMemoryLastSyncTimeRepository()) { @@ -44,11 +44,11 @@ class UserSyncTasksTest { // region syncCounters() @Test fun `syncUser() - not authenticated`() = runTest { - coEvery { accountManager.isAuthenticated() } returns false + coEvery { accountManager.isAuthenticated } returns false assertTrue(tasks.syncUser(Random.nextBoolean())) coVerifyAll { - accountManager.isAuthenticated() + accountManager.isAuthenticated userApi wasNot Called lastSyncTimeRepository wasNot Called } @@ -62,8 +62,8 @@ class UserSyncTasksTest { assertTrue(tasks.syncUser(force = false)) coVerifyAll { - accountManager.isAuthenticated() - accountManager.userId() + accountManager.isAuthenticated + accountManager.userId lastSyncTimeRepository.isLastSyncStale(SYNC_TIME_USER, USER_ID, staleAfter = any()) userApi wasNot Called } @@ -78,8 +78,8 @@ class UserSyncTasksTest { assertTrue(tasks.syncUser(force = true)) coVerifyAll { - accountManager.isAuthenticated() - accountManager.userId() + accountManager.isAuthenticated + accountManager.userId userApi.getUser(any()) syncRepository.storeUser(user, any()) lastSyncTimeRepository.updateLastSyncTime(SYNC_TIME_USER, USER_ID) @@ -97,8 +97,8 @@ class UserSyncTasksTest { assertTrue(tasks.syncUser(force = true)) coVerifyAll { - accountManager.isAuthenticated() - accountManager.userId() + accountManager.isAuthenticated + accountManager.userId userApi.getUser(any()) syncRepository.storeUser(user, any()) lastSyncTimeRepository.updateLastSyncTime(SYNC_TIME_USER, USER_ID) diff --git a/library/user-data/src/main/kotlin/org/cru/godtools/user/data/UserManager.kt b/library/user-data/src/main/kotlin/org/cru/godtools/user/data/UserManager.kt index f96bd069ed..4bd22a78da 100644 --- a/library/user-data/src/main/kotlin/org/cru/godtools/user/data/UserManager.kt +++ b/library/user-data/src/main/kotlin/org/cru/godtools/user/data/UserManager.kt @@ -21,7 +21,7 @@ class UserManager @Inject internal constructor( ) { private val coroutineScope = CoroutineScope(SupervisorJob()) - val userFlow = accountManager.userIdFlow() + val userFlow = accountManager.userIdFlow .flatMapLatest { it?.let { userRepository.findUserFlow(it) } ?: flowOf(null) } .shareIn(coroutineScope, SharingStarted.WhileSubscribed(5_000), 1) .distinctUntilChanged() diff --git a/library/user-data/src/test/kotlin/org/cru/godtools/user/data/UserManagerTest.kt b/library/user-data/src/test/kotlin/org/cru/godtools/user/data/UserManagerTest.kt index 92d07fb214..30e70f59b7 100644 --- a/library/user-data/src/test/kotlin/org/cru/godtools/user/data/UserManagerTest.kt +++ b/library/user-data/src/test/kotlin/org/cru/godtools/user/data/UserManagerTest.kt @@ -21,7 +21,7 @@ class UserManagerTest { private val userFlow = MutableStateFlow(null) private val accountManager: GodToolsAccountManager = mockk { - coEvery { userIdFlow() } returns userIdFlow + coEvery { userIdFlow } returns this@UserManagerTest.userIdFlow } private val userRepository: UserRepository = mockk { every { findUserFlow(USER_ID) } returns userFlow From f214c3f70c8f8f062ec6f2fd5ff880711f1b2b03 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Thu, 12 Oct 2023 13:56:00 -0600 Subject: [PATCH 3/3] there is no need to make SessionInterceptor.userId() suspendable --- .../src/main/kotlin/org/cru/godtools/account/AccountModule.kt | 2 +- .../cru/godtools/api/MobileContentApiSessionInterceptor.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt b/library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt index 91e6a86850..265ad9008b 100644 --- a/library/account/src/main/kotlin/org/cru/godtools/account/AccountModule.kt +++ b/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() } } diff --git a/library/api/src/main/kotlin/org/cru/godtools/api/MobileContentApiSessionInterceptor.kt b/library/api/src/main/kotlin/org/cru/godtools/api/MobileContentApiSessionInterceptor.kt index b2fe0b16e1..26408b9fe4 100644 --- a/library/api/src/main/kotlin/org/cru/godtools/api/MobileContentApiSessionInterceptor.kt +++ b/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(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? }