Skip to content

Commit ddfc2b9

Browse files
W-A-Jamesnbbeeken
andauthoredFeb 2, 2023
fix(NODE-4999): Write Concern 0 Must Not Affect Read Operations (#3541)
Co-authored-by: Neal Beeken <neal.beeken@mongodb.com>

File tree

9 files changed

+192
-84
lines changed

9 files changed

+192
-84
lines changed
 

‎.evergreen/run-tests.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ else
5252
source "$DRIVERS_TOOLS"/.evergreen/csfle/set-temp-creds.sh
5353
fi
5454

55-
npm install mongodb-client-encryption@"2.4.0-alpha.2"
55+
npm install mongodb-client-encryption@"2.4.0"
5656
npm install @mongodb-js/zstd
5757
npm install snappy
5858

‎src/change_stream.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,8 @@ export class ChangeStream<
594594
super();
595595

596596
this.pipeline = pipeline;
597-
this.options = options;
597+
this.options = { ...options };
598+
delete this.options.writeConcern;
598599

599600
if (parent instanceof Collection) {
600601
this.type = CHANGE_DOMAIN_TYPES.COLLECTION;

‎src/operations/aggregate.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export class AggregateOperation<T = Document> extends CommandOperation<T> {
4444
constructor(ns: MongoDBNamespace, pipeline: Document[], options?: AggregateOptions) {
4545
super(undefined, { ...options, dbName: ns.db });
4646

47-
this.options = options ?? {};
47+
this.options = { ...options };
4848

4949
// Covers when ns.collection is null, undefined or the empty string, use DB_AGGREGATE_COLLECTION
5050
this.target = ns.collection || DB_AGGREGATE_COLLECTION;
@@ -65,6 +65,8 @@ export class AggregateOperation<T = Document> extends CommandOperation<T> {
6565

6666
if (this.hasWriteStage) {
6767
this.trySecondaryWrite = true;
68+
} else {
69+
delete this.options.writeConcern;
6870
}
6971

7072
if (this.explain && this.writeConcern) {

‎src/operations/find.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ export class FindOperation extends CommandOperation<Document> {
7777
) {
7878
super(collection, options);
7979

80-
this.options = options;
80+
this.options = { ...options };
81+
delete this.options.writeConcern;
8182
this.ns = ns;
8283

8384
if (typeof filter !== 'object' || Array.isArray(filter)) {

‎src/operations/indexes.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,8 @@ export class ListIndexesOperation extends CommandOperation<Document> {
394394
constructor(collection: Collection, options?: ListIndexesOptions) {
395395
super(collection, options);
396396

397-
this.options = options ?? {};
397+
this.options = { ...options };
398+
delete this.options.writeConcern;
398399
this.collectionNamespace = collection.s.namespace;
399400
}
400401

‎src/operations/list_collections.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ export class ListCollectionsOperation extends CommandOperation<string[]> {
2828
constructor(db: Db, filter: Document, options?: ListCollectionsOptions) {
2929
super(db, options);
3030

31-
this.options = options ?? {};
31+
this.options = { ...options };
32+
delete this.options.writeConcern;
3233
this.db = db;
3334
this.filter = filter;
3435
this.nameOnly = !!this.options.nameOnly;

‎test/integration/read-write-concern/write_concern.test.js

-75
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import { expect } from 'chai';
2+
import { on, once } from 'events';
3+
4+
import { Collection } from '../../../src/collection';
5+
import { LEGACY_HELLO_COMMAND } from '../../../src/constants';
6+
import { Db } from '../../../src/db';
7+
import { MongoClient } from '../../../src/mongo_client';
8+
import * as mock from '../../tools/mongodb-mock/index';
9+
10+
describe('Write Concern', function () {
11+
it('should respect writeConcern from uri', function (done) {
12+
const client = this.configuration.newClient(
13+
`${this.configuration.url()}&w=0&monitorCommands=true`
14+
);
15+
const events: any[] = [];
16+
client.on('commandStarted', event => {
17+
if (event.commandName === 'insert') {
18+
events.push(event);
19+
}
20+
});
21+
22+
expect(client.writeConcern).to.eql({ w: 0 });
23+
client
24+
.db('test')
25+
.collection('test')
26+
.insertOne({ a: 1 }, (err, result) => {
27+
expect(err).to.not.exist;
28+
expect(result).to.exist;
29+
expect(events).to.be.an('array').with.lengthOf(1);
30+
expect(events[0]).to.containSubset({
31+
commandName: 'insert',
32+
command: {
33+
writeConcern: { w: 0 }
34+
}
35+
});
36+
client.close(done);
37+
});
38+
});
39+
40+
describe('mock server write concern test', () => {
41+
let server;
42+
before(() => {
43+
return mock.createServer().then(s => {
44+
server = s;
45+
});
46+
});
47+
48+
after(() => mock.cleanup());
49+
50+
// TODO(NODE-3816): the mock server response is looking for writeConcern on all messages, but endSessions doesn't have it
51+
it.skip('should pipe writeConcern from client down to API call', function () {
52+
server.setMessageHandler(request => {
53+
if (request.document && request.document[LEGACY_HELLO_COMMAND]) {
54+
return request.reply(mock.HELLO);
55+
}
56+
expect(request.document.writeConcern).to.exist;
57+
expect(request.document.writeConcern.w).to.equal('majority');
58+
return request.reply({ ok: 1 });
59+
});
60+
61+
const uri = `mongodb://${server.uri()}`;
62+
const client = new MongoClient(uri, { writeConcern: { w: 'majority' } });
63+
return client
64+
.connect()
65+
.then(() => {
66+
const db = client.db('wc_test');
67+
const collection = db.collection('wc');
68+
69+
return collection.insertMany([{ a: 2 }]);
70+
})
71+
.then(() => {
72+
return client.close();
73+
});
74+
});
75+
});
76+
77+
context('when performing read operations', function () {
78+
context('when writeConcern = 0', function () {
79+
describe('cursor creating operations with a getMore', function () {
80+
let client: MongoClient;
81+
let db: Db;
82+
let col: Collection;
83+
84+
beforeEach(async function () {
85+
client = this.configuration.newClient({ writeConcern: { w: 0 } });
86+
await client.connect();
87+
db = client.db('writeConcernTest');
88+
col = db.collection('writeConcernTest');
89+
90+
const docs: any[] = [];
91+
for (let i = 0; i < 100; i++) {
92+
docs.push({ a: i, b: i + 1 });
93+
}
94+
95+
await col.insertMany(docs);
96+
});
97+
98+
afterEach(async function () {
99+
await db.dropDatabase();
100+
await client.close();
101+
});
102+
103+
it('succeeds on find', async function () {
104+
const findResult = col.find({}, { batchSize: 2 });
105+
const err = await findResult.toArray().catch(e => e);
106+
107+
expect(err).to.not.be.instanceOf(Error);
108+
});
109+
110+
it('succeeds on listCollections', async function () {
111+
const collections: any[] = [];
112+
for (let i = 0; i < 10; i++) {
113+
collections.push(`writeConcernTestCol${i + 1}`);
114+
}
115+
116+
for (const colName of collections) {
117+
await db.createCollection(colName).catch(() => null);
118+
}
119+
120+
const cols = db.listCollections({}, { batchSize: 2 });
121+
122+
const err = await cols.toArray().catch(e => e);
123+
124+
expect(err).to.not.be.instanceOf(Error);
125+
});
126+
127+
it('succeeds on aggregate', async function () {
128+
const aggResult = col.aggregate([{ $match: { a: { $gte: 0 } } }], { batchSize: 2 });
129+
const err = await aggResult.toArray().catch(e => e);
130+
131+
expect(err).to.not.be.instanceOf(Error);
132+
});
133+
134+
it('succeeds on listIndexes', async function () {
135+
await col.createIndex({ a: 1 });
136+
await col.createIndex({ b: -1 });
137+
await col.createIndex({ a: 1, b: -1 });
138+
139+
const listIndexesResult = col.listIndexes({ batchSize: 2 });
140+
const err = await listIndexesResult.toArray().catch(e => e);
141+
142+
expect(err).to.not.be.instanceOf(Error);
143+
});
144+
145+
it('succeeds on changeStream', {
146+
metadata: { requires: { topology: 'replicaset' } },
147+
async test() {
148+
const changeStream = col.watch(undefined, { batchSize: 2 });
149+
const changes = on(changeStream, 'change');
150+
await once(changeStream.cursor, 'init');
151+
152+
await col.insertMany(
153+
[
154+
{ a: 10 },
155+
{ a: 10 },
156+
{ a: 10 },
157+
{ a: 10 },
158+
{ a: 10 },
159+
{ a: 10 },
160+
{ a: 10 },
161+
{ a: 10 },
162+
{ a: 10 },
163+
{ a: 10 },
164+
{ a: 10 },
165+
{ a: 10 }
166+
],
167+
{ writeConcern: { w: 'majority' } }
168+
);
169+
170+
const err = await changes.next().catch(e => e);
171+
expect(err).to.not.be.instanceOf(Error);
172+
}
173+
});
174+
});
175+
});
176+
});
177+
});

‎test/unit/operations/find.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ describe('FindOperation', function () {
2525
const operation = new FindOperation(undefined, namespace, filter, options);
2626

2727
it('sets the namespace', function () {
28-
expect(operation.ns).to.equal(namespace);
28+
expect(operation.ns).to.deep.equal(namespace);
2929
});
3030

3131
it('sets options', function () {
32-
expect(operation.options).to.equal(options);
32+
expect(operation.options).to.deep.equal(options);
3333
});
3434

3535
it('sets filter', function () {
36-
expect(operation.filter).to.equal(filter);
36+
expect(operation.filter).to.deep.equal(filter);
3737
});
3838
});
3939

0 commit comments

Comments
 (0)
Please sign in to comment.