Skip to content
This repository was archived by the owner on May 17, 2021. It is now read-only.

Commit f404cd6

Browse files
imlucasimlucas
committed
Merge pull request #20 from mongodb-js/INT-962-fix-listCollections
INT-962 fix list collections
2 parents 6406be3 + 11f2615 commit f404cd6

File tree

5 files changed

+549
-100
lines changed

5 files changed

+549
-100
lines changed

‎.travis.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ before_install:
88
- npm config -g list -l
99
- npm --version
1010
script: npm run-script ci
11-
cache:
12-
directories:
13-
- node_modules
11+
# cache:
12+
# directories:
13+
# - node_modules
1414
notifications:
1515
flowdock: e3dc17bc8a2c1b3412abe3e5747f8291
1616
env:

‎lib/fetch.js

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ function getBuildInfo(done, results) {
6565
debug('checking we can get buildInfo...');
6666
db.admin().buildInfo(function(err, res) {
6767
if (err) {
68+
// buildInfo doesn't require any privileges to run, so if it fails,
69+
// something really went wrong and we should return the error.
6870
debug('buildInfo failed!', err);
6971
err.command = 'buildInfo';
7072
return done(err);
@@ -131,24 +133,25 @@ function parseHostInfo(resp) {
131133
function getHostInfo(done, results) {
132134
var db = results.db;
133135

134-
debug('checking we can get hostInfo...');
135136
var spec = {
136137
hostInfo: 1
137138
};
138139
var options = {};
139140
db.admin().command(spec, options, function(err, res) {
140141
if (err) {
141-
if (/^not authorized/.test(err.message)) {
142-
debug('hostInfo unavailable for this user and thats ok!');
142+
if (isNotAuthorized(err)) {
143+
// if the error is that the user is not authorized, silently ignore it
144+
// and return an empty document
145+
debug('user does not have hostInfo privilege, returning empty document {}');
143146
done(null, {});
144147
return;
145148
}
149+
// something else went wrong and we should return the error.
146150
debug('driver error', err);
147151
err.command = 'hostInfo';
148152
done(err);
149153
return;
150154
}
151-
debug('got hostInfo successully!');
152155
done(null, parseHostInfo(res));
153156
});
154157
}
@@ -179,11 +182,12 @@ function listDatabases(done, results) {
179182
db.admin().command(spec, options, function(err, res) {
180183
if (err) {
181184
if (isNotAuthorized(err)) {
185+
// we caught this further up already and really should never get here!
182186
debug('listDatabases failed. returning empty list []');
183187
done(null, []);
184188
return;
185189
}
186-
190+
// the command failed for another reason, report the error
187191
debug('listDatabases failed', err);
188192
err.command = 'listDatabases';
189193
done(err);
@@ -270,9 +274,7 @@ function getDatabases(done, results) {
270274
async.parallel(_.map(dbnames, function(name) {
271275
var result = _.partial(getDatabase, db, name);
272276
return result;
273-
}), function(err, res) {
274-
done(err, res);
275-
});
277+
}), done);
276278
}
277279

278280

@@ -284,6 +286,7 @@ function getUserInfo(done, results) {
284286
connectionStatus: 1,
285287
showPrivileges: true
286288
}, function(err, res) {
289+
// no auth required, if this fails there was a real problem
287290
if (err) {
288291
done(err);
289292
}
@@ -297,6 +300,7 @@ function getUserInfo(done, results) {
297300
usersInfo: user,
298301
showPrivileges: true
299302
}, function(_err, _res) {
303+
// should always succeed for the logged-in user
300304
if (_err) {
301305
done(_err);
302306
}
@@ -355,6 +359,15 @@ function getDatabaseCollections(db, done) {
355359

356360
db.listCollections(spec, options).toArray(function(err, res) {
357361
if (err) {
362+
if (isNotAuthorized(err)) {
363+
// if the error is that the user is not authorized, silently ignore it
364+
// and return an empty list
365+
debug('not allowed to run `listCollections` command on %s, returning'
366+
+ ' empty result [].', db.databaseName);
367+
return done(null, []);
368+
}
369+
// the command failed for another reason, report the error
370+
debug('listCollections failed', err);
358371
err.command = 'listCollections';
359372
return done(err);
360373
}
@@ -463,10 +476,10 @@ function getInstanceDetail(db, done) {
463476

464477

465478
module.exports = getInstanceDetail;
466-
467-
// module.exports.getCollections = getCollections;
468-
// module.exports.getDatabaseCollections = getDatabaseCollections;
469-
// module.exports.getDatabases = getDatabases;
470-
// module.exports.getDatabase = getDatabase;
471-
// module.exports.getBuildInfo = getBuildInfo;
472-
// module.exports.getHostInfo = getHostInfo;
479+
module.exports.getBuildInfo = getBuildInfo;
480+
module.exports.getHostInfo = getHostInfo;
481+
module.exports.listDatabases = listDatabases;
482+
module.exports.getAllowedDatabases = getAllowedDatabases;
483+
module.exports.getAllowedCollections = getAllowedCollections;
484+
module.exports.getDatabaseCollections = getDatabaseCollections;
485+
module.exports.listCollections = listCollections;

‎test/fetch-mocked.test.js

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
var assert = require('assert');
2+
var fetch = require('../').fetch;
3+
var fixtures = require('./fixtures');
4+
5+
// var debug = require('debug')('mongodb-instance-model:test:fetch-mocked');
6+
7+
8+
describe('unit tests on fetch functions', function() {
9+
var makeMockDB;
10+
11+
before(function() {
12+
/**
13+
* Create a mock db object that will return an error or a result on
14+
* any of its methods. Pass in either and error or a result, but not
15+
* both (just like regular Errbacks).
16+
*
17+
* @param {Error|null} err if the call should return an error, specify
18+
* the error object here.
19+
* @param {Any} res if the call should return a value, specify
20+
* the value here.
21+
* @return {Object} a db object that behaves like the mongodb
22+
* driver
23+
*/
24+
makeMockDB = function(err, res) {
25+
var db = {};
26+
db.admin = function() {
27+
return {
28+
// add more db methods here as needed.
29+
30+
// buildInfo is a separate function on the admin object
31+
buildInfo: function(callback) {
32+
return callback(err, res);
33+
},
34+
// all other commands return the global err/res results
35+
command: function(command, options, callback) {
36+
return callback(err, res);
37+
}
38+
};
39+
};
40+
return db;
41+
};
42+
});
43+
44+
describe('getBuildInfo', function() {
45+
it('should pass on any error that buildInfo returns', function(done) {
46+
// instead of the real db handle, pass in the mocked one
47+
var results = {
48+
// make a db that always returns error for db.admin().buildInfo()
49+
db: makeMockDB(new Error('some strange error'), null)
50+
};
51+
fetch.getBuildInfo(function(err, res) {
52+
assert.equal(res, null);
53+
assert.equal(err.command, 'buildInfo');
54+
assert.equal(err.message, 'some strange error');
55+
done();
56+
}, results);
57+
});
58+
});
59+
60+
describe('getHostInfo', function() {
61+
it('should ignore auth errors gracefully', function(done) {
62+
// instead of the real db handle, pass in the mocked one
63+
var results = {
64+
db: makeMockDB(new Error('not authorized on fooBarDatabase to execute command '
65+
+ '{listCollections: true, filter: {}, cursor: {}'), null)
66+
};
67+
fetch.getHostInfo(function(err, res) {
68+
assert.equal(err, null);
69+
assert.deepEqual(res, []);
70+
done();
71+
}, results);
72+
});
73+
it('should pass on other errors from the hostInfo command', function(done) {
74+
// instead of the real db handle, pass in the mocked one
75+
var results = {
76+
db: makeMockDB(new Error('some other error from hostInfo'), null)
77+
};
78+
fetch.getHostInfo(function(err, res) {
79+
assert.ok(err);
80+
assert.equal(err.command, 'hostInfo');
81+
assert.deepEqual(res, null);
82+
done();
83+
}, results);
84+
});
85+
});
86+
87+
describe('listDatabases', function() {
88+
var results = {};
89+
90+
beforeEach(function() {
91+
results.userInfo = fixtures.USER_INFO;
92+
});
93+
94+
it('should ignore auth errors gracefully', function(done) {
95+
// instead of the real db handle, pass in the mocked one
96+
results.db = makeMockDB(new Error('not authorized on admin to execute command '
97+
+ '{ listDatabases: 1.0 }'), null);
98+
99+
fetch.listDatabases(function(err, res) {
100+
assert.equal(err, null);
101+
assert.deepEqual(res, []);
102+
done();
103+
}, results);
104+
});
105+
it('should pass on other errors from the listDatabases command', function(done) {
106+
// instead of the real db handle, pass in the mocked one
107+
results.db = makeMockDB(new Error('some other error from hostInfo'), null);
108+
109+
fetch.listDatabases(function(err, res) {
110+
assert.ok(err);
111+
assert.equal(err.command, 'listDatabases');
112+
assert.deepEqual(res, null);
113+
done();
114+
}, results);
115+
});
116+
});
117+
118+
describe('getAllowedDatabases', function() {
119+
it('should return the correct results');
120+
// ...
121+
});
122+
describe('getAllowedCollections', function() {
123+
it('should return the correct results');
124+
// ...
125+
});
126+
describe('getDatabaseCollections', function() {
127+
it('should ignore auth errors gracefully');
128+
it('should pass on other errors from the listCollections command');
129+
// ...
130+
});
131+
describe('listCollections', function() {
132+
it('should merge the two collection lists correctly');
133+
// ...
134+
});
135+
});

‎test/fetch.test.js

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -28,50 +28,6 @@ describe('mongodb-instance-model#fetch', function() {
2828
done();
2929
});
3030
});
31-
// it('should list collections', function(done) {
32-
// assert(db);
33-
// fetch.getAllCollections(db, function(err, res) {
34-
// if (err) {
35-
// return done(err);
36-
// }
37-
// debug('list collections', JSON.stringify(res, null, 2));
38-
// done();
39-
// });
40-
// });
41-
//
42-
// it('should list databases', function(done) {
43-
// assert(db);
44-
// fetch.getDatabases(db, function(err, res) {
45-
// if (err) {
46-
// return done(err);
47-
// }
48-
// debug('list databases', JSON.stringify(res, null, 2));
49-
// done();
50-
// });
51-
// });
52-
//
53-
// it('should get build info', function(done) {
54-
// assert(db);
55-
// fetch.getBuildInfo(db, function(err, res) {
56-
// if (err) {
57-
// return done(err);
58-
// }
59-
// debug('build info', JSON.stringify(res, null, 2));
60-
// done();
61-
// });
62-
// });
63-
//
64-
// it('should get host info', function(done) {
65-
// assert(db);
66-
// fetch.getHostInfo(db, function(err, res) {
67-
// if (err) {
68-
// return done(err);
69-
// }
70-
// debug('host info', JSON.stringify(res, null, 2));
71-
// done();
72-
// });
73-
// });
74-
7531
it('should get instance details', function(done) {
7632
assert(db);
7733
fetch(db, function(err, res) {
@@ -141,44 +97,6 @@ describe('mongodb-instance-model#fetch', function() {
14197
done();
14298
});
14399
});
144-
145-
// it('should list databases', function(done) {
146-
// if (process.env.dry ) {
147-
// this.skip();
148-
// return;
149-
// }
150-
// this.slow(5000);
151-
// this.timeout(10000);
152-
// assert(db, 'requires successful connection');
153-
//
154-
// fetch.getDatabases(db, function(err, res) {
155-
// if (err) return done(err);
156-
//
157-
// assert(Array.isArray(res));
158-
// assert(res.length > 0, 'Database list is empty');
159-
// done();
160-
// });
161-
// });
162-
//
163-
// it('should list collections', function(done) {
164-
// if (process.env.dry ) {
165-
// this.skip();
166-
// return;
167-
// }
168-
// this.slow(5000);
169-
// this.timeout(10000);
170-
// assert(db, 'requires successful connection');
171-
//
172-
// fetch.getAllCollections(db, function(err, res) {
173-
// if (err) return done(err);
174-
//
175-
// assert(Array.isArray(res));
176-
// assert(res.length > 0, 'Collection list is empty');
177-
// done();
178-
// });
179-
// });
180-
181-
182100
it('should get instance details', function(done) {
183101
if (process.env.dry) {
184102
this.skip();

0 commit comments

Comments
 (0)