pre, post middleware are not executed on findByIdAndUpdate #964

Closed
skotchio opened this Issue Jun 15, 2012 · 101 comments

Comments

Projects
None yet
@skotchio

Because a findAndUpdate method is presented to cut down the following code:

 Model.findById(_id, function (err, doc) {
      if (doc) {
          doc.field = 'value';
          doc.save(function (err) {
                 // do something;
          });
      }
 });

to this:

Model
   .findByIdAndUpdate(_id, {$set: {field: 'value'}}, function (err, doc) {
        // do something 
    });

We need to use pre, post middleware exactly the same. At now pre, post middleware are not executed when I make findByIdAndUpdate.

@aheckmann

This comment has been minimized.

Show comment
Hide comment
@aheckmann

aheckmann Jun 15, 2012

Collaborator

by design. there are docs involved to call hooks on.

Collaborator

aheckmann commented Jun 15, 2012

by design. there are docs involved to call hooks on.

@aheckmann aheckmann closed this Jun 15, 2012

@aheckmann

This comment has been minimized.

Show comment
Hide comment
@aheckmann

aheckmann Jun 15, 2012

Collaborator

correction, there are no docs to call hooks on.

Collaborator

aheckmann commented Jun 15, 2012

correction, there are no docs to call hooks on.

@skotchio

This comment has been minimized.

Show comment
Hide comment
@skotchio

skotchio Jun 15, 2012

So? If I want to call pre, post middleware I need to use the first approach?

So? If I want to call pre, post middleware I need to use the first approach?

@aheckmann

This comment has been minimized.

Show comment
Hide comment
@aheckmann

aheckmann Jun 15, 2012

Collaborator

yes that is correct. Model.update,findByIdAndUpdate,findOneAndUpdate,findOneAndRemove,findByIdAndRemove are all commands executed directly in the database.

Collaborator

aheckmann commented Jun 15, 2012

yes that is correct. Model.update,findByIdAndUpdate,findOneAndUpdate,findOneAndRemove,findByIdAndRemove are all commands executed directly in the database.

@oori oori referenced this issue in jspears/mers Oct 9, 2012

Merged

Allow "remove" hooks #3

@jessefulton

This comment has been minimized.

Show comment
Hide comment
@jessefulton

jessefulton Nov 1, 2012

This should definitely be clarified in the guide, especially if you're talking about validation in that same page and describing findByIdAndUpdate as "better"

This should definitely be clarified in the guide, especially if you're talking about validation in that same page and describing findByIdAndUpdate as "better"

@aheckmann

This comment has been minimized.

Show comment
Hide comment
@aheckmann

aheckmann Nov 1, 2012

Collaborator

if you click on the link "better" it takes you to the full documentation of
the method where it explains validation etc.

feel free to send a pull request to add something you feel is better. its
pretty easy to do so:
https://github.com/LearnBoost/mongoose/blob/master/CONTRIBUTING.md

On Wed, Oct 31, 2012 at 6:03 PM, Jesse Fulton notifications@github.comwrote:

This should definitely be clarified in the guidehttp://mongoosejs.com/docs/documents.html,
especially if you're talking about validation in that same page and
describing findByIdAndUpdate as "better"


Reply to this email directly or view it on GitHubhttps://github.com/LearnBoost/mongoose/issues/964#issuecomment-9967865.

Aaron
@aaronheckmann https://twitter.com/#!/aaronheckmann

Collaborator

aheckmann commented Nov 1, 2012

if you click on the link "better" it takes you to the full documentation of
the method where it explains validation etc.

feel free to send a pull request to add something you feel is better. its
pretty easy to do so:
https://github.com/LearnBoost/mongoose/blob/master/CONTRIBUTING.md

On Wed, Oct 31, 2012 at 6:03 PM, Jesse Fulton notifications@github.comwrote:

This should definitely be clarified in the guidehttp://mongoosejs.com/docs/documents.html,
especially if you're talking about validation in that same page and
describing findByIdAndUpdate as "better"


Reply to this email directly or view it on GitHubhttps://github.com/LearnBoost/mongoose/issues/964#issuecomment-9967865.

Aaron
@aaronheckmann https://twitter.com/#!/aaronheckmann

@sstadelman

This comment has been minimized.

Show comment
Hide comment
@sstadelman

sstadelman Oct 25, 2013

Contributor

Added notes to the middleware doc

Pull request: LearnBoost#1750

Contributor

sstadelman commented Oct 25, 2013

Added notes to the middleware doc

Pull request: LearnBoost#1750

@lefnire lefnire referenced this issue in HabitRPG/habitica Oct 31, 2013

Closed

Participant count never increases #1676

@nooysters nooysters referenced this issue in linnovate/mean May 31, 2014

