Add basic validations to identifier setters

README.md

@@ -157,6 +157,10 @@ Klaviyo.setEmail("robin@example.com")
     .setProfileAttribute(ProfileKey.FIRST_NAME, "Robin")
 ```
 
+**Note:** We trim leading and trailing whitespace off of identifier values. 
+Empty strings will be ignored with a logged warning. If you are trying to remove an identifier's value,
+use `setProfile` or `resetProfile`.
+
 ### Anonymous Tracking
 Klaviyo will track unidentified users with an autogenerated ID whenever a push token is set or an event is created.
 That way, you can collect push tokens and track events prior to collecting profile identifiers such as email or

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

@@ -7,6 +7,7 @@ import android.content.Intent
 import com.klaviyo.analytics.model.Event
 import com.klaviyo.analytics.model.EventKey
 import com.klaviyo.analytics.model.EventMetric
+import com.klaviyo.analytics.model.PROFILE_IDENTIFIERS
 import com.klaviyo.analytics.model.Profile
 import com.klaviyo.analytics.model.ProfileKey
 import com.klaviyo.analytics.networking.ApiClient
@@ -67,7 +68,9 @@ object Klaviyo {
 
     /**
      * Assign new identifiers and attributes to the currently tracked profile.
-     * If a profile has already been identified it will be overwritten by calling [resetProfile].
+     * If a profile has already been identified, it will be overwritten by calling [resetProfile].
+     *
+     * Whitespace will be trimmed from all identifier values.
      *
      * The SDK keeps track of current profile details to
      * build analytics requests with profile identifiers
@@ -82,15 +85,38 @@ object Klaviyo {
             resetProfile()
         }
 
-        UserInfo.externalId = profile.externalId ?: ""
-        UserInfo.email = profile.email ?: ""
-        UserInfo.phoneNumber = profile.phoneNumber ?: ""
-        profileOperationQueue.debounceProfileUpdate(profile)
+        // Copy the profile object, so we aren't mutating the argument
+        val mutableProfile = Profile().merge(profile)
+
+        // Route identifiers to the explicit setter functions to re-use that validator logic
+        mutableProfile.externalId?.let {
+            setExternalId(it)
+            mutableProfile.externalId = null
+        }
+
+        mutableProfile.email?.let {
+            setEmail(it)
+            mutableProfile.email = null
+        }
+
+        mutableProfile.phoneNumber?.let {
+            setPhoneNumber(it)
+            mutableProfile.phoneNumber = null
+        }
+
+        // Enqueue any remaining profile attributes
+        if (mutableProfile.propertyCount() > 0) {
+            profileOperationQueue.debounceProfileUpdate(mutableProfile)
+        }
     }
 
     /**
      * Assigns an email address to the currently tracked Klaviyo profile
      *
+     * Whitespace will be trimmed from the value.
+     * Calling this method with an empty string or the same value as currently set
+     * will be ignored. To clear identifiers, use [resetProfile].
+     *
      * The SDK keeps track of current profile details to
      * build analytics requests with profile identifiers
      *
@@ -113,6 +139,10 @@ object Klaviyo {
      * NOTE: Phone number format is not validated, but should conform to Klaviyo formatting
      * see (documentation)[https://help.klaviyo.com/hc/en-us/articles/360046055671-Accepted-phone-number-formats-for-SMS-in-Klaviyo]
      *
+     * Whitespace will be trimmed from the value.
+     * Calling this method with an empty string or the same value as currently set
+     * will be ignored. To clear identifiers, use [resetProfile].
+     *
      * The SDK keeps track of current profile details to
      * build analytics requests with profile identifiers
      *
@@ -139,6 +169,10 @@ object Klaviyo {
      * NOTE: Please consult (documentation)[https://help.klaviyo.com/hc/en-us/articles/12902308138011-Understanding-identity-resolution-in-Klaviyo-]
      * to familiarize yourself with identity resolution before using this identifier.
      *
+     * Whitespace will be trimmed from the value.
+     * Calling this method with an empty string or the same value as currently set
+     * will be ignored. To clear identifiers, use [resetProfile].
+     *
      * The SDK keeps track of current profile details to
      * build analytics requests with profile identifiers
      *
@@ -195,22 +229,31 @@ object Klaviyo {
      * @return Returns [Klaviyo] for call chaining
      */
     fun setProfileAttribute(propertyKey: ProfileKey, value: String): Klaviyo = safeApply {
-        when (propertyKey) {
-            ProfileKey.EMAIL -> {
-                UserInfo.email = value
-                profileOperationQueue.debounceProfileUpdate(UserInfo.getAsProfile())
-            }
-            ProfileKey.EXTERNAL_ID -> {
-                UserInfo.externalId = value
-                profileOperationQueue.debounceProfileUpdate(UserInfo.getAsProfile())
-            }
-            ProfileKey.PHONE_NUMBER -> {
-                UserInfo.phoneNumber = value
-                profileOperationQueue.debounceProfileUpdate(UserInfo.getAsProfile())
-            }
-            else -> {
-                profileOperationQueue.debounceProfileUpdate(Profile(mapOf(propertyKey to value)))
+        if (PROFILE_IDENTIFIERS.contains(propertyKey)) {
+            value.trim().ifEmpty {
+                Registry.log.warning(
+                    "Empty string for $propertyKey will be ignored. To clear identifiers use resetProfile."
+                )
+                null
+            }?.also { validatedIdentifier ->
+                var property by when (propertyKey) {
+                    ProfileKey.EXTERNAL_ID -> UserInfo::externalId
+                    ProfileKey.EMAIL -> UserInfo::email
+                    ProfileKey.PHONE_NUMBER -> UserInfo::phoneNumber
+                    else -> return@safeApply
+                }
+
+                if (property != validatedIdentifier) {
+                    property = validatedIdentifier
+                    profileOperationQueue.debounceProfileUpdate(UserInfo.getAsProfile())
+                } else {
+                    Registry.log.info(
+                        "$propertyKey value was unchanged, the update will be ignored."
+                    )
+                }
             }
+        } else {
+            profileOperationQueue.debounceProfileUpdate(Profile(mapOf(propertyKey to value)))
         }
     }
 

sdk/analytics/src/main/java/com/klaviyo/analytics/model/BaseModel.kt

@@ -29,6 +29,8 @@ abstract class BaseModel<Key, Self>(properties: Map<Key, Serializable>?)
         this.setProperty(key, value)
     }
 
+    fun propertyCount(): Int = propertyMap.count()
+
     /**
      * Convert this data model into a simple map
      */

sdk/analytics/src/main/java/com/klaviyo/analytics/model/ProfileKey.kt

@@ -58,3 +58,10 @@ sealed class ProfileKey(name: String) : Keyword(name) {
         else -> name
     }
 }
+
+internal val PROFILE_IDENTIFIERS = arrayOf(
+    ProfileKey.EXTERNAL_ID,
+    ProfileKey.EMAIL,
+    ProfileKey.PHONE_NUMBER,
+    ProfileKey.ANONYMOUS_ID
+)

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

@@ -183,6 +183,87 @@ internal class KlaviyoTest : BaseTest() {
         assertEquals(stubMiddleName, profile[stubMiddleNameKey])
     }
 
+    @Test
+    fun `Whitespace is trimmed off of profile identifiers for fluent setters`() {
+        Klaviyo.setExternalId("\t$EXTERNAL_ID \n")
+            .setEmail("$EMAIL \t\n")
+            .setPhoneNumber(" $PHONE \t\n")
+
+        verifyProfileDebounced()
+        val profile = capturedProfile.captured
+        assertEquals(EXTERNAL_ID, profile.externalId)
+        assertEquals(EMAIL, profile.email)
+        assertEquals(PHONE, profile.phoneNumber)
+    }
+
+    @Test
+    fun `Whitespace is trimmed off of profile identifiers for setProfile`() {
+        Klaviyo.setProfile(
+            Profile(
+                externalId = "\t$EXTERNAL_ID \n",
+                email = "$EMAIL \t\n",
+                phoneNumber = " $PHONE \t\n"
+            )
+        )
+
+        verifyProfileDebounced()
+        val profile = capturedProfile.captured
+        assertEquals(EXTERNAL_ID, profile.externalId)
+        assertEquals(EMAIL, profile.email)
+        assertEquals(PHONE, profile.phoneNumber)
+    }
+
+    @Test
+    fun `Empty identifiers are ignored with warning`() {
+        Klaviyo.setProfile(
+            Profile(
+                externalId = EXTERNAL_ID,
+                email = EMAIL,
+                phoneNumber = PHONE
+            )
+        )
+
+        verifyProfileDebounced()
+        val profile = capturedProfile.captured
+        assertEquals(EXTERNAL_ID, profile.externalId)
+        assertEquals(EMAIL, profile.email)
+        assertEquals(PHONE, profile.phoneNumber)
+
+        Klaviyo.setExternalId("")
+        Klaviyo.setEmail("")
+        Klaviyo.setPhoneNumber("")
+
+        verifyProfileDebounced() // Should not have enqueued a new request
+        verify(exactly = 3) { logSpy.warning(any(), null) }
+        assertEquals(EXTERNAL_ID, UserInfo.externalId)
+        assertEquals(EMAIL, UserInfo.email)
+        assertEquals(PHONE, UserInfo.phoneNumber)
+    }
+
+    @Test
+    fun `Unchanged profile identifiers do not enqueue new API requests`() {
+        Klaviyo.setProfile(
+            Profile(
+                externalId = EXTERNAL_ID,
+                email = EMAIL,
+                phoneNumber = PHONE
+            )
+        )
+
+        verifyProfileDebounced()
+        val profile = capturedProfile.captured
+        assertEquals(EXTERNAL_ID, profile.externalId)
+        assertEquals(EMAIL, profile.email)
+        assertEquals(PHONE, profile.phoneNumber)
+
+        Klaviyo.setExternalId(EXTERNAL_ID)
+        Klaviyo.setEmail(EMAIL)
+        Klaviyo.setPhoneNumber(PHONE)
+
+        // We should not have enqueued a new request, so still should only have been called once
+        verifyProfileDebounced()
+    }
+
     @Test
     fun `setProfile is debounced`() {
         Klaviyo.setProfile(Profile().setEmail(EMAIL))