Skip to content

Commit

Permalink
fix(schema): avoid throwing duplicate index error if index spec keys …
Browse files Browse the repository at this point in the history
…have different order or index has a custom name

Fix #15109
  • Loading branch information
vkarpov15 committed Dec 17, 2024
1 parent b9136c1 commit e25dafc
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 3 deletions.
32 changes: 32 additions & 0 deletions lib/helpers/indexes/isIndexSpecEqual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';

/**
* Compares two index specifications to determine if they are equal.
*
* #### Example:
* isIndexSpecEqual({ a: 1, b: 1 }, { a: 1, b: 1 }); // true
* isIndexSpecEqual({ a: 1, b: 1 }, { b: 1, a: 1 }); // false
* isIndexSpecEqual({ a: 1, b: -1 }, { a: 1, b: 1 }); // false
*
* @param {Object} spec1 - The first index specification to compare.
* @param {Object} spec2 - The second index specification to compare.
* @returns {Boolean} Returns true if the index specifications are equal, otherwise returns false.
*/

module.exports = function isIndexSpecEqual(spec1, spec2) {
const spec1Keys = Object.keys(spec1);
const spec2Keys = Object.keys(spec2);

if (spec1Keys.length !== spec2Keys.length) {
return false;
}

for (let i = 0; i < spec1Keys.length; i++) {
const key = spec1Keys[i];
if (key !== spec2Keys[i] || spec1[key] !== spec2[key]) {
return false;
}
}

return true;
};
6 changes: 3 additions & 3 deletions lib/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ const getConstructorName = require('./helpers/getConstructorName');
const getIndexes = require('./helpers/schema/getIndexes');
const handleReadPreferenceAliases = require('./helpers/query/handleReadPreferenceAliases');
const idGetter = require('./helpers/schema/idGetter');
const isIndexSpecEqual = require('./helpers/indexes/isIndexSpecEqual');
const merge = require('./helpers/schema/merge');
const mpath = require('mpath');
const setPopulatedVirtualValue = require('./helpers/populate/setPopulatedVirtualValue');
const setupTimestamps = require('./helpers/timestamps/setupTimestamps');
const utils = require('./utils');
const validateRef = require('./helpers/populate/validateRef');
const util = require('util');

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

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

if (typeof index === 'object') {
for (let i = this._indexes.length - 1; i >= 0; --i) {
if (util.isDeepStrictEqual(this._indexes[i][0], index)) {
if (isIndexSpecEqual(this._indexes[i][0], index)) {
this._indexes.splice(i, 1);
}
}
Expand Down Expand Up @@ -2147,7 +2147,7 @@ Schema.prototype.index = function(fields, options) {
}

for (const existingIndex of this.indexes()) {
if (util.isDeepStrictEqual(existingIndex[0], fields)) {
if (options.name == null && existingIndex[1].name == null && isIndexSpecEqual(existingIndex[0], fields)) {
throw new MongooseError(`Schema already has an index on ${JSON.stringify(fields)}`);
}
}
Expand Down
27 changes: 27 additions & 0 deletions test/helpers/indexes.isIndexSpecEqual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

const assert = require('assert');
const isIndexSpecEqual = require('../../lib/helpers/indexes/isIndexSpecEqual');

describe('isIndexSpecEqual', function() {
it('should return true for equal index specifications', () => {
const spec1 = { name: 1, age: -1 };
const spec2 = { name: 1, age: -1 };
const result = isIndexSpecEqual(spec1, spec2);
assert.strictEqual(result, true);
});

it('should return false for different key order', () => {
const spec1 = { name: 1, age: -1 };
const spec2 = { age: -1, name: 1 };
const result = isIndexSpecEqual(spec1, spec2);
assert.strictEqual(result, false);
});

it('should return false for different index keys', () => {
const spec1 = { name: 1, age: -1 };
const spec2 = { name: 1, dob: -1 };
const result = isIndexSpecEqual(spec1, spec2);
assert.strictEqual(result, false);
});
});
9 changes: 9 additions & 0 deletions test/schema.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3294,5 +3294,14 @@ describe('schema', function() {
assert.throws(() => {
ObjectKeySchema.index({ key: 1 });
}, /MongooseError.*already has an index/);

ObjectKeySchema.index({ key: 1, type: 1 });
assert.throws(() => {
ObjectKeySchema.index({ key: 1, type: 1 });
}, /MongooseError.*already has an index/);

ObjectKeySchema.index({ type: 1, key: 1 });
ObjectKeySchema.index({ key: 1, type: -1 });
ObjectKeySchema.index({ key: 1, type: 1 }, { unique: true, name: 'special index' });
});
});

0 comments on commit e25dafc

Please sign in to comment.