Nested schema hooks doesn't run in some case

[Iworb]

Mongoose version: 5.2.1
In findByIdAndUpdate case nested hook doesn't run.

const subSchema = new Schema({
  name: {
    type: String,
    required: true
  }
});

subSchema.pre('findByIdAndUpdate', function (next) { console.log('I\'m not going to run'); return next(); });

const schema = new Schema({
  field: subSchema
});

schema.pre('findByIdAndUpdate', function (next) { console.log('It\'s ok'); return next(); });

[lineus]

@Iworb The idea of calling pre update query hooks on nested schemas was brought up a while back in a comment on #964, and #3125 but it doesn't appear to have been implemented.

Can you elaborate on why you want to define the hook on the nested schema instead of the top level one?

Also, findByIdAndUpdate() triggers the findOneAndUpdate middleware.

[Iworb]

Okay, @lineus, I want to be sure that subSchema document exists, so I'm using to check it in both pre-validate and pre-findByIdAndUpdate middlewares. Pre-validate case works as expected, but findByIdAndUpdate with {runValidators: true, context: 'query'} doesn't run nested validation (subSchema.pre('validate')).
P.S. findOneAndUpdate doesn't run aswell.

[Iworb]

Here's a complicated example:

var itemSchema = function (path) {
  const s = new Schema({
    kind: {
      type: String,
      required: true,
      validate: [
        function (v) {
          return mongoose.modelNames().includes(v)
        },
        'Not Found'
      ]
    },
    item: {
      type: ObjectId,
      refPath: path + '.kind', // <-- (1)
      required: true
    }
  })
  
  const validate = async function (doc, next) {
    let Model
    try {
      Model = mongoose.model(doc.kind)
    } catch (err) {
      return next(false)
    }
    const found = await Model.findById(doc.item)
    return found ? next() : next(false)
  }
  
  s.post('validate', validate)
  s.pre('findOneAndUpdate', async function (next) {
    return validate(this.getUpdate()['$set'][path], next)
  })
  
  return s
}

var parentSchema = new Schema({
  name: String,
  connections: [itemSchema('connections')],
  users: [itemSchema('users')],
  agents: [itemSchema('agents')],
});

const validate = async function (doc, next) {
  // some parent document validation
  return next()
}

parentSchema.post('validate', validate)
parentSchema.pre('findOneAndUpdate', async function (next) {
  return validate(this.getUpdate()['$set'], next)
})

1 - another question: is there a way to retrieve itemSchema directly without wrapping it with path property for dynamic references?

[lineus]

I'm going to break this into 2 separate answers as there are 2 questions here.

Reusing a schema for multiple schemaPaths with dynamic refs

With mongoose as it exists today, your solution of returning the schema from a function with the refPath set based on the parameter of that function is as good a solution as any that I can think of. I do think it would be cool if refPath could accept a function that gets the doc and the current path as it's parameters. I've tested this out and have created PR #6683. With my proposed change the following example would make what you're doing easier.

6669_complete.js

#!/usr/bin/env node
'use strict';

const assert = require('assert');
const mongoose = require('mongoose');
const opts = { useNewUrlParser: true };
mongoose.connect('mongodb://localhost:27017/gh-6669', opts);
const conn = mongoose.connection;
const Schema = mongoose.Schema;

const connectionSchema = new Schema({
  destination: String
});

const Conn = mongoose.model('connection', connectionSchema);

const userSchema = new Schema({
  name: String
});

const User = mongoose.model('user', userSchema);

const agentSchema = new Schema({
  vendor: String
});

const Agent = mongoose.model('agent', agentSchema);

const subSchema = new Schema({
  kind: {
    type: String,
    required: true,
    validate: {
      validator: function(v) {
        return mongoose.modelNames().includes(v);
      },
      message: 'KindError: {VALUE} Collection: -1'
    }
  },
  item: {
    type: Schema.Types.ObjectId,
    refPath: function(doc, path) {
      return path.replace(/\.item$/,'.kind');
    },
    required: true
  }
});

const recordSchema = new Schema({
  name: String,
  connections: [subSchema],
  users: [subSchema],
  agents: [subSchema]
});

const Record = mongoose.model('record', recordSchema);

