-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Enable cross-trigger transactions via shared context #9794
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
base: alpha
Are you sure you want to change the base?
Conversation
I will reformat the title to use the proper commit message syntax. |
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughA new Gitpod configuration file is introduced to automate development environment setup. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RestWrite
participant DatabaseController
Client->>RestWrite: execute(context)
alt context has transaction
RestWrite->>DatabaseController: setTransactionalSession(context.transaction)
end
RestWrite->>DatabaseController: (proceed with write operation)
alt context has transaction
RestWrite->>DatabaseController: setTransactionalSession(null) (finally)
end
sequenceDiagram
participant Trigger
participant getRequestObject
participant DatabaseController
Trigger->>getRequestObject: (for certain trigger types)
getRequestObject->>DatabaseController: bind createTransactionalSession, commitTransactionalSession, abortTransactionalSession
getRequestObject->>Trigger: request.context includes bound methods + original context
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn EBADENGINE Unsupported engine { 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
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
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: 3
🧹 Nitpick comments (3)
.gitpod.yml (1)
7-10
: Gitpod task runs bothnpm run build
andnpm run start
in the same task – can cause blocking
npm run build
generally exits, but if the script starts a watcher or leaves a process running (common withtsc -w
, webpack-watch, etc.), the subsequentnpm run start
command 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
:createTransactionalSession
now 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 thedatabase
instance) 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/RestWrite.js (1)
98-100
: 🛠️ Refactor suggestionGuard session-clear with an explicit
sessionSet
flagThe assignment logic is correct, yet we still mutate the shared
DatabaseController
.
Introduce a localsessionSet
boolean 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
sessionSet
flag instead of repeating thethis.context.transaction
check—clearer intent and safer.- The
});;
leaves an extra semicolon that trips linters (noUnreachable
warning 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)
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.)commit
andabort
timingRestWrite
logic🧠 Why
Out-of-the-box, Parse Server creates a new
DatabaseController
per 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
getRequestObject
intriggers.js
Inject transactional helpers into the
request.context
:2. Extend
DatabaseController.js
Add support for:
3. Patch
RestWrite.execute()
inRestWrite.js
Apply the shared transaction session before executing write logic:
✅ Usage Example in Cloud Code
🧪 Behavior
context.transaction
is injected into every.save()
callRestWrite
ensures internal DB calls are linked to the correct transactionafterSave
)Summary by CodeRabbit
New Features
Bug Fixes