Skip to content

Commit a5e0833

Browse files
authored
Merge pull request #15789 from Automattic/vkarpov15/gh-15782
fix(bulkWrite): pass overwriteImmutable option to castUpdate fixes
2 parents 15e7743 + 82027a4 commit a5e0833

File tree

8 files changed

+318
-38
lines changed

8 files changed

+318
-38
lines changed

lib/helpers/model/castBulkWrite.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ module.exports = function castBulkWrite(originalModel, op, options) {
2828
const model = decideModelByObject(originalModel, op['insertOne']['document']);
2929

3030
const doc = new model(op['insertOne']['document']);
31-
if (model.schema.options.timestamps && options.timestamps !== false) {
31+
if (model.schema.options.timestamps && (op['insertOne'].timestamps ?? options.timestamps ?? true)) {
3232
doc.initializeTimestamps();
3333
}
3434
if (options.session != null) {
@@ -69,7 +69,10 @@ module.exports = function castBulkWrite(originalModel, op, options) {
6969
if (model.schema.$timestamps != null && op['updateOne'].timestamps !== false) {
7070
const createdAt = model.schema.$timestamps.createdAt;
7171
const updatedAt = model.schema.$timestamps.updatedAt;
72-
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateOne']['update'], {});
72+
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateOne']['update'], {
73+
timestamps: op['updateOne'].timestamps,
74+
overwriteImmutable: op['updateOne'].overwriteImmutable
75+
});
7376
}
7477

7578
if (op['updateOne'].timestamps !== false) {
@@ -100,7 +103,8 @@ module.exports = function castBulkWrite(originalModel, op, options) {
100103
op['updateOne']['update'] = castUpdate(model.schema, op['updateOne']['update'], {
101104
strict: strict,
102105
overwrite: false,
103-
upsert: op['updateOne'].upsert
106+
upsert: op['updateOne'].upsert,
107+
overwriteImmutable: op['updateOne'].overwriteImmutable
104108
}, model, op['updateOne']['filter']);
105109
} catch (error) {
106110
return callback(error, null);
@@ -136,7 +140,10 @@ module.exports = function castBulkWrite(originalModel, op, options) {
136140
if (model.schema.$timestamps != null && op['updateMany'].timestamps !== false) {
137141
const createdAt = model.schema.$timestamps.createdAt;
138142
const updatedAt = model.schema.$timestamps.updatedAt;
139-
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateMany']['update'], {});
143+
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateMany']['update'], {
144+
timestamps: op['updateMany'].timestamps,
145+
overwriteImmutable: op['updateMany'].overwriteImmutable
146+
});
140147
}
141148
if (op['updateMany'].timestamps !== false) {
142149
applyTimestampsToChildren(now, op['updateMany']['update'], model.schema);
@@ -158,7 +165,8 @@ module.exports = function castBulkWrite(originalModel, op, options) {
158165
op['updateMany']['update'] = castUpdate(model.schema, op['updateMany']['update'], {
159166
strict: strict,
160167
overwrite: false,
161-
upsert: op['updateMany'].upsert
168+
upsert: op['updateMany'].upsert,
169+
overwriteImmutable: op['updateMany'].overwriteImmutable
162170
}, model, op['updateMany']['filter']);
163171
} catch (error) {
164172
return callback(error, null);
@@ -184,7 +192,7 @@ module.exports = function castBulkWrite(originalModel, op, options) {
184192

185193
// set `skipId`, otherwise we get "_id field cannot be changed"
186194
const doc = new model(op['replaceOne']['replacement'], strict, true);
187-
if (model.schema.options.timestamps) {
195+
if (model.schema.options.timestamps && (op['replaceOne'].timestamps ?? options.timestamps ?? true)) {
188196
doc.initializeTimestamps();
189197
}
190198
if (options.session != null) {
@@ -273,3 +281,4 @@ function decideModelByObject(model, object) {
273281
}
274282
return model;
275283
}
284+

lib/helpers/query/castUpdate.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
240240

241241
if (op !== '$setOnInsert' &&
242242
!options.overwrite &&
243-
handleImmutable(schematype, strict, obj, key, prefix + key, context)) {
243+
handleImmutable(schematype, strict, obj, key, prefix + key, options, context)) {
244244
continue;
245245
}
246246

@@ -335,7 +335,7 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
335335
// You can use `$setOnInsert` with immutable keys
336336
if (op !== '$setOnInsert' &&
337337
!options.overwrite &&
338-
handleImmutable(schematype, strict, obj, key, prefix + key, context)) {
338+
handleImmutable(schematype, strict, obj, key, prefix + key, options, context)) {
339339
continue;
340340
}
341341

lib/helpers/query/handleImmutable.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
const StrictModeError = require('../../error/strict');
44

5-
module.exports = function handleImmutable(schematype, strict, obj, key, fullPath, ctx) {
5+
module.exports = function handleImmutable(schematype, strict, obj, key, fullPath, options, ctx) {
66
if (schematype == null || !schematype.options || !schematype.options.immutable) {
77
return false;
88
}
@@ -15,6 +15,10 @@ module.exports = function handleImmutable(schematype, strict, obj, key, fullPath
1515
return false;
1616
}
1717

18+
if (options && options.overwriteImmutable) {
19+
return false;
20+
}
21+
1822
if (strict === false) {
1923
return false;
2024
}

lib/helpers/update/applyTimestampsToUpdate.js

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -81,33 +81,46 @@ function applyTimestampsToUpdate(now, createdAt, updatedAt, currentUpdate, optio
8181
}
8282

8383
if (!skipCreatedAt && createdAt) {
84-
if (currentUpdate[createdAt]) {
85-
delete currentUpdate[createdAt];
86-
}
87-
if (currentUpdate.$set && currentUpdate.$set[createdAt]) {
88-
delete currentUpdate.$set[createdAt];
89-
}
90-
let timestampSet = false;
91-
if (createdAt.indexOf('.') !== -1) {
92-
const pieces = createdAt.split('.');
93-
for (let i = 1; i < pieces.length; ++i) {
94-
const remnant = pieces.slice(-i).join('.');
95-
const start = pieces.slice(0, -i).join('.');
96-
if (currentUpdate[start] != null) {
97-
currentUpdate[start][remnant] = now;
98-
timestampSet = true;
99-
break;
100-
} else if (currentUpdate.$set && currentUpdate.$set[start]) {
101-
currentUpdate.$set[start][remnant] = now;
102-
timestampSet = true;
103-
break;
84+
const overwriteImmutable = get(options, 'overwriteImmutable', false);
85+
const hasUserCreatedAt = currentUpdate[createdAt] != null || currentUpdate?.$set[createdAt] != null;
86+
87+
// If overwriteImmutable is true and user provided createdAt, keep their value
88+
if (overwriteImmutable && hasUserCreatedAt) {
89+
// Move createdAt from top-level to $set if needed
90+
if (currentUpdate[createdAt] != null) {
91+
updates.$set[createdAt] = currentUpdate[createdAt];
92+
delete currentUpdate[createdAt];
93+
}
94+
// User's value is already in $set, nothing more to do
95+
} else {
96+
if (currentUpdate[createdAt]) {
97+
delete currentUpdate[createdAt];
98+
}
99+
if (currentUpdate.$set && currentUpdate.$set[createdAt]) {
100+
delete currentUpdate.$set[createdAt];
101+
}
102+
let timestampSet = false;
103+
if (createdAt.indexOf('.') !== -1) {
104+
const pieces = createdAt.split('.');
105+
for (let i = 1; i < pieces.length; ++i) {
106+
const remnant = pieces.slice(-i).join('.');
107+
const start = pieces.slice(0, -i).join('.');
108+
if (currentUpdate[start] != null) {
109+
currentUpdate[start][remnant] = now;
110+
timestampSet = true;
111+
break;
112+
} else if (currentUpdate.$set && currentUpdate.$set[start]) {
113+
currentUpdate.$set[start][remnant] = now;
114+
timestampSet = true;
115+
break;
116+
}
104117
}
105118
}
106-
}
107119

108-
if (!timestampSet) {
109-
updates.$setOnInsert = updates.$setOnInsert || {};
110-
updates.$setOnInsert[createdAt] = now;
120+
if (!timestampSet) {
121+
updates.$setOnInsert = updates.$setOnInsert || {};
122+
updates.$setOnInsert[createdAt] = now;
123+
}
111124
}
112125
}
113126

test/model.test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5926,6 +5926,52 @@ describe('Model', function() {
59265926

59275927
});
59285928

5929+
it('bulkWrite can disable timestamps with insertOne and replaceOne (gh-15782)', async function() {
5930+
const userSchema = new Schema({
5931+
name: String
5932+
}, { timestamps: true });
5933+
5934+
const User = db.model('User', userSchema);
5935+
5936+
const user = await User.create({ name: 'Hafez' });
5937+
5938+
await User.bulkWrite([
5939+
{ insertOne: { document: { name: 'insertOne-test' }, timestamps: false } },
5940+
{ replaceOne: { filter: { _id: user._id }, replacement: { name: 'replaceOne-test' }, timestamps: false } }
5941+
]);
5942+
5943+
const insertedDoc = await User.findOne({ name: 'insertOne-test' });
5944+
assert.strictEqual(insertedDoc.createdAt, undefined);
5945+
assert.strictEqual(insertedDoc.updatedAt, undefined);
5946+
5947+
const replacedDoc = await User.findOne({ name: 'replaceOne-test' });
5948+
assert.strictEqual(replacedDoc.createdAt, undefined);
5949+
assert.strictEqual(replacedDoc.updatedAt, undefined);
5950+
});
5951+
5952+
it('bulkWrite insertOne and replaceOne respect per-op timestamps: true when global is false (gh-15782)', async function() {
5953+
const userSchema = new Schema({
5954+
name: String
5955+
}, { timestamps: true });
5956+
5957+
const User = db.model('User', userSchema);
5958+
5959+
const user = await User.create({ name: 'Hafez' });
5960+
5961+
await User.bulkWrite([
5962+
{ insertOne: { document: { name: 'insertOne-test' }, timestamps: true } },
5963+
{ replaceOne: { filter: { _id: user._id }, replacement: { name: 'replaceOne-test' }, timestamps: true } }
5964+
], { timestamps: false });
5965+
5966+
const insertedDoc = await User.findOne({ name: 'insertOne-test' });
5967+
assert.ok(insertedDoc.createdAt instanceof Date);
5968+
assert.ok(insertedDoc.updatedAt instanceof Date);
5969+
5970+
const replacedDoc = await User.findOne({ name: 'replaceOne-test' });
5971+
assert.ok(replacedDoc.createdAt instanceof Date);
5972+
assert.ok(replacedDoc.updatedAt instanceof Date);
5973+
});
5974+
59295975
it('bulkwrite should not change updatedAt on subdocs when timestamps set to false (gh-13611)', async function() {
59305976

59315977
const postSchema = new Schema({

test/model.updateOne.test.js

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,6 +2707,139 @@ describe('model: updateOne: ', function() {
27072707
assert.equal(doc.age, 20);
27082708
});
27092709

2710+
describe('bulkWrite overwriteImmutable option (gh-15781)', function() {
2711+
it('updateOne can update immutable field with overwriteImmutable: true', async function() {
2712+
// Arrange
2713+
const { User } = createTestContext();
2714+
const user = await User.create({ name: 'John', ssn: '123-45-6789' });
2715+
const customCreatedAt = new Date('2020-01-01');
2716+
2717+
// Act
2718+
await User.bulkWrite([{
2719+
updateOne: {
2720+
filter: { _id: user._id },
2721+
update: { createdAt: customCreatedAt, ssn: '999-99-9999' },
2722+
overwriteImmutable: true
2723+
}
2724+
}]);
2725+
2726+
// Assert
2727+
const updatedUser = await User.findById(user._id);
2728+
assert.strictEqual(updatedUser.ssn, '999-99-9999');
2729+
assert.strictEqual(updatedUser.createdAt.valueOf(), customCreatedAt.valueOf());
2730+
});
2731+
2732+
it('updateMany can update immutable field with overwriteImmutable: true', async function() {
2733+
// Arrange
2734+
const { User } = createTestContext();
2735+
const user = await User.create({ name: 'Alice', ssn: '111-11-1111' });
2736+
const customCreatedAt = new Date('2020-01-01');
2737+
2738+
// Act
2739+
await User.bulkWrite([{
2740+
updateMany: {
2741+
filter: { _id: user._id },
2742+
update: { createdAt: customCreatedAt, ssn: '000-00-0000' },
2743+
overwriteImmutable: true
2744+
}
2745+
}]);
2746+
2747+
// Assert
2748+
const updatedUser = await User.findById(user._id);
2749+
assert.strictEqual(updatedUser.ssn, '000-00-0000');
2750+
assert.strictEqual(updatedUser.createdAt.valueOf(), customCreatedAt.valueOf());
2751+
});
2752+
2753+
for (const timestamps of [true, false, null, undefined]) {
2754+
it(`overwriting immutable createdAt with bulkWrite (gh-15781) when \`timestamps\` is \`${timestamps}\``, async function() {
2755+
// Arrange
2756+
const schema = Schema({ name: String }, { timestamps: true });
2757+
2758+
const Model = db.model('Test', schema);
2759+
2760+
const doc1 = await Model.create({ name: 'gh-15781-1' });
2761+
const doc2 = await Model.create({ name: 'gh-15781-2' });
2762+
2763+
// Act
2764+
const createdAt = new Date('2011-06-01');
2765+
2766+
await Model.bulkWrite([
2767+
{
2768+
updateOne: {
2769+
filter: { _id: doc1._id },
2770+
update: { createdAt },
2771+
overwriteImmutable: true,
2772+
timestamps
2773+
}
2774+
},
2775+
{
2776+
updateMany: {
2777+
filter: { _id: doc2._id },
2778+
update: { createdAt },
2779+
overwriteImmutable: true,
2780+
timestamps
2781+
}
2782+
}
2783+
]);
2784+
2785+
// Assert
2786+
const updatesDocs = await Model.find({ _id: { $in: [doc1._id, doc2._id] } });
2787+
2788+
assert.equal(updatesDocs[0].createdAt.valueOf(), createdAt.valueOf());
2789+
assert.equal(updatesDocs[1].createdAt.valueOf(), createdAt.valueOf());
2790+
});
2791+
2792+
it(`can not update immutable fields without overwriteImmutable: true and timestamps: ${timestamps}`, async function() {
2793+
// Arrange
2794+
const { User } = createTestContext();
2795+
const users = await User.create([
2796+
{ name: 'Bob', ssn: '222-22-2222' },
2797+
{ name: 'Eve', ssn: '333-33-3333' }
2798+
]);
2799+
const newCreatedAt = new Date('2020-01-01');
2800+
2801+
// Act
2802+
await User.bulkWrite([
2803+
{
2804+
updateOne: {
2805+
filter: { _id: users[0]._id },
2806+
update: { ssn: '888-88-8888', createdAt: newCreatedAt }
2807+
},
2808+
timestamps
2809+
},
2810+
{
2811+
updateMany: {
2812+
filter: { _id: users[1]._id },
2813+
update: { ssn: '777-77-7777', createdAt: newCreatedAt }
2814+
},
2815+
timestamps
2816+
}
2817+
]);
2818+
2819+
2820+
// Assert
2821+
const [updatedUser1, updatedUser2] = await Promise.all([
2822+
User.findById(users[0]._id),
2823+
User.findById(users[1]._id)
2824+
]);
2825+
assert.strictEqual(updatedUser1.ssn, '222-22-2222');
2826+
assert.notStrictEqual(updatedUser1.createdAt.valueOf(), newCreatedAt.valueOf());
2827+
2828+
assert.strictEqual(updatedUser2.ssn, '333-33-3333');
2829+
assert.notStrictEqual(updatedUser2.createdAt.valueOf(), newCreatedAt.valueOf());
2830+
});
2831+
}
2832+
2833+
function createTestContext() {
2834+
const userSchema = new Schema({
2835+
name: String,
2836+
ssn: { type: String, immutable: true }
2837+
}, { timestamps: true });
2838+
const User = db.model('User', userSchema);
2839+
return { User };
2840+
}
2841+
});
2842+
27102843
it('updates buffers with `runValidators` successfully (gh-8580)', async function() {
27112844
const Test = db.model('Test', Schema({
27122845
data: { type: Buffer, required: true }

0 commit comments

Comments
 (0)