fixes

Author: nbbeeken

src/sdam/server_description.ts

@@ -1,8 +1,7 @@
 import { Document, Long, ObjectId } from '../bson';
 import type { MongoError } from '../error';
 import { arrayStrictEqual, errorStrictEqual, HostAddress, now } from '../utils';
-import type { ClusterTime } from './common';
-import { ServerType } from './common';
+import { ClusterTime, ServerType } from './common';
 
 const WRITABLE_SERVER_TYPES = new Set<ServerType>([
   ServerType.RSPrimary,
@@ -50,31 +49,27 @@ export interface ServerDescriptionOptions {
  * @public
  */
 export class ServerDescription {
-  private _hostAddress: HostAddress;
-  address: string;
-  type: ServerType;
-  hosts: string[];
-  passives: string[];
-  arbiters: string[];
-  tags: TagSet;
-
-  error?: MongoError;
-  topologyVersion?: TopologyVersion;
-  minWireVersion: number;
-  maxWireVersion: number;
-  roundTripTime: number;
-  lastUpdateTime: number;
-  lastWriteDate: number;
-
-  me?: string;
-  primary?: string;
-  setName?: string;
-  setVersion?: number;
-  electionId?: ObjectId;
-  logicalSessionTimeoutMinutes?: number;
-
-  // NOTE: does this belong here? It seems we should gossip the cluster time at the CMAP level
-  $clusterTime?: ClusterTime;
+  private readonly _hostAddress: HostAddress;
+  readonly address: string;
+  readonly type: ServerType;
+  readonly hosts: string[];
+  readonly passives: string[];
+  readonly arbiters: string[];
+  readonly tags: TagSet;
+  readonly error: MongoError | undefined;
+  readonly topologyVersion: TopologyVersion | undefined;
+  readonly minWireVersion: number;
+  readonly maxWireVersion: number;
+  readonly roundTripTime: number;
+  readonly lastUpdateTime: number;
+  readonly lastWriteDate: number;
+  readonly me: string | undefined;
+  readonly primary: string | undefined;
+  readonly setName: string | undefined;
+  readonly setVersion: number | undefined;
+  readonly electionId: ObjectId | undefined;
+  readonly logicalSessionTimeoutMinutes: number | undefined;
+  readonly $clusterTime: ClusterTime | undefined;
 
   /**
    * Create a ServerDescription
@@ -83,7 +78,14 @@ export class ServerDescription {
    * @param address - The address of the server
    * @param hello - An optional hello response for this server
    */
-  constructor(address: HostAddress | string, hello?: Document, options?: ServerDescriptionOptions) {
+  constructor(
+    address: HostAddress | string,
+    hello: Document = {},
+    options: ServerDescriptionOptions = {}
+  ) {
+    hello ??= {};
+    options ??= {};
+
     if (typeof address === 'string') {
       this._hostAddress = new HostAddress(address);
       this.address = this._hostAddress.toString();
@@ -102,43 +104,23 @@ export class ServerDescription {
     this.lastUpdateTime = now();
     this.lastWriteDate = hello?.lastWrite?.lastWriteDate ?? 0;
 
+    this.topologyVersion = undefined;
     if (options?.topologyVersion) {
       this.topologyVersion = options.topologyVersion;
     } else if (hello?.topologyVersion) {
       this.topologyVersion = hello.topologyVersion;
     }
 
-    if (options?.error) {
-      this.error = options.error;
-    }
-
-    if (hello?.primary) {
-      this.primary = hello.primary;
-    }
-
-    if (hello?.me) {
-      this.me = hello.me.toLowerCase();
-    }
-
-    if (hello?.setName) {
-      this.setName = hello.setName;
-    }
+    this.error = options.error;
+    this.primary = hello.primary;
+    this.me = hello.me?.toLowerCase();
+    this.setName = hello.setName;
+    this.setVersion = hello.setVersion;
+    this.electionId = hello.electionId;
+    this.logicalSessionTimeoutMinutes = hello.logicalSessionTimeoutMinutes;
+    this.$clusterTime = hello.$clusterTime;
 
-    if (hello?.setVersion) {
-      this.setVersion = hello.setVersion;
-    }
-
-    if (hello?.electionId) {
-      this.electionId = hello.electionId;
-    }
-
-    if (hello?.logicalSessionTimeoutMinutes) {
-      this.logicalSessionTimeoutMinutes = hello.logicalSessionTimeoutMinutes;
-    }
-
-    if (hello?.$clusterTime) {
-      this.$clusterTime = hello.$clusterTime;
-    }
+    // Object.freeze(this); - Mock server and tests edit serverDescriptions...
   }
 
   get hostAddress(): HostAddress {

src/sdam/topology_description.ts

@@ -31,18 +31,18 @@ export interface TopologyDescriptionOptions {
  * @public
  */
 export class TopologyDescription {
-  type: TopologyType;
-  setName: string | undefined;
-  maxSetVersion: number | undefined;
-  maxElectionId: ObjectId | undefined;
-  servers: Map<string, ServerDescription>;
-  stale: boolean;
-  compatible: boolean;
-  compatibilityError: string | undefined;
-  logicalSessionTimeoutMinutes: number | undefined;
-  heartbeatFrequencyMS: number;
-  localThresholdMS: number;
-  commonWireVersion: number | undefined;
+  readonly type: TopologyType;
+  readonly setName: string | undefined;
+  readonly maxSetVersion: number | undefined;
+  readonly maxElectionId: ObjectId | undefined;
+  readonly servers: Map<string, ServerDescription>;
+  readonly stale: boolean;
+  readonly compatible: boolean;
+  readonly compatibilityError: string | undefined;
+  readonly logicalSessionTimeoutMinutes: number | undefined;
+  readonly heartbeatFrequencyMS: number;
+  readonly localThresholdMS: number;
+  readonly commonWireVersion: number | undefined;
 
   /**
    * Create a TopologyDescription

test/integration/server-discovery-and-monitoring/electionId_precedence.test.ts

@@ -24,21 +24,23 @@ function generateMatchAndDiffSpecialCase(key, expectedObj, actualObj, metadata)
   const expected = expectedObj[key];
   const actual = actualObj[key];
 
-  if (expected === null) {
+  if (expected == null) {
     if (key === 'readConcern') {
-      // HACK: get around NODE-1889
+      // TODO(NODE-1889): In our command construction, we are automatically adding a readConcern.afterClusterTime to every command that has a session.
+      // The Causal Consistency spec only says to add afterClusterTime to read commands, and explicitly says to not add it to runCommand-like behavior.
       return {
         match: true,
         expected: SYMBOL_DOES_NOT_EXIST,
         actual: SYMBOL_DOES_NOT_EXIST
       };
     }
 
-    const match = !Object.prototype.hasOwnProperty.call(actualObj, key);
+    const actualHasKey = Object.prototype.hasOwnProperty.call(actualObj, key);
+    const actualIsNullish = actualObj[key] == null;
     return {
-      match,
-      expected: SYMBOL_DOES_NOT_EXIST,
-      actual: match ? SYMBOL_DOES_NOT_EXIST : actual
+      match: (actualHasKey && actualIsNullish) || !actualHasKey,
+      expected,
+      actual
     };
   }
 
@@ -200,4 +202,7 @@ function matchMongoSpec(chai, utils) {
   };
 }
 
-module.exports.default = matchMongoSpec;
+module.exports = {
+  default: matchMongoSpec,
+  generateMatchAndDiffMongoSpec: generateMatchAndDiff
+};

test/tools/spec-runner/matcher.js

@@ -286,7 +286,6 @@ function assertOutcomeExpectations(topology, events, outcome) {
         const omittedFields = findOmittedFields(expectedServer);
 
         const actualServer = actualServers.get(serverName);
-        // resultCheck(actualServer, expectedServer, null);
         expect(actualServer).to.matchMongoSpec(expectedServer);
 
         if (omittedFields.length) {