Closed

update array of objectId best practice. #538

@dunnkers dunnkers referenced this issue in drudge/mongoose-timestamp Jun 4, 2014

Closed

updatedAt not updated when i do some updated to document #13

@albanm

This comment has been minimized.

Show comment
Hide comment
@albanm

albanm Jun 10, 2014

Hello,

I know the issue is closed, but I am not entirely satisfied with the answer. Shouldn't there be at least a post-save or post-update middleware for findOneAndUpdate and other similar operations ? Is seems ok since the document is returned. Not feasible for pre middlewares or for Model.update, i agree.

It would greatly improve the capabilities of plugins such as mongoosastic that is currently blind to some operations it should be able to support.

If no middleware, does someone have an idea on how to manage some post update operations in a plugin ?

Thanks

albanm commented Jun 10, 2014

Hello,

I know the issue is closed, but I am not entirely satisfied with the answer. Shouldn't there be at least a post-save or post-update middleware for findOneAndUpdate and other similar operations ? Is seems ok since the document is returned. Not feasible for pre middlewares or for Model.update, i agree.

It would greatly improve the capabilities of plugins such as mongoosastic that is currently blind to some operations it should be able to support.

If no middleware, does someone have an idea on how to manage some post update operations in a plugin ?

Thanks

@vkarpov15 vkarpov15 added this to the 4.0 milestone Jun 10, 2014

@jessefulton

This comment has been minimized.

Show comment
Hide comment
@jessefulton

jessefulton Jun 20, 2014

@albanm certain methods bypass mongoose completely, so you don't get the middleware hooks. AFAIK, the only way to get the hooks to execute is to use separate find() and save() calls as mentioned above.

@albanm certain methods bypass mongoose completely, so you don't get the middleware hooks. AFAIK, the only way to get the hooks to execute is to use separate find() and save() calls as mentioned above.

@albanm

This comment has been minimized.

Show comment
Hide comment
@albanm

albanm Jun 20, 2014

I get that and it makes sense. Pre middlewares are out of question as there is no fetching prior to certain methods. But still, mongoose should be able to wrap the update operations that also return the updated documents and trigger some post hooks.

albanm commented Jun 20, 2014

I get that and it makes sense. Pre middlewares are out of question as there is no fetching prior to certain methods. But still, mongoose should be able to wrap the update operations that also return the updated documents and trigger some post hooks.

@beetaa

This comment has been minimized.

Show comment
Hide comment
@beetaa

beetaa Jul 2, 2014

I think @albanm is right, People may want the same functionality when they use the same function. How about wrap those 'directly update' methods with some interception of check that if there's any hooks exist? If hook exist, use it, or call original update methods otherwise.

beetaa commented Jul 2, 2014

I think @albanm is right, People may want the same functionality when they use the same function. How about wrap those 'directly update' methods with some interception of check that if there's any hooks exist? If hook exist, use it, or call original update methods otherwise.

@joafeldmann

This comment has been minimized.

Show comment
Hide comment

+1

@sunfuze

This comment has been minimized.

Show comment
Hide comment

sunfuze commented Jul 25, 2014

+1

@perok

This comment has been minimized.

Show comment
Hide comment

perok commented Jul 29, 2014

+1

@perok perok referenced this issue in nicholasconfer/mongoose-times Jul 29, 2014

Closed

Does not work with findOneAndUpdate and upsert #4

@ludwiksh

This comment has been minimized.

Show comment
Hide comment

+1

@cebor

This comment has been minimized.

Show comment
Hide comment

cebor commented Aug 20, 2014

👍

@Zertz Zertz referenced this issue in florianholzapfel/express-restify-mongoose Aug 21, 2014

Closed

Add ability to use mongoose middleware #62

@syzer

This comment has been minimized.

Show comment
Hide comment

syzer commented Aug 21, 2014

+1

@askhogan

This comment has been minimized.

Show comment
Hide comment
@askhogan

askhogan Aug 29, 2014

👎
I get the feature request, but the longer I work with middleware, I find myself needing to bypass middleware often when calling db update scripts and other items that are not routine to my normal operations. By using static methods it becomes very easy to create a custom wrapper that implements the above feature requests already. Since mongoose is now opening to ideas for version 4, it is unlikely that any API change of this magnitude will happen in v3. I request that this be moved to v4 discussion.

I would however, 👍 if I was able to disable middleware on a save call. I often find myself with a mongoose object that was passed from another function and simply want to perform a .save - in this situation, doing a .save is preferable to writing a new query. If this is possible, please point it out

Either way, amazing library. Kudos to awesome maintainers. Not sure how I would operate without it.

