Fallback to the defaultSettings if cdn cannot be reached (#231)

niallzato · didiergarcia · web-flow · commit d37d2d32cecb · 2024-10-03T14:53:05.000-04:00
* fallback to default settings, not null

* fallback to default settings, not null

* update tests

* test: remove commented out code after confirming that the current test logic is correct.

---------

Co-authored-by: Didier Garcia &lt;digarcia@twilio.com&gt;
diff --git a/core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt b/core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt
@@ -121,5 +121,5 @@ internal fun Analytics.fetchSettings(
         it["writekey"] = writeKey
         it["message"] = "Error retrieving settings"
     }
-    null
+    configuration.defaultSettings
 }
diff --git a/core/src/test/kotlin/com/segment/analytics/kotlin/core/SettingsTests.kt b/core/src/test/kotlin/com/segment/analytics/kotlin/core/SettingsTests.kt
@@ -88,9 +88,7 @@ class SettingsTests {
 
         // no settings available, should not be called
         analytics.add(mockPlugin)
-        verify (exactly = 0){
-            mockPlugin.update(any(), any())
-        }
+
 
         // load settings
         mockHTTPClient()
@@ -104,7 +102,7 @@ class SettingsTests {
         // load settings again
         mockHTTPClient()
         analytics.checkSettings()
-        verify (exactly =  1) {
+        verify (exactly =  2) {
             mockPlugin.update(any(), Plugin.UpdateType.Refresh)
         }
     }
@@ -232,95 +230,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")
@@ -329,21 +334,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)
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -121,5 +121,5 @@ internal fun Analytics.fetchSettings(`
`121`	`121`	`it["writekey"] = writeKey`
`122`	`122`	`it["message"] = "Error retrieving settings"`
`123`	`123`	`}`
`124`		`- null`
	`124`	`+ configuration.defaultSettings`
`125`	`125`	`}`