KCBC-179 KotlinxSerializationJsonSerializer does not work with Query …

…parameters

Motivation
----------
Prior to this change, it's impossible to use
our experimental `kotlinx.serialization` integration
to set query parameters.

kotlinx.serialization refuses to serialize anything
unless you give it precise type information.

Modifications
-------------
Add `named` and `positional` query parameter overloads
that capture the type info required by `kotlinx.serialization`.

Deprecated the existing `named` and `positional`
methods.

Added a `namedFrom` method that gets named parameters
by serializing a parameter block object provided
by the user.

Change-Id: I057096e3b3aa77be40909fe833b83afa172a31b4
Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/222472
Reviewed-by: G. Blake Meike <[email protected]>
Tested-by: Build Bot <[email protected]>

kotlin-client/src/main/kotlin/com/couchbase/client/kotlin/query/QueryParameters.kt

@@ -16,44 +16,179 @@
 
 package com.couchbase.client.kotlin.query
 
+import com.couchbase.client.core.deps.com.fasterxml.jackson.databind.node.ArrayNode
+import com.couchbase.client.core.deps.com.fasterxml.jackson.databind.node.ObjectNode
+import com.couchbase.client.core.json.Mapper
+import com.couchbase.client.kotlin.codec.JsonSerializer
+import com.couchbase.client.kotlin.codec.TypeRef
+import com.couchbase.client.kotlin.codec.typeRef
+import com.couchbase.client.kotlin.query.QueryParameters.Companion.named
+import com.couchbase.client.kotlin.query.QueryParameters.Companion.positional
+
 /**
  * Create instances using the [positional] or [named] factory methods.
  */
 public sealed class QueryParameters {
+    internal open fun serializeIfNamed(serializer: JsonSerializer): ObjectNode? = null
+    internal open fun serializeIfPositional(serializer: JsonSerializer): ArrayNode? = null
 
-    internal abstract fun inject(queryJson: MutableMap<String, Any?>)
+    public object None : QueryParameters()
 
-    public object None : QueryParameters() {
-        override fun inject(queryJson: MutableMap<String, Any?>) {
+    private class Positional(
+        private val values: List<ValueAndType<*>>,
+    ) : QueryParameters() {
+        override fun serializeIfPositional(serializer: JsonSerializer): ArrayNode {
+            val node = Mapper.createArrayNode()
+            values.forEach { node.add(serializer.serializeAsJsonNode(it)) }
+            return node
         }
     }
 
-    internal class Named internal constructor(
-        private val values: Map<String, Any?>,
+    private class Named(
+        private val nameToValue: Map<String, ValueAndType<*>>,
     ) : QueryParameters() {
-        override fun inject(queryJson: MutableMap<String, Any?>): Unit =
-            values.forEach { (key, value) ->
-                queryJson[key.addPrefixIfAbsent("$")] = value
-            }
-
-        private fun String.addPrefixIfAbsent(prefix: String) =
-            if (startsWith(prefix)) this else prefix + this
+        override fun serializeIfNamed(serializer: JsonSerializer): ObjectNode {
+            val node = Mapper.createObjectNode()
+            nameToValue.forEach { node.replace(it.key, serializer.serializeAsJsonNode(it.value)) }
+            return node
+        }
     }
 
-    internal class Positional internal constructor(
-        private val values: List<Any?>,
+    private class NamedFromParameterBlock(
+        private val value: ValueAndType<*>,
     ) : QueryParameters() {
-        override fun inject(queryJson: MutableMap<String, Any?>): Unit {
-            if (values.isNotEmpty()) {
-                queryJson["args"] = values
-            }
+        override fun serializeIfNamed(serializer: JsonSerializer): ObjectNode =
+            serializer.serializeAsJsonNode(value) as ObjectNode
+    }
+
+    @PublishedApi
+    internal data class ValueAndType<T>(val value: T, val type: TypeRef<T>) {
+        companion object {
+            private val any = typeRef<Any?>()
+            fun untyped(value: Any?) = ValueAndType(value, any)
         }
     }
 
+    internal fun <T> JsonSerializer.serializeAsJsonNode(valueAndType: ValueAndType<T>) =
+        Mapper.decodeIntoTree(serialize(valueAndType.value, valueAndType.type))
+
     public companion object {
-        // positional has no varargs overload because there would be ambiguity in the case of values that are themselves lists.
-        public fun positional(values: List<Any?>): QueryParameters = Positional(values)
-        public fun named(values: Map<String, Any?>): QueryParameters = Named(values)
-        public fun named(vararg values: Pair<String, Any?>): QueryParameters = Named(values.toMap())
+        /**
+         * Values to plug into positional placeholders in the query statement.
+         * ```
+         * parameters = QueryParameters.positional {
+         *     param("airline") // replacement for first ?
+         *     param(3)         // replacement for second ?
+         * }
+         * ```
+         * @sample com.couchbase.client.kotlin.samples.queryWithPositionalParameters
+         */
+        public fun positional(paramSetterBlock: PositionalBuilder.() -> Unit): QueryParameters {
+            val builder = PositionalBuilder()
+            builder.apply(paramSetterBlock)
+            return builder.build()
+        }
+
+        public class PositionalBuilder internal constructor() {
+            private val list: MutableList<ValueAndType<*>> = mutableListOf()
+
+            public inline fun <reified T> param(value: T) {
+                typedParam(ValueAndType(value, typeRef<T>()))
+            }
+
+            @PublishedApi
+            internal fun <T> typedParam(value: ValueAndType<T>) {
+                list.add(value)
+            }
+
+            internal fun build(): QueryParameters = Positional(list)
+        }
+
+        /**
+         * Values to plug into named placeholders in the query statement.
+         * ```
+         * parameters = QueryParameters.named {
+         *     param("type", "airline")
+         *     param("limit", 3)
+         * }
+         * ```
+         * @sample com.couchbase.client.kotlin.samples.queryWithNamedParameters
+         */
+        public fun named(paramSetterBlock: NamedBuilder.() -> Unit): QueryParameters {
+            val builder = NamedBuilder()
+            builder.apply(paramSetterBlock)
+            return builder.build()
+        }
+
+        public class NamedBuilder internal constructor() {
+            private val map: MutableMap<String, ValueAndType<*>> = mutableMapOf()
+
+            public inline fun <reified T> param(name: String, value: T) {
+                typedParam(name, ValueAndType(value, typeRef<T>()))
+            }
+
+            @PublishedApi
+            internal fun <T> typedParam(name: String, value: ValueAndType<T>) {
+                map[name] = value
+            }
+
+            internal fun build(): QueryParameters = Named(map)
+        }
+
+        /**
+         * Sets query parameters by using the query's JSON serializer to serialize the
+         * given object. The resulting JSON Object is used as the named parameter map.
+         *
+         * For example, if you have a data class like this:
+         * ```
+         *     // Annotate as @Serializable if using kotlinx.serialization
+         *     data class MyParams(val name: String, val number: Int)
+         * ```
+         * then
+         * ```
+         *     parameters = QueryParameters.namedFrom(MyParams("Fido", 3))
+         * ```
+         * is equivalent to
+         * ```
+         *     parameters = QueryParameters.named {
+         *         param("name", "Fido")
+         *         param("number", 3)
+         *     }
+         * ```
+         * @param parameterBlock The object to serialize to get named parameters.
+         *
+         * @sample com.couchbase.client.kotlin.samples.queryWithNamedParameterBlock
+         */
+        public inline fun <reified T> namedFrom(parameterBlock: T): QueryParameters {
+            return typedNamedFrom(ValueAndType(parameterBlock, typeRef<T>()))
+        }
+
+        @PublishedApi
+        internal fun <T> typedNamedFrom(typedValue: ValueAndType<T>): QueryParameters {
+            return NamedFromParameterBlock(typedValue)
+        }
+
+        @Deprecated(
+            level = DeprecationLevel.WARNING,
+            message = "Not compatible with JsonSerializer implementations that require type information, like kotlinx.serialization." +
+                    " Please use the overload that takes a parameter builder lambda."
+        )
+        public fun named(values: Map<String, Any?>): QueryParameters = Named(values.mapValues { entry -> ValueAndType.untyped(entry.value) })
+
+        @Deprecated(
+            level = DeprecationLevel.WARNING,
+            message = "Not compatible with JsonSerializer implementations that require type information, like kotlinx.serialization." +
+                    " Please use the overload that takes a parameter builder lambda."
+        )
+        @Suppress("DeprecatedCallableAddReplaceWith")
+        public fun named(vararg values: Pair<String, Any?>): QueryParameters = named(values.toMap())
+
+        @Deprecated(
+            level = DeprecationLevel.WARNING,
+            message = "Not compatible with JsonSerializer implementations that require type information, like kotlinx.serialization." +
+                    " Please use the overload that takes a parameter builder lambda."
+        )
+        @Suppress("DeprecatedCallableAddReplaceWith")
+        public fun positional(values: List<Any?>): QueryParameters = Positional(values.map { ValueAndType.untyped(it) })
     }
 }

kotlin-client/src/main/kotlin/com/couchbase/client/kotlin/query/internal/QueryExecutor.kt

@@ -148,25 +148,12 @@ internal fun CoreQueryOptions(
         override fun maxParallelism(): Int? = maxParallelism
         override fun metrics(): Boolean = metrics
 
-        override fun namedParameters(): ObjectNode? = (parameters as? QueryParameters.Named)?.let { params ->
-            // Let the user's serializer serialize arguments
-            val map = mutableMapOf<String, Any?>()
-            params.inject(map)
-            val queryBytes = actualSerializer.serialize(map, typeRef())
-            Mapper.decodeIntoTree(queryBytes) as ObjectNode
-        }
+        override fun namedParameters(): ObjectNode? = parameters.serializeIfNamed(actualSerializer)
+        override fun positionalParameters(): ArrayNode? = parameters.serializeIfPositional(actualSerializer)
 
         override fun pipelineBatch(): Int? = pipelineBatch
         override fun pipelineCap(): Int? = pipelineCap
 
-        override fun positionalParameters(): ArrayNode? = (parameters as? QueryParameters.Positional)?.let { params ->
-            // Let the user's serializer serialize arguments
-            val map = mutableMapOf<String, Any?>()
-            params.inject(map)
-            val queryBytes = actualSerializer.serialize(map, typeRef())
-            Mapper.decodeIntoTree(queryBytes).get("args") as? ArrayNode
-        }
-
         override fun profile(): CoreQueryProfile = profile.core
 
         override fun raw(): JsonNode? {

kotlin-client/src/main/kotlin/com/couchbase/client/kotlin/samples/QuerySamples.kt

@@ -14,14 +14,18 @@
  * limitations under the License.
  */
 
+// Some examples declare unused variables for illustrative purposes
+@file:Suppress("UNUSED_VARIABLE")
+
 package com.couchbase.client.kotlin.samples
 
 import com.couchbase.client.kotlin.Cluster
 import com.couchbase.client.kotlin.query.QueryMetadata
+import com.couchbase.client.kotlin.query.QueryParameters
 import com.couchbase.client.kotlin.query.QueryResult
 import com.couchbase.client.kotlin.query.execute
+import kotlinx.serialization.Serializable
 
-@Suppress("UNUSED_VARIABLE")
 internal suspend fun singleValueQueryAnonymous(cluster: Cluster) {
     // Single-value query with anonymous result
     val count = cluster
@@ -30,7 +34,6 @@ internal suspend fun singleValueQueryAnonymous(cluster: Cluster) {
         .valueAs<Long>() // uses default name "$1"
 }
 
-@Suppress("UNUSED_VARIABLE")
 internal suspend fun singleValueQueryNamed(cluster: Cluster) {
     // Single-value query with named result
     val count = cluster
@@ -39,7 +42,6 @@ internal suspend fun singleValueQueryNamed(cluster: Cluster) {
         .valueAs<Long>("count")
 }
 
-@Suppress("UNUSED_VARIABLE")
 internal suspend fun bufferedQuery(cluster: Cluster) {
     // Buffered query, for when results are known to fit in memory
     val result: QueryResult = cluster
@@ -49,11 +51,57 @@ internal suspend fun bufferedQuery(cluster: Cluster) {
     println(result.metadata)
 }
 
-@Suppress("UNUSED_VARIABLE")
 internal suspend fun streamingQuery(cluster: Cluster) {
     // Streaming query, for when result size is large or unbounded
     val metadata: QueryMetadata = cluster
         .query("select * from `travel-sample`")
         .execute { row -> println(row) }
     println(metadata)
 }
+
+internal suspend fun queryWithNamedParameters(cluster: Cluster) {
+    // Query with named parameters
+    val result: QueryResult = cluster
+        .query(
+            "select * from `travel-sample` where type = @type limit @limit",
+            parameters = QueryParameters.named {
+                param("type", "airline")
+                param("limit", 3)
+            }
+        )
+        .execute()
+
+    result.rows.forEach { println(it) }
+}
+
+internal suspend fun queryWithPositionalParameters(cluster: Cluster) {
+    // Query with positional parameters
+    val result: QueryResult = cluster
+        .query(
+            "select * from `travel-sample` where type = ? limit ?",
+            parameters = QueryParameters.positional {
+                param("airline")
+                param(3)
+            }
+        )
+        .execute()
+
+    result.rows.forEach { println(it) }
+}
+
+internal suspend fun queryWithNamedParameterBlock(cluster: Cluster) {
+    // Query with named parameters from a parameter block
+    @Serializable // (or whatever annotation your JsonSerializer requires)
+    data class MyParameters(val type: String, val limit: Int)
+
+    val result: QueryResult = cluster
+        .query(
+            "select * from `travel-sample` where type = @type limit @limit",
+            parameters = QueryParameters.namedFrom(
+                MyParameters(type = "airline", limit = 3)
+            )
+        )
+        .execute()
+
+    result.rows.forEach { println(it) }
+}