👎
I get the feature request, but the longer I work with middleware, I find myself needing to bypass middleware often when calling db update scripts and other items that are not routine to my normal operations. By using static methods it becomes very easy to create a custom wrapper that implements the above feature requests already. Since mongoose is now opening to ideas for version 4, it is unlikely that any API change of this magnitude will happen in v3. I request that this be moved to v4 discussion.

I would however, 👍 if I was able to disable middleware on a save call. I often find myself with a mongoose object that was passed from another function and simply want to perform a .save - in this situation, doing a .save is preferable to writing a new query. If this is possible, please point it out

Either way, amazing library. Kudos to awesome maintainers. Not sure how I would operate without it.

@roymap

This comment has been minimized.

Show comment
Hide comment

roymap commented Sep 18, 2014

+1

@sailxjx

This comment has been minimized.

Show comment
Hide comment
@sailxjx

sailxjx Sep 27, 2014

A simple way to add hooks for these method is overwrite the embed functions:

_update = UserModel.update
UserModel.update = (conditions, update) ->
  update.updatedAt or= new Date
  _update.apply this, arguments

Then every update call of mongoose will fix the updatedAt key of data.

You may give a try of this model limbo. It is a simple wrapper of mongoose model, supporting bind static/method/overwrite to all the schemas, and call rpc methods to query the mongodb.

sailxjx commented Sep 27, 2014

A simple way to add hooks for these method is overwrite the embed functions:

_update = UserModel.update
UserModel.update = (conditions, update) ->
  update.updatedAt or= new Date
  _update.apply this, arguments

Then every update call of mongoose will fix the updatedAt key of data.

You may give a try of this model limbo. It is a simple wrapper of mongoose model, supporting bind static/method/overwrite to all the schemas, and call rpc methods to query the mongodb.

@SnidelyWhiplash SnidelyWhiplash referenced this issue in achingbrain/mongoose-crate Oct 28, 2014

Closed

Delete is not removing files from s3 bucket. #4

@asinha08

This comment has been minimized.

Show comment
Hide comment
@asinha08

asinha08 Dec 28, 2014

+1, need this feature...

+1, need this feature...

@joerx

This comment has been minimized.

Show comment
Hide comment
@joerx

joerx Dec 30, 2014

While I sort of understand the reasoning, the lack of hooks on atomic updates IMHO makes Mongoose somewhat pointless in practice. When I use atomic updates any validations, defaults, etc. are not executed, so the entire purpose of using an ODM is defeated. Using find/save will do the job, but is there any guarantee this is always used?

Moreover, usually I would try to avoid find/save since it's not an atomic operation. MongoDB compensates it's lack of transaction support by providing powerful atomic query & update features. So I would use these atomic operations but w/o middleware support Mongoose won't provide much value over the native MongoClient.

Even the examples in http://aaronheckmann.tumblr.com/post/48943525537/mongoose-v3-part-1-versioning would use update, hence bypass middleware. I can use versioning properly or middleware but not combine both? Really, where's the point in having it?

I don't even fully understand the technical reasons: If update & co. wrap around the database operations, why can't we intercept the call and pass the query objects so we can do some validation/customization before we actually do the update?

joerx commented Dec 30, 2014

While I sort of understand the reasoning, the lack of hooks on atomic updates IMHO makes Mongoose somewhat pointless in practice. When I use atomic updates any validations, defaults, etc. are not executed, so the entire purpose of using an ODM is defeated. Using find/save will do the job, but is there any guarantee this is always used?

Moreover, usually I would try to avoid find/save since it's not an atomic operation. MongoDB compensates it's lack of transaction support by providing powerful atomic query & update features. So I would use these atomic operations but w/o middleware support Mongoose won't provide much value over the native MongoClient.

Even the examples in http://aaronheckmann.tumblr.com/post/48943525537/mongoose-v3-part-1-versioning would use update, hence bypass middleware. I can use versioning properly or middleware but not combine both? Really, where's the point in having it?

I don't even fully understand the technical reasons: If update & co. wrap around the database operations, why can't we intercept the call and pass the query objects so we can do some validation/customization before we actually do the update?

@syzer

This comment has been minimized.

Show comment
Hide comment
@syzer

syzer Dec 30, 2014

@joerx +1 would suffice.. :) but your reasoning is flawless.

syzer commented Dec 30, 2014

@joerx +1 would suffice.. :) but your reasoning is flawless.

@vkarpov15 vkarpov15 modified the milestones: 4.0.0-rc0, 4.0 Jan 2, 2015

@vkarpov15 vkarpov15 reopened this Jan 2, 2015

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Jan 2, 2015

Collaborator

The 3.9.x branch has support for pre and post hooks for find and findOne - should be easy to add support for findOneAndUpdate and update.

Collaborator

vkarpov15 commented Jan 2, 2015

