Skip to content

Commit 239036f

Browse files
authored
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
1 parent c9996fb commit 239036f

File tree

4 files changed

+83
-11
lines changed

4 files changed

+83
-11
lines changed

lib/constants.js

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
3+
module.exports = {
4+
SYSTEM_NAMESPACE_COLLECTION: 'system.namespaces',
5+
SYSTEM_INDEX_COLLECTION: 'system.indexes',
6+
SYSTEM_PROFILE_COLLECTION: 'system.profile',
7+
SYSTEM_USER_COLLECTION: 'system.users',
8+
SYSTEM_COMMAND_COLLECTION: '$cmd',
9+
SYSTEM_JS_COLLECTION: 'system.js'
10+
};

lib/db.js

+8-7
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const resolveReadPreference = require('./utils').resolveReadPreference;
1919
const ChangeStream = require('./change_stream');
2020
const deprecate = require('util').deprecate;
2121
const deprecateOptions = require('./utils').deprecateOptions;
22+
const CONSTANTS = require('./constants');
2223

2324
// Operations
2425
const addUser = require('./operations/db_ops').addUser;
@@ -532,7 +533,7 @@ Db.prototype.listCollections = function(filter, options) {
532533
// Return options
533534
const _options = { transforms: listCollectionsTransforms(this.s.databaseName) };
534535
// Get the cursor
535-
cursor = this.collection(Db.SYSTEM_NAMESPACE_COLLECTION).find(filter, _options);
536+
cursor = this.collection(CONSTANTS.SYSTEM_NAMESPACE_COLLECTION).find(filter, _options);
536537
// Do we have a readPreference, apply it
537538
if (options.readPreference) cursor.setReadPreference(options.readPreference);
538539
// Set the passed in batch size if one was provided
@@ -974,11 +975,11 @@ Db.prototype.getLogger = function() {
974975
*/
975976

976977
// Constants
977-
Db.SYSTEM_NAMESPACE_COLLECTION = 'system.namespaces';
978-
Db.SYSTEM_INDEX_COLLECTION = 'system.indexes';
979-
Db.SYSTEM_PROFILE_COLLECTION = 'system.profile';
980-
Db.SYSTEM_USER_COLLECTION = 'system.users';
981-
Db.SYSTEM_COMMAND_COLLECTION = '$cmd';
982-
Db.SYSTEM_JS_COLLECTION = 'system.js';
978+
Db.SYSTEM_NAMESPACE_COLLECTION = CONSTANTS.SYSTEM_NAMESPACE_COLLECTION;
979+
Db.SYSTEM_INDEX_COLLECTION = CONSTANTS.SYSTEM_INDEX_COLLECTION;
980+
Db.SYSTEM_PROFILE_COLLECTION = CONSTANTS.SYSTEM_PROFILE_COLLECTION;
981+
Db.SYSTEM_USER_COLLECTION = CONSTANTS.SYSTEM_USER_COLLECTION;
982+
Db.SYSTEM_COMMAND_COLLECTION = CONSTANTS.SYSTEM_COMMAND_COLLECTION;
983+
Db.SYSTEM_JS_COLLECTION = CONSTANTS.SYSTEM_JS_COLLECTION;
983984

984985
module.exports = Db;

lib/operations/db_ops.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ const applyWriteConcern = require('../utils').applyWriteConcern;
44
const Code = require('mongodb-core').BSON.Code;
55
const resolveReadPreference = require('../utils').resolveReadPreference;
66
const crypto = require('crypto');
7-
const Db = require('../db');
87
const debugOptions = require('../utils').debugOptions;
98
const handleCallback = require('../utils').handleCallback;
109
const MongoError = require('mongodb-core').MongoError;
1110
const parseIndexOptions = require('../utils').parseIndexOptions;
1211
const ReadPreference = require('mongodb-core').ReadPreference;
1312
const toError = require('../utils').toError;
13+
const CONSTANTS = require('../constants');
1414

1515
const count = require('./collection_ops').count;
1616
const findOne = require('./collection_ops').findOne;
@@ -64,6 +64,8 @@ const illegalCommandFields = [
6464
* @param {Db~resultCallback} [callback] The command result callback
6565
*/
6666
function addUser(db, username, password, options, callback) {
67+
const Db = require('../db');
68+
6769
// Did the user destroy the topology
6870
if (db.serverConfig && db.serverConfig.isDestroyed())
6971
return callback(new MongoError('topology was destroyed'));
@@ -83,7 +85,7 @@ function addUser(db, username, password, options, callback) {
8385
const db = options.dbName ? new Db(options.dbName, db.s.topology, db.s.options) : db;
8486

8587
// Fetch a user collection
86-
const collection = db.collection(Db.SYSTEM_USER_COLLECTION);
88+
const collection = db.collection(CONSTANTS.SYSTEM_USER_COLLECTION);
8789

8890
// Check if we are inserting the first user
8991
count(collection, {}, finalOptions, (err, count) => {
@@ -296,7 +298,7 @@ function createIndex(db, name, fieldOrSpec, options, callback) {
296298
finalOptions.checkKeys = false;
297299
// Insert document
298300
db.s.topology.insert(
299-
`${db.s.databaseName}.${Db.SYSTEM_INDEX_COLLECTION}`,
301+
`${db.s.databaseName}.${CONSTANTS.SYSTEM_INDEX_COLLECTION}`,
300302
doc,
301303
finalOptions,
302304
(err, result) => {
@@ -630,6 +632,8 @@ function profilingLevel(db, options, callback) {
630632
* @param {Db~resultCallback} [callback] The command result callback
631633
*/
632634
function removeUser(db, username, options, callback) {
635+
const Db = require('../db');
636+
633637
// Attempt to execute command
634638
executeAuthRemoveUserCommand(db, username, options, (err, result) => {
635639
if (err && err.code === -5000) {
@@ -638,7 +642,7 @@ function removeUser(db, username, options, callback) {
638642
const db = options.dbName ? new Db(options.dbName, db.s.topology, db.s.options) : db;
639643

640644
// Fetch a user collection
641-
const collection = db.collection(Db.SYSTEM_USER_COLLECTION);
645+
const collection = db.collection(CONSTANTS.SYSTEM_USER_COLLECTION);
642646

643647
// Locate the user
644648
findOne(collection, { user: username }, finalOptions, (err, user) => {

test/unit/create_index_error_tests.js

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
3+
const expect = require('chai').expect;
4+
const mock = require('mongodb-mock-server');
5+
6+
describe('CreateIndexError', function() {
7+
const test = {};
8+
beforeEach(() => mock.createServer().then(_server => (test.server = _server)));
9+
afterEach(() => mock.cleanup());
10+
11+
it('should error when createIndex fails', function(done) {
12+
const ERROR_RESPONSE = {
13+
ok: 0,
14+
errmsg:
15+
'WiredTigerIndex::insert: key too large to index, failing 1470 { : "56f37cb8e4b089e98d52ab0e", : "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa..." }',
16+
code: 17280
17+
};
18+
19+
test.server.setMessageHandler(request => {
20+
const doc = request.document;
21+
22+
if (doc.ismaster) {
23+
return request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
24+
}
25+
26+
if (doc.createIndexes) {
27+
return request.reply(ERROR_RESPONSE);
28+
}
29+
30+
if (doc.insert === 'system.indexes') {
31+
return request.reply(ERROR_RESPONSE);
32+
}
33+
});
34+
35+
const client = this.configuration.newClient(`mongodb://${test.server.uri()}`);
36+
37+
const close = e => client.close().then(() => done(e));
38+
39+
client
40+
.connect()
41+
.then(() => client.db('foo').collection('bar'))
42+
.then(coll => coll.createIndex({ a: 1 }))
43+
.then(
44+
() => close('Expected createIndex to fail, but it succeeded'),
45+
e => {
46+
try {
47+
expect(e).to.have.property('ok', ERROR_RESPONSE.ok);
48+
expect(e).to.have.property('errmsg', ERROR_RESPONSE.errmsg);
49+
expect(e).to.have.property('code', ERROR_RESPONSE.code);
50+
close();
51+
} catch (err) {
52+
close(err);
53+
}
54+
}
55+
);
56+
});
57+
});

0 commit comments

Comments
 (0)