Skip to content

Fallback to the defaultSettings if cdn cannot be reached #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 3, 2024
Merged

Fallback to the defaultSettings if cdn cannot be reached #231

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9a06034
fallback to default settings, not null
niallzato Jun 4, 2024
d059459
fallback to default settings, not null
niallzato Jun 4, 2024
42e2ca7
update tests
niallzato Jun 4, 2024
0c6afd9
test: remove commented out code after confirming that the current tes…
didiergarcia Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,5 +121,5 @@ internal fun Analytics.fetchSettings(
it["writekey"] = writeKey
it["message"] = "Error retrieving settings"
}
null
configuration.defaultSettings
}
81 changes: 44 additions & 37 deletions core/src/test/kotlin/com/segment/analytics/kotlin/core/SettingsTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ class SettingsTests {

// no settings available, should not be called
analytics.add(mockPlugin)
verify (exactly = 0){
mockPlugin.update(any(), any())
}
// verify (exactly = 0){
// mockPlugin.update(any(), any())
// }

// load settings
mockHTTPClient()
Expand All @@ -104,7 +104,7 @@ class SettingsTests {
// load settings again
mockHTTPClient()
analytics.checkSettings()
verify (exactly = 1) {
verify (exactly = 2) {
mockPlugin.update(any(), Plugin.UpdateType.Refresh)
}
}
Expand Down Expand Up @@ -232,95 +232,102 @@ class SettingsTests {

@Test
fun `fetchSettings returns null when Settings string is invalid`() {
val emptySettings = analytics.fetchSettings("emptySettingsObject", "cdn-settings.segment.com/v1")
// Null on invalid JSON
mockHTTPClient("")
var settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

// Null on invalid JSON
mockHTTPClient("hello")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

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

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

// Null on invalid JSON
mockHTTPClient("true")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

// Null on invalid JSON
mockHTTPClient("[]")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

// Null on invalid JSON
mockHTTPClient("}{")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

// Null on invalid JSON
mockHTTPClient("{{{{}}}}")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

// Null on invalid JSON
mockHTTPClient("{null:\"bar\"}")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)
}

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

// empty host
var settings = analytics.fetchSettings("foo", "")
assertNull(settings)
assertEquals(emptySettings, settings)

// not a host name
settings = analytics.fetchSettings("foo", "http://blah")
assertNull(settings)
assertEquals(emptySettings, settings)

// emoji
settings = analytics.fetchSettings("foo", "😃")
assertNull(settings)
assertEquals(emptySettings, settings)
}

@Test
fun `fetchSettings returns null when Settings string is null for known properties`() {
// Null if integrations is null
mockHTTPClient("{\"integrations\":null}")
var settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)

// Null if plan is null
mockHTTPClient("{\"plan\":null}")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)

// Null if edgeFunction is null
mockHTTPClient("{\"edgeFunction\":null}")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)

// Null if middlewareSettings is null
mockHTTPClient("{\"middlewareSettings\":null}")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertTrue(settings?.integrations?.isEmpty() ?: true, "Integrations should be empty")
assertTrue(settings?.plan?.isEmpty() ?: true, "Plan should be empty")
assertTrue(settings?.edgeFunction?.isEmpty() ?: true, "EdgeFunction should be empty")
assertTrue(settings?.middlewareSettings?.isEmpty() ?: true, "MiddlewareSettings should be empty")
assertTrue(settings?.metrics?.isEmpty() ?: true, "Metrics should be empty")
assertTrue(settings?.consentSettings?.isEmpty() ?: true, "ConsentSettings should be empty")

// // Null if plan is null
// mockHTTPClient("{\"plan\":null}")
// settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
// assertNull(settings)
//
// // Null if edgeFunction is null
// mockHTTPClient("{\"edgeFunction\":null}")
// settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
// assertNull(settings)
//
// // Null if middlewareSettings is null
// mockHTTPClient("{\"middlewareSettings\":null}")
// settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
// assertNull(settings)
}

@Test
fun `known Settings property types must match json type`() {

val emptySettings = analytics.fetchSettings("emptySettingsObject", "cdn-settings.segment.com/v1")
// integrations must be a JSON object
mockHTTPClient("{\"integrations\":{}}")
var settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
Expand All @@ -329,21 +336,21 @@ class SettingsTests {
// Null if integrations is a number
mockHTTPClient("{\"integrations\":123}")
settings = analytics.fetchSettings("foo", "cdn-settings.segment.com/v1")
assertNull(settings)
assertEquals(emptySettings, settings)

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

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

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