From db991d6916306d1fe08508d4c3e8f7a37d7fd21f Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadst@mongodb.com>
Date: Fri, 21 Feb 2020 07:59:19 -0500
Subject: [PATCH] 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
---
 lib/core/sdam/topology_description.js | 32 ++++++++++++++++++++++-----
 test/unit/core/sdam_spec.test.js      | 10 ++++++---
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/lib/core/sdam/topology_description.js b/lib/core/sdam/topology_description.js
index ba6a2507ee..d1beb22c7c 100644
--- a/lib/core/sdam/topology_description.js
+++ b/lib/core/sdam/topology_description.js
@@ -279,6 +279,26 @@ function topologyTypeForServerType(serverType) {
   return TopologyType.ReplicaSetNoPrimary;
 }
 
+function compareObjectId(oid1, oid2) {
+  if (oid1 == null) {
+    return -1;
+  }
+
+  if (oid2 == null) {
+    return 1;
+  }
+
+  if (oid1.id instanceof Buffer && oid2.id instanceof Buffer) {
+    const oid1Buffer = oid1.id;
+    const oid2Buffer = oid2.id;
+    return oid1Buffer.compare(oid2Buffer);
+  }
+
+  const oid1String = oid1.toString();
+  const oid2String = oid2.toString();
+  return oid1String.localeCompare(oid2String);
+}
+
 function updateRsFromPrimary(
   serverDescriptions,
   setName,
@@ -292,11 +312,13 @@ function updateRsFromPrimary(
     return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
   }
 
-  const electionIdOID = serverDescription.electionId ? serverDescription.electionId.$oid : null;
-  const maxElectionIdOID = maxElectionId ? maxElectionId.$oid : null;
-  if (serverDescription.setVersion != null && electionIdOID != null) {
-    if (maxSetVersion != null && maxElectionIdOID != null) {
-      if (maxSetVersion > serverDescription.setVersion || maxElectionIdOID > electionIdOID) {
+  const electionId = serverDescription.electionId ? serverDescription.electionId : null;
+  if (serverDescription.setVersion && electionId) {
+    if (maxSetVersion && maxElectionId) {
+      if (
+        maxSetVersion > serverDescription.setVersion ||
+        compareObjectId(maxElectionId, electionId) > 0
+      ) {
         // this primary is stale, we must remove it
         serverDescriptions.set(
           serverDescription.address,
diff --git a/test/unit/core/sdam_spec.test.js b/test/unit/core/sdam_spec.test.js
index a22b5d3756..8d8eaa3f70 100644
--- a/test/unit/core/sdam_spec.test.js
+++ b/test/unit/core/sdam_spec.test.js
@@ -7,6 +7,7 @@ const ServerDescription = require('../../../lib/core/sdam/server_description').S
 const sdamEvents = require('../../../lib/core/sdam/events');
 const parse = require('../../../lib/core/uri_parser');
 const sinon = require('sinon');
+const EJSON = require('mongodb-extjson');
 
 const chai = require('chai');
 chai.use(require('chai-subset'));
@@ -25,7 +26,10 @@ function collectTests() {
       .readdirSync(path.join(specDir, testType))
       .filter(f => path.extname(f) === '.json')
       .map(f => {
-        const result = JSON.parse(fs.readFileSync(path.join(specDir, testType, f)));
+        const result = EJSON.parse(fs.readFileSync(path.join(specDir, testType, f)), {
+          relaxed: true
+        });
+
         result.type = testType;
         return result;
       });
@@ -226,7 +230,7 @@ function executeSDAMTest(testData, testDone) {
               const omittedFields = findOmittedFields(expectedServer);
 
               const actualServer = actualServers.get(serverName);
-              expect(actualServer).to.deep.include(expectedServer);
+              expect(actualServer).to.matchMongoSpec(expectedServer);
 
               if (omittedFields.length) {
                 expect(actualServer).to.not.have.all.keys(omittedFields);
@@ -254,7 +258,7 @@ function executeSDAMTest(testData, testDone) {
           }
 
           expect(description).to.include.keys(translatedKey);
-          expect(description[translatedKey]).to.eql(outcomeValue);
+          expect(description[translatedKey]).to.eql(outcomeValue, `(key="${translatedKey}")`);
         });
 
         // remove error handler