Skip to content

Commit a1e0849

Browse files
committed
fix(sdam): use ObjectId comparison to track maxElectionId
Code for tracking the `maxElectionId` currently assumes that the id is represented in extended JSON. This fix modifies the test runner to parse the extended JSON into BSON, and modified the comparison logic to assume ObjectId NODE-2464
1 parent 2d1b713 commit a1e0849

File tree

2 files changed

+34
-8
lines changed

2 files changed

+34
-8
lines changed

lib/core/sdam/topology_description.js

+27-5
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,26 @@ function topologyTypeForServerType(serverType) {
279279
return TopologyType.ReplicaSetNoPrimary;
280280
}
281281

282+
function compareObjectId(oid1, oid2) {
283+
if (oid1 == null) {
284+
return -1;
285+
}
286+
287+
if (oid2 == null) {
288+
return 1;
289+
}
290+
291+
if (oid1.id instanceof Buffer && oid2.id instanceof Buffer) {
292+
const oid1Buffer = oid1.id;
293+
const oid2Buffer = oid2.id;
294+
return oid1Buffer.compare(oid2Buffer);
295+
}
296+
297+
const oid1String = oid1.toString();
298+
const oid2String = oid2.toString();
299+
return oid1String.localeCompare(oid2String);
300+
}
301+
282302
function updateRsFromPrimary(
283303
serverDescriptions,
284304
setName,
@@ -292,11 +312,13 @@ function updateRsFromPrimary(
292312
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
293313
}
294314

295-
const electionIdOID = serverDescription.electionId ? serverDescription.electionId.$oid : null;
296-
const maxElectionIdOID = maxElectionId ? maxElectionId.$oid : null;
297-
if (serverDescription.setVersion != null && electionIdOID != null) {
298-
if (maxSetVersion != null && maxElectionIdOID != null) {
299-
if (maxSetVersion > serverDescription.setVersion || maxElectionIdOID > electionIdOID) {
315+
const electionId = serverDescription.electionId ? serverDescription.electionId : null;
316+
if (serverDescription.setVersion && electionId) {
317+
if (maxSetVersion && maxElectionId) {
318+
if (
319+
maxSetVersion > serverDescription.setVersion ||
320+
compareObjectId(maxElectionId, electionId) > 0
321+
) {
300322
// this primary is stale, we must remove it
301323
serverDescriptions.set(
302324
serverDescription.address,

test/unit/core/sdam_spec.test.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const ServerDescription = require('../../../lib/core/sdam/server_description').S
77
const sdamEvents = require('../../../lib/core/sdam/events');
88
const parse = require('../../../lib/core/uri_parser');
99
const sinon = require('sinon');
10+
const EJSON = require('mongodb-extjson');
1011

1112
const chai = require('chai');
1213
chai.use(require('chai-subset'));
@@ -25,7 +26,10 @@ function collectTests() {
2526
.readdirSync(path.join(specDir, testType))
2627
.filter(f => path.extname(f) === '.json')
2728
.map(f => {
28-
const result = JSON.parse(fs.readFileSync(path.join(specDir, testType, f)));
29+
const result = EJSON.parse(fs.readFileSync(path.join(specDir, testType, f)), {
30+
relaxed: true
31+
});
32+
2933
result.type = testType;
3034
return result;
3135
});
@@ -226,7 +230,7 @@ function executeSDAMTest(testData, testDone) {
226230
const omittedFields = findOmittedFields(expectedServer);
227231

228232
const actualServer = actualServers.get(serverName);
229-
expect(actualServer).to.deep.include(expectedServer);
233+
expect(actualServer).to.matchMongoSpec(expectedServer);
230234

231235
if (omittedFields.length) {
232236
expect(actualServer).to.not.have.all.keys(omittedFields);
@@ -254,7 +258,7 @@ function executeSDAMTest(testData, testDone) {
254258
}
255259

256260
expect(description).to.include.keys(translatedKey);
257-
expect(description[translatedKey]).to.eql(outcomeValue);
261+
expect(description[translatedKey]).to.eql(outcomeValue, `(key="${translatedKey}")`);
258262
});
259263

260264
// remove error handler

0 commit comments

Comments
 (0)