From 0305223082c263de8da0d7741888e249379caba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Rebours?= Date: Wed, 18 Jan 2023 17:33:32 +0100 Subject: [PATCH] fix PR comments --- lib/datastore.js | 30 ++++++++++++++++++------------ lib/indexes.js | 19 ++++++++++++++----- lib/model.js | 11 ++++------- test/db.async.test.js | 6 +++--- test/indexes.test.js | 10 +++++----- 5 files changed, 44 insertions(+), 32 deletions(-) diff --git a/lib/datastore.js b/lib/datastore.js index 11050b9..1a6aa48 100755 --- a/lib/datastore.js +++ b/lib/datastore.js @@ -431,7 +431,7 @@ class Datastore extends EventEmitter { /** * Callback version of {@link Datastore#ensureIndex}. * @param {object} options - * @param {string} options.fieldName + * @param {string|string[]} options.fieldName * @param {boolean} [options.unique = false] * @param {boolean} [options.sparse = false] * @param {number} [options.expireAfterSeconds] @@ -448,7 +448,7 @@ class Datastore extends EventEmitter { * This function acts synchronously on the indexes, however the persistence of the indexes is deferred with the * executor. * @param {object} options - * @param {string} options.fieldName Name of the field to index. Use the dot notation to index a field in a nested + * @param {string|string[]} options.fieldName Name of the field to index. Use the dot notation to index a field in a nested * document. For a compound index, use an array of field names. Using a comma in a field name is not permitted. * @param {boolean} [options.unique = false] Enforce field uniqueness. Note that a unique index will raise an error * if you try to index two documents for which the field is not defined. @@ -465,26 +465,32 @@ class Datastore extends EventEmitter { err.missingFieldName = true throw err } - if (Array.isArray(options.fieldName)) { - options.fieldName.sort() - } - if ([].concat(options.fieldName).some(field => field.includes(','))) { + + const _fields = [].concat(options.fieldName).sort() + + if (_fields.some(field => field.includes(','))) { throw new Error('Cannot use comma in index fieldName') } - if (this.indexes[options.fieldName]) return - this.indexes[options.fieldName] = new Index(options) - if (options.expireAfterSeconds !== undefined) this.ttlIndexes[options.fieldName] = options.expireAfterSeconds // With this implementation index creation is not necessary to ensure TTL but we stick with MongoDB's API here + const _options = { + ...options, + fieldName: _fields.join(',') + } + + if (this.indexes[_options.fieldName]) return + + this.indexes[_options.fieldName] = new Index(_options) + if (options.expireAfterSeconds !== undefined) this.ttlIndexes[_options.fieldName] = _options.expireAfterSeconds // With this implementation index creation is not necessary to ensure TTL but we stick with MongoDB's API here try { - this.indexes[options.fieldName].insert(this.getAllData()) + this.indexes[_options.fieldName].insert(this.getAllData()) } catch (e) { - delete this.indexes[options.fieldName] + delete this.indexes[_options.fieldName] throw e } // We may want to force all options to be persisted including defaults, not just the ones passed the index creation function - await this.executor.pushAsync(() => this.persistence.persistNewStateAsync([{ $$indexCreated: options }]), true) + await this.executor.pushAsync(() => this.persistence.persistNewStateAsync([{ $$indexCreated: _options }]), true) } /** diff --git a/lib/indexes.js b/lib/indexes.js index e1b5376..a0335ea 100755 --- a/lib/indexes.js +++ b/lib/indexes.js @@ -38,16 +38,26 @@ class Index { * All methods on an index guarantee that either the whole operation was successful and the index changed * or the operation was unsuccessful and an error is thrown while the index is unchanged * @param {object} options - * @param {string} options.fieldName On which field should the index apply (can use dot notation to index on sub fields) + * @param {string} options.fieldName On which field should the index apply, can use dot notation to index on sub fields, can use comma-separated notation to use compound indexes * @param {boolean} [options.unique = false] Enforces a unique constraint * @param {boolean} [options.sparse = false] Allows a sparse index (we can have documents for which fieldName is `undefined`) */ constructor (options) { /** - * On which field the index applies to (may use dot notation to index on sub fields). + * On which field the index applies to, can use dot notation to index on sub fields, can use comma-separated notation to use compound indexes. * @type {string} */ this.fieldName = options.fieldName + + if (typeof this.fieldName !== 'string') throw new Error('fieldName must be a string') + + /** + * Internal property which is an Array representing the fieldName split with `,`, useful only for compound indexes. + * @type {string[]} + * @private + */ + this._fields = this.fieldName.split(',') + /** * Defines if the index enforces a unique constraint for this index. * @type {boolean} @@ -99,7 +109,7 @@ class Index { return } - const key = model.getDotValues(doc, this.fieldName) + const key = model.getDotValues(doc, this._fields) // We don't index documents that don't contain the field if the index is sparse if ((key === undefined || (typeof key === 'object' && key !== null && Object.values(key).every(el => el === undefined))) && this.sparse) return @@ -171,8 +181,7 @@ class Index { return } - const key = model.getDotValues(doc, this.fieldName) - + const key = model.getDotValues(doc, this._fields) if (key === undefined && this.sparse) return if (!Array.isArray(key)) { diff --git a/lib/model.js b/lib/model.js index 9ba9724..ca9fd89 100755 --- a/lib/model.js +++ b/lib/model.js @@ -507,17 +507,14 @@ const getDotValue = (obj, field) => { * Get dot values for either a bunch of fields or just one. */ const getDotValues = (obj, fields) => { - if (Array.isArray(fields)) { + if (!Array.isArray(fields)) throw new Error('fields must be an Array') + if (fields.length > 1) { const key = {} - const len = fields.length - for (let i = 0; i < len; i++) { - const field = fields[i] + for (const field of fields) { key[field] = getDotValue(obj, field) } return key - } else { - return getDotValue(obj, fields) - } + } else return getDotValue(obj, fields[0]) } /** diff --git a/test/db.async.test.js b/test/db.async.test.js index a8ca172..45cb76e 100644 --- a/test/db.async.test.js +++ b/test/db.async.test.js @@ -1302,7 +1302,7 @@ describe('Database async', function () { assert.equal(d.indexes.z.tree.search('3')[0], d.getAllData()[2]) }) - it('ensureIndex can be called twice on the same field, the second call will ahve no effect', async () => { + it('ensureIndex can be called twice on the same field, the second call will have no effect', async () => { assert.equal(Object.keys(d.indexes).length, 1) assert.equal(Object.keys(d.indexes)[0], '_id') @@ -1327,7 +1327,7 @@ describe('Database async', function () { assert.equal(d.indexes.planet.getAll().length, 2) }) - it('ensureIndex can be called twice on the same compound field, the second call will ahve no effect', async () => { + it('ensureIndex can be called twice on the same compound field, the second call will have no effect', async () => { assert.equal(Object.keys(d.indexes).length, 1) assert.equal(Object.keys(d.indexes)[0], '_id') @@ -1352,7 +1352,7 @@ describe('Database async', function () { assert.equal(d.indexes['planet,star'].getAll().length, 2) }) - it('ensureIndex can be called twice on the same compound field with a different order, the second call will ahve no effect', async () => { + it('ensureIndex can be called twice on the same compound field with a different order, the second call will have no effect', async () => { assert.equal(Object.keys(d.indexes).length, 1) assert.equal(Object.keys(d.indexes)[0], '_id') diff --git a/test/indexes.test.js b/test/indexes.test.js index 3695204..7720619 100755 --- a/test/indexes.test.js +++ b/test/indexes.test.js @@ -30,7 +30,7 @@ describe('Indexes', function () { }) it('Can insert pointers to documents in the index correctly when they have compound fields', function () { - const idx = new Index({ fieldName: ['tf', 'tg'] }) + const idx = new Index({ fieldName: 'tf,tg' }) const doc1 = { a: 5, tf: 'hello', tg: 'world' } const doc2 = { a: 8, tf: 'hello', tg: 'bloup' } const doc3 = { a: 2, tf: 'bloup', tg: 'bloup' } @@ -81,7 +81,7 @@ describe('Indexes', function () { }) it('Inserting twice for the same compound fieldName in a unique index will result in an error thrown', function () { - const idx = new Index({ fieldName: ['tf', 'tg'], unique: true }) + const idx = new Index({ fieldName: 'tf,tg', unique: true }) const doc1 = { a: 5, tf: 'hello', tg: 'world' } idx.insert(doc1) @@ -90,7 +90,7 @@ describe('Indexes', function () { }) it('Inserting twice for a compound fieldName the docs dont have with a unique and sparse index will not throw, since the docs will be non indexed', function () { - const idx = new Index({ fieldName: ['nope', 'nopeNope'], unique: true, sparse: true }) + const idx = new Index({ fieldName: 'nope,nopeNope', unique: true, sparse: true }) const doc1 = { a: 5, tf: 'hello' } const doc2 = { a: 5, tf: 'world' } @@ -245,8 +245,8 @@ describe('Indexes', function () { }) // ==== End of 'Array fields' ==== // describe('Compound Indexes', function () { - it('Supports arrays of fieldNames', function () { - const idx = new Index({ fieldName: ['tf', 'tf2'] }) + it('Supports field names separated by commas', function () { + const idx = new Index({ fieldName: 'tf,tf2' }) const doc1 = { a: 5, tf: 'hello', tf2: 7 } const doc2 = { a: 8, tf: 'hello', tf2: 6 } const doc3 = { a: 2, tf: 'bloup', tf2: 3 }