first remove, then save, why there is no data？

[LeonYanghaha]

please ignore the contradiction of code logic. after the following code is executed, there is no data in mongodb. who can tell me why? according to my understanding, execute document.save()can insert data, but now there is no data. it's strange.

let acl = await Acl.findOneAndRemove({name: "nodejs"});
console.log(acl);
console.log(acl instanceof mongoose.Document); // true
let newAcl = await acl.save();
console.log(newAcl);

nodejs：10.16.3
mongoose: 5.6.9

[AbdelrahmanHafez]

Because under the hood, document.save() either
Model.create(...)s a document if it has document.isNew === true
or
Model.updateOne({ _id: document._id }, changes); if document.isNew === false

Therefore, the code you have attempts to save a document that is removed, but since you haven't made any modifications, mongoose doesn't bother sending a command to the database to update your document, and fails silently, which in my opinion can be confusing, even if it's not clear why the logic would try to remove a document, and then try to save it.

const mongoose = require('mongoose');
mongoose.connect('mongodb://localhost:27017/test', { useUnifiedTopology: true, useNewUrlParser: true, useFindAndModify: true });

const Person = mongoose.model('gh8371', { name: String });


async function run () {
  await Person.deleteMany();
  await Person.create({ name: 'Sam' });

  const removedPerson = await Person.findOneAndRemove({ name: 'Sam' });

  removedPerson.name = 'Hafez'; // removing this line would make the code fail silently

  await removedPerson.save();

  console.log('Done.');
}

run().catch(console.error);

@vkarpov15
Line 13 in the code snippet is what's interesting to me, if we remove this line, the code will just do nothing and fail silently, but if we modify the document, we'll send an update operation, therefore get informed about the DocumentNotFoundError.

Some options to consider:

Add a key in a document that is retrieved via findOneAnd(Remove/Delete) isDeletedFromDB, and

A) Call Model.create(document.toObject()); to re-create that document.

B) Throw an error on calling document.save() since the user may select some specific fields from that document, and re-creating with only the selected fields will create bugs that are hard to debug.

I am inclining to option B.

The code snippet below will probably achieve what you want, even though I recommend rethinking the logic of the application in the first place.

const mongoose = require('mongoose');
mongoose.connect('mongodb://127.0.0.1:27017/test', { family: 4, useCreateIndex: true, useFindAndModify: false, useNewUrlParser: true, useUnifiedTopology: true, poolSize: 10 });

const Person = mongoose.model('gh8371', { name: String });


async function run () {
  await Person.deleteMany();
  await Person.create({ name: 'Sam' });

  const removedPerson = await Person.findOneAndRemove({ name: 'Sam' });

  removedPerson.name = 'Hafez';

  await Person.create(removedPerson.toObject());

  console.log('Done.');
}

run().catch(console.error);

[LeonYanghaha]

I tried to run the codedocument.isNew = true,but there's still no data，code like this。

     let acl = await Acl.findOneAndRemove({name: "nodejs"});
    acl.isNew = true;
    let newAcl = await acl.save();
    console.log(newAcl); 

I think: fails silently is unreasonable.... the logic of the code is to save the data (ignore the contradiction of the code logic), and there is no error message after execution. so i think that the data is saved successfully. but the reality is that there is no data.... this is understandable to senior users.. but it's confusing for junior users.

[AbdelrahmanHafez]

While it's not clear to me why you would want to remove a document, then save it right after. I agree that you should be informed that the document is not saved in some way.

Waiting for @vkarpov15 's take on this issue.

[AbdelrahmanHafez]

@vkarpov15 I find this an interesting challenge. Would be more than happy to help implement a fix if you let me know what you have on mind regarding this issue.

[vkarpov15]

@AbdelrahmanHafez at the very least, setting isNew to true should make it so that save() tries to insert a new document, so this comment is primarily what I want to fix.

I'm a bit less confident that findOneAndRemove() should set isNew to true automatically. That's a potential 2nd step, but the first step is making sure that setting isNew means save() tries to insert the whole doc.

[LeonYanghaha]

@AbdelrahmanHafez @vkarpov15 what can I do for mongoose? I'm willing to contribute to make mongoose better.

[AbdelrahmanHafez]

I just added a test verifying the behavior for setting isNew = true. Apparently, it does insert a new document into the database. The following test passes.

const assert = require('assert');
const mongoose = require('./index');
const { Schema } = mongoose;

const userSchema = new Schema({ name: String });
const User = mongoose.model('User', userSchema);

mongoose.connect('mongodb://127.0.0.1:27017/test', { useNewUrlParser: true, useUnifiedTopology: true });

async function run () {
  const createdUser = await User.create({ name: 'Hafez' });
  const removedUser = await User.findOneAndRemove({ _id: createdUser._id });
  removedUser.isNew = true;

  await removedUser.save();

  const foundUser = await User.findOne({ _id: removedUser._id });
  assert.ok(foundUser);
}

run().catch(console.error);

@LeonYanghaha this is what you're trying to achieve in #8371 (comment), right? If yes, and you're still having a problem with that, please try to provide a reproduction script.

@vkarpov15 I came up with an approach that I think will be cleaner than setting isNew on findOneAndRemove(...).

The current behavior is:

if (doc.isNew) {
 //insert
} else {
  const changes = this.getChanges();
  if (changes) {
    // update with changes
  }
}

which results in failing silently when no changes are present, this could be fixed by:

if (doc.isNew) {
 //insert
} else {
  const changes = this.getChanges();
  if (changes) {
    // update with changes
  } else {
    //check doc existence on db, if it's not found, throw an error, otherwise go on.
  }
}