dataconnect: TestFirebaseAppFactory.kt: work around IllegalStateExcep…

…tion in tests by adding a delay before calling FirebaseApp.delete() (#6447)
firebase · Nov 8, 2024 · f20340a · f20340a
1 parent 5f6bc63
commit f20340a
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 57 deletions.
diff --git a/...stutil/src/main/kotlin/com/google/firebase/dataconnect/testutil/TestFirebaseAppFactory.kt b/...stutil/src/main/kotlin/com/google/firebase/dataconnect/testutil/TestFirebaseAppFactory.kt
@@ -22,6 +22,12 @@ import com.google.firebase.app
 import com.google.firebase.initialize
 import com.google.firebase.util.nextAlphanumericString
 import kotlin.random.Random
+import kotlin.time.Duration.Companion.seconds
+import kotlinx.coroutines.DelicateCoroutinesApi
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.GlobalScope
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 
 /**
  * A JUnit test rule that creates instances of [FirebaseApp] for use during testing, and closes them
@@ -37,6 +43,12 @@ class TestFirebaseAppFactory : FactoryTestRule<FirebaseApp, Nothing>() {
     )
 
   override fun destroyInstance(instance: FirebaseApp) {
-    instance.delete()
+    // Work around app crash due to IllegalStateException from FirebaseAuth if `delete()` is called
+    // very quickly after `FirebaseApp.getInstance()`. See b/378116261 for details.
+    @OptIn(DelicateCoroutinesApi::class)
+    GlobalScope.launch(Dispatchers.IO) {
+      delay(1.seconds)
+      instance.delete()
+    }
   }
 }
diff --git a/...rc/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt b/...rc/main/kotlin/com/google/firebase/dataconnect/core/DataConnectCredentialsTokenManager.kt
@@ -236,9 +236,7 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any, L : Any>(
 
       if (state.compareAndSet(oldState, State.Closed)) {
         providerListenerPair?.run {
-          provider?.let { provider ->
-            runIgnoringFirebaseAppDeleted { removeTokenListener(provider, tokenListener) }
-          }
+          provider?.let { provider -> removeTokenListener(provider, tokenListener) }
         }
         return
       }
@@ -422,7 +420,7 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any, L : Any>(
   @DeferredApi
   private fun onProviderAvailable(newProvider: T, tokenListener: L) {
     logger.debug { "onProviderAvailable(newProvider=$newProvider)" }
-    runIgnoringFirebaseAppDeleted { addTokenListener(newProvider, tokenListener) }
+    addTokenListener(newProvider, tokenListener)
 
     while (true) {
       val oldState = state.get()
@@ -437,7 +435,7 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any, L : Any>(
               "onProviderAvailable(newProvider=$newProvider)" +
                 " unregistering token listener that was just added"
             }
-            runIgnoringFirebaseAppDeleted { removeTokenListener(newProvider, tokenListener) }
+            removeTokenListener(newProvider, tokenListener)
             break
           }
           is State.Ready ->
@@ -486,20 +484,6 @@ internal sealed class DataConnectCredentialsTokenManager<T : Any, L : Any>(
   private class GetTokenCancelledException(cause: Throwable) :
     DataConnectException("getToken() was cancelled, likely by close()", cause)
 
-  // Work around a race condition where addIdTokenListener() and removeIdTokenListener() throw if
-  // the FirebaseApp is deleted during or before its invocation.
-  private fun runIgnoringFirebaseAppDeleted(block: () -> Unit) {
-    try {
-      block()
-    } catch (e: IllegalStateException) {
-      if (e.message == "FirebaseApp was deleted") {
-        logger.warn(e) { "ignoring exception: $e" }
-      } else {
-        throw e
-      }
-    }
-  }
-
   protected data class GetTokenResult(val token: String?)
 
   private companion object {

diff --git a/...taconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt b/...taconnect/src/test/kotlin/com/google/firebase/dataconnect/core/DataConnectAuthUnitTest.kt
@@ -552,43 +552,6 @@ class DataConnectAuthUnitTest {
       )
     }
 
-  @Test
-  fun `addIdTokenListener() throwing IllegalStateException due to FirebaseApp deleted should be ignored`() =
-    runTest {
-      every { mockInternalAuthProvider.addIdTokenListener(any()) } throws
-        firebaseAppDeletedException
-      coEvery { mockInternalAuthProvider.getAccessToken(any()) } returns taskForToken(accessToken)
-      val dataConnectAuth = newDataConnectAuth()
-      dataConnectAuth.initialize()
-      advanceUntilIdle()
-
-      eventually(`check every 100 milliseconds for 2 seconds`) {
-        mockLogger.shouldHaveLoggedExactlyOneMessageContaining(
-          "ignoring exception: $firebaseAppDeletedException"
-        )
-      }
-      val result = dataConnectAuth.getToken(requestId)
-      withClue("result=$result") { result shouldBe accessToken }
-    }
-
-  @Test
-  fun `removeIdTokenListener() throwing IllegalStateException due to FirebaseApp deleted should be ignored`() =
-    runTest {
-      every { mockInternalAuthProvider.removeIdTokenListener(any()) } throws
-        firebaseAppDeletedException
-      val dataConnectAuth = newDataConnectAuth()
-      dataConnectAuth.initialize()
-      advanceUntilIdle()
-
-      dataConnectAuth.close()
-
-      eventually(`check every 100 milliseconds for 2 seconds`) {
-        mockLogger.shouldHaveLoggedExactlyOneMessageContaining(
-          "ignoring exception: $firebaseAppDeletedException"
-        )
-      }
-    }
-
   private fun TestScope.newDataConnectAuth(
     deferredInternalAuthProvider: DeferredInternalAuthProvider =
       ImmediateDeferred(mockInternalAuthProvider),