From 6bdd55c4e16f8250d90a95040a04253d43f7abcd Mon Sep 17 00:00:00 2001 From: Serhii Tatarintsev Date: Tue, 18 Jul 2023 11:52:01 +0200 Subject: [PATCH 1/4] qe: Fix batch results processing for enums (#4072) * qe: Fix batch results processing for enums When JSON protocol is used and batching is happening, if the unique value is enum, we incorrectly compared the results with the inputs: inputs are PrismaValue::String, while outputs are PrismaValue::Enum. Fix prisma/prisma#20227 * Fix mongo tests --- .../queries/batch/select_one_singular.rs | 40 +++++++++++++++++++ query-engine/request-handlers/src/handler.rs | 5 +++ 2 files changed, 45 insertions(+) diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/batch/select_one_singular.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/batch/select_one_singular.rs index a3318dbc851c..0381c605d451 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/batch/select_one_singular.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/batch/select_one_singular.rs @@ -308,6 +308,46 @@ mod singular_batch { Ok(()) } + fn enum_id() -> String { + let schema = indoc! { + r#" + enum IdEnum { + A + B + } + + model TestModel { + #id(id, IdEnum, @id) + } + "# + }; + + schema.to_owned() + } + + #[connector_test(schema(enum_id), capabilities(Enums))] + async fn batch_enum(runner: Runner) -> TestResult<()> { + run_query!(&runner, r#"mutation { createOneTestModel(data: { id: "A" }) { id } }"#); + run_query!(&runner, r#"mutation { createOneTestModel(data: { id: "B" }) { id } }"#); + + let (res, compact_doc) = compact_batch( + &runner, + vec![ + r#"{ findUniqueTestModel(where: { id: "A" }) { id } }"#.to_string(), + r#"{ findUniqueTestModel(where: { id: "B" }) { id } }"#.to_string(), + ], + ) + .await?; + + insta::assert_snapshot!( + res.to_string(), + @r###"{"batchResult":[{"data":{"findUniqueTestModel":{"id":"A"}}},{"data":{"findUniqueTestModel":{"id":"B"}}}]}"### + ); + assert!(compact_doc.is_compact()); + + Ok(()) + } + // Regression test for https://github.com/prisma/prisma/issues/16548 #[connector_test(schema(schemas::generic))] async fn repro_16548(runner: Runner) -> TestResult<()> { diff --git a/query-engine/request-handlers/src/handler.rs b/query-engine/request-handlers/src/handler.rs index 8757593ae020..df25616c2201 100644 --- a/query-engine/request-handlers/src/handler.rs +++ b/query-engine/request-handlers/src/handler.rs @@ -236,6 +236,7 @@ impl<'a> RequestHandler<'a> { /// - DateTime/String: User-input: DateTime / Response: String /// - Int/BigInt: User-input: Int / Response: BigInt /// - (JSON protocol only) Custom types (eg: { "$type": "BigInt", value: "1" }): User-input: Scalar / Response: Object + /// - (JSON protocol only) String/Enum: User-input: String / Response: Enum /// This should likely _not_ be used outside of this specific context. fn compare_values(left: &ArgumentValue, right: &ArgumentValue) -> bool { match (left, right) { @@ -249,6 +250,10 @@ impl<'a> RequestHandler<'a> { | (ArgumentValue::Scalar(PrismaValue::BigInt(i2)), ArgumentValue::Scalar(PrismaValue::Int(i1))) => { *i1 == *i2 } + (ArgumentValue::Scalar(PrismaValue::Enum(s1)), ArgumentValue::Scalar(PrismaValue::String(s2))) + | (ArgumentValue::Scalar(PrismaValue::String(s1)), ArgumentValue::Scalar(PrismaValue::Enum(s2))) => { + *s1 == *s2 + } (ArgumentValue::Object(t1), t2) | (t2, ArgumentValue::Object(t1)) => match Self::unwrap_value(t1) { Some(t1) => Self::compare_values(t1, t2), None => left == right, From b182127de581d0bf6ec89152a6e2fb4652be0f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Tue, 18 Jul 2023 15:09:08 +0200 Subject: [PATCH 2/4] Revert "tech-debt: Context lib (#4060)" (#4066) This reverts commit 27eb2449f178cd9fe1a4b892d732cc4795f75085. --- Cargo.lock | 4 -- libs/context/Cargo.toml | 8 ---- libs/context/src/lib.rs | 87 ----------------------------------------- 3 files changed, 99 deletions(-) delete mode 100644 libs/context/Cargo.toml delete mode 100644 libs/context/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index c190ff823303..8e95a2c82d29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -552,10 +552,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "context" -version = "0.1.0" - [[package]] name = "convert_case" version = "0.6.0" diff --git a/libs/context/Cargo.toml b/libs/context/Cargo.toml deleted file mode 100644 index 885c7fd2b32d..000000000000 --- a/libs/context/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "context" -version = "0.1.0" -edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] diff --git a/libs/context/src/lib.rs b/libs/context/src/lib.rs deleted file mode 100644 index bff4fc553088..000000000000 --- a/libs/context/src/lib.rs +++ /dev/null @@ -1,87 +0,0 @@ -use std::{ - any::{Any, TypeId}, - collections::HashMap, - sync::{Arc, Mutex}, -}; - -#[derive(Debug, Default)] -pub struct Context { - store: HashMap<(String, TypeId), Box>, -} - -impl Context { - pub fn concurrent(self) -> Arc> { - Arc::new(Mutex::new(self)) - } - - pub fn insert(&mut self, key: &str, value: T) { - self.store.insert((key.to_owned(), TypeId::of::()), Box::new(value)); - } - - pub fn get(&self, key: &str) -> Option<&T> { - self.store - .get(&(key.to_owned(), TypeId::of::())) - .map(|v| v.downcast_ref::().unwrap()) - } - - pub fn get_mut(&mut self, key: &str) -> Option<&mut T> { - self.store - .get_mut(&(key.to_owned(), TypeId::of::())) - .map(|v| v.downcast_mut::().unwrap()) - } - - pub fn remove(&mut self, key: &str) -> Option { - self.store - .remove(&(key.to_owned(), TypeId::of::())) - .map(|v| *v.downcast::().unwrap()) - } -} - -#[cfg(test)] -mod tests { - use super::Context; - - #[test] - fn set_and_retrieve() { - let mut ctx: Context = Context::default(); - ctx.insert("foo", 42 as u32); - - let val: u32 = *ctx.get("foo").unwrap(); - assert_eq!(val, 42 as u32) - } - - #[test] - fn concurrent() { - let mut ctx: Context = Context::default(); - ctx.insert("foo", 42 as u32); - - assert_eq!(42 as u32, *ctx.get::("foo").unwrap()); - assert_eq!(None, ctx.get::("bar")); - - let safe_context = ctx.concurrent(); - let mut ctx = safe_context.lock().unwrap(); - ctx.insert("bar", 32 as u32); - assert_eq!(32 as u32, *ctx.get::("bar").unwrap()); - assert_eq!(42 as u32, *ctx.get::("foo").unwrap()); - } - - #[test] - fn get_mut() { - let mut ctx: Context = Context::default(); - ctx.insert("foo", 42 as u32); - - let val: &mut u32 = ctx.get_mut("foo").unwrap(); - *val = 32 as u32; - assert_eq!(32 as u32, *ctx.get::("foo").unwrap()); - } - - #[test] - fn remove() { - let mut ctx: Context = Context::default(); - ctx.insert("foo", 42 as u32); - - let val: u32 = ctx.remove("foo").unwrap(); - assert_eq!(42 as u32, val); - assert_eq!(None, ctx.get::("foo")); - } -} From 4a1c72356ffb884a5752157c21d8bac67afd1720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Thu, 20 Jul 2023 14:51:43 +0200 Subject: [PATCH 3/4] Node-drivers: Homogenize terms (#4075) * Homogenize terms prisma preview feature: 'jsConnectors' string cago feature flag: `js-connectors` crate name: `js-connectors` --- Cargo.lock | 4 +- Cargo.toml | 2 +- psl/psl-core/src/common/preview_features.rs | 4 +- .../connectors/sql-query-connector/Cargo.toml | 2 +- .../sql-query-connector/src/database/mod.rs | 2 +- .../sql-query-connector/src/database/mysql.rs | 6 +-- .../src/database/runtime.rs | 32 +++++++------- .../connectors/sql-query-connector/src/lib.rs | 2 +- .../{js-drivers => js-connectors}/Cargo.toml | 2 +- .../playground/.gitignore | 0 .../playground/README.md | 2 +- .../playground/package.json | 0 .../playground/pnpm-lock.yaml | 0 .../playground/prisma/schema.prisma | 4 +- .../src/engines/types/JsonProtocol.ts | 0 .../playground/src/engines/types/Library.ts | 2 +- .../src/engines/types/QueryEngine.ts | 0 .../src/engines/types/Transaction.ts | 0 .../playground/src/index.sql | 0 .../playground/src/index.ts | 0 .../playground/src/queryable/mock.ts | 0 .../playground/src/queryable/mysql.ts | 0 .../playground/tsconfig.json | 0 query-engine/js-connectors/src/lib.rs | 12 +++++ .../driver.rs => js-connectors/src/proxy.rs} | 44 +++++++++---------- .../src/queryable.rs | 31 +++++++++---- query-engine/js-drivers/src/lib.rs | 8 ---- query-engine/query-engine-node-api/Cargo.toml | 6 +-- .../query-engine-node-api/src/engine.rs | 4 +- 29 files changed, 93 insertions(+), 76 deletions(-) rename query-engine/{js-drivers => js-connectors}/Cargo.toml (93%) rename query-engine/{js-drivers => js-connectors}/playground/.gitignore (100%) rename query-engine/{js-drivers => js-connectors}/playground/README.md (98%) rename query-engine/{js-drivers => js-connectors}/playground/package.json (100%) rename query-engine/{js-drivers => js-connectors}/playground/pnpm-lock.yaml (100%) rename query-engine/{js-drivers => js-connectors}/playground/prisma/schema.prisma (87%) rename query-engine/{js-drivers => js-connectors}/playground/src/engines/types/JsonProtocol.ts (100%) rename query-engine/{js-drivers => js-connectors}/playground/src/engines/types/Library.ts (97%) rename query-engine/{js-drivers => js-connectors}/playground/src/engines/types/QueryEngine.ts (100%) rename query-engine/{js-drivers => js-connectors}/playground/src/engines/types/Transaction.ts (100%) rename query-engine/{js-drivers => js-connectors}/playground/src/index.sql (100%) rename query-engine/{js-drivers => js-connectors}/playground/src/index.ts (100%) rename query-engine/{js-drivers => js-connectors}/playground/src/queryable/mock.ts (100%) rename query-engine/{js-drivers => js-connectors}/playground/src/queryable/mysql.ts (100%) rename query-engine/{js-drivers => js-connectors}/playground/tsconfig.json (100%) create mode 100644 query-engine/js-connectors/src/lib.rs rename query-engine/{js-drivers/src/driver.rs => js-connectors/src/proxy.rs} (80%) rename query-engine/{js-drivers => js-connectors}/src/queryable.rs (81%) delete mode 100644 query-engine/js-drivers/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 8e95a2c82d29..b170e7270499 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1656,7 +1656,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c8af84674fe1f223a982c933a0ee1086ac4d4052aa0fb8060c12c6ad838e754" [[package]] -name = "js-drivers" +name = "js-connectors" version = "0.1.0" dependencies = [ "async-trait", @@ -3333,7 +3333,7 @@ dependencies = [ "async-trait", "connection-string", "futures", - "js-drivers", + "js-connectors", "napi", "napi-build", "napi-derive", diff --git a/Cargo.toml b/Cargo.toml index 7cd3521e6078..85fc54dfa756 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ members = [ "query-engine/black-box-tests", "query-engine/dmmf", "query-engine/metrics", - "query-engine/js-drivers", + "query-engine/js-connectors", "query-engine/prisma-models", "query-engine/query-engine", "query-engine/query-engine-node-api", diff --git a/psl/psl-core/src/common/preview_features.rs b/psl/psl-core/src/common/preview_features.rs index 321fbc6155c6..4e7137b4544f 100644 --- a/psl/psl-core/src/common/preview_features.rs +++ b/psl/psl-core/src/common/preview_features.rs @@ -64,7 +64,7 @@ features!( NamedConstraints, NApi, NativeTypes, - NodeDrivers, + JsConnectors, OrderByAggregateGroup, OrderByNulls, OrderByRelation, @@ -124,7 +124,7 @@ pub const ALL_PREVIEW_FEATURES: FeatureMap = FeatureMap { | UncheckedScalarInputs }), hidden: enumflags2::make_bitflags!(PreviewFeature::{ - NodeDrivers + JsConnectors }), }; diff --git a/query-engine/connectors/sql-query-connector/Cargo.toml b/query-engine/connectors/sql-query-connector/Cargo.toml index c90615e5e17a..6557cc7bb9b5 100644 --- a/query-engine/connectors/sql-query-connector/Cargo.toml +++ b/query-engine/connectors/sql-query-connector/Cargo.toml @@ -5,7 +5,7 @@ version = "0.1.0" [features] vendored-openssl = ["quaint/vendored-openssl"] -js-drivers = [] +js-connectors = [] [dependencies] psl.workspace = true diff --git a/query-engine/connectors/sql-query-connector/src/database/mod.rs b/query-engine/connectors/sql-query-connector/src/database/mod.rs index 93be46d80fe4..8bd1dad7d6f3 100644 --- a/query-engine/connectors/sql-query-connector/src/database/mod.rs +++ b/query-engine/connectors/sql-query-connector/src/database/mod.rs @@ -1,4 +1,4 @@ -#[cfg(feature = "js-drivers")] +#[cfg(feature = "js-connectors")] pub mod js; mod runtime; diff --git a/query-engine/connectors/sql-query-connector/src/database/mysql.rs b/query-engine/connectors/sql-query-connector/src/database/mysql.rs index 5708b88cdec5..a84de669ff0f 100644 --- a/query-engine/connectors/sql-query-connector/src/database/mysql.rs +++ b/query-engine/connectors/sql-query-connector/src/database/mysql.rs @@ -44,7 +44,7 @@ impl FromSource for Mysql { features: psl::PreviewFeatures, ) -> connector_interface::Result { if source.provider == "@prisma/mysql" { - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] { let driver = super::js::registered_driver(); let connection_info = get_connection_info(url)?; @@ -56,10 +56,10 @@ impl FromSource for Mysql { }); } - #[cfg(not(feature = "js-drivers"))] + #[cfg(not(feature = "js-connectors"))] { return Err(ConnectorError::from_kind(ErrorKind::UnsupportedConnector( - "The @prisma/mysql connector requires the `nodeDrivers` preview feature to be enabled.".into(), + "The @prisma/mysql connector requires the `jsConnectors` preview feature to be enabled.".into(), ))); } } diff --git a/query-engine/connectors/sql-query-connector/src/database/runtime.rs b/query-engine/connectors/sql-query-connector/src/database/runtime.rs index d88f7d1690eb..5aa8b14a666b 100644 --- a/query-engine/connectors/sql-query-connector/src/database/runtime.rs +++ b/query-engine/connectors/sql-query-connector/src/database/runtime.rs @@ -7,13 +7,13 @@ use quaint::{ Value, }; -#[cfg(feature = "js-drivers")] +#[cfg(feature = "js-connectors")] type QueryableRef = std::sync::Arc; pub enum RuntimePool { Rust(Quaint), - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Js(QueryableRef), } @@ -22,7 +22,7 @@ impl RuntimePool { match self { Self::Rust(_) => false, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(_) => true, } } @@ -34,7 +34,7 @@ impl RuntimePool { let conn: PooledConnection = pool.check_out().await.map_err(SqlError::from)?; Ok(RuntimeConnection::Rust(conn)) } - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(queryable) => Ok(RuntimeConnection::Js(queryable.clone())), } } @@ -43,7 +43,7 @@ impl RuntimePool { pub enum RuntimeConnection { Rust(PooledConnection), - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Js(QueryableRef), } @@ -53,7 +53,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.query(q).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.query(q).await, } } @@ -62,7 +62,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.query_raw(sql, params).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.query_raw(sql, params).await, } } @@ -71,7 +71,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.query_raw_typed(sql, params).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.query_raw_typed(sql, params).await, } } @@ -80,7 +80,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.execute(q).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.execute(q).await, } } @@ -89,7 +89,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.execute_raw(sql, params).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.execute_raw(sql, params).await, } } @@ -98,7 +98,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.execute_raw_typed(sql, params).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.execute_raw_typed(sql, params).await, } } @@ -109,7 +109,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.raw_cmd(cmd).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.raw_cmd(cmd).await, } } @@ -118,7 +118,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.version().await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.version().await, } } @@ -127,7 +127,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.is_healthy(), - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.is_healthy(), } } @@ -138,7 +138,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.set_tx_isolation_level(isolation_level).await, - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.set_tx_isolation_level(isolation_level).await, } } @@ -148,7 +148,7 @@ impl Queryable for RuntimeConnection { match self { Self::Rust(conn) => conn.requires_isolation_first(), - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] Self::Js(conn) => conn.requires_isolation_first(), } } diff --git a/query-engine/connectors/sql-query-connector/src/lib.rs b/query-engine/connectors/sql-query-connector/src/lib.rs index 6502a2bdd818..bb728c85c6c9 100644 --- a/query-engine/connectors/sql-query-connector/src/lib.rs +++ b/query-engine/connectors/sql-query-connector/src/lib.rs @@ -22,7 +22,7 @@ mod value_ext; use self::{column_metadata::*, context::Context, filter_conversion::*, query_ext::QueryExt, row::*}; use quaint::prelude::Queryable; -#[cfg(feature = "js-drivers")] +#[cfg(feature = "js-connectors")] pub use database::js::register_driver; pub use database::{FromSource, Mssql, Mysql, PostgreSql, Sqlite}; pub use error::SqlError; diff --git a/query-engine/js-drivers/Cargo.toml b/query-engine/js-connectors/Cargo.toml similarity index 93% rename from query-engine/js-drivers/Cargo.toml rename to query-engine/js-connectors/Cargo.toml index 9d6aa4e54523..0db8a6d6cccf 100644 --- a/query-engine/js-drivers/Cargo.toml +++ b/query-engine/js-connectors/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "js-drivers" +name = "js-connectors" version = "0.1.0" edition = "2021" diff --git a/query-engine/js-drivers/playground/.gitignore b/query-engine/js-connectors/playground/.gitignore similarity index 100% rename from query-engine/js-drivers/playground/.gitignore rename to query-engine/js-connectors/playground/.gitignore diff --git a/query-engine/js-drivers/playground/README.md b/query-engine/js-connectors/playground/README.md similarity index 98% rename from query-engine/js-drivers/playground/README.md rename to query-engine/js-connectors/playground/README.md index 69aed2b1307c..52155dc53861 100644 --- a/query-engine/js-drivers/playground/README.md +++ b/query-engine/js-connectors/playground/README.md @@ -36,7 +36,7 @@ Example test output: ``` ❯ pnpm dev -> @prisma/nodejs-playground@1.0.0 dev /Users/jkomyno/work/prisma/prisma-engines-2/query-engine/js-drivers/nodejs-examples +> @prisma/nodejs-playground@1.0.0 dev /Users/jkomyno/work/prisma/prisma-engines-2/query-engine/js-connectors/nodejs-examples > ts-node ./src/index.ts [nodejs] initializing mock connection pool: mysql://root:prisma@localhost:3307/test diff --git a/query-engine/js-drivers/playground/package.json b/query-engine/js-connectors/playground/package.json similarity index 100% rename from query-engine/js-drivers/playground/package.json rename to query-engine/js-connectors/playground/package.json diff --git a/query-engine/js-drivers/playground/pnpm-lock.yaml b/query-engine/js-connectors/playground/pnpm-lock.yaml similarity index 100% rename from query-engine/js-drivers/playground/pnpm-lock.yaml rename to query-engine/js-connectors/playground/pnpm-lock.yaml diff --git a/query-engine/js-drivers/playground/prisma/schema.prisma b/query-engine/js-connectors/playground/prisma/schema.prisma similarity index 87% rename from query-engine/js-drivers/playground/prisma/schema.prisma rename to query-engine/js-connectors/playground/prisma/schema.prisma index abc4fbc5a31f..6eec391c3d8e 100644 --- a/query-engine/js-drivers/playground/prisma/schema.prisma +++ b/query-engine/js-connectors/playground/prisma/schema.prisma @@ -1,6 +1,6 @@ generator client { - provider = "prisma-client-js" - previewFeatures = ["nodeDrivers"] + provider = "prisma-client-js" + previewFeatures = ["jsConnectors"] } datasource db { diff --git a/query-engine/js-drivers/playground/src/engines/types/JsonProtocol.ts b/query-engine/js-connectors/playground/src/engines/types/JsonProtocol.ts similarity index 100% rename from query-engine/js-drivers/playground/src/engines/types/JsonProtocol.ts rename to query-engine/js-connectors/playground/src/engines/types/JsonProtocol.ts diff --git a/query-engine/js-drivers/playground/src/engines/types/Library.ts b/query-engine/js-connectors/playground/src/engines/types/Library.ts similarity index 97% rename from query-engine/js-drivers/playground/src/engines/types/Library.ts rename to query-engine/js-connectors/playground/src/engines/types/Library.ts index 5d17ca2b93df..7f22eabf3cd3 100644 --- a/query-engine/js-drivers/playground/src/engines/types/Library.ts +++ b/query-engine/js-connectors/playground/src/engines/types/Library.ts @@ -27,7 +27,7 @@ export interface Query { args: Array } -// Same order as in rust js-drivers' `ColumnType` +// Same order as in rust js-connectors' `ColumnType` export const enum ColumnType { Int32 = 0, Int64 = 1, diff --git a/query-engine/js-drivers/playground/src/engines/types/QueryEngine.ts b/query-engine/js-connectors/playground/src/engines/types/QueryEngine.ts similarity index 100% rename from query-engine/js-drivers/playground/src/engines/types/QueryEngine.ts rename to query-engine/js-connectors/playground/src/engines/types/QueryEngine.ts diff --git a/query-engine/js-drivers/playground/src/engines/types/Transaction.ts b/query-engine/js-connectors/playground/src/engines/types/Transaction.ts similarity index 100% rename from query-engine/js-drivers/playground/src/engines/types/Transaction.ts rename to query-engine/js-connectors/playground/src/engines/types/Transaction.ts diff --git a/query-engine/js-drivers/playground/src/index.sql b/query-engine/js-connectors/playground/src/index.sql similarity index 100% rename from query-engine/js-drivers/playground/src/index.sql rename to query-engine/js-connectors/playground/src/index.sql diff --git a/query-engine/js-drivers/playground/src/index.ts b/query-engine/js-connectors/playground/src/index.ts similarity index 100% rename from query-engine/js-drivers/playground/src/index.ts rename to query-engine/js-connectors/playground/src/index.ts diff --git a/query-engine/js-drivers/playground/src/queryable/mock.ts b/query-engine/js-connectors/playground/src/queryable/mock.ts similarity index 100% rename from query-engine/js-drivers/playground/src/queryable/mock.ts rename to query-engine/js-connectors/playground/src/queryable/mock.ts diff --git a/query-engine/js-drivers/playground/src/queryable/mysql.ts b/query-engine/js-connectors/playground/src/queryable/mysql.ts similarity index 100% rename from query-engine/js-drivers/playground/src/queryable/mysql.ts rename to query-engine/js-connectors/playground/src/queryable/mysql.ts diff --git a/query-engine/js-drivers/playground/tsconfig.json b/query-engine/js-connectors/playground/tsconfig.json similarity index 100% rename from query-engine/js-drivers/playground/tsconfig.json rename to query-engine/js-connectors/playground/tsconfig.json diff --git a/query-engine/js-connectors/src/lib.rs b/query-engine/js-connectors/src/lib.rs new file mode 100644 index 000000000000..e91a33f6d770 --- /dev/null +++ b/query-engine/js-connectors/src/lib.rs @@ -0,0 +1,12 @@ +//! Query Engine JS Connectors +//! This crate is responsible for defining a quaint::Connector implementation that uses functions +//! exposed by client connectors via N-API. +//! +//! A JsConnector is an object defined in javascript that uses a driver +//! (ex. '@planetscale/database') to provide a similar implementation of that of a quaint Connector. i.e. the ability to query and execute SQL +//! plus some transformation of types to adhere to what a quaint::Value expresses. +//! + +mod proxy; +mod queryable; +pub use queryable::JsQueryable; diff --git a/query-engine/js-drivers/src/driver.rs b/query-engine/js-connectors/src/proxy.rs similarity index 80% rename from query-engine/js-drivers/src/driver.rs rename to query-engine/js-connectors/src/proxy.rs index 2ba3004afa69..6a838e122e71 100644 --- a/query-engine/js-drivers/src/driver.rs +++ b/query-engine/js-connectors/src/proxy.rs @@ -7,11 +7,11 @@ use napi_derive::napi; use quaint::connector::ResultSet as QuaintResultSet; use quaint::Value as QuaintValue; -// Note: Every ThreadsafeFunction should have an explicit `ErrorStrategy::Fatal` set, as to avoid -// "TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received null". -// See: https://github.com/napi-rs/napi-rs/issues/1521. +/// Proxy is a struct wrapping a javascript object that exhibits basic primitives for +/// querying and executing SQL (i.e. a client connector). The Proxy uses NAPI ThreadSafeFunction to +/// invoke the code within the node runtime that implements the client connector. #[derive(Clone)] -pub struct Driver { +pub struct Proxy { /// Execute a query given as SQL, interpolating the given parameters. query_raw: ThreadsafeFunction, @@ -32,15 +32,15 @@ pub struct Driver { is_healthy: ThreadsafeFunction<(), ErrorStrategy::Fatal>, } -// Reify creates a rust representation of the JS driver -pub fn reify(js_driver: JsObject) -> napi::Result { +/// Reify creates a Rust proxy to access the JS driver passed in as a parameter. +pub fn reify(js_driver: JsObject) -> napi::Result { let query_raw = js_driver.get_named_property("queryRaw")?; let execute_raw = js_driver.get_named_property("executeRaw")?; let version = js_driver.get_named_property("version")?; let close = js_driver.get_named_property("close")?; let is_healthy = js_driver.get_named_property("isHealthy")?; - let driver = Driver { + let driver = Proxy { query_raw, execute_raw, version, @@ -50,20 +50,20 @@ pub fn reify(js_driver: JsObject) -> napi::Result { Ok(driver) } -// This result set is more convenient to be manipulated from both Rust and NodeJS. -// Quaint's version of ResultSet is: -// -// pub struct ResultSet { -// pub(crate) columns: Arc>, -// pub(crate) rows: Vec>>, -// pub(crate) last_insert_id: Option, -// } -// -// If we used this ResultSet would we would have worse ergonomics as quaint::Value is a structured -// enum and cannot be used directly with the #[napi(Object)] macro. Thus requiring us to implement -// the FromNapiValue and ToNapiValue traits for quaint::Value, and use a different custom type -// representing the Value in javascript. -// +/// This result set is more convenient to be manipulated from both Rust and NodeJS. +/// Quaint's version of ResultSet is: +/// +/// pub struct ResultSet { +/// pub(crate) columns: Arc>, +/// pub(crate) rows: Vec>>, +/// pub(crate) last_insert_id: Option, +/// } +/// +/// If we used this ResultSet would we would have worse ergonomics as quaint::Value is a structured +/// enum and cannot be used directly with the #[napi(Object)] macro. Thus requiring us to implement +/// the FromNapiValue and ToNapiValue traits for quaint::Value, and use a different custom type +/// representing the Value in javascript. +/// #[napi(object)] #[derive(Debug)] pub struct JSResultSet { @@ -142,7 +142,7 @@ impl From for QuaintResultSet { } } -impl Driver { +impl Proxy { pub async fn query_raw(&self, params: Query) -> napi::Result { let promise = self.query_raw.call_async::>(params).await?; let value = promise.await?; diff --git a/query-engine/js-drivers/src/queryable.rs b/query-engine/js-connectors/src/queryable.rs similarity index 81% rename from query-engine/js-drivers/src/queryable.rs rename to query-engine/js-connectors/src/queryable.rs index 13143ab6e5e0..09b737fd3516 100644 --- a/query-engine/js-drivers/src/queryable.rs +++ b/query-engine/js-connectors/src/queryable.rs @@ -1,4 +1,4 @@ -use crate::driver::{self, Driver, JSResultSet, Query}; +use crate::proxy::{self, JSResultSet, Proxy, Query}; use async_trait::async_trait; use napi::JsObject; use quaint::{ @@ -9,14 +9,27 @@ use quaint::{ }; use tracing::{info_span, Instrument}; +/// A JsQueryable adapts a Proxy to implement quaint's Queryable interface. It has the +/// responsibility of transforming inputs and outputs of `query` and `execute` methods from quaint +/// types to types that can be translated into javascript and viceversa. This is to let the rest of +/// the query engine work as if it was using quaint itself. The aforementioned transformations are: +/// +/// Transforming a `quaint::ast::Query` into SQL by visiting it for the specific flavor of SQL +/// expected by the client connector. (eg. using the mysql visitor for the Planetscale client +/// connector) +/// +/// Transforming a `JSResultSet` (what client connectors implemented in javascript provide) +/// into a `quaint::connector::result_set::ResultSet`. A quaint `ResultSet` is basically a vector +/// of `quaint::Value` but said type is a tagged enum, with non-unit variants that cannot be converted to javascript as is. +/// #[derive(Clone)] pub struct JsQueryable { - pub(crate) driver: Driver, + pub(crate) proxy: Proxy, } impl JsQueryable { - pub fn new(driver: Driver) -> Self { - Self { driver } + pub fn new(proxy: Proxy) -> Self { + Self { proxy } } } @@ -94,7 +107,7 @@ impl QuaintQueryable for JsQueryable { /// parsing or normalization. async fn version(&self) -> quaint::Result> { // Todo: convert napi::Error to quaint::error::Error. - let version = self.driver.version().await.unwrap(); + let version = self.proxy.version().await.unwrap(); Ok(version) } @@ -134,7 +147,7 @@ impl JsQueryable { // Todo: convert napi::Error to quaint::error::Error. let sql_span = info_span!("js:query:sql", user_facing = true, "db.statement" = %sql); - let result_set = self.driver.query_raw(query).instrument(sql_span).await.unwrap(); + let result_set = self.proxy.query_raw(query).instrument(sql_span).await.unwrap(); let len = result_set.len(); let deserialization_span = info_span!("js:query:result", user_facing = true, "length" = %len); @@ -150,7 +163,7 @@ impl JsQueryable { // Todo: convert napi::Error to quaint::error::Error. let sql_span = info_span!("js:query:sql", user_facing = true, "db.statement" = %sql); - let affected_rows = self.driver.execute_raw(query).instrument(sql_span).await.unwrap(); + let affected_rows = self.proxy.execute_raw(query).instrument(sql_span).await.unwrap(); Ok(affected_rows as u64) } @@ -160,7 +173,7 @@ impl TransactionCapable for JsQueryable {} impl From for JsQueryable { fn from(driver: JsObject) -> Self { - let driver = driver::reify(driver).unwrap(); - Self { driver } + let driver = proxy::reify(driver).unwrap(); + Self { proxy: driver } } } diff --git a/query-engine/js-drivers/src/lib.rs b/query-engine/js-drivers/src/lib.rs deleted file mode 100644 index d2a9e19d21fa..000000000000 --- a/query-engine/js-drivers/src/lib.rs +++ /dev/null @@ -1,8 +0,0 @@ -//! Query Engine Node.js Driver -//! This crate is responsible for defining a `Queryable` + `TransactionCapable` + `Send` + `Sync` implementation that -//! uses functions exposed by Node.js drivers via N-API. -//! - -mod driver; -mod queryable; -pub use queryable::JsQueryable; diff --git a/query-engine/query-engine-node-api/Cargo.toml b/query-engine/query-engine-node-api/Cargo.toml index b677440a5a1f..5f19897c2660 100644 --- a/query-engine/query-engine-node-api/Cargo.toml +++ b/query-engine/query-engine-node-api/Cargo.toml @@ -9,9 +9,9 @@ crate-type = ["cdylib"] name = "query_engine" [features] -default = ["js-drivers"] +default = ["js-connectors"] vendored-openssl = ["sql-connector/vendored-openssl"] -js-drivers = ["sql-connector/js-drivers"] +js-connectors = ["sql-connector/js-connectors"] [dependencies] anyhow = "1" @@ -23,7 +23,7 @@ user-facing-errors = { path = "../../libs/user-facing-errors" } psl.workspace = true sql-connector = { path = "../connectors/sql-query-connector", package = "sql-query-connector" } prisma-models = { path = "../prisma-models" } -js-drivers = { path = "../js-drivers" } +js-connectors = { path = "../js-connectors" } napi.workspace = true napi-derive.workspace = true diff --git a/query-engine/query-engine-node-api/src/engine.rs b/query-engine/query-engine-node-api/src/engine.rs index e4af2e0a2fd4..7d9f06824977 100644 --- a/query-engine/query-engine-node-api/src/engine.rs +++ b/query-engine/query-engine-node-api/src/engine.rs @@ -151,9 +151,9 @@ impl QueryEngine { let log_callback = LogCallback::new(napi_env, callback)?; log_callback.unref(&napi_env)?; - #[cfg(feature = "js-drivers")] + #[cfg(feature = "js-connectors")] if let Some(driver) = maybe_driver { - let queryable = js_drivers::JsQueryable::from(driver); + let queryable = js_connectors::JsQueryable::from(driver); sql_connector::register_driver(Arc::new(queryable)); } From 6a4762d4d94d9c5aa24faf7803734c9af7fa014f Mon Sep 17 00:00:00 2001 From: Alexey Orlenko Date: Mon, 24 Jul 2023 10:12:23 +0200 Subject: [PATCH 4/4] Fix clippy errors in quaint (#4078) --- quaint/src/ast/function/uuid.rs | 2 +- quaint/src/connector/mysql.rs | 8 ++--- quaint/src/connector/postgres.rs | 53 +++++++++++++++----------------- quaint/src/tests/query/error.rs | 4 +-- quaint/src/tests/types/mssql.rs | 2 ++ quaint/src/tests/types/mysql.rs | 2 ++ quaint/src/tests/types/sqlite.rs | 2 ++ quaint/src/visitor/mssql.rs | 2 +- quaint/src/visitor/mysql.rs | 2 +- quaint/src/visitor/postgres.rs | 2 +- quaint/src/visitor/sqlite.rs | 2 +- 11 files changed, 42 insertions(+), 39 deletions(-) diff --git a/quaint/src/ast/function/uuid.rs b/quaint/src/ast/function/uuid.rs index e4c4109672c0..f1528eae7523 100644 --- a/quaint/src/ast/function/uuid.rs +++ b/quaint/src/ast/function/uuid.rs @@ -56,7 +56,7 @@ pub fn uuid_to_bin_swapped() -> Expression<'static> { /// # Ok(()) /// # } /// ``` -#[cfg(any(feature = "mysql"))] +#[cfg(feature = "mysql")] pub fn native_uuid() -> Expression<'static> { let func = Function { typ_: FunctionType::Uuid, diff --git a/quaint/src/connector/mysql.rs b/quaint/src/connector/mysql.rs index a94c4697a66d..68f4ac28fc35 100644 --- a/quaint/src/connector/mysql.rs +++ b/quaint/src/connector/mysql.rs @@ -592,16 +592,16 @@ mod tests { fn should_parse_prefer_socket() { let url = MysqlUrl::new(Url::parse("mysql://root:root@localhost:3307/testdb?prefer_socket=false").unwrap()).unwrap(); - assert_eq!(false, url.prefer_socket().unwrap()); + assert!(!url.prefer_socket().unwrap()); } #[test] fn should_parse_sslaccept() { let url = MysqlUrl::new(Url::parse("mysql://root:root@localhost:3307/testdb?sslaccept=strict").unwrap()).unwrap(); - assert_eq!(true, url.query_params.use_ssl); - assert_eq!(false, url.query_params.ssl_opts.skip_domain_validation()); - assert_eq!(false, url.query_params.ssl_opts.accept_invalid_certs()); + assert!(url.query_params.use_ssl); + assert!(!url.query_params.ssl_opts.skip_domain_validation()); + assert!(!url.query_params.ssl_opts.accept_invalid_certs()); } #[test] diff --git a/quaint/src/connector/postgres.rs b/quaint/src/connector/postgres.rs index c610e3ee0dd9..50ecdf09eb16 100644 --- a/quaint/src/connector/postgres.rs +++ b/quaint/src/connector/postgres.rs @@ -1487,41 +1487,38 @@ mod tests { #[test] fn test_safe_ident() { // Safe - assert_eq!(is_safe_identifier("hello"), true); - assert_eq!(is_safe_identifier("_hello"), true); - assert_eq!(is_safe_identifier("àbracadabra"), true); - assert_eq!(is_safe_identifier("h3ll0"), true); - assert_eq!(is_safe_identifier("héllo"), true); - assert_eq!(is_safe_identifier("héll0$"), true); - assert_eq!(is_safe_identifier("héll_0$"), true); - assert_eq!( - is_safe_identifier("disconnect_security_must_honor_connect_scope_one2m"), - true - ); + assert!(is_safe_identifier("hello")); + assert!(is_safe_identifier("_hello")); + assert!(is_safe_identifier("àbracadabra")); + assert!(is_safe_identifier("h3ll0")); + assert!(is_safe_identifier("héllo")); + assert!(is_safe_identifier("héll0$")); + assert!(is_safe_identifier("héll_0$")); + assert!(is_safe_identifier("disconnect_security_must_honor_connect_scope_one2m")); // Not safe - assert_eq!(is_safe_identifier(""), false); - assert_eq!(is_safe_identifier("Hello"), false); - assert_eq!(is_safe_identifier("hEllo"), false); - assert_eq!(is_safe_identifier("$hello"), false); - assert_eq!(is_safe_identifier("hello!"), false); - assert_eq!(is_safe_identifier("hello#"), false); - assert_eq!(is_safe_identifier("he llo"), false); - assert_eq!(is_safe_identifier(" hello"), false); - assert_eq!(is_safe_identifier("he-llo"), false); - assert_eq!(is_safe_identifier("hÉllo"), false); - assert_eq!(is_safe_identifier("1337"), false); - assert_eq!(is_safe_identifier("_HELLO"), false); - assert_eq!(is_safe_identifier("HELLO"), false); - assert_eq!(is_safe_identifier("HELLO$"), false); - assert_eq!(is_safe_identifier("ÀBRACADABRA"), false); + assert!(!is_safe_identifier("")); + assert!(!is_safe_identifier("Hello")); + assert!(!is_safe_identifier("hEllo")); + assert!(!is_safe_identifier("$hello")); + assert!(!is_safe_identifier("hello!")); + assert!(!is_safe_identifier("hello#")); + assert!(!is_safe_identifier("he llo")); + assert!(!is_safe_identifier(" hello")); + assert!(!is_safe_identifier("he-llo")); + assert!(!is_safe_identifier("hÉllo")); + assert!(!is_safe_identifier("1337")); + assert!(!is_safe_identifier("_HELLO")); + assert!(!is_safe_identifier("HELLO")); + assert!(!is_safe_identifier("HELLO$")); + assert!(!is_safe_identifier("ÀBRACADABRA")); for ident in RESERVED_KEYWORDS { - assert_eq!(is_safe_identifier(ident), false); + assert!(!is_safe_identifier(ident)); } for ident in RESERVED_TYPE_FUNCTION_NAMES { - assert_eq!(is_safe_identifier(ident), false); + assert!(!is_safe_identifier(ident)); } } diff --git a/quaint/src/tests/query/error.rs b/quaint/src/tests/query/error.rs index 99da58664c15..63bfd3ef0357 100644 --- a/quaint/src/tests/query/error.rs +++ b/quaint/src/tests/query/error.rs @@ -103,8 +103,8 @@ async fn unique_constraint_violation(api: &mut dyn TestApi) -> crate::Result<()> let fields = fields.iter().map(|s| s.as_str()).collect::>(); assert_eq!(vec!["id1", "id2"], fields) } - DatabaseConstraint::ForeignKey => assert!(false, "Expecting index or field constraints."), - DatabaseConstraint::CannotParse => assert!(false, "Couldn't parse the error message."), + DatabaseConstraint::ForeignKey => panic!("Expecting index or field constraints."), + DatabaseConstraint::CannotParse => panic!("Couldn't parse the error message."), }, _ => panic!("{}", err), } diff --git a/quaint/src/tests/types/mssql.rs b/quaint/src/tests/types/mssql.rs index e90cef01ebaf..2f9a125022cb 100644 --- a/quaint/src/tests/types/mssql.rs +++ b/quaint/src/tests/types/mssql.rs @@ -1,3 +1,5 @@ +#![allow(clippy::approx_constant)] + #[cfg(feature = "bigdecimal")] mod bigdecimal; diff --git a/quaint/src/tests/types/mysql.rs b/quaint/src/tests/types/mysql.rs index 97a920c96bf6..15e2c4f6478b 100644 --- a/quaint/src/tests/types/mysql.rs +++ b/quaint/src/tests/types/mysql.rs @@ -1,3 +1,5 @@ +#![allow(clippy::approx_constant)] + use crate::tests::test_api::*; #[cfg(feature = "bigdecimal")] diff --git a/quaint/src/tests/types/sqlite.rs b/quaint/src/tests/types/sqlite.rs index ead3bf1436a1..f1666b3881b0 100644 --- a/quaint/src/tests/types/sqlite.rs +++ b/quaint/src/tests/types/sqlite.rs @@ -1,3 +1,5 @@ +#![allow(clippy::approx_constant)] + use crate::tests::test_api::sqlite_test_api; #[cfg(feature = "chrono")] use crate::tests::test_api::TestApi; diff --git a/quaint/src/visitor/mssql.rs b/quaint/src/visitor/mssql.rs index fee1ca3afc8b..111b43ed8ebe 100644 --- a/quaint/src/visitor/mssql.rs +++ b/quaint/src/visitor/mssql.rs @@ -717,7 +717,7 @@ mod tests { (String::from(sql), params.into_iter().map(|p| p.into()).collect()) } - fn default_params<'a>(mut additional: Vec>) -> Vec> { + fn default_params(mut additional: Vec>) -> Vec> { let mut result = Vec::new(); for param in additional.drain(0..) { diff --git a/quaint/src/visitor/mysql.rs b/quaint/src/visitor/mysql.rs index 80b2193c7146..68bc62ec617f 100644 --- a/quaint/src/visitor/mysql.rs +++ b/quaint/src/visitor/mysql.rs @@ -635,7 +635,7 @@ mod tests { (String::from(sql), params.into_iter().map(|p| p.into()).collect()) } - fn default_params<'a>(mut additional: Vec>) -> Vec> { + fn default_params(mut additional: Vec>) -> Vec> { let mut result = Vec::new(); for param in additional.drain(0..) { diff --git a/quaint/src/visitor/postgres.rs b/quaint/src/visitor/postgres.rs index 86425833bf86..8dc02180881b 100644 --- a/quaint/src/visitor/postgres.rs +++ b/quaint/src/visitor/postgres.rs @@ -533,7 +533,7 @@ mod tests { (String::from(sql), params.into_iter().map(|p| p.into()).collect()) } - fn default_params<'a>(mut additional: Vec>) -> Vec> { + fn default_params(mut additional: Vec>) -> Vec> { let mut result = Vec::new(); for param in additional.drain(0..) { diff --git a/quaint/src/visitor/sqlite.rs b/quaint/src/visitor/sqlite.rs index d3ee84a186df..91d3240df67d 100644 --- a/quaint/src/visitor/sqlite.rs +++ b/quaint/src/visitor/sqlite.rs @@ -400,7 +400,7 @@ mod tests { (String::from(sql), params.into_iter().map(|p| p.into()).collect()) } - fn default_params<'a>(mut additional: Vec>) -> Vec> { + fn default_params(mut additional: Vec>) -> Vec> { let mut result = Vec::new(); for param in additional.drain(0..) {