2

I am having a problem in findOneAndUpdate in mongoose. The case is that i am updating a document by finding it. The query is as follows:

UserModel.findOneAndUpdate({
individualId: 'some id'
}, {
$push: {
supporterOf: 'some string'
}
})

The 'supporterOf' is the ref of UserModel and its type is 'ObjectId'.

The issue i am facing here is that, 'some string' is being pushed twice under 'supporterOf' in the document.

Can anyone tell me that how to push an array element inside the document?

  • 1
    Your query looks fine, I believe your problem lies somewhere else in your code. Can you please post the rest of your endpoint? Did you try manually fetching the document, pushing the string to the array and saving it using .save()? – BenSower Mar 11 '19 at 14:34
  • No, i have not tried the manual fetching and then update .save() because i want the transaction in one go. I want to find and update the result, its updating but inserting the duplicate item. I have debug the code as well, i am receiving single value from the end point. Can you please tell me that will the time of transaction be same when i (manually fetch and push) and when i (use findOneAndUpdate)? – Syed Faizan Ahmed Mar 12 '19 at 11:04
  • It will be slower, but unless you are planning to update many (e.g. 1000+) docs at the same time, or have very low hardware restrictions, you should be able to neglect these differences. Still, I'd suggest you try to further debug this problem, as I believe there could be an underlying issue that might cause further problems down the line. Therefor, can you please post a proof of concept of your problem? – BenSower Mar 13 '19 at 10:26
  • Yes @BenSower you are right, it will be slower but believe me its happening right now that findOneAndUpdate inserting duplicate entry in Pushed items. In order to reoccur the issue, kindly create a schema in which you have an empty array. Then run findOneAndUpdate and push the items just like in above post, you will get the same problem of duplicate entries. – Syed Faizan Ahmed Mar 20 '19 at 11:32
  • I created this gist https://gist.github.com/BenSower/9800a21c2ae4202d81a46fc64bc55b9e, which calls findOneAndUpdate twice and only pushes one string each. What mongoose version are you using? – BenSower Mar 21 '19 at 13:39

10 Answers10

6

I was having same problem, solution is.

I was keeping await like below.

 **await** schema.findOneAndUpdate(queryParms, {
                "$push": {
                    "array1": arrayDetails,
                    "array2": array2Details
                }
            }, {
                "upsert": true,
                "new": true
            },
            function (error, updateResponse) {
                if (error) {
                    throw new Error (error);
                } else {
                    // do something with updateResponse;
                }
            });

simply removing await helped me resolving this problem. Need to find the root cause. any pointer for references are welcome.

nitin1416
  • 181
  • 2
  • 13
  • 5
    "Mixing promises and callbacks can lead to duplicate entries in arrays", directly from https://mongoosejs.com/docs/queries.html – Sga Mar 31 '21 at 10:27
  • 1
    Of course. You can't use `await` _and_ a callback function together, these are two different styles of dealing with asynchronism. Use _either_ a callback function, _OR_ `Model.findOneAndUpdate().then(...)` _OR_ `await Model.findOneAndUpdate();`. Don't mix different syntax styles together. – Jeremy Thille Mar 03 '22 at 10:48
  • agree, learned and implemented same thing. – nitin1416 Mar 04 '22 at 12:50
3

I have recently encountered the same problem. However, I managed to overcome this issue by some other logics (details given below) but couldn't understand the reason behind that why findOneAndUpdate inserting duplicate entries in mongodb.

You can overcome this problem by following logic.

Use findOne or findById instead of findOneAndUpdate to search the document in your collection and then manually update your document and run save().

You can have better idea with this code snippet

return new Promise(function (resolve, reject) {
    Model.findOne({
            someCondition...
        }, function (err, item) {
            if (err) {
                reject(err);
            } else {
                item.someArray.push({
                    someKeyValue...
                });
                item.save().then((result) => {
                    resolve(result)
                }).catch((err) => {
                    reject(err)
                });
            }
        }).catch((err) => {
            reject(err)
        });
   });

This will not insert duplicate item. However, if you come to know the reasoning behind duplication, must do update this thread.

Asad Ullah Khalid
  • 140
  • 1
  • 3
  • 13
  • 1
    @Faizy You know that mongoose also directly returns a promise (unless you use a rather old version), so you don't have to wrap it in another promise once more? – BenSower Mar 13 '19 at 10:27
  • @BenSower Basically Asad Ullah has told me the workaround of duplicate entries. I want to negate the duplicate entries from database, and right now findOneAndUpdate causing the problem of duplicate entries. – Syed Faizan Ahmed Mar 20 '19 at 11:24
  • 1
    Yes, but that doesn't change the fact that could improve your codestyle using easier to read functions ;-) – BenSower Mar 21 '19 at 11:48
2

The issue seems to stem from combining an await and a callback. I had the same issue until I realised I was using an (err, resp) callback and a .catch(...).

