Merge pull request #2004 from OneSignal/fix/json-ext-functions

Fix JSON Kotlin Extension Functions

Author: jkasten2

OneSignalSDK/build.gradle

@@ -18,6 +18,8 @@ buildscript {
         ktlintPluginVersion = '11.6.1'
         ktlintVersion = '1.0.1'
         junitVersion = '4.13.2'
+        // DO NOT upgrade for tests, using an old version so it matches AOSP
+        tdunningJsonForTest = '1.0'
     }
 
     repositories {

OneSignalSDK/onesignal/core/build.gradle

@@ -88,8 +88,10 @@ dependencies {
     testImplementation("androidx.test:core-ktx:1.4.0")
     testImplementation("androidx.test:core:1.4.0")
     testImplementation("io.mockk:mockk:1.13.2")
-    testImplementation("org.json:json:20180813")
     testImplementation("org.jetbrains.kotlin:kotlin-reflect:$kotlinVersion")
+
+    // com.tdunning:json is needed for non-Robolectric tests.
+    testImplementation("com.tdunning:json:$tdunningJsonForTest")
 }
 
 ktlint {
@@ -99,4 +101,4 @@ ktlint {
     ]
 }
 
-apply from: '../maven-push.gradle'
+apply from: '../maven-push.gradle'

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

@@ -2,6 +2,7 @@ package com.onesignal.common
 
 import org.json.JSONArray
 import org.json.JSONObject
+import org.json.JSONObject.NULL
 
 /**
  * Retrieve an [Int] from the [JSONObject] safely.
@@ -95,15 +96,46 @@ fun JSONObject.safeJSONObject(name: String): JSONObject? {
 
 /**
  * Create a [Map] from the [JSONObject].
+ * Supports nested JSONObjects and JSONArrays.
  */
-fun JSONObject.toMap(): Map<String, Any> {
-    val map = mutableMapOf<String, Any>()
-
+fun JSONObject.toMap(): MutableMap<String, Any?> {
+    val results: MutableMap<String, Any?> = HashMap()
     for (key in this.keys()) {
-        map[key] = this[key]
+        val value = this[key]
+        results[key] =
+            if (NULL.equals(value)) {
+                null
+            } else if (value is JSONObject) {
+                value.toMap()
+            } else if (value is JSONArray) {
+                value.toList()
+            } else {
+                value
+            }
     }
+    return results
+}
 
-    return map
+/**
+ * Create a [List] from the [JSONArray].
+ * Supports nested JSONObjects and JSONArrays.
+ */
+fun JSONArray.toList(): List<Any?>? {
+    val size = this.length()
+    val results: MutableList<Any?> = ArrayList(size)
+    for (i in 0 until size) {
+        val element = this.opt(i)
+        if (NULL.equals(element)) {
+            results.add(null)
+        } else if (element is JSONArray) {
+            results.add(element.toList())
+        } else if (element is JSONObject) {
+            results.add(element.toMap())
+        } else {
+            results.add(element)
+        }
+    }
+    return results
 }
 
 /**

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

@@ -0,0 +1,68 @@
+package com.onesignal.common
+
+import io.kotest.core.spec.style.DescribeSpec
+import io.kotest.matchers.shouldBe
+import io.kotest.runner.junit4.KotestTestRunner
+import io.mockk.mockkStatic
+import io.mockk.verify
+import org.json.JSONArray
+import org.json.JSONObject
+import org.junit.runner.RunWith
+
+@RunWith(KotestTestRunner::class)
+class JSONObjectExtensionsTest : DescribeSpec({
+    describe("toMap") {
+        // Some org.json JVM libraries define their own toMap. We want to
+        // ensure we are testing ours, not theirs.
+        it("is using our extension function") {
+            mockkStatic(JSONObject::toMap) {
+                JSONObject().toMap()
+                verify { any<JSONObject>().toMap() }
+            }
+        }
+
+        it("supports primitives") {
+            val test =
+                JSONObject()
+                    .put("String", "String")
+                    .put("Boolean", true)
+                    .put("Integer", 1)
+                    .put("Long", 2L)
+                    .put("Float", 3.3f)
+                    .put("Double", Double.MAX_VALUE)
+
+            test.toMap() shouldBe
+                mapOf(
+                    "String" to "String",
+                    "Boolean" to true,
+                    "Integer" to 1,
+                    "Long" to 2L,
+                    "Float" to 3.3f,
+                    "Double" to Double.MAX_VALUE,
+                )
+        }
+
+        describe("JSONArray") {
+            it("supports empty") {
+                val test = JSONObject().put("MyArray", JSONArray())
+                test.toMap() shouldBe
+                    mapOf("MyArray" to emptyList<Any>())
+            }
+            it("supports one item") {
+                val test = JSONObject().put("MyArray", JSONArray().put("String"))
+                test.toMap() shouldBe
+                    mapOf("MyArray" to listOf("String"))
+            }
+        }
+
+        it("supports JSONObject") {
+            val test =
+                JSONObject()
+                    .put("MyNestedJSONObject", JSONObject().put("String", "String"))
+            test.toMap() shouldBe
+                mapOf(
+                    "MyNestedJSONObject" to mapOf("String" to "String"),
+                )
+        }
+    }
+})

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/selftest/JSONObjectEnvTest.kt

@@ -0,0 +1,47 @@
+package com.onesignal.selftest
+
+import com.onesignal.common.toMap
+import com.onesignal.testhelpers.extensions.RobolectricTest
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.core.spec.style.funSpec
+import io.kotest.matchers.shouldBe
+import io.kotest.runner.junit4.KotestTestRunner
+import org.json.JSONObject
+import org.junit.runner.RunWith
+
+/**
+ * This purpose of this file is to ensure org.json classes being used in
+ * other tests matches real Android devices.
+*/
+
+val keyAOSPBehavior =
+    funSpec {
+        // This is a key oddity only AOSP seems to do
+        test("JSONObject.getString stringifies JSON object") {
+            val json =
+                JSONObject()
+                    .put(
+                        "obj",
+                        JSONObject().put("nested", "value"),
+                    )
+            json.getString("obj") shouldBe "{\"nested\":\"value\"}"
+        }
+    }
+
+@RobolectricTest
+@RunWith(KotestTestRunner::class)
+class JSONObjectRobolectricEnvTest : FunSpec({
+    test("ensure our src JSON Kotlin extension methods work with @RobolectricTest") {
+        val test = JSONObject()
+        test.put("test", "test")
+        // Ensuring this doesn't throw
+        test.toMap()
+    }
+
+    include(keyAOSPBehavior)
+})
+
+@RunWith(KotestTestRunner::class)
+class JSONObjectJVMEnvTest : FunSpec({
+    include(keyAOSPBehavior)
+})

OneSignalSDK/onesignal/in-app-messages/build.gradle

@@ -88,8 +88,10 @@ dependencies {
     testImplementation("androidx.test:core-ktx:1.4.0")
     testImplementation("androidx.test:core:1.4.0")
     testImplementation("io.mockk:mockk:1.13.2")
-    testImplementation("org.json:json:20180813")
     testImplementation("org.jetbrains.kotlin:kotlin-reflect:$kotlinVersion")
+
+    // com.tdunning:json is needed for non-Robolectric tests.
+    testImplementation("com.tdunning:json:$tdunningJsonForTest")
 }
 
 ktlint {
@@ -99,4 +101,4 @@ ktlint {
     ]
 }
 
-apply from: '../maven-push.gradle'
+apply from: '../maven-push.gradle'

OneSignalSDK/onesignal/location/build.gradle

@@ -87,9 +87,11 @@ dependencies {
     testImplementation("androidx.test:core-ktx:1.4.0")
     testImplementation("androidx.test:core:1.4.0")
     testImplementation("io.mockk:mockk:1.13.2")
-    testImplementation("org.json:json:20180813")
     testImplementation("org.jetbrains.kotlin:kotlin-reflect:$kotlinVersion")
     testImplementation("com.google.android.gms:play-services-location:18.0.0")
+
+    // com.tdunning:json is needed for non-Robolectric tests.
+    testImplementation("com.tdunning:json:$tdunningJsonForTest")
 }
 
 ktlint {
@@ -99,4 +101,4 @@ ktlint {
     ]
 }
 
-apply from: '../maven-push.gradle'
+apply from: '../maven-push.gradle'

OneSignalSDK/onesignal/notifications/build.gradle

@@ -100,8 +100,10 @@ dependencies {
     testImplementation("androidx.test:core-ktx:1.4.0")
     testImplementation("androidx.test:core:1.4.0")
     testImplementation("io.mockk:mockk:1.13.2")
-    testImplementation("org.json:json:20180813")
     testImplementation("org.jetbrains.kotlin:kotlin-reflect:$kotlinVersion")
+
+    // com.tdunning:json is needed for non-Robolectric tests.
+    testImplementation("com.tdunning:json:$tdunningJsonForTest")
 }
 
 ktlint {
@@ -111,4 +113,4 @@ ktlint {
     ]
 }
 
-apply from: '../maven-push.gradle'
+apply from: '../maven-push.gradle'