From 239036fc0e7cdbdb6a9bfed60bee7347a7c4c453 Mon Sep 17 00:00:00 2001 From: Dan Aprahamian Date: Mon, 29 Oct 2018 13:57:28 -0400 Subject: [PATCH] fix(db): move db constants to other file to avoid circular ref (#1858) * fix(db): move db constants to other file to avoid circular ref lib/db.js referenced lib/operations/db_ops.js, which in turn referenced lib/db.js to get some constants. Moved constants to their own file, and used dynamic imports to get DB constructor Fixes NODE-1692 --- lib/constants.js | 10 +++++ lib/db.js | 15 +++---- lib/operations/db_ops.js | 12 ++++-- test/unit/create_index_error_tests.js | 57 +++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 lib/constants.js create mode 100644 test/unit/create_index_error_tests.js diff --git a/lib/constants.js b/lib/constants.js new file mode 100644 index 0000000000..d6cc68add8 --- /dev/null +++ b/lib/constants.js @@ -0,0 +1,10 @@ +'use strict'; + +module.exports = { + SYSTEM_NAMESPACE_COLLECTION: 'system.namespaces', + SYSTEM_INDEX_COLLECTION: 'system.indexes', + SYSTEM_PROFILE_COLLECTION: 'system.profile', + SYSTEM_USER_COLLECTION: 'system.users', + SYSTEM_COMMAND_COLLECTION: '$cmd', + SYSTEM_JS_COLLECTION: 'system.js' +}; diff --git a/lib/db.js b/lib/db.js index 478989638e..b3257f3a3b 100644 --- a/lib/db.js +++ b/lib/db.js @@ -19,6 +19,7 @@ const resolveReadPreference = require('./utils').resolveReadPreference; const ChangeStream = require('./change_stream'); const deprecate = require('util').deprecate; const deprecateOptions = require('./utils').deprecateOptions; +const CONSTANTS = require('./constants'); // Operations const addUser = require('./operations/db_ops').addUser; @@ -532,7 +533,7 @@ Db.prototype.listCollections = function(filter, options) { // Return options const _options = { transforms: listCollectionsTransforms(this.s.databaseName) }; // Get the cursor - cursor = this.collection(Db.SYSTEM_NAMESPACE_COLLECTION).find(filter, _options); + cursor = this.collection(CONSTANTS.SYSTEM_NAMESPACE_COLLECTION).find(filter, _options); // Do we have a readPreference, apply it if (options.readPreference) cursor.setReadPreference(options.readPreference); // Set the passed in batch size if one was provided @@ -974,11 +975,11 @@ Db.prototype.getLogger = function() { */ // Constants -Db.SYSTEM_NAMESPACE_COLLECTION = 'system.namespaces'; -Db.SYSTEM_INDEX_COLLECTION = 'system.indexes'; -Db.SYSTEM_PROFILE_COLLECTION = 'system.profile'; -Db.SYSTEM_USER_COLLECTION = 'system.users'; -Db.SYSTEM_COMMAND_COLLECTION = '$cmd'; -Db.SYSTEM_JS_COLLECTION = 'system.js'; +Db.SYSTEM_NAMESPACE_COLLECTION = CONSTANTS.SYSTEM_NAMESPACE_COLLECTION; +Db.SYSTEM_INDEX_COLLECTION = CONSTANTS.SYSTEM_INDEX_COLLECTION; +Db.SYSTEM_PROFILE_COLLECTION = CONSTANTS.SYSTEM_PROFILE_COLLECTION; +Db.SYSTEM_USER_COLLECTION = CONSTANTS.SYSTEM_USER_COLLECTION; +Db.SYSTEM_COMMAND_COLLECTION = CONSTANTS.SYSTEM_COMMAND_COLLECTION; +Db.SYSTEM_JS_COLLECTION = CONSTANTS.SYSTEM_JS_COLLECTION; module.exports = Db; diff --git a/lib/operations/db_ops.js b/lib/operations/db_ops.js index af0210a97b..3795f8b05c 100644 --- a/lib/operations/db_ops.js +++ b/lib/operations/db_ops.js @@ -4,13 +4,13 @@ const applyWriteConcern = require('../utils').applyWriteConcern; const Code = require('mongodb-core').BSON.Code; const resolveReadPreference = require('../utils').resolveReadPreference; const crypto = require('crypto'); -const Db = require('../db'); const debugOptions = require('../utils').debugOptions; const handleCallback = require('../utils').handleCallback; const MongoError = require('mongodb-core').MongoError; const parseIndexOptions = require('../utils').parseIndexOptions; const ReadPreference = require('mongodb-core').ReadPreference; const toError = require('../utils').toError; +const CONSTANTS = require('../constants'); const count = require('./collection_ops').count; const findOne = require('./collection_ops').findOne; @@ -64,6 +64,8 @@ const illegalCommandFields = [ * @param {Db~resultCallback} [callback] The command result callback */ function addUser(db, username, password, options, callback) { + const Db = require('../db'); + // Did the user destroy the topology if (db.serverConfig && db.serverConfig.isDestroyed()) return callback(new MongoError('topology was destroyed')); @@ -83,7 +85,7 @@ function addUser(db, username, password, options, callback) { const db = options.dbName ? new Db(options.dbName, db.s.topology, db.s.options) : db; // Fetch a user collection - const collection = db.collection(Db.SYSTEM_USER_COLLECTION); + const collection = db.collection(CONSTANTS.SYSTEM_USER_COLLECTION); // Check if we are inserting the first user count(collection, {}, finalOptions, (err, count) => { @@ -296,7 +298,7 @@ function createIndex(db, name, fieldOrSpec, options, callback) { finalOptions.checkKeys = false; // Insert document db.s.topology.insert( - `${db.s.databaseName}.${Db.SYSTEM_INDEX_COLLECTION}`, + `${db.s.databaseName}.${CONSTANTS.SYSTEM_INDEX_COLLECTION}`, doc, finalOptions, (err, result) => { @@ -630,6 +632,8 @@ function profilingLevel(db, options, callback) { * @param {Db~resultCallback} [callback] The command result callback */ function removeUser(db, username, options, callback) { + const Db = require('../db'); + // Attempt to execute command executeAuthRemoveUserCommand(db, username, options, (err, result) => { if (err && err.code === -5000) { @@ -638,7 +642,7 @@ function removeUser(db, username, options, callback) { const db = options.dbName ? new Db(options.dbName, db.s.topology, db.s.options) : db; // Fetch a user collection - const collection = db.collection(Db.SYSTEM_USER_COLLECTION); + const collection = db.collection(CONSTANTS.SYSTEM_USER_COLLECTION); // Locate the user findOne(collection, { user: username }, finalOptions, (err, user) => { diff --git a/test/unit/create_index_error_tests.js b/test/unit/create_index_error_tests.js new file mode 100644 index 0000000000..be89a06893 --- /dev/null +++ b/test/unit/create_index_error_tests.js @@ -0,0 +1,57 @@ +'use strict'; + +const expect = require('chai').expect; +const mock = require('mongodb-mock-server'); + +describe('CreateIndexError', function() { + const test = {}; + beforeEach(() => mock.createServer().then(_server => (test.server = _server))); + afterEach(() => mock.cleanup()); + + it('should error when createIndex fails', function(done) { + const ERROR_RESPONSE = { + ok: 0, + errmsg: + 'WiredTigerIndex::insert: key too large to index, failing 1470 { : "56f37cb8e4b089e98d52ab0e", : "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa..." }', + code: 17280 + }; + + test.server.setMessageHandler(request => { + const doc = request.document; + + if (doc.ismaster) { + return request.reply(Object.assign({}, mock.DEFAULT_ISMASTER)); + } + + if (doc.createIndexes) { + return request.reply(ERROR_RESPONSE); + } + + if (doc.insert === 'system.indexes') { + return request.reply(ERROR_RESPONSE); + } + }); + + const client = this.configuration.newClient(`mongodb://${test.server.uri()}`); + + const close = e => client.close().then(() => done(e)); + + client + .connect() + .then(() => client.db('foo').collection('bar')) + .then(coll => coll.createIndex({ a: 1 })) + .then( + () => close('Expected createIndex to fail, but it succeeded'), + e => { + try { + expect(e).to.have.property('ok', ERROR_RESPONSE.ok); + expect(e).to.have.property('errmsg', ERROR_RESPONSE.errmsg); + expect(e).to.have.property('code', ERROR_RESPONSE.code); + close(); + } catch (err) { + close(err); + } + } + ); + }); +});