The 3.9.x branch has support for pre and post hooks for find and findOne - should be easy to add support for findOneAndUpdate and update.

vkarpov15 added a commit that referenced this issue Jan 4, 2015

vkarpov15 added a commit that referenced this issue Jan 4, 2015

@willemmulder

This comment has been minimized.

Show comment
Hide comment
@willemmulder

willemmulder Nov 7, 2015

Thanks for the suggestion. Just did that:

I run this:

schema.pre('update', function(next) {
    this.update({}, { password: bcrypt.hashSync(this.getUpdate().$set.password) } );
    console.log(this.getUpdate());
    next();
});

which returns this for the console.log

{ '$set':
   { password: '$2a$10$I1oXet30Cl5RUcVMxm3GEOeTFOLFmPWaQvXbr6Z5368zbfpA8nFEK',
     postalAddress: { street: '', houseNumber: '', zipCode: '', city: '', country: '' },
     permissions: [ '' ],
     __v: 0,
     lastName: '',
     firstName: '',
     email: 'test@test.nl',
     _id: 563b0410bd07ce2030eda26d } }

and then this for the Mongoose debug

Mongoose: users.update({ _id: ObjectId("563b0410bd07ce2030eda26d") }) { '$set': { password: 'test', postalAddress: { street: '', houseNumber: '', zipCode: '', city: '', country: '' }, permissions: [ '\u001b[32m\'\'\u001b[39m' ], __v: 0, lastName: '', firstName: '', email: 'test@test.nl', _id: ObjectId("563b0410bd07ce2030eda26d") } } { overwrite: false, strict: true, multi: false, upsert: false, safe: true }
Mongoose: users.findOne({ _id: ObjectId("563b0410bd07ce2030eda26d") }) { fields: { password: 0 } }

Any clue?

Thanks for the suggestion. Just did that:

I run this:

schema.pre('update', function(next) {
    this.update({}, { password: bcrypt.hashSync(this.getUpdate().$set.password) } );
    console.log(this.getUpdate());
    next();
});

which returns this for the console.log

{ '$set':
   { password: '$2a$10$I1oXet30Cl5RUcVMxm3GEOeTFOLFmPWaQvXbr6Z5368zbfpA8nFEK',
     postalAddress: { street: '', houseNumber: '', zipCode: '', city: '', country: '' },
     permissions: [ '' ],
     __v: 0,
     lastName: '',
     firstName: '',
     email: 'test@test.nl',
     _id: 563b0410bd07ce2030eda26d } }

and then this for the Mongoose debug

Mongoose: users.update({ _id: ObjectId("563b0410bd07ce2030eda26d") }) { '$set': { password: 'test', postalAddress: { street: '', houseNumber: '', zipCode: '', city: '', country: '' }, permissions: [ '\u001b[32m\'\'\u001b[39m' ], __v: 0, lastName: '', firstName: '', email: 'test@test.nl', _id: ObjectId("563b0410bd07ce2030eda26d") } } { overwrite: false, strict: true, multi: false, upsert: false, safe: true }
Mongoose: users.findOne({ _id: ObjectId("563b0410bd07ce2030eda26d") }) { fields: { password: 0 } }

Any clue?

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Nov 7, 2015

Collaborator

Not sure, opened up a new issue to track.

Collaborator

vkarpov15 commented Nov 7, 2015

Not sure, opened up a new issue to track.

@willemmulder

This comment has been minimized.

Show comment
Hide comment
@willemmulder

willemmulder Nov 9, 2015

@vkarpov15 Thanks, will track the other issue.

@vkarpov15 Thanks, will track the other issue.

@akoskm

This comment has been minimized.

Show comment
Hide comment
@akoskm

akoskm Apr 1, 2016

@vkarpov15 I think the right way to set options for the ongoing query in pre hook would be something like:

  finishSchema.pre('findOneAndUpdate', function (next) {
    this.setOptions({
      new: true,
      runValidators: true
    });
    this.update({}, {
      lastEdited: Date.now()
    });
    next();
  });

but the documentation, http://mongoosejs.com/docs/api.html#query_Query-setOptions doesn't mention any of those options. If this considered to be a hackish solutions, what would be more appropriate?

akoskm commented Apr 1, 2016

@vkarpov15 I think the right way to set options for the ongoing query in pre hook would be something like:

  finishSchema.pre('findOneAndUpdate', function (next) {
    this.setOptions({
      new: true,
      runValidators: true
    });
    this.update({}, {
      lastEdited: Date.now()
    });
    next();
  });

but the documentation, http://mongoosejs.com/docs/api.html#query_Query-setOptions doesn't mention any of those options. If this considered to be a hackish solutions, what would be more appropriate?

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Apr 3, 2016

