Skip to content

Makes Indexed Paths for Optional Nested Documents Sparse #5387

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Makes Indexed Paths for Optional Nested Documents Sparse #5387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
63df184
Makes indexed fields for optional subdocuments sparse.
Jun 21, 2017
62cb11e
Fixed stylistic issues.
Jun 21, 2017
8db6f4f
Added test: Sparse indexed nested documents ancestor chain.
Jun 21, 2017
89874eb
Fixed style issues.
Jun 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 66 additions & 45 deletions lib/schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,45 +1345,99 @@ Schema.prototype.indexes = function() {
var indexes = [];
var schemaStack = [];

var collectIndexes = function(schema, prefix) {
/*!
* Checks for indexes added to subdocs using Schema.index().
* These indexes need their paths prefixed properly.
*
* schema._indexes = [ [indexObj, options], [indexObj, options] ..]
*/
function fixSubIndexPaths(schema, prefix) {
var subindexes = schema._indexes,
len = subindexes.length,
indexObj,
newindex,
klen,
keys,
key,
subIndex = 0,
keyIndex;

for (subIndex = 0; subIndex < len; ++subIndex) {
indexObj = subindexes[subIndex][0];
keys = Object.keys(indexObj);
klen = keys.length;
newindex = {};

// use forward iteration, order matters
for (keyIndex = 0; keyIndex < klen; ++keyIndex) {
key = keys[keyIndex];
newindex[prefix + key] = indexObj[key];
}

indexes.push([newindex, subindexes[subIndex][1]]);
}
}

function collectIndexes(schema, prefix, optional) {
var keyIndex,
isObject,
options,
field,
index,
type,
path,
keys,
key;

// Ignore infinitely nested schemas, if we've already seen this schema
// along this path there must be a cycle
if (schemaStack.indexOf(schema) !== -1) {
return;
}

schemaStack.push(schema);

prefix = prefix || '';
var key, path, index, field, isObject, options, type;
var keys = Object.keys(schema.paths);
keys = Object.keys(schema.paths);

for (var i = 0; i < keys.length; ++i) {
key = keys[i];
for (keyIndex = 0; keyIndex < keys.length; ++keyIndex) {
key = keys[keyIndex];
path = schema.paths[key];

if ((path instanceof MongooseTypes.DocumentArray) || path.$isSingleNested) {
collectIndexes(path.schema, prefix + key + '.');
if (path instanceof MongooseTypes.DocumentArray || path.$isSingleNested) {
collectIndexes(path.schema, prefix + key + '.', !path.isRequired || optional);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem: isRequired is not necessarily known until you create a document, required: function() { return false; } is also supported.

} else {
index = path._index || (path.caster && path.caster._index);

if (index !== false && index !== null && index !== undefined) {
field = {};
isObject = utils.isObject(index);
options = isObject ? index : {};
type = typeof index === 'string' ? index :
isObject ? index.type :
false;

if (typeof index === 'string') {
type = index;
} else if (isObject) {
type = index.type;
} else {
type = false;
}

if (type && ~Schema.indexTypes.indexOf(type)) {
field[prefix + key] = type;
} else if (options.text) {
field[prefix + key] = 'text';

delete options.text;
} else {
field[prefix + key] = 1;
}

if (optional) {
options.sparse = true;
}

delete options.type;

if (!('background' in options)) {
options.background = true;
}
Expand All @@ -1403,46 +1457,13 @@ Schema.prototype.indexes = function() {
index[1].background = true;
}
});

indexes = indexes.concat(schema._indexes);
}
};
}

collectIndexes(this);
return indexes;

/*!
* Checks for indexes added to subdocs using Schema.index().
* These indexes need their paths prefixed properly.
*
* schema._indexes = [ [indexObj, options], [indexObj, options] ..]
*/

function fixSubIndexPaths(schema, prefix) {
var subindexes = schema._indexes,
len = subindexes.length,
indexObj,
newindex,
klen,
keys,
key,
i = 0,
j;

for (i = 0; i < len; ++i) {
indexObj = subindexes[i][0];
keys = Object.keys(indexObj);
klen = keys.length;
newindex = {};

// use forward iteration, order matters
for (j = 0; j < klen; ++j) {
key = keys[j];
newindex[prefix + key] = indexObj[key];
}

indexes.push([newindex, subindexes[i][1]]);
}
}
};

/**
Expand Down
118 changes: 117 additions & 1 deletion test/model.indexes.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,128 @@ describe('model', function() {
done();
});

it('made sparse for optional nested documents', function(done) {
var GrandchildSchema,
ChildSchema,
ParentSchema,
indexes;

GrandchildSchema = new Schema({
requiredNested2: {
type: String,
required: true,
unique: true
},
optionalNested2: String
});

ChildSchema = new Schema({
requiredNested1: {
type: GrandchildSchema,
required: true
},
optionalNested1: Number
});

ParentSchema = new Schema({
child: ChildSchema
});

indexes = ParentSchema.indexes();

assert.equal(indexes.length, 1);
assert.ok(indexes[0][1].sparse);

done();
});

it('made sparse when the parent schema marks it as optional but the grandparent schema does not mark the parent as optional', function(done) {
var GrandchildSchema,
ChildSchema,
ParentSchema,
indexes;

GrandchildSchema = new Schema({
requiredNested2: {
type: String,
required: true,
unique: true
},
optionalNested2: String
});

ChildSchema = new Schema({
requiredNested1: GrandchildSchema,
optionalNested1: Number
});

ParentSchema = new Schema({
child: ChildSchema
});

ParentSchema.index({child: 1});

indexes = ParentSchema.indexes();

assert.equal(indexes.length, 2);

if (indexes[0][0]['child.requiredNested1.requiredNested2']) {
assert.ok(indexes[0][1].sparse);
} else if (indexes[1][0]['child.requiredNested1.requiredNested2']) {
assert.ok(indexes[1][1].sparse);
} else {
done('requiredNested2 not indexed.');
}

done();
});

it('not made sparse for required nested documents', function(done) {
var GrandchildSchema,
ChildSchema,
ParentSchema,
indexes;

GrandchildSchema = new Schema({
requiredNested2: {
type: String,
required: true,
unique: true
},
optionalNested2: String
});

ChildSchema = new Schema({
requiredNested1: {
type: GrandchildSchema,
required: true
},
optionalNested1: Number
});

ParentSchema = new Schema({
child: {
type: ChildSchema,
required: true
}
});

indexes = ParentSchema.indexes();

assert.equal(indexes.length, 1);
assert.ok(!indexes[0][1].sparse);

done();
});

it('primitive arrays (gh-3347)', function(done) {
var indexes;
var schema = new Schema({
arr: [{ type: String, unique: true }]
});

var indexes = schema.indexes();
indexes = schema.indexes();

assert.equal(indexes.length, 1);
assert.deepEqual(indexes[0][0], { arr: 1 });
assert.ok(indexes[0][1].unique);
Expand Down