const connection = new Conn({ destination: '192.168.1.15' });
const user = new User({ name: 'Kev' });
const agent = new Agent({ vendor: 'chrome' });
const record = new Record({
  connections: [{ kind: 'connection', item: connection._id}],
  users: [{ kind: 'user', item: user._id }],
  agents: [{ kind: 'agent', item: agent._id }]
});

async function run() {
  await conn.dropDatabase();
  await connection.save();
  await user.save();
  await agent.save();
  await record.save();

  let doc = await Record.findOne()
    .populate('connections.item')
    .populate('users.item')
    .populate('agents.item');
  assert.strictEqual(doc.connections[0].item.destination, '192.168.1.15');
  assert.strictEqual(doc.users[0].item.name, 'Kev');
  assert.strictEqual(doc.agents[0].item.vendor, 'chrome');
  return conn.close();
}

run();

Output:

issues: ./6669_complete.js
issues:

Defining the findOneAndUpdate pre-hook on the sub schema

In your example pre-hook you are accessing the path variable from the wrapper function like you did in your refPath definition. I'm not sure how this would change even if we did add the ability to call the pre hook on the subSchema. You can however call the pre-hook on the parentSchema and loop through the update paths, validating each one. Here is an updated version of the same code as before but with the addition of the pre-hook:

6669_complete.js

#!/usr/bin/env node
'use strict';

const assert = require('assert');
const mongoose = require('mongoose');
const opts = { useNewUrlParser: true };
mongoose.connect('mongodb://localhost:27017/gh-6669', opts);
const conn = mongoose.connection;
const Schema = mongoose.Schema;

const connectionSchema = new Schema({
  destination: String
});

const Conn = mongoose.model('connection', connectionSchema);

const userSchema = new Schema({
  name: String
});

const User = mongoose.model('user', userSchema);

const agentSchema = new Schema({
  vendor: String
});

const Agent = mongoose.model('agent', agentSchema);

const subSchema = new Schema({
  kind: {
    type: String,
    required: true,
    validate: {
      validator: function(v) {
        return mongoose.modelNames().includes(v);
      },
      message: 'KindError: {VALUE} Collection: -1'
    }
  },
  item: {
    type: Schema.Types.ObjectId,
    refPath: function(doc, path) {
      return path.replace(/\.item$/,'.kind');
    },
    required: true
  }
});

const recordSchema = new Schema({
  name: String,
  connections: [subSchema],
  users: [subSchema],
  agents: [subSchema]
});

recordSchema.pre('findOneAndUpdate', function() {
  let update = this.getUpdate();
  let promises = [];
  for (let path of Object.keys(update)) {
    update[path].forEach((p) => {
      promises.push(validate(p));
    });
  }
  return Promise.all(promises);
});

const validate = function (doc) {
  let Model;
  try {
    Model = mongoose.model(doc.kind);
    return Model.findById(doc.item);
  } catch (err) {
    return Promise.reject(err);
  }
};

const Record = mongoose.model('record', recordSchema);

const conn1 = new Conn({ destination: '192.168.1.15' });
const conn2 = new Conn({ destination: '192.168.1.28' });
const user1 = new User({ name: 'Kev' });
const user2 = new User({ name: 'lineus' });
const agent1 = new Agent({ vendor: 'chrome' });
const agent2 = new Agent({ vendor: 'safari' });
const record = new Record({
  connections: [{ kind: 'connection', item: conn1._id}],
  users: [{ kind: 'user', item: user1._id }],
  agents: [{ kind: 'agent', item: agent1._id }]
});

async function run() {
  await conn.dropDatabase();
  await User.create([user1, user2]);
  await Agent.create([agent1, agent2]);
  await Conn.create([conn1, conn2]);
  await record.save();

  let doc = await Record.findOne()
    .populate('connections.item')
    .populate('users.item')
    .populate('agents.item');

  assert.strictEqual(doc.connections[0].item.destination, '192.168.1.15');
  assert.strictEqual(doc.users[0].item.name, 'Kev');
  assert.strictEqual(doc.agents[0].item.vendor, 'chrome');

  let update = {
    connections: [{
      kind: 'connection',
      item: conn2._id
    }],
    users: [{
      kind: process.argv[2],
      item: user2._id
    }],
    agents: [{
      kind: 'agent',
      item: agent2._id
    }]
  };
  let opts = { new: true };
  let updated = await Record.findByIdAndUpdate(record._id, update, opts);
  console.log(updated);
  return conn.close();
}

