From 8dea171cc45c5e88f28868f6c72d6ac60ef19c23 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:43:59 +0100 Subject: [PATCH 1/2] refactor: use kotlinx.serialization for DB requests Replace `org.json`, but keep wire compatibility * except for '/' escaping - not needed Assisted-by: GPT-6 --- .../main/java/net/ankiweb/rsdroid/Backend.kt | 90 +++++++++++-- .../ankiweb/DbRequestJsonCompatibilityTest.kt | 78 +++++++++++ .../java/net/ankiweb/DbRequestJsonTest.kt | 121 ++++++++++++++++++ 3 files changed, 281 insertions(+), 8 deletions(-) create mode 100644 rsdroid/src/test/java/net/ankiweb/DbRequestJsonCompatibilityTest.kt create mode 100644 rsdroid/src/test/java/net/ankiweb/DbRequestJsonTest.kt diff --git a/rsdroid/src/main/java/net/ankiweb/rsdroid/Backend.kt b/rsdroid/src/main/java/net/ankiweb/rsdroid/Backend.kt index 6deead738..54b47f23d 100644 --- a/rsdroid/src/main/java/net/ankiweb/rsdroid/Backend.kt +++ b/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 = 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>(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. */ diff --git a/rsdroid/src/test/java/net/ankiweb/DbRequestJsonCompatibilityTest.kt b/rsdroid/src/test/java/net/ankiweb/DbRequestJsonCompatibilityTest.kt new file mode 100644 index 000000000..a07f7e3a3 --- /dev/null +++ b/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)) + } + } + } + } +} diff --git a/rsdroid/src/test/java/net/ankiweb/DbRequestJsonTest.kt b/rsdroid/src/test/java/net/ankiweb/DbRequestJsonTest.kt new file mode 100644 index 000000000..8274149de --- /dev/null +++ b/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(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(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(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(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(-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(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(-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("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(Double.NaN, Double.POSITIVE_INFINITY, Float.NaN)) { + assertThrows(IllegalArgumentException::class.java) { + dbRequestJson("select ?", arrayOf(value)) + } + } + } + + @Test + fun blobsKeepTheirArrayEncoding() { + val json = dbRequestJson("select ?", arrayOf(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) + } +} From 26ec1b653c0c1eb13e79fb16238de24633ba239a Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sat, 3 Oct 2026 16:26:48 +0100 Subject: [PATCH 2/2] lint: add shared dictionary Assisted-by: GPT-6 --- .gitignore | 7 ++++++- .idea/dictionaries/project.xml | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 .idea/dictionaries/project.xml diff --git a/.gitignore b/.gitignore index 88984d72f..7b76a36fb 100644 --- a/.gitignore +++ b/.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 \ No newline at end of file +/.kotlin diff --git a/.idea/dictionaries/project.xml b/.idea/dictionaries/project.xml new file mode 100644 index 000000000..a479afaa1 --- /dev/null +++ b/.idea/dictionaries/project.xml @@ -0,0 +1,7 @@ + + + + serde + + +