Collaborator

That's an issue with the docs, that code you describe looks like it should work at first glance

Collaborator

vkarpov15 commented Apr 3, 2016

That's an issue with the docs, that code you describe looks like it should work at first glance

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Apr 3, 2016

Collaborator

Can you open up a separate issue for that?

Collaborator

vkarpov15 commented Apr 3, 2016

Can you open up a separate issue for that?

@akoskm

This comment has been minimized.

Show comment
Hide comment
@akoskm

akoskm Apr 3, 2016

@vkarpov15 Yes, it does work. I think I wasn't clear enough.

setOptions applies new and runValidators correctly, I was merely asking if setting those options through setOptions should be preferred over this.options.

akoskm commented Apr 3, 2016

@vkarpov15 Yes, it does work. I think I wasn't clear enough.

setOptions applies new and runValidators correctly, I was merely asking if setting those options through setOptions should be preferred over this.options.

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Apr 4, 2016

Collaborator

setOptions() is preferable IMO, but both should work. Or you could just do

this.update({}, { lastEdited: Date.now() }, { new: true, runValidators: true });
Collaborator

vkarpov15 commented Apr 4, 2016

setOptions() is preferable IMO, but both should work. Or you could just do

this.update({}, { lastEdited: Date.now() }, { new: true, runValidators: true });
@nlonguit

This comment has been minimized.

Show comment
Hide comment
@nlonguit

nlonguit Apr 6, 2016

schema.pre('update', function(next) {
this.update({}, { $set : { password: bcrypt.hashSync(this.getUpdate().$set.password) } });
next();
});

This will update password on every update() call. So if i just change the value of other properties, i.e. name or age, then password will be updated also which is not correct!?

nlonguit commented Apr 6, 2016

schema.pre('update', function(next) {
this.update({}, { $set : { password: bcrypt.hashSync(this.getUpdate().$set.password) } });
next();
});

This will update password on every update() call. So if i just change the value of other properties, i.e. name or age, then password will be updated also which is not correct!?

@akoskm

This comment has been minimized.

Show comment
Hide comment
@akoskm

akoskm Apr 6, 2016

@nlonguit I guess it will. But you can access through this the fields that are going to be updated and you could do something like:

if (this._fields.password) { // <- I'm sure about this one, check in debugger the properties of this 
    this.update({}, { $set : { password: bcrypt.hashSync(this.getUpdate().$set.password) } });
}

akoskm commented Apr 6, 2016

@nlonguit I guess it will. But you can access through this the fields that are going to be updated and you could do something like:

if (this._fields.password) { // <- I'm sure about this one, check in debugger the properties of this 
    this.update({}, { $set : { password: bcrypt.hashSync(this.getUpdate().$set.password) } });
}
@nlonguit

This comment has been minimized.

Show comment
Hide comment
@nlonguit

nlonguit Apr 6, 2016

if (this._update.$set.password) { this.update({}, { $set: { password: bcrypt.hashSync(this.getUpdate().$set.password)} }); }

This code is working well for me. Thanks @akoskm

nlonguit commented Apr 6, 2016

if (this._update.$set.password) { this.update({}, { $set: { password: bcrypt.hashSync(this.getUpdate().$set.password)} }); }

This code is working well for me. Thanks @akoskm

@mickyginger

This comment has been minimized.

Show comment
Hide comment
@mickyginger

mickyginger Apr 6, 2016

I wonder if it would be possible to add a pre hook for findByIdAndUpdate as well. Would be nice to have both hooks available.

I wonder if it would be possible to add a pre hook for findByIdAndUpdate as well. Would be nice to have both hooks available.

@keegho

This comment has been minimized.

Show comment
Hide comment
@keegho

keegho Jun 4, 2016

I did it this way and it is working: Just findById then save without updating any fields then use the findByIdAndUpdate method:

dbModel.findById(barId, function (err, bar) {
        if (bar) {

            bar.save(function (err) {
                if (err) throw err;
            });
        }
    });
    dbModel.findByIdAndUpdate(barId, {$set:req.body}, function (err, bar) {
        if (err) throw err;
        res.send('Updated');
    });`

keegho commented Jun 4, 2016

I did it this way and it is working: Just findById then save without updating any fields then use the findByIdAndUpdate method:

dbModel.findById(barId, function (err, bar) {
        if (bar) {

            bar.save(function (err) {
                if (err) throw err;
            });
        }
    });
    dbModel.findByIdAndUpdate(barId, {$set:req.body}, function (err, bar) {
        if (err) throw err;
        res.send('Updated');
    });`
@zilions

This comment has been minimized.

Show comment
Hide comment
@zilions

zilions Jun 9, 2016

I'm trying to set a property to have the length of an array.

