fix: custom events now handle null object within the event properties (#2537)

Co-authored-by: AR Abdul Azeez <[email protected]>

Author: jinliu9508

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/JSONUtils.kt

@@ -187,7 +187,7 @@ object JSONUtils {
      * Recursively convert a JSON-serializable map into a JSON-compatible format, handling
      * nested Maps and Lists appropriately.
      */
-    fun mapToJson(map: Map<String, Any>): JSONObject {
+    fun mapToJson(map: Map<String, Any?>): JSONObject {
         val json = JSONObject()
         for ((key, value) in map) {
             json.put(key, convertToJson(value))
@@ -198,21 +198,23 @@ object JSONUtils {
     /**
      * Recursively converts maps and lists into JSON-compatible objects, transforming maps with
      * String keys into JSON objects, lists into JSON arrays, and leaving primitive values unchanged to support safe JSON serialization.
+     * Null values are converted to JSONObject.NULL to preserve them in the JSON structure.
      */
-    fun convertToJson(value: Any): Any {
+    fun convertToJson(value: Any?): Any? {
         return when (value) {
+            null -> JSONObject.NULL
             is Map<*, *> -> {
                 val subMap =
                     value.entries
                         .filter { it.key is String }
                         .associate {
-                            it.key as String to convertToJson(it.value!!)
+                            it.key as String to convertToJson(it.value)
                         }
                 mapToJson(subMap)
             }
             is List<*> -> {
                 val array = JSONArray()
-                value.forEach { array.put(convertToJson(it!!)) }
+                value.forEach { array.put(convertToJson(it)) }
                 array
             }
             else -> value

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/IUserManager.kt

@@ -175,6 +175,6 @@ interface IUserManager {
      */
     fun trackEvent(
         name: String,
-        properties: Map<String, Any>? = null,
+        properties: Map<String, Any?>? = null,
     )
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt

@@ -249,7 +249,7 @@ internal open class UserManager(
 
     override fun trackEvent(
         name: String,
-        properties: Map<String, Any>?,
+        properties: Map<String, Any?>?,
     ) {
         if (!JSONUtils.isValidJsonObject(properties)) {
             Logging.log(LogLevel.ERROR, "Custom event properties are not JSON-serializable")

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/customEvents/ICustomEventController.kt

@@ -1,8 +1,18 @@
 package com.onesignal.user.internal.customEvents
 
+/**
+ * Interface for sending custom events to track user behavior.
+ */
 interface ICustomEventController {
+    /**
+     * Sends a custom event with optional properties.
+     *
+     * @param name The name of the custom event
+     * @param properties Optional map of event properties. Can contain nested maps, lists, and null values.
+     *                   Properties will be converted to JSON format.
+     */
     fun sendCustomEvent(
         name: String,
-        properties: Map<String, Any>?,
+        properties: Map<String, Any?>?,
     )
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/customEvents/impl/CustomEventController.kt

@@ -8,6 +8,10 @@ import com.onesignal.user.internal.customEvents.ICustomEventController
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.TrackCustomEventOperation
 
+/**
+ * Controller for custom events. Handles the creation and enqueueing of custom event operations
+ * for tracking user events with optional properties.
+ */
 class CustomEventController(
     private val identityModelStore: IdentityModelStore,
     private val configModelStore: ConfigModelStore,
@@ -16,7 +20,7 @@ class CustomEventController(
 ) : ICustomEventController {
     override fun sendCustomEvent(
         name: String,
-        properties: Map<String, Any>?,
+        properties: Map<String, Any?>?,
     ) {
         val op =
             TrackCustomEventOperation(

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/customEvents/impl/CustomEventMetadata.kt

@@ -4,6 +4,10 @@ import com.onesignal.common.putSafe
 import org.json.JSONException
 import org.json.JSONObject
 
+/**
+ * Metadata for custom events containing device and SDK information.
+ * This metadata is included with custom events sent to the OneSignal backend.
+ */
 class CustomEventMetadata(
     val deviceType: String?,
     val sdk: String?,
@@ -12,6 +16,12 @@ class CustomEventMetadata(
     val deviceModel: String?,
     val deviceOS: String?,
 ) {
+    /**
+     * Converts the metadata to a JSONObject for serialization.
+     *
+     * @return JSONObject containing all metadata fields
+     * @throws JSONException if JSON serialization fails
+     */
     @Throws(JSONException::class)
     fun toJSONObject(): JSONObject {
         val json = JSONObject()

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackCustomEventOperation.kt

@@ -5,6 +5,11 @@ import com.onesignal.core.internal.operations.GroupComparisonType
 import com.onesignal.core.internal.operations.Operation
 import com.onesignal.user.internal.operations.impl.executors.CustomEventOperationExecutor
 
+/**
+ * An [Operation] to track a single custom event with properties for the current user.
+ * This operation is enqueued when a user tracks a custom event and will be processed
+ * by the [CustomEventOperationExecutor] to send the event to the OneSignal backend.
+ */
 class TrackCustomEventOperation() : Operation(CustomEventOperationExecutor.CUSTOM_EVENT) {
     /**
      * The OneSignal appId the custom event was created.

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/JSONUtilsTests.kt

@@ -772,6 +772,7 @@ class JSONUtilsTests : FunSpec({
             JSONUtils.convertToJson(true) shouldBe true
             JSONUtils.convertToJson(false) shouldBe false
             JSONUtils.convertToJson(3.14) shouldBe 3.14
+            JSONUtils.convertToJson(null) shouldBe JSONObject.NULL
         }
 
         test("should convert Map to JSONObject") {
@@ -887,18 +888,19 @@ class JSONUtilsTests : FunSpec({
 
         test("should handle List with mixed types") {
             // Given
-            val list = listOf("string", 42, true, 3.14)
+            val list = listOf("string", 42, true, 3.14, null)
 
             // When
             val result = JSONUtils.convertToJson(list)
 
             // Then
             val jsonArray = result as JSONArray
-            jsonArray.length() shouldBe 4
+            jsonArray.length() shouldBe 5
             jsonArray.getString(0) shouldBe "string"
             jsonArray.getInt(1) shouldBe 42
             jsonArray.getBoolean(2) shouldBe true
             jsonArray.getDouble(3) shouldBe 3.14
+            jsonArray.get(4) shouldBe JSONObject.NULL
         }
 
         test("should filter out non-String keys from Map") {
@@ -941,5 +943,124 @@ class JSONUtilsTests : FunSpec({
             val level2Item = level2Array.getJSONObject(0)
             level2Item.getString("level3") shouldBe "deepValue"
         }
+
+        test("should handle null values in maps") {
+            // Given
+            val map = mapOf(
+                "key1" to "value1",
+                "key2" to null,
+                "key3" to 42,
+            )
+
+            // When
+            val result = JSONUtils.convertToJson(map)
+
+            // Then
+            val jsonObject = result as JSONObject
+            jsonObject.getString("key1") shouldBe "value1"
+            jsonObject.isNull("key2") shouldBe true
+            jsonObject.getInt("key3") shouldBe 42
+        }
+
+        test("should handle null values in nested objects") {
+            // Given
+            val map = mapOf(
+                "someObject" to mapOf(
+                    "abc" to "123",
+                    "nested" to mapOf(
+                        "def" to "456",
+                    ),
+                    "ghi" to null,
+                ),
+            )
+
+            // When
+            val result = JSONUtils.convertToJson(map)
+
+            // Then
+            val jsonObject = result as JSONObject
+            val someObject = jsonObject.getJSONObject("someObject")
+            someObject.getString("abc") shouldBe "123"
+            val nested = someObject.getJSONObject("nested")
+            nested.getString("def") shouldBe "456"
+            someObject.isNull("ghi") shouldBe true
+        }
+
+        test("should handle null values in arrays") {
+            // Given
+            val map = mapOf(
+                "someArray" to listOf(1, 2),
+                "someMixedArray" to listOf(1, "2", mapOf("abc" to "123"), null),
+            )
+
+            // When
+            val result = JSONUtils.convertToJson(map)
+
+            // Then
+            val jsonObject = result as JSONObject
+            val someArray = jsonObject.getJSONArray("someArray")
+            someArray.length() shouldBe 2
+            someArray.getInt(0) shouldBe 1
+            someArray.getInt(1) shouldBe 2
+
+            val someMixedArray = jsonObject.getJSONArray("someMixedArray")
+            someMixedArray.length() shouldBe 4
+            someMixedArray.getInt(0) shouldBe 1
+            someMixedArray.getString(1) shouldBe "2"
+            val nestedObj = someMixedArray.getJSONObject(2)
+            nestedObj.getString("abc") shouldBe "123"
+            someMixedArray.get(3) shouldBe JSONObject.NULL
+        }
+
+        test("should handle complete example structure with nulls") {
+            // Given - matches the user's example structure
+            val map = mapOf(
+                "someNum" to 123,
+                "someFloat" to 3.14159,
+                "someString" to "abc",
+                "someBool" to true,
+                "someObject" to mapOf(
+                    "abc" to "123",
+                    "nested" to mapOf(
+                        "def" to "456",
+                    ),
+                    "ghi" to null,
+                ),
+                "someArray" to listOf(1, 2),
+                "someMixedArray" to listOf(1, "2", mapOf("abc" to "123"), null),
+                "someNull" to null,
+            )
+
+            // When
+            val result = JSONUtils.convertToJson(map)
+
+            // Then
+            val jsonObject = result as JSONObject
+            jsonObject.getInt("someNum") shouldBe 123
+            jsonObject.getDouble("someFloat") shouldBe 3.14159
+            jsonObject.getString("someString") shouldBe "abc"
+            jsonObject.getBoolean("someBool") shouldBe true
+
+            val someObject = jsonObject.getJSONObject("someObject")
+            someObject.getString("abc") shouldBe "123"
+            val nested = someObject.getJSONObject("nested")
+            nested.getString("def") shouldBe "456"
+            someObject.isNull("ghi") shouldBe true
+
+            val someArray = jsonObject.getJSONArray("someArray")
+            someArray.length() shouldBe 2
+            someArray.getInt(0) shouldBe 1
+            someArray.getInt(1) shouldBe 2
+
+            val someMixedArray = jsonObject.getJSONArray("someMixedArray")
+            someMixedArray.length() shouldBe 4
+            someMixedArray.getInt(0) shouldBe 1
+            someMixedArray.getString(1) shouldBe "2"
+            val nestedObj = someMixedArray.getJSONObject(2)
+            nestedObj.getString("abc") shouldBe "123"
+            someMixedArray.get(3) shouldBe JSONObject.NULL
+
+            jsonObject.isNull("someNull") shouldBe true
+        }
     }
 })

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt

@@ -210,6 +210,7 @@ class UserManagerTests : FunSpec({
                 "key3" to 5.123,
                 "key4" to mapOf("key4-1" to "value4-1"),
                 "key5" to mapOf("key5-1" to mapOf("key5-1-1" to 0)),
+                "key6" to null,
             )
 
         // When