From 7a77d975f89a1ca375e8ac3e8dd1279138535708 Mon Sep 17 00:00:00 2001 From: jkomyno Date: Tue, 28 Nov 2023 12:50:22 +0100 Subject: [PATCH] chore(driver-adapters): unify napi/wasm logic for transaction.rs --- query-engine/driver-adapters/src/lib.rs | 2 + query-engine/driver-adapters/src/napi/mod.rs | 2 - .../driver-adapters/src/napi/transaction.rs | 134 ------------------ query-engine/driver-adapters/src/proxy.rs | 1 + .../driver-adapters/src/send_future.rs | 7 +- .../src/{wasm => }/transaction.rs | 46 +++--- .../driver-adapters/src/wasm/from_js.rs | 2 +- query-engine/driver-adapters/src/wasm/mod.rs | 3 +- 8 files changed, 38 insertions(+), 159 deletions(-) delete mode 100644 query-engine/driver-adapters/src/napi/transaction.rs rename query-engine/driver-adapters/src/{wasm => }/transaction.rs (81%) diff --git a/query-engine/driver-adapters/src/lib.rs b/query-engine/driver-adapters/src/lib.rs index 230dc1af83d..9873ac0d994 100644 --- a/query-engine/driver-adapters/src/lib.rs +++ b/query-engine/driver-adapters/src/lib.rs @@ -12,9 +12,11 @@ pub(crate) mod error; pub(crate) mod proxy; pub(crate) mod queryable; pub(crate) mod send_future; +pub(crate) mod transaction; pub(crate) mod types; pub use queryable::from_js; +pub(crate) use transaction::JsTransaction; #[cfg(target_arch = "wasm32")] pub use wasm::JsObjectExtern as JsObject; diff --git a/query-engine/driver-adapters/src/napi/mod.rs b/query-engine/driver-adapters/src/napi/mod.rs index 69dd2caa658..c9bb8d24ac3 100644 --- a/query-engine/driver-adapters/src/napi/mod.rs +++ b/query-engine/driver-adapters/src/napi/mod.rs @@ -4,7 +4,5 @@ mod async_js_function; mod conversion; mod error; mod result; -mod transaction; pub(crate) use async_js_function::AsyncJsFunction; -pub(crate) use transaction::JsTransaction; diff --git a/query-engine/driver-adapters/src/napi/transaction.rs b/query-engine/driver-adapters/src/napi/transaction.rs deleted file mode 100644 index b32c408641b..00000000000 --- a/query-engine/driver-adapters/src/napi/transaction.rs +++ /dev/null @@ -1,134 +0,0 @@ -use async_trait::async_trait; -use metrics::decrement_gauge; -use napi::{bindgen_prelude::FromNapiValue, JsObject}; -use quaint::{ - connector::{IsolationLevel, Transaction as QuaintTransaction}, - prelude::{Query as QuaintQuery, Queryable, ResultSet}, - Value, -}; - -use crate::proxy::{CommonProxy, TransactionOptions, TransactionProxy}; -use crate::queryable::JsBaseQueryable; - -// Wrapper around JS transaction objects that implements Queryable -// and quaint::Transaction. Can be used in place of quaint transaction, -// but delegates most operations to JS -pub(crate) struct JsTransaction { - tx_proxy: TransactionProxy, - inner: JsBaseQueryable, -} - -impl JsTransaction { - pub(crate) fn new(inner: JsBaseQueryable, tx_proxy: TransactionProxy) -> Self { - Self { inner, tx_proxy } - } - - pub fn options(&self) -> &TransactionOptions { - self.tx_proxy.options() - } - - pub async fn raw_phantom_cmd(&self, cmd: &str) -> quaint::Result<()> { - let params = &[]; - quaint::connector::metrics::query("js.raw_phantom_cmd", cmd, params, move || async move { Ok(()) }).await - } -} - -#[async_trait] -impl QuaintTransaction for JsTransaction { - async fn commit(&self) -> quaint::Result<()> { - // increment of this gauge is done in DriverProxy::startTransaction - decrement_gauge!("prisma_client_queries_active", 1.0); - - let commit_stmt = "COMMIT"; - - if self.options().use_phantom_query { - let commit_stmt = JsBaseQueryable::phantom_query_message(commit_stmt); - self.raw_phantom_cmd(commit_stmt.as_str()).await?; - } else { - self.inner.raw_cmd(commit_stmt).await?; - } - - self.tx_proxy.commit().await - } - - async fn rollback(&self) -> quaint::Result<()> { - // increment of this gauge is done in DriverProxy::startTransaction - decrement_gauge!("prisma_client_queries_active", 1.0); - - let rollback_stmt = "ROLLBACK"; - - if self.options().use_phantom_query { - let rollback_stmt = JsBaseQueryable::phantom_query_message(rollback_stmt); - self.raw_phantom_cmd(rollback_stmt.as_str()).await?; - } else { - self.inner.raw_cmd(rollback_stmt).await?; - } - - self.tx_proxy.rollback().await - } - - fn as_queryable(&self) -> &dyn Queryable { - self - } -} - -#[async_trait] -impl Queryable for JsTransaction { - async fn query(&self, q: QuaintQuery<'_>) -> quaint::Result { - self.inner.query(q).await - } - - async fn query_raw(&self, sql: &str, params: &[Value<'_>]) -> quaint::Result { - self.inner.query_raw(sql, params).await - } - - async fn query_raw_typed(&self, sql: &str, params: &[Value<'_>]) -> quaint::Result { - self.inner.query_raw_typed(sql, params).await - } - - async fn execute(&self, q: QuaintQuery<'_>) -> quaint::Result { - self.inner.execute(q).await - } - - async fn execute_raw(&self, sql: &str, params: &[Value<'_>]) -> quaint::Result { - self.inner.execute_raw(sql, params).await - } - - async fn execute_raw_typed(&self, sql: &str, params: &[Value<'_>]) -> quaint::Result { - self.inner.execute_raw_typed(sql, params).await - } - - async fn raw_cmd(&self, cmd: &str) -> quaint::Result<()> { - self.inner.raw_cmd(cmd).await - } - - async fn version(&self) -> quaint::Result> { - self.inner.version().await - } - - fn is_healthy(&self) -> bool { - self.inner.is_healthy() - } - - async fn set_tx_isolation_level(&self, isolation_level: IsolationLevel) -> quaint::Result<()> { - self.inner.set_tx_isolation_level(isolation_level).await - } - - fn requires_isolation_first(&self) -> bool { - self.inner.requires_isolation_first() - } -} - -/// Implementing unsafe `from_napi_value` is only way I managed to get threadsafe -/// JsTransaction value in `DriverProxy`. Going through any intermediate safe napi.rs value, -/// like `JsObject` or `JsUnknown` wrapped inside `JsPromise` makes it impossible to extract the value -/// out of promise while keeping the future `Send`. -impl FromNapiValue for JsTransaction { - unsafe fn from_napi_value(env: napi::sys::napi_env, napi_val: napi::sys::napi_value) -> napi::Result { - let object = JsObject::from_napi_value(env, napi_val)?; - let common_proxy = CommonProxy::new(&object)?; - let tx_proxy = TransactionProxy::new(&object)?; - - Ok(Self::new(JsBaseQueryable::new(common_proxy), tx_proxy)) - } -} diff --git a/query-engine/driver-adapters/src/proxy.rs b/query-engine/driver-adapters/src/proxy.rs index 7d66b798e20..7e01176be12 100644 --- a/query-engine/driver-adapters/src/proxy.rs +++ b/query-engine/driver-adapters/src/proxy.rs @@ -178,3 +178,4 @@ macro_rules! impl_send_sync_on_wasm { impl_send_sync_on_wasm!(TransactionProxy); impl_send_sync_on_wasm!(DriverProxy); impl_send_sync_on_wasm!(CommonProxy); +impl_send_sync_on_wasm!(JsTransaction); diff --git a/query-engine/driver-adapters/src/send_future.rs b/query-engine/driver-adapters/src/send_future.rs index 61c64a96045..ed5e78345af 100644 --- a/query-engine/driver-adapters/src/send_future.rs +++ b/query-engine/driver-adapters/src/send_future.rs @@ -11,8 +11,6 @@ use futures::Future; #[pin_project::pin_project] pub struct SendFuture(#[pin] pub F); -unsafe impl Send for SendFuture {} - impl Future for SendFuture { type Output = F::Output; @@ -22,3 +20,8 @@ impl Future for SendFuture { future.poll(cx) } } + +// Note: on Napi.rs, we require the underlying future to be `Send`. +// On Wasm, that's currently not possible. +#[cfg(target_arch = "wasm32")] +unsafe impl Send for SendFuture {} diff --git a/query-engine/driver-adapters/src/wasm/transaction.rs b/query-engine/driver-adapters/src/transaction.rs similarity index 81% rename from query-engine/driver-adapters/src/wasm/transaction.rs rename to query-engine/driver-adapters/src/transaction.rs index 1d543ce6def..7f7180ae6d3 100644 --- a/query-engine/driver-adapters/src/wasm/transaction.rs +++ b/query-engine/driver-adapters/src/transaction.rs @@ -1,16 +1,14 @@ use async_trait::async_trait; -use js_sys::Object as JsObject; use metrics::decrement_gauge; use quaint::{ connector::{IsolationLevel, Transaction as QuaintTransaction}, prelude::{Query as QuaintQuery, Queryable, ResultSet}, Value, }; -use wasm_bindgen::JsCast; -use super::from_js::FromJsValue; use crate::proxy::{TransactionOptions, TransactionProxy}; -use crate::{proxy::CommonProxy, queryable::JsBaseQueryable, send_future::SendFuture, JsObjectExtern}; +use crate::{proxy::CommonProxy, queryable::JsBaseQueryable, send_future::SendFuture}; +use crate::{JsObject, JsResult}; // Wrapper around JS transaction objects that implements Queryable // and quaint::Transaction. Can be used in place of quaint transaction, @@ -35,17 +33,6 @@ impl JsTransaction { } } -impl FromJsValue for JsTransaction { - fn from_js_value(value: wasm_bindgen::prelude::JsValue) -> Result { - let object: JsObjectExtern = value.dyn_into::()?.unchecked_into(); - let common_proxy = CommonProxy::new(&object)?; - let base = JsBaseQueryable::new(common_proxy); - let tx_proxy = TransactionProxy::new(&object)?; - - Ok(Self::new(base, tx_proxy)) - } -} - #[async_trait] impl QuaintTransaction for JsTransaction { async fn commit(&self) -> quaint::Result<()> { @@ -132,6 +119,29 @@ impl Queryable for JsTransaction { } } -// Assume the proxy object will not be sent to service workers, we can unsafe impl Send + Sync. -unsafe impl Send for JsTransaction {} -unsafe impl Sync for JsTransaction {} +#[cfg(target_arch = "wasm32")] +impl super::wasm::FromJsValue for JsTransaction { + fn from_js_value(value: wasm_bindgen::prelude::JsValue) -> JsResult { + use wasm_bindgen::JsCast; + + let object = value.dyn_into::()?; + let common_proxy = CommonProxy::new(&object)?; + let base = JsBaseQueryable::new(common_proxy); + let tx_proxy = TransactionProxy::new(&object)?; + + Ok(Self::new(base, tx_proxy)) + } +} + +/// Implementing unsafe `from_napi_value` allows retrieving a threadsafe `JsTransaction` in `DriverProxy` +/// while keeping derived futures `Send`. +#[cfg(not(target_arch = "wasm32"))] +impl ::napi::bindgen_prelude::FromNapiValue for JsTransaction { + unsafe fn from_napi_value(env: napi::sys::napi_env, napi_val: napi::sys::napi_value) -> JsResult { + let object = JsObject::from_napi_value(env, napi_val)?; + let common_proxy = CommonProxy::new(&object)?; + let tx_proxy = TransactionProxy::new(&object)?; + + Ok(Self::new(JsBaseQueryable::new(common_proxy), tx_proxy)) + } +} diff --git a/query-engine/driver-adapters/src/wasm/from_js.rs b/query-engine/driver-adapters/src/wasm/from_js.rs index aaa0d91223f..9195ea4dabe 100644 --- a/query-engine/driver-adapters/src/wasm/from_js.rs +++ b/query-engine/driver-adapters/src/wasm/from_js.rs @@ -1,7 +1,7 @@ use serde::de::DeserializeOwned; use wasm_bindgen::JsValue; -pub trait FromJsValue: Sized { +pub(crate) trait FromJsValue: Sized { fn from_js_value(value: JsValue) -> Result; } diff --git a/query-engine/driver-adapters/src/wasm/mod.rs b/query-engine/driver-adapters/src/wasm/mod.rs index e60d23ff244..655ea1a6080 100644 --- a/query-engine/driver-adapters/src/wasm/mod.rs +++ b/query-engine/driver-adapters/src/wasm/mod.rs @@ -5,8 +5,7 @@ mod error; mod from_js; mod js_object_extern; mod result; -mod transaction; pub(crate) use async_js_function::AsyncJsFunction; +pub(crate) use from_js::FromJsValue; pub use js_object_extern::JsObjectExtern; -pub(crate) use transaction::JsTransaction;