Skip to content

Commit

Permalink
Merge pull request #13522 from Automattic/vkarpov15/gh-13512
Browse files Browse the repository at this point in the history
feat(query): delay converting documents into POJOs until query execution, allow querying subdocuments with defaults disabled
  • Loading branch information
vkarpov15 committed Jun 19, 2023
2 parents 7539535 + a73af19 commit dda72c6
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 18 deletions.
4 changes: 2 additions & 2 deletions lib/cast.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ const Types = require('./schema/index');
const cast$expr = require('./helpers/query/cast$expr');
const castTextSearch = require('./schema/operators/text');
const get = require('./helpers/get');
const getConstructorName = require('./helpers/getConstructorName');
const getSchemaDiscriminatorByValue = require('./helpers/discriminator/getSchemaDiscriminatorByValue');
const isOperator = require('./helpers/query/isOperator');
const util = require('util');
const isObject = require('./helpers/isObject');
const isMongooseObject = require('./helpers/isMongooseObject');
const utils = require('./utils');

const ALLOWED_GEOWITHIN_GEOJSON_TYPES = ['Polygon', 'MultiPolygon'];

Expand Down Expand Up @@ -291,7 +291,7 @@ module.exports = function cast(schema, obj, options, context) {
}
} else if (val == null) {
continue;
} else if (getConstructorName(val) === 'Object') {
} else if (utils.isPOJO(val)) {
any$conditionals = Object.keys(val).some(isOperator);

if (!any$conditionals) {
Expand Down
18 changes: 3 additions & 15 deletions lib/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminato
const hasDollarKeys = require('./helpers/query/hasDollarKeys');
const helpers = require('./queryhelpers');
const immediate = require('./helpers/immediate');
const internalToObjectOptions = require('./options').internalToObjectOptions;
const isExclusive = require('./helpers/projection/isExclusive');
const isInclusive = require('./helpers/projection/isInclusive');
const isPathSelectedInclusive = require('./helpers/projection/isPathSelectedInclusive');
Expand Down Expand Up @@ -2319,8 +2320,6 @@ Query.prototype.find = function(conditions) {

this.op = 'find';

conditions = utils.toObject(conditions);

if (mquery.canMerge(conditions)) {
this.merge(conditions);

Expand Down Expand Up @@ -2392,6 +2391,8 @@ Query.prototype.merge = function(source) {
utils.merge(this._conditions, { _id: source }, opts);

return this;
} else if (source && source.$__) {
source = source.toObject(internalToObjectOptions);
}

opts.omit = {};
Expand Down Expand Up @@ -2560,9 +2561,6 @@ Query.prototype.findOne = function(conditions, projection, options) {
this.op = 'findOne';
this._validateOp();

// make sure we don't send in the whole Document to merge()
conditions = utils.toObject(conditions);

if (options) {
this.setOptions(options);
}
Expand Down Expand Up @@ -2726,8 +2724,6 @@ Query.prototype.count = function(filter) {
this.op = 'count';
this._validateOp();

filter = utils.toObject(filter);

if (mquery.canMerge(filter)) {
this.merge(filter);
}
Expand Down Expand Up @@ -2822,8 +2818,6 @@ Query.prototype.countDocuments = function(conditions, options) {
this.op = 'countDocuments';
this._validateOp();

conditions = utils.toObject(conditions);

if (mquery.canMerge(conditions)) {
this.merge(conditions);
}
Expand Down Expand Up @@ -2886,7 +2880,6 @@ Query.prototype.distinct = function(field, conditions) {

this.op = 'distinct';
this._validateOp();
conditions = utils.toObject(conditions);

if (mquery.canMerge(conditions)) {
this.merge(conditions);
Expand Down Expand Up @@ -2981,8 +2974,6 @@ Query.prototype.deleteOne = function deleteOne(filter, options) {
this.op = 'deleteOne';
this.setOptions(options);

filter = utils.toObject(filter);

if (mquery.canMerge(filter)) {
this.merge(filter);

Expand Down Expand Up @@ -3058,8 +3049,6 @@ Query.prototype.deleteMany = function(filter, options) {
this.setOptions(options);
this.op = 'deleteMany';

filter = utils.toObject(filter);

if (mquery.canMerge(filter)) {
this.merge(filter);

Expand Down Expand Up @@ -4162,7 +4151,6 @@ function _update(query, op, filter, doc, options, callback) {
// make sure we don't send in the whole Document to merge()
query.op = op;
query._validateOp();
filter = utils.toObject(filter);
doc = doc || {};

// strict is an option used in the update checking, make sure it gets set
Expand Down
6 changes: 5 additions & 1 deletion lib/schema/SubdocumentPath.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,15 @@ SubdocumentPath.prototype.castForQuery = function($conditional, val, context, op
return val;
}

const Constructor = getConstructor(this.caster, val);
if (val instanceof Constructor) {
return val;
}

if (this.options.runSetters) {
val = this._applySetters(val, context);
}

const Constructor = getConstructor(this.caster, val);
const overrideStrict = options != null && options.strict != null ?
options.strict :
void 0;
Expand Down
30 changes: 30 additions & 0 deletions test/query.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4104,4 +4104,34 @@ describe('Query', function() {
await Error.find().sort('-');
}, { message: 'Invalid field "" passed to sort()' });
});
it('allows executing a find() with a subdocument with defaults disabled (gh-13512)', async function() {
const schema = mongoose.Schema({
title: String,
bookHolder: mongoose.Schema({
isReading: Boolean,
tags: [String]
})
});
const Test = db.model('Test', schema);

const BookHolder = schema.path('bookHolder').caster;

await Test.collection.insertOne({
title: 'test-defaults-disabled',
bookHolder: { isReading: true }
});

// Create a new BookHolder subdocument, skip applying defaults
// Otherwise, default `[]` for `tags` would cause this query to
// return no results.
const bookHolder = new BookHolder(
{ isReading: true },
null,
null,
{ defaults: false }
);
const doc = await Test.findOne({ bookHolder });
assert.ok(doc);
assert.equal(doc.title, 'test-defaults-disabled');
});
});

0 comments on commit dda72c6

Please sign in to comment.