Skip to content

Commit 39b8251

Browse files
vkarpov15vkarpov15
authored
Merge pull request #15819 from Automattic/add-bulkwrite-overwriteimmutable-types
fix(model): fix overwriteImmutable not working with timestamps: true, add `overwriteImmutable` types re #15781
2 parents db882c0 + f64ad6d commit 39b8251

File tree

6 files changed

+213
-58
lines changed

6 files changed

+213
-58
lines changed

‎lib/helpers/model/castBulkWrite.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ module.exports.castUpdateOne = function castUpdateOne(originalModel, updateOne,
119119
const createdAt = model.schema.$timestamps.createdAt;
120120
const updatedAt = model.schema.$timestamps.updatedAt;
121121
applyTimestampsToUpdate(now, createdAt, updatedAt, update, {
122-
timestamps: updateOne.timestamps
122+
timestamps: updateOne.timestamps,
123+
overwriteImmutable: updateOne.overwriteImmutable
123124
});
124125
}
125126

@@ -189,7 +190,8 @@ module.exports.castUpdateMany = function castUpdateMany(originalModel, updateMan
189190
const createdAt = model.schema.$timestamps.createdAt;
190191
const updatedAt = model.schema.$timestamps.updatedAt;
191192
applyTimestampsToUpdate(now, createdAt, updatedAt, updateMany['update'], {
192-
timestamps: updateMany.timestamps
193+
timestamps: updateMany.timestamps,
194+
overwriteImmutable: updateMany.overwriteImmutable
193195
});
194196
}
195197
if (doInitTimestamps) {

‎lib/helpers/update/applyTimestampsToUpdate.js‎

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

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

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

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@
9999
"test-tsd": "node ./test/types/check-types-filename && tsd --full",
100100
"setup-test-encryption": "node scripts/setup-encryption-tests.js",
101101
"test-encryption": "mocha --exit ./test/encryption/*.test.js",
102-
"tdd": "mocha ./test/*.test.js --inspect --watch --recursive --watch-files ./**/*.{js,ts}",
102+
"tdd": "mocha --watch --inspect --recursive ./test/*.test.js --watch-files lib/**/*.js test/**/*.js",
103103
"test-coverage": "nyc --reporter=html --reporter=text npm test",
104104
"ts-benchmark": "cd ./benchmarks/typescript/simple && npm install && npm run benchmark | node ../../../scripts/tsc-diagnostics-check",
105105
"attest-benchmark": "node ./benchmarks/typescript/infer.bench.mts"
@@ -139,4 +139,4 @@
139139
"target": "ES2022"
140140
}
141141
}
142-
}
142+
}

‎test/model.updateOne.test.js‎

Lines changed: 128 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2680,34 +2680,137 @@ describe('model: updateOne: ', function() {
26802680
assert.equal(doc.age, 20);
26812681
});
26822682