run().catch(console.error).then(() => { return conn.close();});

Output of running with a good collection name

issues: ./6669_complete.js user
{ _id: 5b3f59206657e01a32fbba2c,
  connections:
   [ { _id: 5b3f59206657e01a32fbba30,
       kind: 'connection',
       item: 5b3f59206657e01a32fbba27 } ],
  users:
   [ { _id: 5b3f59206657e01a32fbba31,
       kind: 'user',
       item: 5b3f59206657e01a32fbba29 } ],
  agents:
   [ { _id: 5b3f59206657e01a32fbba32,
       kind: 'agent',
       item: 5b3f59206657e01a32fbba2b } ],
  __v: 0 }
issues:

Output of running with a bad collection name

issues: ./6669_complete.js fail
{ MissingSchemaError: Schema hasn't been registered for model "fail".
Use mongoose.model(name, schema)
    at new MissingSchemaError (/Users/lineus/dev/opc/mongoose/lib/error/missingSchema.js:20:11)
    at Mongoose.model (/Users/lineus/dev/opc/mongoose/lib/index.js:358:13)
    at validate (/Users/lineus/dev/Help/mongoose5/issues/6669_complete.js:70:22)
    at update.(anonymous function).forEach (/Users/lineus/dev/Help/mongoose5/issues/6669_complete.js:61:21)
    at Array.forEach (<anonymous>)
    at model.Query.<anonymous> (/Users/lineus/dev/Help/mongoose5/issues/6669_complete.js:60:18)
    at next (/Users/lineus/dev/opc/mongoose/node_modules/kareem/index.js:63:31)
    at Kareem.execPre (/Users/lineus/dev/opc/mongoose/node_modules/kareem/index.js:86:8)
    at Kareem.wrap (/Users/lineus/dev/opc/mongoose/node_modules/kareem/index.js:265:8)
    at model.Query._findOneAndUpdate (/Users/lineus/dev/opc/mongoose/node_modules/kareem/index.js:339:11)
    at model.Query.Query.findOneAndUpdate (/Users/lineus/dev/opc/mongoose/lib/query.js:2335:8)
    at utils.promiseOrCallback (/Users/lineus/dev/opc/mongoose/lib/query.js:3328:19)
    at Promise (/Users/lineus/dev/opc/mongoose/lib/utils.js:233:5)
    at new Promise (<anonymous>)
    at Object.promiseOrCallback (/Users/lineus/dev/opc/mongoose/lib/utils.js:232:10)
    at model.Query.exec (/Users/lineus/dev/opc/mongoose/lib/query.js:3322:16)
  message: 'Schema hasn\'t been registered for model "fail".\nUse mongoose.model(name, schema)',
  name: 'MissingSchemaError' }
issues:

[Iworb]

I agree with 1st part of your answer, really great job, I'll be waiting for this PR acception.
About 2nd part there's some things to improve.

recordSchema.pre('findOneAndUpdate', function() {
  let update = this.getUpdate();
  let promises = [];
  for (let path of Object.keys(update)) {
    update[path].forEach((p) => {
      promises.push(validate(p)); // (1)
    });
  }
  return Promise.all(promises);
});

const validate = function (doc) {
  let Model;
  try {
    Model = mongoose.model(doc.kind); // (2)
    return Model.findById(doc.item); // (3)
  } catch (err) {
    return Promise.reject(err);
  }
};

(1) - bad idea. We could update recor's name or any other kind of field and this will throw exception about bad model name.
We should at least check of kind property existance at (2). another improvement is return Model.count({_id: doc.item}) in (3).

[lineus]

I wasn't trying to write all of your code for you 😄, just the relevant part to your question. Hopefully I didn't do anything too silly in my PR. Your reuse of the subSchema is neat, I'm glad you brought this up!

[vkarpov15]

I'm going to close this one for now since #6683 is in. Running query middleware on subschemas is a tricky API to design because 1) what context do they run in? top-level query? and 2) do they run if the subdoc isn't in the query? How about if a nested path is in the query? I'm down to re-open if someone has a solid suggestion for how this API would work.