Merge 306bdc6 into 62508f2

Author: mrober

firebase-sessions/src/androidTest/kotlin/com/google/firebase/sessions/FirebaseSessionsTests.kt

@@ -20,12 +20,10 @@ import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.google.common.truth.Truth.assertThat
 import com.google.firebase.Firebase
-import com.google.firebase.FirebaseApp
 import com.google.firebase.FirebaseOptions
 import com.google.firebase.initialize
 import com.google.firebase.sessions.settings.SessionsSettings
-import org.junit.After
-import org.junit.Before
+import org.junit.BeforeClass
 import org.junit.Test
 import org.junit.runner.RunWith
 
@@ -36,23 +34,6 @@ import org.junit.runner.RunWith
  */
 @RunWith(AndroidJUnit4::class)
 class FirebaseSessionsTests {
-  @Before
-  fun setUp() {
-    Firebase.initialize(
-      ApplicationProvider.getApplicationContext(),
-      FirebaseOptions.Builder()
-        .setApplicationId(APP_ID)
-        .setApiKey(API_KEY)
-        .setProjectId(PROJECT_ID)
-        .build()
-    )
-  }
-
-  @After
-  fun cleanUp() {
-    FirebaseApp.clearInstancesForTest()
-  }
-
   @Test
   fun firebaseSessionsDoesInitialize() {
     assertThat(FirebaseSessions.instance).isNotNull()
@@ -69,5 +50,18 @@ class FirebaseSessionsTests {
     private const val APP_ID = "1:1:android:1a"
     private const val API_KEY = "API-KEY-API-KEY-API-KEY-API-KEY-API-KEY"
     private const val PROJECT_ID = "PROJECT-ID"
+
+    @BeforeClass
+    @JvmStatic
+    fun setUp() {
+      Firebase.initialize(
+        ApplicationProvider.getApplicationContext(),
+        FirebaseOptions.Builder()
+          .setApplicationId(APP_ID)
+          .setApiKey(API_KEY)
+          .setProjectId(PROJECT_ID)
+          .build(),
+      )
+    }
   }
 }

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

@@ -23,13 +23,11 @@ import com.google.firebase.installations.FirebaseInstallationsApi
 import com.google.firebase.sessions.ApplicationInfo
 import com.google.firebase.sessions.InstallationId
 import com.google.firebase.sessions.TimeProvider
-import dagger.Lazy
 import javax.inject.Inject
 import javax.inject.Singleton
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.hours
 import kotlin.time.Duration.Companion.seconds
-import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import org.json.JSONException
@@ -43,11 +41,8 @@ constructor(
   private val firebaseInstallationsApi: FirebaseInstallationsApi,
   private val appInfo: ApplicationInfo,
   private val configsFetcher: CrashlyticsSettingsFetcher,
-  private val lazySettingsCache: Lazy<SettingsCache>,
+  private val settingsCache: SettingsCache,
 ) : SettingsProvider {
-  private val settingsCache: SettingsCache
-    get() = lazySettingsCache.get()
-
   private val fetchInProgress = Mutex()
 
   override val sessionEnabled: Boolean?
@@ -133,7 +128,7 @@ constructor(
               sessionTimeoutSeconds = sessionTimeoutSeconds,
               sessionSamplingRate = sessionSamplingRate,
               cacheDurationSeconds = cacheDuration ?: defaultCacheDuration,
-              cacheUpdatedTimeMs = timeProvider.currentTime().ms,
+              cacheUpdatedTimeSeconds = timeProvider.currentTime().seconds,
             )
           )
         },
