From 00dcf2d51fb27c2b259f8dbf6aedf81196c5f98c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 10 Aug 2022 13:39:50 -0400 Subject: [PATCH] fix(NODE-4159,NODE-4512): remove servers with incorrect setName from topology and fix unix socket parsing (#3348) --- src/sdam/server.ts | 9 +- src/sdam/server_description.ts | 104 +++++++----------- src/sdam/topology.ts | 2 +- src/sdam/topology_description.ts | 87 +++++++-------- src/utils.ts | 6 +- ...rver_discovery_and_monitoring.spec.test.ts | 69 ++++-------- test/unit/operations/find.test.ts | 6 +- test/unit/operations/get_more.test.ts | 18 +-- test/unit/operations/kill_cursors.test.ts | 6 +- test/unit/sdam/server_description.test.ts | 14 +++ test/unit/utils.test.ts | 57 ++++++++++ 11 files changed, 199 insertions(+), 179 deletions(-) diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 8a2d683e2f..ac18d46756 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -29,6 +29,7 @@ import { MongoNetworkError, MongoNetworkTimeoutError, MongoServerClosedError, + MongoServerError, MongoUnexpectedServerResponseError, needsRetryableWriteLabel } from '../error'; @@ -385,7 +386,7 @@ function calculateRoundTripTime(oldRtt: number, duration: number): number { return alpha * duration + (1 - alpha) * oldRtt; } -function markServerUnknown(server: Server, error?: MongoError) { +function markServerUnknown(server: Server, error?: MongoServerError) { // Load balancer servers can never be marked unknown. if (server.loadBalanced) { return; @@ -397,11 +398,7 @@ function markServerUnknown(server: Server, error?: MongoError) { server.emit( Server.DESCRIPTION_RECEIVED, - new ServerDescription(server.description.hostAddress, undefined, { - error, - topologyVersion: - error && error.topologyVersion ? error.topologyVersion : server.description.topologyVersion - }) + new ServerDescription(server.description.hostAddress, undefined, { error }) ); } diff --git a/src/sdam/server_description.ts b/src/sdam/server_description.ts index c80d59e382..1c3feeb629 100644 --- a/src/sdam/server_description.ts +++ b/src/sdam/server_description.ts @@ -1,5 +1,5 @@ import { Document, Long, ObjectId } from '../bson'; -import type { MongoError } from '../error'; +import { MongoRuntimeError, MongoServerError } from '../error'; import { arrayStrictEqual, compareObjectId, errorStrictEqual, HostAddress, now } from '../utils'; import type { ClusterTime } from './common'; import { ServerType } from './common'; @@ -31,14 +31,11 @@ export type TagSet = { [key: string]: string }; /** @internal */ export interface ServerDescriptionOptions { /** An Error used for better reporting debugging */ - error?: MongoError; + error?: MongoServerError; /** The round trip time to ping this server (in ms) */ roundTripTime?: number; - /** The topologyVersion */ - topologyVersion?: TopologyVersion; - /** If the client is in load balancing mode. */ loadBalanced?: boolean; } @@ -50,28 +47,25 @@ 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; + error: MongoServerError | null; + topologyVersion: TopologyVersion | null; minWireVersion: number; maxWireVersion: number; roundTripTime: number; lastUpdateTime: number; lastWriteDate: number; - - me?: string; - primary?: string; - setName?: string; - setVersion?: number; - electionId?: ObjectId; - logicalSessionTimeoutMinutes?: number; + me: string | null; + primary: string | null; + setName: string | null; + setVersion: number | null; + electionId: ObjectId | null; + logicalSessionTimeoutMinutes: number | null; // NOTE: does this belong here? It seems we should gossip the cluster time at the CMAP level $clusterTime?: ClusterTime; @@ -83,14 +77,19 @@ 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) { - if (typeof address === 'string') { - this._hostAddress = new HostAddress(address); - this.address = this._hostAddress.toString(); - } else { - this._hostAddress = address; - this.address = this._hostAddress.toString(); + constructor( + address: HostAddress | string, + hello?: Document, + options: ServerDescriptionOptions = {} + ) { + if (address == null || address === '') { + throw new MongoRuntimeError('ServerDescription must be provided with a non-empty address'); } + + this.address = + typeof address === 'string' + ? HostAddress.fromString(address).toString(false) // Use HostAddress to normalize + : address.toString(false); this.type = parseServerType(hello, options); this.hosts = hello?.hosts?.map((host: string) => host.toLowerCase()) ?? []; this.passives = hello?.passives?.map((host: string) => host.toLowerCase()) ?? []; @@ -101,50 +100,20 @@ export class ServerDescription { this.roundTripTime = options?.roundTripTime ?? -1; this.lastUpdateTime = now(); this.lastWriteDate = hello?.lastWrite?.lastWriteDate ?? 0; - - if (options?.topologyVersion) { - this.topologyVersion = options.topologyVersion; - } else if (hello?.topologyVersion) { - // TODO(NODE-2674): Preserve int64 sent from MongoDB - 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; - } - - 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; - } + this.error = options.error ?? null; + // TODO(NODE-2674): Preserve int64 sent from MongoDB + this.topologyVersion = this.error?.topologyVersion ?? hello?.topologyVersion ?? null; + this.setName = hello?.setName ?? null; + this.setVersion = hello?.setVersion ?? null; + this.electionId = hello?.electionId ?? null; + this.logicalSessionTimeoutMinutes = hello?.logicalSessionTimeoutMinutes ?? null; + this.primary = hello?.primary ?? null; + this.me = hello?.me?.toLowerCase() ?? null; + this.$clusterTime = hello?.$clusterTime ?? null; } get hostAddress(): HostAddress { - if (this._hostAddress) return this._hostAddress; - else return new HostAddress(this.address); + return HostAddress.fromString(this.address); } get allHosts(): string[] { @@ -181,7 +150,8 @@ export class ServerDescription { * in the {@link https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst#serverdescription|SDAM spec} */ equals(other?: ServerDescription | null): boolean { - // TODO(NODE-4510): Check ServerDescription equality logic for nullish topologyVersion meaning "greater than" + // Despite using the comparator that would determine a nullish topologyVersion as greater than + // for equality we should only always perform direct equality comparison const topologyVersionsEqual = this.topologyVersion === other?.topologyVersion || compareTopologyVersion(this.topologyVersion, other?.topologyVersion) === 0; @@ -271,8 +241,8 @@ function tagsStrictEqual(tags: TagSet, tags2: TagSet): boolean { * ``` */ export function compareTopologyVersion( - currentTv?: TopologyVersion, - newTv?: TopologyVersion + currentTv?: TopologyVersion | null, + newTv?: TopologyVersion | null ): 0 | -1 | 1 { if (currentTv == null || newTv == null) { return -1; diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 789db325af..69b82befac 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -724,7 +724,7 @@ export class Topology extends TypedEventEmitter { return this.description.commonWireVersion; } - get logicalSessionTimeoutMinutes(): number | undefined { + get logicalSessionTimeoutMinutes(): number | null { return this.description.logicalSessionTimeoutMinutes; } diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index 7a21a62606..972680afdd 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -1,6 +1,6 @@ import type { ObjectId } from '../bson'; import * as WIRE_CONSTANTS from '../cmap/wire_protocol/constants'; -import { MongoError, MongoRuntimeError } from '../error'; +import { MongoRuntimeError, MongoServerError } from '../error'; import { compareObjectId, shuffle } from '../utils'; import { ServerType, TopologyType } from './common'; import { ServerDescription } from './server_description'; @@ -32,29 +32,29 @@ export interface TopologyDescriptionOptions { */ export class TopologyDescription { type: TopologyType; - setName?: string; - maxSetVersion?: number; - maxElectionId?: ObjectId; + setName: string | null; + maxSetVersion: number | null; + maxElectionId: ObjectId | null; servers: Map; stale: boolean; compatible: boolean; compatibilityError?: string; - logicalSessionTimeoutMinutes?: number; + logicalSessionTimeoutMinutes: number | null; heartbeatFrequencyMS: number; localThresholdMS: number; - commonWireVersion?: number; + commonWireVersion: number; /** * Create a TopologyDescription */ constructor( topologyType: TopologyType, - serverDescriptions?: Map, - setName?: string, - maxSetVersion?: number, - maxElectionId?: ObjectId, - commonWireVersion?: number, - options?: TopologyDescriptionOptions + serverDescriptions: Map | null = null, + setName: string | null = null, + maxSetVersion: number | null = null, + maxElectionId: ObjectId | null = null, + commonWireVersion: number | null = null, + options: TopologyDescriptionOptions | null = null ) { options = options ?? {}; @@ -64,22 +64,10 @@ export class TopologyDescription { this.compatible = true; this.heartbeatFrequencyMS = options.heartbeatFrequencyMS ?? 0; this.localThresholdMS = options.localThresholdMS ?? 15; - - if (setName) { - this.setName = setName; - } - - if (maxSetVersion) { - this.maxSetVersion = maxSetVersion; - } - - if (maxElectionId) { - this.maxElectionId = maxElectionId; - } - - if (commonWireVersion) { - this.commonWireVersion = commonWireVersion; - } + this.setName = setName ?? null; + this.maxElectionId = maxElectionId ?? null; + this.maxSetVersion = maxSetVersion ?? null; + this.commonWireVersion = commonWireVersion ?? 0; // determine server compatibility for (const serverDescription of this.servers.values()) { @@ -108,12 +96,12 @@ export class TopologyDescription { // value among ServerDescriptions of all data-bearing server types. If any have a null // logicalSessionTimeoutMinutes, then TopologyDescription.logicalSessionTimeoutMinutes MUST be // set to null. - this.logicalSessionTimeoutMinutes = undefined; + this.logicalSessionTimeoutMinutes = null; for (const [, server] of this.servers) { if (server.isReadable) { if (server.logicalSessionTimeoutMinutes == null) { // If any of the servers have a null logicalSessionsTimeout, then the whole topology does - this.logicalSessionTimeoutMinutes = undefined; + this.logicalSessionTimeoutMinutes = null; break; } @@ -200,11 +188,6 @@ export class TopologyDescription { // potentially mutated values let { type: topologyType, setName, maxSetVersion, maxElectionId, commonWireVersion } = this; - if (serverDescription.setName && setName && serverDescription.setName !== setName) { - // TODO(NODE-4159): servers with an incorrect setName should be removed not marked Unknown - serverDescription = new ServerDescription(address, undefined); - } - const serverType = serverDescription.type; const serverDescriptions = new Map(this.servers); @@ -217,6 +200,19 @@ export class TopologyDescription { } } + if ( + typeof serverDescription.setName === 'string' && + typeof setName === 'string' && + serverDescription.setName !== setName + ) { + if (topologyType === TopologyType.Single) { + // "Single" Topology with setName mismatch is direct connection usage, mark unknown do not remove + serverDescription = new ServerDescription(address); + } else { + serverDescriptions.delete(address); + } + } + // update the actual server description serverDescriptions.set(address, serverDescription); @@ -311,7 +307,7 @@ export class TopologyDescription { ); } - get error(): MongoError | undefined { + get error(): MongoServerError | null { const descriptionsWithError = Array.from(this.servers.values()).filter( (sd: ServerDescription) => sd.error ); @@ -319,7 +315,8 @@ export class TopologyDescription { if (descriptionsWithError.length > 0) { return descriptionsWithError[0].error; } - return; + + return null; } /** @@ -366,10 +363,10 @@ function topologyTypeForServerType(serverType: ServerType): TopologyType { function updateRsFromPrimary( serverDescriptions: Map, serverDescription: ServerDescription, - setName?: string, - maxSetVersion?: number, - maxElectionId?: ObjectId -): [TopologyType, string?, number?, ObjectId?] { + setName: string | null = null, + maxSetVersion: number | null = null, + maxElectionId: ObjectId | null = null +): [TopologyType, string | null, number | null, ObjectId | null] { setName = setName || serverDescription.setName; if (setName !== serverDescription.setName) { serverDescriptions.delete(serverDescription.address); @@ -436,7 +433,7 @@ function updateRsFromPrimary( function updateRsWithPrimaryFromMember( serverDescriptions: Map, serverDescription: ServerDescription, - setName?: string + setName: string | null = null ): TopologyType { if (setName == null) { // TODO(NODE-3483): should be an appropriate runtime error @@ -456,10 +453,10 @@ function updateRsWithPrimaryFromMember( function updateRsNoPrimaryFromMember( serverDescriptions: Map, serverDescription: ServerDescription, - setName?: string -): [TopologyType, string?] { + setName: string | null = null +): [TopologyType, string | null] { const topologyType = TopologyType.ReplicaSetNoPrimary; - setName = setName || serverDescription.setName; + setName = setName ?? serverDescription.setName; if (setName !== serverDescription.setName) { serverDescriptions.delete(serverDescription.address); return [topologyType, setName]; diff --git a/src/utils.ts b/src/utils.ts index 01d42a4870..62a2bd6f88 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -627,7 +627,7 @@ export function arrayStrictEqual(arr: unknown[], arr2: unknown[]): boolean { } /** @internal */ -export function errorStrictEqual(lhs?: AnyError, rhs?: AnyError): boolean { +export function errorStrictEqual(lhs?: AnyError | null, rhs?: AnyError | null): boolean { if (lhs === rhs) { return true; } @@ -1153,9 +1153,9 @@ export class HostAddress { const escapedHost = hostString.split(' ').join('%20'); // escape spaces, for socket path hosts const { hostname, port } = new URL(`mongodb://${escapedHost}`); - if (hostname.endsWith('.sock')) { + if (escapedHost.endsWith('.sock')) { // heuristically determine if we're working with a domain socket - this.socketPath = decodeURIComponent(hostname); + this.socketPath = decodeURIComponent(escapedHost); } else if (typeof hostname === 'string') { this.isIPv6 = false; diff --git a/test/unit/assorted/server_discovery_and_monitoring.spec.test.ts b/test/unit/assorted/server_discovery_and_monitoring.spec.test.ts index b3b87cd3ea..a6564385c3 100644 --- a/test/unit/assorted/server_discovery_and_monitoring.spec.test.ts +++ b/test/unit/assorted/server_discovery_and_monitoring.spec.test.ts @@ -51,6 +51,8 @@ const SDAM_EVENT_CLASSES = { ServerHeartbeatFailedEvent } as const; +const WIRE_VERSION_KEYS = new Set(['minWireVersion', 'maxWireVersion']); + const specDir = path.resolve(__dirname, '../../spec/server-discovery-and-monitoring'); function collectTests(): Record { const testTypes = fs @@ -263,21 +265,6 @@ function convertOutcomeEvents(events) { }); } -// iterates through expectation building a path of keys that should not exist (null), and -// removes them from the expectation (NOTE: this mutates the expectation) -function findOmittedFields(expected) { - const result = []; - for (const key of Object.keys(expected)) { - if (expected[key] == null) { - result.push(key); - // TODO(NODE-4159): remove this delete - delete expected[key]; - } - } - - return result; -} - function normalizeServerDescription(serverDescription) { if (serverDescription.type === 'PossiblePrimary') { // Some single-threaded drivers care a lot about ordering potential primary @@ -469,62 +456,52 @@ function assertTopologyDescriptionOutcomeExpectations( const actualServers = description.servers; expect(actualServers).to.be.instanceOf(Map); - // TODO(NODE-4159): The node driver keeps unknown servers where it should discard - // expect(actualServers.size).to.equal(expectedServers.size); + expect(actualServers).to.have.lengthOf(expectedServers.size); for (const serverName of expectedServers.keys()) { expect(actualServers).to.include.keys(serverName); const expectedServer = expectedServers.get(serverName); + if (expectedServer == null) expect.fail(`Must have server defined for ${serverName}`); if (expectedServer.pool != null) { - const expectedPool = expectedServers.get(serverName).pool; - const actualPoolGeneration = topology.s.servers.get(serverName).s.pool; + const expectedPool = expectedServer.pool; + const actualServer = topology.s.servers.get(serverName); + if (actualServer == null) expect.fail(`Must have server defined for ${serverName}`); + const actualPoolGeneration = actualServer.s.pool; expect(actualPoolGeneration).to.have.property('generation', expectedPool.generation); delete expectedServer.pool; } const normalizedExpectedServer = normalizeServerDescription(expectedServer); - const omittedFields = findOmittedFields(normalizedExpectedServer); const actualServer = actualServers.get(serverName); - expect(actualServer).to.matchMongoSpec(normalizedExpectedServer); - if (omittedFields.length !== 0) { - // TODO(NODE-4159): There are properties that should be nulled out when the server transitions to Unknown - // instead of looking for keys to be omitted, null them out + const entriesOnExpectedServer = Object.entries(normalizedExpectedServer); + expect(entriesOnExpectedServer).to.not.be.empty; + for (const [expectedKey, expectedValue] of entriesOnExpectedServer) { + if (WIRE_VERSION_KEYS.has(expectedKey) && expectedValue === null) { + // For wireVersion keys we default to zero instead of null + expect(actualServer).to.have.property(expectedKey, 0); + } else { + expect(actualServer).to.have.deep.property(expectedKey, expectedValue); + } } } - if (outcome.setName != null) { - expect(description).to.have.property('setName', outcome.setName); - } else { - expect(description).to.not.have.property('setName'); - } - - if (outcome.maxSetVersion != null) { - expect(description).to.have.property('maxSetVersion', outcome.maxSetVersion); - } else { - expect(description).to.not.have.property('maxSetVersion'); - } - if (outcome.maxElectionId != null) { expect(description).to.have.property('maxElectionId').that.is.instanceOf(ObjectId); - const driverMaxId = description.maxElectionId.toString('hex'); + const driverMaxId = description.maxElectionId?.toString('hex'); const testMaxId = outcome.maxElectionId.toString('hex'); // Much easier to debug a hex string mismatch expect(driverMaxId).to.equal(testMaxId); } else { - expect(description).to.not.have.property('maxElectionId'); - } - - if (outcome.compatible != null) { - expect(description).to.have.property('compatible', outcome.compatible); - } else { - expect(description).to.have.property('compatible', true); + expect(description).to.have.property('maxElectionId', null); } - // logicalSessionTimeoutMinutes is always defined + expect(description).to.have.property('setName', outcome.setName ?? null); + expect(description).to.have.property('maxSetVersion', outcome.maxSetVersion ?? null); + expect(description).to.have.property('compatible', outcome.compatible ?? true); expect(description).to.have.property( 'logicalSessionTimeoutMinutes', - outcome.logicalSessionTimeoutMinutes ?? undefined + outcome.logicalSessionTimeoutMinutes ?? null ); } diff --git a/test/unit/operations/find.test.ts b/test/unit/operations/find.test.ts index f4031bcfbd..43de87b617 100644 --- a/test/unit/operations/find.test.ts +++ b/test/unit/operations/find.test.ts @@ -40,7 +40,11 @@ describe('FindOperation', function () { describe('#execute', function () { context('command construction', () => { const namespace = ns('db.collection'); - const server = new Server(new Topology([], {} as any), new ServerDescription(''), {} as any); + const server = new Server( + new Topology([], {} as any), + new ServerDescription('a:1'), + {} as any + ); it('should build basic find command with filter', async () => { const findOperation = new FindOperation(undefined, namespace, filter); diff --git a/test/unit/operations/get_more.test.ts b/test/unit/operations/get_more.test.ts index c150aae800..b85bd6f124 100644 --- a/test/unit/operations/get_more.test.ts +++ b/test/unit/operations/get_more.test.ts @@ -26,7 +26,7 @@ describe('GetMoreOperation', function () { }); describe('#constructor', function () { - const server = new Server(new Topology([], {} as any), new ServerDescription(''), {} as any); + const server = new Server(new Topology([], {} as any), new ServerDescription('a:1'), {} as any); const operation = new GetMoreOperation(namespace, cursorId, server, options); it('sets the namespace', function () { @@ -47,7 +47,7 @@ describe('GetMoreOperation', function () { it('executes a getMore on the provided server', async function () { const server = new Server( new Topology([], {} as any), - new ServerDescription(''), + new ServerDescription('a:1'), {} as any ); const opts = { ...options, documentsReturnedIn: 'nextBatch', returnFieldSelector: null }; @@ -74,12 +74,12 @@ describe('GetMoreOperation', function () { it('errors in the callback', function (done) { const server1 = new Server( new Topology([], {} as any), - new ServerDescription(''), + new ServerDescription('a:1'), {} as any ); const server2 = new Server( new Topology([], {} as any), - new ServerDescription(''), + new ServerDescription('a:1'), {} as any ); const session = sinon.createStubInstance(ClientSession); @@ -97,7 +97,11 @@ describe('GetMoreOperation', function () { context('command construction', () => { const cursorId = Long.fromBigInt(0xffff_ffffn); const namespace = ns('db.collection'); - const server = new Server(new Topology([], {} as any), new ServerDescription(''), {} as any); + const server = new Server( + new Topology([], {} as any), + new ServerDescription('a:1'), + {} as any + ); it('should build basic getMore command with cursorId and collection', async () => { const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, {}); @@ -178,7 +182,7 @@ describe('GetMoreOperation', function () { it(`${verb} set the comment on the command if the server wire version is ${state}`, async () => { const server = new Server( new Topology([], {} as any), - new ServerDescription(''), + new ServerDescription('a:1'), {} as any ); server.hello = { @@ -195,7 +199,7 @@ describe('GetMoreOperation', function () { }); describe('#hasAspect', function () { - const server = new Server(new Topology([], {} as any), new ServerDescription(''), {} as any); + const server = new Server(new Topology([], {} as any), new ServerDescription('a:1'), {} as any); const operation = new GetMoreOperation(namespace, cursorId, server, options); context('when the aspect is must select same server', function () { diff --git a/test/unit/operations/kill_cursors.test.ts b/test/unit/operations/kill_cursors.test.ts index 041cc4c356..c37ba70b64 100644 --- a/test/unit/operations/kill_cursors.test.ts +++ b/test/unit/operations/kill_cursors.test.ts @@ -18,7 +18,7 @@ describe('class KillCursorsOperation', () => { describe('constructor()', () => { const cursorId = Long.fromBigInt(0xffff_ffffn); const namespace = ns('db.collection'); - const server = new Server(new Topology([], {} as any), new ServerDescription(''), {} as any); + const server = new Server(new Topology([], {} as any), new ServerDescription('a:1'), {} as any); const options = {}; const killCursorsOperation = new KillCursorsOperation(cursorId, namespace, server, options); @@ -38,10 +38,10 @@ describe('class KillCursorsOperation', () => { describe('execute()', () => { const cursorId = Long.fromBigInt(0xffff_ffffn); const namespace = ns('db.collection'); - const server = new Server(new Topology([], {} as any), new ServerDescription(''), {} as any); + const server = new Server(new Topology([], {} as any), new ServerDescription('a:1'), {} as any); const differentServer = new Server( new Topology([], {} as any), - new ServerDescription(''), + new ServerDescription('a:1'), {} as any ); const options = {}; diff --git a/test/unit/sdam/server_description.test.ts b/test/unit/sdam/server_description.test.ts index 00fda2689c..83331a1f4c 100644 --- a/test/unit/sdam/server_description.test.ts +++ b/test/unit/sdam/server_description.test.ts @@ -1,5 +1,6 @@ import { expect } from 'chai'; +import { MongoRuntimeError } from '../../../src'; import { Long, ObjectId } from '../../../src/bson'; import { compareTopologyVersion, @@ -8,6 +9,19 @@ import { } from '../../../src/sdam/server_description'; describe('ServerDescription', function () { + describe('constructor()', () => { + it('should throw if given a null address', () => { + // @ts-expect-error: Passing nullish value to prove error will be thrown + expect(() => new ServerDescription(null)).to.throw(MongoRuntimeError); + // @ts-expect-error: Passing nullish value to prove error will be thrown + expect(() => new ServerDescription()).to.throw(MongoRuntimeError); + }); + + it('should throw if given an empty string for an address', () => { + expect(() => new ServerDescription('')).to.throw(MongoRuntimeError); + }); + }); + describe('error equality', function () { const errorEqualityTests = [ { diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index f41673d038..fc175ebf92 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -6,6 +6,7 @@ import { MongoRuntimeError } from '../../src/error'; import { BufferPool, eachAsync, + HostAddress, isHello, makeInterruptibleAsyncInterval, MongoDBNamespace, @@ -585,4 +586,60 @@ describe('driver utils', function () { }); }); }); + + describe('class HostAddress', () => { + describe('constructor()', () => { + it('should freeze itself', () => { + const ha = new HostAddress('iLoveJavascript:22'); + expect(ha).to.be.frozen; + }); + + const socketPath = '/tmp/mongodb-27017.sock'; + it('should handle decoded unix socket path', () => { + const ha = new HostAddress(socketPath); + expect(ha).to.have.property('socketPath', socketPath); + expect(ha).to.not.have.property('port'); + }); + + it('should handle encoded unix socket path', () => { + const ha = new HostAddress(encodeURIComponent(socketPath)); + expect(ha).to.have.property('socketPath', socketPath); + expect(ha).to.not.have.property('port'); + }); + + it('should handle encoded unix socket path with an unencoded space', () => { + const socketPathWithSpaces = '/tmp/some directory/mongodb-27017.sock'; + const ha = new HostAddress(socketPathWithSpaces); + expect(ha).to.have.property('socketPath', socketPathWithSpaces); + expect(ha).to.not.have.property('port'); + }); + + it('should handle unix socket path that does not begin with a slash', () => { + const socketPathWithoutSlash = 'my_local/directory/mustEndWith.sock'; + const ha = new HostAddress(socketPathWithoutSlash); + expect(ha).to.have.property('socketPath', socketPathWithoutSlash); + expect(ha).to.not.have.property('port'); + }); + + it('should only set the socketPath property on HostAddress when hostString ends in .sock', () => { + // We heuristically determine if we are using a domain socket solely based on .sock + // if someone has .sock in their hostname we will fail to connect to that host + const hostnameThatEndsWithSock = 'iLoveJavascript.sock'; + const ha = new HostAddress(hostnameThatEndsWithSock); + expect(ha).to.have.property('socketPath', hostnameThatEndsWithSock); + expect(ha).to.not.have.property('port'); + expect(ha).to.not.have.property('host'); + }); + + it('should set the host and port property on HostAddress even when hostname ends in .sock if there is a port number specified', () => { + // "should determine unix socket usage based on .sock ending" can be worked around by putting + // the port number at the end of the hostname (even if it is the default) + const hostnameThatEndsWithSockHasPort = 'iLoveJavascript.sock:27017'; + const ha = new HostAddress(hostnameThatEndsWithSockHasPort); + expect(ha).to.not.have.property('socketPath'); + expect(ha).to.have.property('host', 'iLoveJavascript.sock'.toLowerCase()); + expect(ha).to.have.property('port', 27017); + }); + }); + }); });