refactor: use kotlinx.serialization for DB requests

.gitignore

@@ -50,7 +50,12 @@ Thumbs.db
 .idea/
 
 # IDEA/Android Studio Ignore exceptions - this lets you share things you specifically want all team members to use
+!/.idea/
+/.idea/*
 !/.idea/codeStyles/
+# Keep dictionaries to have less false positives in spellcheck inspection.
+# Each user gets their own dictionary and these dictionaries are merged
+!.idea/dictionaries
 
 # Crowdin files
 ankidroid.zip
@@ -74,4 +79,4 @@ target
 
 # Ignore kotlin 2.0 compiler files (.salive: session-is-alive)
 # https://github.com/JetBrains/kotlin/blob/ca34e5d2fd255ed0501bae4fae3d3691dc40d375/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerRunner.kt#L458
-/.kotlin
+/.kotlin

rsdroid/src/main/java/net/ankiweb/rsdroid/Backend.kt

@@ -24,19 +24,27 @@ import anki.backend.GeneratedBackend
 import anki.generic.Int64
 import com.google.protobuf.ByteString
 import com.google.protobuf.InvalidProtocolBufferException
+import kotlinx.serialization.ExperimentalSerializationApi
 import kotlinx.serialization.json.Json
 import kotlinx.serialization.json.JsonArray
+import kotlinx.serialization.json.JsonElement
+import kotlinx.serialization.json.JsonNull
+import kotlinx.serialization.json.JsonPrimitive
+import kotlinx.serialization.json.JsonUnquotedLiteral
+import kotlinx.serialization.json.add
+import kotlinx.serialization.json.buildJsonObject
 import kotlinx.serialization.json.jsonArray
+import kotlinx.serialization.json.put
+import kotlinx.serialization.json.putJsonArray
 import net.ankiweb.rsdroid.database.NotImplementedException
 import net.ankiweb.rsdroid.database.SQLHandler
-import org.json.JSONArray
-import org.json.JSONObject
 import org.slf4j.LoggerFactory
 import java.io.Closeable
 import java.io.File
 import java.util.concurrent.locks.ReentrantReadWriteLock
 import kotlin.concurrent.read
 import kotlin.concurrent.write
+import java.lang.reflect.Array as ReflectArray
 
 private val logger = LoggerFactory.getLogger(Backend::class.java)
 
@@ -209,23 +217,89 @@ open class Backend(
 }
 
 /**
- * Build a JSON DB request
+ * Build a JSON DB request.
+ *
+ * TODO: consider a typed protobuf request upstream: the RunDbCommand* RPCs in
+ *  anki/proto/anki/ankidroid.proto take generic.Json. That would remove the JSON
+ *  encode/parse on every statement and allow binding ByteArray values directly.
  */
