Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated timestamp not get updated for noop, aside of upsert usage #13170

Closed
2 tasks done
cherviakovtaskworld opened this issue Mar 15, 2023 · 3 comments
Closed
2 tasks done

Comments

@cherviakovtaskworld
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

7.0.1

Node.js version

19.6.0

MongoDB server version

5.1.0

Typescript version (if applicable)

No response

Description

We requrie to update documents the way that only updated timestamp get changed, trying to perform update operation with empty object, which is noop. Mongoose seem to catching it and does not perform any operation. If we add upsert: true option though and document exists, mongoose does perform update even if it is noop. Well we can't use upsert as it may lead to many partial documents get inserted, but in the same time seems without upsert and without any actual update to the document we can't get updated timestmap to be updated.

Checked that in pure mongodb nodejs driver when we on update adding $set it is working as expected and update timestamp field every time

await db.collection('users').updateOne({ name: 'test' }, { $setOnInsert: { created: new Date }, $set: { updated: new Date } })

therefore it is issue on mongoose side.

Steps to Reproduce

import mongoose from 'mongoose'

await mongoose.connect('mongodb://127.0.0.1:27017/test')

const UsersSchema = new mongoose.Schema({
  name: String,
  // updated: { type: Date, default: Date.now() },
  // created: { type: Date, default: Date.now() }
}, {
  timestamps: {
    createdAt: 'created',
    updatedAt: 'updated'
  }
})

const UsersModel = mongoose.model('users', UsersSchema)

await UsersModel.deleteMany({})
await UsersModel.create({ name: 'test' })
const user1 = await UsersModel.findOne({ name: 'test' }).lean()
console.log('user1', user1)
await new Promise((resolve) => setTimeout(() => resolve(), 1000))
const user2 = await UsersModel.findOneAndUpdate({ name: 'test' }, {}, { new: true })
// await UsersModel.updateOne({ name: 'test' }, { name: 'test1' })
// const user2 = await UsersModel.findOne({ name: 'test1' }).lean()
console.log('user2', user2)

Expected Behavior

updated timestamp get updated even on noop update operation, or there separate operations to only update timestmpas (e.g. bash touch) or there option to enforce update without creation if document not exists.

@vkarpov15 vkarpov15 added this to the 7.0.3 milestone Mar 15, 2023
@vkarpov15 vkarpov15 added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Mar 15, 2023
@vkarpov15
Copy link
Collaborator

As a workaround, you can explicitly set updated in your update as follows:

const user2 = await UsersModel.findOneAndUpdate({ name: 'test' }, { updated: Date.now() }, { new: true })

We're investigating this to see if this is a bug we can fix or expected behavior.

@vkarpov15
Copy link
Collaborator

I took a closer look and I think we're going to have to call this expected behavior. While we do always apply timestamps to findOneAndReplace() (see #9951), I don't think we should apply timestamps to empty findOneAndUpdate() unless there's an upsert.

The reasoning is that it is easier to apply timestamps to an empty update, than it is to tell Mongoose to not apply timestamps to empty updates if we make this change.

To tell Mongoose to update updatedAt, even if findOneAndUpdate() is empty, you can either add updatedAt to the update as a one-off:

const user2 = await UsersModel.findOneAndUpdate({ name: 'test' }, { updated: Date.now() }, { new: true })

Or add a plugin to handle that for all updates:

mongoose.plugin(schema => {
  schema.pre('findOneAndUpdate', function() {
    if (this.getUpdate() == null || Object.keys(this.getUpdate()).length === 0) {
      this.set('updatedAt', new Date());
    }
  });
});

Given that there's a simple workaround, we don't want to switch this behavior.

@vkarpov15 vkarpov15 added won't fix and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Mar 22, 2023
@vkarpov15 vkarpov15 removed this from the 7.0.3 milestone Mar 22, 2023
vkarpov15 added a commit that referenced this issue Mar 22, 2023
fix(timestamps): set timestamps on empty `replaceOne()`
@cherviakovtaskworld
Copy link
Author

We found workaround for this one:

const user2 = await UsersModel.findOneAndUpdate({ name: 'test' }, { $set: {} }, { new: true })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants