Remove hook not triggered when removing documents

[SirPepe]

The pre/post hooks for remove are only triggered when using the document instance's remove() method, not when the model's methods are used:

var mongoose = require("mongoose");
var db = mongoose.createConnection('mongodb://localhost/experiment');
db.once('open', function(){

  testSchema = new mongoose.Schema({ title: String });
  testSchema.post('remove', function(removed){
    console.log('removed', removed.title);
  });
  var Test = db.model('Test', testSchema);

  // > "removed A"
  new Test({ title: 'A' }).save(function(err, created){
    created.remove();
  });

  // Nothing :(
  Test({ title: 'B' }).save(function(err, created){
    Test.remove({ title: 'B' }).exec();
  });

  // Nothing :(
  Test({ title: 'C' }).save(function(err, created){
    Test.find({ title: 'C' }).remove();
  });

});

[aheckmann]

this works as expected. its the same thing for Model.update. no middleware is called b/c middleware only execute on mongoose documents which are not involved when sending a remove command directly to MongoDB through the use of Model.remove().

I'll leaving this open for now until I fix the Model.remove docs to be inline with Model.update. Thanks for the heads up.

[SirPepe]

I see. Is there any way to hook into all remove operations?

[aheckmann]

not presently.

[SirPepe]

Ok. Thank you!

[aheckmann]

well you cooould if you monkey patched Model.remove but thats up to you :)

[intech]

Not working with findOneAndRemove

[jValdron]

Also doesn't seem to work when you call Model.remove(_id).exec();

[ramseydsilva]

This is how I got around the issue:

Post.findOneAndRemove({'id': data.post, 'user.uid': socket.handshake.user.id}, function(err, post) {
    post.remove();
});

models.js

postSchema.post('remove', function(doc) {
    console.log('removed');
});

// prints 'removed' once

The remove hook is not fired on findOneAndRemove, however, it works when I call remove in the callback

[JXBAR]

Ok but Why ? iam sorry i didnt get why the hook is executed on Model.remove() and not on Query.remove(). Now why should i use the hooks if they arent called everytime ?
what's the use ?

[JXBAR]

I mean : Should i use the hooks and avoid Query.remove or not use the hooks and always remove manually ?

[vkarpov15]

@Darksheep42 if you want to use hooks, use doc.remove(). The reason why things like MyModel.remove(); doesn't support hooks is that the removed document may not exist in memory, so you would first have to query mongodb for the document and then tell mongodb to remove it for hooks to work properly.

[gregcarlin]

Would it be possible to add support for Query.remove() hooks now with the query middleware in 4.0?

[vkarpov15]

Not right now - that's very tricky because there's already middleware for Model.remove()

[gregcarlin]

Would there be any issue other than the name conflict? Couldn't you just call the hook 'remove-query' or something instead of 'remove'?

[gregcarlin]

Or how about do what you seem to do with update - call the query hook even when the doc version of the function is used. Of course this would break backwards-compatibility, but it's an idea.

[vkarpov15]

Seems like a reasonable idea, but I'm hesitant to break API unless there's a really good reason to. Right now the remove hook operates on a document, so the context in the middleware is the document. For general Query.remove(), the document might not even be in memory. I suppose we could name it something else like 'remove-query, but that's nasty. What do you think about adding an extra options arg topre()andpost()`, something like:

schema.pre('remove', { query: true }, function() {
  // This is only attached as query middleware
});

schema.pre('remove', { doc: true }, function() {
  // This will not run on Query.remove(), only doc.remove()
});

[gregcarlin]

I suppose that could work. It's probably better than just using a different name and it doesn't break the API. So I guess if someone did

schema.pre('remove', { query: true, doc: true }, function() {

});

The hook would be called on both query and doc requests. I guess a user could do this if they didn't care about the context. If you do this, though, you'd probably want to also change the update hook to match this new style, and that could potentially break people's code.

[vkarpov15]

Yeah the tricky bit in that context is "what's the default?" In order to preserve API, we'd have to make it so that update is a query middleware by default, but remove is a document middleware by default. That will be very weird. I think we should just open up a separate issue for 5.0 and worry about it later :)

[gregcarlin]

How about, just for the next version or some version in the near future, we can just call the hook 'remove-query' or something? It doesn't break the API and AFAIK should be pretty easy to implement. We could really use this feature, as right now you can't capture 100% of remove requests via middleware and we'd much rather do this than enforce certain techniques in our codebase. In 5.0 we can do it the "right way" but I'd love to just see this quick addition before v5.

[vkarpov15]

Alternative approach: how about we just make a preQuery() and postQuery() function?

[gregcarlin]

So instead of writing

schema.pre('remove', function() {
    // What is this? A doc hook? A query hook? Idk man
});

you'd write

schema.preQuery('remove', function() {
    // This is definitely a query hook
});
schema.pre('remove', function() {
    // This must be a doc hook
});

So would pre be reserved for docs? Also I guess you could put this into a 4.x version without breaking API, but we'd probably want to remove the old pre style for query middleware in 5.0.

[vkarpov15]

Yeah preQuery() would be a good way to preserve the API until we can figure out a cleaner API. I would imagine preQuery() might be deprecated immediately though, because I'm not quite thrilled with the syntax.

[Gp2mv3]

This issue is still open, is it fixed ?
What about preQuery ?

[vkarpov15]

The issue of pre remove for documents vs queries is tracked in #3054, not implemented yet, PRs welcome.