From 9a06034fe0b43941e4ba63a2413858a29000aa0a Mon Sep 17 00:00:00 2001 From: Niall Date: Tue, 4 Jun 2024 14:41:42 +0100 Subject: [PATCH 1/4] fallback to default settings, not null --- .../main/java/com/segment/analytics/kotlin/core/Settings.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index ecd60ce5..ed6dc55c 100644 --- a/core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt +++ b/core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt @@ -112,6 +112,7 @@ internal fun Analytics.fetchSettings( val connection = HTTPClient(writeKey, this.configuration.requestFactory).settings(cdnHost) val settingsString = connection.inputStream?.bufferedReader()?.use(BufferedReader::readText) ?: "" + //configuration.defaultSettings = LenientJson.decodeFromString(settingsString) log("Fetched Settings: $settingsString") LenientJson.decodeFromString(settingsString) } catch (ex: Exception) { @@ -121,5 +122,5 @@ internal fun Analytics.fetchSettings( it["writekey"] = writeKey it["message"] = "Error retrieving settings" } - null + configuration.defaultSettings } \ No newline at end of file From d0594591c6389c00c041655026c5a14c6a05433d Mon Sep 17 00:00:00 2001 From: Niall Date: Tue, 4 Jun 2024 14:44:07 +0100 Subject: [PATCH 2/4] fallback to default settings, not null --- core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt | 1 - 1 file changed, 1 deletion(-) 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 index ed6dc55c..d4db087c 100644 --- a/core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt +++ b/core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt @@ -112,7 +112,6 @@ internal fun Analytics.fetchSettings( val connection = HTTPClient(writeKey, this.configuration.requestFactory).settings(cdnHost) val settingsString = connection.inputStream?.bufferedReader()?.use(BufferedReader::readText) ?: "" - //configuration.defaultSettings = LenientJson.decodeFromString(settingsString) log("Fetched Settings: $settingsString") LenientJson.decodeFromString(settingsString) } catch (ex: Exception) { From 42e2ca71e21c707317b88d0ed5096f888c9b0537 Mon Sep 17 00:00:00 2001 From: Niall Date: Tue, 4 Jun 2024 15:48:05 +0100 Subject: [PATCH 3/4] update tests --- .../analytics/kotlin/core/SettingsTests.kt | 81 ++++++++++--------- 1 file changed, 44 insertions(+), 37 deletions(-) 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 index d67d6963..d30ec720 100644 --- 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,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() @@ -104,7 +104,7 @@ class SettingsTests { // load settings again mockHTTPClient() analytics.checkSettings() - verify (exactly = 1) { + verify (exactly = 2) { mockPlugin.update(any(), Plugin.UpdateType.Refresh) } } @@ -232,67 +232,69 @@ 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("") 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 @@ -300,27 +302,32 @@ class SettingsTests { // 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 +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) } } \ No newline at end of file From 0c6afd915d88b07f02c555905223f6053ca4c81c Mon Sep 17 00:00:00 2001 From: Didier Garcia Date: Thu, 3 Oct 2024 14:38:36 -0400 Subject: [PATCH 4/4] test: remove commented out code after confirming that the current test logic is correct. --- .../kotlin/com/segment/analytics/kotlin/core/SettingsTests.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 index d30ec720..56a1bda8 100644 --- 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()