feat: Enable cross-trigger transactions via shared context #9794
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughIntroduces a Gitpod config and transaction-session plumbing: DatabaseController gains Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RestWrite
participant DatabaseController
Client->>RestWrite: execute(request, context)
alt context.transaction exists
RestWrite->>DatabaseController: setTransactionalSession(context.transaction)
end
RestWrite->>DatabaseController: perform write operations
RestWrite-->>Client: respond
alt context.transaction exists
RestWrite->>DatabaseController: setTransactionalSession(null) (finally)
end
sequenceDiagram
participant Trigger
participant RequestBuilder
participant DatabaseController
Trigger->>RequestBuilder: build request for trigger
RequestBuilder->>DatabaseController: bind createTransactionalSession()
RequestBuilder->>DatabaseController: bind commitTransactionalSession(), abortTransactionalSession()
RequestBuilder->>Trigger: return request with extended request.context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.gitpod.yml (1)
7-10: Gitpod task runs bothnpm run buildandnpm run startin the same task – can cause blocking
npm run buildgenerally exits, but if the script starts a watcher or leaves a process running (common withtsc -w, webpack-watch, etc.), the subsequentnpm run startcommand will never be executed.
Consider splitting the init and workspace tasks or adding a second task entry:tasks: - init: npm install && npm run build - command: npm run startThis keeps the build phase isolated and avoids accidental hangs.
src/Controllers/DatabaseController.js (1)
1694-1697:createTransactionalSessionnow returns the session – good, but missing defensive checkReturning the session is useful. Add a guard to avoid silently re-creating when one is already active:
+ if (this._transactionalSession) { + return Promise.resolve(this._transactionalSession); + } return this.adapter.createTransactionalSession().then(transactionalSession => { this._transactionalSession = transactionalSession; return this._transactionalSession; });Prevents nested calls from starting multiple DB transactions.
src/triggers.js (1)
284-288: Minor: avoid per-call re-binding to cut allocation noiseThe three
bind()calls run on every trigger invocation. Consider hoisting the bound helpers once (e.g. in module scope or caching them on thedatabaseinstance) and reusing the same function references to reduce needless allocations in hot paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitpod.yml(1 hunks)src/Controllers/DatabaseController.js(1 hunks)src/RestWrite.js(1 hunks)src/triggers.js(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/RestWrite.js (1)
98-100: 🛠️ Refactor suggestionGuard session-clear with an explicit
sessionSetflagThe assignment logic is correct, yet we still mutate the shared
DatabaseController.
Introduce a localsessionSetboolean so the subsequent cleanup only runs when this call actually set the session, avoiding accidental removal of a session established by an outer/sibling write.-if (this.context.transaction) { - this.config.database.setTransactionalSession(this.context.transaction) -} +let sessionSet = false; +if (this.context.transaction) { + this.config.database.setTransactionalSession(this.context.transaction); + sessionSet = true; +}
🧹 Nitpick comments (1)
src/RestWrite.js (1)
172-177: Nit: usesessionSet& drop stray semicolon
- Re-use the
sessionSetflag instead of repeating thethis.context.transactioncheck—clearer intent and safer.- The
});;leaves an extra semicolon that trips linters (noUnreachablewarning in Biome).- }).finally(() => { - if (this.context.transaction) { + }) + .finally(() => { + if (sessionSet) { this.config.database.setTransactionalSession(null); } - });; + });🧰 Tools
🪛 Biome (1.9.4)
[error] 177-177: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/RestWrite.js(2 hunks)src/triggers.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/triggers.js
🧰 Additional context used
🪛 Biome (1.9.4)
src/RestWrite.js
[error] 177-177: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
|
#up |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/RestWrite.js`:
- Around line 174-179: Remove the stray empty statement by deleting the extra
semicolon after the finally block; locate the finally() callback that checks
this.context.transaction and calls
this.config.database.setTransactionalSession(null) (appears as "}).;;" in
RestWrite.js) and change it to end with a single semicolon/closing brace only so
there is no double semicolon/empty statement.
🧹 Nitpick comments (1)
src/RestWrite.js (1)
100-101: Missing semicolon on Line 101.Minor style nit: the statement on Line 101 is missing a trailing semicolon, which is inconsistent with the rest of the codebase.
- this.config.database.setTransactionalSession(this.context.transaction) + this.config.database.setTransactionalSession(this.context.transaction);
| }).finally(() => { | ||
| if (this.context.transaction) { | ||
| // Ensure isolation even on uncaught errors | ||
| this.config.database.setTransactionalSession(null); | ||
| } | ||
| });; |
There was a problem hiding this comment.
Double semicolon produces an empty statement; remove the extra ;.
Line 179 has });; — the trailing extra semicolon is flagged by static analysis (Biome: noUnreachable). It's a no-op empty statement that should be removed.
Proposed fix
- }).finally(() => {
- if (this.context.transaction) {
- // Ensure isolation even on uncaught errors
- this.config.database.setTransactionalSession(null);
- }
- });;
+ }).finally(() => {
+ if (this.context.transaction) {
+ // Ensure isolation even on uncaught errors
+ this.config.database.setTransactionalSession(null);
+ }
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }).finally(() => { | |
| if (this.context.transaction) { | |
| // Ensure isolation even on uncaught errors | |
| this.config.database.setTransactionalSession(null); | |
| } | |
| });; | |
| }).finally(() => { | |
| if (this.context.transaction) { | |
| // Ensure isolation even on uncaught errors | |
| this.config.database.setTransactionalSession(null); | |
| } | |
| }); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 179-179: This code will never be reached ...
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In `@src/RestWrite.js` around lines 174 - 179, Remove the stray empty statement by
deleting the extra semicolon after the finally block; locate the finally()
callback that checks this.context.transaction and calls
this.config.database.setTransactionalSession(null) (appears as "}).;;" in
RestWrite.js) and change it to end with a single semicolon/closing brace only so
there is no double semicolon/empty statement.
3 participants
Transaction Support Across Parse Server Triggers
This modification extends Parse Server to support multi-operation transactions across
beforeSave,afterSave, and other Cloud Code triggers using a shared transactional context.✨ Features
beforeSave.save()callsbeforeSave,afterSave, etc.)commitandaborttimingRestWritelogic🧠 Why
Out-of-the-box, Parse Server creates a new
DatabaseControllerper internal operation, which leads to:.save()sThis patch ensures the transaction session is persisted across triggers and reused consistently, enabling ACID-safe operations.
🛠 How It Works
1. Modify
getRequestObjectintriggers.jsInject transactional helpers into the
request.context:2. Extend
DatabaseController.jsAdd support for:
3. Patch
RestWrite.execute()inRestWrite.jsApply the shared transaction session before executing write logic:
✅ Usage Example in Cloud Code
🧪 Behavior
context.transactionis injected into every.save()callRestWriteensures internal DB calls are linked to the correct transactionafterSave)Summary by CodeRabbit
New Features
Bug Fixes