Skip to content

Commit 6acef6d

Browse files
mbroadstdaprahamian
authored andcommitted
refactor(find): use FindOperation for finds
This changeset introduces support for using an operation for find operations. Unlike other operations, this one is still in progress in terms of becoming a proper subclass of `CommandOperationV2`. Since we must maintain support for legacy versions of the server, we cannot simply reuse the command operation.
1 parent 0f88582 commit 6acef6d

File tree

7 files changed

+79
-19
lines changed

7 files changed

+79
-19
lines changed

lib/collection.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ const checkCollectionName = require('./utils').checkCollectionName;
66
const ObjectID = require('./core').BSON.ObjectID;
77
const MongoError = require('./core').MongoError;
88
const toError = require('./utils').toError;
9-
const handleCallback = require('./utils').handleCallback;
109
const normalizeHintField = require('./utils').normalizeHintField;
1110
const decorateCommand = require('./utils').decorateCommand;
1211
const decorateWithCollation = require('./utils').decorateWithCollation;
@@ -45,6 +44,7 @@ const DropCollectionOperation = require('./operations/drop').DropCollectionOpera
4544
const DropIndexOperation = require('./operations/drop_index');
4645
const DropIndexesOperation = require('./operations/drop_indexes');
4746
const EstimatedDocumentCountOperation = require('./operations/estimated_document_count');
47+
const FindOperation = require('./operations/find');
4848
const FindOneOperation = require('./operations/find_one');
4949
const FindAndModifyOperation = require('./operations/find_and_modify');
5050
const FindOneAndDeleteOperation = require('./operations/find_one_and_delete');
@@ -428,10 +428,18 @@ Collection.prototype.find = deprecateOptions(
428428
throw err;
429429
}
430430

431-
// TODO: pass object cursor
432-
const cursor = this.s.topology.cursor(this.s.namespace.toString(), findCommand, newOptions);
431+
const cursor = this.s.topology.cursor(
432+
new FindOperation(this, this.s.namespace, findCommand, newOptions),
433+
newOptions
434+
);
435+
436+
// TODO: remove this when NODE-2074 is resolved
437+
if (typeof callback === 'function') {
438+
callback(null, cursor);
439+
return;
440+
}
433441

434-
return typeof callback === 'function' ? handleCallback(callback, null, cursor) : cursor;
442+
return cursor;
435443
}
436444
);
437445

lib/core/cursor.js

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const executeOperation = require('../operations/execute_operation');
1212
const Readable = require('stream').Readable;
1313
const SUPPORTS = require('../utils').SUPPORTS;
1414
const MongoDBNamespace = require('../utils').MongoDBNamespace;
15+
const OperationBase = require('../operations/operation').OperationBase;
1516