-private fun dbRequestJson(
+internal fun dbRequestJson(
     sql: String = "",
     bindArgs: Array<out Any?> = emptyArray(),
     firstRowOnly: Boolean = false,
 ): ByteString {
-    val o =
-        JSONObject().apply {
+    val request =
+        buildJsonObject {
             put("kind", "query")
             put("sql", sql)
-            put("args", JSONArray(bindArgs.toList()))
+            putJsonArray("args") { bindArgs.forEach { add(it.toBindArgJson()) } }
             put("first_row_only", firstRowOnly)
         }
-    return ByteString.copyFromUtf8(o.toString())
+    return ByteString.copyFromUtf8(request.toString())
 }
 
+/**
+ * Use org.json's number formatting: the backend binds JSON `3` as SQLite INTEGER, but `3.0` as REAL.
+ *
+ * Affects queries such as `SELECT typeof(?)`.
+ */
+@OptIn(ExperimentalSerializationApi::class)
+private fun Any?.toBindArgJson(): JsonElement =
+    when (this) {
+        null -> JsonNull
+        is String -> JsonPrimitive(this)
+        is Char -> JsonPrimitive(toString())
+        is Boolean -> JsonPrimitive(this)
+        // Preserve org.json's signed-byte array encoding. The backend accepts
+        // bytes 0..127 as blobs and rejects negative values, as before.
+        is ByteArray -> JsonArray(map { JsonPrimitive(it) })
+        is Collection<*> -> JsonArray(map { it.toBindArgJson() })
+        is Map<*, *> ->
+            if (keys.all { it is String }) {
+                buildJsonObject {
+                    for ((key, value) in this@toBindArgJson) {
+                        put(key as String, value.toBindArgJson())
+                    }
+                }
+            } else {
+                // org.json returned null when wrapping a map with invalid keys.
+                JsonNull
+            }
+        is Double, is Float -> {
+            val value = (this as Number).toDouble()
+            require(value.isFinite()) {
+                "JSON does not allow non-finite numbers: $value"
+            }
+            val asLong = value.toLong()
+            val literal =
+                when {
+                    // org.json wrote Double -0.0 as -0, but Float -0.0f as 0.
+                    this is Double && equals(-0.0) -> "-0"
+                    value == asLong.toDouble() -> asLong.toString()
+                    else -> toString()
+                }
+            JsonUnquotedLiteral(literal)
+        }
+        is Int, is Long, is Short, is Byte -> JsonUnquotedLiteral(toString())
+        else ->
+            when {
+                javaClass.isArray ->
+                    JsonArray(
+                        List(ReflectArray.getLength(this)) {
+                            ReflectArray.get(this, it).toBindArgJson()
+                        },
+                    )
+                // Accept Android callers' org.json containers, including subclasses,
+                // without requiring org.json on the JVM classpath.
+                generateSequence<Class<*>>(javaClass) { it.superclass }.any {
+                    it.name == "org.json.JSONArray" || it.name == "org.json.JSONObject"
+                } -> Json.parseToJsonElement(toString())
+                javaClass.`package`?.name?.startsWith("java.") == true -> JsonPrimitive(toString())
+                // org.json wrapped other objects as null. This also covers JSONObject.NULL.
+                else -> JsonNull
+            }
+    }
+
 /**
  * Unpack success/error tuple from backend, and throw if error.
  */

rsdroid/src/test/java/net/ankiweb/DbRequestJsonCompatibilityTest.kt

@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+package net.ankiweb
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import anki.ankidroid.SqlValue.DataCase
+import net.ankiweb.rsdroid.BackendException
+import net.ankiweb.rsdroid.BackendFactory.getBackend
+import net.ankiweb.rsdroid.testing.RustBackendLoader.ensureSetup
+import org.json.JSONArray
+import org.json.JSONObject
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/** Covers argument types that Android's org.json used to convert for us. */
+@RunWith(AndroidJUnit4::class)
+class DbRequestJsonCompatibilityTest {
+    @Test
+    fun arraysAndCollectionsRemainBlobs() {
+        val arguments =
+            listOf(
+                intArrayOf(0, 128, 255),
+                longArrayOf(0, 128, 255),
+                shortArrayOf(0, 128, 255),
+                arrayOf(0, 128, 255),
+                listOf(0, 128, 255),
+                linkedSetOf(0, 128, 255),
+                JSONArray(listOf(0, 128, 255)),
+                object : JSONArray(listOf(0, 128, 255)) {},
+            )
+        ensureSetup()
+        getBackend().use { backend ->
+            backend.openCollection(":memory:")
+            for (argument in arguments) {
+                val result = backend.fullQueryProto("SELECT typeof(?1), hex(?1)", arrayOf(argument))
+                val fields = result.result.getRows(0).fieldsList
+                assertEquals(argument.javaClass.name, "blob", fields[0].stringValue)
+                assertEquals(argument.javaClass.name, "0080FF", fields[1].stringValue)
+            }
+        }
+    }
+
+    @Test
+    fun jsonNullAndUnsupportedObjectsRemainNull() {
+        val arguments =
+            listOf(
+                JSONObject.NULL,
+                object {
+                    override fun toString() = "should not be bound as text"
+                },
+            )
+        ensureSetup()
+        getBackend().use { backend ->
+            backend.openCollection(":memory:")
+            for (argument in arguments) {
+                val result = backend.fullQueryProto("SELECT typeof(?1), ?1", arrayOf(argument))
+                val fields = result.result.getRows(0).fieldsList
+                assertEquals(argument.javaClass.name, "null", fields[0].stringValue)
+                assertEquals(argument.javaClass.name, DataCase.DATA_NOT_SET, fields[1].dataCase)
+            }
+        }
+    }
+
+    @Test
+    fun mapsAndJsonObjectsAreStillRejected() {
+        val arguments = listOf(mapOf("value" to 1), JSONObject().put("value", 1))
+        ensureSetup()
+        getBackend().use { backend ->
+            backend.openCollection(":memory:")
+            for (argument in arguments) {
+                assertThrows(BackendException::class.java) {
+                    backend.fullQueryProto("SELECT ?", arrayOf(argument))
+                }
+            }
+        }
+    }
+}

rsdroid/src/test/java/net/ankiweb/DbRequestJsonTest.kt

@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package net.ankiweb
+
+import net.ankiweb.rsdroid.dbRequestJson
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
+import org.junit.Test
+
+/**
+ * Tests the JSON sent to the backend.
+ * `/` is no longer escaped; serde treats it equivalently.
+ */
+class DbRequestJsonTest {
+    @Test
+    fun requestMatchesOrgJsonFormat() {
+        // The backend rejects booleans, but their JSON should match the previous output.
+        val json =
+            dbRequestJson(
+                "select ?, ?, ?, ?, ?, ?",
+                arrayOf<Any?>(null, "quote\" emoji🎴", 42L, 1.0, 2.5, true),
+            ).toStringUtf8()
+        assertEquals(
+            """{"kind":"query","sql":"select ?, ?, ?, ?, ?, ?","args":[null,"quote\" emoji🎴",42,1,2.5,true],"first_row_only":false}""",
+            json,
+        )
+    }
+
+    @Test
+    fun wholeDoublesBindAsIntegers() {
+        val json = dbRequestJson("select ?", arrayOf<Any?>(3.0)).toStringUtf8()
+        assertEquals("""{"kind":"query","sql":"select ?","args":[3],"first_row_only":false}""", json)
+    }
+
+    @Test
+    fun wholeDoublesAboveTenMillionStillBindAsIntegers() {
+        // Double.toString uses scientific notation from 1e7, but org.json wrote
+        // whole values as integers. This includes timestamps in milliseconds.
+        val json =
+            dbRequestJson(
+                "select ?, ?, ?",
+                arrayOf<Any?>(1.0E7, 1751234567890.0, 9.2233720368547758E18),
+            ).toStringUtf8()
+        assertEquals(
+            """{"kind":"query","sql":"select ?, ?, ?","args":[10000000,1751234567890,9223372036854775807],"first_row_only":false}""",
+            json,
+        )
+    }
+
+    @Test
+    fun floatsMatchOrgJson() {
+        val json = dbRequestJson("select ?, ?, ?, ?", arrayOf<Any?>(1.5f, 2.0f, 1.0E8f, 0.1f)).toStringUtf8()
+        assertEquals(
+            """{"kind":"query","sql":"select ?, ?, ?, ?","args":[1.5,2,100000000,0.1],"first_row_only":false}""",
+            json,
+        )
+    }
+
+    @Test
+    fun negativeZeroMatchesOrgJson() {
+        // org.json wrote Double -0.0 as -0, but Float -0.0f as 0.
+        val json = dbRequestJson("select ?, ?", arrayOf<Any?>(-0.0, -0.0f)).toStringUtf8()
+        assertEquals("""{"kind":"query","sql":"select ?, ?","args":[-0,0],"first_row_only":false}""", json)
+    }
+
+    @Test
+    fun positiveZeroMatchesOrgJson() {
+        val json = dbRequestJson("select ?, ?", arrayOf<Any?>(0.0, 0.0f)).toStringUtf8()
+        assertEquals("""{"kind":"query","sql":"select ?, ?","args":[0,0],"first_row_only":false}""", json)
+    }
+
+    @Test
+    fun longLimitsArePreserved() {
+        val json =
+            dbRequestJson(
+                "select ?, ?, ?",
+                arrayOf<Any?>(-1L, Long.MAX_VALUE, Long.MIN_VALUE),
+            ).toStringUtf8()
+        assertEquals(
+            """{"kind":"query","sql":"select ?, ?, ?","args":[-1,9223372036854775807,-9223372036854775808],"first_row_only":false}""",
+            json,
+        )
+    }
+
+    @Test
+    fun stringsAreEscapedCorrectly() {
+        val json =
+            dbRequestJson(
+                "select ?, ?",
+                arrayOf<Any?>("a/b", "line\nbreak\ttab\\bs\u0001"),
+            ).toStringUtf8()
+        assertEquals(
+            """{"kind":"query","sql":"select ?, ?","args":["a/b","line\nbreak\ttab\\bs\u0001"],"first_row_only":false}""",
+            json,
+        )
+    }
+
+    @Test
+    fun nonFiniteNumbersAreRejected() {
+        for (value in listOf<Any>(Double.NaN, Double.POSITIVE_INFINITY, Float.NaN)) {
+            assertThrows(IllegalArgumentException::class.java) {
+                dbRequestJson("select ?", arrayOf<Any?>(value))
+            }
+        }
+    }
+
+    @Test
+    fun blobsKeepTheirArrayEncoding() {
+        val json = dbRequestJson("select ?", arrayOf<Any?>(byteArrayOf(0, 1, 127))).toStringUtf8()
+        assertEquals(
+            """{"kind":"query","sql":"select ?","args":[[0,1,127]],"first_row_only":false}""",
+            json,
+        )
+    }
+
+    @Test
+    fun firstRowOnlyIsSet() {
+        val json = dbRequestJson("select 1", emptyArray(), firstRowOnly = true).toStringUtf8()
+        assertEquals("""{"kind":"query","sql":"select 1","args":[],"first_row_only":true}""", json)
+    }
+}