Refactor singleton service initializers (#172)

* Convert SystemClock to class

* These shouldn't have been mocked in this test class, its masking a bug.

* Remove unnecessary, slightly risky, object initializers in core.

* Restore system clock to object -- I take it back, we don't have to get rid of objects, just have to be more particular about using object initializers.

* KLog can be a singleton object, no risk there.

* This simple fix would take care of the issue, but i still don't love KlaviyoApiClient being a singleton object that can hit an unrecoverable exception in its initializer...

* Found a balanced solution for KlaviyoApiClient -- converting this to a class was complicated by the inner class, and besides it works well as a singleton. The initializer was the real problem, so I extract the listeners from the initializer and created a startService method. This is safe to run multiple times if the SDK is re-initialized.

* Prefer this way: we don't need to wrap any of the code in this method body except the methods that are themselves protected anyway

* Forgot to add back starting the service in these tests' setup

* Naming convention things

* First shot at a pre-init buffer for failed operations. Needs tests

* Added test of in-memory slate

* I don't like import *

* Added comments, logging, and wrapped the retry operations in safeCall

* Trivial: move error logging out of exception class. Now that we wrap all SDK functions in safeCall, it is more straightforward to log from there.

* Allow ONLY handlePush to be buffered

* Remove unused test

* keep 3.0 version increment

* don't add this back

* never trust github

* Update versions.properties

---------

Co-authored-by: Evan Masseau <>

README.md

@@ -284,6 +284,8 @@ This method will check if the app was opened from a notification originating fro
 `Opened Push` event with required message tracking parameters. For example:
 
 ```kotlin
+// Main Activity
+
 override fun onCreate(savedInstanceState: Bundle?) {
     /* ... */
 
@@ -298,8 +300,9 @@ override fun onNewIntent(intent: Intent?) {
 }
 ```
 
-**Note:** Intent handling may differ depending on your app's architecture. Adjust this example to your use-case, 
-ensuring that `Klaviyo.handlePush(intent)` is called when your app is opened from a notification.
+**Note:** Intent handling may differ depending on your app's architecture. By default, the Klaviyo SDK will use your
+app's launch intent for a tapped notification. Adjust this example to your use-case, ensuring that 
+`Klaviyo.handlePush(intent)` is called whenever your app is opened from a notification.
 
 #### Deep Linking 
 [Deep Links](https://help.klaviyo.com/hc/en-us/articles/14750403974043) allow you to navigate to a particular

sdk/analytics/src/main/java/com/klaviyo/analytics/Klaviyo.kt

@@ -66,11 +66,12 @@ object Klaviyo {
             registerActivityLifecycleCallbacks(Registry.lifecycleCallbacks)
         } ?: throw LifecycleException()
 
+        Registry.get<ApiClient>().startService()
+
         Registry.register<State>(KlaviyoState())
         Registry.register<StateSideEffects>(StateSideEffects())
-        Registry.get<State>().apiKey = apiKey
 
-        Registry.get<ApiClient>().startService()
+        Registry.get<State>().apiKey = apiKey
 
         if (preInitQueue.isNotEmpty()) {
             Registry.log.info(
@@ -222,6 +223,10 @@ object Klaviyo {
     /**
      * Creates an [Event] associated with the currently tracked profile
      *
+     * While it is preferable to [initialize] before interacting with the Klaviyo SDK,
+     * due to timing issues on some platforms, events are stored in an in-memory buffer prior to initialization,
+     * and will be replayed once you initialize with your public API key.
+     *
      * @param event A map-like object representing the event attributes
      * @return Returns [Klaviyo] for call chaining
      */
@@ -245,6 +250,10 @@ object Klaviyo {
      * From an opened push Intent, creates an [EventMetric.OPENED_PUSH] [Event]
      * containing appropriate tracking parameters
      *
+     * While it is preferable to [initialize] before interacting with the Klaviyo SDK,
+     * due to timing issues on some platforms, events are stored in an in-memory buffer prior to initialization,
+     * and will be replayed once you initialize with your public API key.
+     *
      * @param intent the [Intent] from opening a notification
      */
     fun handlePush(intent: Intent?) = safeApply(preInitQueue) {

sdk/analytics/src/test/java/com/klaviyo/analytics/KlaviyoPreInitializeTest.kt

@@ -25,12 +25,12 @@ internal class KlaviyoPreInitializeTest : BaseTest() {
         every { applicationContext(any()) } returns this
         every { build() } answers {
             configBuilt = true
-            configMock
+            mockConfig
         }
 
         // Mock real world behavior where accessing data store prior to initializing throws MissingConfig
-        // While also allowing dataStoreSpy to work post-initialization
-        every { dataStoreSpy.fetch(any()) } answers {
+        // While also allowing spyDataStore to work post-initialization
+        every { spyDataStore.fetch(any()) } answers {
             if (!configBuilt) {
                 throw MissingConfig()
             } else {
@@ -52,7 +52,7 @@ internal class KlaviyoPreInitializeTest : BaseTest() {
     override fun setup() {
         super.setup()
         every { Registry.configBuilder } returns mockBuilder
-        every { contextMock.applicationContext } returns mockk<Application>().apply {
+        every { mockContext.applicationContext } returns mockk<Application>().apply {
             every { unregisterActivityLifecycleCallbacks(any()) } returns Unit
             every { registerActivityLifecycleCallbacks(any()) } returns Unit
         }
@@ -70,7 +70,7 @@ internal class KlaviyoPreInitializeTest : BaseTest() {
     }
 
     private inline fun <reified T> assertCaught() where T : Throwable {
-        verify { logSpy.error(any(), any<T>()) }
+        verify { spyLog.error(any(), any<T>()) }
     }
 
     @Test
@@ -85,12 +85,12 @@ internal class KlaviyoPreInitializeTest : BaseTest() {
 
         Klaviyo.initialize(
             apiKey = API_KEY,
-            applicationContext = contextMock
+            applicationContext = mockContext
         )
 
         Klaviyo.initialize(
             apiKey = "different-$API_KEY",
-            applicationContext = contextMock
+            applicationContext = mockContext
         )
 
         verify(inverse = true) {

sdk/analytics/src/test/java/com/klaviyo/analytics/KlaviyoTest.kt

@@ -61,35 +61,35 @@ internal class KlaviyoTest : BaseTest() {
     }
 
     private val capturedProfile = slot<Profile>()
-    private val apiClientMock: ApiClient = mockk<ApiClient>().apply {
+    private val mockApiClient: ApiClient = mockk<ApiClient>().apply {
         every { startService() } returns Unit
         every { onApiRequest(any(), any()) } returns Unit
         every { enqueueProfile(capture(capturedProfile)) } returns Unit
         every { enqueueEvent(any(), any()) } returns Unit
         every { enqueuePushToken(any(), any()) } returns Unit
     }
 
-    private val builderMock = mockk<Config.Builder>().apply {
+    private val mockBuilder = mockk<Config.Builder>().apply {
         every { apiKey(any()) } returns this
         every { applicationContext(any()) } returns this
-        every { build() } returns configMock
+        every { build() } returns mockConfig
     }
 
     private val mockApplication = mockk<Application>().apply {
-        every { contextMock.applicationContext } returns this
+        every { mockContext.applicationContext } returns this
         every { unregisterActivityLifecycleCallbacks(any()) } returns Unit
         every { registerActivityLifecycleCallbacks(any()) } returns Unit
     }
 
     @Before
     override fun setup() {
         super.setup()
-        every { Registry.configBuilder } returns builderMock
-        Registry.register<ApiClient>(apiClientMock)
+        every { Registry.configBuilder } returns mockBuilder
+        Registry.register<ApiClient>(mockApiClient)
         DevicePropertiesTest.mockDeviceProperties()
         Klaviyo.initialize(
             apiKey = API_KEY,
-            applicationContext = contextMock
+            applicationContext = mockContext
         )
     }
 
@@ -107,24 +107,25 @@ internal class KlaviyoTest : BaseTest() {
 
     @Test
     fun `Registered mock api`() {
-        assertEquals(apiClientMock, Registry.get<ApiClient>())
+        assertEquals(mockApiClient, Registry.get<ApiClient>())
+        verify { mockApiClient.startService() }
     }
 
     @Test
     fun `Initialize properly creates new config service and attaches lifecycle listeners`() {
         val expectedListener = Registry.lifecycleCallbacks
         verifyAll {
-            builderMock.apiKey(API_KEY)
-            builderMock.applicationContext(contextMock)
-            builderMock.build()
+            mockBuilder.apiKey(API_KEY)
+            mockBuilder.applicationContext(mockContext)
+            mockBuilder.build()
             mockApplication.unregisterActivityLifecycleCallbacks(match { it == expectedListener })
             mockApplication.registerActivityLifecycleCallbacks(match { it == expectedListener })
         }
     }
 
     private fun verifyProfileDebounced() {
         staticClock.execute(debounceTime.toLong())
-        verify(exactly = 1) { apiClientMock.enqueueProfile(any()) }
+        verify(exactly = 1) { mockApiClient.enqueueProfile(any()) }
     }
 
     @Test
@@ -133,7 +134,7 @@ internal class KlaviyoTest : BaseTest() {
             .setEmail(EMAIL)
             .setPhoneNumber(PHONE)
 
-        verify(exactly = 0) { apiClientMock.enqueueProfile(any()) }
+        verify(exactly = 0) { mockApiClient.enqueueProfile(any()) }
         verifyProfileDebounced()
     }
 
@@ -150,7 +151,7 @@ internal class KlaviyoTest : BaseTest() {
             .setProfileAttribute(ProfileKey.LAST_NAME, stubLastName)
             .setProfileAttribute(stubMiddleNameKey, stubMiddleName)
 
-        verify(exactly = 0) { apiClientMock.enqueueProfile(any()) }
+        verify(exactly = 0) { mockApiClient.enqueueProfile(any()) }
         verifyProfileDebounced()
         assert(capturedProfile.isCaptured)
         val profile = capturedProfile.captured
@@ -213,7 +214,7 @@ internal class KlaviyoTest : BaseTest() {
         Klaviyo.setPhoneNumber("")
 
         verifyProfileDebounced() // Should not have enqueued a new request
-        verify(exactly = 3) { logSpy.warning(any(), null) }
+        verify(exactly = 3) { spyLog.warning(any(), null) }
         assertEquals(EXTERNAL_ID, Registry.get<State>().externalId)
         assertEquals(EMAIL, Registry.get<State>().email)
         assertEquals(PHONE, Registry.get<State>().phoneNumber)
@@ -247,7 +248,7 @@ internal class KlaviyoTest : BaseTest() {
     fun `setProfile is debounced`() {
         Klaviyo.setProfile(Profile().setEmail(EMAIL))
 
-        verify(exactly = 0) { apiClientMock.enqueueProfile(any()) }
+        verify(exactly = 0) { mockApiClient.enqueueProfile(any()) }
         verifyProfileDebounced()
     }
 
@@ -354,13 +355,13 @@ internal class KlaviyoTest : BaseTest() {
         assertNull(Registry.get<State>().externalId)
 
         // Resetting profile flushes the queue immediately
-        verify(exactly = 1) { apiClientMock.enqueueProfile(any()) }
+        verify(exactly = 1) { mockApiClient.enqueueProfile(any()) }
     }
 
     @Test
     fun `Reset removes push token from store`() {
         Registry.get<State>().email = EMAIL
-        dataStoreSpy.store("push_token", PUSH_TOKEN)
+        spyDataStore.store("push_token", PUSH_TOKEN)
 
         Klaviyo.resetProfile()
 
@@ -385,93 +386,93 @@ internal class KlaviyoTest : BaseTest() {
     @Test
     fun `Stores push token and Enqueues a push token API call`() {
         Klaviyo.setPushToken(PUSH_TOKEN)
-        assertEquals(PUSH_TOKEN, dataStoreSpy.fetch("push_token"))
+        assertEquals(PUSH_TOKEN, spyDataStore.fetch("push_token"))
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
     }
 
     @Test
     fun `Push token request is ignored if state has not changed`() {
         Klaviyo.setPushToken(PUSH_TOKEN)
-        assertEquals(PUSH_TOKEN, dataStoreSpy.fetch("push_token"))
+        assertEquals(PUSH_TOKEN, spyDataStore.fetch("push_token"))
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
 
         Klaviyo.setPushToken(PUSH_TOKEN)
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
     }
 
     @Test
     fun `Push token request is repeated if state has changed`() {
         every { DeviceProperties.backgroundDataEnabled } returns true
         Klaviyo.setPushToken(PUSH_TOKEN)
-        assertEquals(PUSH_TOKEN, dataStoreSpy.fetch("push_token"))
+        assertEquals(PUSH_TOKEN, spyDataStore.fetch("push_token"))
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
 
         every { DeviceProperties.backgroundDataEnabled } returns false
         Klaviyo.setPushToken(PUSH_TOKEN)
 
         verify(exactly = 2) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
     }
 
     @Test
     fun `Push token request is made if profile identifiers change and token is set`() {
         Klaviyo.setPushToken(PUSH_TOKEN)
-        assertEquals(PUSH_TOKEN, dataStoreSpy.fetch("push_token"))
+        assertEquals(PUSH_TOKEN, spyDataStore.fetch("push_token"))
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
 
         Klaviyo.setEmail(EMAIL)
             .setPhoneNumber(PHONE)
             .setExternalId(EXTERNAL_ID)
 
         staticClock.execute(debounceTime.toLong())
-        verify(exactly = 2) { apiClientMock.enqueuePushToken(PUSH_TOKEN, any()) }
+        verify(exactly = 2) { mockApiClient.enqueuePushToken(PUSH_TOKEN, any()) }
     }
 
     @Test
     fun `Push token request is made if profile changes and token is set`() {
         Klaviyo.setPushToken(PUSH_TOKEN)
-        assertEquals(PUSH_TOKEN, dataStoreSpy.fetch("push_token"))
+        assertEquals(PUSH_TOKEN, spyDataStore.fetch("push_token"))
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
 
         Klaviyo.setProfile(Profile().setEmail(EMAIL))
 
         staticClock.execute(debounceTime.toLong())
-        verify(exactly = 2) { apiClientMock.enqueuePushToken(PUSH_TOKEN, any()) }
+        verify(exactly = 2) { mockApiClient.enqueuePushToken(PUSH_TOKEN, any()) }
     }
 
     @Test
     fun `Push token request is made for profile attributes when token is set`() {
         Klaviyo.setPushToken(PUSH_TOKEN)
-        assertEquals(PUSH_TOKEN, dataStoreSpy.fetch("push_token"))
+        assertEquals(PUSH_TOKEN, spyDataStore.fetch("push_token"))
 
         verify(exactly = 1) {
-            apiClientMock.enqueuePushToken(PUSH_TOKEN, any())
+            mockApiClient.enqueuePushToken(PUSH_TOKEN, any())
         }
 
         Klaviyo.setProfileAttribute(ProfileKey.FIRST_NAME, "Larry")
         Klaviyo.setProfileAttribute(ProfileKey.LAST_NAME, "David")
 
         staticClock.execute(debounceTime.toLong())
-        verify(exactly = 2) { apiClientMock.enqueuePushToken(PUSH_TOKEN, any()) }
+        verify(exactly = 2) { mockApiClient.enqueuePushToken(PUSH_TOKEN, any()) }
     }
 
     @Test
@@ -483,7 +484,7 @@ internal class KlaviyoTest : BaseTest() {
 
     @Test
     fun `Fetches push token from persistent store`() {
-        dataStoreSpy.store("push_token", PUSH_TOKEN)
+        spyDataStore.store("push_token", PUSH_TOKEN)
         assertEquals(Klaviyo.getPushToken(), PUSH_TOKEN)
     }
 
@@ -492,7 +493,7 @@ internal class KlaviyoTest : BaseTest() {
         // Handle push intent
         Klaviyo.handlePush(mockIntent(stubIntentExtras))
 
-        verify { apiClientMock.enqueueEvent(any(), any()) }
+        verify { mockApiClient.enqueueEvent(any(), any()) }
     }
 
     @Test
@@ -501,7 +502,7 @@ internal class KlaviyoTest : BaseTest() {
         Klaviyo.handlePush(mockIntent(mapOf("com.other.package.message" to "3rd party push")))
         Klaviyo.handlePush(null)
 
-        verify(inverse = true) { apiClientMock.enqueueEvent(any(), any()) }
+        verify(inverse = true) { mockApiClient.enqueueEvent(any(), any()) }
     }
 
     @Test
@@ -510,7 +511,7 @@ internal class KlaviyoTest : BaseTest() {
         Klaviyo.createEvent(stubEvent)
 
         verify(exactly = 1) {
-            apiClientMock.enqueueEvent(stubEvent, any())
+            mockApiClient.enqueueEvent(stubEvent, any())
         }
     }
 
@@ -519,7 +520,7 @@ internal class KlaviyoTest : BaseTest() {
         Klaviyo.createEvent(EventMetric.VIEWED_PRODUCT)
 
         verify(exactly = 1) {
-            apiClientMock.enqueueEvent(match { it.metric == EventMetric.VIEWED_PRODUCT }, any())
+            mockApiClient.enqueueEvent(match { it.metric == EventMetric.VIEWED_PRODUCT }, any())
         }
     }
 }