Update validator for ObjectId fields does not parse nested objects

[letsgolesco]

Bug, mongoose 4.12.4, Node 8.7.0, MongoDB 3.4.9

Current Behavior
Update validator for ObjectId fields is not parsing $pull updates that include objects (example below).

Steps to reproduce

• Create a schema that includes a subdocument array
• Insert a document into that collection
• Attempt a $pull update with an $in operator (e.g. { $pull: { field: { _id: { $in: [] } } } })
• Validator rejects saying the JSON object { $in: [] } is not a valid ObjectId

Illustrated in code:

const mongoose = require('mongoose');
const exampleSchema = mongoose.Schema({
    subdocuments: [{
        name: String
    }]
});
const ExampleModel = mongoose.model('Example', exampleSchema);
const exampleDocument = {
    subdocuments: [{
        // _id field created implicitly by mongoose
        name: 'First'
    }, {
        name: 'Second'
    }]
};
ExampleModel.create(exampleDocument)
    .then(savedDocument => {
        ExampleModel.update({
            _id: savedDocument._id
        }, {
            $pull: {
                subdocuments: {
                    _id: {
                        $in: [] // could be an empty or could contain valid ObjectIds, error occurs either way
                    }
                }
            }
        }, {
            runValidators: true
        })
        .catch(error => {
            // CastError: Cast to ObjectID failed for value "{ '$in': [] }" at path "_id"
        });
    });

What is the expected behavior?

• Validator should check that the values inside the $in array are ObjectIds, not the whole object mapped to _id.

Stacktrace example:

CastError: Cast to ObjectID failed for value "{ '$in': [ 59dff36f566439001cd9bae3 ] }" at path "_id"
    at new CastError (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/error/cast.js:27:11)
    at EmbeddedDocument.Document.set (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/document.js:773:7)
    at EmbeddedDocument._handleIndex (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/document.js:602:14)
    at EmbeddedDocument.Document.set (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/document.js:562:24)
    at EmbeddedDocument.Document (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/document.js:77:12)
    at EmbeddedDocument [as constructor] (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/types/embedded.js:31:12)
    at new EmbeddedDocument (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/schema/documentarray.js:70:17)
    at /Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/schema/documentarray.js:173:26
    at DocumentArray.SchemaType.doValidate (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/schematype.js:766:12)
    at DocumentArray.doValidate (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/schema/documentarray.js:133:35)
    at /Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/lib/services/updateValidators.js:113:22
    at /Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/node_modules/async/internal/parallel.js:27:9
    at eachOfArrayLike (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/node_modules/async/eachOf.js:57:9)
    at exports.default (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/node_modules/async/eachOf.js:9:5)
    at _parallel (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/node_modules/async/internal/parallel.js:26:5)
    at parallelLimit (/Users/richard/Sites/tenthousandcoffees/api-server/node_modules/mongoose/node_modules/async/parallel.js:85:26)

[vkarpov15]

Thanks for the detailed repro, will investigate ASAP

[letsgolesco]

Found another bug that is related to the update validators, once again on a $pull update. It may be close enough to this bug that both can be resolved at once?

Steps to reproduce

• Schema includes subdocument array
• Some field of the subdocuments are required
• Attempt to $pull one of the subdocuments by _id

My suspicion is that the update validator is checking the object passed to $pull.subdocuments for all the required fields, and it's not finding them (because I specify only _id).

Illustrated in code:

const mongoose = require('mongoose');
const exampleSchema = mongoose.Schema({
    subdocuments: [{
        name: String,
        required: true
    }]
});
const ExampleModel = mongoose.model('Example', exampleSchema);
const exampleDocument = {
    subdocuments: [{
        name: 'First'
    }, {
        name: 'Second'
    }]
};
ExampleModel.create(exampleDocument)
    .then(savedDocument => {
        const firstSubDocumentId = savedDocument.subdocuments[0]._id;
        ExampleModel.findByIdAndUpdate(savedDocument._id, {
            $pull: {
                subdocuments: {
                    _id: firstSubDocumentId
                }
            }
        }, {
            runValidators: true
        })
        .catch(error => {
            // ValidationError: subdocuments: Validation failed: value: Path `name` is required.
        });
    });

What is the expected behavior?
$pull update is executed and subdocument with matching _id is removed

Stacktrace:

at ValidationError (/app/node_modules/mongoose/lib/error/validation.js:28)
at (/app/node_modules/mongoose/lib/services/updateValidators.js:148)
at (/app/node_modules/mongoose/node_modules/async/internal/parallel.js:35)
at (/app/node_modules/mongoose/node_modules/async/internal/once.js:12)
at iteratorCallback (/app/node_modules/mongoose/node_modules/async/eachOf.js:52)
at (/app/node_modules/mongoose/node_modules/async/internal/onlyOnce.js:12)
at (/app/node_modules/mongoose/node_modules/async/internal/parallel.js:32)
at apply (/app/node_modules/lodash/_apply.js:15)
at (/app/node_modules/lodash/_overRest.js:32)
at (/app/node_modules/mongoose/lib/services/updateValidators.js:118)
at callback (/app/node_modules/mongoose/lib/schema/documentarray.js:159)
at complete (/app/node_modules/mongoose/lib/document.js:1479)
at (/app/node_modules/mongoose/lib/document.js:1510)
at callback (/app/node_modules/mongoose/lib/schema/documentarray.js:159)
at complete (/app/node_modules/mongoose/lib/document.js:1479)
at (/app/node_modules/mongoose/lib/document.js:1510)

[vkarpov15]

Confirmed that both scripts work with 3b4211e. Thanks for reporting, fix will be in 4.12.5, which should be released tomorrow 👍

[letsgolesco]

Thanks for fixing this so quickly!