refactor: clean up transaction semantics for entity mutation hooks (#206)

* refactor: clean up transaction semantics for entity mutation hooks

* addressing review comments

* update

Author: ymc9

.coderabbit.yaml

@@ -4,6 +4,7 @@ early_access: false
 reviews:
     auto_review:
         enabled: true
+    base_branches: ['dev', 'main']
     sequence_diagrams: false
 chat:
     auto_reply: true

TODO.md

@@ -1,7 +1,5 @@
 ## V3 Alpha Todo
 
-- [ ] Infra
-    - [ ] Dependency injection
 - [ ] CLI
     - [x] generate
     - [x] migrate
@@ -11,7 +9,7 @@
     - [ ] format
     - [x] plugin mechanism
     - [x] built-in plugins
-        - [x] ts
+        - [x] typescript
         - [x] prisma
 - [ ] ZModel
     - [x] Import
@@ -34,10 +32,12 @@
         - [x] Pagination
             - [x] Skip and limit
             - [x] Cursor
-        - [x] Filtering
+        - [ ] Filtering
             - [x] Unique fields
             - [x] Scalar fields
             - [x] Relation fields
+            - [ ] JSON filtering
+            - [ ] Full-text search
         - [x] Sort
             - [x] Scalar fields
             - [x] Relation fields
@@ -46,7 +46,6 @@
             - [x] Sorting
             - [x] Pagination
         - [x] Distinct
-        - [ ] JSON filtering
     - [x] Update
         - [x] Input validation
         - [x] Top-level
@@ -66,10 +65,10 @@
     - [x] Transactions
         - [x] Interactive transaction
         - [x] Sequential transaction
-    - [ ] Extensions
+    - [ ] Extensibility
         - [x] Query builder API
         - [x] Computed fields
-        - [x] Prisma client extension
+        - [x] Plugin
         - [ ] Custom procedures
     - [ ] Misc
         - [x] JSDoc for CRUD methods

packages/runtime/src/client/client-impl.ts

@@ -7,6 +7,7 @@ import {
     Kysely,
     Log,
     sql,
+    Transaction,
     type KyselyProps,
 } from 'kysely';
 import type { GetModels, ProcedureDef, SchemaDef } from '../schema';
@@ -155,6 +156,12 @@ export class ClientImpl<Schema extends SchemaDef> {
         }
     }
 
+    forceTransaction() {
+        if (!this.kysely.isTransaction) {
+            this.kysely = new Transaction(this.kyselyProps);
+        }
+    }
+
     private async interactiveTransaction(
         callback: (tx: ClientContract<Schema>) => Promise<any>,
         options?: { isolationLevel?: TransactionIsolationLevel },

packages/runtime/src/client/crud/operations/base.ts

@@ -22,7 +22,7 @@ import { clone } from '../../../utils/clone';
 import { enumerate } from '../../../utils/enumerate';
 import { extractFields, fieldsToSelectObject } from '../../../utils/object-utils';
 import { NUMERIC_FIELD_TYPES } from '../../constants';
-import type { CRUD } from '../../contract';
+import { TransactionIsolationLevel, type CRUD } from '../../contract';
 import type { FindArgs, SelectIncludeOmit, WhereInput } from '../../crud-types';
 import { InternalError, NotFoundError, QueryError } from '../../errors';
 import type { ToKysely } from '../../query-builder';
@@ -2089,7 +2089,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
         } else {
             // otherwise, create a new transaction and execute the callback
             let txBuilder = this.kysely.transaction();
-            txBuilder = txBuilder.setIsolationLevel(isolationLevel ?? 'repeatable read');
+            txBuilder = txBuilder.setIsolationLevel(isolationLevel ?? TransactionIsolationLevel.RepeatableRead);
             return txBuilder.execute(callback);
         }
     }

packages/runtime/src/client/executor/kysely-utils.ts

