diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js index 3f61c4e6ad..d8c8bda78a 100644 --- a/lib/helpers/model/castBulkWrite.js +++ b/lib/helpers/model/castBulkWrite.js @@ -119,7 +119,8 @@ module.exports.castUpdateOne = function castUpdateOne(originalModel, updateOne, const createdAt = model.schema.$timestamps.createdAt; const updatedAt = model.schema.$timestamps.updatedAt; applyTimestampsToUpdate(now, createdAt, updatedAt, update, { - timestamps: updateOne.timestamps + timestamps: updateOne.timestamps, + overwriteImmutable: updateOne.overwriteImmutable }); } @@ -189,7 +190,8 @@ module.exports.castUpdateMany = function castUpdateMany(originalModel, updateMan const createdAt = model.schema.$timestamps.createdAt; const updatedAt = model.schema.$timestamps.updatedAt; applyTimestampsToUpdate(now, createdAt, updatedAt, updateMany['update'], { - timestamps: updateMany.timestamps + timestamps: updateMany.timestamps, + overwriteImmutable: updateMany.overwriteImmutable }); } if (doInitTimestamps) { diff --git a/lib/helpers/update/applyTimestampsToUpdate.js b/lib/helpers/update/applyTimestampsToUpdate.js index e8d3217fbb..fd0828b5a0 100644 --- a/lib/helpers/update/applyTimestampsToUpdate.js +++ b/lib/helpers/update/applyTimestampsToUpdate.js @@ -80,33 +80,46 @@ function applyTimestampsToUpdate(now, createdAt, updatedAt, currentUpdate, optio } if (!skipCreatedAt && createdAt) { - if (currentUpdate[createdAt]) { - delete currentUpdate[createdAt]; - } - if (currentUpdate.$set && currentUpdate.$set[createdAt]) { - delete currentUpdate.$set[createdAt]; - } - let timestampSet = false; - if (createdAt.indexOf('.') !== -1) { - const pieces = createdAt.split('.'); - for (let i = 1; i < pieces.length; ++i) { - const remnant = pieces.slice(-i).join('.'); - const start = pieces.slice(0, -i).join('.'); - if (currentUpdate[start] != null) { - currentUpdate[start][remnant] = now; - timestampSet = true; - break; - } else if (currentUpdate.$set && currentUpdate.$set[start]) { - currentUpdate.$set[start][remnant] = now; - timestampSet = true; - break; + const overwriteImmutable = get(options, 'overwriteImmutable', false); + const hasUserCreatedAt = currentUpdate[createdAt] != null || currentUpdate?.$set[createdAt] != null; + + // If overwriteImmutable is true and user provided createdAt, keep their value + if (overwriteImmutable && hasUserCreatedAt) { + // Move createdAt from top-level to $set if needed + if (currentUpdate[createdAt] != null) { + updates.$set[createdAt] = currentUpdate[createdAt]; + delete currentUpdate[createdAt]; + } + // User's value is already in $set, nothing more to do + } else { + if (currentUpdate[createdAt]) { + delete currentUpdate[createdAt]; + } + if (currentUpdate.$set && currentUpdate.$set[createdAt]) { + delete currentUpdate.$set[createdAt]; + } + let timestampSet = false; + if (createdAt.indexOf('.') !== -1) { + const pieces = createdAt.split('.'); + for (let i = 1; i < pieces.length; ++i) { + const remnant = pieces.slice(-i).join('.'); + const start = pieces.slice(0, -i).join('.'); + if (currentUpdate[start] != null) { + currentUpdate[start][remnant] = now; + timestampSet = true; + break; + } else if (currentUpdate.$set && currentUpdate.$set[start]) { + currentUpdate.$set[start][remnant] = now; + timestampSet = true; + break; + } } } - } - if (!timestampSet) { - updates.$setOnInsert = updates.$setOnInsert || {}; - updates.$setOnInsert[createdAt] = now; + if (!timestampSet) { + updates.$setOnInsert = updates.$setOnInsert || {}; + updates.$setOnInsert[createdAt] = now; + } } } diff --git a/package.json b/package.json index b560323448..1e0f0c6ac8 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "test-tsd": "node ./test/types/check-types-filename && tsd --full", "setup-test-encryption": "node scripts/setup-encryption-tests.js", "test-encryption": "mocha --exit ./test/encryption/*.test.js", - "tdd": "mocha ./test/*.test.js --inspect --watch --recursive --watch-files ./**/*.{js,ts}", + "tdd": "mocha --watch --inspect --recursive ./test/*.test.js --watch-files lib/**/*.js test/**/*.js", "test-coverage": "nyc --reporter=html --reporter=text npm test", "ts-benchmark": "cd ./benchmarks/typescript/simple && npm install && npm run benchmark | node ../../../scripts/tsc-diagnostics-check", "attest-benchmark": "node ./benchmarks/typescript/infer.bench.mts" @@ -139,4 +139,4 @@ "target": "ES2022" } } -} +} \ No newline at end of file diff --git a/test/model.updateOne.test.js b/test/model.updateOne.test.js index aef03e3d37..d045f40d07 100644 --- a/test/model.updateOne.test.js +++ b/test/model.updateOne.test.js @@ -2680,34 +2680,136 @@ describe('model: updateOne: ', function() { assert.equal(doc.age, 20); }); - it('overwriting immutable createdAt with bulkWrite (gh-15781)', async function() { - const start = new Date().valueOf(); - const schema = Schema({ - createdAt: { - type: mongoose.Schema.Types.Date, - immutable: true - }, - name: String - }, { timestamps: true }); + describe('bulkWrite overwriteImmutable option (gh-15781)', function() { + it('updateOne can update immutable field with overwriteImmutable: true', async function() { + // Arrange + const { User } = createTestContext(); + const user = await User.create({ name: 'John', ssn: '123-45-6789' }); + const customCreatedAt = new Date('2020-01-01'); + + // Act + await User.bulkWrite([{ + updateOne: { + filter: { _id: user._id }, + update: { createdAt: customCreatedAt, ssn: '999-99-9999' }, + overwriteImmutable: true + } + }]); + + // Assert + const updatedUser = await User.findById(user._id); + assert.strictEqual(updatedUser.ssn, '999-99-9999'); + assert.strictEqual(updatedUser.createdAt.valueOf(), customCreatedAt.valueOf()); + }); + + it('updateMany can update immutable field with overwriteImmutable: true', async function() { + // Arrange + const { User } = createTestContext(); + const user = await User.create({ name: 'Alice', ssn: '111-11-1111' }); + const customCreatedAt = new Date('2020-01-01'); + + // Act + await User.bulkWrite([{ + updateMany: { + filter: { _id: user._id }, + update: { createdAt: customCreatedAt, ssn: '000-00-0000' }, + overwriteImmutable: true + } + }]); - const Model = db.model('Test', schema); + // Assert + const updatedUser = await User.findById(user._id); + assert.strictEqual(updatedUser.ssn, '000-00-0000'); + assert.strictEqual(updatedUser.createdAt.valueOf(), customCreatedAt.valueOf()); + }); - await Model.create({ name: 'gh-15781' }); - let doc = await Model.collection.findOne({ name: 'gh-15781' }); - assert.ok(doc.createdAt.valueOf() >= start); + for (const timestamps of [true, false, null, undefined]) { + it(`overwriting immutable createdAt with bulkWrite (gh-15781) when \`timestamps\` is \`${timestamps}\``, async function() { + // Arrange + const schema = Schema({ name: String }, { timestamps: true }); - const createdAt = new Date('2011-06-01'); - assert.ok(createdAt.valueOf() < start.valueOf()); - await Model.bulkWrite([{ - updateOne: { - filter: { _id: doc._id }, - update: { name: 'gh-15781 update', createdAt }, - overwriteImmutable: true, - timestamps: false - } - }]); - doc = await Model.collection.findOne({ name: 'gh-15781 update' }); - assert.equal(doc.createdAt.valueOf(), createdAt.valueOf()); + const Model = db.model('Test', schema); + + const doc1 = await Model.create({ name: 'gh-15781-1' }); + const doc2 = await Model.create({ name: 'gh-15781-2' }); + + // Act + const createdAt = new Date('2011-06-01'); + + await Model.bulkWrite([ + { + updateOne: { + filter: { _id: doc1._id }, + update: { createdAt }, + overwriteImmutable: true, + timestamps + } + }, + { + updateMany: { + filter: { _id: doc2._id }, + update: { createdAt }, + overwriteImmutable: true, + timestamps + } + } + ]); + + // Assert + const updatesDocs = await Model.find({ _id: { $in: [doc1._id, doc2._id] } }); + + assert.equal(updatesDocs[0].createdAt.valueOf(), createdAt.valueOf()); + assert.equal(updatesDocs[1].createdAt.valueOf(), createdAt.valueOf()); + }); + } + + it('can not update immutable fields without overwriteImmutable: true', async function() { + // Arrange + const { User } = createTestContext(); + const users = await User.create([ + { name: 'Bob', ssn: '222-22-2222' }, + { name: 'Eve', ssn: '333-33-3333' } + ]); + const newCreatedAt = new Date('2020-01-01'); + + // Act + await User.bulkWrite([ + { + updateOne: { + filter: { _id: users[0]._id }, + update: { ssn: '888-88-8888', createdAt: newCreatedAt } + } + + }, + { + updateMany: { + filter: { _id: users[1]._id }, + update: { ssn: '777-77-7777', createdAt: newCreatedAt } + } + } + ]); + + + // Assert + const [updatedUser1, updatedUser2] = await Promise.all([ + User.findById(users[0]._id), + User.findById(users[1]._id) + ]); + assert.strictEqual(updatedUser1.ssn, '222-22-2222'); + assert.notStrictEqual(updatedUser1.createdAt.valueOf(), newCreatedAt.valueOf()); + + assert.strictEqual(updatedUser2.ssn, '333-33-3333'); + assert.notStrictEqual(updatedUser2.createdAt.valueOf(), newCreatedAt.valueOf()); + }); + + function createTestContext() { + const userSchema = new Schema({ + name: String, + ssn: { type: String, immutable: true } + }, { timestamps: true }); + const User = db.model('User', userSchema); + return { User }; + } }); it('updates buffers with `runValidators` successfully (gh-8580)', async function() { diff --git a/test/types/models.test.ts b/test/types/models.test.ts index 8142f71b84..0f3c14b7c7 100644 --- a/test/types/models.test.ts +++ b/test/types/models.test.ts @@ -14,14 +14,14 @@ import mongoose, { UpdateQuery, UpdateWriteOpResult, WithLevel1NestedPaths, - createConnection, connection, model, - ObtainSchemaGeneric + UpdateOneModel, + UpdateManyModel } from 'mongoose'; import { expectAssignable, expectError, expectType } from 'tsd'; import { AutoTypedSchemaType, autoTypedSchema } from './schema.test'; -import { ModifyResult, UpdateOneModel, ChangeStreamInsertDocument, ObjectId } from 'mongodb'; +import { ModifyResult, UpdateOneModel as MongoUpdateOneModel, ChangeStreamInsertDocument, ObjectId } from 'mongodb'; function rawDocSyntax(): void { interface ITest { @@ -413,7 +413,7 @@ function gh11911() { const Animal = model('Animal', animalSchema); const changes: UpdateQuery = {}; - expectAssignable({ + expectAssignable({ filter: {}, update: changes }); @@ -507,7 +507,7 @@ function gh12100() { })(); -function modelRemoveOptions() { +async function modelRemoveOptions() { const cmodel = model('Test', new Schema()); const res: DeleteResult = await cmodel.deleteOne({}, {}); @@ -1089,3 +1089,36 @@ async function gh15693() { User.schema.methods.printName.apply(leanInst); } + +async function gh15781() { + const userSchema = new Schema({ + createdAt: { type: Date, immutable: true }, + name: String + }, { timestamps: true }); + + const User = model('User', userSchema); + + await User.bulkWrite([ + { + updateOne: { + filter: { name: 'John' }, + update: { createdAt: new Date() }, + overwriteImmutable: true, + timestamps: false + } + }, + { + updateMany: { + filter: { name: 'Jane' }, + update: { createdAt: new Date() }, + overwriteImmutable: true, + timestamps: false + } + } + ]); + + expectType({} as UpdateOneModel['timestamps']); + expectType({} as UpdateOneModel['overwriteImmutable']); + expectType({} as UpdateManyModel['timestamps']); + expectType({} as UpdateManyModel['overwriteImmutable']); +} diff --git a/types/models.d.ts b/types/models.d.ts index 627666c24e..a049861f63 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -224,6 +224,8 @@ declare module 'mongoose' { upsert?: boolean; /** When false, do not add timestamps. When true, overrides the `timestamps` option set in the `bulkWrite` options. */ timestamps?: boolean; + /** When true, allows updating fields that are marked as `immutable` in the schema. */ + overwriteImmutable?: boolean; } export interface UpdateManyModel { @@ -241,6 +243,8 @@ declare module 'mongoose' { upsert?: boolean; /** When false, do not add timestamps. When true, overrides the `timestamps` option set in the `bulkWrite` options. */ timestamps?: boolean; + /** When true, allows updating fields that are marked as `immutable` in the schema. */ + overwriteImmutable?: boolean; } export interface DeleteOneModel {