Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] option to disable running pre/post hooks #8768

Open
AbdelrahmanHafez opened this issue Apr 4, 2020 · 11 comments
Open

[Feature] option to disable running pre/post hooks #8768

AbdelrahmanHafez opened this issue Apr 4, 2020 · 11 comments
Labels
new feature This change adds new functionality, like a new method or class

Comments

@AbdelrahmanHafez
Copy link
Collaborator

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 #8756 (comment).

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 } }
@vkarpov15
Copy link
Collaborator

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 vkarpov15 added this to the 5.x Unprioritized milestone Apr 10, 2020
@vkarpov15 vkarpov15 added the new feature This change adds new functionality, like a new method or class label Apr 10, 2020
@AbdelrahmanHafez
Copy link
Collaborator Author

@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?

@vkarpov15
Copy link
Collaborator

@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

@ahmedshuhel
Copy link

@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?

@madaher-dev
Copy link

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

@lorand-horvath
Copy link
Contributor

lorand-horvath commented Jun 10, 2023

@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.

@madaher-dev
Copy link

madaher-dev commented Jun 11, 2023

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?

@lorand-horvath
Copy link
Contributor

lorand-horvath commented Jun 11, 2023

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

@vkarpov15
Copy link
Collaborator

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

@lorand-horvath
Copy link
Contributor

lorand-horvath commented Jun 12, 2023

@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.

@vkarpov15
Copy link
Collaborator

@lorand-horvath we will add that to the docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

5 participants