2683-
it('overwriting immutable createdAt with bulkWrite (gh-15781)', async function() {
2684-
const start = new Date().valueOf();
2685-
const schema = Schema({
2686-
createdAt: {
2687-
type: mongoose.Schema.Types.Date,
2688-
immutable: true
2689-
},
2690-
name: String
2691-
}, { timestamps: true });
2683+
describe('bulkWrite overwriteImmutable option (gh-15781)', function() {
2684+
it('updateOne can update immutable field with overwriteImmutable: true', async function() {
2685+
// Arrange
2686+
const { User } = createTestContext();
2687+
const user = await User.create({ name: 'John', ssn: '123-45-6789' });
2688+
const customCreatedAt = new Date('2020-01-01');
2689+
2690+
// Act
2691+
await User.bulkWrite([{
2692+
updateOne: {
2693+
filter: { _id: user._id },
2694+
update: { createdAt: customCreatedAt, ssn: '999-99-9999' },
2695+
overwriteImmutable: true
2696+
}
2697+
}]);
2698+
2699+
// Assert
2700+
const updatedUser = await User.findById(user._id);
2701+
assert.strictEqual(updatedUser.ssn, '999-99-9999');
2702+
assert.strictEqual(updatedUser.createdAt.valueOf(), customCreatedAt.valueOf());
2703+
});
2704+
2705+
it('updateMany can update immutable field with overwriteImmutable: true', async function() {
2706+
// Arrange
2707+
const { User } = createTestContext();
2708+
const user = await User.create({ name: 'Alice', ssn: '111-11-1111' });
2709+
const customCreatedAt = new Date('2020-01-01');
2710+
2711+
// Act
2712+
await User.bulkWrite([{
2713+
updateMany: {
2714+
filter: { _id: user._id },
2715+
update: { createdAt: customCreatedAt, ssn: '000-00-0000' },
2716+
overwriteImmutable: true
2717+
}
2718+
}]);
26922719

2693-
const Model = db.model('Test', schema);
2720+
// Assert
2721+
const updatedUser = await User.findById(user._id);
2722+
assert.strictEqual(updatedUser.ssn, '000-00-0000');
2723+
assert.strictEqual(updatedUser.createdAt.valueOf(), customCreatedAt.valueOf());
2724+
});
26942725

2695-
await Model.create({ name: 'gh-15781' });
2696-
let doc = await Model.collection.findOne({ name: 'gh-15781' });
2697-
assert.ok(doc.createdAt.valueOf() >= start);
2726+
for (const timestamps of [true, false, null, undefined]) {
2727+
it(`overwriting immutable createdAt with bulkWrite (gh-15781) when \`timestamps\` is \`${timestamps}\``, async function() {
2728+
// Arrange
2729+
const schema = Schema({ name: String }, { timestamps: true });
26982730

2699-
const createdAt = new Date('2011-06-01');
2700-
assert.ok(createdAt.valueOf() < start.valueOf());
2701-
await Model.bulkWrite([{
2702-
updateOne: {
2703-
filter: { _id: doc._id },
2704-
update: { name: 'gh-15781 update', createdAt },
2705-
overwriteImmutable: true,
2706-
timestamps: false
2707-
}
2708-
}]);
2709-
doc = await Model.collection.findOne({ name: 'gh-15781 update' });
2710-
assert.equal(doc.createdAt.valueOf(), createdAt.valueOf());
2731+
const Model = db.model('Test', schema);
2732+
2733+
const doc1 = await Model.create({ name: 'gh-15781-1' });
2734+
const doc2 = await Model.create({ name: 'gh-15781-2' });
2735+
2736+
// Act
2737+
const createdAt = new Date('2011-06-01');
2738+
2739+
await Model.bulkWrite([
2740+
{
2741+
updateOne: {
2742+
filter: { _id: doc1._id },
2743+
update: { createdAt },
2744+
overwriteImmutable: true,
2745+
timestamps
2746+
}
2747+
},
2748+
{
2749+
updateMany: {
2750+
filter: { _id: doc2._id },
2751+
update: { createdAt },
2752+
overwriteImmutable: true,
2753+
timestamps
2754+
}
2755+
}
2756+
]);
2757+
2758+
// Assert
2759+
const updatesDocs = await Model.find({ _id: { $in: [doc1._id, doc2._id] } });
2760+
2761+
assert.equal(updatesDocs[0].createdAt.valueOf(), createdAt.valueOf());
2762+
assert.equal(updatesDocs[1].createdAt.valueOf(), createdAt.valueOf());
2763+
});
2764+
2765+
it(`can not update immutable fields without overwriteImmutable: true and timestamps: ${timestamps}`, async function() {
2766+
// Arrange
2767+
const { User } = createTestContext();
2768+
const users = await User.create([
2769+
{ name: 'Bob', ssn: '222-22-2222' },
2770+
{ name: 'Eve', ssn: '333-33-3333' }
2771+
]);
2772+
const newCreatedAt = new Date('2020-01-01');
2773+
2774+
// Act
2775+
await User.bulkWrite([
2776+
{
2777+
updateOne: {
2778+
filter: { _id: users[0]._id },
2779+
update: { ssn: '888-88-8888', createdAt: newCreatedAt }
2780+
},
2781+
timestamps
2782+
},
2783+
{
2784+
updateMany: {
2785+
filter: { _id: users[1]._id },
2786+
update: { ssn: '777-77-7777', createdAt: newCreatedAt }
2787+
},
2788+
timestamps
2789+
}
2790+
]);
2791+
2792+
2793+
// Assert
2794+
const [updatedUser1, updatedUser2] = await Promise.all([
2795+
User.findById(users[0]._id),
2796+
User.findById(users[1]._id)
2797+
]);
2798+
assert.strictEqual(updatedUser1.ssn, '222-22-2222');
2799+
assert.notStrictEqual(updatedUser1.createdAt.valueOf(), newCreatedAt.valueOf());
2800+
2801+
assert.strictEqual(updatedUser2.ssn, '333-33-3333');
2802+
assert.notStrictEqual(updatedUser2.createdAt.valueOf(), newCreatedAt.valueOf());
2803+
});
2804+
}
2805+
2806+
function createTestContext() {
2807+
const userSchema = new Schema({
2808+
name: String,
2809+
ssn: { type: String, immutable: true }
2810+
}, { timestamps: true });
2811+
const User = db.model('User', userSchema);
2812+
return { User };
2813+
}
27112814
});
27122815

27132816
it('updates buffers with `runValidators` successfully (gh-8580)', async function() {

‎test/types/models.test.ts‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import mongoose, {
1414
UpdateQuery,
1515
UpdateWriteOpResult,
1616
WithLevel1NestedPaths,
17-
createConnection,
1817
connection,
1918
model,
20-
ObtainSchemaGeneric
19+
UpdateOneModel,
20+
UpdateManyModel
2121
} from 'mongoose';
2222
import { expectAssignable, expectError, expectType } from 'tsd';
2323
import { AutoTypedSchemaType, autoTypedSchema } from './schema.test';
24-
import { ModifyResult, UpdateOneModel, ChangeStreamInsertDocument, ObjectId } from 'mongodb';
24+
import { ModifyResult, UpdateOneModel as MongoUpdateOneModel, ChangeStreamInsertDocument, ObjectId } from 'mongodb';
2525

2626
function rawDocSyntax(): void {
2727
interface ITest {
@@ -413,7 +413,7 @@ function gh11911() {
413413
const Animal = model<IAnimal>('Animal', animalSchema);
414414

415415
const changes: UpdateQuery<IAnimal> = {};
416-
expectAssignable<UpdateOneModel>({
416+
expectAssignable<MongoUpdateOneModel>({
417417
filter: {},
418418
update: changes
419419
});
@@ -507,7 +507,7 @@ function gh12100() {
507507
})();
508508

509509

510-
function modelRemoveOptions() {
510+
async function modelRemoveOptions() {
511511
const cmodel = model('Test', new Schema());
512512

513513
const res: DeleteResult = await cmodel.deleteOne({}, {});
@@ -1089,3 +1089,36 @@ async function gh15693() {
10891089
User.schema.methods.printName.apply(leanInst);
10901090

10911091
}
1092+
1093+
async function gh15781() {
1094+
const userSchema = new Schema({
1095+
createdAt: { type: Date, immutable: true },
1096+
name: String
1097+
}, { timestamps: true });
1098+
1099+
const User = model('User', userSchema);
1100+
1101+
await User.bulkWrite([
1102+
{
1103+
updateOne: {
1104+
filter: { name: 'John' },
1105+
update: { createdAt: new Date() },
1106+
overwriteImmutable: true,
1107+
timestamps: false
1108+
}
1109+
},
1110+
{
1111+
updateMany: {
1112+
filter: { name: 'Jane' },
1113+
update: { createdAt: new Date() },
1114+
overwriteImmutable: true,
1115+
timestamps: false
1116+
}
1117+
}
1118+
]);
1119+
1120+
expectType<boolean | undefined>({} as UpdateOneModel['timestamps']);
1121+
expectType<boolean | undefined>({} as UpdateOneModel['overwriteImmutable']);
1122+
expectType<boolean | undefined>({} as UpdateManyModel['timestamps']);
1123+
expectType<boolean | undefined>({} as UpdateManyModel['overwriteImmutable']);
1124+
}

‎types/models.d.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,8 @@ declare module 'mongoose' {
224224
upsert?: boolean;
225225
/** When false, do not add timestamps. When true, overrides the `timestamps` option set in the `bulkWrite` options. */
226226
timestamps?: boolean;
227+
/** When true, allows updating fields that are marked as `immutable` in the schema. */
228+
overwriteImmutable?: boolean;
227229
}
228230

229231
export interface UpdateManyModel<TSchema = AnyObject> {
@@ -241,6 +243,8 @@ declare module 'mongoose' {
241243
upsert?: boolean;
242244
/** When false, do not add timestamps. When true, overrides the `timestamps` option set in the `bulkWrite` options. */
243245
timestamps?: boolean;
246+
/** When true, allows updating fields that are marked as `immutable` in the schema. */
247+
overwriteImmutable?: boolean;
244248
}
245249

246250
export interface DeleteOneModel<TSchema = AnyObject> {

0 commit comments

Comments
 (0)