models[auxType].findOneAndUpdate(
    filter,
    updateObject,
    options,
    (err, resp)=>{
        if (err) {
            console.log("Update failed:",err)
            res.json(err)
        } else if (resp) {
            console.log("Update succeeded:",resp)
            res.json(resp)
        } else {
            console.log("No error or response returned by server")
        }
    })
    .catch((e)=>{console.log("Error saving Aux Edit:",e)}); // << THE PROBLEM WAS HERE!!

The problem resolved as soon as I removed the .catch(...) line.

From the mongoose documentation:

2

Use $addToSet instead of $push, it should solve the problem. I believe there is an issue with the data structure used in the creation of a mongoose 'Model'. As we know push is an array (which allows duplication) operation while addToSet may be a Set operation (Sets do not allow duplication).

APO
  • 51
  • 1
  • 4
1

The problem with the accepted answer is that it only solves the problem by wrapping it in an unnecessary additional promise, when the findOneAndUpdate() method already returns a promise. Additionally, it uses both promises AND callbacks, which is something you should almost never do.

Instead, I would take the following approach:

I generally like to keep my update query logic separate from other concerns for both readability and re-usability. so I would make a wrapper function kind of like:

const update = (id, updateObj) => {
    const options = {
      new: true,
      upsert: true
    }
    return model.findOneAndUpdate({_id: id}, {...updateObj}, options).exec()
}

This function could then be reused throughout my application, saving me from having to rewrite repetitive options setup or exec calls.

Then I would have some other function that is responsible for calling my query, passing values to it, and handling what comes back from it.

Something kind of like:

const makePush = async () => {
   try {
     const result = await update('someObjectId', {$push: {someField: value}});
     // do whatever you want to do with the updated document
   catch (e) {
     handleError(e)
    }
 }

No need to create unnecessary promises, no callback hell, no duplicate requests, and stronger adherence to single responsibility principles.

Charles Desbiens
  • 879
  • 6
  • 14
0

I was having the same problem. My code was:

const doc = await model.findOneAndUpdate(
{filter}, {update},
{new: true}, (err, item) =>  if(err) console.log(err) }
)
res.locals.doc = doc
next();

The thing is, for some reason this callback after the "new" option was creating a double entry. I removed the callback and it worked.

Sverissimo
  • 276
  • 4
  • 8
0

I had the same problem. I found a solution for me:

I used a callback and a promise (so using keyword "await") simultaneously.

Using a callback and a promise simultaneously will result in the query being executed twice. You should be using one or the other, but not both.

  options = {
    upsert: true  // creates the object if it doesn't exist. defaults to false.
  };
  await Company.findByIdAndUpdate(company._id,
    { $push: { employees: savedEmployees } },
    options,
    (err) => {
       if (err) {
          debug(err);
       }
    }
  ).exec();

to

  options = {
    upsert: true  // creates the object if it doesn't exist. defaults to false.
  };
  await Company.findByIdAndUpdate(company._id,
    { $push: { employees: savedEmployees } },
    options
  ).exec();
Lukas
  • 446
  • 3
  • 11
0
UserModel.findOneAndUpdate(
{ _id: id },
{ object }
)

Even if you use _id as a parameter don't forget to make the filter explicit by id

0

In my case, changing the async callback solved the problem.

changing this:

await schema.findOneAndUpdate(
    { queryData },
    { updateData },
    { upsert: true },
    (err) => {
      if (err) console.log(err); 
      else await asyncFunction();
    }
  );

To this:

await schema.findOneAndUpdate(
    { queryData },
    { updateData },
    { upsert: true },
    (err) => {
      if (err) console.log(err);
    }
  );
 if (success) await asyncFunction();
Vahid Kiani
  • 89
  • 1
  • 5
0

The $addToSet instead of $push allowed me to prevent duplicate entry in my mongoDb array field of User document like this.

const blockUserServiceFunc = async(req, res) => {

let filter = {
    _id : req.body.userId
}

let update = { $addToSet: { blockedUserIds:  req.body.blockUserId  } };

await User.findOneAndUpdate(filter, update, (err, user) => {
    if (err) {
        res.json({
            status: 501,
            success: false,
            message: messages.FAILURE.SWW
        });
    } else {

        res.json({
            status: 200,
            success: true,
            message: messages.SUCCESS.USER.BLOCKED,
            data: {
                'id': user._id,
                'firstName': user.firstName,
                'lastName': user.lastName,
                'email': user.email,
                'isActive': user.isActive,
                'isDeleted': user.isDeleted,
                'deletedAt': user.deletedAt,
                'mobileNo': user.mobileNo,
                'userName': user.userName,
                'dob': user.dob,
                'role': user.role,
                'reasonForDeleting': user.reasonForDeleting,
                'blockedUserIds': user.blockedUserIds,
                'accountType': user.accountType
            }
        });

    }
}
).catch(err => {
    res.json({
        status: 500,
        success: false,
        message: err
    });
});

} Mongoose output