@@ -148,7 +143,7 @@ constructor(
   override fun isSettingsStale(): Boolean = settingsCache.hasCacheExpired()
 
   @VisibleForTesting
-  internal fun clearCachedSettings() = runBlocking {
+  internal suspend fun clearCachedSettings() {
     settingsCache.updateConfigs(SessionConfigsSerializer.defaultValue)
   }
 

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

@@ -30,7 +30,7 @@ internal data class SessionConfigs(
   val sessionSamplingRate: Double?,
   val sessionTimeoutSeconds: Int?,
   val cacheDurationSeconds: Int?,
-  val cacheUpdatedTimeMs: Long?,
+  val cacheUpdatedTimeSeconds: Long?,
 )
 
 /** DataStore json [Serializer] for [SessionConfigs]. */
@@ -41,7 +41,7 @@ internal object SessionConfigsSerializer : Serializer<SessionConfigs> {
       sessionSamplingRate = null,
       sessionTimeoutSeconds = null,
       cacheDurationSeconds = null,
-      cacheUpdatedTimeMs = null,
+      cacheUpdatedTimeSeconds = null,
     )
 
   override suspend fun readFrom(input: InputStream): SessionConfigs =

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

@@ -17,12 +17,18 @@
 package com.google.firebase.sessions.settings
 
 import android.util.Log
+import androidx.annotation.VisibleForTesting
 import androidx.datastore.core.DataStore
+import com.google.firebase.annotations.concurrent.Background
 import com.google.firebase.sessions.TimeProvider
 import java.io.IOException
+import java.util.concurrent.atomic.AtomicReference
 import javax.inject.Inject
 import javax.inject.Singleton
+import kotlin.coroutines.CoroutineContext
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 
 internal interface SettingsCache {
@@ -41,23 +47,38 @@ internal interface SettingsCache {
 internal class SettingsCacheImpl
 @Inject
 constructor(
+  @Background private val backgroundDispatcher: CoroutineContext,
   private val timeProvider: TimeProvider,
   private val sessionConfigsDataStore: DataStore<SessionConfigs>,
 ) : SettingsCache {
-  private var sessionConfigs: SessionConfigs
+  private val sessionConfigsAtomicReference = AtomicReference<SessionConfigs>()
+
+  private val sessionConfigs: SessionConfigs
+    get() {
+      // Ensure configs are loaded from disk before the first access
+      if (sessionConfigsAtomicReference.get() == null) {
+        // Double check to avoid the `runBlocking` unless necessary
+        sessionConfigsAtomicReference.compareAndSet(
+          null,
+          runBlocking { sessionConfigsDataStore.data.first() },
+        )
+      }
+
+      return sessionConfigsAtomicReference.get()
+    }
 
   init {
-    // Block until the cache is loaded from disk to ensure cache
-    // values are valid and readable from the main thread on init.
-    runBlocking { sessionConfigs = sessionConfigsDataStore.data.first() }
+    CoroutineScope(backgroundDispatcher).launch {
+      sessionConfigsDataStore.data.collect(sessionConfigsAtomicReference::set)
+    }
   }
 
   override fun hasCacheExpired(): Boolean {
-    val cacheUpdatedTimeMs = sessionConfigs.cacheUpdatedTimeMs
+    val cacheUpdatedTimeSeconds = sessionConfigs.cacheUpdatedTimeSeconds
     val cacheDurationSeconds = sessionConfigs.cacheDurationSeconds
 
-    if (cacheUpdatedTimeMs != null && cacheDurationSeconds != null) {
-      val timeDifferenceSeconds = (timeProvider.currentTime().ms - cacheUpdatedTimeMs) / 1000
+    if (cacheUpdatedTimeSeconds != null && cacheDurationSeconds != null) {
+      val timeDifferenceSeconds = timeProvider.currentTime().seconds - cacheUpdatedTimeSeconds
       if (timeDifferenceSeconds < cacheDurationSeconds) {
         return false
       }
@@ -74,12 +95,12 @@ constructor(
   override suspend fun updateConfigs(sessionConfigs: SessionConfigs) {
     try {
       sessionConfigsDataStore.updateData { sessionConfigs }
-      this.sessionConfigs = sessionConfigs
     } catch (ex: IOException) {
       Log.w(TAG, "Failed to update config values: $ex")
     }
   }
 
+  @VisibleForTesting
   internal suspend fun removeConfigs() =
     try {
       sessionConfigsDataStore.updateData { SessionConfigsSerializer.defaultValue }

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

@@ -44,7 +44,7 @@ class SessionDatastoreTest {
           DataStoreFactory.create(
             serializer = SessionDataSerializer,
             scope = CoroutineScope(StandardTestDispatcher(testScheduler, "blocking")),
-            produceFile = { appContext.dataStoreFile("sessionDataTestDataStore.data") },
+            produceFile = { appContext.dataStoreFile("sessionDataStore.data") },
           ),
       )