fix(js connectors): JSON creation, conversion::conv_params à la `my…

…sql` (#4184) * 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
prisma · Aug 31, 2023 · 6473dad · 6473dad
1 parent bb89459
commit 6473dad
Show file tree

Hide file tree

Showing 7 changed files with 157 additions and 17 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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<JsonValue> 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<Self> {
+        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<napi::sys::napi_value> {
+        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<Vec<JSArg>> {
+    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
@@ -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
@@ -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<serde_json::Value>,
+    pub args: Vec<JSArg>,
 }
+
 /// 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
@@ -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<Query> {
         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<ResultSet> {
         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<u64> {
         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?;