mongoose: pre, post middleware are not executed on findByIdAndUpdate

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

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?

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.

👎
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 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!

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

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

TodoSchema.pre('findOneAndUpdate', function() {
  this.findOneAndUpdate({}, { password: hashPassword(this.getUpdate().$set.password) });
});

That should work @willemmulder.

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

@mickyginger we have hooks for findOneAndUpdate, and findByIdAndUpdate() just calls findOneAndUpdate() under the hood

Yeah I was confused and thought you were using update. The below script

var mongoose = require('mongoose');
mongoose.set('debug', true);
var util = require('util');

mongoose.connect('mongodb://localhost:27017/gh964');

var TodoSchema = new mongoose.Schema({
  name: {type: String, required: true},
  note: String,
  completed: {type: Boolean, default: false},
  updatedAt: {type: Date, default: Date.now},
  user: {
    type: mongoose.Schema.ObjectId,
    ref: 'Users'
  }
});
TodoSchema.pre('findOneAndUpdate', function() {
  this.findOneAndUpdate({}, { updatedAt: Date.now() });
});

var Todo = mongoose.model('Todo', TodoSchema);

Todo.findOneAndUpdate({}, { note: "1" }, function(err) {
  if (err) {
    console.log(err);
  }
  console.log('Done');
  process.exit(0);
});

Works correctly and executes the desired query:

Mongoose: todos.findAndModify({}) [] { '$set': { note: '1', updatedAt: new Date("Thu, 07 May 2015 20:36:39 GMT") } } { new: false, upsert: false }
Done

So please use

TodoSchema.pre('findOneAndUpdate', function() {
  this.findOneAndUpdate({}, { updatedAt: Date.now() });
});

Instead of manually manipulating mquery’s internal state - that’s usually a bad idea unless you really really know what you’re doing.

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');
    });`

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

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.

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?

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();
}));

@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

@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) } });
}

Is this working or not ? Only hook that works for me is ‘save’, rest is completely ignored. I’m running on 4.0.6. Thanks

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.