Skip to content

Commit 50da8e4

Browse files
authored
Merge pull request #14346 from Automattic/vkarpov15/gh-14340
Avoid corrupting $set-ed arrays when transaction error occurs
2 parents b9e1f75 + f27e13f commit 50da8e4

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

lib/plugins/trackTransaction.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ function mergeAtomics(destination, source) {
8585
destination.$addToSet = (destination.$addToSet || []).concat(source.$addToSet);
8686
}
8787
if (source.$set != null) {
88-
destination.$set = Object.assign(destination.$set || {}, source.$set);
88+
destination.$set = Array.isArray(source.$set) ? [...source.$set] : Object.assign({}, source.$set);
8989
}
9090

9191
return destination;

test/docs/transactions.test.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,62 @@ describe('transactions', function() {
397397
assert.equal(docs[0].name, 'test');
398398
});
399399

400+
it('handles resetting array state with $set atomic (gh-13698)', async function() {
401+
db.deleteModel(/Test/);
402+
const subItemSchema = new mongoose.Schema(
403+
{
404+
name: { type: String, required: true }
405+
},
406+
{ _id: false }
407+
);
408+
409+
const itemSchema = new mongoose.Schema(
410+
{
411+
name: { type: String, required: true },
412+
subItems: { type: [subItemSchema], required: true }
413+
},
414+
{ _id: false }
415+
);
416+
417+
const schema = new mongoose.Schema({
418+
items: { type: [itemSchema], required: true }
419+
});
420+
421+
const Test = db.model('Test', schema);
422+
423+
const { _id } = await Test.create({
424+
items: [
425+
{ name: 'test1', subItems: [{ name: 'x1' }] },
426+
{ name: 'test2', subItems: [{ name: 'x2' }] }
427+
]
428+
});
429+
430+
const doc = await Test.findById(_id).orFail();
431+
let attempt = 0;
432+
433+
await db.transaction(async(session) => {
434+
await doc.save({ session });
435+
436+
if (attempt === 0) {
437+
attempt += 1;
438+
throw new mongoose.mongo.MongoServerError({
439+
message: 'Test transient transaction failures & retries',
440+
errorLabels: [mongoose.mongo.MongoErrorLabel.TransientTransactionError]
441+
});
442+
}
443+
});
444+
445+
const { items } = await Test.findById(_id).orFail();
446+
assert.ok(Array.isArray(items));
447+
assert.equal(items.length, 2);
448+
assert.equal(items[0].name, 'test1');
449+
assert.equal(items[0].subItems.length, 1);
450+
assert.equal(items[0].subItems[0].name, 'x1');
451+
assert.equal(items[1].name, 'test2');
452+
assert.equal(items[1].subItems.length, 1);
453+
assert.equal(items[1].subItems[0].name, 'x2');
454+
});
455+
400456
it('transaction() retains modified status for documents created outside of the transaction then modified inside the transaction (gh-13973)', async function() {
401457
db.deleteModel(/Test/);
402458
const Test = db.model('Test', Schema({ status: String }));

0 commit comments

Comments
 (0)