Skip to content

Commit e561164

Browse files
authored
Merge pull request Automattic#15112 from Automattic/vkarpov15/Automatticgh-15109
fix(schema): avoid throwing duplicate index error if index spec keys have different order or index has a custom name
2 parents 5782409 + 421c866 commit e561164

File tree

4 files changed

+71
-3
lines changed

4 files changed

+71
-3
lines changed
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
3+
/**
4+
* Compares two index specifications to determine if they are equal.
5+
*
6+
* #### Example:
7+
* isIndexSpecEqual({ a: 1, b: 1 }, { a: 1, b: 1 }); // true
8+
* isIndexSpecEqual({ a: 1, b: 1 }, { b: 1, a: 1 }); // false
9+
* isIndexSpecEqual({ a: 1, b: -1 }, { a: 1, b: 1 }); // false
10+
*
11+
* @param {Object} spec1 The first index specification to compare.
12+
* @param {Object} spec2 The second index specification to compare.
13+
* @returns {Boolean} Returns true if the index specifications are equal, otherwise returns false.
14+
*/
15+
16+
module.exports = function isIndexSpecEqual(spec1, spec2) {
17+
const spec1Keys = Object.keys(spec1);
18+
const spec2Keys = Object.keys(spec2);
19+
20+
if (spec1Keys.length !== spec2Keys.length) {
21+
return false;
22+
}
23+
24+
for (let i = 0; i < spec1Keys.length; i++) {
25+
const key = spec1Keys[i];
26+
if (key !== spec2Keys[i] || spec1[key] !== spec2[key]) {
27+
return false;
28+
}
29+
}
30+
31+
return true;
32+
};

lib/schema.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ const getConstructorName = require('./helpers/getConstructorName');
1818
const getIndexes = require('./helpers/schema/getIndexes');
1919
const handleReadPreferenceAliases = require('./helpers/query/handleReadPreferenceAliases');
2020
const idGetter = require('./helpers/schema/idGetter');
21+
const isIndexSpecEqual = require('./helpers/indexes/isIndexSpecEqual');
2122
const merge = require('./helpers/schema/merge');
2223
const mpath = require('mpath');
2324
const setPopulatedVirtualValue = require('./helpers/populate/setPopulatedVirtualValue');
2425
const setupTimestamps = require('./helpers/timestamps/setupTimestamps');
2526
const utils = require('./utils');
2627
const validateRef = require('./helpers/populate/validateRef');
27-
const util = require('util');
2828

2929
const hasNumericSubpathRegex = /\.\d+(\.|$)/;
3030

@@ -899,7 +899,7 @@ Schema.prototype.removeIndex = function removeIndex(index) {
899899

900900
if (typeof index === 'object') {
901901
for (let i = this._indexes.length - 1; i >= 0; --i) {
902-
if (util.isDeepStrictEqual(this._indexes[i][0], index)) {
902+
if (isIndexSpecEqual(this._indexes[i][0], index)) {
903903
this._indexes.splice(i, 1);
904904
}
905905
}
@@ -2147,7 +2147,7 @@ Schema.prototype.index = function(fields, options) {
21472147
}
21482148

21492149
for (const existingIndex of this.indexes()) {
2150-
if (util.isDeepStrictEqual(existingIndex[0], fields)) {
2150+
if (options.name == null && existingIndex[1].name == null && isIndexSpecEqual(existingIndex[0], fields)) {
21512151
throw new MongooseError(`Schema already has an index on ${JSON.stringify(fields)}`);
21522152
}
21532153
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
const isIndexSpecEqual = require('../../lib/helpers/indexes/isIndexSpecEqual');
5+
6+
describe('isIndexSpecEqual', function() {
7+
it('should return true for equal index specifications', () => {
8+
const spec1 = { name: 1, age: -1 };
9+
const spec2 = { name: 1, age: -1 };
10+
const result = isIndexSpecEqual(spec1, spec2);
11+
assert.strictEqual(result, true);
12+
});
13+
14+
it('should return false for different key order', () => {
15+
const spec1 = { name: 1, age: -1 };
16+
const spec2 = { age: -1, name: 1 };
17+
const result = isIndexSpecEqual(spec1, spec2);
18+
assert.strictEqual(result, false);
19+
});
20+
21+
it('should return false for different index keys', () => {
22+
const spec1 = { name: 1, age: -1 };
23+
const spec2 = { name: 1, dob: -1 };
24+
const result = isIndexSpecEqual(spec1, spec2);
25+
assert.strictEqual(result, false);
26+
});
27+
});

test/schema.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3294,5 +3294,14 @@ describe('schema', function() {
32943294
assert.throws(() => {
32953295
ObjectKeySchema.index({ key: 1 });
32963296
}, /MongooseError.*already has an index/);
3297+
3298+
ObjectKeySchema.index({ key: 1, type: 1 });
3299+
assert.throws(() => {
3300+
ObjectKeySchema.index({ key: 1, type: 1 });
3301+
}, /MongooseError.*already has an index/);
3302+
3303+
ObjectKeySchema.index({ type: 1, key: 1 });
3304+
ObjectKeySchema.index({ key: 1, type: -1 });
3305+
ObjectKeySchema.index({ key: 1, type: 1 }, { unique: true, name: 'special index' });
32973306
});
32983307
});

0 commit comments

Comments
 (0)