fix(NODE-5201): prevent warning when default value for deprecation option is used (#3646)

Author: baileympearson

src/connection_string.ts

@@ -263,7 +263,7 @@ export function parseOptions(
 
   mongoOptions.hosts = isSRV ? [] : hosts.map(HostAddress.fromString);
 
-  const urlOptions = new CaseInsensitiveMap<any[]>();
+  const urlOptions = new CaseInsensitiveMap<unknown[]>();
 
   if (url.pathname !== '/' && url.pathname !== '') {
     const dbName = decodeURIComponent(
@@ -298,7 +298,7 @@ export function parseOptions(
     }
   }
 
-  const objectOptions = new CaseInsensitiveMap(
+  const objectOptions = new CaseInsensitiveMap<unknown>(
     Object.entries(options).filter(([, v]) => v != null)
   );
 
@@ -316,48 +316,42 @@ export function parseOptions(
 
   // All option collection
 
-  const allOptions = new CaseInsensitiveMap();
+  const allProvidedOptions = new CaseInsensitiveMap<unknown[]>();
 
-  const allKeys = new Set<string>([
-    ...urlOptions.keys(),
-    ...objectOptions.keys(),
-    ...DEFAULT_OPTIONS.keys()
-  ]);
+  const allProvidedKeys = new Set<string>([...urlOptions.keys(), ...objectOptions.keys()]);
 
-  for (const key of allKeys) {
+  for (const key of allProvidedKeys) {
     const values = [];
     const objectOptionValue = objectOptions.get(key);
     if (objectOptionValue != null) {
       values.push(objectOptionValue);
     }
-    const urlValue = urlOptions.get(key);
-    if (urlValue != null) {
-      values.push(...urlValue);
-    }
-    const defaultOptionsValue = DEFAULT_OPTIONS.get(key);
-    if (defaultOptionsValue != null) {
-      values.push(defaultOptionsValue);
-    }
-    allOptions.set(key, values);
+
+    const urlValues = urlOptions.get(key) ?? [];
+    values.push(...urlValues);
+    allProvidedOptions.set(key, values);
   }
 
-  if (allOptions.has('tlsCertificateKeyFile') && !allOptions.has('tlsCertificateFile')) {
-    allOptions.set('tlsCertificateFile', allOptions.get('tlsCertificateKeyFile'));
+  if (
+    allProvidedOptions.has('tlsCertificateKeyFile') &&
+    !allProvidedOptions.has('tlsCertificateFile')
+  ) {
+    allProvidedOptions.set('tlsCertificateFile', allProvidedOptions.get('tlsCertificateKeyFile'));
   }
 
-  if (allOptions.has('tls') || allOptions.has('ssl')) {
-    const tlsAndSslOpts = (allOptions.get('tls') || [])
-      .concat(allOptions.get('ssl') || [])
+  if (allProvidedOptions.has('tls') || allProvidedOptions.has('ssl')) {
+    const tlsAndSslOpts = (allProvidedOptions.get('tls') || [])
+      .concat(allProvidedOptions.get('ssl') || [])
       .map(getBoolean.bind(null, 'tls/ssl'));
     if (new Set(tlsAndSslOpts).size !== 1) {
       throw new MongoParseError('All values of tls/ssl must be the same.');
     }
   }
 
-  checkTLSOptions(allOptions);
+  checkTLSOptions(allProvidedOptions);
 
   const unsupportedOptions = setDifference(
-    allKeys,
+    allProvidedKeys,
     Array.from(Object.keys(OPTIONS)).map(s => s.toLowerCase())
   );
   if (unsupportedOptions.size !== 0) {
@@ -371,9 +365,20 @@ export function parseOptions(
   // Option parsing and setting
 
   for (const [key, descriptor] of Object.entries(OPTIONS)) {
-    const values = allOptions.get(key);
-    if (!values || values.length === 0) continue;
-    setOption(mongoOptions, key, descriptor, values);
+    const values = allProvidedOptions.get(key);
+    if (!values || values.length === 0) {
+      if (DEFAULT_OPTIONS.has(key)) {
+        setOption(mongoOptions, key, descriptor, [DEFAULT_OPTIONS.get(key)]);
+      }
+    } else {
+      const { deprecated } = descriptor;
+      if (deprecated) {
+        const deprecatedMsg = typeof deprecated === 'string' ? `: ${deprecated}` : '';
+        emitWarning(`${key} is a deprecated option${deprecatedMsg}`);
+      }
+
+      setOption(mongoOptions, key, descriptor, values);
+    }
   }
 
   if (mongoOptions.credentials) {
@@ -383,7 +388,7 @@ export function parseOptions(
     const isOidc = mongoOptions.credentials.mechanism === AuthMechanism.MONGODB_OIDC;
     if (
       (isGssapi || isX509) &&
-      allOptions.has('authSource') &&
+      allProvidedOptions.has('authSource') &&
       mongoOptions.credentials.source !== '$external'
     ) {
       // If authSource was explicitly given and its incorrect, we error
@@ -395,7 +400,7 @@ export function parseOptions(
     if (
       !(isGssapi || isX509 || isAws || isOidc) &&
       mongoOptions.dbName &&
-      !allOptions.has('authSource')
+      !allProvidedOptions.has('authSource')
     ) {
       // inherit the dbName unless GSSAPI or X509, then silently ignore dbName
       // and there was no specific authSource given
@@ -571,14 +576,9 @@ function setOption(
   descriptor: OptionDescriptor,
   values: unknown[]
 ) {
-  const { target, type, transform, deprecated } = descriptor;
+  const { target, type, transform } = descriptor;
   const name = target ?? key;
 
-  if (deprecated) {
-    const deprecatedMsg = typeof deprecated === 'string' ? `: ${deprecated}` : '';
-    emitWarning(`${key} is a deprecated option${deprecatedMsg}`);
-  }
-
   switch (type) {
     case 'boolean':
       mongoOptions[name] = getBoolean(name, values[0]);

test/integration/mongodb-handshake/mongodb-handshake.test.ts

@@ -1,5 +1,5 @@
 import { expect } from 'chai';
-import * as sinon from 'sinon';
+import Sinon, * as sinon from 'sinon';
 
 import {
   Connection,
@@ -11,6 +11,8 @@ import {
 describe('MongoDB Handshake', () => {
   let client;
 
+  afterEach(() => client.close());
+
   context('when hello is too large', () => {
     before(() => {
       sinon.stub(Connection.prototype, 'command').callsFake(function (ns, cmd, options, callback) {
@@ -42,4 +44,21 @@ describe('MongoDB Handshake', () => {
       expect(error).to.match(/client metadata document must be less/);
     });
   });
+
+  context('when compressors are provided on the mongo client', () => {
+    let spy: Sinon.SinonSpy;
+    before(() => {
+      spy = sinon.spy(Connection.prototype, 'command');
+    });
+
+    after(() => sinon.restore());
+
+    it('constructs a handshake with the specified compressors', async function () {
+      client = this.configuration.newClient({ compressors: ['snappy'] });
+      await client.connect();
+      expect(spy.called).to.be.true;
+      const handshakeDoc = spy.getCall(0).args[1];
+      expect(handshakeDoc).to.have.property('compression').to.deep.equal(['snappy']);
+    });
+  });
 });

test/tools/uri_spec_runner.ts

@@ -336,7 +336,7 @@ export function executeUriValidationTest(
       case 'compressors':
         expect(options, `${errorMessage} ${optionKey}`)
           .to.have.property(optionKey)
-          .deep.equal(optionValue.concat('none')); // TODO(NODE-3923): remove unnecessary appending
+          .deep.equal(optionValue);
         break;
       case 'replicaset': // replicaset appears with both casings in the test expectations
         expect(options, `${errorMessage} replicaSet`)

test/unit/connection_string.test.ts

@@ -14,6 +14,7 @@ import {
   MongoOptions,
   MongoParseError,
   MongoRuntimeError,
+  OPTIONS,
   parseOptions,
   resolveSRVRecord
 } from '../mongodb';
@@ -642,4 +643,36 @@ describe('Connection String', function () {
       ]);
     });
   });
+
+  context('default deprecated values', () => {
+    afterEach(() => sinon.restore());
+    before('ensure that `keepAlive` is deprecated', () => {
+      const { deprecated } = OPTIONS.keepAlive;
+      expect(deprecated).to.exist;
+    });
+    context('when no value is provided', () => {
+      it('uses the default value', () => {
+        const options = parseOptions('mongodb://localhost:27017');
+        expect(options).to.have.property('keepAlive', true);
+      });
+      it('does not emit a deprecation warning', async () => {
+        const spy = sinon.spy(process, 'emitWarning');
+        parseOptions('mongodb://localhost:27017');
+        expect(spy.called).to.be.false;
+      });
+    });
+
+    context('when a value is provided', () => {
+      it('uses the provided value', () => {
+        const options = parseOptions('mongodb://localhost:27017?keepAlive=false');
+        expect(options).to.have.property('keepAlive', false);
+      });
+      it('emits a deprecation warning', async () => {
+        const spy = sinon.spy(process, 'emitWarning');
+        parseOptions('mongodb://localhost:27017?keepAlive=false');
+        expect(spy.called, 'expected a warning to be emitted, but none was').to.be.true;
+        expect(spy.getCalls()[0].args[0]).to.match(/keepAlive is a deprecated option/);
+      });
+    });
+  });
 });

test/unit/mongo_client.test.js

@@ -488,22 +488,16 @@ describe('MongoOptions', function () {
       const clientViaOpt = new MongoClient('mongodb://localhost', {
         compressors: ['zlib', 'snappy']
       });
-      expect(clientViaOpt.options)
-        .to.have.property('compressors')
-        .deep.equal(['zlib', 'snappy', 'none']);
+      expect(clientViaOpt.options).to.have.property('compressors').deep.equal(['zlib', 'snappy']);
     });
 
     it('can be set when passed in as a comma-delimited string in the options object or URI', function () {
       const clientViaOpt = new MongoClient('mongodb://localhost', {
         compressors: 'zlib,snappy'
       });
       const clientViaUri = new MongoClient('mongodb://localhost?compressors=zlib,snappy');
-      expect(clientViaOpt.options)
-        .to.have.property('compressors')
-        .deep.equal(['zlib', 'snappy', 'none']);
-      expect(clientViaUri.options)
-        .to.have.property('compressors')
-        .deep.equal(['zlib', 'snappy', 'none']);
+      expect(clientViaOpt.options).to.have.property('compressors').deep.equal(['zlib', 'snappy']);
+      expect(clientViaUri.options).to.have.property('compressors').deep.equal(['zlib', 'snappy']);
     });
 
     it('should validate that a string or an array of strings is provided as input', function () {