-
-
Notifications
You must be signed in to change notification settings - Fork 12
feat: implement sequential transaction #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update refactors the transaction handling in the client runtime by introducing overloads for the Changes
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Implements support for sequential transactions in the client API alongside existing interactive transactions.
- Adds end-to-end tests for sequential transactions in
transaction.test.ts - Introduces the
UnwrapTuplePromisestype andZenStackPromiseabstraction - Updates client contract and implementation to handle array-based (sequential) transactions, including overloads and runtime logic
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime/test/client-api/transaction.test.ts | Added tests covering empty, successful, ordered, and rollback scenarios for sequential transactions |
| packages/runtime/src/utils/type-utils.ts | Added UnwrapTuplePromises to map a tuple of promises to their unwrapped results |
| packages/runtime/src/client/promise.ts | Renamed and extended the deferred promise utility to createZenStackPromise with a cb callback |
| packages/runtime/src/client/contract.ts | Defined overloads for interactive vs. sequential $transaction and added TransactionIsolationLevel |
| packages/runtime/src/client/constants.ts | Introduced TRANSACTION_UNSUPPORTED_METHODS constant |
| packages/runtime/src/client/client-impl.ts | Implemented $transaction overload dispatch, interactiveTransaction, and sequentialTransaction |
| packages/cli/package.json | Added "zen" alias for the CLI binary |
| TODO.md | Marked sequential transaction feature as completed |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
TODO.md (1)
57-57: Fix indentation to maintain consistent markdown formatting.The list item indentation should be 4 spaces instead of 8 to match the markdown structure.
- - [x] Sequential transaction + - [x] Sequential transaction
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
TODO.md(1 hunks)packages/cli/package.json(1 hunks)packages/runtime/src/client/client-impl.ts(7 hunks)packages/runtime/src/client/constants.ts(1 hunks)packages/runtime/src/client/contract.ts(3 hunks)packages/runtime/src/client/promise.ts(2 hunks)packages/runtime/src/utils/type-utils.ts(1 hunks)packages/runtime/test/client-api/transaction.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/runtime/src/client/promise.ts (2)
packages/sdk/src/schema/schema.ts (1)
SchemaDef(10-17)packages/runtime/src/client/contract.ts (1)
ClientContract(49-158)
🪛 markdownlint-cli2 (0.17.2)
TODO.md
57-57: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (26)
packages/runtime/src/client/constants.ts (1)
11-14: Well-designed constant for transaction method restrictions.The constant properly defines methods that should be unavailable in transaction contexts. The choices are logical - nested transactions, disconnection during transaction, and plugin modifications could all cause issues.
packages/runtime/src/utils/type-utils.ts (1)
72-74: Correct implementation of tuple promise unwrapping utility.The type utility properly maps over a readonly tuple and unwraps each promise element using
Awaited<T[K]>. This will be essential for typing the results of sequential transactions.packages/cli/package.json (1)
21-21: Good UX improvement with shorter CLI alias.Adding the "zen" alias provides a convenient shorter command while maintaining the existing "zenstack" command for backward compatibility.
packages/runtime/src/client/promise.ts (5)
1-3: Appropriate imports for enhanced promise typing.The new imports support the schema-aware promise typing and transaction client integration.
4-13: Well-designed promise type with transaction support.The
ZenStackPromisetype properly extendsPromise<T>while adding:
- Generic schema parameter for type safety
- Private
cbproperty for internal transaction client handling- Optional transaction client parameter in the callback
19-21: Correct function signature for schema-aware promise creation.The function signature properly:
- Adds generic Schema parameter
- Updates callback to accept optional transaction client
- Maintains backward compatibility through optional parameter
23-25: Proper transaction client parameter handling.The callback correctly:
- Accepts optional transaction client parameter
- Passes it to the user-provided callback
- Maintains promise memoization behavior
42-42: Correct exposure of callback function.Exposing the
cbproperty allows the transaction implementation to invoke the callback with the appropriate transaction client.packages/runtime/test/client-api/transaction.test.ts (6)
19-20: Well-structured test organization.The restructuring into separate describe blocks for interactive and sequential transactions improves test organization and readability.
29-29: Good correction to email value.The email value has been corrected from '[email protected]' to '[email protected]' to properly represent the second user in the test.
72-72: Type casting approach for nested transactions.The explicit type casting
(tx as any)is used to access the$transactionmethod on the transaction client. This suggests the transaction client type doesn't include the$transactionmethod, which aligns with the contract changes.Also applies to: 93-93
85-105: Comprehensive nested transaction failure test.The addition of a nested failed transaction test provides good coverage for error handling in nested scenarios, ensuring proper rollback behavior.
108-157: Excellent coverage for sequential transactions.The sequential transaction test suite provides comprehensive coverage including:
- Empty array handling
- Deferred execution verification
- Successful transaction execution with mixed operations
- Execution order preservation
- Rollback behavior on errors
The tests effectively validate the new sequential transaction API behavior.
114-122: Excellent test for deferred execution.This test cleverly verifies that promises are not executed immediately when passed to
$transactionbut only when the transaction is actually executed. This is crucial for the sequential transaction implementation.packages/runtime/src/client/contract.ts (4)
4-4: Good addition of utility types.The import of
UnwrapTuplePromisesutility type is necessary for the sequential transaction return type handling.
31-31: Well-designed type safety for transaction clients.The import of
TRANSACTION_UNSUPPORTED_METHODSconstant and the derivedTransactionUnsupportedMethodstype provide proper type safety by excluding unsupported methods from transaction clients.Also applies to: 33-33
35-44: Standard SQL transaction isolation levels.The
TransactionIsolationLevelenum includes all standard SQL isolation levels with proper string values. This provides good compatibility with database systems.
116-129: Well-designed transaction method overloads.The two overloads provide clear separation between interactive (callback-based) and sequential (array-based) transaction modes:
- Interactive transaction properly excludes unsupported methods from the transaction client
- Sequential transaction uses tuple type to maintain result type correspondence
- Both overloads support optional isolation level configuration
The type system ensures proper usage and return types for each mode.
packages/runtime/src/client/client-impl.ts (8)
1-1: Good addition of invariant for input validation.The import of
invariantfrom@zenstackhq/common-helpersis appropriate for input validation in the transaction dispatch logic.
18-18: Proper import of transaction isolation level.The import of
TransactionIsolationLeveltype from the contract module maintains proper type consistency.
36-36: Enhanced promise type for transaction support.The import of
createZenStackPromiseandZenStackPromisetype supports the new transaction-aware promise handling needed for sequential transactions.
148-168: Well-implemented transaction dispatch logic.The implementation provides:
- Clear overloads for interactive and sequential transaction modes
- Proper input validation using invariant
- Clean dispatch logic based on input type
- Good error messaging for invalid inputs
The logic correctly identifies function vs array inputs and dispatches to appropriate handlers.
170-189: Robust interactive transaction implementation.The interactive transaction implementation properly:
- Checks for existing transaction context to avoid nested transaction issues
- Supports optional isolation level configuration
- Creates proper transaction client with shared state
- Maintains backward compatibility with existing transaction usage
The nested transaction handling is appropriate for the interactive mode.
191-215: Excellent sequential transaction implementation.The sequential transaction implementation correctly:
- Processes promises sequentially within the transaction context
- Maintains execution order through the for loop
- Passes the transaction client to each promise callback
- Handles both nested and new transaction scenarios
- Supports isolation level configuration
The sequential execution ensures proper transaction semantics.
287-290: Updated promise creation for transaction support.The replacement of
createDeferredPromisewithcreateZenStackPromisein raw query methods enables proper integration with the new transaction system while maintaining existing functionality.Also applies to: 294-298, 302-305, 309-313
362-362: Enhanced promise creation with transaction client support.The updated
createZenStackPromisecall now accepts an optional transaction client parameter, enabling sequential transaction execution while maintaining backward compatibility.
Summary by CodeRabbit
New Features
Bug Fixes
Tests