Skip to content

Commit

Permalink
chore: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
nbbeeken committed Jun 14, 2022
1 parent 6d28101 commit 9f011bc
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/operations/execute_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ function executeWithServerSelection<TResult>(

let selector: ReadPreference | ServerSelector;

if (operation.hasAspect(Aspect.CURSOR_ITERATING)) {
// Get more operations must always select the same server, but run through
if (operation.hasAspect(Aspect.MUST_SELECT_SAME_SERVER)) {
// GetMore and KillCursor operations must always select the same server, but run through
// server selection to potentially force monitor checks if the server is
// in an unknown state.
selector = sameServerSelector(operation.server?.description);
Expand Down
2 changes: 1 addition & 1 deletion src/operations/get_more.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,4 @@ export class GetMoreOperation extends AbstractOperation {
}
}

defineAspects(GetMoreOperation, [Aspect.READ_OPERATION, Aspect.CURSOR_ITERATING]);
defineAspects(GetMoreOperation, [Aspect.READ_OPERATION, Aspect.MUST_SELECT_SAME_SERVER]);
4 changes: 2 additions & 2 deletions src/operations/kill_cursors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ export class KillCursorsOperation extends AbstractOperation {
new MongoRuntimeError('Killcursor must run on the same server operation began on')
);
}
server.killCursors(this.ns, [this.cursorId], { session }, () => callback());
server.killCursors(this.ns, [this.cursorId], { session, noResponse: true }, () => callback());
}
}

defineAspects(KillCursorsOperation, [Aspect.CLOSING, Aspect.CURSOR_ITERATING]);
defineAspects(KillCursorsOperation, [Aspect.MUST_SELECT_SAME_SERVER]);
3 changes: 1 addition & 2 deletions src/operations/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ export const Aspect = {
EXPLAINABLE: Symbol('EXPLAINABLE'),
SKIP_COLLATION: Symbol('SKIP_COLLATION'),
CURSOR_CREATING: Symbol('CURSOR_CREATING'),
CURSOR_ITERATING: Symbol('CURSOR_ITERATING'),
CLOSING: Symbol('CLOSING')
MUST_SELECT_SAME_SERVER: Symbol('MUST_SELECT_SAME_SERVER')
} as const;

/** @public */
Expand Down
4 changes: 2 additions & 2 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ export class ClientSession extends TypedEventEmitter<ClientSessionEvents> {
if (serverSession != null) {
// release the server session back to the pool
this.sessionPool.release(serverSession);
// Make sure a new serverSession never makes it on to this ClientSession
// Make sure a new serverSession never makes it onto this ClientSession
Object.defineProperty(this, kServerSession, {
value: ServerSession.clone(serverSession),
writable: false
Expand Down Expand Up @@ -883,7 +883,7 @@ export class ServerSessionPool {

constructor(client: MongoClient) {
if (client == null) {
throw new MongoRuntimeError('ServerSessionPool requires a topology');
throw new MongoRuntimeError('ServerSessionPool requires a MongoClient');
}

this.client = client;
Expand Down
7 changes: 5 additions & 2 deletions test/integration/node-specific/mongo_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import { once } from 'events';
import * as sinon from 'sinon';

import {
CommandFailedEvent,
CommandStartedEvent,
CommandSucceededEvent,
MongoClient,
MongoNotConnectedError,
MongoServerSelectionError,
Expand Down Expand Up @@ -633,8 +636,8 @@ describe('class MongoClient', function () {
await client.db('test').command({ ping: 1 }, { session });
await session.endSession();

const startedEvents = [];
const endEvents = [];
const startedEvents: CommandStartedEvent[] = [];
const endEvents: Array<CommandFailedEvent | CommandSucceededEvent> = [];
client.on('commandStarted', event => startedEvents.push(event));
client.on('commandSucceeded', event => endEvents.push(event));
client.on('commandFailed', event => endEvents.push(event));
Expand Down
23 changes: 23 additions & 0 deletions test/integration/node-specific/topology.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const { expect } = require('chai');

describe('Topology', function () {
it('should correctly track states of a topology', {
metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, // apiVersion not supported by newTopology()
test: function (done) {
const topology = this.configuration.newTopology();

const states = [];
topology.on('stateChanged', (_, newState) => states.push(newState));
topology.connect(err => {
expect(err).to.not.exist;
topology.close(err => {
expect(err).to.not.exist;
expect(topology.isDestroyed()).to.be.true;
expect(states).to.eql(['connecting', 'connected', 'closing', 'closed']);
done();
});
});
}
});
});
4 changes: 2 additions & 2 deletions test/integration/sessions/sessions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ describe('Sessions', function () {
beforeEach(() => sinon.stub(client.topology, 'hasSessionSupport').callsFake(() => false));

it('should not send session', async () => {
const events = [];
const events: CommandStartedEvent[] = [];
client.on('commandStarted', event => events.push(event));

await collection.insertMany([{ a: 1 }, { a: 1 }]);
Expand Down Expand Up @@ -472,7 +472,7 @@ describe('Sessions', function () {
beforeEach(() => sinon.stub(client.topology, 'hasSessionSupport').callsFake(() => true));

it('should send session', async () => {
const events = [];
const events: CommandStartedEvent[] = [];
client.on('commandStarted', event => events.push(event));

await collection.insertMany([{ a: 1 }, { a: 1 }]);
Expand Down
2 changes: 1 addition & 1 deletion test/integration/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ class APMEventCollector {
}

/**
* @param {(client: MongoClient, done: Mocha.Done)} callback
* @param {(client: MongoClient, done: Mocha.Done) => void} callback
*/
function withClientV2(callback) {
return async function testFunction() {
Expand Down
2 changes: 1 addition & 1 deletion test/tools/cluster_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ elif [[ $1 == "sharded_cluster" ]]; then
echo "mongodb://bob:pwd123@localhost:51000,localhost:51001"
elif [[ $1 == "server" ]]; then
mkdir -p $SINGLE_DIR
mlaunch init --dir $SINGLE_DIR --single --setParameter enableTestCommands=1
mlaunch init --dir $SINGLE_DIR --auth --username "bob" --password "pwd123" --single --setParameter enableTestCommands=1
echo "mongodb://bob:pwd123@localhost:27017"
else
echo "unsupported topology: $1"
Expand Down
2 changes: 1 addition & 1 deletion test/unit/cursor/abstract_cursor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ConcreteCursor extends AbstractCursor {
return new ConcreteCursor(new MongoClient('mongodb://iLoveJavascript'));
}
_initialize(session: ClientSession, callback: Callback<ExecutionResult>): void {
return callback(null, { server: {} as Server, session, response: { ok: 1 } });
return callback(undefined, { server: {} as Server, session, response: { ok: 1 } });
}
}

Expand Down

0 comments on commit 9f011bc

Please sign in to comment.