Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds session management support to the mongoose-delete plugin so that delete and restore operations can participate in MongoDB transactions, and includes tests to verify transactional behavior.
- Extended core methods (
updateDocumentsByQuery,delete,deleteById,restore, and related statics) to accept an optionalsessionparameter. - Updated
savecalls to pass through session options and conditional validation. - Added a new test suite in
test/index.jscovering delete/restore within transactions, including commit and abort scenarios.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| index.js | Updated method signatures and internal calls to accept and propagate session options. |
| test/index.js | Introduced a new “session management” test suite to validate transactional delete/restore. |
Comments suppressed due to low confidence (2)
index.js:374
- Consider supporting the overload where
paramsis a function (callback) by checkingtypeof params === 'function'and shifting arguments, to match the pattern used indeleteById.
schema.statics.restore = function (conditions, params = {}, callback) {
test/index.js:872
- The static
restoretest covers commit scenarios but does not exercise rollback behavior; adding an abortTransaction test for multiple-document restore would ensure rollback works as expected.
it("restore() with multiple documents -> should support transactions with session parameter", async function () {
harveyalex
left a comment
harveyalex
left a comment
There was a problem hiding this comment.
Looks good! Just a few minor nit picks from me in the tests
index.js
Outdated
| var options = {}; | ||
| var lastArg = arguments[arguments.length - 1]; | ||
| if (lastArg && typeof lastArg === 'object' && lastArg.session) { | ||
| options.session = lastArg.session; | ||
| } | ||
| var match = { $match : { showAllDocuments : 'true' } }; | ||
| arguments.length ? args[0].unshift(match) : args.push([match]); | ||
| if (options.session) { | ||
| return Model[method].apply(this, args).session(options.session); | ||
| } |
Vesrion 1.0.0.
…eted-discriminators Handle aggregate for discriminators
| if (lastArg && typeof lastArg === 'object' && lastArg.session) { | ||
| session = lastArg.session; |
There was a problem hiding this comment.
❓ Does this provide the backward compatibility with the accidental fallback session support? If not then maybe we just bump the major version and flag this as a breaking change.
There was a problem hiding this comment.
Yes, this is a breaking change now. Maybe it is a good idea to bump it to a major version.
| // Delete multiple documents with session | ||
| const result = await TestModel.delete( | ||
| { name: { $in: ['Anakin Skywalker', 'Obi-Wan Kenobi'] } }, | ||
| { | ||
| deletedBy: userId, | ||
| session: session | ||
| } | ||
| ); |
There was a problem hiding this comment.
🤔 I'd do this as two separate calls, an $in call will be seen as just a single query and doesn't really test the use case for a transaction
| // Delete multiple documents with session | |
| const result = await TestModel.delete( | |
| { name: { $in: ['Anakin Skywalker', 'Obi-Wan Kenobi'] } }, | |
| { | |
| deletedBy: userId, | |
| session: session | |
| } | |
| ); | |
| // Delete multiple documents with session | |
| const result1 = await TestModel.delete( | |
| { name: 'Anakin Skywalker' }, | |
| { | |
| deletedBy: userId, | |
| session: session | |
| } | |
| ); | |
| const result2 = await TestModel.delete( | |
| { name: 'Obi-Wan Kenobi' }, | |
| { | |
| deletedBy: userId, | |
| session: session | |
| } | |
| ); |
| if (mongooseMajorVersion < 5) { | ||
| // Skip test for older mongoose versions without session support | ||
| return this.skip(); | ||
| } |
There was a problem hiding this comment.
👍 Nice, so this all works and avoids the tests if it's not the right Mongoose?
AB#254512
In this PR, we are adding session management support to mongoose-delete
This pull request introduces support for MongoDB transactions using Mongoose sessions in the
mongoose_deleteplugin. It modifies key methods to accept session parameters, ensuring compatibility with transactional workflows. Additionally, new tests are added to validate the behavior of these changes.Core Enhancements for Transaction Support:
updateDocumentsByQuery,delete,deleteById,restore, and related methods to accept and handle session parameters. This ensures that operations like delete and restore can participate in MongoDB transactions. [1] [2] [3] [4]saveoptions to include session handling and conditional validation fordeleteandrestoreoperations. [1] [2]Test Suite Additions:
delete,deleteById, andrestoremethods. These tests cover single-document operations, multi-document operations, and rollback scenarios.directConnection=true, ensuring compatibility with modern MongoDB configurations.