Skip to content

Commit 4b6f2ff

Browse files
W-A-Jamesnbbeeken
authored andcommitted
fix(NODE-4999): Write Concern 0 Must Not Affect Read Operations (#3541)
Co-authored-by: Neal Beeken <[email protected]>
1 parent 0df03ef commit 4b6f2ff

File tree

8 files changed

+188
-83
lines changed

8 files changed

+188
-83
lines changed

src/change_stream.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,8 @@ export class ChangeStream<
586586
super();
587587

588588
this.pipeline = pipeline;
589-
this.options = options;
589+
this.options = { ...options };
590+
delete this.options.writeConcern;
590591

591592
if (parent instanceof Collection) {
592593
this.type = CHANGE_DOMAIN_TYPES.COLLECTION;

src/operations/aggregate.ts

Lines changed: 3 additions & 1 deletion
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

Lines changed: 2 additions & 1 deletion
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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ export class ListIndexesOperation extends CommandOperation<Document> {
395395
constructor(collection: Collection, options?: ListIndexesOptions) {
396396
super(collection, options);
397397

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

src/operations/list_collections.ts

Lines changed: 2 additions & 1 deletion
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

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

test/unit/operations/find.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ describe('FindOperation', function () {
2121
const operation = new FindOperation(undefined, namespace, filter, options);
2222

2323
it('sets the namespace', function () {
24-
expect(operation.ns).to.equal(namespace);
24+
expect(operation.ns).to.deep.equal(namespace);
2525
});
2626

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

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

0 commit comments

Comments
 (0)