1617
const BSON = retrieveBSON();
1718
const Long = BSON.Long;
@@ -77,11 +78,11 @@ class CoreCursor extends Readable {
7778
super({ objectMode: true });
7879
options = options || {};
7980

80-
if (typeof ns !== 'string') {
81+
if (ns instanceof OperationBase) {
8182
this.operation = ns;
8283
ns = this.operation.ns.toString();
8384
options = this.operation.options;
84-
cmd = {};
85+
cmd = this.operation.cmd ? this.operation.cmd : {};
8586
}
8687

8788
// Cursor pool
@@ -161,6 +162,11 @@ class CoreCursor extends Readable {
161162
this.cursorState.cursorId = cmd;
162163
this.cursorState.lastCursorId = cmd;
163164
}
165+
166+
// TODO: remove as part of NODE-2104
167+
if (this.operation) {
168+
this.operation.cursorState = this.cursorState;
169+
}
164170
}
165171

166172
setCursorBatchSize(value) {
@@ -385,6 +391,11 @@ class CoreCursor extends Readable {
385391

386392
if (session && (options.force || session.owner === this)) {
387393
this.cursorState.session = undefined;
394+
395+
if (this.operation) {
396+
this.operation.clearSession();
397+
}
398+
388399
session.endSession(callback);
389400
return true;
390401
}
@@ -523,6 +534,14 @@ class CoreCursor extends Readable {
523534
};
524535

525536
if (cursor.operation) {
537+
if (cursor.logger.isDebug()) {
538+
cursor.logger.debug(
539+
`issue initial query [${JSON.stringify(cursor.cmd)}] with flags [${JSON.stringify(
540+
cursor.query
541+
)}]`
542+
);
543+
}
544+
526545
executeOperation(cursor.topology, cursor.operation, (err, result) => {
527546
if (err) {
528547
done(err);

lib/cursor.js

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,8 @@ const fields = ['numberOfRetries', 'tailableRetryInterval'];
106106
class Cursor extends CoreCursor {
107107
constructor(topology, ns, cmd, options) {
108108
super(topology, ns, cmd, options);
109-
const streamOptions = {};
110-
111-
if (typeof ns !== 'string') {
112-
this.operation = ns;
113-
ns = this.operation.ns.toString();
109+
if (this.operation) {
114110
options = this.operation.options;
115-
cmd = {};
116111
}
117112

118113
// Tailable cursor options
@@ -131,8 +126,6 @@ class Cursor extends CoreCursor {
131126
currentNumberOfRetries: currentNumberOfRetries,
132127
// State
133128
state: CursorState.INIT,
134-
// Stream options
135-
streamOptions,
136129
// Promise library
137130
promiseLibrary,
138131
// Current doc
@@ -153,8 +146,8 @@ class Cursor extends CoreCursor {
153146

154147
// Get the batchSize
155148
let batchSize = 1000;
156-
if (cmd.cursor && cmd.cursor.batchSize) {
157-
batchSize = cmd.cursor && cmd.cursor.batchSize;
149+
if (this.cmd.cursor && this.cmd.cursor.batchSize) {
150+
batchSize = this.cmd.cursor.batchSize;
158151
} else if (options.cursor && options.cursor.batchSize) {
159152
batchSize = options.cursor.batchSize;
160153
} else if (typeof options.batchSize === 'number') {
@@ -960,7 +953,9 @@ class Cursor extends CoreCursor {
960953
* @return {Promise} returns Promise if no callback passed
961954
*/
962955
explain(callback) {
963-
if (this.operation) {
956+
// NOTE: the next line includes a special case for operations which do not
957+
// subclass `CommandOperationV2`. To be removed asap.
958+
if (this.operation && this.operation.cmd == null) {
964959
this.operation.options.explain = true;
965960
return executeOperation(this.topology, this.operation, callback);
966961
}

lib/operations/find.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
'use strict';
2+
3+
const OperationBase = require('./operation').OperationBase;
4+
const Aspect = require('./operation').Aspect;
5+
const defineAspects = require('./operation').defineAspects;
6+
7+
class FindOperation extends OperationBase {
8+
constructor(collection, ns, command, options) {
9+
super(options);
10+
11+
this.ns = ns;
12+
this.cmd = command;
13+
}
14+
15+
execute(server, callback) {
16+
// copied from `CommandOperationV2`, to be subclassed in the future
17+
this.server = server;
18+
19+
const cursorState = this.cursorState || {};
20+
21+
// TOOD: use `MongoDBNamespace` through and through
22+
server.query(this.ns.toString(), this.cmd, cursorState, this.options, callback);
23+
}
24+
}
25+
26+
defineAspects(FindOperation, [
27+
Aspect.READ_OPERATION,
28+
Aspect.RETRYABLE,
29+
Aspect.EXECUTE_WITH_SELECTION,
30+
Aspect.SKIP_SESSION
31+
]);
32+
33+
module.exports = FindOperation;

test/functional/cursor_tests.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ describe('Cursor', function() {
297297
const client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1 });
298298

299299
client.connect((err, client) => {
300+
expect(err).to.not.exist;
300301
const db = client.db(configuration.db);
301302

302303
let internalClientCursor;
@@ -311,7 +312,9 @@ describe('Cursor', function() {
311312
const cursor = db.collection('countTEST').find({ qty: { $gt: 4 } });
312313
cursor.count(true, { readPreference: ReadPreference.SECONDARY }, err => {
313314
expect(err).to.be.null;
314-
expect(internalClientCursor.getCall(0).args[2])
315+
316+
const operation = internalClientCursor.getCall(0).args[0];
317+
expect(operation.options)
315318
.to.have.nested.property('readPreference')
316319
.that.deep.equals(expectedReadPreference);
317320
client.close();

test/functional/find_tests.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('Find', function() {
3636

3737
// Ensure correct insertion testing via the cursor and the count function
3838
collection.find().toArray(function(err, documents) {
39+
expect(err).to.not.exist;
3940
test.equal(2, documents.length);
4041

4142
collection.count(function(err, count) {

test/functional/retryable_reads_tests.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ describe('Retryable Reads', function() {
2525
spec.description.match(/listCollections/i) ||
2626
spec.description.match(/listCollectionNames/i) ||
2727
spec.description.match(/estimatedDocumentCount/i) ||
28-
spec.description.match(/count/i)
28+
spec.description.match(/count/i) ||
29+
spec.description.match(/find/i)
2930
);
3031
});
3132
});

0 commit comments

Comments
 (0)