Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-4999): Write Concern 0 Must Not Affect Read Operations #3541

Merged
merged 22 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9a17960
test(NODE-4999): Convert issue repro to failing test
W-A-James Jan 25, 2023
dc46642
test(NODE-4999): Got repro working
W-A-James Jan 25, 2023
704e3fc
test(NODE-4999): Add tests to check for desired behaviour
W-A-James Jan 26, 2023
fa76f2d
fix(NODE-4999): WIP fix
W-A-James Jan 26, 2023
4d7ff69
fix(NODE-4999): Start implementing fix
W-A-James Jan 27, 2023
1c7c2c2
Merge branch '4.x' into NODE-4999/write_concern_0_must_not_affect_rea…
W-A-James Jan 27, 2023
52f4cc5
fix(NODE-4999): revert unneeded type change and run eslint
W-A-James Jan 27, 2023
a74ebce
Merge branch 'NODE-4999/write_concern_0_must_not_affect_read_operatio…
W-A-James Jan 27, 2023
877cd73
test(NODE-4999): Fix import and extend timeout
W-A-James Jan 27, 2023
7f73b5b
fix(NODE-4999): simplify changes
W-A-James Jan 27, 2023
2c0a14d
test(NODE-4999): Ensure test only runs on replica sets and sharded cl…
W-A-James Jan 27, 2023
82557e3
test(NODE-4999): conditionally execute test
W-A-James Jan 27, 2023
0f06672
fix(NODE-4999): explicitly delete writeConcern key
W-A-James Jan 30, 2023
2f0a3a6
test(NODE-4999): Convert test to typescript
W-A-James Jan 30, 2023
164731c
fix(NODE-4999): ensure that copy is made of options object
W-A-James Jan 30, 2023
9ab6549
test(NODE-4999): extend timeout for changeStream test and change find…
W-A-James Jan 30, 2023
2a26d17
test(NODE-4999): Change test names and ensure that checks in find.tes…
W-A-James Jan 30, 2023
17ebd06
WIP
W-A-James Jan 31, 2023
d77a47b
test(NODE-4999): Fix for failing ci test
W-A-James Feb 1, 2023
0c44d2a
chore(NODE-4999): change mongo-client-encryption version
W-A-James Feb 1, 2023
c5050a8
test(NODE-4999): Fix changeStream test
W-A-James Feb 2, 2023
b1c0c2a
Update test/integration/read-write-concern/write_concern.test.ts
W-A-James Feb 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/change_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ export class ChangeStream<
super();

this.pipeline = pipeline;
this.options = options;
this.options = { ...options, writeConcern: undefined };

if (parent instanceof Collection) {
this.type = CHANGE_DOMAIN_TYPES.COLLECTION;
Expand Down
2 changes: 2 additions & 0 deletions src/operations/aggregate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ export class AggregateOperation<T = Document> extends CommandOperation<T> {

if (this.hasWriteStage) {
this.trySecondaryWrite = true;
} else {
this.options.writeConcern = undefined;
durran marked this conversation as resolved.
Show resolved Hide resolved
}

if (this.explain && this.writeConcern) {
Expand Down
2 changes: 1 addition & 1 deletion src/operations/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class FindOperation extends CommandOperation<Document> {
) {
super(collection, options);

this.options = options;
this.options = { ...options, writeConcern: undefined };
this.ns = ns;

if (typeof filter !== 'object' || Array.isArray(filter)) {
Expand Down
2 changes: 1 addition & 1 deletion src/operations/indexes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ export class ListIndexesOperation extends CommandOperation<Document> {
constructor(collection: Collection, options?: ListIndexesOptions) {
super(collection, options);

this.options = options ?? {};
this.options = { ...options, writeConcern: undefined };
this.collectionNamespace = collection.s.namespace;
}

Expand Down
2 changes: 1 addition & 1 deletion src/operations/list_collections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class ListCollectionsOperation extends CommandOperation<string[]> {
constructor(db: Db, filter: Document, options?: ListCollectionsOptions) {
super(db, options);

this.options = options ?? {};
this.options = { ...options, writeConcern: undefined };
this.db = db;
this.filter = filter;
this.nameOnly = !!this.options.nameOnly;
Expand Down
91 changes: 87 additions & 4 deletions test/integration/read-write-concern/write_concern.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
'use strict';

const { expect } = require('chai');
const { LEGACY_HELLO_COMMAND } = require('../../../src/constants');

const mock = require('../../tools/mongodb-mock/index');
const { MongoClient } = require('../../../src');
const { LEGACY_HELLO_COMMAND } = require('../../../src/constants');
durran marked this conversation as resolved.
Show resolved Hide resolved
const mock = require('../../tools/mongodb-mock/index');

const { setTimeout } = require('timers');

describe('Write Concern', function () {
it('should respect writeConcern from uri', function (done) {
Expand Down Expand Up @@ -72,4 +72,87 @@ describe('Write Concern', function () {
});
});
});

describe('must not affect read operations', function () {
describe('when writeConcern = 0', function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
describe('does not throw an error when getMore is called on cursor', function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
let client;
let db;
let col;

beforeEach(async function () {
client = this.configuration.newClient({ writeConcern: { w: 0 } });
await client.connect();
db = client.db('writeConcernTest');
col = db.collection('writeConcernTest');

const docs = [];
for (let i = 0; i < 100; i++) {
docs.push({ a: i, b: i + 1 });
}

await col.insertMany(docs);
});

afterEach(async function () {
await db.dropDatabase();
await client.close();
});

it('find', async function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
const findResult = col.find({}, { batchSize: 2 });
const err = await findResult.toArray().catch(e => e);

expect(err).to.not.be.instanceOf(Error);
});

it('listCollections', async function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
let collections = [];
for (let i = 0; i < 10; i++) {
collections.push(`writeConcernTestCol${i + 1}`);
}

for (const colName of collections) {
await db.createCollection(colName).catch(() => null);
}

const cols = db.listCollections({}, { batchSize: 2 });

const err = await cols.toArray().catch(e => e);

expect(err).to.not.be.instanceOf(Error);
});

it('aggregate', async function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
const aggResult = col.aggregate([{ $match: { a: { $gte: 0 } } }], { batchSize: 2 });
const err = await aggResult.toArray().catch(e => e);

expect(err).to.not.be.instanceOf(Error);
});

it('listIndexes', async function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
await col.createIndex({ a: 1 });
await col.createIndex({ b: -1 });
await col.createIndex({ a: 1, b: -1 });

const listIndexesResult = col.listIndexes({ batchSize: 2 });
const err = await listIndexesResult.toArray().catch(e => e);

expect(err).to.not.be.instanceOf(Error);
});

it('changeStream', async function () {
durran marked this conversation as resolved.
Show resolved Hide resolved
let changeStream = col.watch(undefined, { batchSize: 2 });

setTimeout(() => {
col.updateMany({}, [{ $addFields: { A: 1 } }]);
}, 500);

const err = await changeStream.next().catch(e => e);

expect(err).to.not.be.instanceOf(Error);
});
});
});
});
});