schema.post('findOneAndUpdate', function(result) {
    console.log(result.comments.length);
    this.findOneAndUpdate({}, { totalNumberOfComments: result.comments.length });
});

The correct length is logged, although the query never sets totalNumberOfComments, and the field stays at 0 (since the schema references default: 0).

When I console.log(this) at the end of the hook, I can see that my query contains the following:

_update: { '$push': { comments: [Object] }, totalNumberOfComments: 27 }

Although when I turn on debug mode, a query is never logged by Mongoose.

Is there something I'm doing wrong, or is this a bug?

zilions commented Jun 9, 2016

I'm trying to set a property to have the length of an array.

schema.post('findOneAndUpdate', function(result) {
    console.log(result.comments.length);
    this.findOneAndUpdate({}, { totalNumberOfComments: result.comments.length });
});

The correct length is logged, although the query never sets totalNumberOfComments, and the field stays at 0 (since the schema references default: 0).

When I console.log(this) at the end of the hook, I can see that my query contains the following:

_update: { '$push': { comments: [Object] }, totalNumberOfComments: 27 }

Although when I turn on debug mode, a query is never logged by Mongoose.

Is there something I'm doing wrong, or is this a bug?

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Jun 11, 2016

Collaborator

@zilions this.findOneAndUpdate({}, { totalNumberOfComments: result.comments.length }).exec(); need to actually execute the query :) Just be careful, you're gonna get an infinite recursion there because your post save hook will trigger another post save hook

Collaborator

vkarpov15 commented Jun 11, 2016

@zilions this.findOneAndUpdate({}, { totalNumberOfComments: result.comments.length }).exec(); need to actually execute the query :) Just be careful, you're gonna get an infinite recursion there because your post save hook will trigger another post save hook

@zilions

This comment has been minimized.

Show comment
Hide comment
@zilions

zilions Jun 11, 2016

@vkarpov15 Ahhhh right! Then I can just use this.update({} {....}).exec() instead :)
Question though, when using this, it sets the totalNumberOfComments field perfectly, although performs the original update of the findOneAndUpdate too.

For example:

Post.findOneAndUpdate({_id: fj394hri3hfj}, {$push: {comments: myNewComment}})

Will trigger the following hook:

schema.post('findOneAndUpdate', function(result) {
    this.update({}, {
        totalNumberOfComments: result.comments.length
    }).exec();
}));

Although, the hook will $push to comments the myNewComment again, therefore making a duplicate entry.

zilions commented Jun 11, 2016

@vkarpov15 Ahhhh right! Then I can just use this.update({} {....}).exec() instead :)
Question though, when using this, it sets the totalNumberOfComments field perfectly, although performs the original update of the findOneAndUpdate too.

For example:

Post.findOneAndUpdate({_id: fj394hri3hfj}, {$push: {comments: myNewComment}})

Will trigger the following hook:

schema.post('findOneAndUpdate', function(result) {
    this.update({}, {
        totalNumberOfComments: result.comments.length
    }).exec();
}));

Although, the hook will $push to comments the myNewComment again, therefore making a duplicate entry.

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Jun 14, 2016

Collaborator

Because you're technically executing the same query -

schema.post('findOneAndUpdate', function(result) {
    this.update({}, {
        totalNumberOfComments: result.comments.length
    }).exec();
}));

is essentially the same as

var query = Post.findOneAndUpdate({_id: fj394hri3hfj}, {$push: {comments: myNewComment}});
query.update({}, {
        totalNumberOfComments: result.comments.length
    }).exec();
query.findOneAndUpdate().exec();

If you want to create a new query from scratch, just do

schema.post('findOneAndUpdate', function(result) {
    this.model.update({}, { // <--- `this.model` gives you access to the `Post` model
        totalNumberOfComments: result.comments.length
    }).exec();
}));
Collaborator

vkarpov15 commented Jun 14, 2016

Because you're technically executing the same query -

schema.post('findOneAndUpdate', function(result) {
    this.update({}, {
        totalNumberOfComments: result.comments.length
    }).exec();
}));

is essentially the same as

var query = Post.findOneAndUpdate({_id: fj394hri3hfj}, {$push: {comments: myNewComment}});
query.update({}, {
        totalNumberOfComments: result.comments.length
    }).exec();
query.findOneAndUpdate().exec();

If you want to create a new query from scratch, just do

schema.post('findOneAndUpdate', function(result) {
    this.model.update({}, { // <--- `this.model` gives you access to the `Post` model
        totalNumberOfComments: result.comments.length
    }).exec();
}));
@moparlakci

This comment has been minimized.

Show comment
Hide comment
@moparlakci

moparlakci Sep 13, 2016

just dont touch your pre save hook,

