Share settings cache between running processes (#6788)

With the multi-process data store change, all processes will read the
settings cache from the same file safely. This means if a second process
started, it would read the cache the first process persisted.

But if 2 processes were already running, and one fetched and cached new
settings, it wouldn't automatically share it with the other running
process.

This change fixes that by having all processes watch the file.

Also cleaned up settings a bit, and made everything in seconds to avoid
converting units. Cleaned up unit tests. Also removed the need to lazy
load the cache by doing a double check in the getter instead.

There is more potential to clean up, but let's do it later.

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") },
           ),
       )