Skip to content

Commit

Permalink
fix(db): move db constants to other file to avoid circular ref (#1858)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
daprahamian authored Oct 29, 2018
1 parent c9996fb commit 239036f
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 11 deletions.
10 changes: 10 additions & 0 deletions lib/constants.js
Original file line number Diff line number Diff line change
@@ -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'
};
15 changes: 8 additions & 7 deletions lib/db.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
12 changes: 8 additions & 4 deletions lib/operations/db_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'));
Expand All @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) => {
Expand Down
57 changes: 57 additions & 0 deletions test/unit/create_index_error_tests.js
Original file line number Diff line number Diff line change
@@ -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);
}
}
);
});
});

0 comments on commit 239036f

Please sign in to comment.