router.put('/:id', jsonParser, function(req, res, next) {

  currentCollection.findByIdAndUpdate(req.params.id, req.body, function (err, item) {
    if (err) {
        res.status(404);
        return res.json({'error': 'Server Error', 'trace': err});
    }
    item.save(); // <=== this is were you save your data again which triggers the pre hook :)
    res.status(200); 
    return res.json({'message': 'Saved successfully'});
  });
});

moparlakci commented Sep 13, 2016

just dont touch your pre save hook,

router.put('/:id', jsonParser, function(req, res, next) {

  currentCollection.findByIdAndUpdate(req.params.id, req.body, function (err, item) {
    if (err) {
        res.status(404);
        return res.json({'error': 'Server Error', 'trace': err});
    }
    item.save(); // <=== this is were you save your data again which triggers the pre hook :)
    res.status(200); 
    return res.json({'message': 'Saved successfully'});
  });
});
@nicky-lenaers

This comment has been minimized.

Show comment
Hide comment
@nicky-lenaers

nicky-lenaers Nov 28, 2016

I found that the order matters in which you define a model and define a pre hook. Allow me to demonstrate:

Does not work:

// Create Model
let model = Database.Connection.model(`UserModel`, this._schema, `users`);

// Attach Pre Hook
this._schema.pre(`findOneAndUpdate`, function(next) {
    console.log('pre update');
    return next();
});

Does work:

// Attach Pre Hook
this._schema.pre(`findOneAndUpdate`, function(next) {
    console.log('pre update');
    return next();
});

// Create Model
let model = Database.Connection.model(`UserModel`, this._schema, `users`);

Hope this helps anyone!

I found that the order matters in which you define a model and define a pre hook. Allow me to demonstrate:

Does not work:

// Create Model
let model = Database.Connection.model(`UserModel`, this._schema, `users`);

// Attach Pre Hook
this._schema.pre(`findOneAndUpdate`, function(next) {
    console.log('pre update');
    return next();
});

Does work:

// Attach Pre Hook
this._schema.pre(`findOneAndUpdate`, function(next) {
    console.log('pre update');
    return next();
});

// Create Model
let model = Database.Connection.model(`UserModel`, this._schema, `users`);

Hope this helps anyone!

@albert-92

This comment has been minimized.

Show comment
Hide comment
@albert-92

albert-92 Nov 30, 2016

I just found out the same thing as @nicky-lenaers.

It works just fine with 'safe'. 'delete'. etc. if you define the hooks after the model is defined.

Is there a workaround do define a 'findOneAndUpdate' hook after the model is defined?

I just found out the same thing as @nicky-lenaers.

It works just fine with 'safe'. 'delete'. etc. if you define the hooks after the model is defined.

Is there a workaround do define a 'findOneAndUpdate' hook after the model is defined?

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
@vkarpov15

vkarpov15 Dec 3, 2016

Collaborator

@albert-92 no not at the moment

Collaborator

vkarpov15 commented Dec 3, 2016

@albert-92 no not at the moment

@marcusjwhelan

This comment has been minimized.

Show comment
Hide comment
@marcusjwhelan

marcusjwhelan Dec 8, 2016

For anyone trying to get something that used to be like

SCHEMA.pre('validate', function(done) {
    // and here use something like 
    this.yourNestedElement 
    // to change a value or maybe create a hashed character    
    done();
});

This should work

SCHEMA.pre('findOneAndUpdate', function(done){
    this._update.yourNestedElement
    done();
});

For anyone trying to get something that used to be like

SCHEMA.pre('validate', function(done) {
    // and here use something like 
    this.yourNestedElement 
    // to change a value or maybe create a hashed character    
    done();
});

This should work

SCHEMA.pre('findOneAndUpdate', function(done){
    this._update.yourNestedElement
    done();
});
@johndeyrup

This comment has been minimized.

Show comment
Hide comment
@johndeyrup

johndeyrup Dec 21, 2016

I can't get post hooks to update the document in the collection.

`module.exports = function (mongoose) {
var mySchema = mongoose.Schema({
id: { type: Number, index: { unique: true } },
field1: { type: String },
field2: { type: String}
}, {
collection: "mySchema",
versionKey: false
});

mySchema.post('findOneAndUpdate', function (result) {
    this.model.update({}, {
        field2: 'New Value'
    }).exec();
});
return mySchema;

}`

mySchema.findOneAndUpdate({id: 1}, {field1: 'test'}, {new: true});

Sets field in collection to { id:1, field1: 'test' ) but should be {id: 1, field1: 'test', field2:'New Value'}
Not sure what I am doing wrong

I can change change the result of the findOneAndUpdate by doing this
mySchema.post('findOneAndUpdate', function (result) { result.field2 = 'something' });

