Skip to content

Commit b958b36

Browse files
authored
Fix session start data issue (#253)
* Revert "Fallback to the defaultSettings if cdn cannot be reached (#231)" This reverts commit d37d2d3. * Update tests after reverting. * add a sane default Settings object similar to swift. * fix: use a sane default settings object in cases where we can't connect. * fix: set DestinationPlugins to enabled by default.
1 parent 26b4495 commit b958b36

File tree

5 files changed

+95
-70
lines changed

5 files changed

+95
-70
lines changed

core/src/main/java/com/segment/analytics/kotlin/core/Configuration.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ data class Configuration(
3333
var flushAt: Int = 20,
3434
var flushInterval: Int = 30,
3535
var flushPolicies: List<FlushPolicy> = emptyList<FlushPolicy>(),
36-
var defaultSettings: Settings = Settings(),
36+
var defaultSettings: Settings? = null,
3737
var autoAddSegmentDestination: Boolean = true,
3838
var apiHost: String = DEFAULT_API_HOST,
3939
var cdnHost: String = DEFAULT_CDN_HOST,

core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ suspend fun Analytics.checkSettings() {
9292
val settingsObj: Settings? = fetchSettings(writeKey, cdnHost)
9393

9494
withContext(analyticsDispatcher) {
95+
9596
settingsObj?.let {
9697
log("Dispatching update settings on ${Thread.currentThread().name}")
9798
store.dispatch(System.UpdateSettingsAction(settingsObj), System::class)
@@ -108,22 +109,27 @@ internal fun Analytics.fetchSettings(
108109
writeKey: String,
109110
cdnHost: String
110111
): Settings? = try {
111-
val connection = HTTPClient(writeKey, this.configuration.requestFactory).settings(cdnHost)
112-
val settingsString =
113-
connection.inputStream?.bufferedReader()?.use(BufferedReader::readText) ?: ""
114-
log("Fetched Settings: $settingsString")
115-
LenientJson.decodeFromString(settingsString)
116-
} catch (ex: Exception) {
117-
reportErrorWithMetrics(
118-
this,
119-
AnalyticsError.SettingsFail(AnalyticsError.NetworkUnknown(URL("https://$cdnHost/projects/$writeKey/settings"), ex)),
120-
"Failed to fetch settings",
121-
Telemetry.INVOKE_ERROR_METRIC,
122-
ex.stackTraceToString()
123-
) {
124-
it["error"] = ex.toString()
125-
it["writekey"] = writeKey
126-
it["message"] = "Error retrieving settings"
127-
}
128-
configuration.defaultSettings
129-
}
112+
val connection = HTTPClient(writeKey, this.configuration.requestFactory).settings(cdnHost)
113+
val settingsString =
114+
connection.inputStream?.bufferedReader()?.use(BufferedReader::readText) ?: ""
115+
log("Fetched Settings: $settingsString")
116+
LenientJson.decodeFromString(settingsString)
117+
} catch (ex: Exception) {
118+
reportErrorWithMetrics(
119+
this,
120+
AnalyticsError.SettingsFail(
121+
AnalyticsError.NetworkUnknown(
122+
URL("https://$cdnHost/projects/$writeKey/settings"),
123+
ex
124+
)
125+
),
126+
"Failed to fetch settings",
127+
Telemetry.INVOKE_ERROR_METRIC,
128+
ex.stackTraceToString()
129+
) {
130+
it["error"] = ex.toString()
131+
it["writekey"] = writeKey
132+
it["message"] = "Error retrieving settings"
133+
}
134+
null
135+
}

core/src/main/java/com/segment/analytics/kotlin/core/State.kt

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,38 @@ data class System(
2525

2626
companion object {
2727
fun defaultState(configuration: Configuration, storage: Storage): System {
28-
val settings = try {
29-
Json.decodeFromString(
30-
Settings.serializer(),
31-
storage.read(Storage.Constants.Settings) ?: ""
32-
)
33-
} catch (ignored: Exception) {
34-
configuration.defaultSettings
28+
val storedSettings = storage.read(Storage.Constants.Settings)
29+
val defaultSettings = configuration.defaultSettings ?: Settings(
30+
integrations = buildJsonObject {
31+
put(
32+
"Segment.io",
33+
buildJsonObject {
34+
put(
35+
"apiKey",
36+
configuration.writeKey
37+
)
38+
put("apiHost", Constants.DEFAULT_API_HOST)
39+
})
40+
},
41+
plan = emptyJsonObject,
42+
edgeFunction = emptyJsonObject,
43+
middlewareSettings = emptyJsonObject
44+
)
45+
46+
// Use stored settings or fallback to default settings
47+
val settings = if (storedSettings == null || storedSettings == "" || storedSettings == "{}") {
48+
defaultSettings
49+
} else {
50+
try {
51+
Json.decodeFromString(
52+
Settings.serializer(),
53+
storedSettings
54+
)
55+
} catch (ignored: Exception) {
56+
defaultSettings
57+
}
3558
}
59+
3660
return System(
3761
configuration = configuration,
3862
settings = settings,

core/src/main/java/com/segment/analytics/kotlin/core/platform/Plugin.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ abstract class DestinationPlugin : EventPlugin {
8989
override val type: Plugin.Type = Plugin.Type.Destination
9090
private val timeline: Timeline = Timeline()
9191
override lateinit var analytics: Analytics
92-
internal var enabled = false
92+
internal var enabled = true
9393
abstract val key: String
9494

9595
override fun setup(analytics: Analytics) {

core/src/test/kotlin/com/segment/analytics/kotlin/core/SettingsTests.kt

Lines changed: 37 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ class SettingsTests {
8888

8989
// no settings available, should not be called
9090
analytics.add(mockPlugin)
91-
91+
verify (exactly = 0){
92+
mockPlugin.update(any(), any())
93+
}
9294

9395
// load settings
9496
mockHTTPClient()
@@ -102,7 +104,7 @@ class SettingsTests {
102104
// load settings again
103105
mockHTTPClient()
104106
analytics.checkSettings()
105-
verify (exactly = 2) {
107+
verify (exactly = 1) {
106108
mockPlugin.update(any(), Plugin.UpdateType.Refresh)
107109
}
108110
}
@@ -230,102 +232,95 @@ class SettingsTests {
230232

231233
@Test
232234
fun `fetchSettings returns null when Settings string is invalid`() {
233-
val emptySettings = analytics.fetchSettings("emptySettingsObject", "cdn-settings.segment.com/v1")
234235
// Null on invalid JSON
235236
mockHTTPClient("")
236237
var settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
237-
assertEquals(emptySettings, settings)
238+
assertNull(settings)
238239

239240
// Null on invalid JSON
240241
mockHTTPClient("hello")
241242
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
242-
assertEquals(emptySettings, settings)
243+
assertNull(settings)
243244

244245
// Null on invalid JSON
245246
mockHTTPClient("#! /bin/sh")
246247
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
247-
assertEquals(emptySettings, settings)
248+
assertNull(settings)
248249

249250
// Null on invalid JSON
250251
mockHTTPClient("<!DOCTYPE html>")
251252
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
252-
assertEquals(emptySettings, settings)
253+
assertNull(settings)
253254

254255
// Null on invalid JSON
255256
mockHTTPClient("true")
256257
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
257-
assertEquals(emptySettings, settings)
258+
assertNull(settings)
258259

259260
// Null on invalid JSON
260261
mockHTTPClient("[]")
261262
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
262-
assertEquals(emptySettings, settings)
263+
assertNull(settings)
263264

264265
// Null on invalid JSON
265266
mockHTTPClient("}{")
266267
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
267-
assertEquals(emptySettings, settings)
268+
assertNull(settings)
268269

269270
// Null on invalid JSON
270271
mockHTTPClient("{{{{}}}}")
271272
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
272-
assertEquals(emptySettings, settings)
273+
assertNull(settings)
273274

274275
// Null on invalid JSON
275276
mockHTTPClient("{null:\"bar\"}")
276277
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
277-
assertEquals(emptySettings, settings)
278+
assertNull(settings)
278279
}
279280

280281
@Test
281282
fun `fetchSettings returns null when parameters are invalid`() {
282-
val emptySettings = analytics.fetchSettings("emptySettingsObject", "cdn-settings.segment.com/v1")
283283
mockHTTPClient("{\"integrations\":{}, \"plan\":{}, \"edgeFunction\": {}, \"middlewareSettings\": {}}")
284284

285285
// empty host
286286
var settings = analytics.fetchSettings("foo", "")
287-
assertEquals(emptySettings, settings)
287+
assertNull(settings)
288288

289289
// not a host name
290290
settings = analytics.fetchSettings("foo", "http://blah")
291-
assertEquals(emptySettings, settings)
291+
assertNull(settings)
292292

293293
// emoji
294294
settings = analytics.fetchSettings("foo", "😃")
295-
assertEquals(emptySettings, settings)
295+
assertNull(settings)
296296
}
297297

298298
@Test
299299
fun `fetchSettings returns null when Settings string is null for known properties`() {
300300
// Null if integrations is null
301301
mockHTTPClient("{\"integrations\":null}")
302302
var settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
303-
assertTrue(settings?.integrations?.isEmpty() ?: true, "Integrations should be empty")
304-
assertTrue(settings?.plan?.isEmpty() ?: true, "Plan should be empty")
305-
assertTrue(settings?.edgeFunction?.isEmpty() ?: true, "EdgeFunction should be empty")
306-
assertTrue(settings?.middlewareSettings?.isEmpty() ?: true, "MiddlewareSettings should be empty")
307-
assertTrue(settings?.metrics?.isEmpty() ?: true, "Metrics should be empty")
308-
assertTrue(settings?.consentSettings?.isEmpty() ?: true, "ConsentSettings should be empty")
309-
310-
// // Null if plan is null
311-
// mockHTTPClient("{\"plan\":null}")
312-
// settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
313-
// assertNull(settings)
314-
//
315-
// // Null if edgeFunction is null
316-
// mockHTTPClient("{\"edgeFunction\":null}")
317-
// settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
318-
// assertNull(settings)
319-
//
320-
// // Null if middlewareSettings is null
321-
// mockHTTPClient("{\"middlewareSettings\":null}")
322-
// settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
323-
// assertNull(settings)
303+
assertNull(settings)
304+
305+
// Null if plan is null
306+
mockHTTPClient("{\"plan\":null}")
307+
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
308+
assertNull(settings)
309+
310+
// Null if edgeFunction is null
311+
mockHTTPClient("{\"edgeFunction\":null}")
312+
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
313+
assertNull(settings)
314+
315+
// Null if middlewareSettings is null
316+
mockHTTPClient("{\"middlewareSettings\":null}")
317+
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
318+
assertNull(settings)
324319
}
325320

326321
@Test
327322
fun `known Settings property types must match json type`() {
328-
val emptySettings = analytics.fetchSettings("emptySettingsObject", "cdn-settings.segment.com/v1")
323+
329324
// integrations must be a JSON object
330325
mockHTTPClient("{\"integrations\":{}}")
331326
var settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
@@ -334,21 +329,21 @@ class SettingsTests {
334329
// Null if integrations is a number
335330
mockHTTPClient("{\"integrations\":123}")
336331
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
337-
assertEquals(emptySettings, settings)
332+
assertNull(settings)
338333

339334
// Null if integrations is a string
340335
mockHTTPClient("{\"integrations\":\"foo\"}")
341336
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
342-
assertEquals(emptySettings, settings)
337+
assertNull(settings)
343338

344339
// Null if integrations is an array
345340
mockHTTPClient("{\"integrations\":[\"foo\"]}")
346341
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
347-
assertEquals(emptySettings, settings)
342+
assertNull(settings)
348343

349344
// Null if integrations is an emoji (UTF-8 string)
350345
mockHTTPClient("{\"integrations\": 😃}")
351346
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
352-
assertEquals(emptySettings, settings)
347+
assertNull(settings)
353348
}
354349
}

0 commit comments

Comments
 (0)