From 6473dadba8a7fff9689b35ad2ca9f9e5aff4d0d0 Mon Sep 17 00:00:00 2001 From: Alberto Schiabel Date: Fri, 1 Sep 2023 01:17:32 +0200 Subject: [PATCH] =?UTF-8?q?fix(js=20connectors):=20JSON=20creation,=20`con?= =?UTF-8?q?version::conv=5Fparams`=20=C3=A0=20la=20`mysql`=20(#4184)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(js-connectors): use one schema per database provider * fix(js-connectors): fix JSON creation, add missing "conversion::conv_params", similarly to mysql's * chore: fix typo. * chore: add clarification comment --- .../smoke-test-js/prisma/mysql/schema.prisma | 6 ++ .../prisma/postgres/schema.prisma | 8 +- .../js/smoke-test-js/src/test.ts | 78 +++++++++++++++++-- query-engine/js-connectors/src/conversion.rs | 60 ++++++++++++++ query-engine/js-connectors/src/lib.rs | 1 + query-engine/js-connectors/src/proxy.rs | 6 +- query-engine/js-connectors/src/queryable.rs | 15 ++-- 7 files changed, 157 insertions(+), 17 deletions(-) create mode 100644 query-engine/js-connectors/src/conversion.rs diff --git a/query-engine/js-connectors/js/smoke-test-js/prisma/mysql/schema.prisma b/query-engine/js-connectors/js/smoke-test-js/prisma/mysql/schema.prisma index d1a3df2b50f3..7de112bb71e3 100644 --- a/query-engine/js-connectors/js/smoke-test-js/prisma/mysql/schema.prisma +++ b/query-engine/js-connectors/js/smoke-test-js/prisma/mysql/schema.prisma @@ -107,3 +107,9 @@ model Author { @@map("authors") } + +model Product { + id String @id @default(cuid()) + properties Json + properties_null Json? +} diff --git a/query-engine/js-connectors/js/smoke-test-js/prisma/postgres/schema.prisma b/query-engine/js-connectors/js/smoke-test-js/prisma/postgres/schema.prisma index 620f78b3cffb..920b18ff7a6d 100644 --- a/query-engine/js-connectors/js/smoke-test-js/prisma/postgres/schema.prisma +++ b/query-engine/js-connectors/js/smoke-test-js/prisma/postgres/schema.prisma @@ -84,10 +84,16 @@ enum type_test_enum_column_null { } model Author { - id Int @id @default(autoincrement()) + id Int @id @default(autoincrement()) firstName String lastName String age Int @@map("authors") } + +model Product { + id String @id @default(cuid()) + properties Json + properties_null Json? +} diff --git a/query-engine/js-connectors/js/smoke-test-js/src/test.ts b/query-engine/js-connectors/js/smoke-test-js/src/test.ts index 4c297c9e39b2..0a1f3605b269 100644 --- a/query-engine/js-connectors/js/smoke-test-js/src/test.ts +++ b/query-engine/js-connectors/js/smoke-test-js/src/test.ts @@ -14,10 +14,9 @@ export async function smokeTest(db: ErrorCapturingConnector, prismaSchemaRelativ await engine.connect('trace') console.log('[nodejs] connected') - // console.log('[nodejs] isHealthy', await conn.isHealthy()) - - const test = new SmokeTest(engine, db, db.flavour) + const test = new SmokeTest(engine, db) + await test.testJSON() await test.testTypeTest2() await test.testFindManyTypeTest() await test.createAutoIncrement() @@ -46,7 +45,70 @@ export async function smokeTest(db: ErrorCapturingConnector, prismaSchemaRelativ } class SmokeTest { - constructor(private readonly engine: QueryEngineInstance, private readonly connector: ErrorCapturingConnector, readonly flavour: ErrorCapturingConnector['flavour']) {} + readonly flavour: ErrorCapturingConnector['flavour'] + + constructor(private readonly engine: QueryEngineInstance, private readonly connector: ErrorCapturingConnector) { + this.flavour = connector.flavour + } + + async testJSON() { + const json = JSON.stringify({ + foo: 'bar', + baz: 1, + }) + + const created = await this.engine.query(` + { + "action": "createOne", + "modelName": "Product", + "query": { + "arguments": { + "data": { + "properties": ${json}, + "properties_null": null + } + }, + "selection": { + "properties": true + } + } + } + `, 'trace', undefined) + + console.log('[nodejs] created', JSON.stringify(JSON.parse(created), null, 2)) + + const resultSet = await this.engine.query(` + { + "action": "findMany", + "modelName": "Product", + "query": { + "selection": { + "id": true, + "properties": true, + "properties_null": true + } + } + } + `, 'trace', undefined) + console.log('[nodejs] findMany resultSet', JSON.stringify(JSON.parse(resultSet), null, 2)) + + await this.engine.query(` + { + "action": "deleteMany", + "modelName": "Product", + "query": { + "arguments": { + "where": {} + }, + "selection": { + "count": true + } + } + } + `, 'trace', undefined) + + return resultSet + } async testTypeTest2() { const create = await this.engine.query(` @@ -68,7 +130,7 @@ class SmokeTest { console.log('[nodejs] create', JSON.stringify(JSON.parse(create), null, 2)) - const findMany = await this.engine.query(` + const resultSet = await this.engine.query(` { "action": "findMany", "modelName": "type_test_2", @@ -85,7 +147,7 @@ class SmokeTest { } `, 'trace', undefined) - console.log('[nodejs] findMany', JSON.stringify(JSON.parse(findMany), null, 2)) + console.log('[nodejs] resultSet', JSON.stringify(JSON.parse(resultSet), null, 2)) await this.engine.query(` { @@ -101,8 +163,10 @@ class SmokeTest { } } `, 'trace', undefined) + + return resultSet } - + async testFindManyTypeTest() { await this.testFindManyTypeTestMySQL() await this.testFindManyTypeTestPostgres() diff --git a/query-engine/js-connectors/src/conversion.rs b/query-engine/js-connectors/src/conversion.rs new file mode 100644 index 000000000000..c64954e389cf --- /dev/null +++ b/query-engine/js-connectors/src/conversion.rs @@ -0,0 +1,60 @@ +use napi::bindgen_prelude::{FromNapiValue, ToNapiValue}; +use quaint::ast::Value as QuaintValue; +use serde::Serialize; +use serde_json::value::Value as JsonValue; + +#[derive(Debug, Serialize)] +#[serde(untagged)] +pub enum JSArg { + RawString(String), + Value(serde_json::Value), +} + +impl From for JSArg { + fn from(v: JsonValue) -> Self { + JSArg::Value(v) + } +} + +// FromNapiValue is the napi equivalent to serde::Deserialize. +// Note: we can safely leave this unimplemented as we don't need deserialize JSArg back to napi_value +// (nor we need to). However, removing this altogether would cause a compile error. +impl FromNapiValue for JSArg { + unsafe fn from_napi_value(_env: napi::sys::napi_env, _napi_value: napi::sys::napi_value) -> napi::Result { + unreachable!() + } +} + +// ToNapiValue is the napi equivalent to serde::Serialize. +impl ToNapiValue for JSArg { + unsafe fn to_napi_value(env: napi::sys::napi_env, value: Self) -> napi::Result { + match value { + JSArg::RawString(s) => ToNapiValue::to_napi_value(env, s), + JSArg::Value(v) => ToNapiValue::to_napi_value(env, v), + } + } +} + +pub fn conv_params(params: &[QuaintValue<'_>]) -> serde_json::Result> { + let mut values = Vec::with_capacity(params.len()); + + for pv in params { + let res = match pv { + QuaintValue::Json(s) => match s { + Some(ref s) => { + let json_str = serde_json::to_string(s)?; + JSArg::RawString(json_str) + } + None => JsonValue::Null.into(), + }, + quaint_value => { + let json: JsonValue = quaint_value.clone().into(); + json.into() + } + }; + + values.push(res); + } + + Ok(values) +} diff --git a/query-engine/js-connectors/src/lib.rs b/query-engine/js-connectors/src/lib.rs index 00c75218e96a..083d9b69005d 100644 --- a/query-engine/js-connectors/src/lib.rs +++ b/query-engine/js-connectors/src/lib.rs @@ -8,6 +8,7 @@ //! mod async_js_function; +mod conversion; mod error; mod proxy; mod queryable; diff --git a/query-engine/js-connectors/src/proxy.rs b/query-engine/js-connectors/src/proxy.rs index 8331a2e46638..7b7c67ca6977 100644 --- a/query-engine/js-connectors/src/proxy.rs +++ b/query-engine/js-connectors/src/proxy.rs @@ -2,6 +2,7 @@ use core::panic; use std::str::FromStr; use crate::async_js_function::AsyncJsFunction; +use crate::conversion::JSArg; use crate::transaction::JsTransaction; use napi::bindgen_prelude::{FromNapiValue, ToNapiValue}; use napi::{JsObject, JsString}; @@ -152,12 +153,11 @@ pub enum ColumnType { #[derive(Debug)] pub struct Query { pub sql: String, - pub args: Vec, + pub args: Vec, } + /// Handle data-type conversion from a JSON value to a Quaint value. /// This is used for most data types, except those that require connector-specific handling, e.g., `ColumnType::Boolean`. -/// In the future, after https://github.com/prisma/team-orm/issues/257, every connector-specific handling should be moved -/// out of Rust and into TypeScript. fn js_value_to_quaint( json_value: serde_json::Value, column_type: ColumnType, diff --git a/query-engine/js-connectors/src/queryable.rs b/query-engine/js-connectors/src/queryable.rs index e75542099e01..ba68498e827c 100644 --- a/query-engine/js-connectors/src/queryable.rs +++ b/query-engine/js-connectors/src/queryable.rs @@ -1,4 +1,7 @@ -use crate::proxy::{CommonProxy, DriverProxy, Query}; +use crate::{ + conversion, + proxy::{CommonProxy, DriverProxy, Query}, +}; use async_trait::async_trait; use napi::JsObject; use psl::datamodel_connector::Flavour; @@ -122,16 +125,16 @@ impl QuaintQueryable for JsBaseQueryable { } impl JsBaseQueryable { - async fn build_query(sql: &str, values: &[quaint::Value<'_>]) -> Query { + async fn build_query(sql: &str, values: &[quaint::Value<'_>]) -> quaint::Result { let sql: String = sql.to_string(); - let args = values.iter().map(|v| v.clone().into()).collect(); - Query { sql, args } + let args = conversion::conv_params(values)?; + Ok(Query { sql, args }) } async fn do_query_raw(&self, sql: &str, params: &[Value<'_>]) -> quaint::Result { let len = params.len(); let serialization_span = info_span!("js:query:args", user_facing = true, "length" = %len); - let query = Self::build_query(sql, params).instrument(serialization_span).await; + let query = Self::build_query(sql, params).instrument(serialization_span).await?; let sql_span = info_span!("js:query:sql", user_facing = true, "db.statement" = %sql); let result_set = self.proxy.query_raw(query).instrument(sql_span).await?; @@ -144,7 +147,7 @@ impl JsBaseQueryable { async fn do_execute_raw(&self, sql: &str, params: &[Value<'_>]) -> quaint::Result { let len = params.len(); let serialization_span = info_span!("js:query:args", user_facing = true, "length" = %len); - let query = Self::build_query(sql, params).instrument(serialization_span).await; + let query = Self::build_query(sql, params).instrument(serialization_span).await?; let sql_span = info_span!("js:query:sql", user_facing = true, "db.statement" = %sql); let affected_rows = self.proxy.execute_raw(query).instrument(sql_span).await?;