Ensure the settings cache has loaded from disk before reading any values

firebase-sessions/firebase-sessions.gradle.kts

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+@file:Suppress("UnstableApiUsage")
+
 plugins {
   id("firebase-library")
   id("kotlin-android")
@@ -45,7 +47,7 @@ android {
 dependencies {
   implementation("androidx.datastore:datastore-preferences:1.0.0")
   implementation("com.google.android.datatransport:transport-api:3.0.0")
-  implementation("com.google.firebase:firebase-common-ktx:20.3.2")
+  implementation("com.google.firebase:firebase-common-ktx:20.3.3")
   implementation("com.google.firebase:firebase-components:17.1.0")
   implementation("com.google.firebase:firebase-encoders-json:18.0.1")
   implementation("com.google.firebase:firebase-encoders:17.0.0")

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

@@ -143,7 +143,7 @@ internal constructor(
 
   /** Calculate whether we should sample events using [sessionSettings] data. */
   private fun shouldCollectEvents(): Boolean {
-    // Sampling rate of 1 means we do not sample.
+    // Sampling rate of 1 means the SDK will send every event.
     val randomValue = Math.random()
     return randomValue <= sessionSettings.samplingRate
   }

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

@@ -18,59 +18,57 @@ package com.google.firebase.sessions.settings
 
 import android.content.Context
 import android.content.pm.PackageManager
+import android.os.Build
+import android.os.Bundle
 import kotlin.time.Duration
 import kotlin.time.DurationUnit
 import kotlin.time.toDuration
 
-internal class LocalOverrideSettings(val context: Context) : SettingsProvider {
-
-  private val sessions_metadata_flag_sessionsEnabled = "firebase_sessions_enabled"
-  private val sessions_metadata_flag_sessionRestartTimeout =
-    "firebase_sessions_sessions_restart_timeout"
-  private val sessions_metadata_flag_samplingRate = "firebase_sessions_sampling_rate"
+internal class LocalOverrideSettings(context: Context) : SettingsProvider {
   private val metadata =
-    context.packageManager
-      .getApplicationInfo(context.packageName, PackageManager.GET_META_DATA)
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
+        context.packageManager.getApplicationInfo(
+          context.packageName,
+          PackageManager.ApplicationInfoFlags.of(PackageManager.GET_META_DATA.toLong()),
+        )
+      } else {
+        @Suppress("DEPRECATION") // For older API levels.
+        context.packageManager.getApplicationInfo(
+          context.packageName,
+          PackageManager.GET_META_DATA,
+        )
+      }
       .metaData
+      ?: Bundle.EMPTY
 
   override val sessionEnabled: Boolean?
-    get() {
-      metadata?.let {
-        if (it.containsKey(sessions_metadata_flag_sessionsEnabled)) {
-          return it.getBoolean(sessions_metadata_flag_sessionsEnabled)
-        }
+    get() =
+      if (metadata.containsKey(SESSIONS_ENABLED)) {
+        metadata.getBoolean(SESSIONS_ENABLED)
+      } else {
+        null
       }
-      return null
-    }
 
   override val sessionRestartTimeout: Duration?
-    get() {
-      metadata?.let {
-        if (it.containsKey(sessions_metadata_flag_sessionRestartTimeout)) {
-          val timeoutInSeconds = it.getInt(sessions_metadata_flag_sessionRestartTimeout)
-          val duration = timeoutInSeconds.toDuration(DurationUnit.SECONDS)
-          return duration
-        }
+    get() =
+      if (metadata.containsKey(SESSION_RESTART_TIMEOUT)) {
+        val timeoutInSeconds = metadata.getInt(SESSION_RESTART_TIMEOUT)
+        timeoutInSeconds.toDuration(DurationUnit.SECONDS)
+      } else {
+        null
       }
-      return null
-    }
 
   override val samplingRate: Double?
-    get() {
-      metadata?.let {
-        if (it.containsKey(sessions_metadata_flag_samplingRate)) {
-          return it.getDouble(sessions_metadata_flag_samplingRate)
-        }
+    get() =
+      if (metadata.containsKey(SAMPLING_RATE)) {
+        metadata.getDouble(SAMPLING_RATE)
+      } else {
+        null
       }
-      return null
-    }
-
-  override fun updateSettings() {
-    // Nothing to be done here since there is nothing to be updated.
-  }
 
-  override fun isSettingsStale(): Boolean {
-    // Settings are never stale since all of these are from Manifest file.
-    return false
+  private companion object {
+    const val SESSIONS_ENABLED = "firebase_sessions_enabled"
+    const val SESSION_RESTART_TIMEOUT = "firebase_sessions_sessions_restart_timeout"
+    const val SAMPLING_RATE = "firebase_sessions_sampling_rate"
   }
 }

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

@@ -16,10 +16,11 @@
 
 package com.google.firebase.sessions.settings
 
-import android.content.Context
 import android.os.Build
 import android.util.Log
-import androidx.datastore.preferences.preferencesDataStore
+import androidx.annotation.VisibleForTesting
+import androidx.datastore.core.DataStore
+import androidx.datastore.preferences.core.Preferences
 import com.google.firebase.installations.FirebaseInstallationsApi
 import com.google.firebase.sessions.ApplicationInfo
 import java.util.concurrent.atomic.AtomicBoolean
@@ -33,19 +34,14 @@ import org.json.JSONException
 import org.json.JSONObject
 
 internal class RemoteSettings(
-  context: Context,
-  blockingDispatcher: CoroutineContext,
   private val backgroundDispatcher: CoroutineContext,
   private val firebaseInstallationsApi: FirebaseInstallationsApi,
   private val appInfo: ApplicationInfo,
-  private val configsFetcher: CrashlyticsSettingsFetcher =
-    RemoteSettingsFetcher(appInfo, blockingDispatcher),
-  dataStoreName: String = SESSION_CONFIGS_NAME,
+  private val configsFetcher: CrashlyticsSettingsFetcher,
+  dataStore: DataStore<Preferences>,
 ) : SettingsProvider {
-  private val Context.dataStore by
-    preferencesDataStore(name = dataStoreName, scope = CoroutineScope(backgroundDispatcher))
-  private val settingsCache = SettingsCache(context.dataStore)
-  private var fetchInProgress = AtomicBoolean(false)
+  private val settingsCache = SettingsCache(dataStore)
+  private val fetchInProgress = AtomicBoolean(false)
 
   override val sessionEnabled: Boolean?
     get() = settingsCache.sessionsEnabled()
@@ -63,6 +59,7 @@ internal class RemoteSettings(
 
   override fun isSettingsStale(): Boolean = settingsCache.hasCacheExpired()
 
+  @VisibleForTesting
   internal fun clearCachedSettings() {
     val scope = CoroutineScope(backgroundDispatcher)
     scope.launch { settingsCache.removeConfigs() }
@@ -81,6 +78,7 @@ internal class RemoteSettings(
 
     fetchInProgress.set(true)
 
+    // TODO(mrober): Avoid sending the fid here, and avoid fetching it when data collection is off.
     // Get the installations ID before making a remote config fetch
     val installationId = firebaseInstallationsApi.id.await()
     if (installationId == null) {
@@ -155,9 +153,9 @@ internal class RemoteSettings(
     return s.replace(FORWARD_SLASH_STRING.toRegex(), "")
   }
 
-  companion object {
-    private const val SESSION_CONFIGS_NAME = "firebase_session_settings"
-    private const val TAG = "SessionConfigFetcher"
-    private const val FORWARD_SLASH_STRING: String = "/"
+  private companion object {
+    const val TAG = "SessionConfigFetcher"
+
+    const val FORWARD_SLASH_STRING: String = "/"
   }
 }

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

@@ -17,39 +17,48 @@
 package com.google.firebase.sessions.settings
 
 import android.content.Context
+import androidx.datastore.core.DataStore
+import androidx.datastore.preferences.core.Preferences
+import androidx.datastore.preferences.preferencesDataStore
 import com.google.firebase.installations.FirebaseInstallationsApi
 import com.google.firebase.sessions.ApplicationInfo
 import kotlin.coroutines.CoroutineContext
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.minutes
 
-/**
- * [SessionsSettings] manages all the configs that are relevant to the sessions library.
- *
- * @hide
- */
+/** [SessionsSettings] manages all the configs that are relevant to the sessions library. */
 internal class SessionsSettings(
-  val context: Context,
-  val blockingDispatcher: CoroutineContext,
-  val backgroundDispatcher: CoroutineContext,
-  val firebaseInstallationsApi: FirebaseInstallationsApi,
-  val appInfo: ApplicationInfo,
-  private val localOverrideSettings: LocalOverrideSettings = LocalOverrideSettings(context),
-  private val remoteSettings: RemoteSettings =
-    RemoteSettings(
-      context,
-      blockingDispatcher,
-      backgroundDispatcher,
-      firebaseInstallationsApi,
-      appInfo
-    )
+  private val localOverrideSettings: SettingsProvider,
+  private val remoteSettings: SettingsProvider,
 ) {
+  constructor(
+    context: Context,
+    blockingDispatcher: CoroutineContext,
+    backgroundDispatcher: CoroutineContext,
+    firebaseInstallationsApi: FirebaseInstallationsApi,
+    appInfo: ApplicationInfo,
+  ) : this(
+    localOverrideSettings = LocalOverrideSettings(context),
+    remoteSettings =
+      RemoteSettings(
+        backgroundDispatcher,
+        firebaseInstallationsApi,
+        appInfo,
+        configsFetcher =
+          RemoteSettingsFetcher(
+            appInfo,
+            blockingDispatcher,
+          ),
+        dataStore = context.dataStore,
+      ),
+  )
+
   // Order of preference for all the configs below:
   // 1. Honor local overrides
   // 2. If no local overrides, use remote config
   // 3. If no remote config, fall back to SDK defaults.
 
-  // Setting to qualify if sessions service is enabled.
+  /** Setting to qualify if sessions service is enabled. */
   val sessionsEnabled: Boolean
     get() {
       localOverrideSettings.sessionEnabled?.let {
@@ -62,36 +71,56 @@ internal class SessionsSettings(
       return true
     }
 
-  // Setting that provides the sessions sampling rate.
+  /** Setting that provides the sessions sampling rate. */
   val samplingRate: Double
     get() {
       localOverrideSettings.samplingRate?.let {
-        return it
+        if (isValidSamplingRate(it)) {
+          return it
+        }
       }
       remoteSettings.samplingRate?.let {
-        return it
+        if (isValidSamplingRate(it)) {
+          return it
+        }
       }
       // SDK Default
       return 1.0
     }
 
-  // Background timeout config value before which a new session is generated
+  /** Background timeout config value before which a new session is generated. */
   val sessionRestartTimeout: Duration
     get() {
       localOverrideSettings.sessionRestartTimeout?.let {
-        return it
+        if (isValidSessionRestartTimeout(it)) {
+          return it
+        }
       }
       remoteSettings.sessionRestartTimeout?.let {
-        return it
+        if (isValidSessionRestartTimeout(it)) {
+          return it
+        }
       }
       // SDK Default
       return 30.minutes
     }
 
-  // Update the settings for all the settings providers
+  private fun isValidSamplingRate(samplingRate: Double): Boolean = samplingRate in 0.0..1.0
+
+  private fun isValidSessionRestartTimeout(sessionRestartTimeout: Duration): Boolean {
+    return sessionRestartTimeout.isPositive() && sessionRestartTimeout.isFinite()
+  }
+
+  /** Update the settings for all the settings providers. */
   fun updateSettings() {
-    // Placeholder to initiate settings update on different sources
     localOverrideSettings.updateSettings()
     remoteSettings.updateSettings()
   }
+
+  private companion object {
+    const val SESSION_CONFIGS_NAME = "firebase_session_settings"
+
+    private val Context.dataStore: DataStore<Preferences> by
+      preferencesDataStore(name = SESSION_CONFIGS_NAME)
+  }
 }