feat: add transactional support - #2864
Open
minottic wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
MongoTransactionModule/getMongoTransactionServiceglobal singleton pattern introduces hidden coupling and ordering constraints (e.g. in tests or multiple Nest apps); consider refactoring@Transactionalinto an interceptor/mixin that uses Nest DI directly instead of relying on mutable module-level state. - In
MongoTransactionService.run, whentransactionsSupported === falseyou bypass thetry/finallyand just callfn(undefined); iffnlater starts using sessions conditionally, you may want to centralize thefninvocation inside a singletryblock for both transactional and fallback paths to keep behavior (e.g. logging, instrumentation, error handling) consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `MongoTransactionModule`/`getMongoTransactionService` global singleton pattern introduces hidden coupling and ordering constraints (e.g. in tests or multiple Nest apps); consider refactoring `@Transactional` into an interceptor/mixin that uses Nest DI directly instead of relying on mutable module-level state.
- In `MongoTransactionService.run`, when `transactionsSupported === false` you bypass the `try/finally` and just call `fn(undefined)`; if `fn` later starts using sessions conditionally, you may want to centralize the `fn` invocation inside a single `try` block for both transactional and fallback paths to keep behavior (e.g. logging, instrumentation, error handling) consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Introduces transactions support in nestJs. Since transactions work in mongo only when replicaSets is enabled, the methods fallback to non-transactions when replicaset is not enabled.
It exposes three helpers:
@Transactionalto use as a decorator it attaches all downstream mongo calls to the same transaction. Useful for example for updates that need to fetch from DB the original value@Transactionalbut more explicitUsing 1 and 2 automatically attaches all downstream mongo operations to the same mongo session (transaction). This happens by adding the session to the asyncLocalStorage and loading it with pre hooks in mongo. Independent calls of 1 and 2 (non nested) will have independent sessions/transactions since they are scoped to the run() closure.
When nesting multiple 1s or 2s together the outermost session is used. This prevents confusion when decorating a function with
@Transactionalthat calls another decorated (with@Transactional) function. For example:Since the session of the transaction is closed when the inner function emits, non awaited functions can "emit" before finish and make the session close before the transaction is complete. To guard against this it's good to enforce awaited promises by linting them, covered by #2863 since it's good practice anyway.
Motivation
Transactions are fundamental when disjoint operations in the DB must have consistent data, for example a multi-stage find and update or a block revert on failure. This PR should make applying them easy
Tests included
Documentation
official documentation info
Summary by Sourcery
Introduce application-wide MongoDB transaction support integrated with NestJS and Mongoose, including a decorator-based API and ambient session handling.
New Features:
Enhancements:
Tests: