Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 36 additions & 36 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -298,7 +298,7 @@ export function parseOptions(
}
}

const objectOptions = new CaseInsensitiveMap(
const objectOptions = new CaseInsensitiveMap<unknown>(
Object.entries(options).filter(([, v]) => v != null)
);

Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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]);
Expand Down
21 changes: 20 additions & 1 deletion test/integration/mongodb-handshake/mongodb-handshake.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from 'chai';
import * as sinon from 'sinon';
import Sinon, * as sinon from 'sinon';

import {
Connection,
Expand All @@ -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) {
Expand Down Expand Up @@ -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']);
});
});
});
2 changes: 1 addition & 1 deletion test/tools/uri_spec_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
33 changes: 33 additions & 0 deletions test/unit/connection_string.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
MongoOptions,
MongoParseError,
MongoRuntimeError,
OPTIONS,
parseOptions,
resolveSRVRecord
} from '../mongodb';
Expand Down Expand Up @@ -641,4 +642,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/);
});
});
});
});
12 changes: 3 additions & 9 deletions test/unit/mongo_client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down