johndeyrup commented Dec 21, 2016

I can't get post hooks to update the document in the collection.

`module.exports = function (mongoose) {
var mySchema = mongoose.Schema({
id: { type: Number, index: { unique: true } },
field1: { type: String },
field2: { type: String}
}, {
collection: "mySchema",
versionKey: false
});

mySchema.post('findOneAndUpdate', function (result) {
    this.model.update({}, {
        field2: 'New Value'
    }).exec();
});
return mySchema;

}`

mySchema.findOneAndUpdate({id: 1}, {field1: 'test'}, {new: true});

Sets field in collection to { id:1, field1: 'test' ) but should be {id: 1, field1: 'test', field2:'New Value'}
Not sure what I am doing wrong

I can change change the result of the findOneAndUpdate by doing this
mySchema.post('findOneAndUpdate', function (result) { result.field2 = 'something' });

@marcusjwhelan

This comment has been minimized.

Show comment
Hide comment
@marcusjwhelan

marcusjwhelan Dec 21, 2016

I think it might be that you are trying to update the model with an element that already exists on the model. Or possibly that you are selecting it wrong. Try printing out "this" in your mySchema.post. Also you don't seem to have a done() or next() in your post. I'm not very knowledgable on the subject but I know printing out this will at least give you an idea of what you are dealing with.

I think it might be that you are trying to update the model with an element that already exists on the model. Or possibly that you are selecting it wrong. Try printing out "this" in your mySchema.post. Also you don't seem to have a done() or next() in your post. I'm not very knowledgable on the subject but I know printing out this will at least give you an idea of what you are dealing with.

@johndeyrup

This comment has been minimized.

Show comment
Hide comment
@johndeyrup

johndeyrup Dec 21, 2016

Isn't the point of update to change an existing document in your model?

this is a query object

You don't need done or next in post hooks as far as I understand.

Isn't the point of update to change an existing document in your model?

this is a query object

You don't need done or next in post hooks as far as I understand.

@marcusjwhelan

This comment has been minimized.

Show comment
Hide comment
@marcusjwhelan

marcusjwhelan Dec 21, 2016

Well you have this.model.update which is the schema not the model of the object. I think.. Which means you would have to use

mySchema.post('findOneAndUpdate', function (result) {
    this.model.update({}, {
        $set: { field2: 'New Value'}
    }).exec();
});
return mySchema;

This seems a little backwards to be calling a model function inside of the model. Since you could just use parts of the "this" object that is given to you. you might be better off just using the findOneAndUpdate instead of calling it and then calling another model function on top of it.
in a manager

var data = yourNewData;
self.findOneAndUpdate({id: something._id}, data, {safe: false, new: true})
    .exec()
    .then(resolve)
    .catch(reject);

In my example above I used this._update because that was the update object that needed to be used off of this.

marcusjwhelan commented Dec 21, 2016

Well you have this.model.update which is the schema not the model of the object. I think.. Which means you would have to use

mySchema.post('findOneAndUpdate', function (result) {
    this.model.update({}, {
        $set: { field2: 'New Value'}
    }).exec();
});
return mySchema;

This seems a little backwards to be calling a model function inside of the model. Since you could just use parts of the "this" object that is given to you. you might be better off just using the findOneAndUpdate instead of calling it and then calling another model function on top of it.
in a manager

var data = yourNewData;
self.findOneAndUpdate({id: something._id}, data, {safe: false, new: true})
    .exec()
    .then(resolve)
    .catch(reject);

In my example above I used this._update because that was the update object that needed to be used off of this.

@johndeyrup

This comment has been minimized.

Show comment
Hide comment
@johndeyrup

johndeyrup Dec 21, 2016

I have tried using $set. It still doesn't change my document in the collection.

I have tried using $set. It still doesn't change my document in the collection.

jhhayashi added a commit to jhhayashi/coupon-api that referenced this issue Jan 11, 2017

Hash pw in POST /admins
Mongoose findOneAndUpdate bypasses hooks
Automattic/mongoose#964

@guoyunhe guoyunhe referenced this issue in talha-asad/mongoose-url-slugs Apr 17, 2017

Closed

update property dont update slug field #25

@lwhiteley lwhiteley referenced this issue in BiteBit/koa-oai-mongoose May 18, 2017

Closed

mongoose hooks not being fired for most endpoints #16

@jcyh0120

This comment has been minimized.

Show comment
Hide comment
@jcyh0120

jcyh0120 Sep 7, 2017

Where can I find the all available pre and post hooks?

jcyh0120 commented Sep 7, 2017

Where can I find the all available pre and post hooks?

@vkarpov15

This comment has been minimized.

Show comment
Hide comment
Collaborator

vkarpov15 commented Sep 16, 2017

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