Merge b7ff899 into 5d56114

Author: mrober

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

@@ -35,11 +35,11 @@ import org.json.JSONException
 import org.json.JSONObject
 
 internal class RemoteSettings(
-  val context: Context,
-  val blockingDispatcher: CoroutineContext,
-  val backgroundDispatcher: CoroutineContext,
-  val firebaseInstallationsApi: FirebaseInstallationsApi,
-  val appInfo: ApplicationInfo,
+  context: Context,
+  private val 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
 ) : SettingsProvider {
@@ -80,7 +80,7 @@ internal class RemoteSettings(
     scope.launch { settingsCache.removeConfigs() }
   }
 
-  suspend private fun fetchConfigs() {
+  private suspend fun fetchConfigs() {
     // Check if a fetch is in progress. If yes, return
     if (fetchInProgress.get()) {
       return
@@ -94,7 +94,7 @@ internal class RemoteSettings(
     fetchInProgress.set(true)
 
     // Get the installations ID before making a remote config fetch
-    var installationId = firebaseInstallationsApi.id.await()
+    val installationId = firebaseInstallationsApi.id.await()
     if (installationId == null) {
       fetchInProgress.set(false)
       return
@@ -141,7 +141,6 @@ internal class RemoteSettings(
           }
         }
 
-        val scope = CoroutineScope(backgroundDispatcher)
         sessionsEnabled?.let { settingsCache.updateSettingsEnabled(sessionsEnabled) }
 
         sessionTimeoutSeconds?.let {
@@ -156,9 +155,9 @@ internal class RemoteSettings(
         settingsCache.updateSessionCacheUpdatedTime(System.currentTimeMillis())
         fetchInProgress.set(false)
       },
-      onFailure = {
+      onFailure = { msg ->
         // Network request failed here.
-        Log.e(TAG, "Error failing to fetch the remote configs")
+        Log.e(TAG, "Error failing to fetch the remote configs: $msg")
         fetchInProgress.set(false)
       }
     )

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

@@ -19,6 +19,7 @@ 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
@@ -27,46 +28,53 @@ import org.json.JSONObject
 internal interface CrashlyticsSettingsFetcher {
   suspend fun doConfigFetch(
     headerOptions: Map<String, String>,
-    onSuccess: suspend ((JSONObject)) -> Unit,
-    onFailure: suspend () -> Unit
+    onSuccess: suspend (JSONObject) -> Unit,
+    onFailure: suspend (msg: String) -> Unit
   )
 }
 
-internal class RemoteSettingsFetcher(val appInfo: ApplicationInfo) : CrashlyticsSettingsFetcher {
+internal class RemoteSettingsFetcher(
+  private val appInfo: ApplicationInfo,
+  private val baseUrl: String = FIREBASE_SESSIONS_BASE_URL_STRING
+) : CrashlyticsSettingsFetcher {
   override suspend fun doConfigFetch(
     headerOptions: Map<String, String>,
-    onSuccess: suspend ((JSONObject)) -> Unit,
-    onFailure: suspend () -> Unit
+    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) }
 
-    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()
+        val responseJson = JSONObject(response.toString())
+        onSuccess(responseJson)
+      } else {
+        onFailure("Bad response code: $responseCode")
+      }
+    } catch (ex: IOException) {
+      onFailure(ex.message ?: ex.toString())
     }
   }
 
-  fun settingsUrl(): URL {
-    var uri =
+  private fun settingsUrl(): URL {
+    val uri =
       Uri.Builder()
         .scheme("https")
-        .authority(FIREBASE_SESSIONS_BASE_URL_STRING)
+        .authority(baseUrl)
         .appendPath("spi")
         .appendPath("v2")
         .appendPath("platforms")

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

@@ -21,22 +21,26 @@ import com.google.common.truth.Truth.assertThat
 import com.google.firebase.FirebaseApp
 import com.google.firebase.concurrent.TestOnlyExecutors
 import com.google.firebase.sessions.settings.RemoteSettings
+import com.google.firebase.sessions.settings.RemoteSettingsFetcher
 import com.google.firebase.sessions.testing.FakeFirebaseApp
 import com.google.firebase.sessions.testing.FakeFirebaseInstallations
 import com.google.firebase.sessions.testing.FakeRemoteConfigFetcher
+import com.google.firebase.sessions.testing.TestSessionEventData.TEST_APPLICATION_INFO
 import kotlin.time.Duration.Companion.minutes
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.asCoroutineDispatcher
 import kotlinx.coroutines.test.runTest
 import org.json.JSONObject
 import org.junit.After
 import org.junit.Test
 import org.junit.runner.RunWith
 
+@OptIn(ExperimentalCoroutinesApi::class)
 @RunWith(AndroidJUnit4::class)
 class RemoteSettingsTest {
 
   @Test
-  fun RemoteSettings_successfulFetchCachesValues() = runTest {
+  fun remoteSettings_successfulFetchCachesValues() = runTest {
     val firebaseApp = FakeFirebaseApp().firebaseApp
     val context = firebaseApp.applicationContext
     val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
@@ -67,7 +71,7 @@ class RemoteSettingsTest {
   }
 
   @Test
-  fun RemoteSettings_successfulFetchWithLessConfigsCachesOnlyReceivedValues() = runTest {
+  fun remoteSettings_successfulFetchWithLessConfigsCachesOnlyReceivedValues() = runTest {
     val firebaseApp = FakeFirebaseApp().firebaseApp
     val context = firebaseApp.applicationContext
     val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
@@ -100,7 +104,7 @@ class RemoteSettingsTest {
   }
 
   @Test
-  fun RemoteSettings_successfulRefetchUpdatesCache() = runTest {
+  fun remoteSettings_successfulReFetchUpdatesCache() = runTest {
     val firebaseApp = FakeFirebaseApp().firebaseApp
     val context = firebaseApp.applicationContext
     val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
@@ -146,7 +150,7 @@ class RemoteSettingsTest {
   }
 
   @Test
-  fun RemoteSettings_successfulFetchWithEmptyConfigRetainsOldConfigs() = runTest {
+  fun remoteSettings_successfulFetchWithEmptyConfigRetainsOldConfigs() = runTest {
     val firebaseApp = FakeFirebaseApp().firebaseApp
     val context = firebaseApp.applicationContext
     val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
@@ -186,6 +190,20 @@ class RemoteSettingsTest {
     remoteSettings.clearCachedSettings()
   }
 
+  @Test
+  fun remoteSettingsFetcher_badFetch_callsOnFailure() = runTest {
+    var failure: String? = null
+
+    RemoteSettingsFetcher(TEST_APPLICATION_INFO, baseUrl = "this.url.is.invalid")
+      .doConfigFetch(
+        headerOptions = emptyMap(),
+        onSuccess = {},
+        onFailure = { failure = it },
+      )
+
+    assertThat(failure).isNotNull()
+  }
+
   @After
   fun cleanUp() {
     FirebaseApp.clearInstancesForTest()

firebase-sessions/src/test/kotlin/com/google/firebase/sessions/testing/FakeRemoteConfigFetcher.kt

@@ -24,7 +24,7 @@ internal class FakeRemoteConfigFetcher(var responseJSONObject: JSONObject = JSON
   override suspend fun doConfigFetch(
     headerOptions: Map<String, String>,
     onSuccess: suspend (JSONObject) -> Unit,
-    onFailure: suspend () -> Unit
+    onFailure: suspend (String) -> Unit
   ) {
     onSuccess(responseJSONObject)
   }

firebase-sessions/src/test/kotlin/com/google/firebase/sessions/testing/TestSessionEventData.kt

@@ -58,23 +58,25 @@ internal object TestSessionEventData {
       firebaseInstallationId = "",
     )
 
+  val TEST_APPLICATION_INFO =
+    ApplicationInfo(
+      appId = FakeFirebaseApp.MOCK_APP_ID,
+      deviceModel = Build.MODEL,
+      sessionSdkVersion = BuildConfig.VERSION_NAME,
+      osVersion = Build.VERSION.RELEASE,
+      logEnvironment = LogEnvironment.LOG_ENVIRONMENT_PROD,
+      AndroidApplicationInfo(
+        packageName = ApplicationProvider.getApplicationContext<Context>().packageName,
+        versionName = FakeFirebaseApp.MOCK_APP_VERSION,
+        appBuildVersion = FakeFirebaseApp.MOCK_APP_BUILD_VERSION,
+        deviceManufacturer = Build.MANUFACTURER,
+      ),
+    )
+
   val TEST_SESSION_EVENT =
     SessionEvent(
       eventType = EventType.SESSION_START,
       sessionData = TEST_SESSION_DATA,
-      applicationInfo =
-        ApplicationInfo(
-          appId = FakeFirebaseApp.MOCK_APP_ID,
-          deviceModel = Build.MODEL,
-          sessionSdkVersion = BuildConfig.VERSION_NAME,
-          osVersion = Build.VERSION.RELEASE,
-          logEnvironment = LogEnvironment.LOG_ENVIRONMENT_PROD,
-          AndroidApplicationInfo(
-            packageName = ApplicationProvider.getApplicationContext<Context>().packageName,
-            versionName = FakeFirebaseApp.MOCK_APP_VERSION,
-            appBuildVersion = FakeFirebaseApp.MOCK_APP_BUILD_VERSION,
-            deviceManufacturer = Build.MANUFACTURER,
-          ),
-        )
+      applicationInfo = TEST_APPLICATION_INFO
     )
 }