From 4e2dcd0b7518c5fb84d01036c2b196f3c1363473 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 12 Nov 2024 18:19:15 +0000 Subject: [PATCH] dataconnect: DataConnectCredentialsTokenManager: initialize synchronously to fix race condition (#6448) --- .../dataconnect/AuthIntegrationTest.kt | 2 + .../dataconnect/core/DataConnectAppCheck.kt | 16 +- .../dataconnect/core/DataConnectAuth.kt | 12 +- .../DataConnectCredentialsTokenManager.kt | 196 +++++------------- .../core/FirebaseDataConnectImpl.kt | 13 +- .../core/DataConnectAuthUnitTest.kt | 59 +----- 6 files changed, 80 insertions(+), 218 deletions(-) diff --git a/firebase-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/AuthIntegrationTest.kt b/firebase-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/AuthIntegrationTest.kt index e4dd0ad20ad..ba314d0f9db 100644 --- a/firebase-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/AuthIntegrationTest.kt +++ b/firebase-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/AuthIntegrationTest.kt @@ -17,6 +17,7 @@ package com.google.firebase.dataconnect import com.google.firebase.auth.FirebaseAuth +import com.google.firebase.dataconnect.core.FirebaseDataConnectInternal import com.google.firebase.dataconnect.testutil.DataConnectBackend import com.google.firebase.dataconnect.testutil.DataConnectIntegrationTestBase import com.google.firebase.dataconnect.testutil.InProcessDataConnectGrpcServer @@ -201,6 +202,7 @@ class AuthIntegrationTest : DataConnectIntegrationTestBase() { } private suspend fun signIn() { + (personSchema.dataConnect as FirebaseDataConnectInternal).awaitAuthReady() val authResult = auth.run { signInAnonymously().await() } withClue("authResult.user returned from signInAnonymously()") { authResult.user.shouldNotBeNull() diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAppCheck.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAppCheck.kt index 176aba53832..abe60656f73 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAppCheck.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAppCheck.kt @@ -32,24 +32,20 @@ internal class DataConnectAppCheck( blockingDispatcher: CoroutineDispatcher, logger: Logger, ) : - DataConnectCredentialsTokenManager( + DataConnectCredentialsTokenManager( deferredProvider = deferredAppCheckTokenProvider, parentCoroutineScope = parentCoroutineScope, blockingDispatcher = blockingDispatcher, logger = logger, ) { - override fun newTokenListener(): AppCheckTokenListener = AppCheckTokenListenerImpl(logger) + private val appCheckTokenListener = AppCheckTokenListenerImpl(logger) @DeferredApi - override fun addTokenListener( - provider: InteropAppCheckTokenProvider, - listener: AppCheckTokenListener - ) = provider.addAppCheckTokenListener(listener) + override fun addTokenListener(provider: InteropAppCheckTokenProvider) = + provider.addAppCheckTokenListener(appCheckTokenListener) - override fun removeTokenListener( - provider: InteropAppCheckTokenProvider, - listener: AppCheckTokenListener - ) = provider.removeAppCheckTokenListener(listener) + override fun removeTokenListener(provider: InteropAppCheckTokenProvider) = + provider.removeAppCheckTokenListener(appCheckTokenListener) override suspend fun getToken(provider: InteropAppCheckTokenProvider, forceRefresh: Boolean) = provider.getToken(forceRefresh).await().let { GetTokenResult(it.token) } diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAuth.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAuth.kt index 1efd00af83e..a3a27ccc31c 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAuth.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectAuth.kt @@ -32,20 +32,20 @@ internal class DataConnectAuth( blockingDispatcher: CoroutineDispatcher, logger: Logger, ) : - DataConnectCredentialsTokenManager( + DataConnectCredentialsTokenManager( deferredProvider = deferredAuthProvider, parentCoroutineScope = parentCoroutineScope, blockingDispatcher = blockingDispatcher, logger = logger, ) { - override fun newTokenListener(): IdTokenListener = IdTokenListenerImpl(logger) + private val idTokenListener = IdTokenListenerImpl(logger) @DeferredApi - override fun addTokenListener(provider: InternalAuthProvider, listener: IdTokenListener) = - provider.addIdTokenListener(listener) + override fun addTokenListener(provider: InternalAuthProvider) = + provider.addIdTokenListener(idTokenListener) - override fun removeTokenListener(provider: InternalAuthProvider, listener: IdTokenListener) = - provider.removeIdTokenListener(listener) + override fun removeTokenListener(provider: InternalAuthProvider) = + provider.removeIdTokenListener(idTokenListener) override suspend fun getToken(provider: InternalAuthProvider, forceRefresh: Boolean) = provider.getAccessToken(forceRefresh).await().let { GetTokenResult(it.token) } diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt index 61b0f9bd769..e6f4049c359 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt @@ -52,10 +52,10 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.yield /** Base class that shares logic for managing the Auth token and AppCheck token. */ -internal sealed class DataConnectCredentialsTokenManager( +internal sealed class DataConnectCredentialsTokenManager( private val deferredProvider: com.google.firebase.inject.Deferred, parentCoroutineScope: CoroutineScope, - private val blockingDispatcher: CoroutineDispatcher, + blockingDispatcher: CoroutineDispatcher, protected val logger: Logger, ) { val instanceId: String @@ -78,24 +78,26 @@ internal sealed class DataConnectCredentialsTokenManager( } ) - private interface ProviderListenerPair { - val provider: T? - val tokenListener: L + init { + // Call `whenAvailable()` on a non-main thread because it accesses SharedPreferences, which + // performs disk i/o, violating the StrictMode policy android.os.strictmode.DiskReadViolation. + val coroutineName = CoroutineName("k6rwgqg9gh $instanceId whenAvailable") + coroutineScope.launch(coroutineName + blockingDispatcher) { + deferredProvider.whenAvailable(DeferredProviderHandlerImpl(weakThis)) + } } - private sealed interface State { + private interface ProviderProvider { + val provider: T? + } - /** State indicating that [initialize] has not yet been invoked. */ - object Uninitialized : State + private sealed interface State { /** State indicating that [close] has been invoked. */ - object Closed : State + object Closed : State - /** - * State indicating that [initialize] has been invoked and there is no outstanding "get token" - * request. - */ - class Ready( + /** State indicating that there is no outstanding "get token" request. */ + class Idle( /** * The [InternalAuthProvider] or [InteropAppCheckTokenProvider]; may be null if the deferred @@ -103,18 +105,12 @@ internal sealed class DataConnectCredentialsTokenManager( */ override val provider: T?, - /** The token listener that is, or will be, registered with [provider]. */ - override val tokenListener: L, - /** The value to specify for `forceRefresh` on the next invocation of [getToken]. */ val forceTokenRefresh: Boolean - ) : State, ProviderListenerPair + ) : State, ProviderProvider - /** - * State indicating that [initialize] has been invoked and there _is_ an outstanding "get token" - * request. - */ - class Active( + /** State indicating that there _is_ an outstanding "get token" request. */ + class Active( /** * The [InternalAuthProvider] or [InteropAppCheckTokenProvider] that is performing the "get @@ -122,12 +118,9 @@ internal sealed class DataConnectCredentialsTokenManager( */ override val provider: T, - /** The token listener that is registered with [provider]. */ - override val tokenListener: L, - /** The job that is performing the "get token" request. */ val job: Deferred>> - ) : State, ProviderListenerPair + ) : State, ProviderProvider } /** @@ -135,30 +128,22 @@ internal sealed class DataConnectCredentialsTokenManager( * in order to be thread-safe. Such a loop should call `yield()` on each iteration to allow other * coroutines to run on the thread. */ - private val state = AtomicReference>(State.Uninitialized) - - /** - * Creates and returns a new "token listener" that will be registered with the provider returned - * from the [deferredProvider] specified to the constructor. - * - * @see addTokenListener - * @see removeTokenListener - */ - protected abstract fun newTokenListener(): L + private val state = + AtomicReference>(State.Idle(provider = null, forceTokenRefresh = false)) /** - * Adds the given listener to the given provider. + * Adds the token listener to the given provider. * * @see removeTokenListener */ - @DeferredApi protected abstract fun addTokenListener(provider: T, listener: L) + @DeferredApi protected abstract fun addTokenListener(provider: T) /** - * Removes the given listener from the given provider. + * Removes the token listener from the given provider. * * @see addTokenListener */ - protected abstract fun removeTokenListener(provider: T, listener: L) + protected abstract fun removeTokenListener(provider: T) /** * Starts an asynchronous task to get a new access token from the given provider, forcing a token @@ -166,45 +151,6 @@ internal sealed class DataConnectCredentialsTokenManager( */ protected abstract suspend fun getToken(provider: T, forceRefresh: Boolean): GetTokenResult - /** - * Initializes this object, acquiring resources and registering necessary listeners. - * - * This method must be called exactly once before any other methods on this object. The only - * exception is that [close] may be invoked without having invoked this method. - * - * @throws IllegalStateException if invoked more than once or after [close]. - * - * @see close - */ - fun initialize() { - val newState = - State.Ready(provider = null, tokenListener = newTokenListener(), forceTokenRefresh = false) - - while (true) { - val oldState = state.get() - if (oldState != State.Uninitialized) { - throw IllegalStateException( - if (oldState == State.Closed) { - "initialize() may not be called after close()" - } else { - "initialize() has already been called" - } - ) - } - - if (state.compareAndSet(oldState, newState)) { - break - } - } - - // Call `whenAvailable()` on a non-main thread because it accesses SharedPreferences, which - // performs disk i/o, violating the StrictMode policy android.os.strictmode.DiskReadViolation. - val coroutineName = CoroutineName("k6rwgqg9gh $instanceId whenAvailable") - coroutineScope.launch(coroutineName + blockingDispatcher) { - deferredProvider.whenAvailable(DeferredProviderHandlerImpl(weakThis, newState.tokenListener)) - } - } - /** * Closes this object, releasing its resources, unregistering any registered listeners, and * cancelling any in-flight token requests. @@ -226,19 +172,16 @@ internal sealed class DataConnectCredentialsTokenManager( private fun setClosedState() { while (true) { val oldState = state.get() - val providerListenerPair: ProviderListenerPair? = + val providerProvider: ProviderProvider = when (oldState) { is State.Closed -> return - is State.Uninitialized -> null - is State.Ready -> oldState + is State.Idle -> oldState is State.Active -> oldState } if (state.compareAndSet(oldState, State.Closed)) { - providerListenerPair?.run { - provider?.let { provider -> removeTokenListener(provider, tokenListener) } - } - return + providerProvider.provider?.let { removeTokenListener(it) } + break } } } @@ -247,19 +190,15 @@ internal sealed class DataConnectCredentialsTokenManager( * Sets a flag to force-refresh the token upon the next call to [getToken]. * * If [close] has been called, this method does nothing. - * - * @throws IllegalStateException if [initialize] has not been called. */ suspend fun forceRefresh() { logger.debug { "forceRefresh()" } while (true) { val oldState = state.get() - val providerListenerPair: ProviderListenerPair = + val oldStateProviderProvider = when (oldState) { - is State.Uninitialized -> - throw IllegalStateException("forceRefresh() cannot be called before initialize()") is State.Closed -> return - is State.Ready -> oldState + is State.Idle -> oldState is State.Active -> { val message = "needs token refresh (wgrwbrvjxt)" oldState.job.cancel(message, ForceRefresh(message)) @@ -267,12 +206,7 @@ internal sealed class DataConnectCredentialsTokenManager( } } - val newState = - State.Ready( - providerListenerPair.provider, - providerListenerPair.tokenListener, - forceTokenRefresh = true - ) + val newState = State.Idle(oldStateProviderProvider.provider, forceTokenRefresh = true) if (state.compareAndSet(oldState, newState)) { break } @@ -284,9 +218,8 @@ internal sealed class DataConnectCredentialsTokenManager( private fun newActiveState( invocationId: String, provider: T, - tokenListener: L, forceRefresh: Boolean - ): State.Active { + ): State.Active { val coroutineName = CoroutineName( "$instanceId 535gmcvv5a $invocationId getToken(" + @@ -299,15 +232,14 @@ internal sealed class DataConnectCredentialsTokenManager( val result = runCatching { getToken(provider, forceRefresh) } SequencedReference(sequenceNumber, result) } - return State.Active(provider, tokenListener, job) + return State.Active(provider, job) } /** * Gets the access token, force-refreshing it if [forceRefresh] has been called. * - * @throws IllegalStateException if [initialize] has not been called. - * @throws DataConnectException if [close] has not been called or is called while the operation is - * in progress. + * @throws DataConnectException if [close] has been called or is called while the operation is in + * progress. */ suspend fun getToken(requestId: String): String? { val invocationId = "gat" + Random.nextAlphanumericString(length = 8) @@ -316,10 +248,8 @@ internal sealed class DataConnectCredentialsTokenManager( val attemptSequenceNumber = nextSequenceNumber() val oldState = state.get() - val newState: State.Active = + val newState: State.Active = when (oldState) { - is State.Uninitialized -> - throw IllegalStateException("getToken() cannot be called before initialize()") is State.Closed -> { logger.debug { "$invocationId getToken() throws CredentialsTokenManagerClosedException" + @@ -327,20 +257,14 @@ internal sealed class DataConnectCredentialsTokenManager( } throw CredentialsTokenManagerClosedException(this) } - is State.Ready -> { + is State.Idle -> { if (oldState.provider === null) { logger.debug { - "$invocationId getToken() returns null" + - " (token provider is not (yet?) available)" + "$invocationId getToken() returns null (token provider is not (yet?) available)" } return null } - newActiveState( - invocationId, - oldState.provider, - oldState.tokenListener, - oldState.forceTokenRefresh - ) + newActiveState(invocationId, oldState.provider, oldState.forceTokenRefresh) } is State.Active -> { if ( @@ -348,12 +272,7 @@ internal sealed class DataConnectCredentialsTokenManager( !oldState.job.isCancelled && oldState.job.await().sequenceNumber < attemptSequenceNumber ) { - newActiveState( - invocationId, - oldState.provider, - oldState.tokenListener, - forceRefresh = false - ) + newActiveState(invocationId, oldState.provider, forceRefresh = false) } else { oldState } @@ -390,7 +309,7 @@ internal sealed class DataConnectCredentialsTokenManager( continue } else if (exception is FirebaseNoSignedInUserException) { logger.debug { - "$invocationId getToken() returns null" + " (FirebaseAuth reports no signed-in user)" + "$invocationId getToken() returns null (FirebaseAuth reports no signed-in user)" } return null } else if (exception is CancellationException) { @@ -418,33 +337,28 @@ internal sealed class DataConnectCredentialsTokenManager( private class NewProvider(message: String) : GetTokenRetry(message) @DeferredApi - private fun onProviderAvailable(newProvider: T, tokenListener: L) { + private fun onProviderAvailable(newProvider: T) { logger.debug { "onProviderAvailable(newProvider=$newProvider)" } - addTokenListener(newProvider, tokenListener) + addTokenListener(newProvider) while (true) { val oldState = state.get() val newState = when (oldState) { - is State.Uninitialized -> - throw IllegalStateException( - "INTERNAL ERROR: onProviderAvailable() called before initialize()" - ) is State.Closed -> { logger.debug { "onProviderAvailable(newProvider=$newProvider)" + " unregistering token listener that was just added" } - removeTokenListener(newProvider, tokenListener) + removeTokenListener(newProvider) break } - is State.Ready -> - State.Ready(newProvider, oldState.tokenListener, oldState.forceTokenRefresh) + is State.Idle -> State.Idle(newProvider, oldState.forceTokenRefresh) is State.Active -> { val newProviderClassName = newProvider::class.qualifiedName val message = "a new provider $newProviderClassName is available (symhxtmazy)" oldState.job.cancel(message, NewProvider(message)) - State.Ready(newProvider, tokenListener, forceTokenRefresh = false) + State.Idle(newProvider, forceTokenRefresh = false) } } @@ -464,25 +378,23 @@ internal sealed class DataConnectCredentialsTokenManager( * strong reference to the [DataConnectCredentialsTokenManager] instance indefinitely, in the case * that the callback never occurs. */ - private class DeferredProviderHandlerImpl( - private val weakCredentialsTokenManagerRef: - WeakReference>, - private val tokenListener: L, + private class DeferredProviderHandlerImpl( + private val weakCredentialsTokenManagerRef: WeakReference> ) : DeferredHandler { override fun handle(provider: Provider) { - weakCredentialsTokenManagerRef.get()?.onProviderAvailable(provider.get(), tokenListener) + weakCredentialsTokenManagerRef.get()?.onProviderAvailable(provider.get()) } } private class CredentialsTokenManagerClosedException( - tokenProvider: DataConnectCredentialsTokenManager<*, *> + tokenProvider: DataConnectCredentialsTokenManager<*> ) : DataConnectException( - "DataConnectCredentialsTokenManager ${tokenProvider.instanceId} was closed" + "DataConnectCredentialsTokenManager ${tokenProvider.instanceId} was closed (code cqrbq4zfvy)" ) private class GetTokenCancelledException(cause: Throwable) : - DataConnectException("getToken() was cancelled, likely by close()", cause) + DataConnectException("getToken() was cancelled, likely by close() (code rqdd4jam9d)", cause) protected data class GetTokenResult(val token: String?) diff --git a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt index b04b6aa6ead..d9bee50b89d 100644 --- a/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt +++ b/firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/core/FirebaseDataConnectImpl.kt @@ -86,9 +86,8 @@ internal class FirebaseDataConnectImpl( override val config: ConnectorConfig, override val blockingExecutor: Executor, override val nonBlockingExecutor: Executor, - private val deferredAuthProvider: com.google.firebase.inject.Deferred, - private val deferredAppCheckProvider: - com.google.firebase.inject.Deferred, + deferredAuthProvider: com.google.firebase.inject.Deferred, + deferredAppCheckProvider: com.google.firebase.inject.Deferred, private val creator: FirebaseDataConnectFactory, override val settings: DataConnectSettings, ) : FirebaseDataConnectInternal { @@ -146,8 +145,8 @@ internal class FirebaseDataConnectImpl( } init { - coroutineScope.launch(CoroutineName("DataConnectAuth initializer for $instanceId")) { - dataConnectAuth.initialize() + val name = CoroutineName("DataConnectAuth isProviderAvailable pipe for $instanceId") + coroutineScope.launch(name) { dataConnectAuth.providerAvailable.collect { isProviderAvailable -> logger.debug { "authProviderAvailable=$isProviderAvailable" } authProviderAvailable.value = isProviderAvailable @@ -168,8 +167,8 @@ internal class FirebaseDataConnectImpl( } init { - coroutineScope.launch(CoroutineName("DataConnectAppCheck initializer for $instanceId")) { - dataConnectAppCheck.initialize() + val name = CoroutineName("DataConnectAppCheck isProviderAvailable pipe for $instanceId") + coroutineScope.launch(name) { dataConnectAppCheck.providerAvailable.collect { isProviderAvailable -> logger.debug { "appCheckProviderAvailable=$isProviderAvailable" } appCheckProviderAvailable.value = isProviderAvailable diff --git a/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt b/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt index b45a943a1f4..a30121a707f 100644 --- a/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt +++ b/firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt @@ -32,6 +32,7 @@ import com.google.firebase.dataconnect.testutil.UnavailableDeferred import com.google.firebase.dataconnect.testutil.newBackgroundScopeThatAdvancesLikeForeground import com.google.firebase.dataconnect.testutil.newMockLogger import com.google.firebase.dataconnect.testutil.property.arbitrary.dataConnect +import com.google.firebase.dataconnect.testutil.shouldContainWithNonAbuttingTextIgnoringCase import com.google.firebase.dataconnect.testutil.shouldHaveLoggedAtLeastOneMessageContaining import com.google.firebase.dataconnect.testutil.shouldHaveLoggedExactlyOneMessageContaining import com.google.firebase.dataconnect.testutil.shouldNotHaveLoggedAnyMessagesContaining @@ -47,7 +48,6 @@ import io.kotest.matchers.collections.shouldContain import io.kotest.matchers.collections.shouldContainExactly import io.kotest.matchers.nulls.shouldBeNull import io.kotest.matchers.shouldBe -import io.kotest.matchers.throwable.shouldHaveMessage import io.kotest.matchers.types.shouldBeSameInstanceAs import io.kotest.property.Arb import io.kotest.property.RandomSource @@ -93,23 +93,14 @@ class DataConnectAuthUnitTest { private val mockLogger = newMockLogger("ecvqkga56c") @Test - fun `close() should succeed if called _before_ initialize()`() = runTest { + fun `close() should succeed if called on a brand new instance()`() = runTest { val dataConnectAuth = newDataConnectAuth() dataConnectAuth.close() } - @Test - fun `close() should succeed if called _after_ initialize()`() = runTest { - val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() - - dataConnectAuth.close() - } - @Test fun `close() should log a message`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() dataConnectAuth.close() @@ -119,7 +110,6 @@ class DataConnectAuthUnitTest { @Test fun `close() should cancel in-flight requests to get a token`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } coAnswers @@ -133,7 +123,8 @@ class DataConnectAuthUnitTest { val exception = shouldThrow { dataConnectAuth.getToken(requestId) } - exception shouldHaveMessage "getToken() was cancelled, likely by close()" + exception.message shouldContainWithNonAbuttingTextIgnoringCase "getToken() was cancelled" + exception.message shouldContainWithNonAbuttingTextIgnoringCase "likely by close()" mockLogger.shouldHaveLoggedExactlyOneMessageContaining(requestId) mockLogger.shouldHaveLoggedExactlyOneMessageContaining("throws GetTokenCancelledException") } @@ -141,7 +132,6 @@ class DataConnectAuthUnitTest { @Test fun `close() should remove the IdTokenListener`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() val idTokenListenerSlot = slot() @@ -156,7 +146,6 @@ class DataConnectAuthUnitTest { @Test fun `close() should be callable multiple times, from multiple threads`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() val latch = SuspendingCountDownLatch(100) @@ -175,15 +164,6 @@ class DataConnectAuthUnitTest { jobs.forEach { it.await() } } - @Test - fun `forceRefresh() should throw if invoked before initialize()`() = runTest { - val dataConnectAuth = newDataConnectAuth() - - val exception = shouldThrow { dataConnectAuth.forceRefresh() } - - exception shouldHaveMessage "forceRefresh() cannot be called before initialize()" - } - @Test fun `forceRefresh() should do nothing if invoked after close()`() = runTest { val dataConnectAuth = newDataConnectAuth() @@ -195,7 +175,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should return null if InternalAuthProvider is not available`() = runTest { val dataConnectAuth = newDataConnectAuth(deferredInternalAuthProvider = UnavailableDeferred()) - dataConnectAuth.initialize() advanceUntilIdle() val result = dataConnectAuth.getToken(requestId) @@ -206,15 +185,6 @@ class DataConnectAuthUnitTest { mockLogger.shouldHaveLoggedExactlyOneMessageContaining("token provider is not (yet?) available") } - @Test - fun `getToken() should throw if invoked before initialize()`() = runTest { - val dataConnectAuth = newDataConnectAuth() - - val exception = shouldThrow { dataConnectAuth.getToken(requestId) } - - exception shouldHaveMessage "getToken() cannot be called before initialize()" - } - @Test fun `getToken() should throw if invoked after close()`() = runTest { val dataConnectAuth = newDataConnectAuth() @@ -222,7 +192,7 @@ class DataConnectAuthUnitTest { val exception = shouldThrow { dataConnectAuth.getToken(requestId) } - exception shouldHaveMessage + exception.message shouldContainWithNonAbuttingTextIgnoringCase "DataConnectCredentialsTokenManager ${dataConnectAuth.instanceId} was closed" mockLogger.shouldHaveLoggedExactlyOneMessageContaining(requestId) mockLogger.shouldHaveLoggedExactlyOneMessageContaining( @@ -234,7 +204,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should return null if no user is signed in`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns Tasks.forException(FirebaseNoSignedInUserException("j8rkghbcnz")) @@ -250,7 +219,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should return the token returned from FirebaseAuth`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns taskForToken(accessToken) @@ -271,7 +239,6 @@ class DataConnectAuthUnitTest { val exception = TestException("xqtbckcn6w") val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns Tasks.forException(exception) @@ -287,13 +254,12 @@ class DataConnectAuthUnitTest { } @Test - fun `getToken() should return re-throw the exception thrown by InternalAuthProvider getAccessToken()`() = + fun `getToken() should re-throw the exception thrown by InternalAuthProvider getAccessToken()`() = runTest { class TestException(message: String) : Exception(message) val exception = TestException("s4c4xr9z4p") val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } answers { throw exception } @@ -310,7 +276,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should force refresh the access token after calling forceRefresh()`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns taskForToken(accessToken) @@ -332,7 +297,6 @@ class DataConnectAuthUnitTest { fun `getToken() should NOT force refresh the access token without calling forceRefresh()`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns taskForToken(accessToken) @@ -347,7 +311,6 @@ class DataConnectAuthUnitTest { fun `getToken() should NOT force refresh the access token after it is force refreshed`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns taskForToken(accessToken) @@ -365,7 +328,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should ask for a token from FirebaseAuth on every invocation`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() val tokens = CopyOnWriteArrayList() coEvery { mockInternalAuthProvider.getAccessToken(any()) } answers @@ -381,7 +343,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should conflate concurrent requests`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() val tokens = CopyOnWriteArrayList() coEvery { mockInternalAuthProvider.getAccessToken(any()) } answers @@ -411,7 +372,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should re-fetch token if invalidated concurrently`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() val invocationCount = AtomicInteger(0) val tokens = CopyOnWriteArrayList().apply { add(accessToken) } @@ -446,7 +406,6 @@ class DataConnectAuthUnitTest { @Test fun `getToken() should ignore results with lower sequence number`() = runTest { val dataConnectAuth = newDataConnectAuth() - dataConnectAuth.initialize() advanceUntilIdle() val invocationCount = AtomicInteger(0) val tokens = CopyOnWriteArrayList() @@ -488,7 +447,6 @@ class DataConnectAuthUnitTest { } val dataConnectAuth = newDataConnectAuth(deferredInternalAuthProvider = deferredInternalAuthProvider) - dataConnectAuth.initialize() advanceUntilIdle() val result = dataConnectAuth.getToken(requestId) @@ -509,7 +467,6 @@ class DataConnectAuthUnitTest { val deferredInternalAuthProvider: DeferredInternalAuthProvider = mockk(relaxed = true) val dataConnectAuth = newDataConnectAuth(deferredInternalAuthProvider = deferredInternalAuthProvider) - dataConnectAuth.initialize() advanceUntilIdle() dataConnectAuth.close() val deferredInternalAuthProviderHandlerSlot = slot>() @@ -531,7 +488,6 @@ class DataConnectAuthUnitTest { val deferredInternalAuthProvider = DelayedDeferred(mockInternalAuthProvider) val dataConnectAuth = newDataConnectAuth(deferredInternalAuthProvider = deferredInternalAuthProvider) - dataConnectAuth.initialize() advanceUntilIdle() every { mockInternalAuthProvider.addIdTokenListener(any()) } answers { @@ -571,9 +527,6 @@ class DataConnectAuthUnitTest { interval = 100.milliseconds } - val firebaseAppDeletedException - get() = java.lang.IllegalStateException("FirebaseApp was deleted") - fun taskForToken(token: String?): Task = Tasks.forResult(mockk(relaxed = true) { every { getToken() } returns token }) }