Skip to content

Commit 1581047

Browse files
mrobermrober
authored
Merge ebcbd95 into 2394d7b 
2 parents 2394d7b + ebcbd95 commit 1581047

File tree

7 files changed

+172
-151
lines changed

7 files changed

+172
-151
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ internal constructor(
4848
applicationInfo,
4949
)
5050
private val timeProvider: TimeProvider = Time()
51-
private val sessionGenerator =
52-
SessionGenerator(collectEvents = shouldCollectEvents(), timeProvider)
51+
private val sessionGenerator: SessionGenerator
5352
private val eventGDTLogger = EventGDTLogger(transportFactoryProvider)
5453
private val sessionCoordinator = SessionCoordinator(firebaseInstallations, eventGDTLogger)
5554

5655
init {
5756
sessionSettings.updateSettings()
57+
sessionGenerator = SessionGenerator(collectEvents = shouldCollectEvents(), timeProvider)
5858

5959
val sessionInitiateListener =
6060
object : SessionInitiateListener {

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

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,53 +27,41 @@ import kotlin.coroutines.CoroutineContext
2727
import kotlin.time.Duration
2828
import kotlin.time.Duration.Companion.seconds
2929
import kotlinx.coroutines.CoroutineScope
30-
import kotlinx.coroutines.Dispatchers
3130
import kotlinx.coroutines.launch
32-
import kotlinx.coroutines.runBlocking
3331
import kotlinx.coroutines.tasks.await
3432
import org.json.JSONException
3533
import org.json.JSONObject
3634

3735
internal class RemoteSettings(
3836
context: Context,
39-
private val blockingDispatcher: CoroutineContext,
37+
blockingDispatcher: CoroutineContext,
4038
private val backgroundDispatcher: CoroutineContext,
4139
private val firebaseInstallationsApi: FirebaseInstallationsApi,
4240
private val appInfo: ApplicationInfo,
43-
private val configsFetcher: CrashlyticsSettingsFetcher = RemoteSettingsFetcher(appInfo),
44-
dataStoreName: String = SESSION_CONFIGS_NAME
41+
private val configsFetcher: CrashlyticsSettingsFetcher =
42+
RemoteSettingsFetcher(appInfo, blockingDispatcher),
43+
dataStoreName: String = SESSION_CONFIGS_NAME,
4544
) : SettingsProvider {
46-
private val Context.dataStore by preferencesDataStore(name = dataStoreName)
45+
private val Context.dataStore by
46+
preferencesDataStore(name = dataStoreName, scope = CoroutineScope(backgroundDispatcher))
4747
private val settingsCache = SettingsCache(context.dataStore)
4848
private var fetchInProgress = AtomicBoolean(false)
4949

5050
override val sessionEnabled: Boolean?
51-
get() {
52-
return settingsCache.sessionsEnabled()
53-
}
51+
get() = settingsCache.sessionsEnabled()
5452

5553
override val sessionRestartTimeout: Duration?
56-
get() {
57-
val durationInSeconds = settingsCache.sessionRestartTimeout()
58-
if (durationInSeconds != null) {
59-
return durationInSeconds.toLong().seconds
60-
}
61-
return null
62-
}
54+
get() = settingsCache.sessionRestartTimeout()?.seconds
6355

6456
override val samplingRate: Double?
65-
get() {
66-
return settingsCache.sessionSamplingRate()
67-
}
57+
get() = settingsCache.sessionSamplingRate()
6858

6959
override fun updateSettings() {
70-
// TODO: Move to blocking coroutine dispatcher.
71-
runBlocking(Dispatchers.Default) { launch { fetchConfigs() } }
60+
val scope = CoroutineScope(backgroundDispatcher)
61+
scope.launch { fetchConfigs() }
7262
}
7363

74-
override fun isSettingsStale(): Boolean {
75-
return settingsCache.hasCacheExpired()
76-
}
64+
override fun isSettingsStale(): Boolean = settingsCache.hasCacheExpired()
7765

7866
internal fun clearCachedSettings() {
7967
val scope = CoroutineScope(backgroundDispatcher)

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

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ package com.google.firebase.sessions.settings
1919
import android.net.Uri
2020
import com.google.firebase.sessions.ApplicationInfo
2121
import java.io.BufferedReader
22-
import java.io.IOException
2322
import java.io.InputStreamReader
2423
import java.net.URL
2524
import javax.net.ssl.HttpsURLConnection
25+
import kotlin.coroutines.CoroutineContext
26+
import kotlinx.coroutines.withContext
2627
import org.json.JSONObject
2728

2829
internal interface CrashlyticsSettingsFetcher {
@@ -35,40 +36,43 @@ internal interface CrashlyticsSettingsFetcher {
3536

3637
internal class RemoteSettingsFetcher(
3738
private val appInfo: ApplicationInfo,
38-
private val baseUrl: String = FIREBASE_SESSIONS_BASE_URL_STRING
39+
private val blockingDispatcher: CoroutineContext,
40+
private val baseUrl: String = FIREBASE_SESSIONS_BASE_URL_STRING,
3941
) : CrashlyticsSettingsFetcher {
42+
@Suppress("BlockingMethodInNonBlockingContext") // blockingDispatcher is safe for blocking calls.
4043
override suspend fun doConfigFetch(
4144
headerOptions: Map<String, String>,
4245
onSuccess: suspend (JSONObject) -> Unit,
4346
onFailure: suspend (String) -> Unit
44-
) {
45-
val connection = settingsUrl().openConnection() as HttpsURLConnection
46-
connection.requestMethod = "GET"
47-
connection.setRequestProperty("Accept", "application/json")
48-
headerOptions.forEach { connection.setRequestProperty(it.key, it.value) }
47+
) =
48+
withContext(blockingDispatcher) {
49+
val connection = settingsUrl().openConnection() as HttpsURLConnection
50+
connection.requestMethod = "GET"
51+
connection.setRequestProperty("Accept", "application/json")
52+
headerOptions.forEach { connection.setRequestProperty(it.key, it.value) }
4953

50-
try {
51-
val responseCode = connection.responseCode
52-
if (responseCode == HttpsURLConnection.HTTP_OK) {
53-
val inputStream = connection.inputStream
54-
val bufferedReader = BufferedReader(InputStreamReader(inputStream))
55-
val response = StringBuilder()
56-
var inputLine: String?
57-
while (bufferedReader.readLine().also { inputLine = it } != null) {
58-
response.append(inputLine)
59-
}
60-
bufferedReader.close()
61-
inputStream.close()
54+
try {
55+
val responseCode = connection.responseCode
56+
if (responseCode == HttpsURLConnection.HTTP_OK) {
57+
val inputStream = connection.inputStream
58+
val bufferedReader = BufferedReader(InputStreamReader(inputStream))
59+
val response = StringBuilder()
60+
var inputLine: String?
61+
while (bufferedReader.readLine().also { inputLine = it } != null) {
62+
response.append(inputLine)
63+
}
64+
bufferedReader.close()
65+
inputStream.close()
6266

63-
val responseJson = JSONObject(response.toString())
64-
onSuccess(responseJson)
65-
} else {
66-
onFailure("Bad response code: $responseCode")
67+
val responseJson = JSONObject(response.toString())
68+
onSuccess(responseJson)
69+
} else {
70+
onFailure("Bad response code: $responseCode")
71+
}
72+
} catch (ex: Exception) {
73+
onFailure(ex.message ?: ex.toString())
6774
}
68-
} catch (ex: IOException) {
69-
onFailure(ex.message ?: ex.toString())
7075
}
71-
}
7276

7377
private fun settingsUrl(): URL {
7478
val uri =

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import com.google.firebase.sessions.testing.TestSessionEventData.TEST_APPLICATIO
2929
import kotlin.time.Duration.Companion.minutes
3030
import kotlinx.coroutines.ExperimentalCoroutinesApi
3131
import kotlinx.coroutines.asCoroutineDispatcher
32+
import kotlinx.coroutines.test.runCurrent
3233
import kotlinx.coroutines.test.runTest
3334
import org.json.JSONObject
3435
import org.junit.After
@@ -63,6 +64,8 @@ class RemoteSettingsTest {
6364
fakeFetcher.responseJSONObject = JSONObject(validResponse)
6465
remoteSettings.updateSettings()
6566

67+
runCurrent()
68+
6669
assertThat(remoteSettings.sessionEnabled).isFalse()
6770
assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
6871
assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -96,6 +99,8 @@ class RemoteSettingsTest {
9699
fakeFetcher.responseJSONObject = fetchedResponse
97100
remoteSettings.updateSettings()
98101

102+
runCurrent()
103+
99104
assertThat(remoteSettings.sessionEnabled).isNull()
100105
assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
101106
assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -128,6 +133,8 @@ class RemoteSettingsTest {
128133
fakeFetcher.responseJSONObject = fetchedResponse
129134
remoteSettings.updateSettings()
130135

136+
runCurrent()
137+
131138
assertThat(remoteSettings.sessionEnabled).isFalse()
132139
assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
133140
assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -142,6 +149,8 @@ class RemoteSettingsTest {
142149
fakeFetcher.responseJSONObject = fetchedResponse
143150
remoteSettings.updateSettings()
144151

152+
runCurrent()
153+
145154
assertThat(remoteSettings.sessionEnabled).isTrue()
146155
assertThat(remoteSettings.samplingRate).isEqualTo(0.25)
147156
assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(20.minutes)
@@ -171,6 +180,8 @@ class RemoteSettingsTest {
171180
fakeFetcher.responseJSONObject = fetchedResponse
172181
remoteSettings.updateSettings()
173182

183+
runCurrent()
184+
174185
assertThat(remoteSettings.sessionEnabled).isFalse()
175186
assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
176187
assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -194,7 +205,11 @@ class RemoteSettingsTest {
194205
fun remoteSettingsFetcher_badFetch_callsOnFailure() = runTest {
195206
var failure: String? = null
196207

197-
RemoteSettingsFetcher(TEST_APPLICATION_INFO, baseUrl = "this.url.is.invalid")
208+
RemoteSettingsFetcher(
209+
TEST_APPLICATION_INFO,
210+
TestOnlyExecutors.blocking().asCoroutineDispatcher() + coroutineContext,
211+
baseUrl = "this.url.is.invalid",
212+
)
198213
.doConfigFetch(
199214
headerOptions = emptyMap(),
200215
onSuccess = {},

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,18 @@ import com.google.firebase.sessions.settings.SessionsSettings
2626
import com.google.firebase.sessions.testing.FakeFirebaseApp
2727
import com.google.firebase.sessions.testing.FakeFirebaseInstallations
2828
import com.google.firebase.sessions.testing.FakeRemoteConfigFetcher
29-
import kotlin.coroutines.coroutineContext
3029
import kotlin.time.Duration.Companion.minutes
30+
import kotlinx.coroutines.ExperimentalCoroutinesApi
3131
import kotlinx.coroutines.asCoroutineDispatcher
32+
import kotlinx.coroutines.test.runCurrent
3233
import kotlinx.coroutines.test.runTest
3334
import org.json.JSONObject
3435
import org.junit.After
3536
import org.junit.Test
3637
import org.junit.runner.RunWith
3738
import org.robolectric.RobolectricTestRunner
3839

40+
@OptIn(ExperimentalCoroutinesApi::class)
3941
@RunWith(RobolectricTestRunner::class)
4042
class SessionsSettingsTest {
4143

@@ -53,6 +55,9 @@ class SessionsSettingsTest {
5355
firebaseInstallations,
5456
SessionEvents.getApplicationInfo(firebaseApp)
5557
)
58+
59+
runCurrent()
60+
5661
assertThat(sessionsSettings.sessionsEnabled).isTrue()
5762
assertThat(sessionsSettings.samplingRate).isEqualTo(1.0)
5863
assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(30.minutes)
@@ -76,6 +81,9 @@ class SessionsSettingsTest {
7681
firebaseInstallations,
7782
SessionEvents.getApplicationInfo(firebaseApp)
7883
)
84+
85+
runCurrent()
86+
7987
assertThat(sessionsSettings.sessionsEnabled).isFalse()
8088
assertThat(sessionsSettings.samplingRate).isEqualTo(0.5)
8189
assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(3.minutes)
@@ -98,13 +106,16 @@ class SessionsSettingsTest {
98106
firebaseInstallations,
99107
SessionEvents.getApplicationInfo(firebaseApp)
100108
)
109+
110+
runCurrent()
111+
101112
assertThat(sessionsSettings.sessionsEnabled).isFalse()
102113
assertThat(sessionsSettings.samplingRate).isEqualTo(0.5)
103114
assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(30.minutes)
104115
}
105116

106117
@Test
107-
fun sessionSettings_RemoteSettingsOverrideDefaultsWhenPresent() = runTest {
118+
fun sessionSettings_remoteSettingsOverrideDefaultsWhenPresent() = runTest {
108119
val firebaseApp = FakeFirebaseApp().firebaseApp
109120
val context = firebaseApp.applicationContext
110121
val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
@@ -133,6 +144,9 @@ class SessionsSettingsTest {
133144
localOverrideSettings = LocalOverrideSettings(context),
134145
remoteSettings = remoteSettings
135146
)
147+
148+
runCurrent()
149+
136150
assertThat(sessionsSettings.sessionsEnabled).isFalse()
137151
assertThat(sessionsSettings.samplingRate).isEqualTo(0.75)
138152
assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -141,7 +155,7 @@ class SessionsSettingsTest {
141155
}
142156

143157
@Test
144-
fun sessionSettings_ManifestOverridesRemoteSettingsAndDefaultsWhenPresent() = runTest {
158+
fun sessionSettings_manifestOverridesRemoteSettingsAndDefaultsWhenPresent() = runTest {
145159
val metadata = Bundle()
146160
metadata.putBoolean("firebase_sessions_enabled", true)
147161
metadata.putDouble("firebase_sessions_sampling_rate", 0.5)
@@ -174,6 +188,9 @@ class SessionsSettingsTest {
174188
localOverrideSettings = LocalOverrideSettings(context),
175189
remoteSettings = remoteSettings
176190
)
191+
192+
runCurrent()
193+
177194
assertThat(sessionsSettings.sessionsEnabled).isTrue()
178195
assertThat(sessionsSettings.samplingRate).isEqualTo(0.5)
179196
assertThat(sessionsSettings.sessionRestartTimeout).isEqualTo(3.minutes)

0 commit comments

Comments
 (0)