Skip to content

Commit

Permalink
feat(model): make ensureIndexes() fail if specifying an index on _id
Browse files Browse the repository at this point in the history
Fix #6605
Re: #5820
Re: #5882
  • Loading branch information
vkarpov15 committed Jul 2, 2018
1 parent 93d06a4 commit 985a845
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 48 deletions.
30 changes: 19 additions & 11 deletions lib/model.js
Expand Up @@ -968,18 +968,16 @@ Model.init = function init(callback) {
const Promise = PromiseProvider.get();
const autoIndex = this.schema.options.autoIndex;
this.$init = new Promise((resolve, reject) => {
process.nextTick(() => {
if (autoIndex || (autoIndex == null && this.db.config.autoIndex)) {
this.ensureIndexes({ _automatic: true }, function(error) {
if (error) {
return reject(error);
}
resolve(this);
});
} else {
if (autoIndex || (autoIndex == null && this.db.config.autoIndex)) {
this.ensureIndexes({ _automatic: true }, function(error) {
if (error) {
return reject(error);
}
resolve(this);
}
});
});
} else {
resolve(this);
}
});

if (callback) {
Expand Down Expand Up @@ -1195,6 +1193,7 @@ Model.createIndexes = function createIndexes(options, callback) {

function _ensureIndexes(model, options, callback) {
const indexes = model.schema.indexes();

options = options || {};

const done = function(err) {
Expand All @@ -1205,6 +1204,15 @@ function _ensureIndexes(model, options, callback) {
callback && callback(err);
};

for (const index of indexes) {
if (index[0]._id != null) {
const err = new Error('Cannot specify a custom index on `_id`, ' +
'MongoDB does not allow overwriting the default `_id` index. See ' +
'http://bit.ly/mongodb-id-index');
return done(err);
}
}

if (!indexes.length) {
utils.immediate(function() {
done();
Expand Down
49 changes: 24 additions & 25 deletions lib/schema.js
Expand Up @@ -4,16 +4,17 @@
* Module dependencies.
*/

var readPref = require('./drivers').ReadPreference;
var EventEmitter = require('events').EventEmitter;
var VirtualType = require('./virtualtype');
var utils = require('./utils');
var MongooseTypes;
var Kareem = require('kareem');
var SchemaType = require('./schematype');
var get = require('lodash.get');
var getIndexes = require('./helpers/schema/getIndexes');
var mpath = require('mpath');
const EventEmitter = require('events').EventEmitter;
const Kareem = require('kareem');
const SchemaType = require('./schematype');
const VirtualType = require('./virtualtype');
const get = require('lodash.get');
const getIndexes = require('./helpers/schema/getIndexes');
const mpath = require('mpath');
const readPref = require('./drivers').ReadPreference;
const utils = require('./utils');

let MongooseTypes;

/**
* Schema constructor.
Expand Down Expand Up @@ -509,38 +510,36 @@ Schema.prototype.path = function(path, obj) {
branch[last] = utils.clone(obj);

this.paths[path] = Schema.interpretAsType(path, obj, this.options);
const schemaType = this.paths[path];

if (this.paths[path].$isSchemaMap) {
if (schemaType.$isSchemaMap) {
// Maps can have arbitrary keys, so `$*` is internal shorthand for "any key"
// The '$' is to imply this path should never be stored in MongoDB so we
// can easily build a regexp out of this path, and '*' to imply "any key."
const mapPath = path + '.$*';
this.paths[path + '.$*'] = Schema.interpretAsType(mapPath,
obj.of || { type: {} }, this.options);
this.paths[path].$__schemaType = this.paths[path + '.$*'];
schemaType.$__schemaType = this.paths[path + '.$*'];
}

if (this.paths[path].$isSingleNested) {
for (let key in this.paths[path].schema.paths) {
this.singleNestedPaths[path + '.' + key] =
this.paths[path].schema.paths[key];
if (schemaType.$isSingleNested) {
for (let key in schemaType.schema.paths) {
this.singleNestedPaths[path + '.' + key] = schemaType.schema.paths[key];
}
for (let key in this.paths[path].schema.singleNestedPaths) {
for (let key in schemaType.schema.singleNestedPaths) {
this.singleNestedPaths[path + '.' + key] =
this.paths[path].schema.singleNestedPaths[key];
schemaType.schema.singleNestedPaths[key];
}

this.childSchemas.push({
schema: this.paths[path].schema,
model: this.paths[path].caster
schema: schemaType.schema,
model: schemaType.caster
});
} else if (this.paths[path].$isMongooseDocumentArray) {
} else if (schemaType.$isMongooseDocumentArray) {
this.childSchemas.push({
schema: this.paths[path].schema,
model: this.paths[path].casterConstructor
schema: schemaType.schema,
model: schemaType.casterConstructor
});
} else if (this.paths[path].path === '_id' && this.paths[path].options && this.paths[path].options.unique) {
throw new Error('Cannot put unique index on _id');
}

return this;
Expand Down
2 changes: 1 addition & 1 deletion test/model.indexes.test.js
Expand Up @@ -139,7 +139,7 @@ describe('model', function() {

it('of multiple embedded documents with same schema', function(done) {
var BlogPosts = new Schema({
_id: {type: ObjectId, index: true},
_id: {type: ObjectId, unique: true},
title: {type: String, index: true},
desc: String
});
Expand Down
11 changes: 0 additions & 11 deletions test/schema.test.js
Expand Up @@ -835,17 +835,6 @@ describe('schema', function() {

done();
});
it('errors if unique index on _id', function(done) {
try {
var schema = new Schema({ _id: { type: mongoose.Schema.Types.ObjectId, unique: true } });

schema.path('_id').index({ unique: true });
done(new Error('Should not have reached this point!'));
} catch (error) {
assert.equal(error.message, 'Cannot put unique index on _id');
done();
}
});

it('with single nested doc (gh-6113)', function(done) {
var pointSchema = new Schema({
Expand Down

0 comments on commit 985a845

Please sign in to comment.