@@ -0,0 +1,14 @@
+import { invariant } from '@zenstackhq/common-helpers';
+import { type OperationNode, AliasNode, IdentifierNode } from 'kysely';
+
+/**
+ * Strips alias from the node if it exists.
+ */
+export function stripAlias(node: OperationNode) {
+    if (AliasNode.is(node)) {
+        invariant(IdentifierNode.is(node.alias), 'Expected identifier as alias');
+        return { alias: node.alias.name, node: node.node };
+    } else {
+        return { alias: undefined, node };
+    }
+}

packages/runtime/src/client/executor/name-mapper.ts

@@ -19,6 +19,7 @@ import {
 } from 'kysely';
 import type { FieldDef, ModelDef, SchemaDef } from '../../schema';
 import { getModel, requireModel } from '../query-utils';
+import { stripAlias } from './kysely-utils';
 
 type Scope = {
     model: string;
@@ -94,7 +95,7 @@ export class QueryNameMapper extends OperationNodeTransformer {
     }
 
     protected override transformJoin(node: JoinNode) {
-        const { alias, node: innerNode } = this.stripAlias(node.table);
+        const { alias, node: innerNode } = stripAlias(node.table);
         if (TableNode.is(innerNode!)) {
             const modelName = innerNode.table.identifier.name;
             if (this.hasMappedColumns(modelName)) {
@@ -150,7 +151,11 @@ export class QueryNameMapper extends OperationNodeTransformer {
     }
 
     protected override transformUpdateQuery(node: UpdateQueryNode) {
-        const { alias, node: innerTable } = this.stripAlias(node.table);
+        if (!node.table) {
+            return super.transformUpdateQuery(node);
+        }
+
+        const { alias, node: innerTable } = stripAlias(node.table);
         if (!innerTable || !TableNode.is(innerTable)) {
             return super.transformUpdateQuery(node);
         }
@@ -170,7 +175,7 @@ export class QueryNameMapper extends OperationNodeTransformer {
 
         // process name mapping in each "from"
         const froms = node.from.froms.map((from) => {
-            const { alias, node: innerNode } = this.stripAlias(from);
+            const { alias, node: innerNode } = stripAlias(from);
             if (TableNode.is(innerNode!)) {
                 // map table name
                 return this.wrapAlias(this.processTableRef(innerNode), alias);
@@ -289,17 +294,6 @@ export class QueryNameMapper extends OperationNodeTransformer {
         }
     }
 
-    private stripAlias(node: OperationNode | undefined) {
-        if (!node) {
-            return { alias: undefined, node };
-        }
-        if (AliasNode.is(node)) {
-            invariant(IdentifierNode.is(node.alias), 'Expected identifier as alias');
-            return { alias: node.alias.name, node: node.node };
-        }
-        return { alias: undefined, node };
-    }
-
     private hasMappedColumns(modelName: string) {
         return [...this.fieldToColumnMap.keys()].some((key) => key.startsWith(modelName + '.'));
     }
@@ -310,7 +304,7 @@ export class QueryNameMapper extends OperationNodeTransformer {
         }
         return node.froms
             .map((from) => {
-                const { alias, node: innerNode } = this.stripAlias(from);
+                const { alias, node: innerNode } = stripAlias(from);
                 if (innerNode && TableNode.is(innerNode)) {
                     return { model: innerNode.table.identifier.name, alias, namesMapped };
                 } else {
@@ -325,7 +319,7 @@ export class QueryNameMapper extends OperationNodeTransformer {
         return {
             ...super.transformFrom(node),
             froms: node.froms.map((from) => {
-                const { alias, node: innerNode } = this.stripAlias(from);
+                const { alias, node: innerNode } = stripAlias(from);
                 if (!innerNode) {
                     return super.transformNode(from);
                 }

packages/runtime/src/client/executor/zenstack-driver.ts

@@ -79,7 +79,12 @@ export class ZenStackDriver implements Driver {
             this.#txConnections.delete(connection);
             if (callbacks) {
                 for (const callback of callbacks) {
-                    await callback();
+                    try {
+                        await callback();
+                    } catch (err) {
+                        // errors in commit callbacks are logged but do not fail the commit
+                        console.error(`Error executing transaction commit callback: ${err}`);
+                    }
                 }
             }
             return result;