Skip to content

Commit fc1c3b2

Browse files
emadumljhaywar
emadum
authored and
ljhaywar
committed
refactor!: remove strict/callback mode from Db.collection helper (#2817)
Strict mode has been removed from the `Db.collection` helper, as well as the callback argument which was required by strict mode. `Db.createCollection` can be used when a collection needs to be created explicitly. NODE-2752
1 parent d2f6052 commit fc1c3b2

17 files changed

+635
-952
lines changed

‎src/collection.ts

-2
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,6 @@ export interface CollectionOptions
104104
WriteConcernOptions,
105105
LoggerOptions {
106106
slaveOk?: boolean;
107-
/** Returns an error if the collection does not exist */
108-
strict?: boolean;
109107
/** Specify a read concern for the collection. (only MongoDB 3.2 or higher supported) */
110108
readConcern?: ReadConcernLike;
111109
/** The preferred read preference (ReadPreference.PRIMARY, ReadPreference.PRIMARY_PREFERRED, ReadPreference.SECONDARY, ReadPreference.SECONDARY_PREFERRED, ReadPreference.NEAREST). */

‎src/db.ts

+10-64
Original file line numberDiff line numberDiff line change
@@ -311,77 +311,23 @@ export class Db {
311311
}
312312

313313
/**
314-
* Fetch a specific collection (containing the actual collection information). If the application does not use strict mode you
315-
* can use it without a callback in the following way: `const collection = db.collection('mycollection');`
314+
* Returns a reference to a MongoDB Collection. If it does not exist it will be created implicitly.
316315
*
317316
* @param name - the collection name we wish to access.
318-
* @returns return the new Collection instance if not in strict mode
317+
* @returns return the new Collection instance
319318
*/
320319
collection<TSchema extends Document = Document>(name: string): Collection<TSchema>;
321320
collection<TSchema extends Document = Document>(
322321
name: string,
323-
options: CollectionOptions
324-
): Collection<TSchema>;
325-
collection<TSchema extends Document = Document>(
326-
name: string,
327-
callback: Callback<Collection<TSchema>>
328-
): void;
329-
collection<TSchema extends Document = Document>(
330-
name: string,
331-
options: CollectionOptions,
332-
callback: Callback<Collection<TSchema>>
333-
): void;
334-
collection<TSchema extends Document = Document>(
335-
name: string,
336-
options?: CollectionOptions | Callback<Collection<TSchema>>,
337-
callback?: Callback<Collection<TSchema>>
338-
): Collection<TSchema> | void {
339-
if (typeof options === 'function') (callback = options), (options = {});
340-
const finalOptions = resolveOptions(this, options);
341-
342-
// Execute
343-
if (!finalOptions.strict) {
344-
try {
345-
const collection = new Collection<TSchema>(this, name, finalOptions);
346-
if (callback) callback(undefined, collection);
347-
return collection;
348-
} catch (err) {
349-
if (err instanceof MongoError && callback) return callback(err);
350-
throw err;
351-
}
352-
}
353-
354-
// Strict mode
355-
if (typeof callback !== 'function') {
356-
throw new MongoError(
357-
`A callback is required in strict mode. While getting collection ${name}`
358-
);
322+
options?: CollectionOptions
323+
): Collection<TSchema> {
324+
if (!options) {
325+
options = {};
326+
} else if (typeof options === 'function') {
327+
throw new TypeError('The callback form of this helper has been removed.');
359328
}
360-
361-
// Did the user destroy the topology
362-
if (getTopology(this).isDestroyed()) {
363-
return callback(new MongoError('topology was destroyed'));
364-
}
365-
366-
const listCollectionOptions: ListCollectionsOptions = Object.assign({}, finalOptions, {
367-
nameOnly: true
368-
});
369-
370-
// Strict mode
371-
this.listCollections({ name }, listCollectionOptions).toArray((err, collections) => {
372-
if (callback == null) return;
373-
if (err != null || !collections) return callback(err);
374-
if (collections.length === 0)
375-
return callback(
376-
new MongoError(`Collection ${name} does not exist. Currently in strict mode.`)
377-
);
378-
379-
try {
380-
return callback(undefined, new Collection(this, name, finalOptions));
381-
} catch (err) {
382-
return callback(err);
383-
}
384-
});
329+
const finalOptions = resolveOptions(this, options);
330+
return new Collection<TSchema>(this, name, finalOptions);
385331
}
386332

387333
/**

‎test/functional/aggregation.test.js

+24-26
Original file line numberDiff line numberDiff line change
@@ -806,37 +806,35 @@ describe('Aggregation', function () {
806806
expect(err).to.not.exist;
807807

808808
var db = client.db(databaseName);
809-
db.collection('te.st', function (err, col) {
809+
const col = db.collection('te.st');
810+
var count = 0;
811+
812+
col.insertMany([{ a: 1 }, { a: 1 }, { a: 1 }], function (err, r) {
810813
expect(err).to.not.exist;
811-
var count = 0;
814+
expect(r).property('insertedCount').to.equal(3);
812815

813-
col.insertMany([{ a: 1 }, { a: 1 }, { a: 1 }], function (err, r) {
814-
expect(err).to.not.exist;
815-
expect(r).property('insertedCount').to.equal(3);
816+
const cursor = col.aggregate([{ $project: { a: 1 } }]);
816817

817-
const cursor = col.aggregate([{ $project: { a: 1 } }]);
818+
cursor.toArray(function (err, docs) {
819+
expect(err).to.not.exist;
820+
expect(docs.length).to.be.greaterThan(0);
818821

819-
cursor.toArray(function (err, docs) {
820-
expect(err).to.not.exist;
821-
expect(docs.length).to.be.greaterThan(0);
822-
823-
//Using cursor - KO
824-
col
825-
.aggregate([{ $project: { a: 1 } }], {
826-
cursor: { batchSize: 10000 }
827-
})
828-
.forEach(
829-
function () {
830-
count = count + 1;
831-
},
832-
function (err) {
833-
expect(err).to.not.exist;
834-
expect(count).to.be.greaterThan(0);
822+
//Using cursor - KO
823+
col
824+
.aggregate([{ $project: { a: 1 } }], {
825+
cursor: { batchSize: 10000 }
826+
})
827+
.forEach(
828+
function () {
829+
count = count + 1;
830+
},
831+
function (err) {
832+
expect(err).to.not.exist;
833+
expect(count).to.be.greaterThan(0);
835834

836-
client.close(done);
837-
}
838-
);
839-
});
835+
client.close(done);
836+
}
837+
);
840838
});
841839
});
842840
});

‎test/functional/collection.test.js

+47-83
Original file line numberDiff line numberDiff line change
@@ -83,35 +83,32 @@ describe('Collection', function () {
8383
db.createCollection('test.spiderman', () => {
8484
db.createCollection('test.mario', () => {
8585
// Insert test documents (creates collections)
86-
db.collection('test.spiderman', (err, spiderman_collection) => {
87-
spiderman_collection.insertOne({ foo: 5 }, configuration.writeConcernMax(), err => {
86+
const spiderman_collection = db.collection('test.spiderman');
87+
spiderman_collection.insertOne({ foo: 5 }, configuration.writeConcernMax(), err => {
88+
expect(err).to.not.exist;
89+
const mario_collection = db.collection('test.mario');
90+
mario_collection.insertOne({ bar: 0 }, configuration.writeConcernMax(), err => {
8891
expect(err).to.not.exist;
89-
db.collection('test.mario', (err, mario_collection) => {
90-
mario_collection.insertOne({ bar: 0 }, configuration.writeConcernMax(), err => {
91-
expect(err).to.not.exist;
92-
// Assert collections
93-
db.collections((err, collections) => {
94-
expect(err).to.not.exist;
92+
// Assert collections
93+
db.collections((err, collections) => {
94+
expect(err).to.not.exist;
9595

96-
let found_spiderman = false;
97-
let found_mario = false;
98-
let found_does_not_exist = false;
99-
100-
collections.forEach(collection => {
101-
if (collection.collectionName === 'test.spiderman') {
102-
found_spiderman = true;
103-
}
104-
if (collection.collectionName === 'test.mario') found_mario = true;
105-
if (collection.collectionName === 'does_not_exist')
106-
found_does_not_exist = true;
107-
});
96+
let found_spiderman = false;
97+
let found_mario = false;
98+
let found_does_not_exist = false;
10899

109-
expect(found_spiderman).to.be.true;
110-
expect(found_mario).to.be.true;
111-
expect(found_does_not_exist).to.be.false;
112-
done();
113-
});
100+
collections.forEach(collection => {
101+
if (collection.collectionName === 'test.spiderman') {
102+
found_spiderman = true;
103+
}
104+
if (collection.collectionName === 'test.mario') found_mario = true;
105+
if (collection.collectionName === 'does_not_exist') found_does_not_exist = true;
114106
});
107+
108+
expect(found_spiderman).to.be.true;
109+
expect(found_mario).to.be.true;
110+
expect(found_does_not_exist).to.be.false;
111+
done();
115112
});
116113
});
117114
});
@@ -166,23 +163,6 @@ describe('Collection', function () {
166163
});
167164
});
168165

169-
it('should ensure strict access collection', function (done) {
170-
db.collection('does-not-exist', { strict: true }, err => {
171-
expect(err).to.be.an.instanceof(Error);
172-
expect(err.message).to.equal(
173-
'Collection does-not-exist does not exist. Currently in strict mode.'
174-
);
175-
db.createCollection('test_strict_access_collection', err => {
176-
expect(err).to.not.exist;
177-
db.collection('test_strict_access_collection', configuration.writeConcernMax(), err => {
178-
expect(err).to.not.exist;
179-
// Let's close the db
180-
done();
181-
});
182-
});
183-
});
184-
});
185-
186166
it('should fail to insert due to illegal keys', function (done) {
187167
db.createCollection('test_invalid_key_names', (err, collection) => {
188168
// Legal inserts
@@ -280,39 +260,25 @@ describe('Collection', function () {
280260
});
281261

282262
it('should fail due to illegal listCollections', function (done) {
283-
db.collection(5, err => {
284-
expect(err.message).to.equal('collection name must be a String');
285-
});
286-
287-
db.collection('', err => {
288-
expect(err.message).to.equal('collection names cannot be empty');
289-
});
290-
291-
db.collection('te$t', err => {
292-
expect(err.message).to.equal("collection names must not contain '$'");
293-
});
294-
295-
db.collection('.test', err => {
296-
expect(err.message).to.equal("collection names must not start or end with '.'");
297-
});
298-
299-
db.collection('test.', err => {
300-
expect(err.message).to.equal("collection names must not start or end with '.'");
301-
});
302-
303-
db.collection('test..t', err => {
304-
expect(err.message).to.equal('collection names cannot be empty');
305-
done();
306-
});
263+
expect(() => db.collection(5)).to.throw('collection name must be a String');
264+
expect(() => db.collection('')).to.throw('collection names cannot be empty');
265+
expect(() => db.collection('te$t')).to.throw("collection names must not contain '$'");
266+
expect(() => db.collection('.test')).to.throw(
267+
"collection names must not start or end with '.'"
268+
);
269+
expect(() => db.collection('test.')).to.throw(
270+
"collection names must not start or end with '.'"
271+
);
272+
expect(() => db.collection('test..t')).to.throw('collection names cannot be empty');
273+
done();
307274
});
308275

309276
it('should correctly count on non-existent collection', function (done) {
310-
db.collection('test_multiple_insert_2', (err, collection) => {
311-
collection.countDocuments((err, count) => {
312-
expect(count).to.equal(0);
313-
// Let's close the db
314-
done();
315-
});
277+
const collection = db.collection('test_multiple_insert_2');
278+
collection.countDocuments((err, count) => {
279+
expect(count).to.equal(0);
280+
// Let's close the db
281+
done();
316282
});
317283
});
318284

@@ -351,22 +317,20 @@ describe('Collection', function () {
351317
});
352318

353319
it('should perform collection remove with no callback', function (done) {
354-
db.collection('remove_with_no_callback_bug_test', (err, collection) => {
320+
const collection = db.collection('remove_with_no_callback_bug_test');
321+
collection.insertOne({ a: 1 }, configuration.writeConcernMax(), err => {
355322
expect(err).to.not.exist;
356-
collection.insertOne({ a: 1 }, configuration.writeConcernMax(), err => {
323+
collection.insertOne({ b: 1 }, configuration.writeConcernMax(), err => {
357324
expect(err).to.not.exist;
358-
collection.insertOne({ b: 1 }, configuration.writeConcernMax(), err => {
325+
collection.insertOne({ c: 1 }, configuration.writeConcernMax(), err => {
359326
expect(err).to.not.exist;
360-
collection.insertOne({ c: 1 }, configuration.writeConcernMax(), err => {
327+
collection.remove({ a: 1 }, configuration.writeConcernMax(), err => {
361328
expect(err).to.not.exist;
362-
collection.remove({ a: 1 }, configuration.writeConcernMax(), err => {
329+
// Let's perform a count
330+
collection.countDocuments((err, count) => {
363331
expect(err).to.not.exist;
364-
// Let's perform a count
365-
collection.countDocuments((err, count) => {
366-
expect(err).to.not.exist;
367-
expect(count).to.equal(2);
368-
done();
369-
});
332+
expect(count).to.equal(2);
333+
done();
370334
});
371335
});
372336
});

‎test/functional/connection.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('Connection - functional', function () {
110110
expect(err).to.not.exist;
111111
var db = client.db(configuration.db);
112112

113-
db.collection(testName, function (err, collection) {
113+
db.createCollection(testName, function (err, collection) {
114114
expect(err).to.not.exist;
115115

116116
collection.insert({ foo: 123 }, { writeConcern: { w: 1 } }, function (err) {

0 commit comments

Comments
 (0)