mongoose icon indicating copy to clipboard operation
mongoose copied to clipboard

[Feature] option to disable running pre/post hooks

Open AbdelrahmanHafez opened this issue 4 years ago • 11 comments

I am not sure whether this is present or not, but it would be great if we have a way to optionally disable running middlewares, a couple of use cases that I could think of is for internal use in the library, this would help us avoid situations like #8739.

Other developers could also use this in their applications if they need to run a query without triggering middlewares, or inside the logic of the middleware in itself to avoid an infinite loop such as the custom option in https://github.com/Automattic/mongoose/issues/8756#issuecomment-607605315.

Considering the following API:

User.findOne({ name: 'John' }, null, { middlewares: false })

Later on, we could make it so that the options accept an object

{ middlewares: { pre: true, post: false } }

AbdelrahmanHafez avatar Apr 04 '20 05:04 AbdelrahmanHafez

I like this idea, definitely something to consider for a future minor release. Right now the only easy workaround is to just use User.collection.findOne(), but then that bypasses all of Mongoose.

I think I'd call the option middleware though, not middlewares.

vkarpov15 avatar Apr 10 '20 20:04 vkarpov15

@vkarpov15 I've been trying to implement this, but I couldn't find where the middleware-execution-related code is.

I see that we're defining pre/post hooks using Kareem, and in schema.s.hooks. Could you point me in the right direction?

AbdelrahmanHafez avatar Apr 19 '20 01:04 AbdelrahmanHafez

@AbdelrahmanHafez Kareem's createWrapper() function is probably where these changes should end up. Mongoose calls that function to wrap Mongoose's built-in functions with middleware for documents and queries

vkarpov15 avatar Apr 21 '20 17:04 vkarpov15

@AbdelrahmanHafez did you make any progress on this one? It could be a very useful feature. In our case, we have a bunch of scripts written, mostly for data migration, etc where we want the pre/post hooks to be disabled by default. I was thinking of a global setting like mongoose.set('disableMiddlewares', true) with false by default. Can we consider that?

ahmedshuhel avatar Mar 02 '21 13:03 ahmedshuhel

We have found a workaround for this by adding disableMiddlewares field in the query options and then check for it in the middleware itself

 let options = {
      new: true,
      upsert: true,
      setDefaultsOnInsert: true,
      rawResult: true,
      disableMiddlewares: true, // can be checked in middleware with this.options.disableMiddlewares
    }; 

  if (!this.options.disableMiddlewares) this.find({ active: { $ne: false } });
  next();
});

madaher-dev avatar Oct 10 '22 12:10 madaher-dev

@madaher-dev I've been using something similar to your code to pass custom query options to the pre('find') query middleware and it has been working nicely. Basically I saved the custom query option on app.locals and read it in the pre hook and obviously needed to reset it on app.locals back to undefined on every new request... so your solution is more elegant and robust.

I have another issue though (and that's how I have found this issue): I would need to pass a custom query option to the pre('save') hook, which is a document middleware (not query middleware), so I'm wondering if it's at all possible to do that on the document itself. To be clear, this points to the document being saved in the pre('save') hook, instead of the query.

lorand-horvath avatar Jun 10 '23 15:06 lorand-horvath

this.options would not work on this case. Note that save pre middleware works save() and .create() but if you want to skip that why not use findOneAndUpdate with upsert option to skip the pre save middleware?

madaher-dev avatar Jun 11 '23 07:06 madaher-dev

findOneAndUpdate() wouldn't work in my case for a couple of reasons that are related to the specific way I have set up the app. However, I have found a much better method of disabling pre('save') hooks: https://mongoosejs.com/docs/api/model.html#Model.bulkWrite()

const ops = docs.map(document => ({insertOne: {document}}));
await TheModel.bulkWrite(ops);

This function does not trigger any middleware, neither save(), nor update()

Also, wanted to mention for the pre('find') query middleware, wouldn't it be better to use this.getOptions() as described here https://mongoosejs.com/docs/api/query.html#Query.prototype.getOptions() ? I have to check whether it returns custom options as well, or we must access them directly as you did above with this.options.disableMiddlewares.

Edit: It works as imagined: await query.setOptions({disablePreHook: true}) in the controller and then in the pre('find') hook: if (this.getOptions().disablePreHook) return next();

lorand-horvath avatar Jun 11 '23 07:06 lorand-horvath

Re: this.options vs this.getOptions() for query middleware, getOptions() just returns this.options so they're equivalent. Whichever you prefer.

Re: getting save options in pre('save'), below is an example of how you can access that in middleware:

'use strict';

const mongoose = require('mongoose');

run().catch(err => console.log(err));

async function run() {
  await mongoose.connect('mongodb://127.0.0.1:27017/mongoose_test');

  const schema = mongoose.Schema({
    name: String
  });
  schema.pre('save', function(next, opts) {
    console.log('SaveOptions:', opts); // 'SaveOptions: SaveOptions { myopt: true }'
  });
  const Test = mongoose.model('Test', schema);
  const doc = new Test({ name: 'bar' });
  await doc.save({ myopt: true });
  console.log('Done');
  process.exit(0);
}

vkarpov15 avatar Jun 12 '23 01:06 vkarpov15

@vkarpov15 Huh, that's nice! I haven't seen this approach documented in https://mongoosejs.com/docs/middleware.html Passing options as the second argument to the pre('save') callback is very handy, I could have thought about that by looking at the pre('init', pojo => {...}) or the post('find', function(result) {...}) examples in the docs.

I assume I can pass custom options from Model.create(docs, options) to the underlying doc.save() and access them in pre('save', function(next, options) {...}). Edit: indeed, it works properly.

lorand-horvath avatar Jun 12 '23 06:06 lorand-horvath

@lorand-horvath we will add that to the docs

vkarpov15 avatar Jun 12 '23 13:06 vkarpov15