Make remote settings fetch non-blocking

firebase-sessions/src/main/kotlin/com/google/firebase/sessions/FirebaseSessions.kt

@@ -48,13 +48,13 @@ internal constructor(
       applicationInfo,
     )
   private val timeProvider: TimeProvider = Time()
-  private val sessionGenerator =
-    SessionGenerator(collectEvents = shouldCollectEvents(), timeProvider)
+  private val sessionGenerator: SessionGenerator
   private val eventGDTLogger = EventGDTLogger(transportFactoryProvider)
   private val sessionCoordinator = SessionCoordinator(firebaseInstallations, eventGDTLogger)
 
   init {
     sessionSettings.updateSettings()
+    sessionGenerator = SessionGenerator(collectEvents = shouldCollectEvents(), timeProvider)
 
     val sessionInitiateListener =
       object : SessionInitiateListener {

firebase-sessions/src/main/kotlin/com/google/firebase/sessions/settings/RemoteSettings.kt

@@ -27,53 +27,41 @@ import kotlin.coroutines.CoroutineContext
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.seconds
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.tasks.await
 import org.json.JSONException
 import org.json.JSONObject
 
 internal class RemoteSettings(
   context: Context,
-  private val blockingDispatcher: CoroutineContext,
+  blockingDispatcher: CoroutineContext,
   private val backgroundDispatcher: CoroutineContext,
   private val firebaseInstallationsApi: FirebaseInstallationsApi,
   private val appInfo: ApplicationInfo,
-  private val configsFetcher: CrashlyticsSettingsFetcher = RemoteSettingsFetcher(appInfo),
-  dataStoreName: String = SESSION_CONFIGS_NAME
+  private val configsFetcher: CrashlyticsSettingsFetcher =
+    RemoteSettingsFetcher(appInfo, blockingDispatcher),
+  dataStoreName: String = SESSION_CONFIGS_NAME,
 ) : SettingsProvider {
-  private val Context.dataStore by preferencesDataStore(name = dataStoreName)
+  private val Context.dataStore by
+    preferencesDataStore(name = dataStoreName, scope = CoroutineScope(backgroundDispatcher))
   private val settingsCache = SettingsCache(context.dataStore)
   private var fetchInProgress = AtomicBoolean(false)
 
   override val sessionEnabled: Boolean?
-    get() {
-      return settingsCache.sessionsEnabled()
-    }
+    get() = settingsCache.sessionsEnabled()
 
   override val sessionRestartTimeout: Duration?
-    get() {
-      val durationInSeconds = settingsCache.sessionRestartTimeout()
-      if (durationInSeconds != null) {
-        return durationInSeconds.toLong().seconds
-      }
-      return null
-    }
+    get() = settingsCache.sessionRestartTimeout()?.seconds
 
   override val samplingRate: Double?
-    get() {
-      return settingsCache.sessionSamplingRate()
-    }
+    get() = settingsCache.sessionSamplingRate()
 
   override fun updateSettings() {
-    // TODO: Move to blocking coroutine dispatcher.
-    runBlocking(Dispatchers.Default) { launch { fetchConfigs() } }
+    val scope = CoroutineScope(backgroundDispatcher)
+    scope.launch { fetchConfigs() }
   }
 
-  override fun isSettingsStale(): Boolean {
-    return settingsCache.hasCacheExpired()
-  }
+  override fun isSettingsStale(): Boolean = settingsCache.hasCacheExpired()
 
   internal fun clearCachedSettings() {
     val scope = CoroutineScope(backgroundDispatcher)

firebase-sessions/src/main/kotlin/com/google/firebase/sessions/settings/RemoteSettingsFetcher.kt

@@ -19,10 +19,11 @@ package com.google.firebase.sessions.settings
 import android.net.Uri
 import com.google.firebase.sessions.ApplicationInfo
 import java.io.BufferedReader
-import java.io.IOException
 import java.io.InputStreamReader
 import java.net.URL
 import javax.net.ssl.HttpsURLConnection
+import kotlin.coroutines.CoroutineContext
+import kotlinx.coroutines.withContext
 import org.json.JSONObject
 
 internal interface CrashlyticsSettingsFetcher {
@@ -35,40 +36,43 @@ internal interface CrashlyticsSettingsFetcher {
 
 internal class RemoteSettingsFetcher(
   private val appInfo: ApplicationInfo,
-  private val baseUrl: String = FIREBASE_SESSIONS_BASE_URL_STRING
+  private val blockingDispatcher: CoroutineContext,
+  private val baseUrl: String = FIREBASE_SESSIONS_BASE_URL_STRING,
 ) : CrashlyticsSettingsFetcher {
+  @Suppress("BlockingMethodInNonBlockingContext") // blockingDispatcher is safe for blocking calls.
   override suspend fun doConfigFetch(
     headerOptions: Map<String, String>,
     onSuccess: suspend (JSONObject) -> Unit,
     onFailure: suspend (String) -> Unit
-  ) {
-    val connection = settingsUrl().openConnection() as HttpsURLConnection
-    connection.requestMethod = "GET"
-    connection.setRequestProperty("Accept", "application/json")
-    headerOptions.forEach { connection.setRequestProperty(it.key, it.value) }
+  ) =
+    withContext(blockingDispatcher) {
+      val connection = settingsUrl().openConnection() as HttpsURLConnection
+      connection.requestMethod = "GET"
+      connection.setRequestProperty("Accept", "application/json")
+      headerOptions.forEach { connection.setRequestProperty(it.key, it.value) }
 
-    try {
-      val responseCode = connection.responseCode
-      if (responseCode == HttpsURLConnection.HTTP_OK) {
-        val inputStream = connection.inputStream
-        val bufferedReader = BufferedReader(InputStreamReader(inputStream))
-        val response = StringBuilder()
-        var inputLine: String?
-        while (bufferedReader.readLine().also { inputLine = it } != null) {
-          response.append(inputLine)
-        }
-        bufferedReader.close()
-        inputStream.close()
+      try {
+        val responseCode = connection.responseCode
+        if (responseCode == HttpsURLConnection.HTTP_OK) {
+          val inputStream = connection.inputStream
+          val bufferedReader = BufferedReader(InputStreamReader(inputStream))
+          val response = StringBuilder()
+          var inputLine: String?
+          while (bufferedReader.readLine().also { inputLine = it } != null) {
+            response.append(inputLine)
+          }
+          bufferedReader.close()
+          inputStream.close()
 
-        val responseJson = JSONObject(response.toString())
-        onSuccess(responseJson)
-      } else {
-        onFailure("Bad response code: $responseCode")
+          val responseJson = JSONObject(response.toString())
+          onSuccess(responseJson)
+        } else {
+          onFailure("Bad response code: $responseCode")
+        }
+      } catch (ex: Exception) {
+        onFailure(ex.message ?: ex.toString())
       }
-    } catch (ex: IOException) {
-      onFailure(ex.message ?: ex.toString())
     }
-  }
 
   private fun settingsUrl(): URL {
     val uri =

firebase-sessions/src/test/kotlin/com/google/firebase/sessions/RemoteSettingsTest.kt

@@ -29,6 +29,7 @@ import com.google.firebase.sessions.testing.TestSessionEventData.TEST_APPLICATIO
 import kotlin.time.Duration.Companion.minutes
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.asCoroutineDispatcher
+import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import org.json.JSONObject
 import org.junit.After
@@ -63,6 +64,8 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = JSONObject(validResponse)
     remoteSettings.updateSettings()
 
+    runCurrent()
+
     assertThat(remoteSettings.sessionEnabled).isFalse()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -96,6 +99,8 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
+    runCurrent()
+
     assertThat(remoteSettings.sessionEnabled).isNull()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -128,6 +133,8 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
+    runCurrent()
+
     assertThat(remoteSettings.sessionEnabled).isFalse()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -142,6 +149,8 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
+    runCurrent()
+
     assertThat(remoteSettings.sessionEnabled).isTrue()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.25)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(20.minutes)
@@ -171,6 +180,8 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
+    runCurrent()
+
     assertThat(remoteSettings.sessionEnabled).isFalse()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -194,7 +205,11 @@ class RemoteSettingsTest {
   fun remoteSettingsFetcher_badFetch_callsOnFailure() = runTest {
     var failure: String? = null
 
-    RemoteSettingsFetcher(TEST_APPLICATION_INFO, baseUrl = "this.url.is.invalid")
+    RemoteSettingsFetcher(
+        TEST_APPLICATION_INFO,
+        TestOnlyExecutors.blocking().asCoroutineDispatcher() + coroutineContext,
+        baseUrl = "this.url.is.invalid",
+      )
       .doConfigFetch(
         headerOptions = emptyMap(),
         onSuccess = {},

firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionsSettingsTest.kt

@@ -26,16 +26,18 @@ import com.google.firebase.sessions.settings.SessionsSettings
 import com.google.firebase.sessions.testing.FakeFirebaseApp
 import com.google.firebase.sessions.testing.FakeFirebaseInstallations
 import com.google.firebase.sessions.testing.FakeRemoteConfigFetcher
-import kotlin.coroutines.coroutineContext
 import kotlin.time.Duration.Companion.minutes
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.asCoroutineDispatcher
+import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import org.json.JSONObject
 import org.junit.After
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
+@OptIn(ExperimentalCoroutinesApi::class)
 @RunWith(RobolectricTestRunner::class)
 class SessionsSettingsTest {
 
@@ -53,6 +55,9 @@ class SessionsSettingsTest {
         firebaseInstallations,
         SessionEvents.getApplicationInfo(firebaseApp)
       )
+
+    runCurrent()
+
     assertThat(sessionsSettings.sessionsEnabled).isTrue()
     assertThat(sessionsSettings.samplingRate).isEqualTo(1.0)
     assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(30.minutes)
@@ -76,6 +81,9 @@ class SessionsSettingsTest {
         firebaseInstallations,
         SessionEvents.getApplicationInfo(firebaseApp)
       )
+
+    runCurrent()
+
     assertThat(sessionsSettings.sessionsEnabled).isFalse()
     assertThat(sessionsSettings.samplingRate).isEqualTo(0.5)
     assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(3.minutes)
@@ -98,13 +106,16 @@ class SessionsSettingsTest {
         firebaseInstallations,
         SessionEvents.getApplicationInfo(firebaseApp)
       )
+
+    runCurrent()
+
     assertThat(sessionsSettings.sessionsEnabled).isFalse()
     assertThat(sessionsSettings.samplingRate).isEqualTo(0.5)
     assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(30.minutes)
   }
 
   @Test
-  fun sessionSettings_RemoteSettingsOverrideDefaultsWhenPresent() = runTest {
+  fun sessionSettings_remoteSettingsOverrideDefaultsWhenPresent() = runTest {
     val firebaseApp = FakeFirebaseApp().firebaseApp
     val context = firebaseApp.applicationContext
     val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
@@ -133,6 +144,9 @@ class SessionsSettingsTest {
         localOverrideSettings = LocalOverrideSettings(context),
         remoteSettings = remoteSettings
       )
+
+    runCurrent()
+
     assertThat(sessionsSettings.sessionsEnabled).isFalse()
     assertThat(sessionsSettings.samplingRate).isEqualTo(0.75)
     assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -141,7 +155,7 @@ class SessionsSettingsTest {
   }
 
   @Test
-  fun sessionSettings_ManifestOverridesRemoteSettingsAndDefaultsWhenPresent() = runTest {
+  fun sessionSettings_manifestOverridesRemoteSettingsAndDefaultsWhenPresent() = runTest {
     val metadata = Bundle()
     metadata.putBoolean("firebase_sessions_enabled", true)
     metadata.putDouble("firebase_sessions_sampling_rate", 0.5)
@@ -174,6 +188,9 @@ class SessionsSettingsTest {
         localOverrideSettings = LocalOverrideSettings(context),
         remoteSettings = remoteSettings
       )
+
+    runCurrent()
+
     assertThat(sessionsSettings.sessionsEnabled).isTrue()
     assertThat(sessionsSettings.samplingRate).isEqualTo(0.5)
     assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(3.minutes)